diff --git a/src/def/lower.rs b/src/def/lower.rs index b1a2631..9f3df6a 100644 --- a/src/def/lower.rs +++ b/src/def/lower.rs @@ -2,26 +2,16 @@ use super::{ AstPtr, Attrpath, BindingKey, BindingValue, Bindings, Expr, ExprId, Literal, Module, ModuleSourceMap, NameDef, NameDefId, Pat, Path, PathAnchor, }; -use crate::{Diagnostic, FileId, FileRange, InFile}; +use crate::{Diagnostic, DiagnosticKind, FileId, InFile}; use indexmap::IndexMap; use la_arena::Arena; use rowan::ast::AstNode; -use rowan::TextRange; use smol_str::SmolStr; use std::str; use syntax::ast::{self, HasBindings, HasStringParts, LiteralKind}; -use syntax::Parse; +use syntax::{Parse, TextRange}; pub(super) fn lower(parse: InFile) -> (Module, ModuleSourceMap) { - let diagnostics = parse - .value - .errors() - .iter() - .map(|&(err, pos)| { - Diagnostic::SyntaxError(InFile::new(parse.file_id, TextRange::empty(pos)), err) - }) - .collect(); - let mut ctx = LowerCtx { file_id: parse.file_id, module: Module { @@ -29,7 +19,7 @@ pub(super) fn lower(parse: InFile) -> (Module, ModuleSourceMap) { name_defs: Arena::new(), // Placeholder. entry_expr: ExprId::from_raw(0.into()), - diagnostics, + diagnostics: Vec::new(), }, source_map: ModuleSourceMap::default(), }; @@ -61,12 +51,8 @@ impl LowerCtx { id } - fn file_range(&self, ptr: &AstPtr) -> FileRange { - InFile::new(self.file_id, ptr.text_range()) - } - - fn push_diagnostic(&mut self, diag: Diagnostic) { - self.module.diagnostics.push(diag); + fn diagnostic(&mut self, range: TextRange, kind: DiagnosticKind) { + self.module.diagnostics.push(Diagnostic { range, kind }); } fn lower_name(&mut self, node: ast::Name) -> NameDefId { @@ -175,8 +161,7 @@ impl LowerCtx { for (key, _) in bindings.entries.iter() { if let BindingKey::Dynamic(expr) = key { if let Some(ptr) = self.source_map.expr_node(*expr) { - let range = self.file_range(&ptr); - self.push_diagnostic(Diagnostic::InvalidDynamic(range)); + self.diagnostic(ptr.text_range(), DiagnosticKind::InvalidDynamic); } } } @@ -413,7 +398,7 @@ impl MergingSet { let key = match ctx.lower_key(self.is_rec, attr) { // `inherit ${expr}` or `inherit (expr) ${expr}` is invalid. BindingKey::Dynamic(expr) => { - ctx.push_diagnostic(Diagnostic::InvalidDynamic(ctx.file_range(&ptr))); + ctx.diagnostic(ptr.text_range(), DiagnosticKind::InvalidDynamic); self.recover_error(ctx, expr, ptr.clone()); continue; } @@ -537,22 +522,20 @@ impl MergingValue { } fn emit_duplicated_key(&self, ctx: &mut LowerCtx, ptr: &AstPtr) { - let cur_range = ctx.file_range(ptr); + ctx.diagnostic(ptr.text_range(), DiagnosticKind::DuplicatedKey); let prev_range = match self { Self::Placeholder => unreachable!(), - Self::Attrset(set) => ctx.file_range(&set.ptr), + Self::Attrset(set) => set.ptr.text_range(), Self::Final(BindingValue::Inherit(prev_expr) | BindingValue::Expr(prev_expr)) => { match ctx.source_map.expr_node(*prev_expr) { - Some(ptr) => ctx.file_range(&ptr), + Some(ptr) => ptr.text_range(), None => return, } } - Self::Final(BindingValue::InheritFrom(_)) => { - // FIXME: Cannot get inherit from attrs. - cur_range - } + // FIXME: Cannot get inherit from attrs. + Self::Final(BindingValue::InheritFrom(_)) => return, }; - ctx.push_diagnostic(Diagnostic::DuplicatedKey(prev_range, cur_range)); + ctx.diagnostic(prev_range, DiagnosticKind::DuplicatedKey); } fn finish(self, ctx: &mut LowerCtx) -> BindingValue { @@ -987,19 +970,19 @@ mod tests { check_error( "let ${a} = 1; in 1", expect![[r#" - InvalidDynamic(InFile { file_id: FileId(0), value: 6..7 }) + Diagnostic { range: 6..7, kind: InvalidDynamic } "#]], ); check_error( "{ inherit ${a}; }", expect![[r#" - InvalidDynamic(InFile { file_id: FileId(0), value: 10..14 }) + Diagnostic { range: 10..14, kind: InvalidDynamic } "#]], ); check_error( "{ inherit (a) ${a}; }", expect![[r#" - InvalidDynamic(InFile { file_id: FileId(0), value: 14..18 }) + Diagnostic { range: 14..18, kind: InvalidDynamic } "#]], ); } @@ -1011,28 +994,46 @@ mod tests { check_error( "{ a = 1; a = 2; }", expect![[r#" - DuplicatedKey(InFile { file_id: FileId(0), value: 6..7 }, InFile { file_id: FileId(0), value: 13..14 }) + Diagnostic { range: 13..14, kind: DuplicatedKey } + Diagnostic { range: 6..7, kind: DuplicatedKey } "#]], ); // Set and value. check_error( "{ a.b = 1; a = 2; }", expect![[r#" - DuplicatedKey(InFile { file_id: FileId(0), value: 4..5 }, InFile { file_id: FileId(0), value: 15..16 }) + Diagnostic { range: 15..16, kind: DuplicatedKey } + Diagnostic { range: 4..5, kind: DuplicatedKey } "#]], ); // Value and set. check_error( "{ a = 1; a.b = 2; }", expect![[r#" - DuplicatedKey(InFile { file_id: FileId(0), value: 6..7 }, InFile { file_id: FileId(0), value: 11..12 }) + Diagnostic { range: 11..12, kind: DuplicatedKey } + Diagnostic { range: 6..7, kind: DuplicatedKey } "#]], ); // Inherit and value. check_error( "{ inherit a; a = 1; }", expect![[r#" - DuplicatedKey(InFile { file_id: FileId(0), value: 10..11 }, InFile { file_id: FileId(0), value: 17..18 }) + Diagnostic { range: 17..18, kind: DuplicatedKey } + Diagnostic { range: 10..11, kind: DuplicatedKey } + "#]], + ); + } + + #[test] + // FIXME: Errors are duplicated. + fn attrset_many_duplicated_error() { + check_error( + "{ a = 1; a = 2; a = 3; }", + expect![[r#" + Diagnostic { range: 13..14, kind: DuplicatedKey } + Diagnostic { range: 6..7, kind: DuplicatedKey } + Diagnostic { range: 20..21, kind: DuplicatedKey } + Diagnostic { range: 6..7, kind: DuplicatedKey } "#]], ); } diff --git a/src/diagnostic.rs b/src/diagnostic.rs index 1367734..bc98143 100644 --- a/src/diagnostic.rs +++ b/src/diagnostic.rs @@ -1,8 +1,55 @@ -use crate::FileRange; +use syntax::{ErrorKind as SynErrorKind, TextRange}; -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum Diagnostic { - SyntaxError(FileRange, syntax::Error), - InvalidDynamic(FileRange), - DuplicatedKey(FileRange, FileRange), +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Diagnostic { + pub range: TextRange, + pub kind: DiagnosticKind, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum DiagnosticKind { + SyntaxError(SynErrorKind), + InvalidDynamic, + DuplicatedKey, +} + +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum Severity { + Error, + IncompleteSyntax, +} + +impl Diagnostic { + pub fn severity(&self) -> Severity { + match self.kind { + DiagnosticKind::SyntaxError(kind) => match kind { + SynErrorKind::MultipleRoots + | SynErrorKind::PathTrailingSlash + | SynErrorKind::PathDuplicatedSlashes + | SynErrorKind::MultipleNoAssoc => Severity::Error, + SynErrorKind::UnexpectedToken + | SynErrorKind::MissingToken(_) + | SynErrorKind::MissingExpr + | SynErrorKind::MissingAttr => Severity::IncompleteSyntax, + }, + DiagnosticKind::InvalidDynamic | DiagnosticKind::DuplicatedKey => Severity::Error, + } + } + + pub fn message(&self) -> String { + match self.kind { + DiagnosticKind::SyntaxError(kind) => kind.to_string(), + DiagnosticKind::InvalidDynamic => "Invalid location of dynamic attribute".into(), + DiagnosticKind::DuplicatedKey => "Duplicated name definition".into(), + } + } +} + +impl From for Diagnostic { + fn from(err: syntax::Error) -> Self { + Self { + range: err.range, + kind: DiagnosticKind::SyntaxError(err.kind), + } + } } diff --git a/src/lib.rs b/src/lib.rs index 75348ce..c20c28d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,7 +8,7 @@ mod ide; mod tests; pub use base::{Change, FileId, FilePos, FileRange, InFile}; -pub use diagnostic::Diagnostic; +pub use diagnostic::{Diagnostic, DiagnosticKind}; pub use ide::{ Analysis, AnalysisHost, CompletionItem, CompletionItemKind, NavigationTarget, RootDatabase, }; diff --git a/syntax/src/lib.rs b/syntax/src/lib.rs index d06dbf2..403f3ed 100644 --- a/syntax/src/lib.rs +++ b/syntax/src/lib.rs @@ -16,7 +16,13 @@ pub use self::kind::SyntaxKind; pub use self::parser::{parse_file, Parse}; #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub enum Error { +pub struct Error { + pub range: TextRange, + pub kind: ErrorKind, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum ErrorKind { MultipleRoots, UnexpectedToken, MultipleNoAssoc, @@ -27,7 +33,7 @@ pub enum Error { PathDuplicatedSlashes, } -impl fmt::Display for Error { +impl fmt::Display for ErrorKind { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::MultipleRoots => "Multiple root expressions", @@ -43,7 +49,19 @@ impl fmt::Display for Error { } } -impl std::error::Error for Error {} +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "{} at {}..{}", + self.kind, + u32::from(self.range.start()), + u32::from(self.range.end()), + ) + } +} + +impl std::error::Error for ErrorKind {} #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub enum NixLanguage {} diff --git a/syntax/src/parser.rs b/syntax/src/parser.rs index ca2eb99..350709a 100644 --- a/syntax/src/parser.rs +++ b/syntax/src/parser.rs @@ -1,13 +1,13 @@ use crate::ast::SourceFile; use crate::SyntaxKind::{self, *}; -use crate::{lexer, Error, SyntaxNode}; +use crate::{lexer, Error, ErrorKind, SyntaxNode}; use rowan::ast::AstNode; -use rowan::{Checkpoint, GreenNode, GreenNodeBuilder, TextSize}; +use rowan::{Checkpoint, GreenNode, GreenNodeBuilder, TextRange, TextSize}; #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct Parse { green: GreenNode, - errors: Vec<(Error, TextSize)>, + errors: Vec, } impl Parse { @@ -23,7 +23,7 @@ impl Parse { SyntaxNode::new_root(self.green.clone()) } - pub fn errors(&self) -> &[(Error, TextSize)] { + pub fn errors(&self) -> &[Error] { &self.errors } } @@ -44,7 +44,7 @@ pub fn parse_file(src: &str) -> Parse { struct Parser<'i> { tokens: lexer::LexTokens, builder: GreenNodeBuilder<'static>, - errors: Vec<(Error, TextSize)>, + errors: Vec, src: &'i str, } @@ -55,7 +55,7 @@ impl<'i> Parser<'i> { self.expr_function_opt(); while self.peek_non_ws().is_some() { // Tolerate multiple exprs and just emit errors. - self.error(Error::MultipleRoots); + self.error(ErrorKind::MultipleRoots); let prev = self.tokens.len(); self.expr_function_opt(); @@ -72,13 +72,13 @@ impl<'i> Parser<'i> { } } - fn error(&mut self, err: Error) { - let loc = self + fn error(&mut self, kind: ErrorKind) { + let range = self .tokens .last() - .map(|(_, range)| range.start()) - .unwrap_or_else(|| (self.src.len() as u32).into()); - self.errors.push((err, loc)); + .map(|&(_, range)| range) + .unwrap_or_else(|| TextRange::empty(TextSize::from(self.src.len() as u32))); + self.errors.push(Error { range, kind }); } fn checkpoint(&mut self) -> Checkpoint { @@ -135,7 +135,7 @@ impl<'i> Parser<'i> { if self.peek_non_ws() == Some(expect) { self.bump(); } else { - self.error(Error::MissingToken(expect)); + self.error(ErrorKind::MissingToken(expect)); } } @@ -148,11 +148,11 @@ impl<'i> Parser<'i> { break; } Some(_) => { - self.error(Error::UnexpectedToken); + self.error(ErrorKind::UnexpectedToken); self.bump(); } None => { - self.error(Error::MissingToken(expect)); + self.error(ErrorKind::MissingToken(expect)); break; } } @@ -194,7 +194,7 @@ impl<'i> Parser<'i> { if matches!(self.iter_non_ws().nth(1), Some(T!['{'])) { self.expr_operator_opt() } else { - self.error(Error::UnexpectedToken); + self.error(ErrorKind::UnexpectedToken); self.bump(); } } @@ -269,7 +269,7 @@ impl<'i> Parser<'i> { if self.peek_non_ws() == Some(T!['{']) { self.pat(); } else { - self.error(Error::MissingToken(T!['{'])); + self.error(ErrorKind::MissingToken(T!['{'])); } } self.finish_node(); @@ -326,7 +326,7 @@ impl<'i> Parser<'i> { // Tolerate incorrect usage of no associative operators, and just mark them. while matches!(self.peek_non_ws(), Some(k) if ops.contains(&k)) { - self.error(Error::MultipleNoAssoc); + self.error(ErrorKind::MultipleNoAssoc); self.bump(); next(self); } @@ -468,7 +468,7 @@ impl<'i> Parser<'i> { self.finish_node(); // Otherwise, simply skip one token to help better recovery. } else { - self.error(Error::UnexpectedToken); + self.error(ErrorKind::UnexpectedToken); } } Some(T!['{']) => { @@ -492,11 +492,11 @@ impl<'i> Parser<'i> { continue; } Some(_) => { - self.error(Error::UnexpectedToken); + self.error(ErrorKind::UnexpectedToken); self.bump(); } None => { - self.error(Error::MissingToken(T![']'])); + self.error(ErrorKind::MissingToken(T![']'])); break; } } @@ -504,7 +504,7 @@ impl<'i> Parser<'i> { self.finish_node(); } _ => { - self.error(Error::UnexpectedToken); + self.error(ErrorKind::UnexpectedToken); } } } @@ -539,18 +539,24 @@ impl<'i> Parser<'i> { Some((PATH_FRAGMENT | PATH, range)) => *range, _ => unreachable!(), }; - let mut last_is_slash = false; - for (ch, i) in self.src[range].chars().zip(0u32..) { - let cur_is_slash = ch == '/'; - if last_is_slash && cur_is_slash { - let pos = range.start() + TextSize::from(i); - self.errors.push((Error::PathDuplicatedSlashes, pos)); - } - last_is_slash = cur_is_slash; - } - if !allow_trailing_slash && last_is_slash { - let pos = range.end() - TextSize::from(1); - self.errors.push((Error::PathTrailingSlash, pos)); + // N.B. Path tokens are ASCII-only, which are verified by the lexer. + self.src[range] + .as_bytes() + .windows(2) + .zip(1u32..) + .filter(|(w, _)| w == b"//") + .for_each(|(_, i)| { + self.errors.push(Error { + range: TextRange::at(range.start() + TextSize::from(i), 1.into()), + kind: ErrorKind::PathDuplicatedSlashes, + }); + }); + let last_char = TextRange::at(range.end() - TextSize::from(1), 1.into()); + if !allow_trailing_slash && &self.src[last_char] == "/" { + self.errors.push(Error { + range: last_char, + kind: ErrorKind::PathTrailingSlash, + }); } } @@ -565,7 +571,7 @@ impl<'i> Parser<'i> { if expect_delimiter { self.bump(); } else { - self.error(Error::UnexpectedToken); + self.error(ErrorKind::UnexpectedToken); } continue; } @@ -582,7 +588,7 @@ impl<'i> Parser<'i> { self.bump(); break; } - self.error(Error::UnexpectedToken); + self.error(ErrorKind::UnexpectedToken); expect_delimiter = false; } Some(IDENT) => { @@ -598,7 +604,7 @@ impl<'i> Parser<'i> { expect_delimiter = true; } Some(_) => { - self.error(Error::UnexpectedToken); + self.error(ErrorKind::UnexpectedToken); self.bump(); expect_delimiter = false; } @@ -613,7 +619,7 @@ impl<'i> Parser<'i> { loop { match self.peek_non_ws() { None => { - self.error(Error::MissingToken(guard)); + self.error(ErrorKind::MissingToken(guard)); break; } Some(k) if k == guard => { @@ -649,7 +655,7 @@ impl<'i> Parser<'i> { // Consume one token if it cannot start an AttrPath and is not the guard. // This can happen for some extra tokens (eg. unfinished exprs) in AttrSet or LetIn. Some(_) => { - self.error(Error::UnexpectedToken); + self.error(ErrorKind::UnexpectedToken); self.bump(); } } @@ -678,7 +684,7 @@ impl<'i> Parser<'i> { } Some(T!["${"]) => self.dynamic(), Some(T!['"']) => self.string(STRING), - _ => self.error(Error::MissingAttr), + _ => self.error(ErrorKind::MissingAttr), } } @@ -701,7 +707,7 @@ impl<'i> Parser<'i> { // No skipping whitespace. match self.peek() { None => { - self.error(Error::MissingToken(if node == STRING { + self.error(ErrorKind::MissingToken(if node == STRING { T!['"'] } else { T!["''"] diff --git a/syntax/src/tests.rs b/syntax/src/tests.rs index 4ed48b9..e0fa6b9 100644 --- a/syntax/src/tests.rs +++ b/syntax/src/tests.rs @@ -26,8 +26,8 @@ fn run_test(dir: &Path, ok: bool) { let ast = parse_file(&src); let mut got = String::new(); - for (err, loc) in ast.errors() { - writeln!(got, "{} at {}", err, u32::from(*loc)).unwrap(); + for err in ast.errors() { + writeln!(got, "{}", err).unwrap(); } write!(got, "{:#?}", ast.syntax_node()).unwrap(); @@ -37,7 +37,6 @@ fn run_test(dir: &Path, ok: bool) { } let expect_path = path.with_extension("ast"); - println!("{:?}", expect_path); expect_file![expect_path].assert_eq(&got); } } diff --git a/syntax/test_data/parser/err/empty.ast b/syntax/test_data/parser/err/empty.ast index e7f1552..85653e0 100644 --- a/syntax/test_data/parser/err/empty.ast +++ b/syntax/test_data/parser/err/empty.ast @@ -1,2 +1,2 @@ -Unexpected token at 0 +Unexpected token at 0..0 SOURCE_FILE@0..0 diff --git a/syntax/test_data/parser/err/invalid-or.ast b/syntax/test_data/parser/err/invalid-or.ast index 5b3c01a..11ab628 100644 --- a/syntax/test_data/parser/err/invalid-or.ast +++ b/syntax/test_data/parser/err/invalid-or.ast @@ -1,6 +1,6 @@ -Unexpected token at 2 -Unexpected token at 9 -Unexpected token at 14 +Unexpected token at 2..4 +Unexpected token at 9..11 +Unexpected token at 14..16 SOURCE_FILE@0..21 BINARY_OP@0..21 BINARY_OP@0..12 diff --git a/syntax/test_data/parser/err/multiple-top-exprs.ast b/syntax/test_data/parser/err/multiple-top-exprs.ast index 862e2b9..9465a35 100644 --- a/syntax/test_data/parser/err/multiple-top-exprs.ast +++ b/syntax/test_data/parser/err/multiple-top-exprs.ast @@ -1,4 +1,4 @@ -Multiple root expressions at 12 +Multiple root expressions at 12..18 SOURCE_FILE@0..24 ASSERT@0..12 KW_ASSERT@0..6 "assert" diff --git a/syntax/test_data/parser/err/path-duplicated-slashes.ast b/syntax/test_data/parser/err/path-duplicated-slashes.ast index 1dbac19..c5e677e 100644 --- a/syntax/test_data/parser/err/path-duplicated-slashes.ast +++ b/syntax/test_data/parser/err/path-duplicated-slashes.ast @@ -1,5 +1,5 @@ -Path has duplicated slashes at 7 -Path has duplicated slashes at 20 +Path has duplicated slashes at 7..8 +Path has duplicated slashes at 20..21 SOURCE_FILE@0..25 LIST@0..24 L_BRACK@0..1 "[" diff --git a/syntax/test_data/parser/err/path-trailing-slash.ast b/syntax/test_data/parser/err/path-trailing-slash.ast index 1f3760b..859e128 100644 --- a/syntax/test_data/parser/err/path-trailing-slash.ast +++ b/syntax/test_data/parser/err/path-trailing-slash.ast @@ -1,6 +1,6 @@ -Path has trailing slash at 6 -Path has trailing slash at 13 -Path has trailing slash at 29 +Path has trailing slash at 6..7 +Path has trailing slash at 13..14 +Path has trailing slash at 29..30 SOURCE_FILE@0..33 LIST@0..32 L_BRACK@0..1 "["