From 9f0459987f7917f0e91e1dfd002ce0907a6f4775 Mon Sep 17 00:00:00 2001 From: SpontanCombust <61706594+SpontanCombust@users.noreply.github.com> Date: Wed, 12 Jun 2024 02:39:04 +0200 Subject: [PATCH] diagnostics for annotations --- .../syntax_analysis/contextual_analysis.rs | 84 ++++++++++++++++++- crates/core/src/ast/annotation.rs | 20 +++++ crates/diagnostics/src/lib.rs | 27 +++++- crates/lsp/src/providers/common.rs | 38 +++++---- crates/lsp/src/tasks/script_analysis_tasks.rs | 3 +- 5 files changed, 151 insertions(+), 21 deletions(-) diff --git a/crates/analysis/src/jobs/syntax_analysis/contextual_analysis.rs b/crates/analysis/src/jobs/syntax_analysis/contextual_analysis.rs index 2077822f..690d771b 100644 --- a/crates/analysis/src/jobs/syntax_analysis/contextual_analysis.rs +++ b/crates/analysis/src/jobs/syntax_analysis/contextual_analysis.rs @@ -1,12 +1,15 @@ +use std::str::FromStr; +use lsp_types as lsp; use smallvec::SmallVec; -use witcherscript::{ast::*, attribs::*, tokens::Keyword, Script}; +use witcherscript::{ast::*, attribs::*, script_document::ScriptDocument, tokens::*, NamedSyntaxNode, Script}; use witcherscript_diagnostics::{Diagnostic, DiagnosticKind}; /// For situations where a valid syntax tree is produced by tree-sitter, /// but we can still deduce some based only of the AST. -pub fn contextual_syntax_analysis(script: &Script, diagnostics: &mut Vec) { +pub fn contextual_syntax_analysis(script: &Script, doc: &ScriptDocument, diagnostics: &mut Vec) { let mut visitor = ContextualSyntaxAnalysis { + doc, diagnostics }; @@ -14,9 +17,59 @@ pub fn contextual_syntax_analysis(script: &Script, diagnostics: &mut Vec { + doc: &'a ScriptDocument, diagnostics: &'a mut Vec } +impl ContextualSyntaxAnalysis<'_> { + fn visit_annotation(&mut self, n: &AnnotationNode, annotated_node_kind: &str, annotated_node_range: lsp::Range) { + let annot_name = n.name(); + match AnnotationKind::from_str(&annot_name.value(self.doc)) { + Ok(kind) => { + if kind.requires_arg() && n.arg().is_none() { + self.diagnostics.push(Diagnostic { + range: annot_name.range(), + kind: DiagnosticKind::MissingAnnotationArgument { + missing: kind.arg_type().unwrap_or_default().to_string() + } + }) + } + + match kind { + AnnotationKind::AddMethod + | AnnotationKind::ReplaceMethod + | AnnotationKind::WrapMethod => { + if annotated_node_kind != FunctionDeclarationNode::NODE_KIND { + self.diagnostics.push(Diagnostic { + range: annotated_node_range, + kind: DiagnosticKind::IncompatibleAnnotation { + expected_sym: "a method declaration".into() + } + }) + } + }, + AnnotationKind::AddField => { + if annotated_node_kind != MemberVarDeclarationNode::NODE_KIND { + self.diagnostics.push(Diagnostic { + range: annotated_node_range, + kind: DiagnosticKind::IncompatibleAnnotation { + expected_sym: "a field declaration".into() + } + }) + } + } + } + }, + Err(_) => { + self.diagnostics.push(Diagnostic { + range: annot_name.range(), + kind: DiagnosticKind::InvalidAnnotation + }) + }, + } + } +} + impl SyntaxNodeVisitor for ContextualSyntaxAnalysis<'_> { fn traversal_policy_default(&self) -> bool { false @@ -103,6 +156,10 @@ impl SyntaxNodeVisitor for ContextualSyntaxAnalysis<'_> { } fn visit_global_func_decl(&mut self, n: &FunctionDeclarationNode) -> FunctionDeclarationTraversalPolicy { + if let Some(annot) = n.annotation() { + self.visit_annotation(&annot, FunctionDeclarationNode::NODE_KIND, n.range()); + } + let mut specifiers = SmallVec::<[GlobalFunctionSpecifier; 2]>::new(); for (spec, range) in n.specifiers().map(|specn| (specn.value(), specn.range())) { @@ -141,6 +198,15 @@ impl SyntaxNodeVisitor for ContextualSyntaxAnalysis<'_> { } fn visit_global_var_decl(&mut self, n: &MemberVarDeclarationNode) { + if let Some(annot) = n.annotation() { + self.visit_annotation(&annot, MemberVarDeclarationNode::NODE_KIND, n.range()); + } else { + self.diagnostics.push(Diagnostic { + range: n.range(), + kind: DiagnosticKind::GlobalScopeVarDecl + }) + } + let mut specifiers = SmallVec::<[MemberVarSpecifier; 6]>::new(); let mut found_access_modif_before = false; @@ -175,6 +241,13 @@ impl SyntaxNodeVisitor for ContextualSyntaxAnalysis<'_> { } fn visit_member_var_decl(&mut self, n: &MemberVarDeclarationNode, _: DeclarationTraversalContext) { + if let Some(range) = n.annotation().map(|ann| ann.range()) { + self.diagnostics.push(Diagnostic { + range, + kind: DiagnosticKind::InvalidAnnotationPlacement + }) + } + let mut specifiers = SmallVec::<[MemberVarSpecifier; 6]>::new(); let mut found_access_modif_before = false; @@ -209,6 +282,13 @@ impl SyntaxNodeVisitor for ContextualSyntaxAnalysis<'_> { } fn visit_member_func_decl(&mut self, n: &FunctionDeclarationNode, _: DeclarationTraversalContext) -> FunctionDeclarationTraversalPolicy { + if let Some(range) = n.annotation().map(|ann| ann.range()) { + self.diagnostics.push(Diagnostic { + range, + kind: DiagnosticKind::InvalidAnnotationPlacement + }) + } + let mut specifiers = SmallVec::<[MemberFunctionSpecifier; 4]>::new(); let mut found_access_modif_before = false; diff --git a/crates/core/src/ast/annotation.rs b/crates/core/src/ast/annotation.rs index ea981c96..bb21a0b7 100644 --- a/crates/core/src/ast/annotation.rs +++ b/crates/core/src/ast/annotation.rs @@ -19,6 +19,26 @@ pub enum AnnotationKind { WrapMethod } +impl AnnotationKind { + pub fn requires_arg(&self) -> bool { + match self { + AnnotationKind::AddMethod => true, + AnnotationKind::AddField => true, + AnnotationKind::ReplaceMethod => false, + AnnotationKind::WrapMethod => true, + } + } + + pub fn arg_type(&self) -> Option<&'static str> { + match self { + AnnotationKind::AddMethod => Some("a class identifier"), + AnnotationKind::AddField => Some("a class identifier"), + AnnotationKind::ReplaceMethod => Some("a class identifier"), + AnnotationKind::WrapMethod => Some("a class identifier"), + } + } +} + pub const WRAPPED_METHOD_NAME: &'static str = "wrappedMethod"; diff --git a/crates/diagnostics/src/lib.rs b/crates/diagnostics/src/lib.rs index 372c2af6..13cfebcb 100644 --- a/crates/diagnostics/src/lib.rs +++ b/crates/diagnostics/src/lib.rs @@ -81,7 +81,15 @@ pub enum DiagnosticKind { }, RepeatedSpecifier, MultipleAccessModifiers, - //TODO annotation check diagnostics + InvalidAnnotation, + InvalidAnnotationPlacement, + MissingAnnotationArgument { + missing: String + }, + IncompatibleAnnotation { + expected_sym: String + }, + GlobalScopeVarDecl, // symbol anaysis SymbolNameTaken { @@ -125,7 +133,12 @@ impl DiagnosticKind { IncompatibleSpecifier { .. } | IncompatibleFunctionFlavour { .. } | RepeatedSpecifier - | MultipleAccessModifiers => DiagnosticDomain::ContextualSyntaxAnalysis, + | MultipleAccessModifiers + | InvalidAnnotation + | InvalidAnnotationPlacement + | MissingAnnotationArgument { .. } + | IncompatibleAnnotation { .. } + | GlobalScopeVarDecl => DiagnosticDomain::ContextualSyntaxAnalysis, SymbolNameTaken { .. } | MissingTypeArg | UnnecessaryTypeArg => DiagnosticDomain::SymbolAnalysis, @@ -154,6 +167,11 @@ impl DiagnosticKind { IncompatibleFunctionFlavour { .. } => lsp::DiagnosticSeverity::ERROR, RepeatedSpecifier => lsp::DiagnosticSeverity::ERROR, MultipleAccessModifiers => lsp::DiagnosticSeverity::ERROR, + InvalidAnnotation => lsp::DiagnosticSeverity::ERROR, + InvalidAnnotationPlacement => lsp::DiagnosticSeverity::ERROR, + MissingAnnotationArgument { .. } => lsp::DiagnosticSeverity::ERROR, + IncompatibleAnnotation { .. } => lsp::DiagnosticSeverity::ERROR, + GlobalScopeVarDecl => lsp::DiagnosticSeverity::ERROR, SymbolNameTaken { .. } => lsp::DiagnosticSeverity::ERROR, MissingTypeArg => lsp::DiagnosticSeverity::ERROR, @@ -183,6 +201,11 @@ impl DiagnosticKind { IncompatibleFunctionFlavour { flavour_name, sym_name } => format!("\"{}\" cannot be used for {}", flavour_name, sym_name), RepeatedSpecifier => "Specifiers can not be repeating".into(), MultipleAccessModifiers => "Only one access modifier is allowed".into(), + InvalidAnnotation => "Unsupported annotation".into(), + InvalidAnnotationPlacement => "Annotations can only be used at the global scope".into(), + MissingAnnotationArgument { missing } => format!("This annotation requires {missing} argument"), + IncompatibleAnnotation { expected_sym } => format!("The annotation expects {}", expected_sym), + GlobalScopeVarDecl => "Syntax error: variable declarations in the global scope are not allowed unless you use the @addField annotation.".into(), SymbolNameTaken { name, .. } => format!("The name \"{}\" is defined multiple times", name), MissingTypeArg => "Missing type argument".into(), diff --git a/crates/lsp/src/providers/common.rs b/crates/lsp/src/providers/common.rs index 4e5b240a..4cde26c9 100644 --- a/crates/lsp/src/providers/common.rs +++ b/crates/lsp/src/providers/common.rs @@ -237,6 +237,12 @@ impl<'a> TextDocumentPositionResolver<'a> { } } } + + fn visit_annotation(&mut self, n: &AnnotationNode) { + if let Some(arg) = n.arg().filter(|arg| arg.spans_position(self.pos)) { + self.found_type_ident(&arg); + } + } } @@ -316,12 +322,23 @@ impl SyntaxNodeVisitor for TextDocumentPositionResolver<'_> { } fn visit_global_var_decl(&mut self, n: &MemberVarDeclarationNode) { - //TODO handle annotated var + let var_type = n.var_type(); + + if var_type.spans_position(self.pos) { + self.visit_type_annotation(&var_type); + } + else if let Some(annot) = n.annotation().filter(|annot| annot.spans_position(self.pos)) { + self.visit_annotation(&annot); + } + else if let Some(name) = n.names().find(|name| name.spans_position(self.pos)) { + self.found_data_decl_ident(&name); + } } fn visit_member_var_decl(&mut self, n: &MemberVarDeclarationNode, _: DeclarationTraversalContext) { let var_type = n.var_type(); + // not checking the annotation, because it'll be erroneous anyways if var_type.spans_position(self.pos) { self.visit_type_annotation(&var_type); } @@ -347,7 +364,6 @@ impl SyntaxNodeVisitor for TextDocumentPositionResolver<'_> { let member = n.member(); if member.spans_position(self.pos) { - // self.found_unqualified_data_ident(&member); self.found_expression_ident(&member, member.clone().into(), None); } } @@ -360,7 +376,6 @@ impl SyntaxNodeVisitor for TextDocumentPositionResolver<'_> { let member = n.member(); if member.spans_position(self.pos) { - // self.found_unqualified_data_ident(&member); self.found_expression_ident(&member, member.clone().into(), None); } } @@ -372,21 +387,21 @@ impl SyntaxNodeVisitor for TextDocumentPositionResolver<'_> { let member = n.member(); if member.spans_position(self.pos) { - // self.found_unqualified_data_ident(&member); self.found_expression_ident(&member, member.clone().into(), None); } } fn visit_global_func_decl(&mut self, n: &FunctionDeclarationNode) -> FunctionDeclarationTraversalPolicy { - //TODO handle annotated functions - if self.pos_filter_payload.borrow().done { let name = n.name(); if name.spans_position(self.pos) { self.found_callable_decl_ident(&name); } + else if let Some(annot) = n.annotation().filter(|annot| annot.spans_position(self.pos)) { + self.visit_annotation(&annot); + } else if let Some(rt) = n.return_type().filter(|rt| rt.spans_position(self.pos)) { self.visit_type_annotation(&rt); } @@ -399,6 +414,7 @@ impl SyntaxNodeVisitor for TextDocumentPositionResolver<'_> { if self.pos_filter_payload.borrow().done { let name = n.name(); + // not checking the annotation, because it'll be erroneous anyways if name.spans_position(self.pos) { self.found_callable_decl_ident(&name); } @@ -470,11 +486,6 @@ impl SyntaxNodeVisitor for TextDocumentPositionResolver<'_> { } fn visit_identifier_expr(&mut self, n: &IdentifierNode, cx: ExpressionTraversalContext) { - // if cx == ExpressionTraversalContext::FunctionCallExpressionFunc { - // self.found_unqualified_callable_ident(n); - // } else { - // self.found_unqualified_data_ident(n); - // }; self.found_expression_ident(n, n.clone().into(), Some(cx)); } @@ -483,11 +494,6 @@ impl SyntaxNodeVisitor for TextDocumentPositionResolver<'_> { let member = n.member(); if member.spans_position(self.pos) { - // if cx == ExpressionTraversalContext::FunctionCallExpressionFunc { - // self.found_qualified_callable_ident(&member); - // } else { - // self.found_qualified_data_ident(&member); - // }; self.found_expression_ident(&member, n.clone().into(), Some(cx)); } } diff --git a/crates/lsp/src/tasks/script_analysis_tasks.rs b/crates/lsp/src/tasks/script_analysis_tasks.rs index ed62ad96..ab3c4043 100644 --- a/crates/lsp/src/tasks/script_analysis_tasks.rs +++ b/crates/lsp/src/tasks/script_analysis_tasks.rs @@ -46,9 +46,10 @@ impl Backend { if let Some(kv) = scripts.get(&script_path) { let script_state = kv.value(); let script = &script_state.script; + let doc = &script_state.buffer; let mut diags = Vec::new(); jobs::syntax_analysis(script, &mut diags); - jobs::contextual_syntax_analysis(script, &mut diags); + jobs::contextual_syntax_analysis(script, doc, &mut diags); drop(kv); Some((script_path, diags))