From 70eede4fd012f49dfab0e2e27fb3a4e4bbff6325 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sat, 1 Feb 2020 22:04:34 +0000 Subject: Implement once nice error using annotate_snippets --- dhall/src/syntax/ast/span.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) (limited to 'dhall/src/syntax') diff --git a/dhall/src/syntax/ast/span.rs b/dhall/src/syntax/ast/span.rs index f9c7008..ffdd82c 100644 --- a/dhall/src/syntax/ast/span.rs +++ b/dhall/src/syntax/ast/span.rs @@ -24,6 +24,20 @@ pub enum Span { Artificial, } +impl ParsedSpan { + pub(crate) fn to_input(&self) -> String { + self.input.to_string() + } + /// Convert to a char range for consumption by annotate_snippets. + /// This compensates for https://github.com/rust-lang/annotate-snippets-rs/issues/24 + pub(crate) fn as_char_range(&self) -> (usize, usize) { + ( + char_idx_from_byte_idx(&self.input, self.start), + char_idx_from_byte_idx(&self.input, self.end), + ) + } +} + impl Span { pub(crate) fn make(input: Rc, sp: pest::Span) -> Self { Span::Parsed(ParsedSpan { @@ -79,3 +93,19 @@ impl Span { format!("{}", err) } } + +/// Convert a byte idx into a string into a char idx for consumption by annotate_snippets. +fn char_idx_from_byte_idx(input: &str, idx: usize) -> usize { + let char_idx = input + .char_indices() + .enumerate() + .find(|(_, (i, _))| *i == idx) + .unwrap() + .0; + // Unix-style newlines are counted as two chars (see + // https://github.com/rust-lang/annotate-snippets-rs/issues/24). + let nbr_newlines = input[..idx].chars().filter(|c| *c == '\n').count(); + let nbr_carriage_returns = + input[..idx].chars().filter(|c| *c == '\r').count(); + char_idx + nbr_newlines - nbr_carriage_returns +} -- cgit v1.3.1 From 4f98c23963e82eaf08c9d7291d0988d13571d337 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 2 Feb 2020 17:36:15 +0000 Subject: Fix spans for unions and records --- dhall/src/syntax/ast/expr.rs | 6 ++++ dhall/src/syntax/text/parser.rs | 35 +++++++++++----------- .../type-errors/unit/MergeHandlerNotFunction.txt | 4 +-- .../unit/MergeHandlerNotMatchAlternativeType.txt | 4 +-- 4 files changed, 28 insertions(+), 21 deletions(-) (limited to 'dhall/src/syntax') diff --git a/dhall/src/syntax/ast/expr.rs b/dhall/src/syntax/ast/expr.rs index 28a0aee..b493fdb 100644 --- a/dhall/src/syntax/ast/expr.rs +++ b/dhall/src/syntax/ast/expr.rs @@ -275,6 +275,12 @@ impl Expr { span: self.span.clone(), } } + pub fn with_span(self, span: Span) -> Self { + Expr { + kind: self.kind, + span, + } + } pub fn traverse_resolve_mut( &mut self, diff --git a/dhall/src/syntax/text/parser.rs b/dhall/src/syntax/text/parser.rs index 2ec63e2..ceae2cd 100644 --- a/dhall/src/syntax/text/parser.rs +++ b/dhall/src/syntax/text/parser.rs @@ -11,7 +11,7 @@ use crate::syntax::ExprKind::*; use crate::syntax::{ Double, FilePath, FilePrefix, Hash, ImportLocation, ImportMode, Integer, InterpolatedText, InterpolatedTextContents, Label, NaiveDouble, Natural, - Scheme, Span, UnspannedExpr, URL, V, + Scheme, Span, URL, V, }; use crate::Normalized; @@ -21,6 +21,7 @@ use crate::Normalized; // are here and hopefully you can figure out how they work. type Expr = syntax::Expr; +type UnspannedExpr = syntax::UnspannedExpr; type ParsedText = InterpolatedText; type ParsedTextContents = InterpolatedTextContents; type ParseInput<'input> = pest_consume::Node<'input, Rule, Rc>; @@ -78,13 +79,13 @@ impl crate::syntax::Builtin { fn input_to_span(input: ParseInput) -> Span { Span::make(input.user_data().clone(), input.as_pair().as_span()) } -fn spanned(input: ParseInput, x: UnspannedExpr) -> Expr { +fn spanned(input: ParseInput, x: UnspannedExpr) -> Expr { Expr::new(x, input_to_span(input)) } fn spanned_union( span1: Span, span2: Span, - x: UnspannedExpr, + x: UnspannedExpr, ) -> Expr { Expr::new(x, span1.union(&span2)) } @@ -845,25 +846,27 @@ impl DhallParser { [integer_literal(n)] => spanned(input, IntegerLit(n)), [double_quote_literal(s)] => spanned(input, TextLit(s)), [single_quote_literal(s)] => spanned(input, TextLit(s)), + [record_type_or_literal(e)] => spanned(input, e), + [union_type(e)] => spanned(input, e), [expression(e)] => e, )) } - #[alias(expression)] - fn empty_record_literal(input: ParseInput) -> ParseResult { - Ok(spanned(input, RecordLit(Default::default()))) + #[alias(record_type_or_literal)] + fn empty_record_literal(input: ParseInput) -> ParseResult { + Ok(RecordLit(Default::default())) } - #[alias(expression)] - fn empty_record_type(input: ParseInput) -> ParseResult { - Ok(spanned(input, RecordType(Default::default()))) + #[alias(record_type_or_literal)] + fn empty_record_type(input: ParseInput) -> ParseResult { + Ok(RecordType(Default::default())) } - #[alias(expression)] + #[alias(record_type_or_literal)] fn non_empty_record_type_or_literal( input: ParseInput, - ) -> ParseResult { - let e = match_nodes!(input.children(); + ) -> ParseResult { + Ok(match_nodes!(input.children(); [label(first_label), non_empty_record_type(rest)] => { let (first_expr, mut map) = rest; map.insert(first_label, first_expr); @@ -874,8 +877,7 @@ impl DhallParser { map.insert(first_label, first_expr); RecordLit(map) }, - ); - Ok(spanned(input, e)) + )) } fn non_empty_record_type( @@ -910,13 +912,12 @@ impl DhallParser { )) } - #[alias(expression)] - fn union_type(input: ParseInput) -> ParseResult { + fn union_type(input: ParseInput) -> ParseResult { let map = match_nodes!(input.children(); [empty_union_type(_)] => Default::default(), [union_type_entry(entries)..] => entries.collect(), ); - Ok(spanned(input, UnionType(map))) + Ok(UnionType(map)) } fn empty_union_type(_input: ParseInput) -> ParseResult<()> { diff --git a/dhall/tests/type-errors/unit/MergeHandlerNotFunction.txt b/dhall/tests/type-errors/unit/MergeHandlerNotFunction.txt index 16931a8..3a12c06 100644 --- a/dhall/tests/type-errors/unit/MergeHandlerNotFunction.txt +++ b/dhall/tests/type-errors/unit/MergeHandlerNotFunction.txt @@ -3,7 +3,7 @@ Type error: Unhandled error: error: Handler is not a function | 1 | merge { x = True } (< x : Bool >.x True) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this merge expression - | ^^^^^^^^ the handler `x` had type: `Bool` - | ----------------- help: the corresponding variant has type: `Bool` + | ^^^^^^^^^^^^ the handler `x` had type: `Bool` + | ------------------- help: the corresponding variant has type: `Bool` | = help: a handler for this variant must be a function that takes an input of type: `Bool` diff --git a/dhall/tests/type-errors/unit/MergeHandlerNotMatchAlternativeType.txt b/dhall/tests/type-errors/unit/MergeHandlerNotMatchAlternativeType.txt index 1762a7c..2afbc78 100644 --- a/dhall/tests/type-errors/unit/MergeHandlerNotMatchAlternativeType.txt +++ b/dhall/tests/type-errors/unit/MergeHandlerNotMatchAlternativeType.txt @@ -3,7 +3,7 @@ Type error: Unhandled error: error: Wrong handler input type | 1 | merge { x = λ(_ : Bool) → _ } (< x : Natural >.x 1) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ in this merge expression - | ^^^^^^^^^^^^^^^^^^^ the handler `x` expects a value of type: `Bool` - | ^^^^^^^^^^^^^^^^^ but the corresponding variant has type: `Natural` + | ^^^^^^^^^^^^^^^^^^^^^^^ the handler `x` expects a value of type: `Bool` + | ^^^^^^^^^^^^^^^^^^^ but the corresponding variant has type: `Natural` | = help: only functions can be applied to -- cgit v1.3.1 From b6625eccbf3f2d1bcfe1a88f4d556439281e91de Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 2 Feb 2020 17:49:49 +0000 Subject: Use Spans consistently by value --- dhall/src/error/builder.rs | 10 +++++----- dhall/src/semantics/nze/value.rs | 6 +++--- dhall/src/semantics/tck/tyexpr.rs | 4 ++-- dhall/src/semantics/tck/typecheck.rs | 16 ++++++++-------- dhall/src/syntax/text/parser.rs | 6 +----- 5 files changed, 19 insertions(+), 23 deletions(-) (limited to 'dhall/src/syntax') diff --git a/dhall/src/error/builder.rs b/dhall/src/error/builder.rs index 29d0d35..8c7eef3 100644 --- a/dhall/src/error/builder.rs +++ b/dhall/src/error/builder.rs @@ -63,7 +63,7 @@ impl ErrorBuilder { consumed: false, } } - pub fn new_span_err(span: &Span, message: impl ToString) -> Self { + pub fn new_span_err(span: Span, message: impl ToString) -> Self { let message = message.to_string(); let mut builder = Self::new(message.clone()); builder.span_err(span, message); @@ -72,7 +72,7 @@ impl ErrorBuilder { pub fn span_annot( &mut self, - span: &Span, + span: Span, message: impl ToString, annotation_type: AnnotationType, ) -> &mut Self { @@ -82,7 +82,7 @@ impl ErrorBuilder { _ => return self, }; self.annotations.push(SpannedAnnotation { - span: span.clone(), + span, message: message.to_string(), annotation_type, }); @@ -102,14 +102,14 @@ impl ErrorBuilder { pub fn span_err( &mut self, - span: &Span, + span: Span, message: impl ToString, ) -> &mut Self { self.span_annot(span, message, AnnotationType::Error) } pub fn span_help( &mut self, - span: &Span, + span: Span, message: impl ToString, ) -> &mut Self { self.span_annot(span, message, AnnotationType::Help) diff --git a/dhall/src/semantics/nze/value.rs b/dhall/src/semantics/nze/value.rs index d97b8c4..1143781 100644 --- a/dhall/src/semantics/nze/value.rs +++ b/dhall/src/semantics/nze/value.rs @@ -174,9 +174,9 @@ impl Value { _ => None, } } - pub(crate) fn span(&self) -> Span { - self.0.span.clone() - } + // pub(crate) fn span(&self) -> Span { + // self.0.span.clone() + // } /// This is what you want if you want to pattern-match on the value. /// WARNING: drop this ref before normalizing the same value or you will run into BorrowMut diff --git a/dhall/src/semantics/tck/tyexpr.rs b/dhall/src/semantics/tck/tyexpr.rs index 1a048f9..1886646 100644 --- a/dhall/src/semantics/tck/tyexpr.rs +++ b/dhall/src/semantics/tck/tyexpr.rs @@ -48,8 +48,8 @@ impl TyExpr { pub fn kind(&self) -> &TyExprKind { &*self.kind } - pub fn span(&self) -> &Span { - &self.span + pub fn span(&self) -> Span { + self.span.clone() } pub fn get_type(&self) -> Result { match &self.ty { diff --git a/dhall/src/semantics/tck/typecheck.rs b/dhall/src/semantics/tck/typecheck.rs index 48794a8..7518e90 100644 --- a/dhall/src/semantics/tck/typecheck.rs +++ b/dhall/src/semantics/tck/typecheck.rs @@ -76,7 +76,7 @@ fn type_one_layer( annot.get_type()?.to_expr_tyenv(env), )) .span_err( - &annot.span(), + annot.span(), format!( "this has type: `{}`", annot.get_type()?.to_expr_tyenv(env) @@ -503,11 +503,11 @@ fn type_one_layer( "Wrong handler input type" )) .span_err( - &span, + span, format!("in this merge expression",), ) .span_err( - &record.span(), + record.span(), format!( "the handler `{}` expects a value \ of type: `{}`", @@ -516,7 +516,7 @@ fn type_one_layer( ), ) .span_err( - &union.span(), + union.span(), format!( "but the corresponding variant \ has type: `{}`", @@ -538,11 +538,11 @@ fn type_one_layer( "Handler is not a function" )) .span_err( - &span, + span, format!("in this merge expression",), ) .span_err( - &record.span(), + record.span(), format!( "the handler `{}` had type: `{}`", x, @@ -550,7 +550,7 @@ fn type_one_layer( ), ) .span_help( - &union.span(), + union.span(), format!( "the corresponding variant has type: \ `{}`", @@ -647,7 +647,7 @@ pub(crate) fn type_with( None => { return mkerr( ErrorBuilder::new(format!("unbound variable `{}`", var)) - .span_err(&expr.span(), "not found in this scope") + .span_err(expr.span(), "not found in this scope") .format(), ) } diff --git a/dhall/src/syntax/text/parser.rs b/dhall/src/syntax/text/parser.rs index ceae2cd..8d571c0 100644 --- a/dhall/src/syntax/text/parser.rs +++ b/dhall/src/syntax/text/parser.rs @@ -82,11 +82,7 @@ fn input_to_span(input: ParseInput) -> Span { fn spanned(input: ParseInput, x: UnspannedExpr) -> Expr { Expr::new(x, input_to_span(input)) } -fn spanned_union( - span1: Span, - span2: Span, - x: UnspannedExpr, -) -> Expr { +fn spanned_union(span1: Span, span2: Span, x: UnspannedExpr) -> Expr { Expr::new(x, span1.union(&span2)) } -- cgit v1.3.1