Skip to content
Permalink

Comparing changes

This is a direct comparison between two commits made in this repository or its related repositories. View the default comparison for this range or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: rust-lang/rust-clippy
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 373d6b0c46edf52723e5b392ebc97ed0e1cdc311
Choose a base ref
..
head repository: rust-lang/rust-clippy
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 137a762954ec291813d74b76281fa91eb5fa405c
Choose a head ref
5 changes: 4 additions & 1 deletion clippy_lints/src/block_in_if_condition.rs
View file
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@ use crate::utils::*;
use matches::matches;
use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc::hir::*;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass};
use rustc::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
@@ -72,6 +72,9 @@ const COMPLEX_BLOCK_MESSAGE: &str = "in an 'if' condition, avoid complex blocks

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlockInIfCondition {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
if in_external_macro(cx.sess(), expr.span) {
return;
}
if let Some((check, then, _)) = higher::if_block(&expr) {
if let ExprKind::Block(block, _) = &check.node {
if block.rules == DefaultBlock {
156 changes: 25 additions & 131 deletions clippy_lints/src/cognitive_complexity.rs
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
//! calculate cognitive complexity and warn about overly complex functions
use rustc::cfg::CFG;
use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc::hir::*;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintContext, LintPass};
use rustc::ty;
use rustc::{declare_tool_lint, impl_lint_pass};
use syntax::ast::Attribute;
use syntax::source_map::Span;

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

declare_clippy_lint! {
/// **What it does:** Checks for methods with high cognitive complexity.
@@ -46,30 +44,11 @@ impl CognitiveComplexity {
return;
}

let cfg = CFG::new(cx.tcx, body);
let expr = &body.value;
let n = cfg.graph.len_nodes() as u64;
let e = cfg.graph.len_edges() as u64;
if e + 2 < n {
// the function has unreachable code, other lints should catch this
return;
}
let cc = e + 2 - n;
let mut helper = CCHelper {
match_arms: 0,
divergence: 0,
short_circuits: 0,
returns: 0,
cx,
};

let mut helper = CCHelper { cc: 1, returns: 0 };
helper.visit_expr(expr);
let CCHelper {
match_arms,
divergence,
short_circuits,
returns,
..
} = helper;
let CCHelper { cc, returns } = helper;
let ret_ty = cx.tables.node_type(expr.hir_id);
let ret_adjust = if match_type(cx, ret_ty, &paths::RESULT) {
returns
@@ -78,36 +57,23 @@ impl CognitiveComplexity {
(returns / 2)
};

if cc + divergence < match_arms + short_circuits {
report_cc_bug(
let mut rust_cc = cc;
// prevent degenerate cases where unreachable code contains `return` statements
if rust_cc >= ret_adjust {
rust_cc -= ret_adjust;
}
if rust_cc > self.limit.limit() {
span_help_and_lint(
cx,
cc,
match_arms,
divergence,
short_circuits,
ret_adjust,
COGNITIVE_COMPLEXITY,
span,
body.id().hir_id,
&format!(
"the function has a cognitive complexity of ({}/{})",
rust_cc,
self.limit.limit()
),
"you could split it up into multiple smaller functions",
);
} else {
let mut rust_cc = cc + divergence - match_arms - short_circuits;
// prevent degenerate cases where unreachable code contains `return` statements
if rust_cc >= ret_adjust {
rust_cc -= ret_adjust;
}
if rust_cc > self.limit.limit() {
span_help_and_lint(
cx,
COGNITIVE_COMPLEXITY,
span,
&format!(
"the function has a cognitive complexity of ({}/{})",
rust_cc,
self.limit.limit()
),
"you could split it up into multiple smaller functions",
);
}
}
}
}
@@ -136,99 +102,27 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CognitiveComplexity {
}
}

struct CCHelper<'a, 'tcx> {
match_arms: u64,
divergence: u64,
struct CCHelper {
cc: u64,
returns: u64,
short_circuits: u64, // && and ||
cx: &'a LateContext<'a, 'tcx>,
}

impl<'a, 'tcx> Visitor<'tcx> for CCHelper<'a, 'tcx> {
impl<'tcx> Visitor<'tcx> for CCHelper {
fn visit_expr(&mut self, e: &'tcx Expr) {
walk_expr(self, e);
match e.node {
ExprKind::Match(_, ref arms, _) => {
walk_expr(self, e);
let arms_n: u64 = arms.iter().map(|arm| arm.pats.len() as u64).sum();
if arms_n > 1 {
self.match_arms += arms_n - 2;
}
},
ExprKind::Call(ref callee, _) => {
walk_expr(self, e);
let ty = self.cx.tables.node_type(callee.hir_id);
match ty.sty {
ty::FnDef(..) | ty::FnPtr(_) => {
let sig = ty.fn_sig(self.cx.tcx);
if sig.skip_binder().output().sty == ty::Never {
self.divergence += 1;
}
},
_ => (),
}
},
ExprKind::Closure(.., _) => (),
ExprKind::Binary(op, _, _) => {
walk_expr(self, e);
match op.node {
BinOpKind::And | BinOpKind::Or => self.short_circuits += 1,
_ => (),
self.cc += 1;
}
self.cc += arms.iter().filter(|arm| arm.guard.is_some()).count() as u64;
},
ExprKind::Ret(_) => self.returns += 1,
_ => walk_expr(self, e),
_ => {},
}
}
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
NestedVisitorMap::None
}
}

#[cfg(feature = "debugging")]
#[allow(clippy::too_many_arguments)]
fn report_cc_bug(
_: &LateContext<'_, '_>,
cc: u64,
narms: u64,
div: u64,
shorts: u64,
returns: u64,
span: Span,
_: HirId,
) {
span_bug!(
span,
"Clippy encountered a bug calculating cognitive complexity: cc = {}, arms = {}, \
div = {}, shorts = {}, returns = {}. Please file a bug report.",
cc,
narms,
div,
shorts,
returns
);
}
#[cfg(not(feature = "debugging"))]
#[allow(clippy::too_many_arguments)]
fn report_cc_bug(
cx: &LateContext<'_, '_>,
cc: u64,
narms: u64,
div: u64,
shorts: u64,
returns: u64,
span: Span,
id: HirId,
) {
if !is_allowed(cx, COGNITIVE_COMPLEXITY, id) {
cx.sess().span_note_without_error(
span,
&format!(
"Clippy encountered a bug calculating cognitive complexity \
(hide this message with `#[allow(cognitive_complexity)]`): \
cc = {}, arms = {}, div = {}, shorts = {}, returns = {}. \
Please file a bug report.",
cc, narms, div, shorts, returns
),
);
}
}
4 changes: 2 additions & 2 deletions clippy_lints/src/get_last_with_len.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! lint on using `x.get(x.len() - 1)` instead of `x.last()`
use crate::utils::{match_type, paths, snippet_with_applicability, span_lint_and_sugg, SpanlessEq};
use crate::utils::{is_type_diagnostic_item, snippet_with_applicability, span_lint_and_sugg, SpanlessEq};
use if_chain::if_chain;
use rustc::hir::{BinOpKind, Expr, ExprKind};
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
@@ -56,7 +56,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for GetLastWithLen {
// Argument 0 (the struct we're calling the method on) is a vector
if let Some(struct_calling_on) = args.get(0);
let struct_ty = cx.tables.expr_ty(struct_calling_on);
if match_type(cx, struct_ty, &paths::VEC);
if is_type_diagnostic_item(cx, struct_ty, Symbol::intern("vec_type"));

// Argument to "get" is a subtraction
if let Some(get_index_arg) = args.get(1);
20 changes: 10 additions & 10 deletions clippy_lints/src/loops.rs
View file
Original file line number Diff line number Diff line change
@@ -11,7 +11,7 @@ use rustc::{declare_lint_pass, declare_tool_lint};
// use rustc::middle::region::CodeExtent;
use crate::consts::{constant, Constant};
use crate::utils::usage::mutated_variables;
use crate::utils::{sext, sugg};
use crate::utils::{sext, sugg, is_type_diagnostic_item};
use rustc::middle::expr_use_visitor::*;
use rustc::middle::mem_categorization::cmt_;
use rustc::middle::mem_categorization::Categorization;
@@ -23,11 +23,11 @@ use std::iter::{once, Iterator};
use std::mem;
use syntax::ast;
use syntax::source_map::Span;
use syntax_pos::BytePos;
use syntax_pos::{BytePos, Symbol};

use crate::utils::paths;
use crate::utils::{
get_enclosing_block, get_parent_expr, has_iter_method, higher, is_integer_literal, is_refutable, last_path_segment,
get_enclosing_block, get_parent_expr, has_iter_method, higher, is_integer_const, is_refutable, last_path_segment,
match_trait_method, match_type, match_var, multispan_sugg, snippet, snippet_opt, snippet_with_applicability,
span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, SpanlessEq,
};
@@ -795,7 +795,7 @@ fn is_slice_like<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'_>) -> bool {
_ => false,
};

is_slice || match_type(cx, ty, &paths::VEC) || match_type(cx, ty, &paths::VEC_DEQUE)
is_slice || is_type_diagnostic_item(cx, ty, Symbol::intern("vec_type")) || match_type(cx, ty, &paths::VEC_DEQUE)
}

fn get_fixed_offset_var<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &Expr, var: HirId) -> Option<FixedOffsetVar> {
@@ -1096,7 +1096,7 @@ fn check_for_loop_range<'a, 'tcx>(
return;
}

let starts_at_zero = is_integer_literal(start, 0);
let starts_at_zero = is_integer_const(cx, start, 0);

let skip = if starts_at_zero {
String::new()
@@ -1950,7 +1950,7 @@ fn is_ref_iterable_type(cx: &LateContext<'_, '_>, e: &Expr) -> bool {
// will allow further borrows afterwards
let ty = cx.tables.expr_ty(e);
is_iterable_array(ty, cx) ||
match_type(cx, ty, &paths::VEC) ||
is_type_diagnostic_item(cx, ty, Symbol::intern("vec_type")) ||
match_type(cx, ty, &paths::LINKED_LIST) ||
match_type(cx, ty, &paths::HASHMAP) ||
match_type(cx, ty, &paths::HASHSET) ||
@@ -2042,7 +2042,7 @@ impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> {
match parent.node {
ExprKind::AssignOp(op, ref lhs, ref rhs) => {
if lhs.hir_id == expr.hir_id {
if op.node == BinOpKind::Add && is_integer_literal(rhs, 1) {
if op.node == BinOpKind::Add && is_integer_const(self.cx, rhs, 1) {
*state = match *state {
VarState::Initial if self.depth == 0 => VarState::IncrOnce,
_ => VarState::DontWarn,
@@ -2094,7 +2094,7 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {
self.name = Some(ident.name);

self.state = if let Some(ref init) = local.init {
if is_integer_literal(init, 0) {
if is_integer_const(&self.cx, init, 0) {
VarState::Warn
} else {
VarState::Declared
@@ -2130,7 +2130,7 @@ impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {
self.state = VarState::DontWarn;
},
ExprKind::Assign(ref lhs, ref rhs) if lhs.hir_id == expr.hir_id => {
self.state = if is_integer_literal(rhs, 0) && self.depth == 0 {
self.state = if is_integer_const(&self.cx, rhs, 0) && self.depth == 0 {
VarState::Warn
} else {
VarState::DontWarn
@@ -2399,7 +2399,7 @@ fn check_needless_collect<'a, 'tcx>(expr: &'tcx Expr, cx: &LateContext<'a, 'tcx>
if let Some(GenericArg::Type(ref ty)) = generic_args.args.get(0);
then {
let ty = cx.tables.node_type(ty.hir_id);
if match_type(cx, ty, &paths::VEC) ||
if is_type_diagnostic_item(cx, ty, Symbol::intern("vec_type")) ||
match_type(cx, ty, &paths::VEC_DEQUE) ||
match_type(cx, ty, &paths::BTREEMAP) ||
match_type(cx, ty, &paths::HASHMAP) {
Loading