From a54e481646edb151075d12ca6903091abe7cfc4e Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 20 Oct 2020 17:48:43 +0200 Subject: Simplify diagnostic construction, add unused field --- crates/ide/src/diagnostics.rs | 73 +++++++++++++-------------- crates/ide/src/diagnostics/field_shorthand.rs | 32 +++++------- crates/rust-analyzer/src/handlers.rs | 14 ++--- 3 files changed, 55 insertions(+), 64 deletions(-) (limited to 'crates') diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 1e5ea4617..c92c1c066 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -31,6 +31,21 @@ pub struct Diagnostic { pub range: TextRange, pub severity: Severity, pub fix: Option, + pub unused: bool, +} + +impl Diagnostic { + fn error(range: TextRange, message: String) -> Self { + Self { message, range, severity: Severity::Error, fix: None, unused: false } + } + + fn hint(range: TextRange, message: String) -> Self { + Self { message, range, severity: Severity::WeakWarning, fix: None, unused: false } + } + + fn with_fix(self, fix: Option) -> Self { + Self { fix, ..self } + } } #[derive(Debug)] @@ -71,13 +86,13 @@ pub(crate) fn diagnostics( let mut res = Vec::new(); // [#34344] Only take first 128 errors to prevent slowing down editor/ide, the number 128 is chosen arbitrarily. - res.extend(parse.errors().iter().take(128).map(|err| Diagnostic { - // name: None, - range: err.range(), - message: format!("Syntax Error: {}", err), - severity: Severity::Error, - fix: None, - })); + res.extend( + parse + .errors() + .iter() + .take(128) + .map(|err| Diagnostic::error(err.range(), format!("Syntax Error: {}", err))), + ); for node in parse.tree().syntax().descendants() { check_unnecessary_braces_in_use_statement(&mut res, file_id, &node); @@ -108,13 +123,8 @@ pub(crate) fn diagnostics( let mut sink = sink_builder // Diagnostics not handled above get no fix and default treatment. .build(|d| { - res.borrow_mut().push(Diagnostic { - // name: Some(d.name().into()), - message: d.message(), - range: sema.diagnostics_display_range(d).range, - severity: Severity::Error, - fix: None, - }) + res.borrow_mut() + .push(Diagnostic::error(sema.diagnostics_display_range(d).range, d.message())); }); if let Some(m) = sema.to_module_def(file_id) { @@ -125,22 +135,11 @@ pub(crate) fn diagnostics( } fn diagnostic_with_fix(d: &D, sema: &Semantics) -> Diagnostic { - Diagnostic { - // name: Some(d.name().into()), - range: sema.diagnostics_display_range(d).range, - message: d.message(), - severity: Severity::Error, - fix: d.fix(&sema), - } + Diagnostic::error(sema.diagnostics_display_range(d).range, d.message()).with_fix(d.fix(&sema)) } fn warning_with_fix(d: &D, sema: &Semantics) -> Diagnostic { - Diagnostic { - range: sema.diagnostics_display_range(d).range, - message: d.message(), - severity: Severity::WeakWarning, - fix: d.fix(&sema), - } + Diagnostic::hint(sema.diagnostics_display_range(d).range, d.message()).with_fix(d.fix(&sema)) } fn check_unnecessary_braces_in_use_statement( @@ -161,17 +160,14 @@ fn check_unnecessary_braces_in_use_statement( edit_builder.finish() }); - acc.push(Diagnostic { - // name: None, - range: use_range, - message: "Unnecessary braces in use statement".to_string(), - severity: Severity::WeakWarning, - fix: Some(Fix::new( - "Remove unnecessary braces", - SourceFileEdit { file_id, edit }.into(), - use_range, - )), - }); + acc.push( + Diagnostic::hint(use_range, "Unnecessary braces in use statement".to_string()) + .with_fix(Some(Fix::new( + "Remove unnecessary braces", + SourceFileEdit { file_id, edit }.into(), + use_range, + ))), + ); } Some(()) @@ -578,6 +574,7 @@ fn test_fn() { fix_trigger_range: 0..8, }, ), + unused: false, }, ] "#]], diff --git a/crates/ide/src/diagnostics/field_shorthand.rs b/crates/ide/src/diagnostics/field_shorthand.rs index 2c4acd783..54e9fce9e 100644 --- a/crates/ide/src/diagnostics/field_shorthand.rs +++ b/crates/ide/src/diagnostics/field_shorthand.rs @@ -6,7 +6,7 @@ use ide_db::source_change::SourceFileEdit; use syntax::{ast, match_ast, AstNode, SyntaxNode}; use text_edit::TextEdit; -use crate::{Diagnostic, Fix, Severity}; +use crate::{Diagnostic, Fix}; pub(super) fn check(acc: &mut Vec, file_id: FileId, node: &SyntaxNode) { match_ast! { @@ -46,17 +46,15 @@ fn check_expr_field_shorthand( let edit = edit_builder.finish(); let field_range = record_field.syntax().text_range(); - acc.push(Diagnostic { - // name: None, - range: field_range, - message: "Shorthand struct initialization".to_string(), - severity: Severity::WeakWarning, - fix: Some(Fix::new( - "Use struct shorthand initialization", - SourceFileEdit { file_id, edit }.into(), - field_range, - )), - }); + acc.push( + Diagnostic::hint(field_range, "Shorthand struct initialization".to_string()).with_fix( + Some(Fix::new( + "Use struct shorthand initialization", + SourceFileEdit { file_id, edit }.into(), + field_range, + )), + ), + ); } } @@ -88,17 +86,13 @@ fn check_pat_field_shorthand( let edit = edit_builder.finish(); let field_range = record_pat_field.syntax().text_range(); - acc.push(Diagnostic { - // name: None, - range: field_range, - message: "Shorthand struct pattern".to_string(), - severity: Severity::WeakWarning, - fix: Some(Fix::new( + acc.push(Diagnostic::hint(field_range, "Shorthand struct pattern".to_string()).with_fix( + Some(Fix::new( "Use struct field shorthand", SourceFileEdit { file_id, edit }.into(), field_range, )), - }); + )); } } diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index 215be850f..f2d57f986 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -16,12 +16,12 @@ use lsp_server::ErrorCode; use lsp_types::{ CallHierarchyIncomingCall, CallHierarchyIncomingCallsParams, CallHierarchyItem, CallHierarchyOutgoingCall, CallHierarchyOutgoingCallsParams, CallHierarchyPrepareParams, - CodeActionKind, CodeLens, Command, CompletionItem, Diagnostic, DocumentFormattingParams, - DocumentHighlight, DocumentSymbol, FoldingRange, FoldingRangeParams, HoverContents, Location, - Position, PrepareRenameResponse, Range, RenameParams, SemanticTokensDeltaParams, - SemanticTokensFullDeltaResult, SemanticTokensParams, SemanticTokensRangeParams, - SemanticTokensRangeResult, SemanticTokensResult, SymbolInformation, SymbolTag, - TextDocumentIdentifier, Url, WorkspaceEdit, + CodeActionKind, CodeLens, Command, CompletionItem, Diagnostic, DiagnosticTag, + DocumentFormattingParams, DocumentHighlight, DocumentSymbol, FoldingRange, FoldingRangeParams, + HoverContents, Location, Position, PrepareRenameResponse, Range, RenameParams, + SemanticTokensDeltaParams, SemanticTokensFullDeltaResult, SemanticTokensParams, + SemanticTokensRangeParams, SemanticTokensRangeResult, SemanticTokensResult, SymbolInformation, + SymbolTag, TextDocumentIdentifier, Url, WorkspaceEdit, }; use project_model::TargetKind; use serde::{Deserialize, Serialize}; @@ -1124,7 +1124,7 @@ pub(crate) fn publish_diagnostics( source: Some("rust-analyzer".to_string()), message: d.message, related_information: None, - tags: None, + tags: if d.unused { Some(vec![DiagnosticTag::Unnecessary]) } else { None }, }) .collect(); Ok(diagnostics) -- cgit v1.2.3 From 80d27414016903fa591548cff22939d3c43cdd8d Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 20 Oct 2020 17:49:21 +0200 Subject: Add a (hint) diagnostic for unconfigured items --- crates/hir/src/diagnostics.rs | 2 +- crates/hir_def/src/diagnostics.rs | 25 +++++++++++++++++++++++++ crates/hir_def/src/item_tree.rs | 18 ++++++++++++++++++ crates/hir_def/src/nameres.rs | 15 ++++++++++++++- crates/hir_def/src/nameres/collector.rs | 13 +++++++++++++ crates/ide/src/diagnostics.rs | 16 +++++++++++++++- 6 files changed, 86 insertions(+), 3 deletions(-) (limited to 'crates') diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index da2b40849..d476ca3b9 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -1,5 +1,5 @@ //! FIXME: write short doc here -pub use hir_def::diagnostics::UnresolvedModule; +pub use hir_def::diagnostics::{UnconfiguredCode, UnresolvedModule}; pub use hir_expand::diagnostics::{Diagnostic, DiagnosticSink, DiagnosticSinkBuilder}; pub use hir_ty::diagnostics::{ IncorrectCase, MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, diff --git a/crates/hir_def/src/diagnostics.rs b/crates/hir_def/src/diagnostics.rs index fcfbbbad3..53cf1aca1 100644 --- a/crates/hir_def/src/diagnostics.rs +++ b/crates/hir_def/src/diagnostics.rs @@ -86,3 +86,28 @@ impl Diagnostic for UnresolvedImport { true } } + +// Diagnostic: unconfigured-code +// +// This diagnostic is shown for code with inactive `#[cfg]` attributes. +#[derive(Debug)] +pub struct UnconfiguredCode { + pub file: HirFileId, + pub node: SyntaxNodePtr, +} + +impl Diagnostic for UnconfiguredCode { + fn code(&self) -> DiagnosticCode { + DiagnosticCode("unconfigured-code") + } + fn message(&self) -> String { + // FIXME: say *why* it is configured out + "configured out".to_string() + } + fn display_source(&self) -> InFile { + InFile::new(self.file, self.node.clone()) + } + fn as_any(&self) -> &(dyn Any + Send + 'static) { + self + } +} diff --git a/crates/hir_def/src/item_tree.rs b/crates/hir_def/src/item_tree.rs index 8a1121bbd..7eb388bae 100644 --- a/crates/hir_def/src/item_tree.rs +++ b/crates/hir_def/src/item_tree.rs @@ -672,6 +672,24 @@ impl ModItem { pub fn downcast(self) -> Option> { N::id_from_mod_item(self) } + + pub fn ast_id(&self, tree: &ItemTree) -> FileAstId { + match self { + ModItem::Import(it) => tree[it.index].ast_id().upcast(), + ModItem::ExternCrate(it) => tree[it.index].ast_id().upcast(), + ModItem::Function(it) => tree[it.index].ast_id().upcast(), + ModItem::Struct(it) => tree[it.index].ast_id().upcast(), + ModItem::Union(it) => tree[it.index].ast_id().upcast(), + ModItem::Enum(it) => tree[it.index].ast_id().upcast(), + ModItem::Const(it) => tree[it.index].ast_id().upcast(), + ModItem::Static(it) => tree[it.index].ast_id().upcast(), + ModItem::Trait(it) => tree[it.index].ast_id().upcast(), + ModItem::Impl(it) => tree[it.index].ast_id().upcast(), + ModItem::TypeAlias(it) => tree[it.index].ast_id().upcast(), + ModItem::Mod(it) => tree[it.index].ast_id().upcast(), + ModItem::MacroCall(it) => tree[it.index].ast_id().upcast(), + } + } } #[derive(Debug, Copy, Clone, Eq, PartialEq)] diff --git a/crates/hir_def/src/nameres.rs b/crates/hir_def/src/nameres.rs index 3d04f81c6..8bb3a659f 100644 --- a/crates/hir_def/src/nameres.rs +++ b/crates/hir_def/src/nameres.rs @@ -286,7 +286,7 @@ mod diagnostics { use hir_expand::diagnostics::DiagnosticSink; use hir_expand::hygiene::Hygiene; use hir_expand::InFile; - use syntax::{ast, AstPtr}; + use syntax::{ast, AstPtr, SyntaxNodePtr}; use crate::path::ModPath; use crate::{db::DefDatabase, diagnostics::*, nameres::LocalModuleId, AstId}; @@ -298,6 +298,8 @@ mod diagnostics { UnresolvedExternCrate { ast: AstId }, UnresolvedImport { ast: AstId, index: usize }, + + UnconfiguredCode { ast: InFile }, } #[derive(Debug, PartialEq, Eq)] @@ -336,6 +338,13 @@ mod diagnostics { Self { in_module: container, kind: DiagnosticKind::UnresolvedImport { ast, index } } } + pub(super) fn unconfigured_code( + container: LocalModuleId, + ast: InFile, + ) -> Self { + Self { in_module: container, kind: DiagnosticKind::UnconfiguredCode { ast } } + } + pub(super) fn add_to( &self, db: &dyn DefDatabase, @@ -385,6 +394,10 @@ mod diagnostics { sink.push(UnresolvedImport { file: ast.file_id, node: AstPtr::new(&tree) }); } } + + DiagnosticKind::UnconfiguredCode { ast } => { + sink.push(UnconfiguredCode { file: ast.file_id, node: ast.value.clone() }); + } } } } diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs index c8cd04264..bff8edb62 100644 --- a/crates/hir_def/src/nameres/collector.rs +++ b/crates/hir_def/src/nameres/collector.rs @@ -913,6 +913,7 @@ impl ModCollector<'_, '_> { for &item in items { let attrs = self.item_tree.attrs(item.into()); if !self.is_cfg_enabled(attrs) { + self.emit_unconfigured_diagnostic(item); continue; } let module = @@ -1323,6 +1324,18 @@ impl ModCollector<'_, '_> { fn is_cfg_enabled(&self, attrs: &Attrs) -> bool { attrs.is_cfg_enabled(self.def_collector.cfg_options) } + + fn emit_unconfigured_diagnostic(&mut self, item: ModItem) { + let ast_id = item.ast_id(self.item_tree); + let id_map = self.def_collector.db.ast_id_map(self.file_id); + let syntax_ptr = id_map.get(ast_id).syntax_node_ptr(); + + let ast_node = InFile::new(self.file_id, syntax_ptr); + self.def_collector + .def_map + .diagnostics + .push(DefDiagnostic::unconfigured_code(self.module_id, ast_node)); + } } fn is_macro_rules(path: &ModPath) -> bool { diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index c92c1c066..394365bc8 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -10,7 +10,10 @@ mod field_shorthand; use std::cell::RefCell; use base_db::SourceDatabase; -use hir::{diagnostics::DiagnosticSinkBuilder, Semantics}; +use hir::{ + diagnostics::{Diagnostic as _, DiagnosticSinkBuilder}, + Semantics, +}; use ide_db::RootDatabase; use itertools::Itertools; use rustc_hash::FxHashSet; @@ -46,6 +49,10 @@ impl Diagnostic { fn with_fix(self, fix: Option) -> Self { Self { fix, ..self } } + + fn with_unused(self, unused: bool) -> Self { + Self { unused, ..self } + } } #[derive(Debug)] @@ -115,6 +122,13 @@ pub(crate) fn diagnostics( .on::(|d| { res.borrow_mut().push(warning_with_fix(d, &sema)); }) + .on::(|d| { + // Override severity and mark as unused. + res.borrow_mut().push( + Diagnostic::hint(sema.diagnostics_display_range(d).range, d.message()) + .with_unused(true), + ); + }) // Only collect experimental diagnostics when they're enabled. .filter(|diag| !(diag.is_experimental() && config.disable_experimental)) .filter(|diag| !config.disabled.contains(diag.code().as_str())); -- cgit v1.2.3 From 4cb3cf352f5496161bf3cfad92ca87dd92d51d37 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 20 Oct 2020 18:22:31 +0200 Subject: Rename UnconfiguredCode -> InactiveCode --- crates/hir/src/diagnostics.rs | 2 +- crates/hir_def/src/diagnostics.rs | 6 +++--- crates/hir_def/src/nameres.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) (limited to 'crates') diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index d476ca3b9..c18c1c587 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -1,5 +1,5 @@ //! FIXME: write short doc here -pub use hir_def::diagnostics::{UnconfiguredCode, UnresolvedModule}; +pub use hir_def::diagnostics::{InactiveCode, UnresolvedModule}; pub use hir_expand::diagnostics::{Diagnostic, DiagnosticSink, DiagnosticSinkBuilder}; pub use hir_ty::diagnostics::{ IncorrectCase, MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, diff --git a/crates/hir_def/src/diagnostics.rs b/crates/hir_def/src/diagnostics.rs index 53cf1aca1..ad3b6bf26 100644 --- a/crates/hir_def/src/diagnostics.rs +++ b/crates/hir_def/src/diagnostics.rs @@ -91,14 +91,14 @@ impl Diagnostic for UnresolvedImport { // // This diagnostic is shown for code with inactive `#[cfg]` attributes. #[derive(Debug)] -pub struct UnconfiguredCode { +pub struct InactiveCode { pub file: HirFileId, pub node: SyntaxNodePtr, } -impl Diagnostic for UnconfiguredCode { +impl Diagnostic for InactiveCode { fn code(&self) -> DiagnosticCode { - DiagnosticCode("unconfigured-code") + DiagnosticCode("inactive-code") } fn message(&self) -> String { // FIXME: say *why* it is configured out diff --git a/crates/hir_def/src/nameres.rs b/crates/hir_def/src/nameres.rs index 8bb3a659f..01a28aeeb 100644 --- a/crates/hir_def/src/nameres.rs +++ b/crates/hir_def/src/nameres.rs @@ -396,7 +396,7 @@ mod diagnostics { } DiagnosticKind::UnconfiguredCode { ast } => { - sink.push(UnconfiguredCode { file: ast.file_id, node: ast.value.clone() }); + sink.push(InactiveCode { file: ast.file_id, node: ast.value.clone() }); } } } -- cgit v1.2.3 From 3fa04f35d279253865d060b6d4ecbd849fb22139 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 20 Oct 2020 18:23:55 +0200 Subject: More detailed message --- crates/hir_def/src/diagnostics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'crates') diff --git a/crates/hir_def/src/diagnostics.rs b/crates/hir_def/src/diagnostics.rs index ad3b6bf26..c9c08b01f 100644 --- a/crates/hir_def/src/diagnostics.rs +++ b/crates/hir_def/src/diagnostics.rs @@ -102,7 +102,7 @@ impl Diagnostic for InactiveCode { } fn message(&self) -> String { // FIXME: say *why* it is configured out - "configured out".to_string() + "code is inactive due to #[cfg] directives".to_string() } fn display_source(&self) -> InFile { InFile::new(self.file, self.node.clone()) -- cgit v1.2.3 From 74ac83a5acc9f53db69577fc32a4a6e3985d2ef9 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 20 Oct 2020 18:29:47 +0200 Subject: Fixup botched rename --- crates/ide/src/diagnostics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'crates') diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 394365bc8..90574cb35 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -122,7 +122,7 @@ pub(crate) fn diagnostics( .on::(|d| { res.borrow_mut().push(warning_with_fix(d, &sema)); }) - .on::(|d| { + .on::(|d| { // Override severity and mark as unused. res.borrow_mut().push( Diagnostic::hint(sema.diagnostics_display_range(d).range, d.message()) -- cgit v1.2.3