From 1a3a3a325f84f911fc55d06f4a8b5cfbab0e9dcd Mon Sep 17 00:00:00 2001 From: Bas Zalmstra Date: Fri, 4 Oct 2019 00:52:15 +0200 Subject: [PATCH] feat: adds detection of duplicate definition names --- crates/mun/test/main.mun | 2 +- crates/mun_compiler/src/lib.rs | 16 ++++++ crates/mun_hir/src/code_model.rs | 68 +++++++++++++++++++++++-- crates/mun_hir/src/diagnostics.rs | 26 ++++++++++ crates/mun_syntax/src/ast/extensions.rs | 30 ++++++++++- 5 files changed, 135 insertions(+), 7 deletions(-) diff --git a/crates/mun/test/main.mun b/crates/mun/test/main.mun index 50827b6c9..008a566cf 100644 --- a/crates/mun/test/main.mun +++ b/crates/mun/test/main.mun @@ -16,4 +16,4 @@ fn multiply(a:float, b:float):float { fn main():int { 5 -} \ No newline at end of file +} diff --git a/crates/mun_compiler/src/lib.rs b/crates/mun_compiler/src/lib.rs index a1e0714f1..6a7ce031d 100644 --- a/crates/mun_compiler/src/lib.rs +++ b/crates/mun_compiler/src/lib.rs @@ -14,6 +14,7 @@ use std::sync::{Arc, Mutex}; use termcolor::{ColorChoice, StandardStream}; pub use mun_codegen::OptimizationLevel; +use mun_syntax::{ast, SyntaxKind}; use mun_target::spec; #[derive(Debug, Clone)] @@ -154,6 +155,21 @@ fn diagnostics(db: &CompilerDatabase, file_id: FileId) -> Vec { d.found.display(db) ), }); + }) + .on::(|d| { + result.borrow_mut().push(Diagnostic { + level: Level::Error, + loc: match d.definition.kind() { + SyntaxKind::FUNCTION_DEF => { + ast::FunctionDef::cast(d.definition.to_node(&parse.tree().syntax())) + .map(|f| f.signature_range()) + .unwrap_or(d.highlight_range()) + .into() + } + _ => d.highlight_range().into(), + }, + message: d.message(), + }); }); if let Some(module) = Module::package_modules(db) diff --git a/crates/mun_hir/src/code_model.rs b/crates/mun_hir/src/code_model.rs index 42780d3d5..3da049549 100644 --- a/crates/mun_hir/src/code_model.rs +++ b/crates/mun_hir/src/code_model.rs @@ -42,6 +42,9 @@ impl Module { } pub fn diagnostics(self, db: &impl HirDatabase, sink: &mut DiagnosticSink) { + for diag in db.module_data(self.file_id).diagnostics.iter() { + diag.add_to(db, self, sink); + } for decl in self.declarations(db) { match decl { ModuleDef::Function(f) => f.diagnostics(db, sink), @@ -54,6 +57,7 @@ impl Module { #[derive(Debug, Clone, Hash, PartialEq, Eq, Default)] pub struct ModuleData { definitions: Vec, + diagnostics: Vec } #[derive(Debug, Default, PartialEq, Eq, Clone)] @@ -66,13 +70,25 @@ impl ModuleData { let items = db.raw_items(file_id); let mut data = ModuleData::default(); let loc_ctx = LocationCtx::new(db, file_id); + let mut definition_by_name = FxHashMap::default(); for item in items.items().iter() { match item { - RawFileItem::Definition(def) => match items[*def].kind { - DefKind::Function(ast_id) => { - data.definitions.push(ModuleDef::Function(Function { - id: FunctionId::from_ast_id(loc_ctx, ast_id), - })) + RawFileItem::Definition(def) => { + if let Some(prev_definition) = definition_by_name.get(&items[*def].name) { + data.diagnostics.push(diagnostics::ModuleDefinitionDiagnostic::DuplicateName { + name: items[*def].name.clone(), + definition: *def, + first_definition: *prev_definition + }) + } else { + definition_by_name.insert(items[*def].name.clone(), *def); + } + match items[*def].kind { + DefKind::Function(ast_id) => { + data.definitions.push(ModuleDef::Function(Function { + id: FunctionId::from_ast_id(loc_ctx, ast_id), + })) + } } }, }; @@ -258,6 +274,8 @@ pub enum BuiltinType { } use crate::name::*; +use crate::code_model::diagnostics::ModuleDefinitionDiagnostic; + impl BuiltinType { #[rustfmt::skip] pub(crate) const ALL: &'static [(Name, BuiltinType)] = &[ @@ -265,3 +283,43 @@ impl BuiltinType { (INT, BuiltinType::Int), ]; } + +mod diagnostics { + use crate::{Name, DefDatabase}; + use crate::raw::{DefId, DefKind}; + use crate::diagnostics::{DiagnosticSink, DuplicateDefinition}; + use super::Module; + use mun_syntax::{SyntaxNodePtr, AstNode}; + + #[derive(Debug, PartialEq, Eq, Clone, Hash)] + pub(super) enum ModuleDefinitionDiagnostic { + DuplicateName { name: Name, definition: DefId, first_definition: DefId }, + } + + fn syntax_ptr_from_def(db: &impl DefDatabase, owner: Module, kind: DefKind) -> SyntaxNodePtr { + match kind { + DefKind::Function(id) => SyntaxNodePtr::new(id.with_file_id(owner.file_id).to_node(db).syntax()), + } + } + + impl ModuleDefinitionDiagnostic { + pub(super) fn add_to( + &self, + db: &impl DefDatabase, + owner: Module, + sink: &mut DiagnosticSink, + ) { + match self { + ModuleDefinitionDiagnostic::DuplicateName { name, definition, first_definition } => { + let raw_items = db.raw_items(owner.file_id); + sink.push(DuplicateDefinition { + file: owner.file_id, + name: name.to_string(), + definition: syntax_ptr_from_def(db, owner, raw_items[*definition].kind), + first_definition: syntax_ptr_from_def(db, owner, raw_items[*first_definition].kind), + }) + }, + } + } + } +} \ No newline at end of file diff --git a/crates/mun_hir/src/diagnostics.rs b/crates/mun_hir/src/diagnostics.rs index 06b6a940f..75546ec51 100644 --- a/crates/mun_hir/src/diagnostics.rs +++ b/crates/mun_hir/src/diagnostics.rs @@ -173,3 +173,29 @@ impl Diagnostic for CannotApplyBinaryOp { self } } + +#[derive(Debug)] +pub struct DuplicateDefinition { + pub file: FileId, + pub name: String, + pub first_definition: SyntaxNodePtr, + pub definition: SyntaxNodePtr, +} + +impl Diagnostic for DuplicateDefinition { + fn message(&self) -> String { + format!("the name `{}` is defined multiple times", self.name) + } + + fn file(&self) -> FileId { + self.file + } + + fn syntax_node_ptr(&self) -> SyntaxNodePtr { + self.definition + } + + fn as_any(&self) -> &(dyn Any + Send + 'static) { + self + } +} \ No newline at end of file diff --git a/crates/mun_syntax/src/ast/extensions.rs b/crates/mun_syntax/src/ast/extensions.rs index c11b097f0..e4c717af2 100644 --- a/crates/mun_syntax/src/ast/extensions.rs +++ b/crates/mun_syntax/src/ast/extensions.rs @@ -1,8 +1,10 @@ +use crate::ast::NameOwner; use crate::{ ast::{self, AstNode}, - T, + SyntaxKind, T, }; use crate::{SmolStr, SyntaxNode}; +use text_unit::TextRange; impl ast::Name { pub fn text(&self) -> &SmolStr { @@ -16,6 +18,32 @@ impl ast::NameRef { } } +impl ast::FunctionDef { + pub fn signature_range(&self) -> TextRange { + let fn_kw = self + .syntax() + .children_with_tokens() + .find(|p| p.kind() == SyntaxKind::FN_KW) + .map(|kw| kw.text_range()); + let name = self.name().map(|n| n.syntax.text_range()); + let param_list = self.param_list().map(|p| p.syntax.text_range()); + let ret_type = self.ret_type().map(|r| r.syntax.text_range()); + + let start = fn_kw + .map(|kw| kw.start()) + .unwrap_or(self.syntax.text_range().start()); + + let end = ret_type + .map(|p| p.end()) + .or(param_list.map(|name| name.end())) + .or(name.map(|name| name.end())) + .or(fn_kw.map(|kw| kw.end())) + .unwrap_or(self.syntax().text_range().end()); + + TextRange::from_to(start, end) + } +} + fn text_of_first_token(node: &SyntaxNode) -> &SmolStr { match node.0.green().children().first() { Some(rowan::GreenElement::Token(it)) => it.text(),