Skip to content

Fix Clippy handling of ExpnKind::Desugaring #73468

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
@@ -554,10 +554,7 @@ impl Step for Clippy {

builder.add_rustc_lib_path(compiler, &mut cargo);

// FIXME: Disable clippy tests for now, they're failing on master
// (generally this would mean a toolstate failure but we don't have
// toolstate for clippy anymore).
// builder.run(&mut cargo.into());
builder.run(&mut cargo.into());
}
}

27 changes: 15 additions & 12 deletions src/librustc_middle/lint.rs
Original file line number Diff line number Diff line change
@@ -7,8 +7,8 @@ use rustc_errors::{DiagnosticBuilder, DiagnosticId};
use rustc_hir::HirId;
use rustc_session::lint::{builtin, Level, Lint, LintId};
use rustc_session::{DiagnosticMessageId, Session};
use rustc_span::hygiene::MacroKind;
use rustc_span::source_map::{DesugaringKind, ExpnKind, MultiSpan};
use rustc_span::hygiene::{ClosestAstOrMacro, MacroKind};
use rustc_span::source_map::{ExpnKind, MultiSpan};
use rustc_span::{Span, Symbol};

/// How a lint level was set.
@@ -337,16 +337,19 @@ pub fn struct_lint_level<'s, 'd>(
/// This is used to test whether a lint should not even begin to figure out whether it should
/// be reported on the current node.
pub fn in_external_macro(sess: &Session, span: Span) -> bool {
let expn_data = span.ctxt().outer_expn_data();
match expn_data.kind {
ExpnKind::Root
| ExpnKind::Desugaring(DesugaringKind::ForLoop(_))
| ExpnKind::Desugaring(DesugaringKind::Operator) => false,
ExpnKind::AstPass(_) | ExpnKind::Desugaring(_) => true, // well, it's "external"
ExpnKind::Macro(MacroKind::Bang, _) => {
// Dummy span for the `def_site` means it's an external macro.
expn_data.def_site.is_dummy() || sess.source_map().is_imported(expn_data.def_site)
match span.ctxt().outer_expn().closest_ast_or_macro() {
ClosestAstOrMacro::Expn(expn_id) => {
let data = expn_id.expn_data();
match data.kind {
ExpnKind::Macro(MacroKind::Bang, _) => {
// Dummy span for the `def_site` means it's an external macro.
data.def_site.is_dummy() || sess.source_map().is_imported(data.def_site)
}
ExpnKind::Macro(_, _) => true, // definitely a plugin
ExpnKind::AstPass(_) => true, // well, it's "External"
_ => unreachable!("unexpected ExpnData {:?}", data),
}
}
ExpnKind::Macro(..) => true, // definitely a plugin
ClosestAstOrMacro::None => false,
}
}
62 changes: 61 additions & 1 deletion src/librustc_span/hygiene.rs
Original file line number Diff line number Diff line change
@@ -104,9 +104,21 @@ impl ExpnId {
HygieneData::with(|data| data.expn_data(self).clone())
}

/// Retrieves the `ClosestAstOrMacro` associated with this `ExpnId`
/// See `ClosestAstOrMacro` for more details
#[inline]
pub fn closest_ast_or_macro(self) -> ClosestAstOrMacro {
HygieneData::with(|data| data.closest_ast_or_macro(self))
}

#[inline]
pub fn set_expn_data(self, expn_data: ExpnData) {
HygieneData::with(|data| {
let closest = data.determine_closest_ast_or_macro(self, &expn_data);
let old_closest = &mut data.closest_ast_or_macro[self.0 as usize];
assert!(old_closest.is_none(), "closest ast/macro data reset for an expansion ID");
*old_closest = Some(closest);

let old_expn_data = &mut data.expn_data[self.0 as usize];
assert!(old_expn_data.is_none(), "expansion data is reset for an expansion ID");
*old_expn_data = Some(expn_data);
@@ -149,6 +161,10 @@ crate struct HygieneData {
/// between creation of an expansion ID and obtaining its data (e.g. macros are collected
/// first and then resolved later), so we use an `Option` here.
expn_data: Vec<Option<ExpnData>>,
/// Stores the computed `ClosestAstOrMacro` for each `ExpnId`. This is updated
/// at the same time as `expn_data`, and its contents it determined entirely
/// by the `ExpnData` - this field is just a cache.
closest_ast_or_macro: Vec<Option<ClosestAstOrMacro>>,
syntax_context_data: Vec<SyntaxContextData>,
syntax_context_map: FxHashMap<(SyntaxContext, ExpnId, Transparency), SyntaxContext>,
}
@@ -162,6 +178,7 @@ impl HygieneData {
edition,
Some(DefId::local(CRATE_DEF_INDEX)),
))],
closest_ast_or_macro: vec![Some(ClosestAstOrMacro::None)],
syntax_context_data: vec![SyntaxContextData {
outer_expn: ExpnId::root(),
outer_transparency: Transparency::Opaque,
@@ -178,9 +195,36 @@ impl HygieneData {
GLOBALS.with(|globals| f(&mut *globals.hygiene_data.borrow_mut()))
}

fn determine_closest_ast_or_macro(
&self,
id: ExpnId,
expn_data: &ExpnData,
) -> ClosestAstOrMacro {
match expn_data.kind {
ExpnKind::Macro(_, _) | ExpnKind::AstPass(_) => ClosestAstOrMacro::Expn(id),
ExpnKind::Desugaring(_) | ExpnKind::Root => {
// Avoid using `HygieneData` when construction root
// `ExpnData`
if expn_data.call_site.ctxt() == SyntaxContext::root() {
ClosestAstOrMacro::None
} else {
self.closest_ast_or_macro(self.outer_expn(expn_data.call_site.ctxt()))
}
}
}
}

fn closest_ast_or_macro(&self, expn_id: ExpnId) -> ClosestAstOrMacro {
self.closest_ast_or_macro[expn_id.0 as usize].as_ref().copied().unwrap()
}

fn fresh_expn(&mut self, expn_data: Option<ExpnData>) -> ExpnId {
let expn_id = ExpnId(self.expn_data.len() as u32);
self.closest_ast_or_macro.push(
expn_data.as_ref().map(|data| self.determine_closest_ast_or_macro(expn_id, data)),
);
self.expn_data.push(expn_data);
ExpnId(self.expn_data.len() as u32 - 1)
expn_id
}

fn expn_data(&self, expn_id: ExpnId) -> &ExpnData {
@@ -684,6 +728,22 @@ pub struct ExpnData {
pub macro_def_id: Option<DefId>,
}

/// The closest `ExpnKind::AstPass` or `ExpnKind::Macro` to an `ExpnData`.
/// 'Closest' is determined by starting at the current `ExpnData`,
/// and walking up the `call_site` tree. If an `ExpnData` with
/// `Expn::AstPass` or `ExpnKind::Macro` is found, it is represented
/// by `ClosestAstOrMacro::Expn(id)`, where `id` is the `EpxId` of
/// the found `ExpnData`.
///
/// A `ClosestAstOrMacro` implies that no `ExpnKind::AstPass` or `ExpnKind::Macro`
/// are found anywhere in the `call_site` tree - that is, there no macro
/// expansions or ast pass expansions.
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub enum ClosestAstOrMacro {
None,
Expn(ExpnId),
}

impl ExpnData {
/// Constructs expansion data with default properties.
pub fn default(
6 changes: 3 additions & 3 deletions src/tools/clippy/clippy_lints/src/assertions_on_constants.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::consts::{constant, Constant};
use crate::utils::paths;
use crate::utils::{paths, in_macro};
use crate::utils::{is_direct_expn_of, is_expn_of, match_function_call, snippet_opt, span_lint_and_help};
use if_chain::if_chain;
use rustc_ast::ast::LitKind;
@@ -67,7 +67,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
};

if let Some(debug_assert_span) = is_expn_of(e.span, "debug_assert") {
if debug_assert_span.from_expansion() {
if in_macro(debug_assert_span) {
return;
}
if_chain! {
@@ -79,7 +79,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
}
};
} else if let Some(assert_span) = is_direct_expn_of(e.span, "assert") {
if assert_span.from_expansion() {
if in_macro(assert_span) {
return;
}
if let Some(assert_match) = match_assert_with_message(&cx, e) {
7 changes: 2 additions & 5 deletions src/tools/clippy/clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
//! checks for attributes

use crate::reexport::Name;
use crate::utils::{
first_line_of_span, is_present_in_source, match_def_path, paths, snippet_opt, span_lint, span_lint_and_sugg,
span_lint_and_then, without_block_comments,
};
use crate::utils::{first_line_of_span, is_present_in_source, match_def_path, paths, snippet_opt, span_lint, span_lint_and_sugg, span_lint_and_then, without_block_comments, in_macro};
use if_chain::if_chain;
use rustc_ast::ast::{AttrKind, AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem};
use rustc_ast::util::lev_distance::find_best_match_for_name;
@@ -474,7 +471,7 @@ fn is_relevant_expr(cx: &LateContext<'_, '_>, tables: &ty::TypeckTables<'_>, exp
}

fn check_attrs(cx: &LateContext<'_, '_>, span: Span, name: Name, attrs: &[Attribute]) {
if span.from_expansion() {
if in_macro(span) {
return;
}

8 changes: 4 additions & 4 deletions src/tools/clippy/clippy_lints/src/blocks_in_if_conditions.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{differing_macro_contexts, higher, snippet_block_with_applicability, span_lint, span_lint_and_sugg};
use crate::utils::{differing_macro_contexts, higher, snippet_block_with_applicability, span_lint, span_lint_and_sugg, in_macro};
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::{BlockCheckMode, Expr, ExprKind};
@@ -55,7 +55,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ExVisitor<'a, 'tcx> {
if let ExprKind::Closure(_, _, eid, _, _) = expr.kind {
let body = self.cx.tcx.hir().body(eid);
let ex = &body.value;
if matches!(ex.kind, ExprKind::Block(_, _)) && !body.value.span.from_expansion() {
if matches!(ex.kind, ExprKind::Block(_, _)) && !in_macro(body.value.span) {
self.found_block = Some(ex);
return;
}
@@ -83,7 +83,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlocksInIfConditions {
if let Some(ex) = &block.expr {
// don't dig into the expression here, just suggest that they remove
// the block
if expr.span.from_expansion() || differing_macro_contexts(expr.span, ex.span) {
if in_macro(expr.span) || differing_macro_contexts(expr.span, ex.span) {
return;
}
let mut applicability = Applicability::MachineApplicable;
@@ -108,7 +108,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlocksInIfConditions {
}
} else {
let span = block.expr.as_ref().map_or_else(|| block.stmts[0].span, |e| e.span);
if span.from_expansion() || differing_macro_contexts(expr.span, span) {
if in_macro(span) || differing_macro_contexts(expr.span, span) {
return;
}
// move block higher
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/booleans.rs
Original file line number Diff line number Diff line change
@@ -107,7 +107,7 @@ impl<'a, 'tcx, 'v> Hir2Qmm<'a, 'tcx, 'v> {
}

// prevent folding of `cfg!` macros and the like
if !e.span.from_expansion() {
if !in_macro(e.span) {
match &e.kind {
ExprKind::Unary(UnOp::UnNot, inner) => return Ok(Bool::Not(box self.run(inner)?)),
ExprKind::Binary(binop, lhs, rhs) => match &binop.node {
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/cognitive_complexity.rs
Original file line number Diff line number Diff line change
@@ -9,7 +9,7 @@ use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::source_map::Span;
use rustc_span::BytePos;

use crate::utils::{is_type_diagnostic_item, snippet_opt, span_lint_and_help, LimitStack};
use crate::utils::{is_type_diagnostic_item, snippet_opt, span_lint_and_help, LimitStack, in_macro};

declare_clippy_lint! {
/// **What it does:** Checks for methods with high cognitive complexity.
@@ -51,7 +51,7 @@ impl CognitiveComplexity {
body: &'tcx Body<'_>,
body_span: Span,
) {
if body_span.from_expansion() {
if in_macro(body_span) {
return;
}

6 changes: 3 additions & 3 deletions src/tools/clippy/clippy_lints/src/collapsible_if.rs
Original file line number Diff line number Diff line change
@@ -18,7 +18,7 @@ use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};

use crate::utils::sugg::Sugg;
use crate::utils::{snippet_block, snippet_block_with_applicability, span_lint_and_sugg, span_lint_and_then};
use crate::utils::{snippet_block, snippet_block_with_applicability, span_lint_and_sugg, span_lint_and_then, in_macro};
use rustc_errors::Applicability;

declare_clippy_lint! {
@@ -75,7 +75,7 @@ declare_lint_pass!(CollapsibleIf => [COLLAPSIBLE_IF]);

impl EarlyLintPass for CollapsibleIf {
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) {
if !expr.span.from_expansion() {
if !in_macro(expr.span) {
check_if(cx, expr)
}
}
@@ -106,7 +106,7 @@ fn check_collapsible_maybe_if_let(cx: &EarlyContext<'_>, else_: &ast::Expr) {
if let ast::ExprKind::Block(ref block, _) = else_.kind;
if !block_starts_with_comment(cx, block);
if let Some(else_) = expr_block(block);
if !else_.span.from_expansion();
if !in_macro(else_.span);
if let ast::ExprKind::If(..) = else_.kind;
then {
let mut applicability = Applicability::MachineApplicable;
6 changes: 2 additions & 4 deletions src/tools/clippy/clippy_lints/src/comparison_chain.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use crate::utils::{
get_trait_def_id, if_sequence, implements_trait, parent_node_is_if_expr, paths, span_lint_and_help, SpanlessEq,
};
use crate::utils::{get_trait_def_id, if_sequence, implements_trait, parent_node_is_if_expr, paths, span_lint_and_help, SpanlessEq, in_macro};
use rustc_hir::{BinOpKind, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -54,7 +52,7 @@ declare_lint_pass!(ComparisonChain => [COMPARISON_CHAIN]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ComparisonChain {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
if expr.span.from_expansion() {
if in_macro(expr.span) {
return;
}

4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/copies.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{get_parent_expr, higher, if_sequence, snippet, span_lint_and_note, span_lint_and_then};
use crate::utils::{get_parent_expr, higher, if_sequence, snippet, span_lint_and_note, span_lint_and_then, in_macro};
use crate::utils::{SpanlessEq, SpanlessHash};
use rustc_data_structures::fx::FxHashMap;
use rustc_hir::{Arm, Block, Expr, ExprKind, MatchSource, Pat, PatKind};
@@ -153,7 +153,7 @@ declare_lint_pass!(CopyAndPaste => [IFS_SAME_COND, SAME_FUNCTIONS_IN_IF_CONDITIO

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CopyAndPaste {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
if !expr.span.from_expansion() {
if !in_macro(expr.span) {
// skip ifs directly in else, it will be checked in the parent if
if let Some(expr) = get_parent_expr(cx, expr) {
if let Some((_, _, Some(ref else_expr))) = higher::if_block(&expr) {
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/dereference.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{get_parent_expr, implements_trait, snippet, span_lint_and_sugg};
use crate::utils::{get_parent_expr, implements_trait, snippet, span_lint_and_sugg, in_macro};
use if_chain::if_chain;
use rustc_ast::util::parser::{ExprPrecedence, PREC_POSTFIX, PREC_PREFIX};
use rustc_errors::Applicability;
@@ -41,7 +41,7 @@ declare_lint_pass!(Dereferencing => [
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Dereferencing {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
if_chain! {
if !expr.span.from_expansion();
if !in_macro(expr.span);
if let ExprKind::MethodCall(ref method_name, _, ref args, _) = &expr.kind;
if args.len() == 1;

4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/double_parens.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::span_lint;
use crate::utils::{span_lint, in_macro};
use rustc_ast::ast::{Expr, ExprKind};
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -41,7 +41,7 @@ declare_lint_pass!(DoubleParens => [DOUBLE_PARENS]);

impl EarlyLintPass for DoubleParens {
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
if expr.span.from_expansion() {
if in_macro(expr.span) {
return;
}

4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/enum_variants.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! lint on enum variants that are prefixed or suffixed by the same characters

use crate::utils::{camel_case, is_present_in_source};
use crate::utils::{camel_case, is_present_in_source, in_macro};
use crate::utils::{span_lint, span_lint_and_help};
use rustc_ast::ast::{EnumDef, Item, ItemKind, VisibilityKind};
use rustc_lint::{EarlyContext, EarlyLintPass, Lint};
@@ -271,7 +271,7 @@ impl EarlyLintPass for EnumVariantNames {
let item_name = item.ident.name.as_str();
let item_name_chars = item_name.chars().count();
let item_camel = to_camel_case(&item_name);
if !item.span.from_expansion() && is_present_in_source(cx, item.span) {
if !in_macro(item.span) && is_present_in_source(cx, item.span) {
if let Some(&(ref mod_name, ref mod_camel)) = self.modules.last() {
// constants don't have surrounding modules
if !mod_camel.is_empty() {
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/eq_op.rs
Original file line number Diff line number Diff line change
@@ -56,7 +56,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EqOp {
#[allow(clippy::similar_names, clippy::too_many_lines)]
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr<'_>) {
if let ExprKind::Binary(op, ref left, ref right) = e.kind {
if e.span.from_expansion() {
if in_macro(e.span) {
return;
}
let macro_with_not_op = |expr_kind: &ExprKind<'_>| {
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/erasing_op.rs
Original file line number Diff line number Diff line change
@@ -4,7 +4,7 @@ use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;

use crate::consts::{constant_simple, Constant};
use crate::utils::span_lint;
use crate::utils::{span_lint, in_macro};

declare_clippy_lint! {
/// **What it does:** Checks for erasing operations, e.g., `x * 0`.
@@ -31,7 +31,7 @@ declare_lint_pass!(ErasingOp => [ERASING_OP]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ErasingOp {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr<'_>) {
if e.span.from_expansion() {
if in_macro(e.span) {
return;
}
if let ExprKind::Binary(ref cmp, ref left, ref right) = e.kind {
Loading