From 753449bdaeb5382c48cebd60fa58c660dd06b491 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Fri, 5 Dec 2025 07:04:18 +0200 Subject: [PATCH] Handle lint attributes via hir-expand attr handling This avoids code duplication. --- crates/hir/src/lib.rs | 4 +- crates/hir/src/semantics.rs | 66 ++++++++++++++++- crates/ide-diagnostics/src/lib.rs | 119 ++++++------------------------ 3 files changed, 90 insertions(+), 99 deletions(-) diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 2210bb79cd..63da5a3a37 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -117,8 +117,8 @@ pub use crate::{ diagnostics::*, has_source::HasSource, semantics::{ - PathResolution, PathResolutionPerNs, Semantics, SemanticsImpl, SemanticsScope, TypeInfo, - VisibleTraits, + LintAttr, PathResolution, PathResolutionPerNs, Semantics, SemanticsImpl, SemanticsScope, + TypeInfo, VisibleTraits, }, }; diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs index 82e60bff5e..195fafad5c 100644 --- a/crates/hir/src/semantics.rs +++ b/crates/hir/src/semantics.rs @@ -39,8 +39,8 @@ use smallvec::{SmallVec, smallvec}; use span::{FileId, SyntaxContext}; use stdx::{TupleExt, always}; use syntax::{ - AstNode, AstToken, Direction, SyntaxKind, SyntaxNode, SyntaxNodePtr, SyntaxToken, TextRange, - TextSize, + AstNode, AstToken, Direction, SmolStr, SmolStrBuilder, SyntaxElement, SyntaxKind, SyntaxNode, + SyntaxNodePtr, SyntaxToken, T, TextRange, TextSize, algo::skip_trivia_token, ast::{self, HasAttrs as _, HasGenericParams}, }; @@ -174,6 +174,15 @@ impl<'db, DB: ?Sized> ops::Deref for Semantics<'db, DB> { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum LintAttr { + Allow, + Expect, + Warn, + Deny, + Forbid, +} + // Note: while this variant of `Semantics<'_, _>` might seem unused, as it does not // find actual use within the rust-analyzer project itself, it exists to enable the use // within e.g. tracked salsa functions in third-party crates that build upon `ra_ap_hir`. @@ -254,6 +263,59 @@ impl Semantics<'_, DB> { .filter_map(ast::NameLike::cast) } + pub fn lint_attrs( + &self, + krate: Crate, + item: ast::AnyHasAttrs, + ) -> impl Iterator { + let mut cfg_options = None; + let cfg_options = || *cfg_options.get_or_insert_with(|| krate.id.cfg_options(self.db)); + let mut result = Vec::new(); + hir_expand::attrs::expand_cfg_attr::( + ast::attrs_including_inner(&item), + cfg_options, + |attr, _, _, _| { + let hir_expand::attrs::Meta::TokenTree { path, tt } = attr else { + return ControlFlow::Continue(()); + }; + if path.segments.len() != 1 { + return ControlFlow::Continue(()); + } + let lint_attr = match path.segments[0].text() { + "allow" => LintAttr::Allow, + "expect" => LintAttr::Expect, + "warn" => LintAttr::Warn, + "deny" => LintAttr::Deny, + "forbid" => LintAttr::Forbid, + _ => return ControlFlow::Continue(()), + }; + let mut lint = SmolStrBuilder::new(); + for token in + tt.syntax().children_with_tokens().filter_map(SyntaxElement::into_token) + { + match token.kind() { + T![:] | T![::] => lint.push_str(token.text()), + kind if kind.is_any_identifier() => lint.push_str(token.text()), + T![,] => { + let lint = mem::replace(&mut lint, SmolStrBuilder::new()).finish(); + if !lint.is_empty() { + result.push((lint_attr, lint)); + } + } + _ => {} + } + } + let lint = lint.finish(); + if !lint.is_empty() { + result.push((lint_attr, lint)); + } + + ControlFlow::Continue(()) + }, + ); + result.into_iter() + } + pub fn resolve_range_pat(&self, range_pat: &ast::RangePat) -> Option { self.imp.resolve_range_pat(range_pat).map(Struct::from) } diff --git a/crates/ide-diagnostics/src/lib.rs b/crates/ide-diagnostics/src/lib.rs index 41ae854455..c7827f13f4 100644 --- a/crates/ide-diagnostics/src/lib.rs +++ b/crates/ide-diagnostics/src/lib.rs @@ -83,9 +83,8 @@ mod handlers { #[cfg(test)] mod tests; -use std::{iter, sync::LazyLock}; +use std::sync::LazyLock; -use either::Either; use hir::{ Crate, DisplayTarget, InFile, Semantics, db::ExpandDatabase, diagnostics::AnyDiagnostic, }; @@ -97,11 +96,9 @@ use ide_db::{ imports::insert_use::InsertUseConfig, label::Label, source_change::SourceChange, - syntax_helpers::node_ext::parse_tt_as_comma_sep_paths, }; -use itertools::Itertools; use syntax::{ - AstPtr, Edition, NodeOrToken, SmolStr, SyntaxKind, SyntaxNode, SyntaxNodePtr, T, TextRange, + AstPtr, Edition, SmolStr, SyntaxNode, SyntaxNodePtr, TextRange, ast::{self, AstNode}, }; @@ -483,7 +480,7 @@ pub fn semantic_diagnostics( // The edition isn't accurate (each diagnostics may have its own edition due to macros), // but it's okay as it's only being used for error recovery. - handle_lints(&ctx.sema, &mut lints, editioned_file_id.edition(db)); + handle_lints(&ctx.sema, krate, &mut lints, editioned_file_id.edition(db)); res.retain(|d| d.severity != Severity::Allow); @@ -591,6 +588,7 @@ fn build_lints_map( fn handle_lints( sema: &Semantics<'_, RootDatabase>, + krate: hir::Crate, diagnostics: &mut [(InFile, &mut Diagnostic)], edition: Edition, ) { @@ -606,10 +604,10 @@ fn handle_lints( } let mut diag_severity = - lint_severity_at(sema, node, &lint_groups(&diag.code, edition), edition); + lint_severity_at(sema, krate, node, &lint_groups(&diag.code, edition)); if let outline_diag_severity @ Some(_) = - find_outline_mod_lint_severity(sema, node, diag, edition) + find_outline_mod_lint_severity(sema, krate, node, diag, edition) { diag_severity = outline_diag_severity; } @@ -632,6 +630,7 @@ fn default_lint_severity(lint: &Lint, edition: Edition) -> Severity { fn find_outline_mod_lint_severity( sema: &Semantics<'_, RootDatabase>, + krate: hir::Crate, node: &InFile, diag: &Diagnostic, edition: Edition, @@ -648,8 +647,8 @@ fn find_outline_mod_lint_severity( let lint_groups = lint_groups(&diag.code, edition); lint_attrs( sema, - &ast::AnyHasAttrs::cast(module_source_file.value).expect("SourceFile always has attrs"), - edition, + krate, + ast::AnyHasAttrs::cast(module_source_file.value).expect("SourceFile always has attrs"), ) .for_each(|(lint, severity)| { if lint_groups.contains(&lint) { @@ -661,106 +660,36 @@ fn find_outline_mod_lint_severity( fn lint_severity_at( sema: &Semantics<'_, RootDatabase>, + krate: hir::Crate, node: &InFile, lint_groups: &LintGroups, - edition: Edition, ) -> Option { node.value .ancestors() .filter_map(ast::AnyHasAttrs::cast) .find_map(|ancestor| { - lint_attrs(sema, &ancestor, edition) + lint_attrs(sema, krate, ancestor) .find_map(|(lint, severity)| lint_groups.contains(&lint).then_some(severity)) }) .or_else(|| { - lint_severity_at(sema, &sema.find_parent_file(node.file_id)?, lint_groups, edition) + lint_severity_at(sema, krate, &sema.find_parent_file(node.file_id)?, lint_groups) }) } // FIXME: Switch this to analysis' `expand_cfg_attr`. -fn lint_attrs<'a>( - sema: &'a Semantics<'a, RootDatabase>, - ancestor: &'a ast::AnyHasAttrs, - edition: Edition, -) -> impl Iterator + 'a { - ast::attrs_including_inner(ancestor) - .filter_map(|attr| { - attr.as_simple_call().and_then(|(name, value)| match &*name { - "allow" | "expect" => Some(Either::Left(iter::once((Severity::Allow, value)))), - "warn" => Some(Either::Left(iter::once((Severity::Warning, value)))), - "forbid" | "deny" => Some(Either::Left(iter::once((Severity::Error, value)))), - "cfg_attr" => { - let mut lint_attrs = Vec::new(); - cfg_attr_lint_attrs(sema, &value, &mut lint_attrs); - Some(Either::Right(lint_attrs.into_iter())) - } - _ => None, - }) - }) - .flatten() - .flat_map(move |(severity, lints)| { - parse_tt_as_comma_sep_paths(lints, edition).into_iter().flat_map(move |lints| { - // Rejoin the idents with `::`, so we have no spaces in between. - lints.into_iter().map(move |lint| { - ( - lint.segments().filter_map(|segment| segment.name_ref()).join("::").into(), - severity, - ) - }) - }) - }) -} - -fn cfg_attr_lint_attrs( +fn lint_attrs( sema: &Semantics<'_, RootDatabase>, - value: &ast::TokenTree, - lint_attrs: &mut Vec<(Severity, ast::TokenTree)>, -) { - let prev_len = lint_attrs.len(); - - let mut iter = value.token_trees_and_tokens().filter(|it| match it { - NodeOrToken::Node(_) => true, - NodeOrToken::Token(it) => !it.kind().is_trivia(), - }); - - // Skip the condition. - for value in &mut iter { - if value.as_token().is_some_and(|it| it.kind() == T![,]) { - break; - } - } - - while let Some(value) = iter.next() { - if let Some(token) = value.as_token() - && token.kind() == SyntaxKind::IDENT - { - let severity = match token.text() { - "allow" | "expect" => Some(Severity::Allow), - "warn" => Some(Severity::Warning), - "forbid" | "deny" => Some(Severity::Error), - "cfg_attr" => { - if let Some(NodeOrToken::Node(value)) = iter.next() { - cfg_attr_lint_attrs(sema, &value, lint_attrs); - } - None - } - _ => None, - }; - if let Some(severity) = severity { - let lints = iter.next(); - if let Some(NodeOrToken::Node(lints)) = lints { - lint_attrs.push((severity, lints)); - } - } - } - } - - if prev_len != lint_attrs.len() - && let Some(false) | None = sema.check_cfg_attr(value) - { - // Discard the attributes when the condition is false. - lint_attrs.truncate(prev_len); - } + krate: hir::Crate, + ancestor: ast::AnyHasAttrs, +) -> impl Iterator { + sema.lint_attrs(krate, ancestor).map(|(lint_attr, lint)| { + let severity = match lint_attr { + hir::LintAttr::Allow | hir::LintAttr::Expect => Severity::Allow, + hir::LintAttr::Warn => Severity::Warning, + hir::LintAttr::Deny | hir::LintAttr::Forbid => Severity::Error, + }; + (lint, severity) + }) } #[derive(Debug)]