Skip to content

Commit

Permalink
new lint local_assigned_single_value
Browse files Browse the repository at this point in the history
  • Loading branch information
Centri3 committed Jun 17, 2023
1 parent e11f36c commit 9877a96
Show file tree
Hide file tree
Showing 15 changed files with 348 additions and 37 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4900,6 +4900,7 @@ Released 2018-09-13
[`lines_filter_map_ok`]: https://rust-lang.github.io/rust-clippy/master/index.html#lines_filter_map_ok
[`linkedlist`]: https://rust-lang.github.io/rust-clippy/master/index.html#linkedlist
[`little_endian_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#little_endian_bytes
[`local_assigned_single_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#local_assigned_single_value
[`logic_bug`]: https://rust-lang.github.io/rust-clippy/master/index.html#logic_bug
[`lossy_float_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#lossy_float_literal
[`macro_use_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#macro_use_imports
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::literal_representation::MISTYPED_LITERAL_SUFFIXES_INFO,
crate::literal_representation::UNREADABLE_LITERAL_INFO,
crate::literal_representation::UNUSUAL_BYTE_GROUPINGS_INFO,
crate::local_assigned_single_value::LOCAL_ASSIGNED_SINGLE_VALUE_INFO,
crate::loops::EMPTY_LOOP_INFO,
crate::loops::EXPLICIT_COUNTER_LOOP_INFO,
crate::loops::EXPLICIT_INTO_ITER_LOOP_INFO,
Expand Down
3 changes: 3 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
extern crate rustc_arena;
extern crate rustc_ast;
extern crate rustc_ast_pretty;
extern crate rustc_const_eval;
extern crate rustc_data_structures;
extern crate rustc_driver;
extern crate rustc_errors;
Expand Down Expand Up @@ -177,6 +178,7 @@ mod let_with_type_underscore;
mod lifetimes;
mod lines_filter_map_ok;
mod literal_representation;
mod local_assigned_single_value;
mod loops;
mod macro_use;
mod main_recursion;
Expand Down Expand Up @@ -1050,6 +1052,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(move |_| Box::new(large_stack_frames::LargeStackFrames::new(stack_size_threshold)));
store.register_late_pass(|_| Box::new(single_range_in_vec_init::SingleRangeInVecInit));
store.register_late_pass(|_| Box::new(incorrect_impls::IncorrectImpls));
store.register_late_pass(|_| Box::new(local_assigned_single_value::LocalAssignedSingleValue));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
185 changes: 185 additions & 0 deletions clippy_lints/src/local_assigned_single_value.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::fn_has_unsatisfiable_preds;
use clippy_utils::source::snippet_opt;
use itertools::Itertools;
use rustc_const_eval::interpret::Scalar;
use rustc_data_structures::fx::FxHashMap;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::{intravisit::FnKind, Body, FnDecl};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::mir::{
self, interpret::ConstValue, visit::Visitor, Constant, Location, Operand, Rvalue, Statement, StatementKind,
};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::Span;

declare_clippy_lint! {
/// ### What it does
/// Checks for locals that are always assigned the same value.
///
/// ### Why is this bad?
/// It's almost always a typo. If not, it can be made immutable, or turned into a constant.
///
/// ### Example
/// ```rust
/// let mut x = 1;
/// x = 1;
/// ```
/// Use instead:
/// ```rust
/// let x = 1;
/// ```
#[clippy::version = "1.72.0"]
pub LOCAL_ASSIGNED_SINGLE_VALUE,
correctness,
"disallows assigning locals many times with the same value"
}
declare_lint_pass!(LocalAssignedSingleValue => [LOCAL_ASSIGNED_SINGLE_VALUE]);

impl LateLintPass<'_> for LocalAssignedSingleValue {
fn check_fn(
&mut self,
cx: &LateContext<'_>,
_: FnKind<'_>,
_: &FnDecl<'_>,
_: &Body<'_>,
_: Span,
def_id: LocalDefId,
) {
// Building MIR for `fn`s with unsatisfiable preds results in ICE.
if fn_has_unsatisfiable_preds(cx, def_id.to_def_id()) {
return;
}

let mir = cx.tcx.optimized_mir(def_id.to_def_id());
let mut v = V {
body: mir,
cx,
local_usage: mir
.local_decls
.iter_enumerated()
.map(|(local, _)| (local, LocalUsageValues::default()))
.collect(),
};
v.visit_body(mir);

for (local, usage) in &v.local_usage {
if should_lint(&v.local_usage, *local, usage) {
let LocalUsageValues {
usage,
mut_ref_acquired: _,
assigned_non_const: _,
} = usage;

if let Some(local_decl) = mir.local_decls.get(*local)
&& let [dbg_info] = &*mir
.var_debug_info
.iter()
.filter(|info| info.source_info.span == local_decl.source_info.span)
.collect_vec()
// Don't handle function arguments.
&& dbg_info.argument_index.is_none()
// Ignore anything from a procedural macro, or locals we cannot prove aren't
// temporaries
&& let Some(snippet) = snippet_opt(cx, dbg_info.source_info.span)
&& snippet.ends_with(dbg_info.name.as_str())
{
span_lint(
cx,
LOCAL_ASSIGNED_SINGLE_VALUE,
usage.iter().map(|(span, _)| *span).collect_vec(),
"local only ever assigned single value",
);
}
}
}
}
}

type LocalUsage = FxHashMap<mir::Local, LocalUsageValues>;

/// Holds the data we have for the usage of a local.
#[derive(Default)]
struct LocalUsageValues {
/// Where and what this local is assigned.
usage: Vec<(Span, Scalar)>,
/// Whether it's mutably borrowed, ever. We should not lint this.
mut_ref_acquired: bool,
/// Whether it's assigned a value we cannot prove is constant, ever. We should not lint this.
assigned_non_const: bool,
}

struct V<'a, 'tcx> {
#[allow(dead_code)]
body: &'a mir::Body<'tcx>,
cx: &'a LateContext<'tcx>,
local_usage: LocalUsage,
}

impl<'a, 'tcx> Visitor<'tcx> for V<'a, 'tcx> {
fn visit_statement(&mut self, stmt: &Statement<'tcx>, _: Location) {
let Self {
body: _,
cx,
local_usage,
} = self;

if stmt.source_info.span.from_expansion() {
return;
}

if let StatementKind::Assign(assign) = &stmt.kind {
let (place, rvalue) = &**assign;
// Do not lint if there are any mutable borrows to a local
if let Rvalue::Ref(_, mir::BorrowKind::Unique | mir::BorrowKind::Mut { .. }, place) = rvalue
&& let Some(other) = local_usage.get_mut(&place.local)
{
other.assigned_non_const = true;
return;
}
let Some(usage) = local_usage.get_mut(&place.local) else {
return;
};

if let Rvalue::Use(Operand::Constant(constant)) = rvalue
&& let Constant { literal, .. } = **constant
&& let Some(ConstValue::Scalar(val)) = literal.try_to_value(cx.tcx)
{
usage.usage.push((stmt.source_info.span, val));
} else if let Rvalue::Use(Operand::Copy(place)) = rvalue
&& let [_base_proj, ..] = place.projection.as_slice()
{
// While this could be `let [x, y] = [1, 1]` or `let (x, y) = (1, 1)`, which should
// indeed be considered a constant, handling such a case would overcomplicate this
// lint.
//
// TODO(Centri3): Let's do the above as a follow-up, in the future! In particular,
// we need to handle `ProjectionElem::Field` and `ProjectionElem::Index`.
usage.assigned_non_const = true;
} else {
// We can also probably handle stuff like `x += 1` here, maybe. But this would be
// very very complex. Let's keep it simple enough.
usage.assigned_non_const = true;
}
}
}
}

fn should_lint(_local_usage: &LocalUsage, _local: mir::Local, usage: &LocalUsageValues) -> bool {
let LocalUsageValues {
usage,
mut_ref_acquired,
assigned_non_const,
} = usage;

if usage.len() > 1
&& !mut_ref_acquired
&& !assigned_non_const
&& let [(_, head), tail @ ..] = &**usage
&& tail.iter().all(|(_, i)| i == head)
{
return true;
}

false
}
2 changes: 1 addition & 1 deletion tests/ui/diverging_sub_expression.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![warn(clippy::diverging_sub_expression)]
#![allow(clippy::match_same_arms, clippy::overly_complex_bool_expr)]
#![allow(clippy::nonminimal_bool)]
#![allow(clippy::local_assigned_single_value, clippy::nonminimal_bool)]
#[allow(clippy::empty_loop)]
fn diverge() -> ! {
loop {}
Expand Down
6 changes: 5 additions & 1 deletion tests/ui/explicit_counter_loop.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
#![warn(clippy::explicit_counter_loop)]
#![allow(clippy::uninlined_format_args, clippy::useless_vec)]
#![allow(
clippy::local_assigned_single_value,
clippy::uninlined_format_args,
clippy::useless_vec
)]

fn main() {
let mut vec = vec![1, 2, 3, 4];
Expand Down
18 changes: 9 additions & 9 deletions tests/ui/explicit_counter_loop.stderr
Original file line number Diff line number Diff line change
@@ -1,55 +1,55 @@
error: the variable `_index` is used as a loop counter
--> $DIR/explicit_counter_loop.rs:7:5
--> $DIR/explicit_counter_loop.rs:11:5
|
LL | for _v in &vec {
| ^^^^^^^^^^^^^^ help: consider using: `for (_index, _v) in vec.iter().enumerate()`
|
= note: `-D clippy::explicit-counter-loop` implied by `-D warnings`

error: the variable `_index` is used as a loop counter
--> $DIR/explicit_counter_loop.rs:13:5
--> $DIR/explicit_counter_loop.rs:17:5
|
LL | for _v in &vec {
| ^^^^^^^^^^^^^^ help: consider using: `for (_index, _v) in vec.iter().enumerate()`

error: the variable `_index` is used as a loop counter
--> $DIR/explicit_counter_loop.rs:18:5
--> $DIR/explicit_counter_loop.rs:22:5
|
LL | for _v in &mut vec {
| ^^^^^^^^^^^^^^^^^^ help: consider using: `for (_index, _v) in vec.iter_mut().enumerate()`

error: the variable `_index` is used as a loop counter
--> $DIR/explicit_counter_loop.rs:23:5
--> $DIR/explicit_counter_loop.rs:27:5
|
LL | for _v in vec {
| ^^^^^^^^^^^^^ help: consider using: `for (_index, _v) in vec.into_iter().enumerate()`

error: the variable `count` is used as a loop counter
--> $DIR/explicit_counter_loop.rs:110:9
--> $DIR/explicit_counter_loop.rs:114:9
|
LL | for ch in text.chars() {
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `for (count, ch) in text.chars().enumerate()`

error: the variable `count` is used as a loop counter
--> $DIR/explicit_counter_loop.rs:121:9
--> $DIR/explicit_counter_loop.rs:125:9
|
LL | for ch in text.chars() {
| ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `for (count, ch) in text.chars().enumerate()`

error: the variable `count` is used as a loop counter
--> $DIR/explicit_counter_loop.rs:179:9
--> $DIR/explicit_counter_loop.rs:183:9
|
LL | for _i in 3..10 {
| ^^^^^^^^^^^^^^^ help: consider using: `for (count, _i) in (3..10).enumerate()`

error: the variable `idx_usize` is used as a loop counter
--> $DIR/explicit_counter_loop.rs:219:9
--> $DIR/explicit_counter_loop.rs:223:9
|
LL | for _item in slice {
| ^^^^^^^^^^^^^^^^^^ help: consider using: `for (idx_usize, _item) in slice.iter().enumerate()`

error: the variable `idx_u32` is used as a loop counter
--> $DIR/explicit_counter_loop.rs:231:9
--> $DIR/explicit_counter_loop.rs:235:9
|
LL | for _item in slice {
| ^^^^^^^^^^^^^^^^^^ help: consider using: `for (idx_u32, _item) in (0_u32..).zip(slice.iter())`
Expand Down
8 changes: 7 additions & 1 deletion tests/ui/let_unit.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@

#![feature(lint_reasons)]
#![warn(clippy::let_unit_value)]
#![allow(unused, clippy::no_effect, clippy::needless_late_init, path_statements)]
#![allow(
unused,
clippy::local_assigned_single_value,
clippy::no_effect,
clippy::needless_late_init,
path_statements
)]

macro_rules! let_and_return {
($n:expr) => {{
Expand Down
8 changes: 7 additions & 1 deletion tests/ui/let_unit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@

#![feature(lint_reasons)]
#![warn(clippy::let_unit_value)]
#![allow(unused, clippy::no_effect, clippy::needless_late_init, path_statements)]
#![allow(
unused,
clippy::local_assigned_single_value,
clippy::no_effect,
clippy::needless_late_init,
path_statements
)]

macro_rules! let_and_return {
($n:expr) => {{
Expand Down
Loading

0 comments on commit 9877a96

Please sign in to comment.