Repository: tweag/nickel
Pull request #399 (2021-09-17 12:05:37)

Add positional information for unbound type variables


Patch:
diff --git a/src/error.rs b/src/error.rs
index dbae1397..b5b20ff0 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -215,7 +215,7 @@ pub enum ParseError {
         Option<RawSpan>,
     ),
     /// Unbound type variable
-    UnboundTypeVariables(Vec<Ident>),
+    UnboundTypeVariables(Vec<Ident>, RawSpan),
 }
 
 /// An error occurring during the resolution of an import.
@@ -358,8 +358,8 @@ impl ParseError {
                 InternalParseError::Lexical(LexicalError::InvalidAsciiEscapeCode(location)) => {
                     ParseError::InvalidAsciiEscapeCode(mk_span(file_id, location, location + 2))
                 }
-                InternalParseError::UnboundTypeVariables(idents) => {
-                    ParseError::UnboundTypeVariables(idents)
+                InternalParseError::UnboundTypeVariables(idents, span) => {
+                    ParseError::UnboundTypeVariables(idents, span)
                 }
             },
         }
@@ -1160,14 +1160,16 @@ impl ToDiagnostic<FileId> for ParseError {
                     .with_message(format!("{} parse error: {}", format, msg))
                     .with_labels(labels)
             }
-            ParseError::UnboundTypeVariables(idents) => Diagnostic::error().with_message(format!(
-                "Unbound type variable(s): {}",
-                idents
-                    .into_iter()
-                    .map(|x| format!("`{}`", x))
-                    .collect::<Vec<_>>()
-                    .join(",")
-            )),
+            ParseError::UnboundTypeVariables(idents, span) => Diagnostic::error()
+                .with_message(format!(
+                    "Unbound type variable(s): {}",
+                    idents
+                        .into_iter()
+                        .map(|x| format!("`{}`", x))
+                        .collect::<Vec<_>>()
+                        .join(",")
+                ))
+                .with_labels(vec![primary(span)]),
         };
 
         vec![diagnostic]
diff --git a/src/grammar.lalrpop b/src/grammar.lalrpop
index f7e086a5..665ccf0d 100644
--- a/src/grammar.lalrpop
+++ b/src/grammar.lalrpop
@@ -4,7 +4,7 @@ use std::ffi::OsString;
 use codespan::FileId;
 
 use super::ExtendedTerm;
-use super::utils::{StringKind, mk_pos, mk_label, strip_indent, SwitchCase,
+use super::utils::{StringKind, mk_pos, mk_label, mk_span, strip_indent, SwitchCase,
     FieldPathElem, strip_indent_doc, build_record, elaborate_field_path, check_unbound,
     ChunkLiteralPart, RecordLastField};
 use super::lexer::{Token, NormalToken, StringToken, MultiStringToken};
@@ -20,7 +20,7 @@ use crate::types::{Types, AbsType};
 grammar<'input>(src_id: FileId);
 
 WithPos<Rule>: RichTerm = <l: @L> <t: Rule> <r: @R> => t.with_pos(mk_pos(src_id, l, r));
-CheckUnbound<Rule>: Types = <r: Rule> =>? check_unbound(&r).map_err(|e| lalrpop_util::ParseError::User{error: e}).and(Ok(r));
+CheckUnbound<Rule>: Types = <l: @L> <t: Rule> <r: @R> =>? check_unbound(&t, mk_span(src_id, l, r)).map_err(|e| lalrpop_util::ParseError::User{error: e}).and(Ok(t));
 
 TypeAnnot: MetaValue = ":" <l: @L> <ty_res: CheckUnbound<Types>> <r: @R> => MetaValue {
     doc: None,
diff --git a/src/parser/error.rs b/src/parser/error.rs
index 1e3a124b..6d71bdf5 100644
--- a/src/parser/error.rs
+++ b/src/parser/error.rs
@@ -1,4 +1,5 @@
 use crate::identifier::Ident;
+use crate::position::RawSpan;
 
 #[derive(Clone, PartialEq, Debug)]
 pub enum LexicalError {
@@ -17,5 +18,5 @@ pub enum ParseError {
     /// A specific lexical error
     Lexical(LexicalError),
     /// Unbound type variable(s)
-    UnboundTypeVariables(Vec<Ident>),
+    UnboundTypeVariables(Vec<Ident>, RawSpan),
 }
diff --git a/src/parser/utils.rs b/src/parser/utils.rs
index a94eb9d2..4ea57a06 100644
--- a/src/parser/utils.rs
+++ b/src/parser/utils.rs
@@ -366,7 +366,7 @@ pub fn strip_indent_doc(doc: String) -> String {
 }
 
 /// Recursively checks for unbound type variables in a type
-pub fn check_unbound(types: &Types) -> Result<(), ParseError> {
+pub fn check_unbound(types: &Types, span: RawSpan) -> Result<(), ParseError> {
     // heavy lifting function, recurses into a type expression and returns a set of unbound vars
     fn find_unbound_vars(types: &Types, unbound_set: &mut HashSet<Ident>) {
         match &types.0 {
@@ -419,6 +419,7 @@ pub fn check_unbound(types: &Types) -> Result<(), ParseError> {
     if !unbound_set.is_empty() {
         return Err(ParseError::UnboundTypeVariables(
             unbound_set.into_iter().collect(),
+            span,
         ));
     }
 


Test patch:
diff --git a/src/parser/tests.rs b/src/parser/tests.rs
index 75708af7..e5139240 100644
--- a/src/parser/tests.rs
+++ b/src/parser/tests.rs
@@ -70,7 +70,7 @@ fn strings() {
                 mk_single_chunk("World"),
             )
             .into(),
-            mk_single_chunk("!!")
+            mk_single_chunk("!!"),
         )
         .into()
     )
@@ -87,7 +87,7 @@ fn plus() {
         Op2(
             BinaryOp::Plus(),
             Op2(BinaryOp::Plus(), Bool(true).into(), Bool(false).into()).into(),
-            Num(4.).into()
+            Num(4.).into(),
         )
         .into()
     );
@@ -169,11 +169,11 @@ fn record_terms() {
             vec![
                 (Ident::from("a"), Num(1.).into()),
                 (Ident::from("b"), Num(2.).into()),
-                (Ident::from("c"), Num(3.).into())
+                (Ident::from("c"), Num(3.).into()),
             ]
             .into_iter()
             .collect(),
-            Default::default()
+            Default::default(),
         )
         .into()
     );
@@ -361,24 +361,24 @@ fn unbound_type_variables() {
     // should fail, "a" is unbound
     assert_matches!(
         parse("1 | a"),
-        Err(ParseError::UnboundTypeVariables(unbound_vars)) if (unbound_vars.contains(&Ident("a".into())) && unbound_vars.len() == 1)
+        Err(ParseError::UnboundTypeVariables(unbound_vars, _)) if (unbound_vars.contains(&Ident("a".into())) && unbound_vars.len() == 1)
     );
 
     // should fail, "d" is unbound
     assert_matches!(
         parse("null: forall a b c. a -> (b -> List c) -> {foo : List {_ : d}, bar: b | Dyn}"),
-        Err(ParseError::UnboundTypeVariables(unbound_vars)) if (unbound_vars.contains(&Ident("d".into())) && unbound_vars.len() == 1)
+        Err(ParseError::UnboundTypeVariables(unbound_vars, _)) if (unbound_vars.contains(&Ident("d".into())) && unbound_vars.len() == 1)
     );
 
     // should fail, "e" is unbound
     assert_matches!(
         parse("null: forall a b c. a -> (b -> List c) -> {foo : List {_ : a}, bar: b | e}"),
-        Err(ParseError::UnboundTypeVariables(unbound_vars)) if (unbound_vars.contains(&Ident("e".into())) && unbound_vars.len() == 1)
+        Err(ParseError::UnboundTypeVariables(unbound_vars, _)) if (unbound_vars.contains(&Ident("e".into())) && unbound_vars.len() == 1)
     );
 
     // should fail, "a" is unbound
     assert_matches!(
         parse("null: a -> (forall a. a -> a)"),
-        Err(ParseError::UnboundTypeVariables(unbound_vars)) if (unbound_vars.contains(&Ident("a".into())) && unbound_vars.len() == 1)
+        Err(ParseError::UnboundTypeVariables(unbound_vars, _)) if (unbound_vars.contains(&Ident("a".into())) && unbound_vars.len() == 1)
     );
 }