Skip to content

Commit

Permalink
Auto merge of #4082 - Manishearth:macro-check-split, r=oli-obk
Browse files Browse the repository at this point in the history
Make most macro checks also check for desugarings

We should audit the macro checks one by one and re-add `in_macro`. I suspect it's applicable to most of them.

fixes #4081
  • Loading branch information
bors committed May 12, 2019
2 parents 3710ec5 + abf6481 commit feb18c3
Show file tree
Hide file tree
Showing 44 changed files with 133 additions and 118 deletions.
10 changes: 5 additions & 5 deletions clippy_lints/src/assertions_on_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rustc::{declare_lint_pass, declare_tool_lint};
use syntax_pos::Span;

use crate::consts::{constant, Constant};
use crate::utils::{in_macro, is_direct_expn_of, span_help_and_lint};
use crate::utils::{in_macro_or_desugar, is_direct_expn_of, span_help_and_lint};

declare_clippy_lint! {
/// **What it does:** Checks for `assert!(true)` and `assert!(false)` calls.
Expand Down Expand Up @@ -34,16 +34,16 @@ declare_lint_pass!(AssertionsOnConstants => [ASSERTIONS_ON_CONSTANTS]);
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
let mut is_debug_assert = false;
let debug_assert_not_in_macro = |span: Span| {
let debug_assert_not_in_macro_or_desugar = |span: Span| {
is_debug_assert = true;
// Check that `debug_assert!` itself is not inside a macro
!in_macro(span)
!in_macro_or_desugar(span)
};
if_chain! {
if let Some(assert_span) = is_direct_expn_of(e.span, "assert");
if !in_macro(assert_span)
if !in_macro_or_desugar(assert_span)
|| is_direct_expn_of(assert_span, "debug_assert")
.map_or(false, debug_assert_not_in_macro);
.map_or(false, debug_assert_not_in_macro_or_desugar);
if let ExprKind::Unary(_, ref lit) = e.node;
if let Some(bool_const) = constant(cx, cx.tables, lit);
then {
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use crate::reexport::*;
use crate::utils::{
in_macro, is_present_in_source, last_line_of_span, paths, snippet_opt, span_lint, span_lint_and_sugg,
in_macro_or_desugar, is_present_in_source, last_line_of_span, paths, snippet_opt, span_lint, span_lint_and_sugg,
span_lint_and_then, without_block_comments,
};
use if_chain::if_chain;
Expand Down Expand Up @@ -408,7 +408,7 @@ fn is_relevant_expr(cx: &LateContext<'_, '_>, tables: &ty::TypeckTables<'_>, exp
}

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

Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/block_in_if_condition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl<'a, 'tcx: 'a> Visitor<'tcx> for ExVisitor<'a, 'tcx> {
if let ExprKind::Closure(_, _, eid, _, _) = expr.node {
let body = self.cx.tcx.hir().body(eid);
let ex = &body.value;
if matches!(ex.node, ExprKind::Block(_, _)) && !in_macro(body.value.span) {
if matches!(ex.node, ExprKind::Block(_, _)) && !in_macro_or_desugar(body.value.span) {
self.found_block = Some(ex);
return;
}
Expand All @@ -79,7 +79,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlockInIfCondition {
if let Some(ex) = &block.expr {
// don't dig into the expression here, just suggest that they remove
// the block
if in_macro(expr.span) || differing_macro_contexts(expr.span, ex.span) {
if in_macro_or_desugar(expr.span) || differing_macro_contexts(expr.span, ex.span) {
return;
}
span_help_and_lint(
Expand All @@ -96,7 +96,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlockInIfCondition {
}
} else {
let span = block.expr.as_ref().map_or_else(|| block.stmts[0].span, |e| e.span);
if in_macro(span) || differing_macro_contexts(expr.span, span) {
if in_macro_or_desugar(span) || differing_macro_contexts(expr.span, span) {
return;
}
// move block higher
Expand Down
5 changes: 3 additions & 2 deletions clippy_lints/src/booleans.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::utils::{
get_trait_def_id, implements_trait, in_macro, match_type, paths, snippet_opt, span_lint_and_then, SpanlessEq,
get_trait_def_id, implements_trait, in_macro, in_macro_or_desugar, match_type, paths, snippet_opt,
span_lint_and_then, SpanlessEq,
};
use rustc::hir::intravisit::*;
use rustc::hir::*;
Expand Down Expand Up @@ -93,7 +94,7 @@ impl<'a, 'tcx, 'v> Hir2Qmm<'a, 'tcx, 'v> {

fn run(&mut self, e: &'v Expr) -> Result<Bool, String> {
// prevent folding of `cfg!` macros and the like
if !in_macro(e.span) {
if !in_macro_or_desugar(e.span) {
match &e.node {
ExprKind::Unary(UnNot, inner) => return Ok(Bool::Not(box self.run(inner)?)),
ExprKind::Binary(binop, lhs, rhs) => match &binop.node {
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/cognitive_complexity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc::{declare_tool_lint, impl_lint_pass};
use syntax::ast::Attribute;
use syntax::source_map::Span;

use crate::utils::{in_macro, is_allowed, match_type, paths, span_help_and_lint, LimitStack};
use crate::utils::{in_macro_or_desugar, is_allowed, match_type, paths, span_help_and_lint, LimitStack};

declare_clippy_lint! {
/// **What it does:** Checks for methods with high cognitive complexity.
Expand Down Expand Up @@ -42,7 +42,7 @@ impl_lint_pass!(CognitiveComplexity => [COGNITIVE_COMPLEXITY]);

impl CognitiveComplexity {
fn check<'a, 'tcx: 'a>(&mut self, cx: &'a LateContext<'a, 'tcx>, body: &'tcx Body, span: Span) {
if in_macro(span) {
if in_macro_or_desugar(span) {
return;
}

Expand Down
8 changes: 5 additions & 3 deletions clippy_lints/src/collapsible_if.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ use rustc::{declare_lint_pass, declare_tool_lint};
use syntax::ast;

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

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

impl EarlyLintPass for CollapsibleIf {
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &ast::Expr) {
if !in_macro(expr.span) {
if !in_macro_or_desugar(expr.span) {
check_if(cx, expr)
}
}
Expand Down Expand Up @@ -110,7 +112,7 @@ fn check_collapsible_maybe_if_let(cx: &EarlyContext<'_>, else_: &ast::Expr) {
if let ast::ExprKind::Block(ref block, _) = else_.node;
if !block_starts_with_comment(cx, block);
if let Some(else_) = expr_block(block);
if !in_macro(else_.span);
if !in_macro_or_desugar(else_.span);
then {
match else_.node {
ast::ExprKind::If(..) | ast::ExprKind::IfLet(..) => {
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/const_static_lifetime.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{in_macro, snippet, span_lint_and_then};
use crate::utils::{in_macro_or_desugar, snippet, span_lint_and_then};
use rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass};
use rustc::{declare_lint_pass, declare_tool_lint};
use rustc_errors::Applicability;
Expand Down Expand Up @@ -81,7 +81,7 @@ impl StaticConst {

impl EarlyLintPass for StaticConst {
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
if !in_macro(item.span) {
if !in_macro_or_desugar(item.span) {
// Match only constants...
if let ItemKind::Const(ref var_type, _) = item.node {
self.visit_type(var_type, cx);
Expand Down
4 changes: 2 additions & 2 deletions 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, in_macro, snippet, span_lint_and_then, span_note_and_lint};
use crate::utils::{get_parent_expr, higher, in_macro_or_desugar, snippet, span_lint_and_then, span_note_and_lint};
use crate::utils::{SpanlessEq, SpanlessHash};
use rustc::hir::*;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
Expand Down Expand Up @@ -107,7 +107,7 @@ declare_lint_pass!(CopyAndPaste => [IFS_SAME_COND, IF_SAME_THEN_ELSE, MATCH_SAME

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CopyAndPaste {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
if !in_macro(expr.span) {
if !in_macro_or_desugar(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) {
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/double_parens.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{in_macro, span_lint};
use crate::utils::{in_macro_or_desugar, span_lint};
use rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass};
use rustc::{declare_lint_pass, declare_tool_lint};
use syntax::ast::*;
Expand Down Expand Up @@ -26,7 +26,7 @@ declare_lint_pass!(DoubleParens => [DOUBLE_PARENS]);

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

Expand Down
4 changes: 2 additions & 2 deletions 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, in_macro, is_present_in_source};
use crate::utils::{camel_case, in_macro_or_desugar, is_present_in_source};
use crate::utils::{span_help_and_lint, span_lint};
use rustc::lint::{EarlyContext, EarlyLintPass, Lint, LintArray, LintPass};
use rustc::{declare_tool_lint, impl_lint_pass};
Expand Down Expand Up @@ -244,7 +244,7 @@ impl EarlyLintPass for EnumVariantNames {
let item_name = item.ident.as_str();
let item_name_chars = item_name.chars().count();
let item_camel = to_camel_case(&item_name);
if !in_macro(item.span) && is_present_in_source(cx, item.span) {
if !in_macro_or_desugar(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() {
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/eq_op.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::utils::{
implements_trait, in_macro, is_copy, multispan_sugg, snippet, span_lint, span_lint_and_then, SpanlessEq,
implements_trait, in_macro_or_desugar, is_copy, multispan_sugg, snippet, span_lint, span_lint_and_then, SpanlessEq,
};
use rustc::hir::*;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
Expand Down Expand Up @@ -52,7 +52,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.node {
if in_macro(e.span) {
if in_macro_or_desugar(e.span) {
return;
}
if is_valid_operator(op) && SpanlessEq::new(cx).ignore_fn().eq_expr(left, right) {
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/erasing_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use rustc::{declare_lint_pass, declare_tool_lint};
use syntax::source_map::Span;

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

declare_clippy_lint! {
/// **What it does:** Checks for erasing operations, e.g., `x * 0`.
Expand All @@ -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 in_macro(e.span) {
if in_macro_or_desugar(e.span) {
return;
}
if let ExprKind::Binary(ref cmp, ref left, ref right) = e.node {
Expand Down
5 changes: 3 additions & 2 deletions clippy_lints/src/format.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::utils::paths;
use crate::utils::{
in_macro, is_expn_of, last_path_segment, match_type, resolve_node, snippet, span_lint_and_then, walk_ptrs_ty,
in_macro_or_desugar, is_expn_of, last_path_segment, match_type, resolve_node, snippet, span_lint_and_then,
walk_ptrs_ty,
};
use if_chain::if_chain;
use rustc::hir::*;
Expand Down Expand Up @@ -38,7 +39,7 @@ declare_lint_pass!(UselessFormat => [USELESS_FORMAT]);
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessFormat {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
if let Some(span) = is_expn_of(expr.span, "format") {
if in_macro(span) {
if in_macro_or_desugar(span) {
return;
}
match expr.node {
Expand Down
8 changes: 4 additions & 4 deletions clippy_lints/src/formatting.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{differing_macro_contexts, in_macro, snippet_opt, span_note_and_lint};
use crate::utils::{differing_macro_contexts, in_macro_or_desugar, snippet_opt, span_note_and_lint};
use if_chain::if_chain;
use rustc::lint::{in_external_macro, EarlyContext, EarlyLintPass, LintArray, LintPass};
use rustc::{declare_lint_pass, declare_tool_lint};
Expand Down Expand Up @@ -108,7 +108,7 @@ impl EarlyLintPass for Formatting {
/// Implementation of the `SUSPICIOUS_ASSIGNMENT_FORMATTING` lint.
fn check_assign(cx: &EarlyContext<'_>, expr: &ast::Expr) {
if let ast::ExprKind::Assign(ref lhs, ref rhs) = expr.node {
if !differing_macro_contexts(lhs.span, rhs.span) && !in_macro(lhs.span) {
if !differing_macro_contexts(lhs.span, rhs.span) && !in_macro_or_desugar(lhs.span) {
let eq_span = lhs.span.between(rhs.span);
if let ast::ExprKind::Unary(op, ref sub_rhs) = rhs.node {
if let Some(eq_snippet) = snippet_opt(cx, eq_span) {
Expand Down Expand Up @@ -140,7 +140,7 @@ fn check_else(cx: &EarlyContext<'_>, expr: &ast::Expr) {
if let Some((then, &Some(ref else_))) = unsugar_if(expr);
if is_block(else_) || unsugar_if(else_).is_some();
if !differing_macro_contexts(then.span, else_.span);
if !in_macro(then.span) && !in_external_macro(cx.sess, expr.span);
if !in_macro_or_desugar(then.span) && !in_external_macro(cx.sess, expr.span);

// workaround for rust-lang/rust#43081
if expr.span.lo().0 != 0 && expr.span.hi().0 != 0;
Expand Down Expand Up @@ -206,7 +206,7 @@ fn check_array(cx: &EarlyContext<'_>, expr: &ast::Expr) {

fn check_missing_else(cx: &EarlyContext<'_>, first: &ast::Expr, second: &ast::Expr) {
if !differing_macro_contexts(first.span, second.span)
&& !in_macro(first.span)
&& !in_macro_or_desugar(first.span)
&& unsugar_if(first).is_some()
&& (is_block(second) || unsugar_if(second).is_some())
{
Expand Down
6 changes: 4 additions & 2 deletions clippy_lints/src/identity_conversion.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::utils::{in_macro, match_trait_method, same_tys, snippet, snippet_with_macro_callsite, span_lint_and_then};
use crate::utils::{
in_macro_or_desugar, match_trait_method, same_tys, snippet, snippet_with_macro_callsite, span_lint_and_then,
};
use crate::utils::{paths, resolve_node};
use rustc::hir::*;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
Expand Down Expand Up @@ -31,7 +33,7 @@ impl_lint_pass!(IdentityConversion => [IDENTITY_CONVERSION]);

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

Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/identity_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rustc::{declare_lint_pass, declare_tool_lint};
use syntax::source_map::Span;

use crate::consts::{constant_simple, Constant};
use crate::utils::{clip, in_macro, snippet, span_lint, unsext};
use crate::utils::{clip, in_macro_or_desugar, snippet, span_lint, unsext};

declare_clippy_lint! {
/// **What it does:** Checks for identity operations, e.g., `x + 0`.
Expand All @@ -28,7 +28,7 @@ declare_lint_pass!(IdentityOp => [IDENTITY_OP]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityOp {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
if in_macro(e.span) {
if in_macro_or_desugar(e.span) {
return;
}
if let ExprKind::Binary(ref cmp, ref left, ref right) = e.node {
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/implicit_return.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{in_macro, is_expn_of, snippet_opt, span_lint_and_then};
use crate::utils::{in_macro_or_desugar, is_expn_of, snippet_opt, span_lint_and_then};
use rustc::hir::{intravisit::FnKind, Body, ExprKind, FnDecl, HirId, MatchSource};
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc::{declare_lint_pass, declare_tool_lint};
Expand Down Expand Up @@ -118,7 +118,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImplicitReturn {

// checking return type through MIR, HIR is not able to determine inferred closure return types
// make sure it's not a macro
if !mir.return_ty().is_unit() && !in_macro(span) {
if !mir.return_ty().is_unit() && !in_macro_or_desugar(span) {
Self::expr_match(cx, &body.value);
}
}
Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/items_after_statements.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! lint when items are used after statements
use crate::utils::{in_macro, span_lint};
use crate::utils::{in_macro_or_desugar, span_lint};
use matches::matches;
use rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass};
use rustc::{declare_lint_pass, declare_tool_lint};
Expand Down Expand Up @@ -38,7 +38,7 @@ declare_lint_pass!(ItemsAfterStatements => [ITEMS_AFTER_STATEMENTS]);

impl EarlyLintPass for ItemsAfterStatements {
fn check_block(&mut self, cx: &EarlyContext<'_>, item: &Block) {
if in_macro(item.span) {
if in_macro_or_desugar(item.span) {
return;
}

Expand All @@ -52,7 +52,7 @@ impl EarlyLintPass for ItemsAfterStatements {
// lint on all further items
for stmt in stmts {
if let StmtKind::Item(ref it) = *stmt {
if in_macro(it.span) {
if in_macro_or_desugar(it.span) {
return;
}
if let ItemKind::MacroDef(..) = it.node {
Expand Down
8 changes: 5 additions & 3 deletions clippy_lints/src/len_zero.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::utils::{get_item_name, in_macro, snippet_with_applicability, span_lint, span_lint_and_sugg, walk_ptrs_ty};
use crate::utils::{
get_item_name, in_macro_or_desugar, snippet_with_applicability, span_lint, span_lint_and_sugg, walk_ptrs_ty,
};
use rustc::hir::def_id::DefId;
use rustc::hir::*;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
Expand Down Expand Up @@ -73,7 +75,7 @@ declare_lint_pass!(LenZero => [LEN_ZERO, LEN_WITHOUT_IS_EMPTY]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LenZero {
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) {
if in_macro(item.span) {
if in_macro_or_desugar(item.span) {
return;
}

Expand All @@ -85,7 +87,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LenZero {
}

fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
if in_macro(expr.span) {
if in_macro_or_desugar(expr.span) {
return;
}

Expand Down
Loading

0 comments on commit feb18c3

Please sign in to comment.