Merge #6299

bors[bot] · Jonas Schievink · web-flow · commit 20d369a826e8 · 2020-10-20T16:35:05.000Z
6299: Diagnose items that are #[cfg]d out r=jonas-schievink a=jonas-schievink

This emits a hint-level diagnostic with `Unnecessary` tag to "gray out" any items whose `#[cfg]` attributes remove the item before name resolution.

Co-authored-by: Jonas Schievink &lt;jonas.schievink@ferrous-systems.com&gt;
diff --git 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::{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
@@ -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 InactiveCode {
+    pub file: HirFileId,
+    pub node: SyntaxNodePtr,
+}
+
+impl Diagnostic for InactiveCode {
+    fn code(&self) -> DiagnosticCode {
+        DiagnosticCode("inactive-code")
+    }
+    fn message(&self) -> String {
+        // FIXME: say *why* it is configured out
+        "code is inactive due to #[cfg] directives".to_string()
+    }
+    fn display_source(&self) -> InFile<SyntaxNodePtr> {
+        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
@@ -672,6 +672,24 @@ impl ModItem {
     pub fn downcast<N: ItemTreeNode>(self) -> Option<FileItemTreeId<N>> {
         N::id_from_mod_item(self)
     }
+
+    pub fn ast_id(&self, tree: &ItemTree) -> FileAstId<ast::Item> {
+        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
@@ -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<ast::ExternCrate> },
 
         UnresolvedImport { ast: AstId<ast::Use>, index: usize },
+
+        UnconfiguredCode { ast: InFile<SyntaxNodePtr> },
     }
 
     #[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<SyntaxNodePtr>,
+        ) -> 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(InactiveCode { 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
@@ -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
@@ -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;
@@ -31,6 +34,25 @@ pub struct Diagnostic {
     pub range: TextRange,
     pub severity: Severity,
     pub fix: Option<Fix>,
+    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<Fix>) -> Self {
+        Self { fix, ..self }
+    }
+
+    fn with_unused(self, unused: bool) -> Self {
+        Self { unused, ..self }
+    }
 }
 
 #[derive(Debug)]
@@ -71,13 +93,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);
@@ -100,6 +122,13 @@ pub(crate) fn diagnostics(
         .on::<hir::diagnostics::IncorrectCase, _>(|d| {
             res.borrow_mut().push(warning_with_fix(d, &sema));
         })
+        .on::<hir::diagnostics::InactiveCode, _>(|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()));
@@ -108,13 +137,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 +149,11 @@ pub(crate) fn diagnostics(
 }
 
 fn diagnostic_with_fix<D: DiagnosticWithFix>(d: &D, sema: &Semantics<RootDatabase>) -> 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: DiagnosticWithFix>(d: &D, sema: &Semantics<RootDatabase>) -> 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 +174,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 +588,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
@@ -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<Diagnostic>, 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
@@ -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)

Original file line number	Diff line number	Diff line change
`@@ -286,7 +286,7 @@ mod diagnostics {`
`286`	`286`	`use hir_expand::diagnostics::DiagnosticSink;`
`287`	`287`	`use hir_expand::hygiene::Hygiene;`
`288`	`288`	`use hir_expand::InFile;`
`289`		`- use syntax::{ast, AstPtr};`
	`289`	`+ use syntax::{ast, AstPtr, SyntaxNodePtr};`
`290`	`290`
`291`	`291`	`use crate::path::ModPath;`
`292`	`292`	`use crate::{db::DefDatabase, diagnostics::*, nameres::LocalModuleId, AstId};`
`@@ -298,6 +298,8 @@ mod diagnostics {`
`298`	`298`	`UnresolvedExternCrate { ast: AstId<ast::ExternCrate> },`
`299`	`299`
`300`	`300`	`UnresolvedImport { ast: AstId<ast::Use>, index: usize },`
	`301`	`+`
	`302`	`+ UnconfiguredCode { ast: InFile<SyntaxNodePtr> },`
`301`	`303`	`}`
`302`	`304`
`303`	`305`	`#[derive(Debug, PartialEq, Eq)]`
`@@ -336,6 +338,13 @@ mod diagnostics {`
`336`	`338`	`Self { in_module: container, kind: DiagnosticKind::UnresolvedImport { ast, index } }`
`337`	`339`	`}`
`338`	`340`
	`341`	`+ pub(super) fn unconfigured_code(`
	`342`	`+ container: LocalModuleId,`
	`343`	`+ ast: InFile<SyntaxNodePtr>,`
	`344`	`+ ) -> Self {`
	`345`	`+ Self { in_module: container, kind: DiagnosticKind::UnconfiguredCode { ast } }`
	`346`	`+ }`
	`347`	`+`
`339`	`348`	`pub(super) fn add_to(`
`340`	`349`	`&self,`
`341`	`350`	`db: &dyn DefDatabase,`
`@@ -385,6 +394,10 @@ mod diagnostics {`
`385`	`394`	`sink.push(UnresolvedImport { file: ast.file_id, node: AstPtr::new(&tree) });`
`386`	`395`	`}`
`387`	`396`	`}`
	`397`	`+`
	`398`	`+ DiagnosticKind::UnconfiguredCode { ast } => {`
	`399`	`+ sink.push(InactiveCode { file: ast.file_id, node: ast.value.clone() });`
	`400`	`+ }`
`388`	`401`	`}`
`389`	`402`	`}`
`390`	`403`	`}`