Skip to content

Commit

Permalink
WIP
Browse files Browse the repository at this point in the history
  • Loading branch information
flip1995 committed Jun 17, 2020
1 parent c0b9f66 commit 59cb7f1
Show file tree
Hide file tree
Showing 10 changed files with 81 additions and 47 deletions.
10 changes: 6 additions & 4 deletions clippy_lints/src/blocks_in_if_conditions.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::utils::{differing_macro_contexts, higher, snippet_block_with_applicability, span_lint, span_lint_and_sugg};
use crate::utils::{
differing_macro_contexts, higher, in_macro, snippet_block_with_applicability, span_lint, span_lint_and_sugg,
};
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::{BlockCheckMode, Expr, ExprKind};
Expand Down Expand Up @@ -55,7 +57,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;
}
Expand Down Expand Up @@ -83,7 +85,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;
Expand All @@ -108,7 +110,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
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/booleans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/eq_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<'_>| {
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_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::{in_macro, 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 e.span.from_expansion() {
if in_macro(e.span) {
return;
}
if let ExprKind::Binary(ref cmp, ref left, ref right) = e.kind {
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 @@ -6,7 +6,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::{clip, snippet, span_lint, unsext};
use crate::utils::{clip, in_macro, snippet, span_lint, unsext};

declare_clippy_lint! {
/// **What it does:** Checks for identity operations, e.g., `x + 0`.
Expand All @@ -30,7 +30,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 e.span.from_expansion() {
if in_macro(e.span) {
return;
}
if let ExprKind::Binary(cmp, ref left, ref right) = e.kind {
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, higher, snippet_with_applicability, span_lint, span_lint_and_sugg, walk_ptrs_ty};
use crate::utils::{
get_item_name, higher, in_macro, snippet_with_applicability, span_lint, span_lint_and_sugg, walk_ptrs_ty,
};
use rustc_ast::ast::LitKind;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::Applicability;
Expand Down Expand Up @@ -72,7 +74,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 item.span.from_expansion() {
if in_macro(item.span) {
return;
}

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

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

Expand Down
6 changes: 4 additions & 2 deletions clippy_lints/src/needless_bool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
//! This lint is **warn** by default

use crate::utils::sugg::Sugg;
use crate::utils::{higher, parent_node_is_if_expr, snippet_with_applicability, span_lint, span_lint_and_sugg};
use crate::utils::{
higher, in_macro, parent_node_is_if_expr, snippet_with_applicability, span_lint, span_lint_and_sugg,
};
use if_chain::if_chain;
use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
Expand Down Expand Up @@ -129,7 +131,7 @@ declare_lint_pass!(BoolComparison => [BOOL_COMPARISON]);

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

Expand Down
8 changes: 4 additions & 4 deletions clippy_lints/src/no_effect.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{has_drop, qpath_res, snippet_opt, span_lint, span_lint_and_sugg};
use crate::utils::{has_drop, in_macro, qpath_res, snippet_opt, span_lint, span_lint_and_sugg};
use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{BinOpKind, BlockCheckMode, Expr, ExprKind, Stmt, StmtKind, UnsafeSource};
Expand Down Expand Up @@ -43,7 +43,7 @@ declare_clippy_lint! {
}

fn has_no_effect(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool {
if expr.span.from_expansion() {
if in_macro(expr.span) {
return false;
}
match expr.kind {
Expand Down Expand Up @@ -95,7 +95,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NoEffect {
} else if let Some(reduced) = reduce_expression(cx, expr) {
let mut snippet = String::new();
for e in reduced {
if e.span.from_expansion() {
if in_macro(e.span) {
return;
}
if let Some(snip) = snippet_opt(cx, e.span) {
Expand All @@ -120,7 +120,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NoEffect {
}

fn reduce_expression<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr<'a>) -> Option<Vec<&'a Expr<'a>>> {
if expr.span.from_expansion() {
if in_macro(expr.span) {
return None;
}
match expr.kind {
Expand Down
55 changes: 29 additions & 26 deletions clippy_lints/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,11 @@ use rustc_typeck::hir_ty_to_ty;
use crate::consts::{constant, Constant};
use crate::utils::paths;
use crate::utils::{
clip, comparisons, differing_macro_contexts, higher, in_constant, indent_of, int_bits, is_type_diagnostic_item,
last_path_segment, match_def_path, match_path, method_chain_args, multispan_sugg, numeric_literal::NumericLiteral,
qpath_res, sext, snippet, snippet_block_with_applicability, snippet_opt, snippet_with_applicability,
snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, unsext,
clip, comparisons, differing_macro_contexts, higher, in_constant, in_macro, indent_of, int_bits,
is_type_diagnostic_item, last_path_segment, match_def_path, match_path, method_chain_args, multispan_sugg,
numeric_literal::NumericLiteral, qpath_res, sext, snippet, snippet_block_with_applicability, snippet_opt,
snippet_with_applicability, snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg,
span_lint_and_then, unsext,
};

declare_clippy_lint! {
Expand Down Expand Up @@ -1965,35 +1966,37 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AbsurdExtremeComparisons {
use crate::types::AbsurdComparisonResult::{AlwaysFalse, AlwaysTrue, InequalityImpossible};
use crate::types::ExtremeType::{Maximum, Minimum};

if in_macro(expr.span) {
return;
}

if let ExprKind::Binary(ref cmp, ref lhs, ref rhs) = expr.kind {
if let Some((culprit, result)) = detect_absurd_comparison(cx, cmp.node, lhs, rhs) {
if !expr.span.from_expansion() {
let msg = "this comparison involving the minimum or maximum element for this \
let msg = "this comparison involving the minimum or maximum element for this \
type contains a case that is always true or always false";

let conclusion = match result {
AlwaysFalse => "this comparison is always false".to_owned(),
AlwaysTrue => "this comparison is always true".to_owned(),
InequalityImpossible => format!(
"the case where the two sides are not equal never occurs, consider using `{} == {}` \
let conclusion = match result {
AlwaysFalse => "this comparison is always false".to_owned(),
AlwaysTrue => "this comparison is always true".to_owned(),
InequalityImpossible => format!(
"the case where the two sides are not equal never occurs, consider using `{} == {}` \
instead",
snippet(cx, lhs.span, "lhs"),
snippet(cx, rhs.span, "rhs")
),
};
snippet(cx, lhs.span, "lhs"),
snippet(cx, rhs.span, "rhs")
),
};

let help = format!(
"because `{}` is the {} value for this type, {}",
snippet(cx, culprit.expr.span, "x"),
match culprit.which {
Minimum => "minimum",
Maximum => "maximum",
},
conclusion
);
let help = format!(
"because `{}` is the {} value for this type, {}",
snippet(cx, culprit.expr.span, "x"),
match culprit.which {
Minimum => "minimum",
Maximum => "maximum",
},
conclusion
);

span_lint_and_help(cx, ABSURD_EXTREME_COMPARISONS, expr.span, msg, None, &help);
}
span_lint_and_help(cx, ABSURD_EXTREME_COMPARISONS, expr.span, msg, None, &help);
}
}
}
Expand Down
29 changes: 27 additions & 2 deletions clippy_lints/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use rustc_infer::infer::TyCtxtInferExt;
use rustc_lint::{LateContext, Level, Lint, LintContext};
use rustc_middle::hir::map::Map;
use rustc_middle::ty::{self, layout::IntegerExt, subst::GenericArg, Ty, TyCtxt, TypeFoldable};
use rustc_span::hygiene::{ExpnKind, MacroKind};
use rustc_span::hygiene::{DesugaringKind, ExpnKind, MacroKind};
use rustc_span::source_map::original_sp;
use rustc_span::symbol::{self, kw, Symbol};
use rustc_span::{BytePos, Pos, Span, DUMMY_SP};
Expand All @@ -58,7 +58,31 @@ use crate::reexport::Name;
/// from a macro and one isn't).
#[must_use]
pub fn differing_macro_contexts(lhs: Span, rhs: Span) -> bool {
rhs.ctxt() != lhs.ctxt()
let lhs_op_expn = from_operator_expansion(lhs);
let rhs_op_expn = from_operator_expansion(rhs);

if lhs_op_expn && rhs_op_expn {
// Ignore if both sides are just from operator expansions
false
} else if lhs_op_expn {
// if only LHS is from operator expansion and RHS is from another expansion kind
// => return true
rhs.from_expansion()
} else if rhs_op_expn {
// if only RHS is from operator expansion and LHS is from another expansion kind
// => return true
lhs.from_expansion()
} else {
// of neither side is from an operator expansion, compare the `SpanContext`s
rhs.ctxt() != lhs.ctxt()
}
}

fn from_operator_expansion(span: Span) -> bool {
matches!(
span.ctxt().outer_expn_data().kind,
ExpnKind::Desugaring(DesugaringKind::Operator)
)
}

/// Returns `true` if the given `NodeId` is inside a constant context
Expand Down Expand Up @@ -111,6 +135,7 @@ pub fn in_macro(span: Span) -> bool {
false
}
}

// If the snippet is empty, it's an attribute that was inserted during macro
// expansion and we want to ignore those, because they could come from external
// sources that the user has no control over.
Expand Down

0 comments on commit 59cb7f1

Please sign in to comment.