From 93dc6f511bedb7c18319bbf3efe47a7db4b2aa53 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 23 Oct 2020 19:27:04 +0200 Subject: Diagnose #[cfg]s in bodies --- crates/hir/src/code_model.rs | 1 + crates/hir_def/src/body.rs | 63 +++++++-------------- crates/hir_def/src/body/diagnostics.rs | 20 +++++++ crates/hir_def/src/body/lower.rs | 60 +++++++++++++++----- crates/hir_def/src/body/tests.rs | 75 +++++++++++++++++++++++++ crates/hir_def/src/diagnostics.rs | 11 +++- crates/hir_def/src/nameres/tests/diagnostics.rs | 34 +---------- crates/hir_def/src/test_db.rs | 44 ++++++++++++++- 8 files changed, 214 insertions(+), 94 deletions(-) create mode 100644 crates/hir_def/src/body/diagnostics.rs create mode 100644 crates/hir_def/src/body/tests.rs (limited to 'crates') diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs index 7f169ccd2..864f9c0c8 100644 --- a/crates/hir/src/code_model.rs +++ b/crates/hir/src/code_model.rs @@ -781,6 +781,7 @@ impl Function { } pub fn diagnostics(self, db: &dyn HirDatabase, sink: &mut DiagnosticSink) { + hir_def::diagnostics::validate_body(db.upcast(), self.id.into(), sink); hir_ty::diagnostics::validate_module_item(db, self.id.into(), sink); hir_ty::diagnostics::validate_body(db, self.id.into(), sink); } diff --git a/crates/hir_def/src/body.rs b/crates/hir_def/src/body.rs index d51036e4f..d10b1af01 100644 --- a/crates/hir_def/src/body.rs +++ b/crates/hir_def/src/body.rs @@ -1,6 +1,9 @@ //! Defines `Body`: a lowered representation of bodies of functions, statics and //! consts. mod lower; +mod diagnostics; +#[cfg(test)] +mod tests; pub mod scope; use std::{mem, ops::Index, sync::Arc}; @@ -10,7 +13,10 @@ use base_db::CrateId; use cfg::CfgOptions; use drop_bomb::DropBomb; use either::Either; -use hir_expand::{ast_id_map::AstIdMap, hygiene::Hygiene, AstId, HirFileId, InFile, MacroDefId}; +use hir_expand::{ + ast_id_map::AstIdMap, diagnostics::DiagnosticSink, hygiene::Hygiene, AstId, HirFileId, InFile, + MacroDefId, +}; use rustc_hash::FxHashMap; use syntax::{ast, AstNode, AstPtr}; use test_utils::mark; @@ -150,8 +156,12 @@ impl Expander { InFile { file_id: self.current_file_id, value } } - pub(crate) fn is_cfg_enabled(&self, owner: &dyn ast::AttrsOwner) -> bool { - self.cfg_expander.is_cfg_enabled(owner) + pub(crate) fn parse_attrs(&self, owner: &dyn ast::AttrsOwner) -> Attrs { + self.cfg_expander.parse_attrs(owner) + } + + pub(crate) fn cfg_options(&self) -> &CfgOptions { + &self.cfg_expander.cfg_options } fn parse_path(&mut self, path: ast::Path) -> Option { @@ -219,6 +229,10 @@ pub struct BodySourceMap { pat_map_back: ArenaMap>, field_map: FxHashMap<(ExprId, usize), InFile>>, expansions: FxHashMap>, HirFileId>, + + /// Diagnostics accumulated during body lowering. These contain `AstPtr`s and so are stored in + /// the source map (since they're just as volatile). + diagnostics: Vec, } #[derive(Default, Debug, Eq, PartialEq, Clone, Copy)] @@ -318,45 +332,10 @@ impl BodySourceMap { pub fn field_syntax(&self, expr: ExprId, field: usize) -> InFile> { self.field_map[&(expr, field)].clone() } -} - -#[cfg(test)] -mod tests { - use base_db::{fixture::WithFixture, SourceDatabase}; - use test_utils::mark; - use crate::ModuleDefId; - - use super::*; - - fn lower(ra_fixture: &str) -> Arc { - let (db, file_id) = crate::test_db::TestDB::with_single_file(ra_fixture); - - let krate = db.crate_graph().iter().next().unwrap(); - let def_map = db.crate_def_map(krate); - let module = def_map.modules_for_file(file_id).next().unwrap(); - let module = &def_map[module]; - let fn_def = match module.scope.declarations().next().unwrap() { - ModuleDefId::FunctionId(it) => it, - _ => panic!(), - }; - - db.body(fn_def.into()) - } - - #[test] - fn your_stack_belongs_to_me() { - mark::check!(your_stack_belongs_to_me); - lower( - " -macro_rules! n_nuple { - ($e:tt) => (); - ($($rest:tt)*) => {{ - (n_nuple!($($rest)*)None,) - }}; -} -fn main() { n_nuple!(1,2,3); } -", - ); + pub(crate) fn add_diagnostics(&self, _db: &dyn DefDatabase, sink: &mut DiagnosticSink<'_>) { + for diag in &self.diagnostics { + diag.add_to(sink); + } } } diff --git a/crates/hir_def/src/body/diagnostics.rs b/crates/hir_def/src/body/diagnostics.rs new file mode 100644 index 000000000..cfa47d189 --- /dev/null +++ b/crates/hir_def/src/body/diagnostics.rs @@ -0,0 +1,20 @@ +//! Diagnostics emitted during body lowering. + +use hir_expand::diagnostics::DiagnosticSink; + +use crate::diagnostics::InactiveCode; + +#[derive(Debug, Eq, PartialEq)] +pub enum BodyDiagnostic { + InactiveCode(InactiveCode), +} + +impl BodyDiagnostic { + pub fn add_to(&self, sink: &mut DiagnosticSink<'_>) { + match self { + BodyDiagnostic::InactiveCode(diag) => { + sink.push(diag.clone()); + } + } + } +} diff --git a/crates/hir_def/src/body/lower.rs b/crates/hir_def/src/body/lower.rs index 01e72690a..ddc267b83 100644 --- a/crates/hir_def/src/body/lower.rs +++ b/crates/hir_def/src/body/lower.rs @@ -16,7 +16,7 @@ use syntax::{ self, ArgListOwner, ArrayExprKind, AstChildren, LiteralKind, LoopBodyOwner, NameOwner, SlicePatComponents, }, - AstNode, AstPtr, + AstNode, AstPtr, SyntaxNodePtr, }; use test_utils::mark; @@ -25,6 +25,7 @@ use crate::{ body::{Body, BodySourceMap, Expander, PatPtr, SyntheticSyntax}, builtin_type::{BuiltinFloat, BuiltinInt}, db::DefDatabase, + diagnostics::InactiveCode, expr::{ dummy_expr_id, ArithOp, Array, BinaryOp, BindingAnnotation, CmpOp, Expr, ExprId, Literal, LogicOp, MatchArm, Ordering, Pat, PatId, RecordFieldPat, RecordLitField, Statement, @@ -37,7 +38,7 @@ use crate::{ StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc, }; -use super::{ExprSource, PatSource}; +use super::{diagnostics::BodyDiagnostic, ExprSource, PatSource}; pub(crate) struct LowerCtx { hygiene: Hygiene, @@ -176,7 +177,7 @@ impl ExprCollector<'_> { fn collect_expr(&mut self, expr: ast::Expr) -> ExprId { let syntax_ptr = AstPtr::new(&expr); - if !self.expander.is_cfg_enabled(&expr) { + if self.check_cfg(&expr).is_none() { return self.missing_expr(); } @@ -354,13 +355,15 @@ impl ExprCollector<'_> { let arms = if let Some(match_arm_list) = e.match_arm_list() { match_arm_list .arms() - .map(|arm| MatchArm { - pat: self.collect_pat_opt(arm.pat()), - expr: self.collect_expr_opt(arm.expr()), - guard: arm - .guard() - .and_then(|guard| guard.expr()) - .map(|e| self.collect_expr(e)), + .filter_map(|arm| { + self.check_cfg(&arm).map(|()| MatchArm { + pat: self.collect_pat_opt(arm.pat()), + expr: self.collect_expr_opt(arm.expr()), + guard: arm + .guard() + .and_then(|guard| guard.expr()) + .map(|e| self.collect_expr(e)), + }) }) .collect() } else { @@ -406,9 +409,8 @@ impl ExprCollector<'_> { .fields() .inspect(|field| field_ptrs.push(AstPtr::new(field))) .filter_map(|field| { - if !self.expander.is_cfg_enabled(&field) { - return None; - } + self.check_cfg(&field)?; + let name = field.field_name()?.as_name(); Some(RecordLitField { @@ -620,15 +622,23 @@ impl ExprCollector<'_> { .filter_map(|s| { let stmt = match s { ast::Stmt::LetStmt(stmt) => { + self.check_cfg(&stmt)?; + let pat = self.collect_pat_opt(stmt.pat()); let type_ref = stmt.ty().map(|it| TypeRef::from_ast(&self.ctx(), it)); let initializer = stmt.initializer().map(|e| self.collect_expr(e)); Statement::Let { pat, type_ref, initializer } } ast::Stmt::ExprStmt(stmt) => { + self.check_cfg(&stmt)?; + Statement::Expr(self.collect_expr_opt(stmt.expr())) } - ast::Stmt::Item(_) => return None, + ast::Stmt::Item(item) => { + self.check_cfg(&item)?; + + return None; + } }; Some(stmt) }) @@ -872,6 +882,28 @@ impl ExprCollector<'_> { (args, ellipsis) } + + /// Returns `None` (and emits diagnostics) when `owner` if `#[cfg]`d out, and `Some(())` when + /// not. + fn check_cfg(&mut self, owner: &dyn ast::AttrsOwner) -> Option<()> { + match self.expander.parse_attrs(owner).cfg() { + Some(cfg) => { + if self.expander.cfg_options().check(&cfg) != Some(false) { + return Some(()); + } + + self.source_map.diagnostics.push(BodyDiagnostic::InactiveCode(InactiveCode { + file: self.expander.current_file_id, + node: SyntaxNodePtr::new(owner.syntax()), + cfg, + opts: self.expander.cfg_options().clone(), + })); + + None + } + None => Some(()), + } + } } impl From for BinaryOp { diff --git a/crates/hir_def/src/body/tests.rs b/crates/hir_def/src/body/tests.rs new file mode 100644 index 000000000..f07df5cee --- /dev/null +++ b/crates/hir_def/src/body/tests.rs @@ -0,0 +1,75 @@ +use base_db::{fixture::WithFixture, SourceDatabase}; +use test_utils::mark; + +use crate::{test_db::TestDB, ModuleDefId}; + +use super::*; + +fn lower(ra_fixture: &str) -> Arc { + let (db, file_id) = crate::test_db::TestDB::with_single_file(ra_fixture); + + let krate = db.crate_graph().iter().next().unwrap(); + let def_map = db.crate_def_map(krate); + let module = def_map.modules_for_file(file_id).next().unwrap(); + let module = &def_map[module]; + let fn_def = match module.scope.declarations().next().unwrap() { + ModuleDefId::FunctionId(it) => it, + _ => panic!(), + }; + + db.body(fn_def.into()) +} + +fn check_diagnostics(ra_fixture: &str) { + let db: TestDB = TestDB::with_files(ra_fixture); + db.check_diagnostics(); +} + +#[test] +fn your_stack_belongs_to_me() { + mark::check!(your_stack_belongs_to_me); + lower( + " +macro_rules! n_nuple { + ($e:tt) => (); + ($($rest:tt)*) => {{ + (n_nuple!($($rest)*)None,) + }}; +} +fn main() { n_nuple!(1,2,3); } +", + ); +} + +#[test] +fn cfg_diagnostics() { + check_diagnostics( + r" +fn f() { + // The three g̶e̶n̶d̶e̶r̶s̶ statements: + + #[cfg(a)] fn f() {} // Item statement + //^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled + #[cfg(a)] {} // Expression statement + //^^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled + #[cfg(a)] let x = 0; // let statement + //^^^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled + + abc(#[cfg(a)] 0); + //^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled + let x = Struct { + #[cfg(a)] f: 0, + //^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled + }; + match () { + () => (), + #[cfg(a)] () => (), + //^^^^^^^^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled + } + + #[cfg(a)] 0 // Trailing expression of block + //^^^^^^^^^^^ code is inactive due to #[cfg] directives: a is disabled +} + ", + ); +} diff --git a/crates/hir_def/src/diagnostics.rs b/crates/hir_def/src/diagnostics.rs index 532496b62..90d9cdcba 100644 --- a/crates/hir_def/src/diagnostics.rs +++ b/crates/hir_def/src/diagnostics.rs @@ -4,10 +4,17 @@ use std::any::Any; use stdx::format_to; use cfg::{CfgExpr, CfgOptions, DnfExpr}; -use hir_expand::diagnostics::{Diagnostic, DiagnosticCode}; +use hir_expand::diagnostics::{Diagnostic, DiagnosticCode, DiagnosticSink}; use hir_expand::{HirFileId, InFile}; use syntax::{ast, AstPtr, SyntaxNodePtr}; +use crate::{db::DefDatabase, DefWithBodyId}; + +pub fn validate_body(db: &dyn DefDatabase, owner: DefWithBodyId, sink: &mut DiagnosticSink<'_>) { + let source_map = db.body_with_source_map(owner).1; + source_map.add_diagnostics(db, sink); +} + // Diagnostic: unresolved-module // // This diagnostic is triggered if rust-analyzer is unable to discover referred module. @@ -91,7 +98,7 @@ impl Diagnostic for UnresolvedImport { // Diagnostic: unconfigured-code // // This diagnostic is shown for code with inactive `#[cfg]` attributes. -#[derive(Debug)] +#[derive(Debug, Clone, Eq, PartialEq)] pub struct InactiveCode { pub file: HirFileId, pub node: SyntaxNodePtr, diff --git a/crates/hir_def/src/nameres/tests/diagnostics.rs b/crates/hir_def/src/nameres/tests/diagnostics.rs index 5972248de..1a7b98831 100644 --- a/crates/hir_def/src/nameres/tests/diagnostics.rs +++ b/crates/hir_def/src/nameres/tests/diagnostics.rs @@ -1,42 +1,10 @@ use base_db::fixture::WithFixture; -use base_db::FileId; -use base_db::SourceDatabaseExt; -use hir_expand::db::AstDatabase; -use rustc_hash::FxHashMap; -use syntax::TextRange; -use syntax::TextSize; use crate::test_db::TestDB; fn check_diagnostics(ra_fixture: &str) { let db: TestDB = TestDB::with_files(ra_fixture); - let annotations = db.extract_annotations(); - assert!(!annotations.is_empty()); - - let mut actual: FxHashMap> = FxHashMap::default(); - db.diagnostics(|d| { - let src = d.display_source(); - let root = db.parse_or_expand(src.file_id).unwrap(); - // FIXME: macros... - let file_id = src.file_id.original_file(&db); - let range = src.value.to_node(&root).text_range(); - let message = d.message().to_owned(); - actual.entry(file_id).or_default().push((range, message)); - }); - - for (file_id, diags) in actual.iter_mut() { - diags.sort_by_key(|it| it.0.start()); - let text = db.file_text(*file_id); - // For multiline spans, place them on line start - for (range, content) in diags { - if text[*range].contains('\n') { - *range = TextRange::new(range.start(), range.start() + TextSize::from(1)); - *content = format!("... {}", content); - } - } - } - - assert_eq!(annotations, actual); + db.check_diagnostics(); } #[test] diff --git a/crates/hir_def/src/test_db.rs b/crates/hir_def/src/test_db.rs index fb1d3c974..2b36c824a 100644 --- a/crates/hir_def/src/test_db.rs +++ b/crates/hir_def/src/test_db.rs @@ -12,10 +12,10 @@ use hir_expand::diagnostics::Diagnostic; use hir_expand::diagnostics::DiagnosticSinkBuilder; use rustc_hash::FxHashMap; use rustc_hash::FxHashSet; -use syntax::TextRange; +use syntax::{TextRange, TextSize}; use test_utils::extract_annotations; -use crate::db::DefDatabase; +use crate::{db::DefDatabase, ModuleDefId}; #[salsa::database( base_db::SourceDatabaseExtStorage, @@ -135,9 +135,47 @@ impl TestDB { let crate_def_map = self.crate_def_map(krate); let mut sink = DiagnosticSinkBuilder::new().build(&mut cb); - for (module_id, _) in crate_def_map.modules.iter() { + for (module_id, module) in crate_def_map.modules.iter() { crate_def_map.add_diagnostics(self, module_id, &mut sink); + + for decl in module.scope.declarations() { + if let ModuleDefId::FunctionId(it) = decl { + let source_map = self.body_with_source_map(it.into()).1; + source_map.add_diagnostics(self, &mut sink); + } + } } } } + + pub fn check_diagnostics(&self) { + let db: &TestDB = self; + let annotations = db.extract_annotations(); + assert!(!annotations.is_empty()); + + let mut actual: FxHashMap> = FxHashMap::default(); + db.diagnostics(|d| { + let src = d.display_source(); + let root = db.parse_or_expand(src.file_id).unwrap(); + // FIXME: macros... + let file_id = src.file_id.original_file(db); + let range = src.value.to_node(&root).text_range(); + let message = d.message().to_owned(); + actual.entry(file_id).or_default().push((range, message)); + }); + + for (file_id, diags) in actual.iter_mut() { + diags.sort_by_key(|it| it.0.start()); + let text = db.file_text(*file_id); + // For multiline spans, place them on line start + for (range, content) in diags { + if text[*range].contains('\n') { + *range = TextRange::new(range.start(), range.start() + TextSize::from(1)); + *content = format!("... {}", content); + } + } + } + + assert_eq!(annotations, actual); + } } -- cgit v1.2.3