bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Merge commit '6ed6f1e6a1' into clippyup

Browse source
This commit is contained in:
flip1995 2021-03-12 15:30:50 +01:00
parent 36a27ecaac
commit f2f2a005b4
297 changed files with 15401 additions and 12253 deletions
Download patch file Download diff file Expand all files Collapse all files

2
.cargo/config
View file

@ -1,7 +1,7 @@
[alias]
uitest = "test --test compile-test"
dev = "run --target-dir clippy_dev/target --package clippy_dev --bin clippy_dev --manifest-path clippy_dev/Cargo.toml --"
dev-lintcheck = "run --target-dir clippy_dev/target --package clippy_dev --bin clippy_dev --manifest-path clippy_dev/Cargo.toml --features lintcheck -- lintcheck"
lintcheck = "run --target-dir lintcheck/target --package lintcheck --bin lintcheck --manifest-path lintcheck/Cargo.toml -- "
[build]
rustflags = ["-Zunstable-options"]

18
.github/workflows/clippy.yml vendored
View file

@ -27,6 +27,7 @@ env:
jobs:
base:
# NOTE: If you modify this job, make sure you copy the changes to clippy_bors.yml
runs-on: ubuntu-latest
steps:
@ -50,11 +51,16 @@ jobs:
- name: Build
run: cargo build --features deny-warnings,internal-lints
- name: Test "--fix -Zunstable-options"
run: cargo run --features deny-warnings,internal-lints --bin cargo-clippy -- clippy --fix -Zunstable-options
- name: Test
run: cargo test --features deny-warnings,internal-lints
- name: Test Workspace
run: cargo test --all --features deny-warnings,internal-lints
- name: Test clippy_lints
run: cargo test --features deny-warnings,internal-lints
working-directory: clippy_lints
- name: Test rustc_tools_util
run: cargo test --features deny-warnings
working-directory: rustc_tools_util
- name: Test clippy_dev
run: cargo test --features deny-warnings
@ -64,6 +70,10 @@ jobs:
run: ../target/debug/cargo-clippy
working-directory: clippy_workspace_tests
- name: Test cargo-clippy --fix
run: ../target/debug/cargo-clippy clippy --fix -Zunstable-options
working-directory: clippy_workspace_tests
- name: Test clippy-driver
run: bash .github/driver.sh
env:

24
.github/workflows/clippy_bors.yml vendored
View file

@ -72,6 +72,7 @@ jobs:
runs-on: ${{ matrix.os }}
# NOTE: If you modify this job, make sure you copy the changes to clippy.yml
steps:
# Setup
- uses: rust-lang/simpleinfra/github-actions/cancel-outdated-builds@master
@ -112,8 +113,16 @@ jobs:
- name: Build
run: cargo build --features deny-warnings,internal-lints
- name: Test Workspace
run: cargo test --all --features deny-warnings,internal-lints
- name: Test
run: cargo test --features deny-warnings,internal-lints
- name: Test clippy_lints
run: cargo test --features deny-warnings,internal-lints
working-directory: clippy_lints
- name: Test rustc_tools_util
run: cargo test --features deny-warnings
working-directory: rustc_tools_util
- name: Test clippy_dev
run: cargo test --features deny-warnings
@ -123,11 +132,22 @@ jobs:
run: ../target/debug/cargo-clippy
working-directory: clippy_workspace_tests
- name: Test cargo-clippy --fix
run: ../target/debug/cargo-clippy clippy --fix -Zunstable-options
working-directory: clippy_workspace_tests
- name: Test clippy-driver
run: bash .github/driver.sh
env:
OS: ${{ runner.os }}
- name: Test cargo dev new lint
run: |
cargo dev new_lint --name new_early_pass --pass early
cargo dev new_lint --name new_late_pass --pass late
cargo check
git reset --hard HEAD
integration_build:
needs: changelog
runs-on: ubuntu-latest

1
.gitignore vendored
View file

@ -21,6 +21,7 @@ out
/clippy_utils/target
/clippy_workspace_tests/target
/clippy_dev/target
/lintcheck/target
/rustc_tools_util/target
# Generated by dogfood

2
CHANGELOG.md
View file

@ -2104,6 +2104,7 @@ Released 2018-09-13
[`if_not_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_not_else
[`if_same_then_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else
[`ifs_same_cond`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond
[`implicit_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_clone
[`implicit_hasher`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher
[`implicit_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_return
[`implicit_saturating_sub`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_saturating_sub
@ -2134,6 +2135,7 @@ Released 2018-09-13
[`invisible_characters`]: https://rust-lang.github.io/rust-clippy/master/index.html#invisible_characters
[`items_after_statements`]: https://rust-lang.github.io/rust-clippy/master/index.html#items_after_statements
[`iter_cloned_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_cloned_collect
[`iter_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_count
[`iter_next_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_next_loop
[`iter_next_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_next_slice
[`iter_nth`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth

13
Cargo.toml
View file

@ -1,13 +1,7 @@
[package]
name = "clippy"
version = "0.1.52"
authors = [
"Manish Goregaokar <manishsmail@gmail.com>",
"Andre Bogus <bogusandre@gmail.com>",
"Georg Brandl <georg@python.org>",
"Martin Carton <cartonmartin@gmail.com>",
"Oliver Schneider <clippy-iethah7aipeen8neex1a@oli-obk.de>"
]
authors = ["The Rust Clippy Developers"]
description = "A bunch of helpful lints to avoid common pitfalls in Rust"
repository = "https://github.com/rust-lang/rust-clippy"
readme = "README.md"
@ -42,6 +36,7 @@ tester = "0.9"
clippy-mini-macro-test = { version = "0.2", path = "mini-macro" }
serde = { version = "1.0", features = ["derive"] }
derive-new = "0.5"
regex = "1.4"
# A noop dependency that changes in the Rust repository, it's a bit of a hack.
# See the `src/tools/rustc-workspace-hack/README.md` file in `rust-lang/rust`
@ -55,3 +50,7 @@ rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util" }
deny-warnings = []
integration = ["tempfile"]
internal-lints = ["clippy_lints/internal-lints"]
[package.metadata.rust-analyzer]
# This package uses #[feature(rustc_private)]
rustc_private = true

1
README.md
View file

@ -202,7 +202,6 @@ the lint(s) you are interested in:
```terminal
cargo clippy -- -A clippy::all -W clippy::useless_format -W clippy::...
```
Note that if you've run clippy before, this may only take effect after you've modified a file or ran `cargo clean`.
### Specifying the minimum supported Rust version

12
clippy_dev/Cargo.toml
View file

@ -1,27 +1,17 @@
[package]
name = "clippy_dev"
version = "0.0.1"
authors = ["Philipp Hansch <dev@phansch.net>"]
authors = ["The Rust Clippy Developers"]
edition = "2018"
[dependencies]
bytecount = "0.6"
clap = "2.33"
flate2 = { version = "1.0.19", optional = true }
fs_extra = { version = "1.2.0", optional = true }
itertools = "0.9"
opener = "0.4"
regex = "1"
serde = { version = "1.0", features = ["derive"], optional = true }
serde_json = { version = "1.0", optional = true }
shell-escape = "0.1"
tar = { version = "0.4.30", optional = true }
toml = { version = "0.5", optional = true }
ureq = { version = "2.0.0-rc3", optional = true }
rayon = { version = "1.5.0", optional = true }
walkdir = "2"
[features]
lintcheck = ["flate2", "serde_json", "tar", "toml", "ureq", "serde", "fs_extra", "rayon"]
deny-warnings = []

1
clippy_dev/src/fmt.rs
View file

@ -54,6 +54,7 @@ pub fn run(check: bool, verbose: bool) {
success &= cargo_fmt(context, project_root.as_path())?;
success &= cargo_fmt(context, &project_root.join("clippy_dev"))?;
success &= cargo_fmt(context, &project_root.join("rustc_tools_util"))?;
success &= cargo_fmt(context, &project_root.join("lintcheck"))?;
for entry in WalkDir::new(project_root.join("tests")) {
let entry = entry?;

3
clippy_dev/src/lib.rs
View file

@ -12,7 +12,6 @@ use walkdir::WalkDir;
pub mod bless;
pub mod fmt;
pub mod lintcheck;
pub mod new_lint;
pub mod ra_setup;
pub mod serve;
@ -530,7 +529,7 @@ fn test_gen_deprecated() {
#[should_panic]
fn test_gen_deprecated_fail() {
let lints = vec![Lint::new("should_assert_eq2", "group2", "abc", None, "module_name")];
let _ = gen_deprecated(lints.iter());
let _deprecated_lints = gen_deprecated(lints.iter());
}
#[test]

44
clippy_dev/src/main.rs
View file

@ -2,10 +2,6 @@
use clap::{App, Arg, ArgMatches, SubCommand};
use clippy_dev::{bless, fmt, new_lint, ra_setup, serve, stderr_length_check, update_lints};
#[cfg(feature = "lintcheck")]
use clippy_dev::lintcheck;
fn main() {
let matches = get_clap_config();
@ -13,10 +9,6 @@ fn main() {
("bless", Some(matches)) => {
bless::bless(matches.is_present("ignore-timestamp"));
},
#[cfg(feature = "lintcheck")]
("lintcheck", Some(matches)) => {
lintcheck::run(&matches);
},
("fmt", Some(matches)) => {
fmt::run(matches.is_present("check"), matches.is_present("verbose"));
},
@ -53,33 +45,7 @@ fn main() {
}
fn get_clap_config<'a>() -> ArgMatches<'a> {
#[cfg(feature = "lintcheck")]
let lintcheck_sbcmd = SubCommand::with_name("lintcheck")
.about("run clippy on a set of crates and check output")
.arg(
Arg::with_name("only")
.takes_value(true)
.value_name("CRATE")
.long("only")
.help("only process a single crate of the list"),
)
.arg(
Arg::with_name("crates-toml")
.takes_value(true)
.value_name("CRATES-SOURCES-TOML-PATH")
.long("crates-toml")
.help("set the path for a crates.toml where lintcheck should read the sources from"),
)
.arg(
Arg::with_name("threads")
.takes_value(true)
.value_name("N")
.short("j")
.long("jobs")
.help("number of threads to use, 0 automatic choice"),
);
let app = App::new("Clippy developer tooling")
App::new("Clippy developer tooling")
.subcommand(
SubCommand::with_name("bless")
.about("bless the test output changes")
@ -196,10 +162,6 @@ fn get_clap_config<'a>() -> ArgMatches<'a> {
.validator_os(serve::validate_port),
)
.arg(Arg::with_name("lint").help("Which lint's page to load initially (optional)")),
);
#[cfg(feature = "lintcheck")]
let app = app.subcommand(lintcheck_sbcmd);
app.get_matches()
)
.get_matches()
}

2
clippy_dummy/Cargo.toml
View file

@ -1,7 +1,7 @@
[package]
name = "clippy_dummy" # rename to clippy before publishing
version = "0.0.303"
authors = ["Manish Goregaokar <manishsmail@gmail.com>"]
authors = ["The Rust Clippy Developers"]
edition = "2018"
readme = "crates-readme.md"
description = "A bunch of helpful lints to avoid common pitfalls in Rust."

11
clippy_lints/Cargo.toml
View file

@ -3,12 +3,7 @@ name = "clippy_lints"
# begin automatic update
version = "0.1.52"
# end automatic update
authors = [
"Manish Goregaokar <manishsmail@gmail.com>",
"Andre Bogus <bogusandre@gmail.com>",
"Georg Brandl <georg@python.org>",
"Martin Carton <cartonmartin@gmail.com>"
]
authors = ["The Rust Clippy Developers"]
description = "A bunch of helpful lints to avoid common pitfalls in Rust"
repository = "https://github.com/rust-lang/rust-clippy"
readme = "README.md"
@ -40,3 +35,7 @@ syn = { version = "1", features = ["full"] }
deny-warnings = []
# build clippy with internal lints enabled, off by default
internal-lints = ["clippy_utils/internal-lints"]
[package.metadata.rust-analyzer]
# This crate uses #[feature(rustc_private)]
rustc_private = true

2
clippy_lints/src/assign_ops.rs
View file

@ -209,7 +209,7 @@ fn lint_misrefactored_assign_op(
diag.span_suggestion(
expr.span,
&format!(
"Did you mean `{} = {} {} {}` or `{}`? Consider replacing it with",
"did you mean `{} = {} {} {}` or `{}`? Consider replacing it with",
snip_a,
snip_a,
op.node.as_str(),

3
clippy_lints/src/async_yields_async.rs
View file

@ -50,8 +50,7 @@ impl<'tcx> LateLintPass<'tcx> for AsyncYieldsAsync {
let body_id = BodyId {
hir_id: body.value.hir_id,
};
let def_id = cx.tcx.hir().body_owner_def_id(body_id);
let typeck_results = cx.tcx.typeck(def_id);
let typeck_results = cx.tcx.typeck_body(body_id);
let expr_ty = typeck_results.expr_ty(&body.value);
if implements_trait(cx, expr_ty, future_trait_def_id, &[]) {

2
clippy_lints/src/attrs.rs
View file

@ -640,7 +640,7 @@ fn check_mismatched_target_os(cx: &EarlyContext<'_>, attr: &Attribute) {
diag.span_suggestion(span, "try", sugg, Applicability::MaybeIncorrect);
if !unix_suggested && is_unix(os) {
diag.help("Did you mean `unix`?");
diag.help("did you mean `unix`?");
unix_suggested = true;
}
}

7
clippy_lints/src/await_holding_invalid.rs
View file

@ -97,8 +97,7 @@ impl LateLintPass<'_> for AwaitHolding {
let body_id = BodyId {
hir_id: body.value.hir_id,
};
let def_id = cx.tcx.hir().body_owner_def_id(body_id);
let typeck_results = cx.tcx.typeck(def_id);
let typeck_results = cx.tcx.typeck_body(body_id);
check_interior_types(
cx,
&typeck_results.generator_interior_types.as_ref().skip_binder(),
@ -116,7 +115,7 @@ fn check_interior_types(cx: &LateContext<'_>, ty_causes: &[GeneratorInteriorType
cx,
AWAIT_HOLDING_LOCK,
ty_cause.span,
"this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await.",
"this MutexGuard is held across an 'await' point. Consider using an async-aware Mutex type or ensuring the MutexGuard is dropped before calling await",
ty_cause.scope_span.or(Some(span)),
"these are all the await points this lock is held through",
);
@ -126,7 +125,7 @@ fn check_interior_types(cx: &LateContext<'_>, ty_causes: &[GeneratorInteriorType
cx,
AWAIT_HOLDING_REFCELL_REF,
ty_cause.span,
"this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await.",
"this RefCell Ref is held across an 'await' point. Consider ensuring the Ref is dropped before calling await",
ty_cause.scope_span.or(Some(span)),
"these are all the await points this ref is held through",
);

2
clippy_lints/src/bytecount.rs
View file

@ -28,7 +28,7 @@ declare_clippy_lint! {
/// &vec.iter().filter(|x| **x == 0u8).count(); // use bytecount::count instead
/// ```
pub NAIVE_BYTECOUNT,
perf,
pedantic,
"use of naive `<slice>.filter(|&x| x == y).count()` to count byte values"
}

7
clippy_lints/src/case_sensitive_file_extension_comparisons.rs
View file

@ -1,12 +1,11 @@
use crate::utils::paths::STRING;
use crate::utils::{match_def_path, span_lint_and_help};
use crate::utils::span_lint_and_help;
use if_chain::if_chain;
use rustc_ast::ast::LitKind;
use rustc_hir::{Expr, ExprKind, PathSegment};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{source_map::Spanned, Span};
use rustc_span::{source_map::Spanned, symbol::sym, Span};
declare_clippy_lint! {
/// **What it does:**
@ -59,7 +58,7 @@ fn check_case_sensitive_file_extension_comparison(ctx: &LateContext<'_>, expr: &
return Some(span);
},
ty::Adt(&ty::AdtDef { did, .. }, _) => {
if match_def_path(ctx, did, &STRING) {
if ctx.tcx.is_diagnostic_item(sym::string_type, did) {
return Some(span);
}
},

87
clippy_lints/src/casts/cast_lossless.rs Normal file
View file

@ -0,0 +1,87 @@
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, FloatTy, Ty};
use crate::utils::{in_constant, is_isize_or_usize, snippet_opt, span_lint_and_sugg};
use super::{utils, CAST_LOSSLESS};
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
if !should_lint(cx, expr, cast_from, cast_to) {
return;
}
// The suggestion is to use a function call, so if the original expression
// has parens on the outside, they are no longer needed.
let mut applicability = Applicability::MachineApplicable;
let opt = snippet_opt(cx, cast_op.span);
let sugg = opt.as_ref().map_or_else(
|| {
applicability = Applicability::HasPlaceholders;
".."
},
|snip| {
if should_strip_parens(cast_op, snip) {
&snip[1..snip.len() - 1]
} else {
snip.as_str()
}
},
);
span_lint_and_sugg(
cx,
CAST_LOSSLESS,
expr.span,
&format!(
"casting `{}` to `{}` may become silently lossy if you later change the type",
cast_from, cast_to
),
"try",
format!("{}::from({})", cast_to, sugg),
applicability,
);
}
fn should_lint(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) -> bool {
// Do not suggest using From in consts/statics until it is valid to do so (see #2267).
if in_constant(cx, expr.hir_id) {
return false;
}
match (cast_from.is_integral(), cast_to.is_integral()) {
(true, true) => {
let cast_signed_to_unsigned = cast_from.is_signed() && !cast_to.is_signed();
let from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx);
let to_nbits = utils::int_ty_to_nbits(cast_to, cx.tcx);
!is_isize_or_usize(cast_from)
&& !is_isize_or_usize(cast_to)
&& from_nbits < to_nbits
&& !cast_signed_to_unsigned
},
(true, false) => {
let from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx);
let to_nbits = if let ty::Float(FloatTy::F32) = cast_to.kind() {
32
} else {
64
};
from_nbits < to_nbits
},
(_, _) => {
matches!(cast_from.kind(), ty::Float(FloatTy::F32)) && matches!(cast_to.kind(), ty::Float(FloatTy::F64))
},
}
}
fn should_strip_parens(cast_expr: &Expr<'_>, snip: &str) -> bool {
if let ExprKind::Binary(_, _, _) = cast_expr.kind {
if snip.starts_with('(') && snip.ends_with(')') {
return true;
}
}
false
}

54
clippy_lints/src/casts/cast_possible_truncation.rs Normal file
View file

@ -0,0 +1,54 @@
use rustc_hir::Expr;
use rustc_lint::LateContext;
use rustc_middle::ty::{self, FloatTy, Ty};
use crate::utils::{is_isize_or_usize, span_lint};
use super::{utils, CAST_POSSIBLE_TRUNCATION};
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
let msg = match (cast_from.is_integral(), cast_to.is_integral()) {
(true, true) => {
let from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx);
let to_nbits = utils::int_ty_to_nbits(cast_to, cx.tcx);
let (should_lint, suffix) = match (is_isize_or_usize(cast_from), is_isize_or_usize(cast_to)) {
(true, true) | (false, false) => (to_nbits < from_nbits, ""),
(true, false) => (
to_nbits <= 32,
if to_nbits == 32 {
" on targets with 64-bit wide pointers"
} else {
""
},
),
(false, true) => (from_nbits == 64, " on targets with 32-bit wide pointers"),
};
if !should_lint {
return;
}
format!(
"casting `{}` to `{}` may truncate the value{}",
cast_from, cast_to, suffix,
)
},
(false, true) => {
format!("casting `{}` to `{}` may truncate the value", cast_from, cast_to)
},
(_, _) => {
if matches!(cast_from.kind(), &ty::Float(FloatTy::F64))
&& matches!(cast_to.kind(), &ty::Float(FloatTy::F32))
{
"casting `f64` to `f32` may truncate the value".to_string()
} else {
return;
}
},
};
span_lint(cx, CAST_POSSIBLE_TRUNCATION, expr.span, &msg);
}

44
clippy_lints/src/casts/cast_possible_wrap.rs Normal file
View file

@ -0,0 +1,44 @@
use rustc_hir::Expr;
use rustc_lint::LateContext;
use rustc_middle::ty::Ty;
use crate::utils::{is_isize_or_usize, span_lint};
use super::{utils, CAST_POSSIBLE_WRAP};
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
if !(cast_from.is_integral() && cast_to.is_integral()) {
return;
}
let arch_64_suffix = " on targets with 64-bit wide pointers";
let arch_32_suffix = " on targets with 32-bit wide pointers";
let cast_unsigned_to_signed = !cast_from.is_signed() && cast_to.is_signed();
let from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx);
let to_nbits = utils::int_ty_to_nbits(cast_to, cx.tcx);
let (should_lint, suffix) = match (is_isize_or_usize(cast_from), is_isize_or_usize(cast_to)) {
(true, true) | (false, false) => (to_nbits == from_nbits && cast_unsigned_to_signed, ""),
(true, false) => (to_nbits <= 32 && cast_unsigned_to_signed, arch_32_suffix),
(false, true) => (
cast_unsigned_to_signed,
if from_nbits == 64 {
arch_64_suffix
} else {
arch_32_suffix
},
),
};
if should_lint {
span_lint(
cx,
CAST_POSSIBLE_WRAP,
expr.span,
&format!(
"casting `{}` to `{}` may wrap around the value{}",
cast_from, cast_to, suffix,
),
);
}
}

51
clippy_lints/src/casts/cast_precision_loss.rs Normal file
View file

@ -0,0 +1,51 @@
use rustc_hir::Expr;
use rustc_lint::LateContext;
use rustc_middle::ty::{self, FloatTy, Ty};
use crate::utils::{is_isize_or_usize, span_lint};
use super::{utils, CAST_PRECISION_LOSS};
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
if !cast_from.is_integral() || cast_to.is_integral() {
return;
}
let from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx);
let to_nbits = if let ty::Float(FloatTy::F32) = cast_to.kind() {
32
} else {
64
};
if !(is_isize_or_usize(cast_from) || from_nbits >= to_nbits) {
return;
}
let cast_to_f64 = to_nbits == 64;
let mantissa_nbits = if cast_to_f64 { 52 } else { 23 };
let arch_dependent = is_isize_or_usize(cast_from) && cast_to_f64;
let arch_dependent_str = "on targets with 64-bit wide pointers ";
let from_nbits_str = if arch_dependent {
"64".to_owned()
} else if is_isize_or_usize(cast_from) {
"32 or 64".to_owned()
} else {
utils::int_ty_to_nbits(cast_from, cx.tcx).to_string()
};
span_lint(
cx,
CAST_PRECISION_LOSS,
expr.span,
&format!(
"casting `{0}` to `{1}` causes a loss of precision {2}(`{0}` is {3} bits wide, \
but `{1}`'s mantissa is only {4} bits wide)",
cast_from,
if cast_to_f64 { "f64" } else { "f32" },
if arch_dependent { arch_dependent_str } else { "" },
from_nbits_str,
mantissa_nbits
),
);
}

81
clippy_lints/src/casts/cast_ptr_alignment.rs Normal file
View file

@ -0,0 +1,81 @@
use rustc_hir::{Expr, ExprKind, GenericArg};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty};
use rustc_span::symbol::sym;
use rustc_target::abi::LayoutOf;
use if_chain::if_chain;
use crate::utils::{is_hir_ty_cfg_dependant, span_lint};
use super::CAST_PTR_ALIGNMENT;
pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let ExprKind::Cast(ref cast_expr, cast_to) = expr.kind {
if is_hir_ty_cfg_dependant(cx, cast_to) {
return;
}
let (cast_from, cast_to) = (
cx.typeck_results().expr_ty(cast_expr),
cx.typeck_results().expr_ty(expr),
);
lint_cast_ptr_alignment(cx, expr, cast_from, cast_to);
} else if let ExprKind::MethodCall(method_path, _, args, _) = expr.kind {
if_chain! {
if method_path.ident.name == sym!(cast);
if let Some(generic_args) = method_path.args;
if let [GenericArg::Type(cast_to)] = generic_args.args;
// There probably is no obvious reason to do this, just to be consistent with `as` cases.
if !is_hir_ty_cfg_dependant(cx, cast_to);
then {
let (cast_from, cast_to) =
(cx.typeck_results().expr_ty(&args[0]), cx.typeck_results().expr_ty(expr));
lint_cast_ptr_alignment(cx, expr, cast_from, cast_to);
}
}
}
}
fn lint_cast_ptr_alignment<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, cast_from: Ty<'tcx>, cast_to: Ty<'tcx>) {
if_chain! {
if let ty::RawPtr(from_ptr_ty) = &cast_from.kind();
if let ty::RawPtr(to_ptr_ty) = &cast_to.kind();
if let Ok(from_layout) = cx.layout_of(from_ptr_ty.ty);
if let Ok(to_layout) = cx.layout_of(to_ptr_ty.ty);
if from_layout.align.abi < to_layout.align.abi;
// with c_void, we inherently need to trust the user
if !is_c_void(cx, from_ptr_ty.ty);
// when casting from a ZST, we don't know enough to properly lint
if !from_layout.is_zst();
then {
span_lint(
cx,
CAST_PTR_ALIGNMENT,
expr.span,
&format!(
"casting from `{}` to a more-strictly-aligned pointer (`{}`) ({} < {} bytes)",
cast_from,
cast_to,
from_layout.align.abi.bytes(),
to_layout.align.abi.bytes(),
),
);
}
}
}
/// Check if the given type is either `core::ffi::c_void` or
/// one of the platform specific `libc::<platform>::c_void` of libc.
fn is_c_void(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
if let ty::Adt(adt, _) = ty.kind() {
let names = cx.get_def_path(adt.did);
if names.is_empty() {
return false;
}
if names[0] == sym::libc || names[0] == sym::core && *names.last().unwrap() == sym!(c_void) {
return true;
}
}
false
}

28
clippy_lints/src/casts/cast_ref_to_mut.rs Normal file
View file

@ -0,0 +1,28 @@
use rustc_hir::{Expr, ExprKind, MutTy, Mutability, TyKind, UnOp};
use rustc_lint::LateContext;
use rustc_middle::ty;
use if_chain::if_chain;
use crate::utils::span_lint;
use super::CAST_REF_TO_MUT;
pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if_chain! {
if let ExprKind::Unary(UnOp::Deref, e) = &expr.kind;
if let ExprKind::Cast(e, t) = &e.kind;
if let TyKind::Ptr(MutTy { mutbl: Mutability::Mut, .. }) = t.kind;
if let ExprKind::Cast(e, t) = &e.kind;
if let TyKind::Ptr(MutTy { mutbl: Mutability::Not, .. }) = t.kind;
if let ty::Ref(..) = cx.typeck_results().node_type(e.hir_id).kind();
then {
span_lint(
cx,
CAST_REF_TO_MUT,
expr.span,
"casting `&T` to `&mut T` may cause undefined behavior, consider instead using an `UnsafeCell`",
);
}
}
}

70
clippy_lints/src/casts/cast_sign_loss.rs Normal file
View file

@ -0,0 +1,70 @@
use rustc_hir::{Expr, ExprKind};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty};
use if_chain::if_chain;
use crate::consts::{constant, Constant};
use crate::utils::{method_chain_args, sext, span_lint};
use super::CAST_SIGN_LOSS;
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
if should_lint(cx, cast_op, cast_from, cast_to) {
span_lint(
cx,
CAST_SIGN_LOSS,
expr.span,
&format!(
"casting `{}` to `{}` may lose the sign of the value",
cast_from, cast_to
),
);
}
}
fn should_lint(cx: &LateContext<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) -> bool {
match (cast_from.is_integral(), cast_to.is_integral()) {
(true, true) => {
if !cast_from.is_signed() || cast_to.is_signed() {
return false;
}
// Don't lint for positive constants.
let const_val = constant(cx, &cx.typeck_results(), cast_op);
if_chain! {
if let Some((Constant::Int(n), _)) = const_val;
if let ty::Int(ity) = *cast_from.kind();
if sext(cx.tcx, n, ity) >= 0;
then {
return false;
}
}
// Don't lint for the result of methods that always return non-negative values.
if let ExprKind::MethodCall(ref path, _, _, _) = cast_op.kind {
let mut method_name = path.ident.name.as_str();
let allowed_methods = ["abs", "checked_abs", "rem_euclid", "checked_rem_euclid"];
if_chain! {
if method_name == "unwrap";
if let Some(arglist) = method_chain_args(cast_op, &["unwrap"]);
if let ExprKind::MethodCall(ref inner_path, _, _, _) = &arglist[0][0].kind;
then {
method_name = inner_path.ident.name.as_str();
}
}
if allowed_methods.iter().any(|&name| method_name == name) {
return false;
}
}
true
},
(false, true) => !cast_to.is_signed(),
(_, _) => false,
}
}

42
clippy_lints/src/casts/char_lit_as_u8.rs Normal file
View file

@ -0,0 +1,42 @@
use rustc_ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, UintTy};
use if_chain::if_chain;
use crate::utils::{snippet_with_applicability, span_lint_and_then};
use super::CHAR_LIT_AS_U8;
pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if_chain! {
if let ExprKind::Cast(e, _) = &expr.kind;
if let ExprKind::Lit(l) = &e.kind;
if let LitKind::Char(c) = l.node;
if ty::Uint(UintTy::U8) == *cx.typeck_results().expr_ty(expr).kind();
then {
let mut applicability = Applicability::MachineApplicable;
let snippet = snippet_with_applicability(cx, e.span, "'x'", &mut applicability);
span_lint_and_then(
cx,
CHAR_LIT_AS_U8,
expr.span,
"casting a character literal to `u8` truncates",
|diag| {
diag.note("`char` is four bytes wide, but `u8` is a single byte");
if c.is_ascii() {
diag.span_suggestion(
expr.span,
"use a byte literal instead",
format!("b{}", snippet),
applicability,
);
}
});
}
}
}

37
clippy_lints/src/casts/fn_to_numeric_cast.rs Normal file
View file

@ -0,0 +1,37 @@
use rustc_errors::Applicability;
use rustc_hir::Expr;
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty, UintTy};
use crate::utils::{snippet_with_applicability, span_lint_and_sugg};
use super::{utils, FN_TO_NUMERIC_CAST};
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
// We only want to check casts to `ty::Uint` or `ty::Int`
match cast_to.kind() {
ty::Uint(_) | ty::Int(..) => { /* continue on */ },
_ => return,
}
match cast_from.kind() {
ty::FnDef(..) | ty::FnPtr(_) => {
let mut applicability = Applicability::MaybeIncorrect;
let from_snippet = snippet_with_applicability(cx, cast_expr.span, "x", &mut applicability);
let to_nbits = utils::int_ty_to_nbits(cast_to, cx.tcx);
if (to_nbits >= cx.tcx.data_layout.pointer_size.bits()) && (*cast_to.kind() != ty::Uint(UintTy::Usize)) {
span_lint_and_sugg(
cx,
FN_TO_NUMERIC_CAST,
expr.span,
&format!("casting function pointer `{}` to `{}`", from_snippet, cast_to),
"try",
format!("{} as usize", from_snippet),
applicability,
);
}
},
_ => {},
}
}

39
clippy_lints/src/casts/fn_to_numeric_cast_with_truncation.rs Normal file
View file

@ -0,0 +1,39 @@
use rustc_errors::Applicability;
use rustc_hir::Expr;
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty};
use crate::utils::{snippet_with_applicability, span_lint_and_sugg};
use super::{utils, FN_TO_NUMERIC_CAST_WITH_TRUNCATION};
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
// We only want to check casts to `ty::Uint` or `ty::Int`
match cast_to.kind() {
ty::Uint(_) | ty::Int(..) => { /* continue on */ },
_ => return,
}
match cast_from.kind() {
ty::FnDef(..) | ty::FnPtr(_) => {
let mut applicability = Applicability::MaybeIncorrect;
let from_snippet = snippet_with_applicability(cx, cast_expr.span, "x", &mut applicability);
let to_nbits = utils::int_ty_to_nbits(cast_to, cx.tcx);
if to_nbits < cx.tcx.data_layout.pointer_size.bits() {
span_lint_and_sugg(
cx,
FN_TO_NUMERIC_CAST_WITH_TRUNCATION,
expr.span,
&format!(
"casting function pointer `{}` to `{}`, which truncates the value",
from_snippet, cast_to
),
"try",
format!("{} as usize", from_snippet),
applicability,
);
}
},
_ => {},
}
}

407
clippy_lints/src/casts/mod.rs Normal file
View file

@ -0,0 +1,407 @@
mod cast_lossless;
mod cast_possible_truncation;
mod cast_possible_wrap;
mod cast_precision_loss;
mod cast_ptr_alignment;
mod cast_ref_to_mut;
mod cast_sign_loss;
mod char_lit_as_u8;
mod fn_to_numeric_cast;
mod fn_to_numeric_cast_with_truncation;
mod ptr_as_ptr;
mod unnecessary_cast;
mod utils;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use crate::utils::is_hir_ty_cfg_dependant;
declare_clippy_lint! {
/// **What it does:** Checks for casts from any numerical to a float type where
/// the receiving type cannot store all values from the original type without
/// rounding errors. This possible rounding is to be expected, so this lint is
/// `Allow` by default.
///
/// Basically, this warns on casting any integer with 32 or more bits to `f32`
/// or any 64-bit integer to `f64`.
///
/// **Why is this bad?** It's not bad at all. But in some applications it can be
/// helpful to know where precision loss can take place. This lint can help find
/// those places in the code.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// let x = u64::MAX;
/// x as f64;
/// ```
pub CAST_PRECISION_LOSS,
pedantic,
"casts that cause loss of precision, e.g., `x as f32` where `x: u64`"
}
declare_clippy_lint! {
/// **What it does:** Checks for casts from a signed to an unsigned numerical
/// type. In this case, negative values wrap around to large positive values,
/// which can be quite surprising in practice. However, as the cast works as
/// defined, this lint is `Allow` by default.
///
/// **Why is this bad?** Possibly surprising results. You can activate this lint
/// as a one-time check to see where numerical wrapping can arise.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// let y: i8 = -1;
/// y as u128; // will return 18446744073709551615
/// ```
pub CAST_SIGN_LOSS,
pedantic,
"casts from signed types to unsigned types, e.g., `x as u32` where `x: i32`"
}
declare_clippy_lint! {
/// **What it does:** Checks for casts between numerical types that may
/// truncate large values. This is expected behavior, so the cast is `Allow` by
/// default.
///
/// **Why is this bad?** In some problem domains, it is good practice to avoid
/// truncation. This lint can be activated to help assess where additional
/// checks could be beneficial.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// fn as_u8(x: u64) -> u8 {
/// x as u8
/// }
/// ```
pub CAST_POSSIBLE_TRUNCATION,
pedantic,
"casts that may cause truncation of the value, e.g., `x as u8` where `x: u32`, or `x as i32` where `x: f32`"
}
declare_clippy_lint! {
/// **What it does:** Checks for casts from an unsigned type to a signed type of
/// the same size. Performing such a cast is a 'no-op' for the compiler,
/// i.e., nothing is changed at the bit level, and the binary representation of
/// the value is reinterpreted. This can cause wrapping if the value is too big
/// for the target signed type. However, the cast works as defined, so this lint
/// is `Allow` by default.
///
/// **Why is this bad?** While such a cast is not bad in itself, the results can
/// be surprising when this is not the intended behavior, as demonstrated by the
/// example below.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// u32::MAX as i32; // will yield a value of `-1`
/// ```
pub CAST_POSSIBLE_WRAP,
pedantic,
"casts that may cause wrapping around the value, e.g., `x as i32` where `x: u32` and `x > i32::MAX`"
}
declare_clippy_lint! {
/// **What it does:** Checks for casts between numerical types that may
/// be replaced by safe conversion functions.
///
/// **Why is this bad?** Rust's `as` keyword will perform many kinds of
/// conversions, including silently lossy conversions. Conversion functions such
/// as `i32::from` will only perform lossless conversions. Using the conversion
/// functions prevents conversions from turning into silent lossy conversions if
/// the types of the input expressions ever change, and make it easier for
/// people reading the code to know that the conversion is lossless.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// fn as_u64(x: u8) -> u64 {
/// x as u64
/// }
/// ```
///
/// Using `::from` would look like this:
///
/// ```rust
/// fn as_u64(x: u8) -> u64 {
/// u64::from(x)
/// }
/// ```
pub CAST_LOSSLESS,
pedantic,
"casts using `as` that are known to be lossless, e.g., `x as u64` where `x: u8`"
}
declare_clippy_lint! {
/// **What it does:** Checks for casts to the same type, casts of int literals to integer types
/// and casts of float literals to float types.
///
/// **Why is this bad?** It's just unnecessary.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// let _ = 2i32 as i32;
/// let _ = 0.5 as f32;
/// ```
///
/// Better:
///
/// ```rust
/// let _ = 2_i32;
/// let _ = 0.5_f32;
/// ```
pub UNNECESSARY_CAST,
complexity,
"cast to the same type, e.g., `x as i32` where `x: i32`"
}
declare_clippy_lint! {
/// **What it does:** Checks for casts, using `as` or `pointer::cast`,
/// from a less-strictly-aligned pointer to a more-strictly-aligned pointer
///
/// **Why is this bad?** Dereferencing the resulting pointer may be undefined
/// behavior.
///
/// **Known problems:** Using `std::ptr::read_unaligned` and `std::ptr::write_unaligned` or similar
/// on the resulting pointer is fine. Is over-zealous: Casts with manual alignment checks or casts like
/// u64-> u8 -> u16 can be fine. Miri is able to do a more in-depth analysis.
///
/// **Example:**
/// ```rust
/// let _ = (&1u8 as *const u8) as *const u16;
/// let _ = (&mut 1u8 as *mut u8) as *mut u16;
///
/// (&1u8 as *const u8).cast::<u16>();
/// (&mut 1u8 as *mut u8).cast::<u16>();
/// ```
pub CAST_PTR_ALIGNMENT,
pedantic,
"cast from a pointer to a more-strictly-aligned pointer"
}
declare_clippy_lint! {
/// **What it does:** Checks for casts of function pointers to something other than usize
///
/// **Why is this bad?**
/// Casting a function pointer to anything other than usize/isize is not portable across
/// architectures, because you end up losing bits if the target type is too small or end up with a
/// bunch of extra bits that waste space and add more instructions to the final binary than
/// strictly necessary for the problem
///
/// Casting to isize also doesn't make sense since there are no signed addresses.
///
/// **Example**
///
/// ```rust
/// // Bad
/// fn fun() -> i32 { 1 }
/// let a = fun as i64;
///
/// // Good
/// fn fun2() -> i32 { 1 }
/// let a = fun2 as usize;
/// ```
pub FN_TO_NUMERIC_CAST,
style,
"casting a function pointer to a numeric type other than usize"
}
declare_clippy_lint! {
/// **What it does:** Checks for casts of a function pointer to a numeric type not wide enough to
/// store address.
///
/// **Why is this bad?**
/// Such a cast discards some bits of the function's address. If this is intended, it would be more
/// clearly expressed by casting to usize first, then casting the usize to the intended type (with
/// a comment) to perform the truncation.
///
/// **Example**
///
/// ```rust
/// // Bad
/// fn fn1() -> i16 {
/// 1
/// };
/// let _ = fn1 as i32;
///
/// // Better: Cast to usize first, then comment with the reason for the truncation
/// fn fn2() -> i16 {
/// 1
/// };
/// let fn_ptr = fn2 as usize;
/// let fn_ptr_truncated = fn_ptr as i32;
/// ```
pub FN_TO_NUMERIC_CAST_WITH_TRUNCATION,
style,
"casting a function pointer to a numeric type not wide enough to store the address"
}
declare_clippy_lint! {
/// **What it does:** Checks for casts of `&T` to `&mut T` anywhere in the code.
///
/// **Why is this bad?** It’s basically guaranteed to be undefined behaviour.
/// `UnsafeCell` is the only way to obtain aliasable data that is considered
/// mutable.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust,ignore
/// fn x(r: &i32) {
/// unsafe {
/// *(r as *const _ as *mut _) += 1;
/// }
/// }
/// ```
///
/// Instead consider using interior mutability types.
///
/// ```rust
/// use std::cell::UnsafeCell;
///
/// fn x(r: &UnsafeCell<i32>) {
/// unsafe {
/// *r.get() += 1;
/// }
/// }
/// ```
pub CAST_REF_TO_MUT,
correctness,
"a cast of reference to a mutable pointer"
}
declare_clippy_lint! {
/// **What it does:** Checks for expressions where a character literal is cast
/// to `u8` and suggests using a byte literal instead.
///
/// **Why is this bad?** In general, casting values to smaller types is
/// error-prone and should be avoided where possible. In the particular case of
/// converting a character literal to u8, it is easy to avoid by just using a
/// byte literal instead. As an added bonus, `b'a'` is even slightly shorter
/// than `'a' as u8`.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust,ignore
/// 'x' as u8
/// ```
///
/// A better version, using the byte literal:
///
/// ```rust,ignore
/// b'x'
/// ```
pub CHAR_LIT_AS_U8,
complexity,
"casting a character literal to `u8` truncates"
}
declare_clippy_lint! {
/// **What it does:**
/// Checks for `as` casts between raw pointers without changing its mutability,
/// namely `*const T` to `*const U` and `*mut T` to `*mut U`.
///
/// **Why is this bad?**
/// Though `as` casts between raw pointers is not terrible, `pointer::cast` is safer because
/// it cannot accidentally change the pointer's mutability nor cast the pointer to other types like `usize`.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// let ptr: *const u32 = &42_u32;
/// let mut_ptr: *mut u32 = &mut 42_u32;
/// let _ = ptr as *const i32;
/// let _ = mut_ptr as *mut i32;
/// ```
/// Use instead:
/// ```rust
/// let ptr: *const u32 = &42_u32;
/// let mut_ptr: *mut u32 = &mut 42_u32;
/// let _ = ptr.cast::<i32>();
/// let _ = mut_ptr.cast::<i32>();
/// ```
pub PTR_AS_PTR,
pedantic,
"casting using `as` from and to raw pointers that doesn't change its mutability, where `pointer::cast` could take the place of `as`"
}
pub struct Casts {
msrv: Option<RustcVersion>,
}
impl Casts {
#[must_use]
pub fn new(msrv: Option<RustcVersion>) -> Self {
Self { msrv }
}
}
impl_lint_pass!(Casts => [
CAST_PRECISION_LOSS,
CAST_SIGN_LOSS,
CAST_POSSIBLE_TRUNCATION,
CAST_POSSIBLE_WRAP,
CAST_LOSSLESS,
CAST_REF_TO_MUT,
CAST_PTR_ALIGNMENT,
UNNECESSARY_CAST,
FN_TO_NUMERIC_CAST,
FN_TO_NUMERIC_CAST_WITH_TRUNCATION,
CHAR_LIT_AS_U8,
PTR_AS_PTR,
]);
impl<'tcx> LateLintPass<'tcx> for Casts {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if expr.span.from_expansion() {
return;
}
if let ExprKind::Cast(ref cast_expr, cast_to) = expr.kind {
if is_hir_ty_cfg_dependant(cx, cast_to) {
return;
}
let (cast_from, cast_to) = (
cx.typeck_results().expr_ty(cast_expr),
cx.typeck_results().expr_ty(expr),
);
if unnecessary_cast::check(cx, expr, cast_expr, cast_from, cast_to) {
return;
}
fn_to_numeric_cast::check(cx, expr, cast_expr, cast_from, cast_to);
fn_to_numeric_cast_with_truncation::check(cx, expr, cast_expr, cast_from, cast_to);
if cast_from.is_numeric() && cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) {
cast_possible_truncation::check(cx, expr, cast_from, cast_to);
cast_possible_wrap::check(cx, expr, cast_from, cast_to);
cast_precision_loss::check(cx, expr, cast_from, cast_to);
cast_lossless::check(cx, expr, cast_expr, cast_from, cast_to);
cast_sign_loss::check(cx, expr, cast_expr, cast_from, cast_to);
}
}
cast_ref_to_mut::check(cx, expr);
cast_ptr_alignment::check(cx, expr);
char_lit_as_u8::check(cx, expr);
ptr_as_ptr::check(cx, expr, &self.msrv);
}
extract_msrv_attr!(LateContext);
}

52
clippy_lints/src/casts/ptr_as_ptr.rs Normal file
View file

@ -0,0 +1,52 @@
use std::borrow::Cow;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, Mutability, TyKind};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, TypeAndMut};
use rustc_semver::RustcVersion;
use if_chain::if_chain;
use crate::utils::sugg::Sugg;
use crate::utils::{meets_msrv, span_lint_and_sugg};
use super::PTR_AS_PTR;
const PTR_AS_PTR_MSRV: RustcVersion = RustcVersion::new(1, 38, 0);
pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: &Option<RustcVersion>) {
if !meets_msrv(msrv.as_ref(), &PTR_AS_PTR_MSRV) {
return;
}
if_chain! {
if let ExprKind::Cast(cast_expr, cast_to_hir_ty) = expr.kind;
let (cast_from, cast_to) = (cx.typeck_results().expr_ty(cast_expr), cx.typeck_results().expr_ty(expr));
if let ty::RawPtr(TypeAndMut { mutbl: from_mutbl, .. }) = cast_from.kind();
if let ty::RawPtr(TypeAndMut { ty: to_pointee_ty, mutbl: to_mutbl }) = cast_to.kind();
if matches!((from_mutbl, to_mutbl),
(Mutability::Not, Mutability::Not) | (Mutability::Mut, Mutability::Mut));
// The `U` in `pointer::cast` have to be `Sized`
// as explained here: https://github.com/rust-lang/rust/issues/60602.
if to_pointee_ty.is_sized(cx.tcx.at(expr.span), cx.param_env);
then {
let mut applicability = Applicability::MachineApplicable;
let cast_expr_sugg = Sugg::hir_with_applicability(cx, cast_expr, "_", &mut applicability);
let turbofish = match &cast_to_hir_ty.kind {
TyKind::Infer => Cow::Borrowed(""),
TyKind::Ptr(mut_ty) if matches!(mut_ty.ty.kind, TyKind::Infer) => Cow::Borrowed(""),
_ => Cow::Owned(format!("::<{}>", to_pointee_ty)),
};
span_lint_and_sugg(
cx,
PTR_AS_PTR,
expr.span,
"`as` casting between raw pointers without changing its mutability",
"try `pointer::cast`, a safer alternative",
format!("{}.cast{}()", cast_expr_sugg.maybe_par(), turbofish),
applicability,
);
}
}
}

106
clippy_lints/src/casts/unnecessary_cast.rs Normal file
View file

@ -0,0 +1,106 @@
use rustc_ast::{LitFloatType, LitIntType, LitKind};
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, Lit, UnOp};
use rustc_lint::{LateContext, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::{self, FloatTy, InferTy, Ty};
use if_chain::if_chain;
use crate::utils::{numeric_literal::NumericLiteral, snippet_opt, span_lint, span_lint_and_sugg};
use super::UNNECESSARY_CAST;
pub(super) fn check(
cx: &LateContext<'_>,
expr: &Expr<'_>,
cast_expr: &Expr<'_>,
cast_from: Ty<'_>,
cast_to: Ty<'_>,
) -> bool {
if let Some(lit) = get_numeric_literal(cast_expr) {
let literal_str = snippet_opt(cx, cast_expr.span).unwrap_or_default();
if_chain! {
if let LitKind::Int(n, _) = lit.node;
if let Some(src) = snippet_opt(cx, lit.span);
if cast_to.is_floating_point();
if let Some(num_lit) = NumericLiteral::from_lit_kind(&src, &lit.node);
let from_nbits = 128 - n.leading_zeros();
let to_nbits = fp_ty_mantissa_nbits(cast_to);
if from_nbits != 0 && to_nbits != 0 && from_nbits <= to_nbits && num_lit.is_decimal();
then {
let literal_str = if is_unary_neg(cast_expr) { format!("-{}", num_lit.integer) } else { num_lit.integer.into() };
lint_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to);
return true
}
}
match lit.node {
LitKind::Int(_, LitIntType::Unsuffixed) if cast_to.is_integral() => {
lint_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to);
},
LitKind::Float(_, LitFloatType::Unsuffixed) if cast_to.is_floating_point() => {
lint_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to);
},
LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::Float(_, LitFloatType::Unsuffixed) => {},
_ => {
if cast_from.kind() == cast_to.kind() && !in_external_macro(cx.sess(), expr.span) {
span_lint(
cx,
UNNECESSARY_CAST,
expr.span,
&format!(
"casting to the same type is unnecessary (`{}` -> `{}`)",
cast_from, cast_to
),
);
return true;
}
},
}
}
false
}
fn lint_unnecessary_cast(cx: &LateContext<'_>, expr: &Expr<'_>, literal_str: &str, cast_from: Ty<'_>, cast_to: Ty<'_>) {
let literal_kind_name = if cast_from.is_integral() { "integer" } else { "float" };
span_lint_and_sugg(
cx,
UNNECESSARY_CAST,
expr.span,
&format!("casting {} literal to `{}` is unnecessary", literal_kind_name, cast_to),
"try",
format!("{}_{}", literal_str.trim_end_matches('.'), cast_to),
Applicability::MachineApplicable,
);
}
fn get_numeric_literal<'e>(expr: &'e Expr<'e>) -> Option<&'e Lit> {
match expr.kind {
ExprKind::Lit(ref lit) => Some(lit),
ExprKind::Unary(UnOp::Neg, e) => {
if let ExprKind::Lit(ref lit) = e.kind {
Some(lit)
} else {
None
}
},
_ => None,
}
}
/// Returns the mantissa bits wide of a fp type.
/// Will return 0 if the type is not a fp
fn fp_ty_mantissa_nbits(typ: Ty<'_>) -> u32 {
match typ.kind() {
ty::Float(FloatTy::F32) => 23,
ty::Float(FloatTy::F64) | ty::Infer(InferTy::FloatVar(_)) => 52,
_ => 0,
}
}
fn is_unary_neg(expr: &Expr<'_>) -> bool {
matches!(expr.kind, ExprKind::Unary(UnOp::Neg, _))
}

25
clippy_lints/src/casts/utils.rs Normal file
View file

@ -0,0 +1,25 @@
use rustc_middle::ty::{self, IntTy, Ty, TyCtxt, UintTy};
/// Returns the size in bits of an integral type.
/// Will return 0 if the type is not an int or uint variant
pub(super) fn int_ty_to_nbits(typ: Ty<'_>, tcx: TyCtxt<'_>) -> u64 {
match typ.kind() {
ty::Int(i) => match i {
IntTy::Isize => tcx.data_layout.pointer_size.bits(),
IntTy::I8 => 8,
IntTy::I16 => 16,
IntTy::I32 => 32,
IntTy::I64 => 64,
IntTy::I128 => 128,
},
ty::Uint(i) => match i {
UintTy::Usize => tcx.data_layout.pointer_size.bits(),
UintTy::U8 => 8,
UintTy::U16 => 16,
UintTy::U32 => 32,
UintTy::U64 => 64,
UintTy::U128 => 128,
},
_ => 0,
}
}

2
clippy_lints/src/comparison_chain.rs
View file

@ -117,7 +117,7 @@ impl<'tcx> LateLintPass<'tcx> for ComparisonChain {
expr.span,
"`if` chain can be rewritten with `match`",
None,
"Consider rewriting the `if` chain to use `cmp` and `match`.",
"consider rewriting the `if` chain to use `cmp` and `match`",
)
}
}

5
clippy_lints/src/default_numeric_fallback.rs
View file

@ -130,10 +130,9 @@ impl<'a, 'tcx> Visitor<'tcx> for NumericFallbackVisitor<'a, 'tcx> {
}
},
ExprKind::Struct(qpath, fields, base) => {
ExprKind::Struct(_, fields, base) => {
if_chain! {
if let Some(def_id) = self.cx.qpath_res(qpath, expr.hir_id).opt_def_id();
let ty = self.cx.tcx.type_of(def_id);
let ty = self.cx.typeck_results().expr_ty(expr);
if let Some(adt_def) = ty.ty_adt_def();
if adt_def.is_struct();
if let Some(variant) = adt_def.variants.iter().next();

9
clippy_lints/src/derive.rs
View file

@ -1,6 +1,6 @@
use crate::utils::paths;
use crate::utils::{
get_trait_def_id, is_allowed, is_automatically_derived, is_copy, match_def_path, match_path, span_lint_and_help,
get_trait_def_id, is_allowed, is_automatically_derived, is_copy, match_def_path, span_lint_and_help,
span_lint_and_note, span_lint_and_then,
};
use if_chain::if_chain;
@ -294,7 +294,12 @@ fn check_ord_partial_ord<'tcx>(
/// Implementation of the `EXPL_IMPL_CLONE_ON_COPY` lint.
fn check_copy_clone<'tcx>(cx: &LateContext<'tcx>, item: &Item<'_>, trait_ref: &TraitRef<'_>, ty: Ty<'tcx>) {
if match_path(&trait_ref.path, &paths::CLONE_TRAIT) {
if cx
.tcx
.lang_items()
.clone_trait()
.map_or(false, |id| Some(id) == trait_ref.trait_def_id())
{
if !is_copy(cx, ty) {
return;
}

6
clippy_lints/src/doc.rs
View file

@ -329,9 +329,9 @@ fn lint_for_missing_headers<'tcx>(
if_chain! {
if let Some(body_id) = body_id;
if let Some(future) = cx.tcx.lang_items().future_trait();
let def_id = cx.tcx.hir().body_owner_def_id(body_id);
let mir = cx.tcx.optimized_mir(def_id.to_def_id());
let ret_ty = mir.return_ty();
let typeck = cx.tcx.typeck_body(body_id);
let body = cx.tcx.hir().body(body_id);
let ret_ty = typeck.expr_ty(&body.value);
if implements_trait(cx, ret_ty, future, &[]);
if let ty::Opaque(_, subs) = ret_ty.kind();
if let Some(gen) = subs.types().next();

8
clippy_lints/src/drop_forget_ref.rs
View file

@ -98,13 +98,13 @@ declare_clippy_lint! {
}
const DROP_REF_SUMMARY: &str = "calls to `std::mem::drop` with a reference instead of an owned value. \
Dropping a reference does nothing.";
Dropping a reference does nothing";
const FORGET_REF_SUMMARY: &str = "calls to `std::mem::forget` with a reference instead of an owned value. \
Forgetting a reference does nothing.";
Forgetting a reference does nothing";
const DROP_COPY_SUMMARY: &str = "calls to `std::mem::drop` with a value that implements `Copy`. \
Dropping a copy leaves the original intact.";
Dropping a copy leaves the original intact";
const FORGET_COPY_SUMMARY: &str = "calls to `std::mem::forget` with a value that implements `Copy`. \
Forgetting a copy leaves the original intact.";
Forgetting a copy leaves the original intact";
declare_lint_pass!(DropForgetRef => [DROP_REF, FORGET_REF, DROP_COPY, FORGET_COPY]);

30
clippy_lints/src/eta_reduction.rs
View file

@ -10,6 +10,8 @@ use crate::utils::{
implements_trait, is_adjusted, iter_input_pats, snippet_opt, span_lint_and_sugg, span_lint_and_then,
type_is_unsafe_function,
};
use clippy_utils::higher;
use clippy_utils::higher::VecArgs;
declare_clippy_lint! {
/// **What it does:** Checks for closures which just call another function where
@ -74,8 +76,11 @@ impl<'tcx> LateLintPass<'tcx> for EtaReduction {
match expr.kind {
ExprKind::Call(_, args) | ExprKind::MethodCall(_, _, args, _) => {
for arg in args {
// skip `foo(macro!())`
if arg.span.ctxt() == expr.span.ctxt() {
check_closure(cx, arg)
}
}
},
_ => (),
}
@ -87,6 +92,23 @@ fn check_closure(cx: &LateContext<'_>, expr: &Expr<'_>) {
let body = cx.tcx.hir().body(eid);
let ex = &body.value;
if ex.span.ctxt() != expr.span.ctxt() {
if let Some(VecArgs::Vec(&[])) = higher::vec_macro(cx, ex) {
// replace `|| vec![]` with `Vec::new`
span_lint_and_sugg(
cx,
REDUNDANT_CLOSURE,
expr.span,
"redundant closure",
"replace the closure with `Vec::new`",
"std::vec::Vec::new".into(),
Applicability::MachineApplicable,
);
}
// skip `foo(|| macro!())`
return;
}
if_chain!(
if let ExprKind::Call(ref caller, ref args) = ex.kind;
@ -107,11 +129,11 @@ fn check_closure(cx: &LateContext<'_>, expr: &Expr<'_>) {
if compare_inputs(&mut iter_input_pats(decl, body), &mut args.iter());
then {
span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure found", |diag| {
span_lint_and_then(cx, REDUNDANT_CLOSURE, expr.span, "redundant closure", |diag| {
if let Some(snippet) = snippet_opt(cx, caller.span) {
diag.span_suggestion(
expr.span,
"remove closure as shown",
"replace the closure with the function itself",
snippet,
Applicability::MachineApplicable,
);
@ -141,8 +163,8 @@ fn check_closure(cx: &LateContext<'_>, expr: &Expr<'_>) {
cx,
REDUNDANT_CLOSURE_FOR_METHOD_CALLS,
expr.span,
"redundant closure found",
"remove closure as shown",
"redundant closure",
"replace the closure with the method itself",
format!("{}::{}", name, path.ident.name),
Applicability::MachineApplicable,
);

2
clippy_lints/src/fallible_impl_from.rs
View file

@ -133,7 +133,7 @@ fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_items: &[h
move |diag| {
diag.help(
"`From` is intended for infallible conversions only. \
Use `TryFrom` if there's a possibility for the conversion to fail.");
Use `TryFrom` if there's a possibility for the conversion to fail");
diag.span_note(fpu.result, "potential failure(s)");
});
}

6
clippy_lints/src/float_literal.rs
View file

@ -145,11 +145,7 @@ fn count_digits(s: &str) -> usize {
.take_while(|c| *c != 'e' && *c != 'E')
.fold(0, |count, c| {
// leading zeros
if c == '0' && count == 0 {
count
} else {
count + 1
}
if c == '0' && count == 0 { count } else { count + 1 }
})
}

4
clippy_lints/src/format.rs
View file

@ -28,11 +28,11 @@ declare_clippy_lint! {
/// ```rust
///
/// // Bad
/// # let foo = "foo";
/// let foo = "foo";
/// format!("{}", foo);
///
/// // Good
/// format!("foo");
/// foo.to_owned();
/// ```
pub USELESS_FORMAT,
complexity,

4
clippy_lints/src/functions.rs
View file

@ -317,9 +317,7 @@ impl<'tcx> LateLintPass<'tcx> for Functions {
let attr = must_use_attr(attrs);
if let Some(attr) = attr {
check_needless_must_use(cx, &sig.decl, item.hir_id(), item.span, fn_header_span, attr);
} else if is_public
&& !is_proc_macro(cx.sess(), attrs)
&& trait_ref_of_method(cx, item.hir_id()).is_none()
} else if is_public && !is_proc_macro(cx.sess(), attrs) && trait_ref_of_method(cx, item.hir_id()).is_none()
{
check_must_use_candidate(
cx,

18
clippy_lints/src/implicit_return.rs
View file

@ -1,4 +1,4 @@
use crate::utils::{fn_has_unsatisfiable_preds, match_panic_def_id, snippet_opt, span_lint_and_then};
use crate::utils::{match_panic_def_id, snippet_opt, span_lint_and_then};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::intravisit::FnKind;
@ -133,19 +133,13 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitReturn {
span: Span,
_: HirId,
) {
let def_id = cx.tcx.hir().body_owner_def_id(body.id());
// Building MIR for `fn`s with unsatisfiable preds results in ICE.
if fn_has_unsatisfiable_preds(cx, def_id.to_def_id()) {
if span.from_expansion() {
return;
}
let body = cx.tcx.hir().body(body.id());
if cx.typeck_results().expr_ty(&body.value).is_unit() {
return;
}
let mir = cx.tcx.optimized_mir(def_id.to_def_id());
// 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() && !span.from_expansion() {
expr_match(cx, &body.value);
}
}
}

3
clippy_lints/src/inconsistent_struct_constructor.rs
View file

@ -66,8 +66,7 @@ impl LateLintPass<'_> for InconsistentStructConstructor {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
if_chain! {
if let ExprKind::Struct(qpath, fields, base) = expr.kind;
if let Some(def_id) = cx.qpath_res(qpath, expr.hir_id).opt_def_id();
let ty = cx.tcx.type_of(def_id);
let ty = cx.typeck_results().expr_ty(expr);
if let Some(adt_def) = ty.ty_adt_def();
if adt_def.is_struct();
if let Some(variant) = adt_def.variants.iter().next();

12
clippy_lints/src/indexing_slicing.rs
View file

@ -132,13 +132,13 @@ impl<'tcx> LateLintPass<'tcx> for IndexingSlicing {
}
let help_msg = match (range.start, range.end) {
(None, Some(_)) => "Consider using `.get(..n)`or `.get_mut(..n)` instead",
(Some(_), None) => "Consider using `.get(n..)` or .get_mut(n..)` instead",
(Some(_), Some(_)) => "Consider using `.get(n..m)` or `.get_mut(n..m)` instead",
(None, Some(_)) => "consider using `.get(..n)`or `.get_mut(..n)` instead",
(Some(_), None) => "consider using `.get(n..)` or .get_mut(n..)` instead",
(Some(_), Some(_)) => "consider using `.get(n..m)` or `.get_mut(n..m)` instead",
(None, None) => return, // [..] is ok.
};
span_lint_and_help(cx, INDEXING_SLICING, expr.span, "slicing may panic.", None, help_msg);
span_lint_and_help(cx, INDEXING_SLICING, expr.span, "slicing may panic", None, help_msg);
} else {
// Catchall non-range index, i.e., [n] or [n << m]
if let ty::Array(..) = ty.kind() {
@ -153,9 +153,9 @@ impl<'tcx> LateLintPass<'tcx> for IndexingSlicing {
cx,
INDEXING_SLICING,
expr.span,
"indexing may panic.",
"indexing may panic",
None,
"Consider using `.get(n)` or `.get_mut(n)` instead",
"consider using `.get(n)` or `.get_mut(n)` instead",
);
}
}

6
clippy_lints/src/infinite_iter.rs
View file

@ -89,11 +89,7 @@ impl Finiteness {
impl From<bool> for Finiteness {
#[must_use]
fn from(b: bool) -> Self {
if b {
Infinite
} else {
Finite
}
if b { Infinite } else { Finite }
}
}

2
clippy_lints/src/inherent_to_string.rs
View file

@ -139,7 +139,7 @@ fn show_lint(cx: &LateContext<'_>, item: &ImplItem<'_>) {
self_type.to_string()
),
None,
&format!("remove the inherent method from type `{}`", self_type.to_string())
&format!("remove the inherent method from type `{}`", self_type.to_string()),
);
} else {
span_lint_and_help(

2
clippy_lints/src/integer_division.rs
View file

@ -39,7 +39,7 @@ impl<'tcx> LateLintPass<'tcx> for IntegerDivision {
expr.span,
"integer division",
None,
"division of integers may cause loss of precision. consider using floats.",
"division of integers may cause loss of precision. consider using floats",
);
}
}

162
clippy_lints/src/len_zero.rs
View file

@ -1,11 +1,17 @@
use crate::utils::{get_item_name, snippet_with_applicability, span_lint, span_lint_and_sugg};
use crate::utils::{
get_item_name, get_parent_as_impl, is_allowed, snippet_with_applicability, span_lint, span_lint_and_sugg,
span_lint_and_then,
};
use if_chain::if_chain;
use rustc_ast::ast::LitKind;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::Applicability;
use rustc_hir::def_id::DefId;
use rustc_hir::{AssocItemKind, BinOpKind, Expr, ExprKind, Impl, ImplItemRef, Item, ItemKind, TraitItemRef};
use rustc_hir::{
def_id::DefId, AssocItemKind, BinOpKind, Expr, ExprKind, FnRetTy, ImplItem, ImplItemKind, ImplicitSelfKind, Item,
ItemKind, Mutability, Node, TraitItemRef, TyKind,
};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_middle::ty::{self, AssocKind, FnSig};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::{Span, Spanned, Symbol};
@ -113,14 +119,38 @@ impl<'tcx> LateLintPass<'tcx> for LenZero {
return;
}
match item.kind {
ItemKind::Trait(_, _, _, _, ref trait_items) => check_trait_items(cx, item, trait_items),
ItemKind::Impl(Impl {
of_trait: None,
items: ref impl_items,
..
}) => check_impl_items(cx, item, impl_items),
_ => (),
if let ItemKind::Trait(_, _, _, _, ref trait_items) = item.kind {
check_trait_items(cx, item, trait_items);
}
}
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) {
if_chain! {
if item.ident.as_str() == "len";
if let ImplItemKind::Fn(sig, _) = &item.kind;
if sig.decl.implicit_self.has_implicit_self();
if cx.access_levels.is_exported(item.hir_id());
if matches!(sig.decl.output, FnRetTy::Return(_));
if let Some(imp) = get_parent_as_impl(cx.tcx, item.hir_id());
if imp.of_trait.is_none();
if let TyKind::Path(ty_path) = &imp.self_ty.kind;
if let Some(ty_id) = cx.qpath_res(ty_path, imp.self_ty.hir_id).opt_def_id();
if let Some(local_id) = ty_id.as_local();
let ty_hir_id = cx.tcx.hir().local_def_id_to_hir_id(local_id);
if !is_allowed(cx, LEN_WITHOUT_IS_EMPTY, ty_hir_id);
then {
let (name, kind) = match cx.tcx.hir().find(ty_hir_id) {
Some(Node::ForeignItem(x)) => (x.ident.name, "extern type"),
Some(Node::Item(x)) => match x.kind {
ItemKind::Struct(..) => (x.ident.name, "struct"),
ItemKind::Enum(..) => (x.ident.name, "enum"),
ItemKind::Union(..) => (x.ident.name, "union"),
_ => (x.ident.name, "type"),
}
_ => return,
};
check_for_is_empty(cx, sig.span, sig.decl.implicit_self, ty_id, name, kind)
}
}
}
@ -202,40 +232,94 @@ fn check_trait_items(cx: &LateContext<'_>, visited_trait: &Item<'_>, trait_items
}
}
fn check_impl_items(cx: &LateContext<'_>, item: &Item<'_>, impl_items: &[ImplItemRef<'_>]) {
fn is_named_self(cx: &LateContext<'_>, item: &ImplItemRef<'_>, name: &str) -> bool {
item.ident.name.as_str() == name
&& if let AssocItemKind::Fn { has_self } = item.kind {
has_self && cx.tcx.fn_sig(item.id.def_id).inputs().skip_binder().len() == 1
} else {
false
/// Checks if the given signature matches the expectations for `is_empty`
fn check_is_empty_sig(cx: &LateContext<'_>, sig: FnSig<'_>, self_kind: ImplicitSelfKind) -> bool {
match &**sig.inputs_and_output {
[arg, res] if *res == cx.tcx.types.bool => {
matches!(
(arg.kind(), self_kind),
(ty::Ref(_, _, Mutability::Not), ImplicitSelfKind::ImmRef)
| (ty::Ref(_, _, Mutability::Mut), ImplicitSelfKind::MutRef)
) || (!arg.is_ref() && matches!(self_kind, ImplicitSelfKind::Imm | ImplicitSelfKind::Mut))
},
_ => false,
}
}
let is_empty = if let Some(is_empty) = impl_items.iter().find(|i| is_named_self(cx, i, "is_empty")) {
if cx.access_levels.is_exported(is_empty.id.hir_id()) {
return;
}
"a private"
} else {
"no corresponding"
/// Checks if the given type has an `is_empty` method with the appropriate signature.
fn check_for_is_empty(
cx: &LateContext<'_>,
span: Span,
self_kind: ImplicitSelfKind,
impl_ty: DefId,
item_name: Symbol,
item_kind: &str,
) {
let is_empty = Symbol::intern("is_empty");
let is_empty = cx
.tcx
.inherent_impls(impl_ty)
.iter()
.flat_map(|&id| cx.tcx.associated_items(id).filter_by_name_unhygienic(is_empty))
.find(|item| item.kind == AssocKind::Fn);
let (msg, is_empty_span, self_kind) = match is_empty {
None => (
format!(
"{} `{}` has a public `len` method, but no `is_empty` method",
item_kind,
item_name.as_str(),
),
None,
None,
),
Some(is_empty)
if !cx
.access_levels
.is_exported(cx.tcx.hir().local_def_id_to_hir_id(is_empty.def_id.expect_local())) =>
{
(
format!(
"{} `{}` has a public `len` method, but a private `is_empty` method",
item_kind,
item_name.as_str(),
),
Some(cx.tcx.def_span(is_empty.def_id)),
None,
)
},
Some(is_empty)
if !(is_empty.fn_has_self_parameter
&& check_is_empty_sig(cx, cx.tcx.fn_sig(is_empty.def_id).skip_binder(), self_kind)) =>
{
(
format!(
"{} `{}` has a public `len` method, but the `is_empty` method has an unexpected signature",
item_kind,
item_name.as_str(),
),
Some(cx.tcx.def_span(is_empty.def_id)),
Some(self_kind),
)
},
Some(_) => return,
};
if let Some(i) = impl_items.iter().find(|i| is_named_self(cx, i, "len")) {
if cx.access_levels.is_exported(i.id.hir_id()) {
let ty = cx.tcx.type_of(item.def_id);
span_lint(
cx,
LEN_WITHOUT_IS_EMPTY,
item.span,
&format!(
"item `{}` has a public `len` method but {} `is_empty` method",
ty, is_empty
),
);
span_lint_and_then(cx, LEN_WITHOUT_IS_EMPTY, span, &msg, |db| {
if let Some(span) = is_empty_span {
db.span_note(span, "`is_empty` defined here");
}
if let Some(self_kind) = self_kind {
db.note(&format!(
"expected signature: `({}self) -> bool`",
match self_kind {
ImplicitSelfKind::ImmRef => "&",
ImplicitSelfKind::MutRef => "&mut ",
_ => "",
}
));
}
});
}
fn check_cmp(cx: &LateContext<'_>, span: Span, method: &Expr<'_>, lit: &Expr<'_>, op: &str, compare_to: u32) {

72
clippy_lints/src/lib.rs
View file

@ -182,6 +182,7 @@ mod booleans;
mod bytecount;
mod cargo_common_metadata;
mod case_sensitive_file_extension_comparisons;
mod casts;
mod checked_conversions;
mod cognitive_complexity;
mod collapsible_if;
@ -586,6 +587,18 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&bytecount::NAIVE_BYTECOUNT,
&cargo_common_metadata::CARGO_COMMON_METADATA,
&case_sensitive_file_extension_comparisons::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS,
&casts::CAST_LOSSLESS,
&casts::CAST_POSSIBLE_TRUNCATION,
&casts::CAST_POSSIBLE_WRAP,
&casts::CAST_PRECISION_LOSS,
&casts::CAST_PTR_ALIGNMENT,
&casts::CAST_REF_TO_MUT,
&casts::CAST_SIGN_LOSS,
&casts::CHAR_LIT_AS_U8,
&casts::FN_TO_NUMERIC_CAST,
&casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION,
&casts::PTR_AS_PTR,
&casts::UNNECESSARY_CAST,
&checked_conversions::CHECKED_CONVERSIONS,
&cognitive_complexity::COGNITIVE_COMPLEXITY,
&collapsible_if::COLLAPSIBLE_ELSE_IF,
@ -769,11 +782,13 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&methods::FLAT_MAP_IDENTITY,
&methods::FROM_ITER_INSTEAD_OF_COLLECT,
&methods::GET_UNWRAP,
&methods::IMPLICIT_CLONE,
&methods::INEFFICIENT_TO_STRING,
&methods::INSPECT_FOR_EACH,
&methods::INTO_ITER_ON_REF,
&methods::ITERATOR_STEP_BY_ZERO,
&methods::ITER_CLONED_COLLECT,
&methods::ITER_COUNT,
&methods::ITER_NEXT_SLICE,
&methods::ITER_NTH,
&methods::ITER_NTH_ZERO,
@ -941,28 +956,16 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&types::ABSURD_EXTREME_COMPARISONS,
&types::BORROWED_BOX,
&types::BOX_VEC,
&types::CAST_LOSSLESS,
&types::CAST_POSSIBLE_TRUNCATION,
&types::CAST_POSSIBLE_WRAP,
&types::CAST_PRECISION_LOSS,
&types::CAST_PTR_ALIGNMENT,
&types::CAST_REF_TO_MUT,
&types::CAST_SIGN_LOSS,
&types::CHAR_LIT_AS_U8,
&types::FN_TO_NUMERIC_CAST,
&types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION,
&types::IMPLICIT_HASHER,
&types::INVALID_UPCAST_COMPARISONS,
&types::LET_UNIT_VALUE,
&types::LINKEDLIST,
&types::OPTION_OPTION,
&types::PTR_AS_PTR,
&types::RC_BUFFER,
&types::REDUNDANT_ALLOCATION,
&types::TYPE_COMPLEXITY,
&types::UNIT_ARG,
&types::UNIT_CMP,
&types::UNNECESSARY_CAST,
&types::VEC_BOX,
&undropped_manually_drops::UNDROPPED_MANUALLY_DROPS,
&unicode::INVISIBLE_CHARACTERS,
@ -1073,6 +1076,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(move || box use_self::UseSelf::new(msrv));
store.register_late_pass(move || box missing_const_for_fn::MissingConstForFn::new(msrv));
store.register_late_pass(move || box needless_question_mark::NeedlessQuestionMark::new(msrv));
store.register_late_pass(move || box casts::Casts::new(msrv));
store.register_late_pass(|| box size_of_in_element_count::SizeOfInElementCount);
store.register_late_pass(|| box map_clone::MapClone);
@ -1084,7 +1088,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box main_recursion::MainRecursion::default());
store.register_late_pass(|| box lifetimes::Lifetimes);
store.register_late_pass(|| box entry::HashMapPass);
store.register_late_pass(|| box types::Casts);
let type_complexity_threshold = conf.type_complexity_threshold;
store.register_late_pass(move || box types::TypeComplexity::new(type_complexity_threshold));
store.register_late_pass(|| box minmax::MinMaxPass);
@ -1105,7 +1108,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box panic_unimplemented::PanicUnimplemented);
store.register_late_pass(|| box strings::StringLitAsBytes);
store.register_late_pass(|| box derive::Derive);
store.register_late_pass(|| box types::CharLitAsU8);
store.register_late_pass(|| box get_last_with_len::GetLastWithLen);
store.register_late_pass(|| box drop_forget_ref::DropForgetRef);
store.register_late_pass(|| box empty_enum::EmptyEnum);
@ -1173,7 +1175,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box slow_vector_initialization::SlowVectorInit);
store.register_late_pass(|| box unnecessary_sort_by::UnnecessarySortBy);
store.register_late_pass(|| box unnecessary_wraps::UnnecessaryWraps);
store.register_late_pass(|| box types::RefToMut);
store.register_late_pass(|| box assertions_on_constants::AssertionsOnConstants);
store.register_late_pass(|| box transmuting_null::TransmutingNull);
store.register_late_pass(|| box path_buf_push_overwrite::PathBufPushOverwrite);
@ -1275,7 +1276,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box strings::StringToString);
store.register_late_pass(|| box zero_sized_map_values::ZeroSizedMapValues);
store.register_late_pass(|| box vec_init_then_push::VecInitThenPush::default());
store.register_late_pass(move || box types::PtrAsPtr::new(msrv));
store.register_late_pass(|| box case_sensitive_file_extension_comparisons::CaseSensitiveFileExtensionComparisons);
store.register_late_pass(|| box redundant_slicing::RedundantSlicing);
store.register_late_pass(|| box from_str_radix_10::FromStrRadix10);
@ -1341,7 +1341,15 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&await_holding_invalid::AWAIT_HOLDING_LOCK),
LintId::of(&await_holding_invalid::AWAIT_HOLDING_REFCELL_REF),
LintId::of(&bit_mask::VERBOSE_BIT_MASK),
LintId::of(&bytecount::NAIVE_BYTECOUNT),
LintId::of(&case_sensitive_file_extension_comparisons::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS),
LintId::of(&casts::CAST_LOSSLESS),
LintId::of(&casts::CAST_POSSIBLE_TRUNCATION),
LintId::of(&casts::CAST_POSSIBLE_WRAP),
LintId::of(&casts::CAST_PRECISION_LOSS),
LintId::of(&casts::CAST_PTR_ALIGNMENT),
LintId::of(&casts::CAST_SIGN_LOSS),
LintId::of(&casts::PTR_AS_PTR),
LintId::of(&checked_conversions::CHECKED_CONVERSIONS),
LintId::of(&copies::SAME_FUNCTIONS_IN_IF_CONDITION),
LintId::of(&copy_iterator::COPY_ITERATOR),
@ -1380,6 +1388,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&matches::SINGLE_MATCH_ELSE),
LintId::of(&methods::FILTER_MAP),
LintId::of(&methods::FILTER_MAP_NEXT),
LintId::of(&methods::IMPLICIT_CLONE),
LintId::of(&methods::INEFFICIENT_TO_STRING),
LintId::of(&methods::MAP_FLATTEN),
LintId::of(&methods::MAP_UNWRAP_OR),
@ -1400,18 +1409,11 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&strings::STRING_ADD_ASSIGN),
LintId::of(&trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS),
LintId::of(&trait_bounds::TYPE_REPETITION_IN_BOUNDS),
LintId::of(&types::CAST_LOSSLESS),
LintId::of(&types::CAST_POSSIBLE_TRUNCATION),
LintId::of(&types::CAST_POSSIBLE_WRAP),
LintId::of(&types::CAST_PRECISION_LOSS),
LintId::of(&types::CAST_PTR_ALIGNMENT),
LintId::of(&types::CAST_SIGN_LOSS),
LintId::of(&types::IMPLICIT_HASHER),
LintId::of(&types::INVALID_UPCAST_COMPARISONS),
LintId::of(&types::LET_UNIT_VALUE),
LintId::of(&types::LINKEDLIST),
LintId::of(&types::OPTION_OPTION),
LintId::of(&types::PTR_AS_PTR),
LintId::of(&unicode::NON_ASCII_LITERAL),
LintId::of(&unicode::UNICODE_NOT_NFC),
LintId::of(&unnecessary_wraps::UNNECESSARY_WRAPS),
@ -1455,7 +1457,11 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS),
LintId::of(&booleans::LOGIC_BUG),
LintId::of(&booleans::NONMINIMAL_BOOL),
LintId::of(&bytecount::NAIVE_BYTECOUNT),
LintId::of(&casts::CAST_REF_TO_MUT),
LintId::of(&casts::CHAR_LIT_AS_U8),
LintId::of(&casts::FN_TO_NUMERIC_CAST),
LintId::of(&casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION),
LintId::of(&casts::UNNECESSARY_CAST),
LintId::of(&collapsible_if::COLLAPSIBLE_ELSE_IF),
LintId::of(&collapsible_if::COLLAPSIBLE_IF),
LintId::of(&collapsible_match::COLLAPSIBLE_MATCH),
@ -1576,6 +1582,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&methods::INTO_ITER_ON_REF),
LintId::of(&methods::ITERATOR_STEP_BY_ZERO),
LintId::of(&methods::ITER_CLONED_COLLECT),
LintId::of(&methods::ITER_COUNT),
LintId::of(&methods::ITER_NEXT_SLICE),
LintId::of(&methods::ITER_NTH),
LintId::of(&methods::ITER_NTH_ZERO),
@ -1695,15 +1702,10 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&types::ABSURD_EXTREME_COMPARISONS),
LintId::of(&types::BORROWED_BOX),
LintId::of(&types::BOX_VEC),
LintId::of(&types::CAST_REF_TO_MUT),
LintId::of(&types::CHAR_LIT_AS_U8),
LintId::of(&types::FN_TO_NUMERIC_CAST),
LintId::of(&types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION),
LintId::of(&types::REDUNDANT_ALLOCATION),
LintId::of(&types::TYPE_COMPLEXITY),
LintId::of(&types::UNIT_ARG),
LintId::of(&types::UNIT_CMP),
LintId::of(&types::UNNECESSARY_CAST),
LintId::of(&types::VEC_BOX),
LintId::of(&undropped_manually_drops::UNDROPPED_MANUALLY_DROPS),
LintId::of(&unicode::INVISIBLE_CHARACTERS),
@ -1736,6 +1738,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&attrs::BLANKET_CLIPPY_RESTRICTION_LINTS),
LintId::of(&blacklisted_name::BLACKLISTED_NAME),
LintId::of(&blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS),
LintId::of(&casts::FN_TO_NUMERIC_CAST),
LintId::of(&casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION),
LintId::of(&collapsible_if::COLLAPSIBLE_ELSE_IF),
LintId::of(&collapsible_if::COLLAPSIBLE_IF),
LintId::of(&collapsible_match::COLLAPSIBLE_MATCH),
@ -1832,8 +1836,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&tabs_in_doc_comments::TABS_IN_DOC_COMMENTS),
LintId::of(&to_digit_is_some::TO_DIGIT_IS_SOME),
LintId::of(&try_err::TRY_ERR),
LintId::of(&types::FN_TO_NUMERIC_CAST),
LintId::of(&types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION),
LintId::of(&unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME),
LintId::of(&unused_unit::UNUSED_UNIT),
LintId::of(&upper_case_acronyms::UPPER_CASE_ACRONYMS),
@ -1849,6 +1851,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&assign_ops::MISREFACTORED_ASSIGN_OP),
LintId::of(&attrs::DEPRECATED_CFG_ATTR),
LintId::of(&booleans::NONMINIMAL_BOOL),
LintId::of(&casts::CHAR_LIT_AS_U8),
LintId::of(&casts::UNNECESSARY_CAST),
LintId::of(&double_comparison::DOUBLE_COMPARISONS),
LintId::of(&double_parens::DOUBLE_PARENS),
LintId::of(&duration_subsec::DURATION_SUBSEC),
@ -1881,6 +1885,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&methods::FILTER_NEXT),
LintId::of(&methods::FLAT_MAP_IDENTITY),
LintId::of(&methods::INSPECT_FOR_EACH),
LintId::of(&methods::ITER_COUNT),
LintId::of(&methods::MANUAL_FILTER_MAP),
LintId::of(&methods::MANUAL_FIND_MAP),
LintId::of(&methods::OPTION_AS_REF_DEREF),
@ -1924,10 +1929,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&transmute::TRANSMUTE_PTR_TO_PTR),
LintId::of(&transmute::TRANSMUTE_PTR_TO_REF),
LintId::of(&types::BORROWED_BOX),
LintId::of(&types::CHAR_LIT_AS_U8),
LintId::of(&types::TYPE_COMPLEXITY),
LintId::of(&types::UNIT_ARG),
LintId::of(&types::UNNECESSARY_CAST),
LintId::of(&types::VEC_BOX),
LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY),
LintId::of(&unwrap::UNNECESSARY_UNWRAP),
@ -1945,6 +1948,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&bit_mask::BAD_BIT_MASK),
LintId::of(&bit_mask::INEFFECTIVE_BIT_MASK),
LintId::of(&booleans::LOGIC_BUG),
LintId::of(&casts::CAST_REF_TO_MUT),
LintId::of(&copies::IFS_SAME_COND),
LintId::of(&copies::IF_SAME_THEN_ELSE),
LintId::of(&derive::DERIVE_HASH_XOR_EQ),
@ -1997,7 +2001,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&transmute::WRONG_TRANSMUTE),
LintId::of(&transmuting_null::TRANSMUTING_NULL),
LintId::of(&types::ABSURD_EXTREME_COMPARISONS),
LintId::of(&types::CAST_REF_TO_MUT),
LintId::of(&types::UNIT_CMP),
LintId::of(&undropped_manually_drops::UNDROPPED_MANUALLY_DROPS),
LintId::of(&unicode::INVISIBLE_CHARACTERS),
@ -2010,7 +2013,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
]);
store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![
LintId::of(&bytecount::NAIVE_BYTECOUNT),
LintId::of(&entry::MAP_ENTRY),
LintId::of(&escape::BOXED_LOCAL),
LintId::of(&large_const_arrays::LARGE_CONST_ARRAYS),

20
clippy_lints/src/lifetimes.rs
View file

@ -1,5 +1,4 @@
use crate::utils::paths;
use crate::utils::{get_trait_def_id, in_macro, span_lint, trait_ref_of_method};
use crate::utils::{in_macro, span_lint, trait_ref_of_method};
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_hir::intravisit::{
walk_fn_decl, walk_generic_param, walk_generics, walk_item, walk_param_bound, walk_poly_trait_ref, walk_ty,
@ -8,8 +7,8 @@ use rustc_hir::intravisit::{
use rustc_hir::FnRetTy::Return;
use rustc_hir::{
BareFnTy, BodyId, FnDecl, GenericArg, GenericBound, GenericParam, GenericParamKind, Generics, ImplItem,
ImplItemKind, Item, ItemKind, Lifetime, LifetimeName, ParamName, PolyTraitRef, TraitBoundModifier, TraitFn,
TraitItem, TraitItemKind, Ty, TyKind, WhereClause, WherePredicate,
ImplItemKind, Item, ItemKind, LangItem, Lifetime, LifetimeName, ParamName, PolyTraitRef, TraitBoundModifier,
TraitFn, TraitItem, TraitItemKind, Ty, TyKind, WhereClause, WherePredicate,
};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::map::Map;
@ -300,7 +299,7 @@ fn unique_lifetimes(lts: &[RefLt]) -> usize {
lts.iter().collect::<FxHashSet<_>>().len()
}
const CLOSURE_TRAIT_BOUNDS: [&[&str]; 3] = [&paths::FN, &paths::FN_MUT, &paths::FN_ONCE];
const CLOSURE_TRAIT_BOUNDS: [LangItem; 3] = [LangItem::Fn, LangItem::FnMut, LangItem::FnOnce];
/// A visitor usable for `rustc_front::visit::walk_ty()`.
struct RefVisitor<'a, 'tcx> {
@ -359,10 +358,13 @@ impl<'a, 'tcx> Visitor<'tcx> for RefVisitor<'a, 'tcx> {
fn visit_poly_trait_ref(&mut self, poly_tref: &'tcx PolyTraitRef<'tcx>, tbm: TraitBoundModifier) {
let trait_ref = &poly_tref.trait_ref;
if CLOSURE_TRAIT_BOUNDS
.iter()
.any(|trait_path| trait_ref.trait_def_id() == get_trait_def_id(self.cx, trait_path))
{
if CLOSURE_TRAIT_BOUNDS.iter().any(|&item| {
self.cx
.tcx
.lang_items()
.require(item)
.map_or(false, |id| Some(id) == trait_ref.trait_def_id())
}) {
let mut sub_visitor = RefVisitor::new(self.cx);
sub_visitor.visit_trait_ref(trait_ref);
self.nested_elision_site_lts.append(&mut sub_visitor.all_lts());

3182
clippy_lints/src/loops.rs
View file

File diff suppressed because it is too large Load diff

17
clippy_lints/src/loops/empty_loop.rs Normal file
View file

@ -0,0 +1,17 @@
use super::EMPTY_LOOP;
use crate::utils::{is_in_panic_handler, is_no_std_crate, span_lint_and_help};
use rustc_hir::{Block, Expr};
use rustc_lint::LateContext;
pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, loop_block: &'tcx Block<'_>) {
if loop_block.stmts.is_empty() && loop_block.expr.is_none() && !is_in_panic_handler(cx, expr) {
let msg = "empty `loop {}` wastes CPU cycles";
let help = if is_no_std_crate(cx) {
"you should either use `panic!()` or add a call pausing or sleeping the thread to the loop body"
} else {
"you should either use `panic!()` or add `std::thread::sleep(..);` to the loop body"
};
span_lint_and_help(cx, EMPTY_LOOP, expr.span, msg, None, help);
}
}

58
clippy_lints/src/loops/explicit_counter_loop.rs Normal file
View file

@ -0,0 +1,58 @@
use super::{
get_span_of_entire_for_loop, make_iterator_snippet, IncrementVisitor, InitializeVisitor, EXPLICIT_COUNTER_LOOP,
};
use crate::utils::{get_enclosing_block, is_integer_const, snippet_with_applicability, span_lint_and_sugg};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_block, walk_expr};
use rustc_hir::{Expr, Pat};
use rustc_lint::LateContext;
// To trigger the EXPLICIT_COUNTER_LOOP lint, a variable must be
// incremented exactly once in the loop body, and initialized to zero
// at the start of the loop.
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,
arg: &'tcx Expr<'_>,
body: &'tcx Expr<'_>,
expr: &'tcx Expr<'_>,
) {
// Look for variables that are incremented once per loop iteration.
let mut increment_visitor = IncrementVisitor::new(cx);
walk_expr(&mut increment_visitor, body);
// For each candidate, check the parent block to see if
// it's initialized to zero at the start of the loop.
if let Some(block) = get_enclosing_block(&cx, expr.hir_id) {
for id in increment_visitor.into_results() {
let mut initialize_visitor = InitializeVisitor::new(cx, expr, id);
walk_block(&mut initialize_visitor, block);
if_chain! {
if let Some((name, initializer)) = initialize_visitor.get_result();
if is_integer_const(cx, initializer, 0);
then {
let mut applicability = Applicability::MachineApplicable;
let for_span = get_span_of_entire_for_loop(expr);
span_lint_and_sugg(
cx,
EXPLICIT_COUNTER_LOOP,
for_span.with_hi(arg.span.hi()),
&format!("the variable `{}` is used as a loop counter", name),
"consider using",
format!(
"for ({}, {}) in {}.enumerate()",
name,
snippet_with_applicability(cx, pat.span, "item", &mut applicability),
make_iterator_snippet(cx, arg, &mut applicability),
),
applicability,
);
}
}
}
}
}

27
clippy_lints/src/loops/explicit_into_iter_loop.rs Normal file
View file

@ -0,0 +1,27 @@
use super::EXPLICIT_INTO_ITER_LOOP;
use crate::utils::{snippet_with_applicability, span_lint_and_sugg};
use rustc_errors::Applicability;
use rustc_hir::Expr;
use rustc_lint::LateContext;
use rustc_middle::ty::TyS;
pub(super) fn check(cx: &LateContext<'_>, args: &'hir [Expr<'hir>], arg: &Expr<'_>) {
let receiver_ty = cx.typeck_results().expr_ty(&args[0]);
let receiver_ty_adjusted = cx.typeck_results().expr_ty_adjusted(&args[0]);
if !TyS::same_type(receiver_ty, receiver_ty_adjusted) {
return;
}
let mut applicability = Applicability::MachineApplicable;
let object = snippet_with_applicability(cx, args[0].span, "_", &mut applicability);
span_lint_and_sugg(
cx,
EXPLICIT_INTO_ITER_LOOP,
arg.span,
"it is more concise to loop over containers instead of using explicit \
iteration methods",
"to write this more concisely, try",
object.to_string(),
applicability,
);
}

74
clippy_lints/src/loops/explicit_iter_loop.rs Normal file
View file

@ -0,0 +1,74 @@
use super::EXPLICIT_ITER_LOOP;
use crate::utils::{match_trait_method, snippet_with_applicability, span_lint_and_sugg};
use rustc_errors::Applicability;
use rustc_hir::{Expr, Mutability};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty, TyS};
use rustc_span::sym;
use crate::utils::{is_type_diagnostic_item, match_type, paths};
pub(super) fn check(cx: &LateContext<'_>, args: &[Expr<'_>], arg: &Expr<'_>, method_name: &str) {
let should_lint = match method_name {
"iter" | "iter_mut" => is_ref_iterable_type(cx, &args[0]),
"into_iter" if match_trait_method(cx, arg, &paths::INTO_ITERATOR) => {
let receiver_ty = cx.typeck_results().expr_ty(&args[0]);
let receiver_ty_adjusted = cx.typeck_results().expr_ty_adjusted(&args[0]);
let ref_receiver_ty = cx.tcx.mk_ref(
cx.tcx.lifetimes.re_erased,
ty::TypeAndMut {
ty: receiver_ty,
mutbl: Mutability::Not,
},
);
TyS::same_type(receiver_ty_adjusted, ref_receiver_ty)
},
_ => false,
};
if !should_lint {
return;
}
let mut applicability = Applicability::MachineApplicable;
let object = snippet_with_applicability(cx, args[0].span, "_", &mut applicability);
let muta = if method_name == "iter_mut" { "mut " } else { "" };
span_lint_and_sugg(
cx,
EXPLICIT_ITER_LOOP,
arg.span,
"it is more concise to loop over references to containers instead of using explicit \
iteration methods",
"to write this more concisely, try",
format!("&{}{}", muta, object),
applicability,
)
}
/// Returns `true` if the type of expr is one that provides `IntoIterator` impls
/// for `&T` and `&mut T`, such as `Vec`.
#[rustfmt::skip]
fn is_ref_iterable_type(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
// no walk_ptrs_ty: calling iter() on a reference can make sense because it
// will allow further borrows afterwards
let ty = cx.typeck_results().expr_ty(e);
is_iterable_array(ty, cx) ||
is_type_diagnostic_item(cx, ty, sym::vec_type) ||
match_type(cx, ty, &paths::LINKED_LIST) ||
is_type_diagnostic_item(cx, ty, sym::hashmap_type) ||
is_type_diagnostic_item(cx, ty, sym::hashset_type) ||
is_type_diagnostic_item(cx, ty, sym::vecdeque_type) ||
match_type(cx, ty, &paths::BINARY_HEAP) ||
match_type(cx, ty, &paths::BTREEMAP) ||
match_type(cx, ty, &paths::BTREESET)
}
fn is_iterable_array<'tcx>(ty: Ty<'tcx>, cx: &LateContext<'tcx>) -> bool {
// IntoIterator is currently only implemented for array sizes <= 32 in rustc
match ty.kind() {
ty::Array(_, n) => n
.try_eval_usize(cx.tcx, cx.param_env)
.map_or(false, |val| (0..=32).contains(&val)),
_ => false,
}
}

71
clippy_lints/src/loops/for_kv_map.rs Normal file
View file

@ -0,0 +1,71 @@
use super::FOR_KV_MAP;
use crate::utils::visitors::LocalUsedVisitor;
use crate::utils::{is_type_diagnostic_item, match_type, multispan_sugg, paths, snippet, span_lint_and_then, sugg};
use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, Pat, PatKind};
use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_span::sym;
/// Checks for the `FOR_KV_MAP` lint.
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,
arg: &'tcx Expr<'_>,
body: &'tcx Expr<'_>,
expr: &'tcx Expr<'_>,
) {
let pat_span = pat.span;
if let PatKind::Tuple(ref pat, _) = pat.kind {
if pat.len() == 2 {
let arg_span = arg.span;
let (new_pat_span, kind, ty, mutbl) = match *cx.typeck_results().expr_ty(arg).kind() {
ty::Ref(_, ty, mutbl) => match (&pat[0].kind, &pat[1].kind) {
(key, _) if pat_is_wild(cx, key, body) => (pat[1].span, "value", ty, mutbl),
(_, value) if pat_is_wild(cx, value, body) => (pat[0].span, "key", ty, Mutability::Not),
_ => return,
},
_ => return,
};
let mutbl = match mutbl {
Mutability::Not => "",
Mutability::Mut => "_mut",
};
let arg = match arg.kind {
ExprKind::AddrOf(BorrowKind::Ref, _, ref expr) => &**expr,
_ => arg,
};
if is_type_diagnostic_item(cx, ty, sym::hashmap_type) || match_type(cx, ty, &paths::BTREEMAP) {
span_lint_and_then(
cx,
FOR_KV_MAP,
expr.span,
&format!("you seem to want to iterate on a map's {}s", kind),
|diag| {
let map = sugg::Sugg::hir(cx, arg, "map");
multispan_sugg(
diag,
"use the corresponding method",
vec![
(pat_span, snippet(cx, new_pat_span, kind).into_owned()),
(arg_span, format!("{}.{}s{}()", map.maybe_par(), kind, mutbl)),
],
);
},
);
}
}
}
}
/// Returns `true` if the pattern is a `PatWild` or an ident prefixed with `_`.
fn pat_is_wild<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx PatKind<'_>, body: &'tcx Expr<'_>) -> bool {
match *pat {
PatKind::Wild => true,
PatKind::Binding(_, id, ident, None) if ident.as_str().starts_with('_') => {
!LocalUsedVisitor::new(cx, id).check_expr(body)
},
_ => false,
}
}

45
clippy_lints/src/loops/for_loops_over_fallibles.rs Normal file
View file

@ -0,0 +1,45 @@
use super::FOR_LOOPS_OVER_FALLIBLES;
use crate::utils::{is_type_diagnostic_item, snippet, span_lint_and_help};
use rustc_hir::{Expr, Pat};
use rustc_lint::LateContext;
use rustc_span::symbol::sym;
/// Checks for `for` loops over `Option`s and `Result`s.
pub(super) fn check(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>) {
let ty = cx.typeck_results().expr_ty(arg);
if is_type_diagnostic_item(cx, ty, sym::option_type) {
span_lint_and_help(
cx,
FOR_LOOPS_OVER_FALLIBLES,
arg.span,
&format!(
"for loop over `{0}`, which is an `Option`. This is more readably written as an \
`if let` statement",
snippet(cx, arg.span, "_")
),
None,
&format!(
"consider replacing `for {0} in {1}` with `if let Some({0}) = {1}`",
snippet(cx, pat.span, "_"),
snippet(cx, arg.span, "_")
),
);
} else if is_type_diagnostic_item(cx, ty, sym::result_type) {
span_lint_and_help(
cx,
FOR_LOOPS_OVER_FALLIBLES,
arg.span,
&format!(
"for loop over `{0}`, which is a `Result`. This is more readably written as an \
`if let` statement",
snippet(cx, arg.span, "_")
),
None,
&format!(
"consider replacing `for {0} in {1}` with `if let Ok({0}) = {1}`",
snippet(cx, pat.span, "_"),
snippet(cx, arg.span, "_")
),
);
}
}

19
clippy_lints/src/loops/iter_next_loop.rs Normal file
View file

@ -0,0 +1,19 @@
use super::ITER_NEXT_LOOP;
use crate::utils::{match_trait_method, paths, span_lint};
use rustc_hir::Expr;
use rustc_lint::LateContext;
pub(super) fn check(cx: &LateContext<'_>, arg: &Expr<'_>, expr: &Expr<'_>) -> bool {
if match_trait_method(cx, arg, &paths::ITERATOR) {
span_lint(
cx,
ITER_NEXT_LOOP,
expr.span,
"you are iterating over `Iterator::next()` which is an Option; this will compile but is \
probably not what you want",
);
true
} else {
false
}
}

79
clippy_lints/src/loops/manual_flatten.rs Normal file
View file

@ -0,0 +1,79 @@
use super::utils::make_iterator_snippet;
use super::MANUAL_FLATTEN;
use crate::utils::{is_ok_ctor, is_some_ctor, path_to_local_id, span_lint_and_then};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, MatchSource, Pat, PatKind, QPath, StmtKind};
use rustc_lint::LateContext;
use rustc_span::source_map::Span;
/// Check for unnecessary `if let` usage in a for loop where only the `Some` or `Ok` variant of the
/// iterator element is used.
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,
arg: &'tcx Expr<'_>,
body: &'tcx Expr<'_>,
span: Span,
) {
if let ExprKind::Block(ref block, _) = body.kind {
// Ensure the `if let` statement is the only expression or statement in the for-loop
let inner_expr = if block.stmts.len() == 1 && block.expr.is_none() {
let match_stmt = &block.stmts[0];
if let StmtKind::Semi(inner_expr) = match_stmt.kind {
Some(inner_expr)
} else {
None
}
} else if block.stmts.is_empty() {
block.expr
} else {
None
};
if_chain! {
if let Some(inner_expr) = inner_expr;
if let ExprKind::Match(
ref match_expr, ref match_arms, MatchSource::IfLetDesugar{ contains_else_clause: false }
) = inner_expr.kind;
// Ensure match_expr in `if let` statement is the same as the pat from the for-loop
if let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind;
if path_to_local_id(match_expr, pat_hir_id);
// Ensure the `if let` statement is for the `Some` variant of `Option` or the `Ok` variant of `Result`
if let PatKind::TupleStruct(QPath::Resolved(None, path), _, _) = match_arms[0].pat.kind;
let some_ctor = is_some_ctor(cx, path.res);
let ok_ctor = is_ok_ctor(cx, path.res);
if some_ctor || ok_ctor;
let if_let_type = if some_ctor { "Some" } else { "Ok" };
then {
// Prepare the error message
let msg = format!("unnecessary `if let` since only the `{}` variant of the iterator element is used", if_let_type);
// Prepare the help message
let mut applicability = Applicability::MaybeIncorrect;
let arg_snippet = make_iterator_snippet(cx, arg, &mut applicability);
span_lint_and_then(
cx,
MANUAL_FLATTEN,
span,
&msg,
|diag| {
let sugg = format!("{}.flatten()", arg_snippet);
diag.span_suggestion(
arg.span,
"try",
sugg,
Applicability::MaybeIncorrect,
);
diag.span_help(
inner_expr.span,
"...and remove the `if let` statement in the for loop",
);
}
);
}
}
}
}

451
clippy_lints/src/loops/manual_memcpy.rs Normal file
View file

@ -0,0 +1,451 @@
use super::{get_span_of_entire_for_loop, IncrementVisitor, InitializeVisitor, MANUAL_MEMCPY};
use crate::utils::sugg::Sugg;
use crate::utils::{
get_enclosing_block, higher, is_type_diagnostic_item, path_to_local, snippet, span_lint_and_sugg, sugg,
};
use if_chain::if_chain;
use rustc_ast::ast;
use rustc_errors::Applicability;
use rustc_hir::intravisit::walk_block;
use rustc_hir::{BinOpKind, Block, Expr, ExprKind, HirId, Pat, PatKind, StmtKind};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty};
use rustc_span::symbol::sym;
use std::iter::Iterator;
/// Checks for for loops that sequentially copy items from one slice-like
/// object to another.
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,
arg: &'tcx Expr<'_>,
body: &'tcx Expr<'_>,
expr: &'tcx Expr<'_>,
) -> bool {
if let Some(higher::Range {
start: Some(start),
end: Some(end),
limits,
}) = higher::range(arg)
{
// the var must be a single name
if let PatKind::Binding(_, canonical_id, _, _) = pat.kind {
let mut starts = vec![Start {
id: canonical_id,
kind: StartKind::Range,
}];
// This is one of few ways to return different iterators
// derived from: https://stackoverflow.com/questions/29760668/conditionally-iterate-over-one-of-several-possible-iterators/52064434#52064434
let mut iter_a = None;
let mut iter_b = None;
if let ExprKind::Block(block, _) = body.kind {
if let Some(loop_counters) = get_loop_counters(cx, block, expr) {
starts.extend(loop_counters);
}
iter_a = Some(get_assignments(block, &starts));
} else {
iter_b = Some(get_assignment(body));
}
let assignments = iter_a.into_iter().flatten().chain(iter_b.into_iter());
let big_sugg = assignments
// The only statements in the for loops can be indexed assignments from
// indexed retrievals (except increments of loop counters).
.map(|o| {
o.and_then(|(lhs, rhs)| {
let rhs = fetch_cloned_expr(rhs);
if_chain! {
if let ExprKind::Index(base_left, idx_left) = lhs.kind;
if let ExprKind::Index(base_right, idx_right) = rhs.kind;
if is_slice_like(cx, cx.typeck_results().expr_ty(base_left))
&& is_slice_like(cx, cx.typeck_results().expr_ty(base_right));
if let Some((start_left, offset_left)) = get_details_from_idx(cx, &idx_left, &starts);
if let Some((start_right, offset_right)) = get_details_from_idx(cx, &idx_right, &starts);
// Source and destination must be different
if path_to_local(base_left) != path_to_local(base_right);
then {
Some((IndexExpr { base: base_left, idx: start_left, idx_offset: offset_left },
IndexExpr { base: base_right, idx: start_right, idx_offset: offset_right }))
} else {
None
}
}
})
})
.map(|o| o.map(|(dst, src)| build_manual_memcpy_suggestion(cx, start, end, limits, &dst, &src)))
.collect::<Option<Vec<_>>>()
.filter(|v| !v.is_empty())
.map(|v| v.join("\n "));
if let Some(big_sugg) = big_sugg {
span_lint_and_sugg(
cx,
MANUAL_MEMCPY,
get_span_of_entire_for_loop(expr),
"it looks like you're manually copying between slices",
"try replacing the loop by",
big_sugg,
Applicability::Unspecified,
);
return true;
}
}
}
false
}
fn build_manual_memcpy_suggestion<'tcx>(
cx: &LateContext<'tcx>,
start: &Expr<'_>,
end: &Expr<'_>,
limits: ast::RangeLimits,
dst: &IndexExpr<'_>,
src: &IndexExpr<'_>,
) -> String {
fn print_offset(offset: MinifyingSugg<'static>) -> MinifyingSugg<'static> {
if offset.as_str() == "0" {
sugg::EMPTY.into()
} else {
offset
}
}
let print_limit = |end: &Expr<'_>, end_str: &str, base: &Expr<'_>, sugg: MinifyingSugg<'static>| {
if_chain! {
if let ExprKind::MethodCall(method, _, len_args, _) = end.kind;
if method.ident.name == sym!(len);
if len_args.len() == 1;
if let Some(arg) = len_args.get(0);
if path_to_local(arg) == path_to_local(base);
then {
if sugg.as_str() == end_str {
sugg::EMPTY.into()
} else {
sugg
}
} else {
match limits {
ast::RangeLimits::Closed => {
sugg + &sugg::ONE.into()
},
ast::RangeLimits::HalfOpen => sugg,
}
}
}
};
let start_str = Sugg::hir(cx, start, "").into();
let end_str: MinifyingSugg<'_> = Sugg::hir(cx, end, "").into();
let print_offset_and_limit = |idx_expr: &IndexExpr<'_>| match idx_expr.idx {
StartKind::Range => (
print_offset(apply_offset(&start_str, &idx_expr.idx_offset)).into_sugg(),
print_limit(
end,
end_str.as_str(),
idx_expr.base,
apply_offset(&end_str, &idx_expr.idx_offset),
)
.into_sugg(),
),
StartKind::Counter { initializer } => {
let counter_start = Sugg::hir(cx, initializer, "").into();
(
print_offset(apply_offset(&counter_start, &idx_expr.idx_offset)).into_sugg(),
print_limit(
end,
end_str.as_str(),
idx_expr.base,
apply_offset(&end_str, &idx_expr.idx_offset) + &counter_start - &start_str,
)
.into_sugg(),
)
},
};
let (dst_offset, dst_limit) = print_offset_and_limit(&dst);
let (src_offset, src_limit) = print_offset_and_limit(&src);
let dst_base_str = snippet(cx, dst.base.span, "???");
let src_base_str = snippet(cx, src.base.span, "???");
let dst = if dst_offset == sugg::EMPTY && dst_limit == sugg::EMPTY {
dst_base_str
} else {
format!(
"{}[{}..{}]",
dst_base_str,
dst_offset.maybe_par(),
dst_limit.maybe_par()
)
.into()
};
format!(
"{}.clone_from_slice(&{}[{}..{}]);",
dst,
src_base_str,
src_offset.maybe_par(),
src_limit.maybe_par()
)
}
/// a wrapper of `Sugg`. Besides what `Sugg` do, this removes unnecessary `0`;
/// and also, it avoids subtracting a variable from the same one by replacing it with `0`.
/// it exists for the convenience of the overloaded operators while normal functions can do the
/// same.
#[derive(Clone)]
struct MinifyingSugg<'a>(Sugg<'a>);
impl<'a> MinifyingSugg<'a> {
fn as_str(&self) -> &str {
let (Sugg::NonParen(s) | Sugg::MaybeParen(s) | Sugg::BinOp(_, s)) = &self.0;
s.as_ref()
}
fn into_sugg(self) -> Sugg<'a> {
self.0
}
}
impl<'a> From<Sugg<'a>> for MinifyingSugg<'a> {
fn from(sugg: Sugg<'a>) -> Self {
Self(sugg)
}
}
impl std::ops::Add for &MinifyingSugg<'static> {
type Output = MinifyingSugg<'static>;
fn add(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> {
match (self.as_str(), rhs.as_str()) {
("0", _) => rhs.clone(),
(_, "0") => self.clone(),
(_, _) => (&self.0 + &rhs.0).into(),
}
}
}
impl std::ops::Sub for &MinifyingSugg<'static> {
type Output = MinifyingSugg<'static>;
fn sub(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> {
match (self.as_str(), rhs.as_str()) {
(_, "0") => self.clone(),
("0", _) => (-rhs.0.clone()).into(),
(x, y) if x == y => sugg::ZERO.into(),
(_, _) => (&self.0 - &rhs.0).into(),
}
}
}
impl std::ops::Add<&MinifyingSugg<'static>> for MinifyingSugg<'static> {
type Output = MinifyingSugg<'static>;
fn add(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> {
match (self.as_str(), rhs.as_str()) {
("0", _) => rhs.clone(),
(_, "0") => self,
(_, _) => (self.0 + &rhs.0).into(),
}
}
}
impl std::ops::Sub<&MinifyingSugg<'static>> for MinifyingSugg<'static> {
type Output = MinifyingSugg<'static>;
fn sub(self, rhs: &MinifyingSugg<'static>) -> MinifyingSugg<'static> {
match (self.as_str(), rhs.as_str()) {
(_, "0") => self,
("0", _) => (-rhs.0.clone()).into(),
(x, y) if x == y => sugg::ZERO.into(),
(_, _) => (self.0 - &rhs.0).into(),
}
}
}
/// a wrapper around `MinifyingSugg`, which carries a operator like currying
/// so that the suggested code become more efficient (e.g. `foo + -bar` `foo - bar`).
struct Offset {
value: MinifyingSugg<'static>,
sign: OffsetSign,
}
#[derive(Clone, Copy)]
enum OffsetSign {
Positive,
Negative,
}
impl Offset {
fn negative(value: Sugg<'static>) -> Self {
Self {
value: value.into(),
sign: OffsetSign::Negative,
}
}
fn positive(value: Sugg<'static>) -> Self {
Self {
value: value.into(),
sign: OffsetSign::Positive,
}
}
fn empty() -> Self {
Self::positive(sugg::ZERO)
}
}
fn apply_offset(lhs: &MinifyingSugg<'static>, rhs: &Offset) -> MinifyingSugg<'static> {
match rhs.sign {
OffsetSign::Positive => lhs + &rhs.value,
OffsetSign::Negative => lhs - &rhs.value,
}
}
#[derive(Debug, Clone, Copy)]
enum StartKind<'hir> {
Range,
Counter { initializer: &'hir Expr<'hir> },
}
struct IndexExpr<'hir> {
base: &'hir Expr<'hir>,
idx: StartKind<'hir>,
idx_offset: Offset,
}
struct Start<'hir> {
id: HirId,
kind: StartKind<'hir>,
}
fn is_slice_like<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'_>) -> bool {
let is_slice = match ty.kind() {
ty::Ref(_, subty, _) => is_slice_like(cx, subty),
ty::Slice(..) | ty::Array(..) => true,
_ => false,
};
is_slice || is_type_diagnostic_item(cx, ty, sym::vec_type) || is_type_diagnostic_item(cx, ty, sym::vecdeque_type)
}
fn fetch_cloned_expr<'tcx>(expr: &'tcx Expr<'tcx>) -> &'tcx Expr<'tcx> {
if_chain! {
if let ExprKind::MethodCall(method, _, args, _) = expr.kind;
if method.ident.name == sym::clone;
if args.len() == 1;
if let Some(arg) = args.get(0);
then { arg } else { expr }
}
}
fn get_details_from_idx<'tcx>(
cx: &LateContext<'tcx>,
idx: &Expr<'_>,
starts: &[Start<'tcx>],
) -> Option<(StartKind<'tcx>, Offset)> {
fn get_start<'tcx>(e: &Expr<'_>, starts: &[Start<'tcx>]) -> Option<StartKind<'tcx>> {
let id = path_to_local(e)?;
starts.iter().find(|start| start.id == id).map(|start| start.kind)
}
fn get_offset<'tcx>(cx: &LateContext<'tcx>, e: &Expr<'_>, starts: &[Start<'tcx>]) -> Option<Sugg<'static>> {
match &e.kind {
ExprKind::Lit(l) => match l.node {
ast::LitKind::Int(x, _ty) => Some(Sugg::NonParen(x.to_string().into())),
_ => None,
},
ExprKind::Path(..) if get_start(e, starts).is_none() => Some(Sugg::hir(cx, e, "???")),
_ => None,
}
}
match idx.kind {
ExprKind::Binary(op, lhs, rhs) => match op.node {
BinOpKind::Add => {
let offset_opt = get_start(lhs, starts)
.and_then(|s| get_offset(cx, rhs, starts).map(|o| (s, o)))
.or_else(|| get_start(rhs, starts).and_then(|s| get_offset(cx, lhs, starts).map(|o| (s, o))));
offset_opt.map(|(s, o)| (s, Offset::positive(o)))
},
BinOpKind::Sub => {
get_start(lhs, starts).and_then(|s| get_offset(cx, rhs, starts).map(|o| (s, Offset::negative(o))))
},
_ => None,
},
ExprKind::Path(..) => get_start(idx, starts).map(|s| (s, Offset::empty())),
_ => None,
}
}
fn get_assignment<'tcx>(e: &'tcx Expr<'tcx>) -> Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)> {
if let ExprKind::Assign(lhs, rhs, _) = e.kind {
Some((lhs, rhs))
} else {
None
}
}
/// Get assignments from the given block.
/// The returned iterator yields `None` if no assignment expressions are there,
/// filtering out the increments of the given whitelisted loop counters;
/// because its job is to make sure there's nothing other than assignments and the increments.
fn get_assignments<'a, 'tcx>(
Block { stmts, expr, .. }: &'tcx Block<'tcx>,
loop_counters: &'a [Start<'tcx>],
) -> impl Iterator<Item = Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)>> + 'a {
// As the `filter` and `map` below do different things, I think putting together
// just increases complexity. (cc #3188 and #4193)
stmts
.iter()
.filter_map(move |stmt| match stmt.kind {
StmtKind::Local(..) | StmtKind::Item(..) => None,
StmtKind::Expr(e) | StmtKind::Semi(e) => Some(e),
})
.chain((*expr).into_iter())
.filter(move |e| {
if let ExprKind::AssignOp(_, place, _) = e.kind {
path_to_local(place).map_or(false, |id| {
!loop_counters
.iter()
// skip the first item which should be `StartKind::Range`
// this makes it possible to use the slice with `StartKind::Range` in the same iterator loop.
.skip(1)
.any(|counter| counter.id == id)
})
} else {
true
}
})
.map(get_assignment)
}
fn get_loop_counters<'a, 'tcx>(
cx: &'a LateContext<'tcx>,
body: &'tcx Block<'tcx>,
expr: &'tcx Expr<'_>,
) -> Option<impl Iterator<Item = Start<'tcx>> + 'a> {
// Look for variables that are incremented once per loop iteration.
let mut increment_visitor = IncrementVisitor::new(cx);
walk_block(&mut increment_visitor, body);
// For each candidate, check the parent block to see if
// it's initialized to zero at the start of the loop.
get_enclosing_block(&cx, expr.hir_id).and_then(|block| {
increment_visitor
.into_results()
.filter_map(move |var_id| {
let mut initialize_visitor = InitializeVisitor::new(cx, expr, var_id);
walk_block(&mut initialize_visitor, block);
initialize_visitor.get_result().map(|(_, initializer)| Start {
id: var_id,
kind: StartKind::Counter { initializer },
})
})
.into()
})
}

627
clippy_lints/src/loops/mod.rs Normal file
View file

@ -0,0 +1,627 @@
mod empty_loop;
mod explicit_counter_loop;
mod explicit_into_iter_loop;
mod explicit_iter_loop;
mod for_kv_map;
mod for_loops_over_fallibles;
mod iter_next_loop;
mod manual_flatten;
mod manual_memcpy;
mod mut_range_bound;
mod needless_collect;
mod needless_range_loop;
mod never_loop;
mod same_item_push;
mod single_element_loop;
mod utils;
mod while_immutable_condition;
mod while_let_loop;
mod while_let_on_iterator;
use crate::utils::higher;
use rustc_hir::{Expr, ExprKind, LoopSource, Pat};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;
use utils::{get_span_of_entire_for_loop, make_iterator_snippet, IncrementVisitor, InitializeVisitor};
declare_clippy_lint! {
/// **What it does:** Checks for for-loops that manually copy items between
/// slices that could be optimized by having a memcpy.
///
/// **Why is this bad?** It is not as fast as a memcpy.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// # let src = vec![1];
/// # let mut dst = vec![0; 65];
/// for i in 0..src.len() {
/// dst[i + 64] = src[i];
/// }
/// ```
/// Could be written as:
/// ```rust
/// # let src = vec![1];
/// # let mut dst = vec![0; 65];
/// dst[64..(src.len() + 64)].clone_from_slice(&src[..]);
/// ```
pub MANUAL_MEMCPY,
perf,
"manually copying items between slices"
}
declare_clippy_lint! {
/// **What it does:** Checks for looping over the range of `0..len` of some
/// collection just to get the values by index.
///
/// **Why is this bad?** Just iterating the collection itself makes the intent
/// more clear and is probably faster.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// let vec = vec!['a', 'b', 'c'];
/// for i in 0..vec.len() {
/// println!("{}", vec[i]);
/// }
/// ```
/// Could be written as:
/// ```rust
/// let vec = vec!['a', 'b', 'c'];
/// for i in vec {
/// println!("{}", i);
/// }
/// ```
pub NEEDLESS_RANGE_LOOP,
style,
"for-looping over a range of indices where an iterator over items would do"
}
declare_clippy_lint! {
/// **What it does:** Checks for loops on `x.iter()` where `&x` will do, and
/// suggests the latter.
///
/// **Why is this bad?** Readability.
///
/// **Known problems:** False negatives. We currently only warn on some known
/// types.
///
/// **Example:**
/// ```rust
/// // with `y` a `Vec` or slice:
/// # let y = vec![1];
/// for x in y.iter() {
/// // ..
/// }
/// ```
/// can be rewritten to
/// ```rust
/// # let y = vec![1];
/// for x in &y {
/// // ..
/// }
/// ```
pub EXPLICIT_ITER_LOOP,
pedantic,
"for-looping over `_.iter()` or `_.iter_mut()` when `&_` or `&mut _` would do"
}
declare_clippy_lint! {
/// **What it does:** Checks for loops on `y.into_iter()` where `y` will do, and
/// suggests the latter.
///
/// **Why is this bad?** Readability.
///
/// **Known problems:** None
///
/// **Example:**
/// ```rust
/// # let y = vec![1];
/// // with `y` a `Vec` or slice:
/// for x in y.into_iter() {
/// // ..
/// }
/// ```
/// can be rewritten to
/// ```rust
/// # let y = vec![1];
/// for x in y {
/// // ..
/// }
/// ```
pub EXPLICIT_INTO_ITER_LOOP,
pedantic,
"for-looping over `_.into_iter()` when `_` would do"
}
declare_clippy_lint! {
/// **What it does:** Checks for loops on `x.next()`.
///
/// **Why is this bad?** `next()` returns either `Some(value)` if there was a
/// value, or `None` otherwise. The insidious thing is that `Option<_>`
/// implements `IntoIterator`, so that possibly one value will be iterated,
/// leading to some hard to find bugs. No one will want to write such code
/// [except to win an Underhanded Rust
/// Contest](https://www.reddit.com/r/rust/comments/3hb0wm/underhanded_rust_contest/cu5yuhr).
///
/// **Known problems:** None.
///
/// **Example:**
/// ```ignore
/// for x in y.next() {
/// ..
/// }
/// ```
pub ITER_NEXT_LOOP,
correctness,
"for-looping over `_.next()` which is probably not intended"
}
declare_clippy_lint! {
/// **What it does:** Checks for `for` loops over `Option` or `Result` values.
///
/// **Why is this bad?** Readability. This is more clearly expressed as an `if
/// let`.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// # let opt = Some(1);
///
/// // Bad
/// for x in opt {
/// // ..
/// }
///
/// // Good
/// if let Some(x) = opt {
/// // ..
/// }
/// ```
///
/// // or
///
/// ```rust
/// # let res: Result<i32, std::io::Error> = Ok(1);
///
/// // Bad
/// for x in &res {
/// // ..
/// }
///
/// // Good
/// if let Ok(x) = res {
/// // ..
/// }
/// ```
pub FOR_LOOPS_OVER_FALLIBLES,
correctness,
"for-looping over an `Option` or a `Result`, which is more clearly expressed as an `if let`"
}
declare_clippy_lint! {
/// **What it does:** Detects `loop + match` combinations that are easier
/// written as a `while let` loop.
///
/// **Why is this bad?** The `while let` loop is usually shorter and more
/// readable.
///
/// **Known problems:** Sometimes the wrong binding is displayed ([#383](https://github.com/rust-lang/rust-clippy/issues/383)).
///
/// **Example:**
/// ```rust,no_run
/// # let y = Some(1);
/// loop {
/// let x = match y {
/// Some(x) => x,
/// None => break,
/// };
/// // .. do something with x
/// }
/// // is easier written as
/// while let Some(x) = y {
/// // .. do something with x
/// };
/// ```
pub WHILE_LET_LOOP,
complexity,
"`loop { if let { ... } else break }`, which can be written as a `while let` loop"
}
declare_clippy_lint! {
/// **What it does:** Checks for functions collecting an iterator when collect
/// is not needed.
///
/// **Why is this bad?** `collect` causes the allocation of a new data structure,
/// when this allocation may not be needed.
///
/// **Known problems:**
/// None
///
/// **Example:**
/// ```rust
/// # let iterator = vec![1].into_iter();
/// let len = iterator.clone().collect::<Vec<_>>().len();
/// // should be
/// let len = iterator.count();
/// ```
pub NEEDLESS_COLLECT,
perf,
"collecting an iterator when collect is not needed"
}
declare_clippy_lint! {
/// **What it does:** Checks `for` loops over slices with an explicit counter
/// and suggests the use of `.enumerate()`.
///
/// **Why is it bad?** Using `.enumerate()` makes the intent more clear,
/// declutters the code and may be faster in some instances.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// # let v = vec![1];
/// # fn bar(bar: usize, baz: usize) {}
/// let mut i = 0;
/// for item in &v {
/// bar(i, *item);
/// i += 1;
/// }
/// ```
/// Could be written as
/// ```rust
/// # let v = vec![1];
/// # fn bar(bar: usize, baz: usize) {}
/// for (i, item) in v.iter().enumerate() { bar(i, *item); }
/// ```
pub EXPLICIT_COUNTER_LOOP,
complexity,
"for-looping with an explicit counter when `_.enumerate()` would do"
}
declare_clippy_lint! {
/// **What it does:** Checks for empty `loop` expressions.
///
/// **Why is this bad?** These busy loops burn CPU cycles without doing
/// anything. It is _almost always_ a better idea to `panic!` than to have
/// a busy loop.
///
/// If panicking isn't possible, think of the environment and either:
/// - block on something
/// - sleep the thread for some microseconds
/// - yield or pause the thread
///
/// For `std` targets, this can be done with
/// [`std::thread::sleep`](https://doc.rust-lang.org/std/thread/fn.sleep.html)
/// or [`std::thread::yield_now`](https://doc.rust-lang.org/std/thread/fn.yield_now.html).
///
/// For `no_std` targets, doing this is more complicated, especially because
/// `#[panic_handler]`s can't panic. To stop/pause the thread, you will
/// probably need to invoke some target-specific intrinsic. Examples include:
/// - [`x86_64::instructions::hlt`](https://docs.rs/x86_64/0.12.2/x86_64/instructions/fn.hlt.html)
/// - [`cortex_m::asm::wfi`](https://docs.rs/cortex-m/0.6.3/cortex_m/asm/fn.wfi.html)
///
/// **Known problems:** None.
///
/// **Example:**
/// ```no_run
/// loop {}
/// ```
pub EMPTY_LOOP,
style,
"empty `loop {}`, which should block or sleep"
}
declare_clippy_lint! {
/// **What it does:** Checks for `while let` expressions on iterators.
///
/// **Why is this bad?** Readability. A simple `for` loop is shorter and conveys
/// the intent better.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```ignore
/// while let Some(val) = iter() {
/// ..
/// }
/// ```
pub WHILE_LET_ON_ITERATOR,
style,
"using a `while let` loop instead of a for loop on an iterator"
}
declare_clippy_lint! {
/// **What it does:** Checks for iterating a map (`HashMap` or `BTreeMap`) and
/// ignoring either the keys or values.
///
/// **Why is this bad?** Readability. There are `keys` and `values` methods that
/// can be used to express that don't need the values or keys.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```ignore
/// for (k, _) in &map {
/// ..
/// }
/// ```
///
/// could be replaced by
///
/// ```ignore
/// for k in map.keys() {
/// ..
/// }
/// ```
pub FOR_KV_MAP,
style,
"looping on a map using `iter` when `keys` or `values` would do"
}
declare_clippy_lint! {
/// **What it does:** Checks for loops that will always `break`, `return` or
/// `continue` an outer loop.
///
/// **Why is this bad?** This loop never loops, all it does is obfuscating the
/// code.
///
/// **Known problems:** None
///
/// **Example:**
/// ```rust
/// loop {
/// ..;
/// break;
/// }
/// ```
pub NEVER_LOOP,
correctness,
"any loop that will always `break` or `return`"
}
declare_clippy_lint! {
/// **What it does:** Checks for loops which have a range bound that is a mutable variable
///
/// **Why is this bad?** One might think that modifying the mutable variable changes the loop bounds
///
/// **Known problems:** None
///
/// **Example:**
/// ```rust
/// let mut foo = 42;
/// for i in 0..foo {
/// foo -= 1;
/// println!("{}", i); // prints numbers from 0 to 42, not 0 to 21
/// }
/// ```
pub MUT_RANGE_BOUND,
complexity,
"for loop over a range where one of the bounds is a mutable variable"
}
declare_clippy_lint! {
/// **What it does:** Checks whether variables used within while loop condition
/// can be (and are) mutated in the body.
///
/// **Why is this bad?** If the condition is unchanged, entering the body of the loop
/// will lead to an infinite loop.
///
/// **Known problems:** If the `while`-loop is in a closure, the check for mutation of the
/// condition variables in the body can cause false negatives. For example when only `Upvar` `a` is
/// in the condition and only `Upvar` `b` gets mutated in the body, the lint will not trigger.
///
/// **Example:**
/// ```rust
/// let i = 0;
/// while i > 10 {
/// println!("let me loop forever!");
/// }
/// ```
pub WHILE_IMMUTABLE_CONDITION,
correctness,
"variables used within while expression are not mutated in the body"
}
declare_clippy_lint! {
/// **What it does:** Checks whether a for loop is being used to push a constant
/// value into a Vec.
///
/// **Why is this bad?** This kind of operation can be expressed more succinctly with
/// `vec![item;SIZE]` or `vec.resize(NEW_SIZE, item)` and using these alternatives may also
/// have better performance.
/// **Known problems:** None
///
/// **Example:**
/// ```rust
/// let item1 = 2;
/// let item2 = 3;
/// let mut vec: Vec<u8> = Vec::new();
/// for _ in 0..20 {
/// vec.push(item1);
/// }
/// for _ in 0..30 {
/// vec.push(item2);
/// }
/// ```
/// could be written as
/// ```rust
/// let item1 = 2;
/// let item2 = 3;
/// let mut vec: Vec<u8> = vec![item1; 20];
/// vec.resize(20 + 30, item2);
/// ```
pub SAME_ITEM_PUSH,
style,
"the same item is pushed inside of a for loop"
}
declare_clippy_lint! {
/// **What it does:** Checks whether a for loop has a single element.
///
/// **Why is this bad?** There is no reason to have a loop of a
/// single element.
/// **Known problems:** None
///
/// **Example:**
/// ```rust
/// let item1 = 2;
/// for item in &[item1] {
/// println!("{}", item);
/// }
/// ```
/// could be written as
/// ```rust
/// let item1 = 2;
/// let item = &item1;
/// println!("{}", item);
/// ```
pub SINGLE_ELEMENT_LOOP,
complexity,
"there is no reason to have a single element loop"
}
declare_clippy_lint! {
/// **What it does:** Check for unnecessary `if let` usage in a for loop
/// where only the `Some` or `Ok` variant of the iterator element is used.
///
/// **Why is this bad?** It is verbose and can be simplified
/// by first calling the `flatten` method on the `Iterator`.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// let x = vec![Some(1), Some(2), Some(3)];
/// for n in x {
/// if let Some(n) = n {
/// println!("{}", n);
/// }
/// }
/// ```
/// Use instead:
/// ```rust
/// let x = vec![Some(1), Some(2), Some(3)];
/// for n in x.into_iter().flatten() {
/// println!("{}", n);
/// }
/// ```
pub MANUAL_FLATTEN,
complexity,
"for loops over `Option`s or `Result`s with a single expression can be simplified"
}
declare_lint_pass!(Loops => [
MANUAL_MEMCPY,
MANUAL_FLATTEN,
NEEDLESS_RANGE_LOOP,
EXPLICIT_ITER_LOOP,
EXPLICIT_INTO_ITER_LOOP,
ITER_NEXT_LOOP,
FOR_LOOPS_OVER_FALLIBLES,
WHILE_LET_LOOP,
NEEDLESS_COLLECT,
EXPLICIT_COUNTER_LOOP,
EMPTY_LOOP,
WHILE_LET_ON_ITERATOR,
FOR_KV_MAP,
NEVER_LOOP,
MUT_RANGE_BOUND,
WHILE_IMMUTABLE_CONDITION,
SAME_ITEM_PUSH,
SINGLE_ELEMENT_LOOP,
]);
impl<'tcx> LateLintPass<'tcx> for Loops {
#[allow(clippy::too_many_lines)]
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let Some((pat, arg, body, span)) = higher::for_loop(expr) {
// we don't want to check expanded macros
// this check is not at the top of the function
// since higher::for_loop expressions are marked as expansions
if body.span.from_expansion() {
return;
}
check_for_loop(cx, pat, arg, body, expr, span);
}
// we don't want to check expanded macros
if expr.span.from_expansion() {
return;
}
// check for never_loop
never_loop::check(cx, expr);
// check for `loop { if let {} else break }` that could be `while let`
// (also matches an explicit "match" instead of "if let")
// (even if the "match" or "if let" is used for declaration)
if let ExprKind::Loop(ref block, _, LoopSource::Loop, _) = expr.kind {
// also check for empty `loop {}` statements, skipping those in #[panic_handler]
empty_loop::check(cx, expr, block);
while_let_loop::check(cx, expr, block);
}
while_let_on_iterator::check(cx, expr);
if let Some((cond, body)) = higher::while_loop(&expr) {
while_immutable_condition::check(cx, cond, body);
}
needless_collect::check(expr, cx);
}
}
fn check_for_loop<'tcx>(
cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,
arg: &'tcx Expr<'_>,
body: &'tcx Expr<'_>,
expr: &'tcx Expr<'_>,
span: Span,
) {
let is_manual_memcpy_triggered = manual_memcpy::check(cx, pat, arg, body, expr);
if !is_manual_memcpy_triggered {
needless_range_loop::check(cx, pat, arg, body, expr);
explicit_counter_loop::check(cx, pat, arg, body, expr);
}
check_for_loop_arg(cx, pat, arg, expr);
for_kv_map::check(cx, pat, arg, body, expr);
mut_range_bound::check(cx, arg, body);
single_element_loop::check(cx, pat, arg, body, expr);
same_item_push::check(cx, pat, arg, body, expr);
manual_flatten::check(cx, pat, arg, body, span);
}
fn check_for_loop_arg(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>, expr: &Expr<'_>) {
let mut next_loop_linted = false; // whether or not ITER_NEXT_LOOP lint was used
if let ExprKind::MethodCall(ref method, _, ref args, _) = arg.kind {
// just the receiver, no arguments
if args.len() == 1 {
let method_name = &*method.ident.as_str();
// check for looping over x.iter() or x.iter_mut(), could use &x or &mut x
match method_name {
"iter" | "iter_mut" => explicit_iter_loop::check(cx, args, arg, method_name),
"into_iter" => {
explicit_iter_loop::check(cx, args, arg, method_name);
explicit_into_iter_loop::check(cx, args, arg);
},
"next" => {
next_loop_linted = iter_next_loop::check(cx, arg, expr);
},
_ => {},
}
}
}
if !next_loop_linted {
for_loops_over_fallibles::check(cx, pat, arg);
}
}

115
clippy_lints/src/loops/mut_range_bound.rs Normal file
View file

@ -0,0 +1,115 @@
use super::MUT_RANGE_BOUND;
use crate::utils::{higher, path_to_local, span_lint};
use if_chain::if_chain;
use rustc_hir::{BindingAnnotation, Expr, HirId, Node, PatKind};
use rustc_infer::infer::TyCtxtInferExt;
use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_span::source_map::Span;
use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId};
pub(super) fn check(cx: &LateContext<'_>, arg: &Expr<'_>, body: &Expr<'_>) {
if let Some(higher::Range {
start: Some(start),
end: Some(end),
..
}) = higher::range(arg)
{
let mut_ids = vec![check_for_mutability(cx, start), check_for_mutability(cx, end)];
if mut_ids[0].is_some() || mut_ids[1].is_some() {
let (span_low, span_high) = check_for_mutation(cx, body, &mut_ids);
mut_warn_with_span(cx, span_low);
mut_warn_with_span(cx, span_high);
}
}
}
fn mut_warn_with_span(cx: &LateContext<'_>, span: Option<Span>) {
if let Some(sp) = span {
span_lint(
cx,
MUT_RANGE_BOUND,
sp,
"attempt to mutate range bound within loop; note that the range of the loop is unchanged",
);
}
}
fn check_for_mutability(cx: &LateContext<'_>, bound: &Expr<'_>) -> Option<HirId> {
if_chain! {
if let Some(hir_id) = path_to_local(bound);
if let Node::Binding(pat) = cx.tcx.hir().get(hir_id);
if let PatKind::Binding(BindingAnnotation::Mutable, ..) = pat.kind;
then {
return Some(hir_id);
}
}
None
}
fn check_for_mutation<'tcx>(
cx: &LateContext<'tcx>,
body: &Expr<'_>,
bound_ids: &[Option<HirId>],
) -> (Option<Span>, Option<Span>) {
let mut delegate = MutatePairDelegate {
cx,
hir_id_low: bound_ids[0],
hir_id_high: bound_ids[1],
span_low: None,
span_high: None,
};
cx.tcx.infer_ctxt().enter(|infcx| {
ExprUseVisitor::new(
&mut delegate,
&infcx,
body.hir_id.owner,
cx.param_env,
cx.typeck_results(),
)
.walk_expr(body);
});
delegate.mutation_span()
}
struct MutatePairDelegate<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
hir_id_low: Option<HirId>,
hir_id_high: Option<HirId>,
span_low: Option<Span>,
span_high: Option<Span>,
}
impl<'tcx> Delegate<'tcx> for MutatePairDelegate<'_, 'tcx> {
fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId, _: ConsumeMode) {}
fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, diag_expr_id: HirId, bk: ty::BorrowKind) {
if let ty::BorrowKind::MutBorrow = bk {
if let PlaceBase::Local(id) = cmt.place.base {
if Some(id) == self.hir_id_low {
self.span_low = Some(self.cx.tcx.hir().span(diag_expr_id))
}
if Some(id) == self.hir_id_high {
self.span_high = Some(self.cx.tcx.hir().span(diag_expr_id))
}
}
}
}
fn mutate(&mut self, cmt: &PlaceWithHirId<'tcx>, diag_expr_id: HirId) {
if let PlaceBase::Local(id) = cmt.place.base {
if Some(id) == self.hir_id_low {
self.span_low = Some(self.cx.tcx.hir().span(diag_expr_id))
}
if Some(id) == self.hir_id_high {
self.span_high = Some(self.cx.tcx.hir().span(diag_expr_id))
}
}
}
}
impl MutatePairDelegate<'_, '_> {
fn mutation_span(&self) -> (Option<Span>, Option<Span>) {
(self.span_low, self.span_high)
}
}

283
clippy_lints/src/loops/needless_collect.rs Normal file
View file

@ -0,0 +1,283 @@
use super::NEEDLESS_COLLECT;
use crate::utils::sugg::Sugg;
use crate::utils::{
is_type_diagnostic_item, match_trait_method, match_type, path_to_local_id, paths, snippet, span_lint_and_sugg,
span_lint_and_then,
};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_block, walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::{Block, Expr, ExprKind, GenericArg, HirId, Local, Pat, PatKind, QPath, StmtKind};
use rustc_lint::LateContext;
use rustc_middle::hir::map::Map;
use rustc_span::source_map::Span;
use rustc_span::symbol::{sym, Ident};
const NEEDLESS_COLLECT_MSG: &str = "avoid using `collect()` when not needed";
pub(super) fn check<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) {
check_needless_collect_direct_usage(expr, cx);
check_needless_collect_indirect_usage(expr, cx);
}
fn check_needless_collect_direct_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) {
if_chain! {
if let ExprKind::MethodCall(ref method, _, ref args, _) = expr.kind;
if let ExprKind::MethodCall(ref chain_method, _, _, _) = args[0].kind;
if chain_method.ident.name == sym!(collect) && match_trait_method(cx, &args[0], &paths::ITERATOR);
if let Some(ref generic_args) = chain_method.args;
if let Some(GenericArg::Type(ref ty)) = generic_args.args.get(0);
then {
let ty = cx.typeck_results().node_type(ty.hir_id);
if is_type_diagnostic_item(cx, ty, sym::vec_type) ||
is_type_diagnostic_item(cx, ty, sym::vecdeque_type) ||
match_type(cx, ty, &paths::BTREEMAP) ||
is_type_diagnostic_item(cx, ty, sym::hashmap_type) {
if method.ident.name == sym!(len) {
let span = shorten_needless_collect_span(expr);
span_lint_and_sugg(
cx,
NEEDLESS_COLLECT,
span,
NEEDLESS_COLLECT_MSG,
"replace with",
"count()".to_string(),
Applicability::MachineApplicable,
);
}
if method.ident.name == sym!(is_empty) {
let span = shorten_needless_collect_span(expr);
span_lint_and_sugg(
cx,
NEEDLESS_COLLECT,
span,
NEEDLESS_COLLECT_MSG,
"replace with",
"next().is_none()".to_string(),
Applicability::MachineApplicable,
);
}
if method.ident.name == sym!(contains) {
let contains_arg = snippet(cx, args[1].span, "??");
let span = shorten_needless_collect_span(expr);
span_lint_and_then(
cx,
NEEDLESS_COLLECT,
span,
NEEDLESS_COLLECT_MSG,
|diag| {
let (arg, pred) = contains_arg
.strip_prefix('&')
.map_or(("&x", &*contains_arg), |s| ("x", s));
diag.span_suggestion(
span,
"replace with",
format!(
"any(|{}| x == {})",
arg, pred
),
Applicability::MachineApplicable,
);
}
);
}
}
}
}
}
fn check_needless_collect_indirect_usage<'tcx>(expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) {
if let ExprKind::Block(ref block, _) = expr.kind {
for ref stmt in block.stmts {
if_chain! {
if let StmtKind::Local(
Local { pat: Pat { hir_id: pat_id, kind: PatKind::Binding(_, _, ident, .. ), .. },
init: Some(ref init_expr), .. }
) = stmt.kind;
if let ExprKind::MethodCall(ref method_name, _, &[ref iter_source], ..) = init_expr.kind;
if method_name.ident.name == sym!(collect) && match_trait_method(cx, &init_expr, &paths::ITERATOR);
if let Some(ref generic_args) = method_name.args;
if let Some(GenericArg::Type(ref ty)) = generic_args.args.get(0);
if let ty = cx.typeck_results().node_type(ty.hir_id);
if is_type_diagnostic_item(cx, ty, sym::vec_type) ||
is_type_diagnostic_item(cx, ty, sym::vecdeque_type) ||
match_type(cx, ty, &paths::LINKED_LIST);
if let Some(iter_calls) = detect_iter_and_into_iters(block, *ident);
if iter_calls.len() == 1;
then {
let mut used_count_visitor = UsedCountVisitor {
cx,
id: *pat_id,
count: 0,
};
walk_block(&mut used_count_visitor, block);
if used_count_visitor.count > 1 {
return;
}
// Suggest replacing iter_call with iter_replacement, and removing stmt
let iter_call = &iter_calls[0];
span_lint_and_then(
cx,
super::NEEDLESS_COLLECT,
stmt.span.until(iter_call.span),
NEEDLESS_COLLECT_MSG,
|diag| {
let iter_replacement = format!("{}{}", Sugg::hir(cx, iter_source, ".."), iter_call.get_iter_method(cx));
diag.multipart_suggestion(
iter_call.get_suggestion_text(),
vec![
(stmt.span, String::new()),
(iter_call.span, iter_replacement)
],
Applicability::MachineApplicable,// MaybeIncorrect,
).emit();
},
);
}
}
}
}
}
struct IterFunction {
func: IterFunctionKind,
span: Span,
}
impl IterFunction {
fn get_iter_method(&self, cx: &LateContext<'_>) -> String {
match &self.func {
IterFunctionKind::IntoIter => String::new(),
IterFunctionKind::Len => String::from(".count()"),
IterFunctionKind::IsEmpty => String::from(".next().is_none()"),
IterFunctionKind::Contains(span) => {
let s = snippet(cx, *span, "..");
if let Some(stripped) = s.strip_prefix('&') {
format!(".any(|x| x == {})", stripped)
} else {
format!(".any(|x| x == *{})", s)
}
},
}
}
fn get_suggestion_text(&self) -> &'static str {
match &self.func {
IterFunctionKind::IntoIter => {
"use the original Iterator instead of collecting it and then producing a new one"
},
IterFunctionKind::Len => {
"take the original Iterator's count instead of collecting it and finding the length"
},
IterFunctionKind::IsEmpty => {
"check if the original Iterator has anything instead of collecting it and seeing if it's empty"
},
IterFunctionKind::Contains(_) => {
"check if the original Iterator contains an element instead of collecting then checking"
},
}
}
}
enum IterFunctionKind {
IntoIter,
Len,
IsEmpty,
Contains(Span),
}
struct IterFunctionVisitor {
uses: Vec<IterFunction>,
seen_other: bool,
target: Ident,
}
impl<'tcx> Visitor<'tcx> for IterFunctionVisitor {
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
// Check function calls on our collection
if_chain! {
if let ExprKind::MethodCall(method_name, _, ref args, _) = &expr.kind;
if let Some(Expr { kind: ExprKind::Path(QPath::Resolved(_, ref path)), .. }) = args.get(0);
if let &[name] = &path.segments;
if name.ident == self.target;
then {
let len = sym!(len);
let is_empty = sym!(is_empty);
let contains = sym!(contains);
match method_name.ident.name {
sym::into_iter => self.uses.push(
IterFunction { func: IterFunctionKind::IntoIter, span: expr.span }
),
name if name == len => self.uses.push(
IterFunction { func: IterFunctionKind::Len, span: expr.span }
),
name if name == is_empty => self.uses.push(
IterFunction { func: IterFunctionKind::IsEmpty, span: expr.span }
),
name if name == contains => self.uses.push(
IterFunction { func: IterFunctionKind::Contains(args[1].span), span: expr.span }
),
_ => self.seen_other = true,
}
return
}
}
// Check if the collection is used for anything else
if_chain! {
if let Expr { kind: ExprKind::Path(QPath::Resolved(_, ref path)), .. } = expr;
if let &[name] = &path.segments;
if name.ident == self.target;
then {
self.seen_other = true;
} else {
walk_expr(self, expr);
}
}
}
type Map = Map<'tcx>;
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
}
struct UsedCountVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
id: HirId,
count: usize,
}
impl<'a, 'tcx> Visitor<'tcx> for UsedCountVisitor<'a, 'tcx> {
type Map = Map<'tcx>;
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if path_to_local_id(expr, self.id) {
self.count += 1;
} else {
walk_expr(self, expr);
}
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::OnlyBodies(self.cx.tcx.hir())
}
}
/// Detect the occurrences of calls to `iter` or `into_iter` for the
/// given identifier
fn detect_iter_and_into_iters<'tcx>(block: &'tcx Block<'tcx>, identifier: Ident) -> Option<Vec<IterFunction>> {
let mut visitor = IterFunctionVisitor {
uses: Vec::new(),
target: identifier,
seen_other: false,
};
visitor.visit_block(block);
if visitor.seen_other { None } else { Some(visitor.uses) }
}
fn shorten_needless_collect_span(expr: &Expr<'_>) -> Span {
if_chain! {
if let ExprKind::MethodCall(.., args, _) = &expr.kind;
if let ExprKind::MethodCall(_, span, ..) = &args[0].kind;
then {
return expr.span.with_lo(span.lo());
}
}
unreachable!();
}

391
clippy_lints/src/loops/needless_range_loop.rs Normal file
View file

@ -0,0 +1,391 @@
use super::NEEDLESS_RANGE_LOOP;
use crate::utils::visitors::LocalUsedVisitor;
use crate::utils::{
contains_name, has_iter_method, higher, is_integer_const, match_trait_method, multispan_sugg, path_to_local_id,
paths, snippet, span_lint_and_then, sugg, SpanlessEq,
};
use if_chain::if_chain;
use rustc_ast::ast;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_hir::def::{DefKind, Res};
use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, HirId, Mutability, Pat, PatKind, QPath};
use rustc_lint::LateContext;
use rustc_middle::hir::map::Map;
use rustc_middle::middle::region;
use rustc_middle::ty::{self, Ty};
use rustc_span::symbol::{sym, Symbol};
use std::iter::Iterator;
use std::mem;
/// Checks for looping over a range and then indexing a sequence with it.
/// The iteratee must be a range literal.
#[allow(clippy::too_many_lines)]
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,
arg: &'tcx Expr<'_>,
body: &'tcx Expr<'_>,
expr: &'tcx Expr<'_>,
) {
if let Some(higher::Range {
start: Some(start),
ref end,
limits,
}) = higher::range(arg)
{
// the var must be a single name
if let PatKind::Binding(_, canonical_id, ident, _) = pat.kind {
let mut visitor = VarVisitor {
cx,
var: canonical_id,
indexed_mut: FxHashSet::default(),
indexed_indirectly: FxHashMap::default(),
indexed_directly: FxHashMap::default(),
referenced: FxHashSet::default(),
nonindex: false,
prefer_mutable: false,
};
walk_expr(&mut visitor, body);
// linting condition: we only indexed one variable, and indexed it directly
if visitor.indexed_indirectly.is_empty() && visitor.indexed_directly.len() == 1 {
let (indexed, (indexed_extent, indexed_ty)) = visitor
.indexed_directly
.into_iter()
.next()
.expect("already checked that we have exactly 1 element");
// ensure that the indexed variable was declared before the loop, see #601
if let Some(indexed_extent) = indexed_extent {
let parent_id = cx.tcx.hir().get_parent_item(expr.hir_id);
let parent_def_id = cx.tcx.hir().local_def_id(parent_id);
let region_scope_tree = cx.tcx.region_scope_tree(parent_def_id);
let pat_extent = region_scope_tree.var_scope(pat.hir_id.local_id);
if region_scope_tree.is_subscope_of(indexed_extent, pat_extent) {
return;
}
}
// don't lint if the container that is indexed does not have .iter() method
let has_iter = has_iter_method(cx, indexed_ty);
if has_iter.is_none() {
return;
}
// don't lint if the container that is indexed into is also used without
// indexing
if visitor.referenced.contains(&indexed) {
return;
}
let starts_at_zero = is_integer_const(cx, start, 0);
let skip = if starts_at_zero {
String::new()
} else if visitor.indexed_mut.contains(&indexed) && contains_name(indexed, start) {
return;
} else {
format!(".skip({})", snippet(cx, start.span, ".."))
};
let mut end_is_start_plus_val = false;
let take = if let Some(end) = *end {
let mut take_expr = end;
if let ExprKind::Binary(ref op, ref left, ref right) = end.kind {
if let BinOpKind::Add = op.node {
let start_equal_left = SpanlessEq::new(cx).eq_expr(start, left);
let start_equal_right = SpanlessEq::new(cx).eq_expr(start, right);
if start_equal_left {
take_expr = right;
} else if start_equal_right {
take_expr = left;
}
end_is_start_plus_val = start_equal_left | start_equal_right;
}
}
if is_len_call(end, indexed) || is_end_eq_array_len(cx, end, limits, indexed_ty) {
String::new()
} else if visitor.indexed_mut.contains(&indexed) && contains_name(indexed, take_expr) {
return;
} else {
match limits {
ast::RangeLimits::Closed => {
let take_expr = sugg::Sugg::hir(cx, take_expr, "<count>");
format!(".take({})", take_expr + sugg::ONE)
},
ast::RangeLimits::HalfOpen => format!(".take({})", snippet(cx, take_expr.span, "..")),
}
}
} else {
String::new()
};
let (ref_mut, method) = if visitor.indexed_mut.contains(&indexed) {
("mut ", "iter_mut")
} else {
("", "iter")
};
let take_is_empty = take.is_empty();
let mut method_1 = take;
let mut method_2 = skip;
if end_is_start_plus_val {
mem::swap(&mut method_1, &mut method_2);
}
if visitor.nonindex {
span_lint_and_then(
cx,
NEEDLESS_RANGE_LOOP,
expr.span,
&format!("the loop variable `{}` is used to index `{}`", ident.name, indexed),
|diag| {
multispan_sugg(
diag,
"consider using an iterator",
vec![
(pat.span, format!("({}, <item>)", ident.name)),
(
arg.span,
format!("{}.{}().enumerate(){}{}", indexed, method, method_1, method_2),
),
],
);
},
);
} else {
let repl = if starts_at_zero && take_is_empty {
format!("&{}{}", ref_mut, indexed)
} else {
format!("{}.{}(){}{}", indexed, method, method_1, method_2)
};
span_lint_and_then(
cx,
NEEDLESS_RANGE_LOOP,
expr.span,
&format!("the loop variable `{}` is only used to index `{}`", ident.name, indexed),
|diag| {
multispan_sugg(
diag,
"consider using an iterator",
vec![(pat.span, "<item>".to_string()), (arg.span, repl)],
);
},
);
}
}
}
}
}
fn is_len_call(expr: &Expr<'_>, var: Symbol) -> bool {
if_chain! {
if let ExprKind::MethodCall(ref method, _, ref len_args, _) = expr.kind;
if len_args.len() == 1;
if method.ident.name == sym!(len);
if let ExprKind::Path(QPath::Resolved(_, ref path)) = len_args[0].kind;
if path.segments.len() == 1;
if path.segments[0].ident.name == var;
then {
return true;
}
}
false
}
fn is_end_eq_array_len<'tcx>(
cx: &LateContext<'tcx>,
end: &Expr<'_>,
limits: ast::RangeLimits,
indexed_ty: Ty<'tcx>,
) -> bool {
if_chain! {
if let ExprKind::Lit(ref lit) = end.kind;
if let ast::LitKind::Int(end_int, _) = lit.node;
if let ty::Array(_, arr_len_const) = indexed_ty.kind();
if let Some(arr_len) = arr_len_const.try_eval_usize(cx.tcx, cx.param_env);
then {
return match limits {
ast::RangeLimits::Closed => end_int + 1 >= arr_len.into(),
ast::RangeLimits::HalfOpen => end_int >= arr_len.into(),
};
}
}
false
}
struct VarVisitor<'a, 'tcx> {
/// context reference
cx: &'a LateContext<'tcx>,
/// var name to look for as index
var: HirId,
/// indexed variables that are used mutably
indexed_mut: FxHashSet<Symbol>,
/// indirectly indexed variables (`v[(i + 4) % N]`), the extend is `None` for global
indexed_indirectly: FxHashMap<Symbol, Option<region::Scope>>,
/// subset of `indexed` of vars that are indexed directly: `v[i]`
/// this will not contain cases like `v[calc_index(i)]` or `v[(i + 4) % N]`
indexed_directly: FxHashMap<Symbol, (Option<region::Scope>, Ty<'tcx>)>,
/// Any names that are used outside an index operation.
/// Used to detect things like `&mut vec` used together with `vec[i]`
referenced: FxHashSet<Symbol>,
/// has the loop variable been used in expressions other than the index of
/// an index op?
nonindex: bool,
/// Whether we are inside the `$` in `&mut $` or `$ = foo` or `$.bar`, where bar
/// takes `&mut self`
prefer_mutable: bool,
}
impl<'a, 'tcx> VarVisitor<'a, 'tcx> {
fn check(&mut self, idx: &'tcx Expr<'_>, seqexpr: &'tcx Expr<'_>, expr: &'tcx Expr<'_>) -> bool {
if_chain! {
// the indexed container is referenced by a name
if let ExprKind::Path(ref seqpath) = seqexpr.kind;
if let QPath::Resolved(None, ref seqvar) = *seqpath;
if seqvar.segments.len() == 1;
then {
let index_used_directly = path_to_local_id(idx, self.var);
let indexed_indirectly = {
let mut used_visitor = LocalUsedVisitor::new(self.cx, self.var);
walk_expr(&mut used_visitor, idx);
used_visitor.used
};
if indexed_indirectly || index_used_directly {
if self.prefer_mutable {
self.indexed_mut.insert(seqvar.segments[0].ident.name);
}
let res = self.cx.qpath_res(seqpath, seqexpr.hir_id);
match res {
Res::Local(hir_id) => {
let parent_id = self.cx.tcx.hir().get_parent_item(expr.hir_id);
let parent_def_id = self.cx.tcx.hir().local_def_id(parent_id);
let extent = self.cx.tcx.region_scope_tree(parent_def_id).var_scope(hir_id.local_id);
if indexed_indirectly {
self.indexed_indirectly.insert(seqvar.segments[0].ident.name, Some(extent));
}
if index_used_directly {
self.indexed_directly.insert(
seqvar.segments[0].ident.name,
(Some(extent), self.cx.typeck_results().node_type(seqexpr.hir_id)),
);
}
return false; // no need to walk further *on the variable*
}
Res::Def(DefKind::Static | DefKind::Const, ..) => {
if indexed_indirectly {
self.indexed_indirectly.insert(seqvar.segments[0].ident.name, None);
}
if index_used_directly {
self.indexed_directly.insert(
seqvar.segments[0].ident.name,
(None, self.cx.typeck_results().node_type(seqexpr.hir_id)),
);
}
return false; // no need to walk further *on the variable*
}
_ => (),
}
}
}
}
true
}
}
impl<'a, 'tcx> Visitor<'tcx> for VarVisitor<'a, 'tcx> {
type Map = Map<'tcx>;
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if_chain! {
// a range index op
if let ExprKind::MethodCall(ref meth, _, ref args, _) = expr.kind;
if (meth.ident.name == sym::index && match_trait_method(self.cx, expr, &paths::INDEX))
|| (meth.ident.name == sym::index_mut && match_trait_method(self.cx, expr, &paths::INDEX_MUT));
if !self.check(&args[1], &args[0], expr);
then { return }
}
if_chain! {
// an index op
if let ExprKind::Index(ref seqexpr, ref idx) = expr.kind;
if !self.check(idx, seqexpr, expr);
then { return }
}
if_chain! {
// directly using a variable
if let ExprKind::Path(QPath::Resolved(None, path)) = expr.kind;
if let Res::Local(local_id) = path.res;
then {
if local_id == self.var {
self.nonindex = true;
} else {
// not the correct variable, but still a variable
self.referenced.insert(path.segments[0].ident.name);
}
}
}
let old = self.prefer_mutable;
match expr.kind {
ExprKind::AssignOp(_, ref lhs, ref rhs) | ExprKind::Assign(ref lhs, ref rhs, _) => {
self.prefer_mutable = true;
self.visit_expr(lhs);
self.prefer_mutable = false;
self.visit_expr(rhs);
},
ExprKind::AddrOf(BorrowKind::Ref, mutbl, ref expr) => {
if mutbl == Mutability::Mut {
self.prefer_mutable = true;
}
self.visit_expr(expr);
},
ExprKind::Call(ref f, args) => {
self.visit_expr(f);
for expr in args {
let ty = self.cx.typeck_results().expr_ty_adjusted(expr);
self.prefer_mutable = false;
if let ty::Ref(_, _, mutbl) = *ty.kind() {
if mutbl == Mutability::Mut {
self.prefer_mutable = true;
}
}
self.visit_expr(expr);
}
},
ExprKind::MethodCall(_, _, args, _) => {
let def_id = self.cx.typeck_results().type_dependent_def_id(expr.hir_id).unwrap();
for (ty, expr) in self.cx.tcx.fn_sig(def_id).inputs().skip_binder().iter().zip(args) {
self.prefer_mutable = false;
if let ty::Ref(_, _, mutbl) = *ty.kind() {
if mutbl == Mutability::Mut {
self.prefer_mutable = true;
}
}
self.visit_expr(expr);
}
},
ExprKind::Closure(_, _, body_id, ..) => {
let body = self.cx.tcx.hir().body(body_id);
self.visit_expr(&body.value);
},
_ => walk_expr(self, expr),
}
self.prefer_mutable = old;
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
}

172
clippy_lints/src/loops/never_loop.rs Normal file
View file

@ -0,0 +1,172 @@
use super::NEVER_LOOP;
use crate::utils::span_lint;
use rustc_hir::{Block, Expr, ExprKind, HirId, InlineAsmOperand, Stmt, StmtKind};
use rustc_lint::LateContext;
use std::iter::{once, Iterator};
pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let ExprKind::Loop(ref block, _, _, _) = expr.kind {
match never_loop_block(block, expr.hir_id) {
NeverLoopResult::AlwaysBreak => span_lint(cx, NEVER_LOOP, expr.span, "this loop never actually loops"),
NeverLoopResult::MayContinueMainLoop | NeverLoopResult::Otherwise => (),
}
}
}
enum NeverLoopResult {
// A break/return always get triggered but not necessarily for the main loop.
AlwaysBreak,
// A continue may occur for the main loop.
MayContinueMainLoop,
Otherwise,
}
#[must_use]
fn absorb_break(arg: &NeverLoopResult) -> NeverLoopResult {
match *arg {
NeverLoopResult::AlwaysBreak | NeverLoopResult::Otherwise => NeverLoopResult::Otherwise,
NeverLoopResult::MayContinueMainLoop => NeverLoopResult::MayContinueMainLoop,
}
}
// Combine two results for parts that are called in order.
#[must_use]
fn combine_seq(first: NeverLoopResult, second: NeverLoopResult) -> NeverLoopResult {
match first {
NeverLoopResult::AlwaysBreak | NeverLoopResult::MayContinueMainLoop => first,
NeverLoopResult::Otherwise => second,
}
}
// Combine two results where both parts are called but not necessarily in order.
#[must_use]
fn combine_both(left: NeverLoopResult, right: NeverLoopResult) -> NeverLoopResult {
match (left, right) {
(NeverLoopResult::MayContinueMainLoop, _) | (_, NeverLoopResult::MayContinueMainLoop) => {
NeverLoopResult::MayContinueMainLoop
},
(NeverLoopResult::AlwaysBreak, _) | (_, NeverLoopResult::AlwaysBreak) => NeverLoopResult::AlwaysBreak,
(NeverLoopResult::Otherwise, NeverLoopResult::Otherwise) => NeverLoopResult::Otherwise,
}
}
// Combine two results where only one of the part may have been executed.
#[must_use]
fn combine_branches(b1: NeverLoopResult, b2: NeverLoopResult) -> NeverLoopResult {
match (b1, b2) {
(NeverLoopResult::AlwaysBreak, NeverLoopResult::AlwaysBreak) => NeverLoopResult::AlwaysBreak,
(NeverLoopResult::MayContinueMainLoop, _) | (_, NeverLoopResult::MayContinueMainLoop) => {
NeverLoopResult::MayContinueMainLoop
},
(NeverLoopResult::Otherwise, _) | (_, NeverLoopResult::Otherwise) => NeverLoopResult::Otherwise,
}
}
fn never_loop_block(block: &Block<'_>, main_loop_id: HirId) -> NeverLoopResult {
let stmts = block.stmts.iter().map(stmt_to_expr);
let expr = once(block.expr.as_deref());
let mut iter = stmts.chain(expr).flatten();
never_loop_expr_seq(&mut iter, main_loop_id)
}
fn never_loop_expr_seq<'a, T: Iterator<Item = &'a Expr<'a>>>(es: &mut T, main_loop_id: HirId) -> NeverLoopResult {
es.map(|e| never_loop_expr(e, main_loop_id))
.fold(NeverLoopResult::Otherwise, combine_seq)
}
fn stmt_to_expr<'tcx>(stmt: &Stmt<'tcx>) -> Option<&'tcx Expr<'tcx>> {
match stmt.kind {
StmtKind::Semi(ref e, ..) | StmtKind::Expr(ref e, ..) => Some(e),
StmtKind::Local(ref local) => local.init.as_deref(),
_ => None,
}
}
fn never_loop_expr(expr: &Expr<'_>, main_loop_id: HirId) -> NeverLoopResult {
match expr.kind {
ExprKind::Box(ref e)
| ExprKind::Unary(_, ref e)
| ExprKind::Cast(ref e, _)
| ExprKind::Type(ref e, _)
| ExprKind::Field(ref e, _)
| ExprKind::AddrOf(_, _, ref e)
| ExprKind::Struct(_, _, Some(ref e))
| ExprKind::Repeat(ref e, _)
| ExprKind::DropTemps(ref e) => never_loop_expr(e, main_loop_id),
ExprKind::Array(ref es) | ExprKind::MethodCall(_, _, ref es, _) | ExprKind::Tup(ref es) => {
never_loop_expr_all(&mut es.iter(), main_loop_id)
},
ExprKind::Call(ref e, ref es) => never_loop_expr_all(&mut once(&**e).chain(es.iter()), main_loop_id),
ExprKind::Binary(_, ref e1, ref e2)
| ExprKind::Assign(ref e1, ref e2, _)
| ExprKind::AssignOp(_, ref e1, ref e2)
| ExprKind::Index(ref e1, ref e2) => never_loop_expr_all(&mut [&**e1, &**e2].iter().cloned(), main_loop_id),
ExprKind::Loop(ref b, _, _, _) => {
// Break can come from the inner loop so remove them.
absorb_break(&never_loop_block(b, main_loop_id))
},
ExprKind::If(ref e, ref e2, ref e3) => {
let e1 = never_loop_expr(e, main_loop_id);
let e2 = never_loop_expr(e2, main_loop_id);
let e3 = e3
.as_ref()
.map_or(NeverLoopResult::Otherwise, |e| never_loop_expr(e, main_loop_id));
combine_seq(e1, combine_branches(e2, e3))
},
ExprKind::Match(ref e, ref arms, _) => {
let e = never_loop_expr(e, main_loop_id);
if arms.is_empty() {
e
} else {
let arms = never_loop_expr_branch(&mut arms.iter().map(|a| &*a.body), main_loop_id);
combine_seq(e, arms)
}
},
ExprKind::Block(ref b, _) => never_loop_block(b, main_loop_id),
ExprKind::Continue(d) => {
let id = d
.target_id
.expect("target ID can only be missing in the presence of compilation errors");
if id == main_loop_id {
NeverLoopResult::MayContinueMainLoop
} else {
NeverLoopResult::AlwaysBreak
}
},
ExprKind::Break(_, ref e) | ExprKind::Ret(ref e) => e.as_ref().map_or(NeverLoopResult::AlwaysBreak, |e| {
combine_seq(never_loop_expr(e, main_loop_id), NeverLoopResult::AlwaysBreak)
}),
ExprKind::InlineAsm(ref asm) => asm
.operands
.iter()
.map(|(o, _)| match o {
InlineAsmOperand::In { expr, .. }
| InlineAsmOperand::InOut { expr, .. }
| InlineAsmOperand::Const { expr }
| InlineAsmOperand::Sym { expr } => never_loop_expr(expr, main_loop_id),
InlineAsmOperand::Out { expr, .. } => never_loop_expr_all(&mut expr.iter(), main_loop_id),
InlineAsmOperand::SplitInOut { in_expr, out_expr, .. } => {
never_loop_expr_all(&mut once(in_expr).chain(out_expr.iter()), main_loop_id)
},
})
.fold(NeverLoopResult::Otherwise, combine_both),
ExprKind::Struct(_, _, None)
| ExprKind::Yield(_, _)
| ExprKind::Closure(_, _, _, _, _)
| ExprKind::LlvmInlineAsm(_)
| ExprKind::Path(_)
| ExprKind::ConstBlock(_)
| ExprKind::Lit(_)
| ExprKind::Err => NeverLoopResult::Otherwise,
}
}
fn never_loop_expr_all<'a, T: Iterator<Item = &'a Expr<'a>>>(es: &mut T, main_loop_id: HirId) -> NeverLoopResult {
es.map(|e| never_loop_expr(e, main_loop_id))
.fold(NeverLoopResult::Otherwise, combine_both)
}
fn never_loop_expr_branch<'a, T: Iterator<Item = &'a Expr<'a>>>(e: &mut T, main_loop_id: HirId) -> NeverLoopResult {
e.map(|e| never_loop_expr(e, main_loop_id))
.fold(NeverLoopResult::AlwaysBreak, combine_branches)
}

169
clippy_lints/src/loops/same_item_push.rs Normal file
View file

@ -0,0 +1,169 @@
use super::SAME_ITEM_PUSH;
use crate::utils::{implements_trait, is_type_diagnostic_item, snippet_with_macro_callsite, span_lint_and_help};
use if_chain::if_chain;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, Node, Pat, PatKind, Stmt, StmtKind};
use rustc_lint::LateContext;
use rustc_middle::hir::map::Map;
use rustc_span::symbol::sym;
use std::iter::Iterator;
/// Detects for loop pushing the same item into a Vec
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,
_: &'tcx Expr<'_>,
body: &'tcx Expr<'_>,
_: &'tcx Expr<'_>,
) {
fn emit_lint(cx: &LateContext<'_>, vec: &Expr<'_>, pushed_item: &Expr<'_>) {
let vec_str = snippet_with_macro_callsite(cx, vec.span, "");
let item_str = snippet_with_macro_callsite(cx, pushed_item.span, "");
span_lint_and_help(
cx,
SAME_ITEM_PUSH,
vec.span,
"it looks like the same item is being pushed into this Vec",
None,
&format!(
"try using vec![{};SIZE] or {}.resize(NEW_SIZE, {})",
item_str, vec_str, item_str
),
)
}
if !matches!(pat.kind, PatKind::Wild) {
return;
}
// Determine whether it is safe to lint the body
let mut same_item_push_visitor = SameItemPushVisitor {
should_lint: true,
vec_push: None,
cx,
};
walk_expr(&mut same_item_push_visitor, body);
if same_item_push_visitor.should_lint {
if let Some((vec, pushed_item)) = same_item_push_visitor.vec_push {
let vec_ty = cx.typeck_results().expr_ty(vec);
let ty = vec_ty.walk().nth(1).unwrap().expect_ty();
if cx
.tcx
.lang_items()
.clone_trait()
.map_or(false, |id| implements_trait(cx, ty, id, &[]))
{
// Make sure that the push does not involve possibly mutating values
match pushed_item.kind {
ExprKind::Path(ref qpath) => {
match cx.qpath_res(qpath, pushed_item.hir_id) {
// immutable bindings that are initialized with literal or constant
Res::Local(hir_id) => {
if_chain! {
let node = cx.tcx.hir().get(hir_id);
if let Node::Binding(pat) = node;
if let PatKind::Binding(bind_ann, ..) = pat.kind;
if !matches!(bind_ann, BindingAnnotation::RefMut | BindingAnnotation::Mutable);
let parent_node = cx.tcx.hir().get_parent_node(hir_id);
if let Some(Node::Local(parent_let_expr)) = cx.tcx.hir().find(parent_node);
if let Some(init) = parent_let_expr.init;
then {
match init.kind {
// immutable bindings that are initialized with literal
ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item),
// immutable bindings that are initialized with constant
ExprKind::Path(ref path) => {
if let Res::Def(DefKind::Const, ..) = cx.qpath_res(path, init.hir_id) {
emit_lint(cx, vec, pushed_item);
}
}
_ => {},
}
}
}
},
// constant
Res::Def(DefKind::Const, ..) => emit_lint(cx, vec, pushed_item),
_ => {},
}
},
ExprKind::Lit(..) => emit_lint(cx, vec, pushed_item),
_ => {},
}
}
}
}
}
// Scans the body of the for loop and determines whether lint should be given
struct SameItemPushVisitor<'a, 'tcx> {
should_lint: bool,
// this field holds the last vec push operation visited, which should be the only push seen
vec_push: Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)>,
cx: &'a LateContext<'tcx>,
}
impl<'a, 'tcx> Visitor<'tcx> for SameItemPushVisitor<'a, 'tcx> {
type Map = Map<'tcx>;
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
match &expr.kind {
// Non-determinism may occur ... don't give a lint
ExprKind::Loop(..) | ExprKind::Match(..) => self.should_lint = false,
ExprKind::Block(block, _) => self.visit_block(block),
_ => {},
}
}
fn visit_block(&mut self, b: &'tcx Block<'_>) {
for stmt in b.stmts.iter() {
self.visit_stmt(stmt);
}
}
fn visit_stmt(&mut self, s: &'tcx Stmt<'_>) {
let vec_push_option = get_vec_push(self.cx, s);
if vec_push_option.is_none() {
// Current statement is not a push so visit inside
match &s.kind {
StmtKind::Expr(expr) | StmtKind::Semi(expr) => self.visit_expr(&expr),
_ => {},
}
} else {
// Current statement is a push ...check whether another
// push had been previously done
if self.vec_push.is_none() {
self.vec_push = vec_push_option;
} else {
// There are multiple pushes ... don't lint
self.should_lint = false;
}
}
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
}
// Given some statement, determine if that statement is a push on a Vec. If it is, return
// the Vec being pushed into and the item being pushed
fn get_vec_push<'tcx>(cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) -> Option<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)> {
if_chain! {
// Extract method being called
if let StmtKind::Semi(semi_stmt) = &stmt.kind;
if let ExprKind::MethodCall(path, _, args, _) = &semi_stmt.kind;
// Figure out the parameters for the method call
if let Some(self_expr) = args.get(0);
if let Some(pushed_item) = args.get(1);
// Check that the method being called is push() on a Vec
if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(self_expr), sym::vec_type);
if path.ident.name.as_str() == "push";
then {
return Some((self_expr, pushed_item))
}
}
None
}

42
clippy_lints/src/loops/single_element_loop.rs Normal file
View file

@ -0,0 +1,42 @@
use super::{get_span_of_entire_for_loop, SINGLE_ELEMENT_LOOP};
use crate::utils::{indent_of, single_segment_path, snippet, span_lint_and_sugg};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{BorrowKind, Expr, ExprKind, Pat, PatKind};
use rustc_lint::LateContext;
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
pat: &'tcx Pat<'_>,
arg: &'tcx Expr<'_>,
body: &'tcx Expr<'_>,
expr: &'tcx Expr<'_>,
) {
if_chain! {
if let ExprKind::AddrOf(BorrowKind::Ref, _, ref arg_expr) = arg.kind;
if let PatKind::Binding(.., target, _) = pat.kind;
if let ExprKind::Array([arg_expression]) = arg_expr.kind;
if let ExprKind::Path(ref list_item) = arg_expression.kind;
if let Some(list_item_name) = single_segment_path(list_item).map(|ps| ps.ident.name);
if let ExprKind::Block(ref block, _) = body.kind;
if !block.stmts.is_empty();
then {
let for_span = get_span_of_entire_for_loop(expr);
let mut block_str = snippet(cx, block.span, "..").into_owned();
block_str.remove(0);
block_str.pop();
span_lint_and_sugg(
cx,
SINGLE_ELEMENT_LOOP,
for_span,
"for loop over a single element",
"try",
format!("{{\n{}let {} = &{};{}}}", " ".repeat(indent_of(cx, block.stmts[0].span).unwrap_or(0)), target.name, list_item_name, block_str),
Applicability::MachineApplicable
)
}
}
}

350
clippy_lints/src/loops/utils.rs Normal file
View file

@ -0,0 +1,350 @@
use crate::utils::{
get_parent_expr, get_trait_def_id, has_iter_method, implements_trait, is_integer_const, path_to_local,
path_to_local_id, paths, sugg,
};
use if_chain::if_chain;
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_expr, walk_pat, walk_stmt, NestedVisitorMap, Visitor};
use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, HirId, Mutability, Pat, PatKind, Stmt, StmtKind};
use rustc_lint::LateContext;
use rustc_middle::hir::map::Map;
use rustc_span::source_map::Span;
use rustc_span::symbol::Symbol;
use std::iter::Iterator;
#[derive(Debug, PartialEq)]
enum IncrementVisitorVarState {
Initial, // Not examined yet
IncrOnce, // Incremented exactly once, may be a loop counter
DontWarn,
}
/// Scan a for loop for variables that are incremented exactly once and not used after that.
pub(super) struct IncrementVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>, // context reference
states: FxHashMap<HirId, IncrementVisitorVarState>, // incremented variables
depth: u32, // depth of conditional expressions
done: bool,
}
impl<'a, 'tcx> IncrementVisitor<'a, 'tcx> {
pub(super) fn new(cx: &'a LateContext<'tcx>) -> Self {
Self {
cx,
states: FxHashMap::default(),
depth: 0,
done: false,
}
}
pub(super) fn into_results(self) -> impl Iterator<Item = HirId> {
self.states.into_iter().filter_map(|(id, state)| {
if state == IncrementVisitorVarState::IncrOnce {
Some(id)
} else {
None
}
})
}
}
impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> {
type Map = Map<'tcx>;
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if self.done {
return;
}
// If node is a variable
if let Some(def_id) = path_to_local(expr) {
if let Some(parent) = get_parent_expr(self.cx, expr) {
let state = self.states.entry(def_id).or_insert(IncrementVisitorVarState::Initial);
if *state == IncrementVisitorVarState::IncrOnce {
*state = IncrementVisitorVarState::DontWarn;
return;
}
match parent.kind {
ExprKind::AssignOp(op, ref lhs, ref rhs) => {
if lhs.hir_id == expr.hir_id {
*state = if op.node == BinOpKind::Add
&& is_integer_const(self.cx, rhs, 1)
&& *state == IncrementVisitorVarState::Initial
&& self.depth == 0
{
IncrementVisitorVarState::IncrOnce
} else {
// Assigned some other value or assigned multiple times
IncrementVisitorVarState::DontWarn
};
}
},
ExprKind::Assign(ref lhs, _, _) if lhs.hir_id == expr.hir_id => {
*state = IncrementVisitorVarState::DontWarn
},
ExprKind::AddrOf(BorrowKind::Ref, mutability, _) if mutability == Mutability::Mut => {
*state = IncrementVisitorVarState::DontWarn
},
_ => (),
}
}
walk_expr(self, expr);
} else if is_loop(expr) || is_conditional(expr) {
self.depth += 1;
walk_expr(self, expr);
self.depth -= 1;
} else if let ExprKind::Continue(_) = expr.kind {
self.done = true;
} else {
walk_expr(self, expr);
}
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
}
enum InitializeVisitorState<'hir> {
Initial, // Not examined yet
Declared(Symbol), // Declared but not (yet) initialized
Initialized {
name: Symbol,
initializer: &'hir Expr<'hir>,
},
DontWarn,
}
/// Checks whether a variable is initialized at the start of a loop and not modified
/// and used after the loop.
pub(super) struct InitializeVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>, // context reference
end_expr: &'tcx Expr<'tcx>, // the for loop. Stop scanning here.
var_id: HirId,
state: InitializeVisitorState<'tcx>,
depth: u32, // depth of conditional expressions
past_loop: bool,
}
impl<'a, 'tcx> InitializeVisitor<'a, 'tcx> {
pub(super) fn new(cx: &'a LateContext<'tcx>, end_expr: &'tcx Expr<'tcx>, var_id: HirId) -> Self {
Self {
cx,
end_expr,
var_id,
state: InitializeVisitorState::Initial,
depth: 0,
past_loop: false,
}
}
pub(super) fn get_result(&self) -> Option<(Symbol, &'tcx Expr<'tcx>)> {
if let InitializeVisitorState::Initialized { name, initializer } = self.state {
Some((name, initializer))
} else {
None
}
}
}
impl<'a, 'tcx> Visitor<'tcx> for InitializeVisitor<'a, 'tcx> {
type Map = Map<'tcx>;
fn visit_stmt(&mut self, stmt: &'tcx Stmt<'_>) {
// Look for declarations of the variable
if_chain! {
if let StmtKind::Local(ref local) = stmt.kind;
if local.pat.hir_id == self.var_id;
if let PatKind::Binding(.., ident, _) = local.pat.kind;
then {
self.state = local.init.map_or(InitializeVisitorState::Declared(ident.name), |init| {
InitializeVisitorState::Initialized {
initializer: init,
name: ident.name,
}
})
}
}
walk_stmt(self, stmt);
}
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if matches!(self.state, InitializeVisitorState::DontWarn) {
return;
}
if expr.hir_id == self.end_expr.hir_id {
self.past_loop = true;
return;
}
// No need to visit expressions before the variable is
// declared
if matches!(self.state, InitializeVisitorState::Initial) {
return;
}
// If node is the desired variable, see how it's used
if path_to_local_id(expr, self.var_id) {
if self.past_loop {
self.state = InitializeVisitorState::DontWarn;
return;
}
if let Some(parent) = get_parent_expr(self.cx, expr) {
match parent.kind {
ExprKind::AssignOp(_, ref lhs, _) if lhs.hir_id == expr.hir_id => {
self.state = InitializeVisitorState::DontWarn;
},
ExprKind::Assign(ref lhs, ref rhs, _) if lhs.hir_id == expr.hir_id => {
self.state = if_chain! {
if self.depth == 0;
if let InitializeVisitorState::Declared(name)
| InitializeVisitorState::Initialized { name, ..} = self.state;
then {
InitializeVisitorState::Initialized { initializer: rhs, name }
} else {
InitializeVisitorState::DontWarn
}
}
},
ExprKind::AddrOf(BorrowKind::Ref, mutability, _) if mutability == Mutability::Mut => {
self.state = InitializeVisitorState::DontWarn
},
_ => (),
}
}
walk_expr(self, expr);
} else if !self.past_loop && is_loop(expr) {
self.state = InitializeVisitorState::DontWarn;
} else if is_conditional(expr) {
self.depth += 1;
walk_expr(self, expr);
self.depth -= 1;
} else {
walk_expr(self, expr);
}
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::OnlyBodies(self.cx.tcx.hir())
}
}
fn is_loop(expr: &Expr<'_>) -> bool {
matches!(expr.kind, ExprKind::Loop(..))
}
fn is_conditional(expr: &Expr<'_>) -> bool {
matches!(expr.kind, ExprKind::If(..) | ExprKind::Match(..))
}
#[derive(PartialEq, Eq)]
pub(super) enum Nesting {
Unknown, // no nesting detected yet
RuledOut, // the iterator is initialized or assigned within scope
LookFurther, // no nesting detected, no further walk required
}
use self::Nesting::{LookFurther, RuledOut, Unknown};
pub(super) struct LoopNestVisitor {
pub(super) hir_id: HirId,
pub(super) iterator: HirId,
pub(super) nesting: Nesting,
}
impl<'tcx> Visitor<'tcx> for LoopNestVisitor {
type Map = Map<'tcx>;
fn visit_stmt(&mut self, stmt: &'tcx Stmt<'_>) {
if stmt.hir_id == self.hir_id {
self.nesting = LookFurther;
} else if self.nesting == Unknown {
walk_stmt(self, stmt);
}
}
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if self.nesting != Unknown {
return;
}
if expr.hir_id == self.hir_id {
self.nesting = LookFurther;
return;
}
match expr.kind {
ExprKind::Assign(ref path, _, _) | ExprKind::AssignOp(_, ref path, _) => {
if path_to_local_id(path, self.iterator) {
self.nesting = RuledOut;
}
},
_ => walk_expr(self, expr),
}
}
fn visit_pat(&mut self, pat: &'tcx Pat<'_>) {
if self.nesting != Unknown {
return;
}
if let PatKind::Binding(_, id, ..) = pat.kind {
if id == self.iterator {
self.nesting = RuledOut;
return;
}
}
walk_pat(self, pat)
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
}
// this function assumes the given expression is a `for` loop.
pub(super) fn get_span_of_entire_for_loop(expr: &Expr<'_>) -> Span {
// for some reason this is the only way to get the `Span`
// of the entire `for` loop
if let ExprKind::Match(_, arms, _) = &expr.kind {
arms[0].body.span
} else {
unreachable!()
}
}
/// If `arg` was the argument to a `for` loop, return the "cleanest" way of writing the
/// actual `Iterator` that the loop uses.
pub(super) fn make_iterator_snippet(cx: &LateContext<'_>, arg: &Expr<'_>, applic_ref: &mut Applicability) -> String {
let impls_iterator = get_trait_def_id(cx, &paths::ITERATOR).map_or(false, |id| {
implements_trait(cx, cx.typeck_results().expr_ty(arg), id, &[])
});
if impls_iterator {
format!(
"{}",
sugg::Sugg::hir_with_applicability(cx, arg, "_", applic_ref).maybe_par()
)
} else {
// (&x).into_iter() ==> x.iter()
// (&mut x).into_iter() ==> x.iter_mut()
match &arg.kind {
ExprKind::AddrOf(BorrowKind::Ref, mutability, arg_inner)
if has_iter_method(cx, cx.typeck_results().expr_ty(&arg_inner)).is_some() =>
{
let meth_name = match mutability {
Mutability::Mut => "iter_mut",
Mutability::Not => "iter",
};
format!(
"{}.{}()",
sugg::Sugg::hir_with_applicability(cx, &arg_inner, "_", applic_ref).maybe_par(),
meth_name,
)
}
_ => format!(
"{}.into_iter()",
sugg::Sugg::hir_with_applicability(cx, arg, "_", applic_ref).maybe_par()
),
}
}
}

139
clippy_lints/src/loops/while_immutable_condition.rs Normal file
View file

@ -0,0 +1,139 @@
use super::WHILE_IMMUTABLE_CONDITION;
use crate::consts::constant;
use crate::utils::span_lint_and_then;
use crate::utils::usage::mutated_variables;
use if_chain::if_chain;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_hir::def::{DefKind, Res};
use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::{def_id, Expr, ExprKind, HirId, QPath};
use rustc_lint::LateContext;
use rustc_middle::hir::map::Map;
use std::iter::Iterator;
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, cond: &'tcx Expr<'_>, expr: &'tcx Expr<'_>) {
if constant(cx, cx.typeck_results(), cond).is_some() {
// A pure constant condition (e.g., `while false`) is not linted.
return;
}
let mut var_visitor = VarCollectorVisitor {
cx,
ids: FxHashSet::default(),
def_ids: FxHashMap::default(),
skip: false,
};
var_visitor.visit_expr(cond);
if var_visitor.skip {
return;
}
let used_in_condition = &var_visitor.ids;
let no_cond_variable_mutated = if let Some(used_mutably) = mutated_variables(expr, cx) {
used_in_condition.is_disjoint(&used_mutably)
} else {
return;
};
let mutable_static_in_cond = var_visitor.def_ids.iter().any(|(_, v)| *v);
let mut has_break_or_return_visitor = HasBreakOrReturnVisitor {
has_break_or_return: false,
};
has_break_or_return_visitor.visit_expr(expr);
let has_break_or_return = has_break_or_return_visitor.has_break_or_return;
if no_cond_variable_mutated && !mutable_static_in_cond {
span_lint_and_then(
cx,
WHILE_IMMUTABLE_CONDITION,
cond.span,
"variables in the condition are not mutated in the loop body",
|diag| {
diag.note("this may lead to an infinite or to a never running loop");
if has_break_or_return {
diag.note("this loop contains `return`s or `break`s");
diag.help("rewrite it as `if cond { loop { } }`");
}
},
);
}
}
struct HasBreakOrReturnVisitor {
has_break_or_return: bool,
}
impl<'tcx> Visitor<'tcx> for HasBreakOrReturnVisitor {
type Map = Map<'tcx>;
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if self.has_break_or_return {
return;
}
match expr.kind {
ExprKind::Ret(_) | ExprKind::Break(_, _) => {
self.has_break_or_return = true;
return;
},
_ => {},
}
walk_expr(self, expr);
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
}
/// Collects the set of variables in an expression
/// Stops analysis if a function call is found
/// Note: In some cases such as `self`, there are no mutable annotation,
/// All variables definition IDs are collected
struct VarCollectorVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
ids: FxHashSet<HirId>,
def_ids: FxHashMap<def_id::DefId, bool>,
skip: bool,
}
impl<'a, 'tcx> VarCollectorVisitor<'a, 'tcx> {
fn insert_def_id(&mut self, ex: &'tcx Expr<'_>) {
if_chain! {
if let ExprKind::Path(ref qpath) = ex.kind;
if let QPath::Resolved(None, _) = *qpath;
let res = self.cx.qpath_res(qpath, ex.hir_id);
then {
match res {
Res::Local(hir_id) => {
self.ids.insert(hir_id);
},
Res::Def(DefKind::Static, def_id) => {
let mutable = self.cx.tcx.is_mutable_static(def_id);
self.def_ids.insert(def_id, mutable);
},
_ => {},
}
}
}
}
}
impl<'a, 'tcx> Visitor<'tcx> for VarCollectorVisitor<'a, 'tcx> {
type Map = Map<'tcx>;
fn visit_expr(&mut self, ex: &'tcx Expr<'_>) {
match ex.kind {
ExprKind::Path(_) => self.insert_def_id(ex),
// If there is any function/method call… we just stop analysis
ExprKind::Call(..) | ExprKind::MethodCall(..) => self.skip = true,
_ => walk_expr(self, ex),
}
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
}

87
clippy_lints/src/loops/while_let_loop.rs Normal file
View file

@ -0,0 +1,87 @@
use super::WHILE_LET_LOOP;
use crate::utils::{snippet_with_applicability, span_lint_and_sugg};
use rustc_errors::Applicability;
use rustc_hir::{Block, Expr, ExprKind, MatchSource, StmtKind};
use rustc_lint::{LateContext, LintContext};
use rustc_middle::lint::in_external_macro;
pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, loop_block: &'tcx Block<'_>) {
// extract the expression from the first statement (if any) in a block
let inner_stmt_expr = extract_expr_from_first_stmt(loop_block);
// or extract the first expression (if any) from the block
if let Some(inner) = inner_stmt_expr.or_else(|| extract_first_expr(loop_block)) {
if let ExprKind::Match(ref matchexpr, ref arms, ref source) = inner.kind {
// ensure "if let" compatible match structure
match *source {
MatchSource::Normal | MatchSource::IfLetDesugar { .. } => {
if arms.len() == 2
&& arms[0].guard.is_none()
&& arms[1].guard.is_none()
&& is_simple_break_expr(&arms[1].body)
{
if in_external_macro(cx.sess(), expr.span) {
return;
}
// NOTE: we used to build a body here instead of using
// ellipsis, this was removed because:
// 1) it was ugly with big bodies;
// 2) it was not indented properly;
// 3) it wasn’t very smart (see #675).
let mut applicability = Applicability::HasPlaceholders;
span_lint_and_sugg(
cx,
WHILE_LET_LOOP,
expr.span,
"this loop could be written as a `while let` loop",
"try",
format!(
"while let {} = {} {{ .. }}",
snippet_with_applicability(cx, arms[0].pat.span, "..", &mut applicability),
snippet_with_applicability(cx, matchexpr.span, "..", &mut applicability),
),
applicability,
);
}
},
_ => (),
}
}
}
}
/// If a block begins with a statement (possibly a `let` binding) and has an
/// expression, return it.
fn extract_expr_from_first_stmt<'tcx>(block: &Block<'tcx>) -> Option<&'tcx Expr<'tcx>> {
if block.stmts.is_empty() {
return None;
}
if let StmtKind::Local(ref local) = block.stmts[0].kind {
local.init //.map(|expr| expr)
} else {
None
}
}
/// If a block begins with an expression (with or without semicolon), return it.
fn extract_first_expr<'tcx>(block: &Block<'tcx>) -> Option<&'tcx Expr<'tcx>> {
match block.expr {
Some(ref expr) if block.stmts.is_empty() => Some(expr),
None if !block.stmts.is_empty() => match block.stmts[0].kind {
StmtKind::Expr(ref expr) | StmtKind::Semi(ref expr) => Some(expr),
StmtKind::Local(..) | StmtKind::Item(..) => None,
},
_ => None,
}
}
/// Returns `true` if expr contains a single break expr without destination label
/// and
/// passed expression. The expression may be within a block.
fn is_simple_break_expr(expr: &Expr<'_>) -> bool {
match expr.kind {
ExprKind::Break(dest, ref passed_expr) if dest.label.is_none() && passed_expr.is_none() => true,
ExprKind::Block(ref b, _) => extract_first_expr(b).map_or(false, |subexpr| is_simple_break_expr(subexpr)),
_ => false,
}
}

171
clippy_lints/src/loops/while_let_on_iterator.rs Normal file
View file

@ -0,0 +1,171 @@
use super::utils::{LoopNestVisitor, Nesting};
use super::WHILE_LET_ON_ITERATOR;
use crate::utils::usage::mutated_variables;
use crate::utils::{
get_enclosing_block, get_trait_def_id, implements_trait, is_refutable, last_path_segment, match_trait_method,
path_to_local, path_to_local_id, paths, snippet_with_applicability, span_lint_and_sugg,
};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::intravisit::{walk_block, walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::{Expr, ExprKind, HirId, MatchSource, Node, PatKind};
use rustc_lint::LateContext;
use rustc_middle::hir::map::Map;
use rustc_span::symbol::sym;
pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let ExprKind::Match(ref match_expr, ref arms, MatchSource::WhileLetDesugar) = expr.kind {
let pat = &arms[0].pat.kind;
if let (
&PatKind::TupleStruct(ref qpath, ref pat_args, _),
&ExprKind::MethodCall(ref method_path, _, ref method_args, _),
) = (pat, &match_expr.kind)
{
let iter_expr = &method_args[0];
// Don't lint when the iterator is recreated on every iteration
if_chain! {
if let ExprKind::MethodCall(..) | ExprKind::Call(..) = iter_expr.kind;
if let Some(iter_def_id) = get_trait_def_id(cx, &paths::ITERATOR);
if implements_trait(cx, cx.typeck_results().expr_ty(iter_expr), iter_def_id, &[]);
then {
return;
}
}
let lhs_constructor = last_path_segment(qpath);
if method_path.ident.name == sym::next
&& match_trait_method(cx, match_expr, &paths::ITERATOR)
&& lhs_constructor.ident.name == sym::Some
&& (pat_args.is_empty()
|| !is_refutable(cx, &pat_args[0])
&& !is_used_inside(cx, iter_expr, &arms[0].body)
&& !is_iterator_used_after_while_let(cx, iter_expr)
&& !is_nested(cx, expr, &method_args[0]))
{
let mut applicability = Applicability::MachineApplicable;
let iterator = snippet_with_applicability(cx, method_args[0].span, "_", &mut applicability);
let loop_var = if pat_args.is_empty() {
"_".to_string()
} else {
snippet_with_applicability(cx, pat_args[0].span, "_", &mut applicability).into_owned()
};
span_lint_and_sugg(
cx,
WHILE_LET_ON_ITERATOR,
expr.span.with_hi(match_expr.span.hi()),
"this loop could be written as a `for` loop",
"try",
format!("for {} in {}", loop_var, iterator),
applicability,
);
}
}
}
}
fn is_used_inside<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, container: &'tcx Expr<'_>) -> bool {
let def_id = match path_to_local(expr) {
Some(id) => id,
None => return false,
};
if let Some(used_mutably) = mutated_variables(container, cx) {
if used_mutably.contains(&def_id) {
return true;
}
}
false
}
fn is_iterator_used_after_while_let<'tcx>(cx: &LateContext<'tcx>, iter_expr: &'tcx Expr<'_>) -> bool {
let def_id = match path_to_local(iter_expr) {
Some(id) => id,
None => return false,
};
let mut visitor = VarUsedAfterLoopVisitor {
def_id,
iter_expr_id: iter_expr.hir_id,
past_while_let: false,
var_used_after_while_let: false,
};
if let Some(enclosing_block) = get_enclosing_block(cx, def_id) {
walk_block(&mut visitor, enclosing_block);
}
visitor.var_used_after_while_let
}
fn is_nested(cx: &LateContext<'_>, match_expr: &Expr<'_>, iter_expr: &Expr<'_>) -> bool {
if_chain! {
if let Some(loop_block) = get_enclosing_block(cx, match_expr.hir_id);
let parent_node = cx.tcx.hir().get_parent_node(loop_block.hir_id);
if let Some(Node::Expr(loop_expr)) = cx.tcx.hir().find(parent_node);
then {
return is_loop_nested(cx, loop_expr, iter_expr)
}
}
false
}
fn is_loop_nested(cx: &LateContext<'_>, loop_expr: &Expr<'_>, iter_expr: &Expr<'_>) -> bool {
let mut id = loop_expr.hir_id;
let iter_id = if let Some(id) = path_to_local(iter_expr) {
id
} else {
return true;
};
loop {
let parent = cx.tcx.hir().get_parent_node(id);
if parent == id {
return false;
}
match cx.tcx.hir().find(parent) {
Some(Node::Expr(expr)) => {
if let ExprKind::Loop(..) = expr.kind {
return true;
};
},
Some(Node::Block(block)) => {
let mut block_visitor = LoopNestVisitor {
hir_id: id,
iterator: iter_id,
nesting: Nesting::Unknown,
};
walk_block(&mut block_visitor, block);
if block_visitor.nesting == Nesting::RuledOut {
return false;
}
},
Some(Node::Stmt(_)) => (),
_ => {
return false;
},
}
id = parent;
}
}
struct VarUsedAfterLoopVisitor {
def_id: HirId,
iter_expr_id: HirId,
past_while_let: bool,
var_used_after_while_let: bool,
}
impl<'tcx> Visitor<'tcx> for VarUsedAfterLoopVisitor {
type Map = Map<'tcx>;
fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
if self.past_while_let {
if path_to_local_id(expr, self.def_id) {
self.var_used_after_while_let = true;
}
} else if self.iter_expr_id == expr.hir_id {
self.past_while_let = true;
}
walk_expr(self, expr);
}
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
}

118
clippy_lints/src/manual_map.rs
View file

@ -2,17 +2,25 @@ use crate::{
map_unit_fn::OPTION_MAP_UNIT_FN,
matches::MATCH_AS_REF,
utils::{
is_allowed, is_type_diagnostic_item, match_def_path, match_var, paths, peel_hir_expr_refs,
peel_mid_ty_refs_is_mutable, snippet_with_applicability, span_lint_and_sugg,
can_partially_move_ty, is_allowed, is_type_diagnostic_item, match_def_path, match_var, paths,
peel_hir_expr_refs, peel_mid_ty_refs_is_mutable, snippet_with_applicability, snippet_with_context,
span_lint_and_sugg,
},
};
use rustc_ast::util::parser::PREC_POSTFIX;
use rustc_errors::Applicability;
use rustc_hir::{Arm, BindingAnnotation, Block, Expr, ExprKind, Mutability, Pat, PatKind, QPath};
use rustc_hir::{
def::Res,
intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor},
Arm, BindingAnnotation, Block, Expr, ExprKind, Mutability, Pat, PatKind, Path, QPath,
};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::symbol::{sym, Ident};
use rustc_span::{
symbol::{sym, Ident},
SyntaxContext,
};
declare_clippy_lint! {
/// **What it does:** Checks for usages of `match` which could be implemented using `map`
@ -52,14 +60,17 @@ impl LateLintPass<'_> for ManualMap {
{
let (scrutinee_ty, ty_ref_count, ty_mutability) =
peel_mid_ty_refs_is_mutable(cx.typeck_results().expr_ty(scrutinee));
if !is_type_diagnostic_item(cx, scrutinee_ty, sym::option_type)
|| !is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::option_type)
if !(is_type_diagnostic_item(cx, scrutinee_ty, sym::option_type)
&& is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::option_type))
{
return;
}
let (some_expr, some_pat, pat_ref_count, is_wild_none) =
match (try_parse_pattern(cx, arm1.pat), try_parse_pattern(cx, arm2.pat)) {
let expr_ctxt = expr.span.ctxt();
let (some_expr, some_pat, pat_ref_count, is_wild_none) = match (
try_parse_pattern(cx, arm1.pat, expr_ctxt),
try_parse_pattern(cx, arm2.pat, expr_ctxt),
) {
(Some(OptionPat::Wild), Some(OptionPat::Some { pattern, ref_count }))
if is_none_expr(cx, arm1.body) =>
{
@ -88,7 +99,7 @@ impl LateLintPass<'_> for ManualMap {
return;
}
let some_expr = match get_some_expr(cx, some_expr) {
let some_expr = match get_some_expr(cx, some_expr, expr_ctxt) {
Some(expr) => expr,
None => return,
};
@ -99,6 +110,10 @@ impl LateLintPass<'_> for ManualMap {
return;
}
if !can_move_expr_to_closure(cx, some_expr) {
return;
}
// Determine which binding mode to use.
let explicit_ref = some_pat.contains_explicit_ref_binding();
let binding_ref = explicit_ref.or_else(|| (ty_ref_count != pat_ref_count).then(|| ty_mutability));
@ -111,21 +126,23 @@ impl LateLintPass<'_> for ManualMap {
let mut app = Applicability::MachineApplicable;
// Remove address-of expressions from the scrutinee. `as_ref` will be called,
// the type is copyable, or the option is being passed by value.
// Remove address-of expressions from the scrutinee. Either `as_ref` will be called, or
// it's being passed by value.
let scrutinee = peel_hir_expr_refs(scrutinee).0;
let scrutinee_str = snippet_with_applicability(cx, scrutinee.span, "_", &mut app);
let scrutinee_str = if expr.precedence().order() < PREC_POSTFIX {
// Parens are needed to chain method calls.
let scrutinee_str = snippet_with_context(cx, scrutinee.span, expr_ctxt, "..", &mut app);
let scrutinee_str =
if scrutinee.span.ctxt() == expr.span.ctxt() && scrutinee.precedence().order() < PREC_POSTFIX {
format!("({})", scrutinee_str)
} else {
scrutinee_str.into()
};
let body_str = if let PatKind::Binding(annotation, _, some_binding, None) = some_pat.kind {
if let Some(func) = can_pass_as_func(cx, some_binding, some_expr) {
match can_pass_as_func(cx, some_binding, some_expr) {
Some(func) if func.span.ctxt() == some_expr.span.ctxt() => {
snippet_with_applicability(cx, func.span, "..", &mut app).into_owned()
} else {
},
_ => {
if match_var(some_expr, some_binding.name)
&& !is_allowed(cx, MATCH_AS_REF, expr.hir_id)
&& binding_ref.is_some()
@ -143,15 +160,16 @@ impl LateLintPass<'_> for ManualMap {
"|{}{}| {}",
annotation,
some_binding,
snippet_with_applicability(cx, some_expr.span, "..", &mut app)
snippet_with_context(cx, some_expr.span, expr_ctxt, "..", &mut app)
)
},
}
} else if !is_wild_none && explicit_ref.is_none() {
// TODO: handle explicit reference annotations.
format!(
"|{}| {}",
snippet_with_applicability(cx, some_pat.span, "..", &mut app),
snippet_with_applicability(cx, some_expr.span, "..", &mut app)
snippet_with_context(cx, some_pat.span, expr_ctxt, "..", &mut app),
snippet_with_context(cx, some_expr.span, expr_ctxt, "..", &mut app)
)
} else {
// Refutable bindings and mixed reference annotations can't be handled by `map`.
@ -171,6 +189,51 @@ impl LateLintPass<'_> for ManualMap {
}
}
// Checks if the expression can be moved into a closure as is.
fn can_move_expr_to_closure(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> bool {
struct V<'cx, 'tcx> {
cx: &'cx LateContext<'tcx>,
make_closure: bool,
}
impl Visitor<'tcx> for V<'_, 'tcx> {
type Map = ErasedMap<'tcx>;
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::None
}
fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
match e.kind {
ExprKind::Break(..)
| ExprKind::Continue(_)
| ExprKind::Ret(_)
| ExprKind::Yield(..)
| ExprKind::InlineAsm(_)
| ExprKind::LlvmInlineAsm(_) => {
self.make_closure = false;
},
// Accessing a field of a local value can only be done if the type isn't
// partially moved.
ExprKind::Field(base_expr, _)
if matches!(
base_expr.kind,
ExprKind::Path(QPath::Resolved(_, Path { res: Res::Local(_), .. }))
) && can_partially_move_ty(self.cx, self.cx.typeck_results().expr_ty(base_expr)) =>
{
// TODO: check if the local has been partially moved. Assume it has for now.
self.make_closure = false;
return;
}
_ => (),
};
walk_expr(self, e);
}
}
let mut v = V { cx, make_closure: true };
v.visit_expr(expr);
v.make_closure
}
// Checks whether the expression could be passed as a function, or whether a closure is needed.
// Returns the function to be passed to `map` if it exists.
fn can_pass_as_func(cx: &LateContext<'tcx>, binding: Ident, expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
@ -198,11 +261,11 @@ enum OptionPat<'a> {
// Try to parse into a recognized `Option` pattern.
// i.e. `_`, `None`, `Some(..)`, or a reference to any of those.
fn try_parse_pattern(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) -> Option<OptionPat<'tcx>> {
fn f(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, ref_count: usize) -> Option<OptionPat<'tcx>> {
fn try_parse_pattern(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, ctxt: SyntaxContext) -> Option<OptionPat<'tcx>> {
fn f(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, ref_count: usize, ctxt: SyntaxContext) -> Option<OptionPat<'tcx>> {
match pat.kind {
PatKind::Wild => Some(OptionPat::Wild),
PatKind::Ref(pat, _) => f(cx, pat, ref_count + 1),
PatKind::Ref(pat, _) => f(cx, pat, ref_count + 1, ctxt),
PatKind::Path(QPath::Resolved(None, path))
if path
.res
@ -215,18 +278,19 @@ fn try_parse_pattern(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) -> Option<Optio
if path
.res
.opt_def_id()
.map_or(false, |id| match_def_path(cx, id, &paths::OPTION_SOME)) =>
.map_or(false, |id| match_def_path(cx, id, &paths::OPTION_SOME))
&& pat.span.ctxt() == ctxt =>
{
Some(OptionPat::Some { pattern, ref_count })
},
_ => None,
}
}
f(cx, pat, 0)
f(cx, pat, 0, ctxt)
}
// Checks for an expression wrapped by the `Some` constructor. Returns the contained expression.
fn get_some_expr(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
fn get_some_expr(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, ctxt: SyntaxContext) -> Option<&'tcx Expr<'tcx>> {
// TODO: Allow more complex expressions.
match expr.kind {
ExprKind::Call(
@ -235,7 +299,7 @@ fn get_some_expr(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<&'tcx E
..
},
[arg],
) => {
) if ctxt == expr.span.ctxt() => {
if match_def_path(cx, path.res.opt_def_id()?, &paths::OPTION_SOME) {
Some(arg)
} else {
@ -249,7 +313,7 @@ fn get_some_expr(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<&'tcx E
..
},
_,
) => get_some_expr(cx, expr),
) => get_some_expr(cx, expr, ctxt),
_ => None,
}
}

4
clippy_lints/src/map_clone.rs
View file

@ -79,7 +79,9 @@ impl<'tcx> LateLintPass<'tcx> for MapClone {
},
hir::ExprKind::MethodCall(ref method, _, [obj], _) => if_chain! {
if ident_eq(name, obj) && method.ident.name == sym::clone;
if match_trait_method(cx, closure_expr, &paths::CLONE_TRAIT);
if let Some(fn_id) = cx.typeck_results().type_dependent_def_id(closure_expr.hir_id);
if let Some(trait_id) = cx.tcx.trait_of_item(fn_id);
if cx.tcx.lang_items().clone_trait().map_or(false, |id| id == trait_id);
// no autoderefs
if !cx.typeck_results().expr_adjustments(obj).iter()
.any(|a| matches!(a.kind, Adjust::Deref(Some(..))));

4
clippy_lints/src/matches.rs
View file

@ -1173,9 +1173,9 @@ fn check_wild_in_or_pats(cx: &LateContext<'_>, arms: &[Arm<'_>]) {
cx,
WILDCARD_IN_OR_PATTERNS,
arm.pat.span,
"wildcard pattern covers any other pattern as it will match anyway.",
"wildcard pattern covers any other pattern as it will match anyway",
None,
"Consider handling `_` separately.",
"consider handling `_` separately",
);
}
}

2
clippy_lints/src/methods/bind_instead_of_map.rs
View file

@ -158,7 +158,7 @@ pub(crate) trait BindInsteadOfMap {
}
/// Lint use of `_.and_then(|x| Some(y))` for `Option`s
fn lint(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) -> bool {
fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) -> bool {
if !match_type(cx, cx.typeck_results().expr_ty(&args[0]), Self::TYPE_QPATH) {
return false;
}

2
clippy_lints/src/methods/bytes_nth.rs
View file

@ -7,7 +7,7 @@ use rustc_span::sym;
use super::BYTES_NTH;
pub(super) fn lints<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, iter_args: &'tcx [Expr<'tcx>]) {
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, iter_args: &'tcx [Expr<'tcx>]) {
if_chain! {
if let ExprKind::MethodCall(_, _, ref args, _) = expr.kind;
let ty = cx.typeck_results().expr_ty(&iter_args[0]).peel_refs();

109
clippy_lints/src/methods/clone_on_copy.rs Normal file
View file

@ -0,0 +1,109 @@
use crate::utils::{is_copy, span_lint_and_then, sugg};
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty};
use std::iter;
use super::CLONE_DOUBLE_REF;
use super::CLONE_ON_COPY;
/// Checks for the `CLONE_ON_COPY` lint.
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr<'_>, arg_ty: Ty<'_>) {
let ty = cx.typeck_results().expr_ty(expr);
if let ty::Ref(_, inner, _) = arg_ty.kind() {
if let ty::Ref(_, innermost, _) = inner.kind() {
span_lint_and_then(
cx,
CLONE_DOUBLE_REF,
expr.span,
&format!(
"using `clone` on a double-reference; \
this will copy the reference of type `{}` instead of cloning the inner type",
ty
),
|diag| {
if let Some(snip) = sugg::Sugg::hir_opt(cx, arg) {
let mut ty = innermost;
let mut n = 0;
while let ty::Ref(_, inner, _) = ty.kind() {
ty = inner;
n += 1;
}
let refs: String = iter::repeat('&').take(n + 1).collect();
let derefs: String = iter::repeat('*').take(n).collect();
let explicit = format!("<{}{}>::clone({})", refs, ty, snip);
diag.span_suggestion(
expr.span,
"try dereferencing it",
format!("{}({}{}).clone()", refs, derefs, snip.deref()),
Applicability::MaybeIncorrect,
);
diag.span_suggestion(
expr.span,
"or try being explicit if you are sure, that you want to clone a reference",
explicit,
Applicability::MaybeIncorrect,
);
}
},
);
return; // don't report clone_on_copy
}
}
if is_copy(cx, ty) {
let snip;
if let Some(snippet) = sugg::Sugg::hir_opt(cx, arg) {
let parent = cx.tcx.hir().get_parent_node(expr.hir_id);
match &cx.tcx.hir().get(parent) {
hir::Node::Expr(parent) => match parent.kind {
// &*x is a nop, &x.clone() is not
hir::ExprKind::AddrOf(..) => return,
// (*x).func() is useless, x.clone().func() can work in case func borrows mutably
hir::ExprKind::MethodCall(_, _, parent_args, _) if expr.hir_id == parent_args[0].hir_id => {
return;
},
_ => {},
},
hir::Node::Stmt(stmt) => {
if let hir::StmtKind::Local(ref loc) = stmt.kind {
if let hir::PatKind::Ref(..) = loc.pat.kind {
// let ref y = *x borrows x, let ref y = x.clone() does not
return;
}
}
},
_ => {},
}
// x.clone() might have dereferenced x, possibly through Deref impls
if cx.typeck_results().expr_ty(arg) == ty {
snip = Some(("try removing the `clone` call", format!("{}", snippet)));
} else {
let deref_count = cx
.typeck_results()
.expr_adjustments(arg)
.iter()
.filter(|adj| matches!(adj.kind, ty::adjustment::Adjust::Deref(_)))
.count();
let derefs: String = iter::repeat('*').take(deref_count).collect();
snip = Some(("try dereferencing it", format!("{}{}", derefs, snippet)));
}
} else {
snip = None;
}
span_lint_and_then(
cx,
CLONE_ON_COPY,
expr.span,
&format!("using `clone` on type `{}` which implements the `Copy` trait", ty),
|diag| {
if let Some((text, snip)) = snip {
diag.span_suggestion(expr.span, text, snip, Applicability::MachineApplicable);
}
},
);
}
}

36
clippy_lints/src/methods/clone_on_ref_ptr.rs Normal file
View file

@ -0,0 +1,36 @@
use crate::utils::{is_type_diagnostic_item, match_type, paths, snippet_with_macro_callsite, span_lint_and_sugg};
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_span::symbol::sym;
use super::CLONE_ON_REF_PTR;
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr<'_>) {
let obj_ty = cx.typeck_results().expr_ty(arg).peel_refs();
if let ty::Adt(_, subst) = obj_ty.kind() {
let caller_type = if is_type_diagnostic_item(cx, obj_ty, sym::Rc) {
"Rc"
} else if is_type_diagnostic_item(cx, obj_ty, sym::Arc) {
"Arc"
} else if match_type(cx, obj_ty, &paths::WEAK_RC) || match_type(cx, obj_ty, &paths::WEAK_ARC) {
"Weak"
} else {
return;
};
let snippet = snippet_with_macro_callsite(cx, arg.span, "..");
span_lint_and_sugg(
cx,
CLONE_ON_REF_PTR,
expr.span,
"using `.clone()` on a ref-counted pointer",
"try this",
format!("{}::<{}>::clone(&{})", caller_type, subst.type_at(0), snippet),
Applicability::Unspecified, // Sometimes unnecessary ::<_> after Rc/Arc/Weak
);
}
}

199
clippy_lints/src/methods/expect_fun_call.rs Normal file
View file

@ -0,0 +1,199 @@
use crate::utils::{is_expn_of, is_type_diagnostic_item, snippet, snippet_with_applicability, span_lint_and_sugg};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_span::source_map::Span;
use rustc_span::symbol::sym;
use std::borrow::Cow;
use super::EXPECT_FUN_CALL;
/// Checks for the `EXPECT_FUN_CALL` lint.
#[allow(clippy::too_many_lines)]
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, method_span: Span, name: &str, args: &[hir::Expr<'_>]) {
// Strip `&`, `as_ref()` and `as_str()` off `arg` until we're left with either a `String` or
// `&str`
fn get_arg_root<'a>(cx: &LateContext<'_>, arg: &'a hir::Expr<'a>) -> &'a hir::Expr<'a> {
let mut arg_root = arg;
loop {
arg_root = match &arg_root.kind {
hir::ExprKind::AddrOf(hir::BorrowKind::Ref, _, expr) => expr,
hir::ExprKind::MethodCall(method_name, _, call_args, _) => {
if call_args.len() == 1
&& (method_name.ident.name == sym::as_str || method_name.ident.name == sym!(as_ref))
&& {
let arg_type = cx.typeck_results().expr_ty(&call_args[0]);
let base_type = arg_type.peel_refs();
*base_type.kind() == ty::Str || is_type_diagnostic_item(cx, base_type, sym::string_type)
}
{
&call_args[0]
} else {
break;
}
},
_ => break,
};
}
arg_root
}
// Only `&'static str` or `String` can be used directly in the `panic!`. Other types should be
// converted to string.
fn requires_to_string(cx: &LateContext<'_>, arg: &hir::Expr<'_>) -> bool {
let arg_ty = cx.typeck_results().expr_ty(arg);
if is_type_diagnostic_item(cx, arg_ty, sym::string_type) {
return false;
}
if let ty::Ref(_, ty, ..) = arg_ty.kind() {
if *ty.kind() == ty::Str && can_be_static_str(cx, arg) {
return false;
}
};
true
}
// Check if an expression could have type `&'static str`, knowing that it
// has type `&str` for some lifetime.
fn can_be_static_str(cx: &LateContext<'_>, arg: &hir::Expr<'_>) -> bool {
match arg.kind {
hir::ExprKind::Lit(_) => true,
hir::ExprKind::Call(fun, _) => {
if let hir::ExprKind::Path(ref p) = fun.kind {
match cx.qpath_res(p, fun.hir_id) {
hir::def::Res::Def(hir::def::DefKind::Fn | hir::def::DefKind::AssocFn, def_id) => matches!(
cx.tcx.fn_sig(def_id).output().skip_binder().kind(),
ty::Ref(ty::ReStatic, ..)
),
_ => false,
}
} else {
false
}
},
hir::ExprKind::MethodCall(..) => {
cx.typeck_results()
.type_dependent_def_id(arg.hir_id)
.map_or(false, |method_id| {
matches!(
cx.tcx.fn_sig(method_id).output().skip_binder().kind(),
ty::Ref(ty::ReStatic, ..)
)
})
},
hir::ExprKind::Path(ref p) => matches!(
cx.qpath_res(p, arg.hir_id),
hir::def::Res::Def(hir::def::DefKind::Const | hir::def::DefKind::Static, _)
),
_ => false,
}
}
fn generate_format_arg_snippet(
cx: &LateContext<'_>,
a: &hir::Expr<'_>,
applicability: &mut Applicability,
) -> Vec<String> {
if_chain! {
if let hir::ExprKind::AddrOf(hir::BorrowKind::Ref, _, ref format_arg) = a.kind;
if let hir::ExprKind::Match(ref format_arg_expr, _, _) = format_arg.kind;
if let hir::ExprKind::Tup(ref format_arg_expr_tup) = format_arg_expr.kind;
then {
format_arg_expr_tup
.iter()
.map(|a| snippet_with_applicability(cx, a.span, "..", applicability).into_owned())
.collect()
} else {
unreachable!()
}
}
}
fn is_call(node: &hir::ExprKind<'_>) -> bool {
match node {
hir::ExprKind::AddrOf(hir::BorrowKind::Ref, _, expr) => {
is_call(&expr.kind)
},
hir::ExprKind::Call(..)
| hir::ExprKind::MethodCall(..)
// These variants are debatable or require further examination
| hir::ExprKind::If(..)
| hir::ExprKind::Match(..)
| hir::ExprKind::Block{ .. } => true,
_ => false,
}
}
if args.len() != 2 || name != "expect" || !is_call(&args[1].kind) {
return;
}
let receiver_type = cx.typeck_results().expr_ty_adjusted(&args[0]);
let closure_args = if is_type_diagnostic_item(cx, receiver_type, sym::option_type) {
"||"
} else if is_type_diagnostic_item(cx, receiver_type, sym::result_type) {
"|_|"
} else {
return;
};
let arg_root = get_arg_root(cx, &args[1]);
let span_replace_word = method_span.with_hi(expr.span.hi());
let mut applicability = Applicability::MachineApplicable;
//Special handling for `format!` as arg_root
if_chain! {
if let hir::ExprKind::Block(block, None) = &arg_root.kind;
if block.stmts.len() == 1;
if let hir::StmtKind::Local(local) = &block.stmts[0].kind;
if let Some(arg_root) = &local.init;
if let hir::ExprKind::Call(ref inner_fun, ref inner_args) = arg_root.kind;
if is_expn_of(inner_fun.span, "format").is_some() && inner_args.len() == 1;
if let hir::ExprKind::Call(_, format_args) = &inner_args[0].kind;
then {
let fmt_spec = &format_args[0];
let fmt_args = &format_args[1];
let mut args = vec![snippet(cx, fmt_spec.span, "..").into_owned()];
args.extend(generate_format_arg_snippet(cx, fmt_args, &mut applicability));
let sugg = args.join(", ");
span_lint_and_sugg(
cx,
EXPECT_FUN_CALL,
span_replace_word,
&format!("use of `{}` followed by a function call", name),
"try this",
format!("unwrap_or_else({} panic!({}))", closure_args, sugg),
applicability,
);
return;
}
}
let mut arg_root_snippet: Cow<'_, _> = snippet_with_applicability(cx, arg_root.span, "..", &mut applicability);
if requires_to_string(cx, arg_root) {
arg_root_snippet.to_mut().push_str(".to_string()");
}
span_lint_and_sugg(
cx,
EXPECT_FUN_CALL,
span_replace_word,
&format!("use of `{}` followed by a function call", name),
"try this",
format!(
"unwrap_or_else({} {{ panic!(\"{{}}\", {}) }})",
closure_args, arg_root_snippet
),
applicability,
);
}

30
clippy_lints/src/methods/expect_used.rs Normal file
View file

@ -0,0 +1,30 @@
use crate::utils::{is_type_diagnostic_item, span_lint_and_help};
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_span::sym;
use super::EXPECT_USED;
/// lint use of `expect()` for `Option`s and `Result`s
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, expect_args: &[hir::Expr<'_>]) {
let obj_ty = cx.typeck_results().expr_ty(&expect_args[0]).peel_refs();
let mess = if is_type_diagnostic_item(cx, obj_ty, sym::option_type) {
Some((EXPECT_USED, "an Option", "None"))
} else if is_type_diagnostic_item(cx, obj_ty, sym::result_type) {
Some((EXPECT_USED, "a Result", "Err"))
} else {
None
};
if let Some((lint, kind, none_value)) = mess {
span_lint_and_help(
cx,
lint,
expr.span,
&format!("used `expect()` on `{}` value", kind,),
None,
&format!("if this value is an `{}`, it will panic", none_value,),
);
}
}

39
clippy_lints/src/methods/filetype_is_file.rs Normal file
View file

@ -0,0 +1,39 @@
use crate::utils::{get_parent_expr, match_type, paths, span_lint_and_help};
use if_chain::if_chain;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_span::source_map::Span;
use super::FILETYPE_IS_FILE;
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) {
let ty = cx.typeck_results().expr_ty(&args[0]);
if !match_type(cx, ty, &paths::FILE_TYPE) {
return;
}
let span: Span;
let verb: &str;
let lint_unary: &str;
let help_unary: &str;
if_chain! {
if let Some(parent) = get_parent_expr(cx, expr);
if let hir::ExprKind::Unary(op, _) = parent.kind;
if op == hir::UnOp::Not;
then {
lint_unary = "!";
verb = "denies";
help_unary = "";
span = parent.span;
} else {
lint_unary = "";
verb = "covers";
help_unary = "!";
span = expr.span;
}
}
let lint_msg = format!("`{}FileType::is_file()` only {} regular files", lint_unary, verb);
let help_msg = format!("use `{}FileType::is_dir()` instead", help_unary);
span_lint_and_help(cx, FILETYPE_IS_FILE, span, &lint_msg, None, &help_msg);
}

21
clippy_lints/src/methods/filter_flat_map.rs Normal file
View file

@ -0,0 +1,21 @@
use crate::utils::{match_trait_method, paths, span_lint_and_help};
use rustc_hir as hir;
use rustc_lint::LateContext;
use super::FILTER_MAP;
/// lint use of `filter().flat_map()` for `Iterators`
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
_filter_args: &'tcx [hir::Expr<'_>],
_map_args: &'tcx [hir::Expr<'_>],
) {
// lint if caller of `.filter().flat_map()` is an Iterator
if match_trait_method(cx, expr, &paths::ITERATOR) {
let msg = "called `filter(..).flat_map(..)` on an `Iterator`";
let hint = "this is more succinctly expressed by calling `.flat_map(..)` \
and filtering by returning `iter::empty()`";
span_lint_and_help(cx, FILTER_MAP, expr.span, msg, None, hint);
}
}

85
clippy_lints/src/methods/filter_map.rs Normal file
View file

@ -0,0 +1,85 @@
use crate::utils::{match_trait_method, path_to_local_id, paths, snippet, span_lint_and_sugg, SpanlessEq};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::{Expr, ExprKind, PatKind, UnOp};
use rustc_lint::LateContext;
use rustc_middle::ty::TyS;
use rustc_span::symbol::sym;
use super::MANUAL_FILTER_MAP;
use super::MANUAL_FIND_MAP;
/// lint use of `filter().map()` or `find().map()` for `Iterators`
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, is_find: bool) {
if_chain! {
if let ExprKind::MethodCall(_, _, [map_recv, map_arg], map_span) = expr.kind;
if let ExprKind::MethodCall(_, _, [_, filter_arg], filter_span) = map_recv.kind;
if match_trait_method(cx, map_recv, &paths::ITERATOR);
// filter(|x| ...is_some())...
if let ExprKind::Closure(_, _, filter_body_id, ..) = filter_arg.kind;
let filter_body = cx.tcx.hir().body(filter_body_id);
if let [filter_param] = filter_body.params;
// optional ref pattern: `filter(|&x| ..)`
let (filter_pat, is_filter_param_ref) = if let PatKind::Ref(ref_pat, _) = filter_param.pat.kind {
(ref_pat, true)
} else {
(filter_param.pat, false)
};
// closure ends with is_some() or is_ok()
if let PatKind::Binding(_, filter_param_id, _, None) = filter_pat.kind;
if let ExprKind::MethodCall(path, _, [filter_arg], _) = filter_body.value.kind;
if let Some(opt_ty) = cx.typeck_results().expr_ty(filter_arg).ty_adt_def();
if let Some(is_result) = if cx.tcx.is_diagnostic_item(sym::option_type, opt_ty.did) {
Some(false)
} else if cx.tcx.is_diagnostic_item(sym::result_type, opt_ty.did) {
Some(true)
} else {
None
};
if path.ident.name.as_str() == if is_result { "is_ok" } else { "is_some" };
// ...map(|x| ...unwrap())
if let ExprKind::Closure(_, _, map_body_id, ..) = map_arg.kind;
let map_body = cx.tcx.hir().body(map_body_id);
if let [map_param] = map_body.params;
if let PatKind::Binding(_, map_param_id, map_param_ident, None) = map_param.pat.kind;
// closure ends with expect() or unwrap()
if let ExprKind::MethodCall(seg, _, [map_arg, ..], _) = map_body.value.kind;
if matches!(seg.ident.name, sym::expect | sym::unwrap | sym::unwrap_or);
let eq_fallback = |a: &Expr<'_>, b: &Expr<'_>| {
// in `filter(|x| ..)`, replace `*x` with `x`
let a_path = if_chain! {
if !is_filter_param_ref;
if let ExprKind::Unary(UnOp::Deref, expr_path) = a.kind;
then { expr_path } else { a }
};
// let the filter closure arg and the map closure arg be equal
if_chain! {
if path_to_local_id(a_path, filter_param_id);
if path_to_local_id(b, map_param_id);
if TyS::same_type(cx.typeck_results().expr_ty_adjusted(a), cx.typeck_results().expr_ty_adjusted(b));
then {
return true;
}
}
false
};
if SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, map_arg);
then {
let span = filter_span.to(map_span);
let (filter_name, lint) = if is_find {
("find", MANUAL_FIND_MAP)
} else {
("filter", MANUAL_FILTER_MAP)
};
let msg = format!("`{}(..).map(..)` can be simplified as `{0}_map(..)`", filter_name);
let to_opt = if is_result { ".ok()" } else { "" };
let sugg = format!("{}_map(|{}| {}{})", filter_name, map_param_ident,
snippet(cx, map_arg.span, ".."), to_opt);
span_lint_and_sugg(cx, lint, span, &msg, "try", sugg, Applicability::MachineApplicable);
}
}
}

21
clippy_lints/src/methods/filter_map_flat_map.rs Normal file
View file

@ -0,0 +1,21 @@
use crate::utils::{match_trait_method, paths, span_lint_and_help};
use rustc_hir as hir;
use rustc_lint::LateContext;
use super::FILTER_MAP;
/// lint use of `filter_map().flat_map()` for `Iterators`
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
_filter_args: &'tcx [hir::Expr<'_>],
_map_args: &'tcx [hir::Expr<'_>],
) {
// lint if caller of `.filter_map().flat_map()` is an Iterator
if match_trait_method(cx, expr, &paths::ITERATOR) {
let msg = "called `filter_map(..).flat_map(..)` on an `Iterator`";
let hint = "this is more succinctly expressed by calling `.flat_map(..)` \
and filtering by returning `iter::empty()`";
span_lint_and_help(cx, FILTER_MAP, expr.span, msg, None, hint);
}
}

20
clippy_lints/src/methods/filter_map_map.rs Normal file
View file

@ -0,0 +1,20 @@
use crate::utils::{match_trait_method, paths, span_lint_and_help};
use rustc_hir as hir;
use rustc_lint::LateContext;
use super::FILTER_MAP;
/// lint use of `filter_map().map()` for `Iterators`
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
_filter_args: &'tcx [hir::Expr<'_>],
_map_args: &'tcx [hir::Expr<'_>],
) {
// lint if caller of `.filter_map().map()` is an Iterator
if match_trait_method(cx, expr, &paths::ITERATOR) {
let msg = "called `filter_map(..).map(..)` on an `Iterator`";
let hint = "this is more succinctly expressed by only calling `.filter_map(..)` instead";
span_lint_and_help(cx, FILTER_MAP, expr.span, msg, None, hint);
}
}

40
clippy_lints/src/methods/filter_map_next.rs Normal file
View file

@ -0,0 +1,40 @@
use crate::utils::{match_trait_method, meets_msrv, paths, snippet, span_lint, span_lint_and_sugg};
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_semver::RustcVersion;
use super::FILTER_MAP_NEXT;
const FILTER_MAP_NEXT_MSRV: RustcVersion = RustcVersion::new(1, 30, 0);
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
filter_args: &'tcx [hir::Expr<'_>],
msrv: Option<&RustcVersion>,
) {
if match_trait_method(cx, expr, &paths::ITERATOR) {
if !meets_msrv(msrv, &FILTER_MAP_NEXT_MSRV) {
return;
}
let msg = "called `filter_map(..).next()` on an `Iterator`. This is more succinctly expressed by calling \
`.find_map(..)` instead";
let filter_snippet = snippet(cx, filter_args[1].span, "..");
if filter_snippet.lines().count() <= 1 {
let iter_snippet = snippet(cx, filter_args[0].span, "..");
span_lint_and_sugg(
cx,
FILTER_MAP_NEXT,
expr.span,
msg,
"try this",
format!("{}.find_map({})", iter_snippet, filter_snippet),
Applicability::MachineApplicable,
);
} else {
span_lint(cx, FILTER_MAP_NEXT, expr.span, msg);
}
}
}

31
clippy_lints/src/methods/filter_next.rs Normal file
View file

@ -0,0 +1,31 @@
use crate::utils::{match_trait_method, paths, snippet, span_lint, span_lint_and_sugg};
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use super::FILTER_NEXT;
/// lint use of `filter().next()` for `Iterators`
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, filter_args: &'tcx [hir::Expr<'_>]) {
// lint if caller of `.filter().next()` is an Iterator
if match_trait_method(cx, expr, &paths::ITERATOR) {
let msg = "called `filter(..).next()` on an `Iterator`. This is more succinctly expressed by calling \
`.find(..)` instead";
let filter_snippet = snippet(cx, filter_args[1].span, "..");
if filter_snippet.lines().count() <= 1 {
let iter_snippet = snippet(cx, filter_args[0].span, "..");
// add note if not multi-line
span_lint_and_sugg(
cx,
FILTER_NEXT,
expr.span,
msg,
"try this",
format!("{}.find({})", iter_snippet, filter_snippet),
Applicability::MachineApplicable,
);
} else {
span_lint(cx, FILTER_NEXT, expr.span, msg);
}
}
}

57
clippy_lints/src/methods/flat_map_identity.rs Normal file
View file

@ -0,0 +1,57 @@
use crate::utils::{match_qpath, match_trait_method, paths, span_lint_and_sugg};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_span::source_map::Span;
use super::FLAT_MAP_IDENTITY;
/// lint use of `flat_map` for `Iterators` where `flatten` would be sufficient
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
flat_map_args: &'tcx [hir::Expr<'_>],
flat_map_span: Span,
) {
if match_trait_method(cx, expr, &paths::ITERATOR) {
let arg_node = &flat_map_args[1].kind;
let apply_lint = |message: &str| {
span_lint_and_sugg(
cx,
FLAT_MAP_IDENTITY,
flat_map_span.with_hi(expr.span.hi()),
message,
"try",
"flatten()".to_string(),
Applicability::MachineApplicable,
);
};
if_chain! {
if let hir::ExprKind::Closure(_, _, body_id, _, _) = arg_node;
let body = cx.tcx.hir().body(*body_id);
if let hir::PatKind::Binding(_, _, binding_ident, _) = body.params[0].pat.kind;
if let hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) = body.value.kind;
if path.segments.len() == 1;
if path.segments[0].ident.name == binding_ident.name;
then {
apply_lint("called `flat_map(|x| x)` on an `Iterator`");
}
}
if_chain! {
if let hir::ExprKind::Path(ref qpath) = arg_node;
if match_qpath(qpath, &paths::STD_CONVERT_IDENTITY);
then {
apply_lint("called `flat_map(std::convert::identity)` on an `Iterator`");
}
}
}
}

67
clippy_lints/src/methods/from_iter_instead_of_collect.rs Normal file
View file

@ -0,0 +1,67 @@
use crate::utils::{get_trait_def_id, implements_trait, paths, span_lint_and_sugg, sugg};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::{LateContext, LintContext};
use rustc_middle::ty::Ty;
use super::FROM_ITER_INSTEAD_OF_COLLECT;
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) {
let ty = cx.typeck_results().expr_ty(expr);
let arg_ty = cx.typeck_results().expr_ty(&args[0]);
if_chain! {
if let Some(from_iter_id) = get_trait_def_id(cx, &paths::FROM_ITERATOR);
if let Some(iter_id) = get_trait_def_id(cx, &paths::ITERATOR);
if implements_trait(cx, ty, from_iter_id, &[]) && implements_trait(cx, arg_ty, iter_id, &[]);
then {
// `expr` implements `FromIterator` trait
let iter_expr = sugg::Sugg::hir(cx, &args[0], "..").maybe_par();
let turbofish = extract_turbofish(cx, expr, ty);
let sugg = format!("{}.collect::<{}>()", iter_expr, turbofish);
span_lint_and_sugg(
cx,
FROM_ITER_INSTEAD_OF_COLLECT,
expr.span,
"usage of `FromIterator::from_iter`",
"use `.collect()` instead of `::from_iter()`",
sugg,
Applicability::MaybeIncorrect,
);
}
}
}
fn extract_turbofish(cx: &LateContext<'_>, expr: &hir::Expr<'_>, ty: Ty<'tcx>) -> String {
if_chain! {
let call_site = expr.span.source_callsite();
if let Ok(snippet) = cx.sess().source_map().span_to_snippet(call_site);
let snippet_split = snippet.split("::").collect::<Vec<_>>();
if let Some((_, elements)) = snippet_split.split_last();
then {
// is there a type specifier? (i.e.: like `<u32>` in `collections::BTreeSet::<u32>::`)
if let Some(type_specifier) = snippet_split.iter().find(|e| e.starts_with('<') && e.ends_with('>')) {
// remove the type specifier from the path elements
let without_ts = elements.iter().filter_map(|e| {
if e == type_specifier { None } else { Some((*e).to_string()) }
}).collect::<Vec<_>>();
// join and add the type specifier at the end (i.e.: `collections::BTreeSet<u32>`)
format!("{}{}", without_ts.join("::"), type_specifier)
} else {
// type is not explicitly specified so wildcards are needed
// i.e.: 2 wildcards in `std::collections::BTreeMap<&i32, &char>`
let ty_str = ty.to_string();
let start = ty_str.find('<').unwrap_or(0);
let end = ty_str.find('>').unwrap_or_else(|| ty_str.len());
let nb_wildcard = ty_str[start..end].split(',').count();
let wildcards = format!("_{}", ", _".repeat(nb_wildcard - 1));
format!("{}<{}>", elements.join("::"), wildcards)
}
} else {
ty.to_string()
}
}
}

84
clippy_lints/src/methods/get_unwrap.rs Normal file
View file

@ -0,0 +1,84 @@
use crate::methods::derefs_to_slice;
use crate::utils::{
get_parent_expr, is_type_diagnostic_item, match_type, paths, snippet_with_applicability, span_lint_and_sugg,
};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_span::sym;
use super::GET_UNWRAP;
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, get_args: &'tcx [hir::Expr<'_>], is_mut: bool) {
// Note: we don't want to lint `get_mut().unwrap` for `HashMap` or `BTreeMap`,
// because they do not implement `IndexMut`
let mut applicability = Applicability::MachineApplicable;
let expr_ty = cx.typeck_results().expr_ty(&get_args[0]);
let get_args_str = if get_args.len() > 1 {
snippet_with_applicability(cx, get_args[1].span, "..", &mut applicability)
} else {
return; // not linting on a .get().unwrap() chain or variant
};
let mut needs_ref;
let caller_type = if derefs_to_slice(cx, &get_args[0], expr_ty).is_some() {
needs_ref = get_args_str.parse::<usize>().is_ok();
"slice"
} else if is_type_diagnostic_item(cx, expr_ty, sym::vec_type) {
needs_ref = get_args_str.parse::<usize>().is_ok();
"Vec"
} else if is_type_diagnostic_item(cx, expr_ty, sym::vecdeque_type) {
needs_ref = get_args_str.parse::<usize>().is_ok();
"VecDeque"
} else if !is_mut && is_type_diagnostic_item(cx, expr_ty, sym::hashmap_type) {
needs_ref = true;
"HashMap"
} else if !is_mut && match_type(cx, expr_ty, &paths::BTREEMAP) {
needs_ref = true;
"BTreeMap"
} else {
return; // caller is not a type that we want to lint
};
let mut span = expr.span;
// Handle the case where the result is immediately dereferenced
// by not requiring ref and pulling the dereference into the
// suggestion.
if_chain! {
if needs_ref;
if let Some(parent) = get_parent_expr(cx, expr);
if let hir::ExprKind::Unary(hir::UnOp::Deref, _) = parent.kind;
then {
needs_ref = false;
span = parent.span;
}
}
let mut_str = if is_mut { "_mut" } else { "" };
let borrow_str = if !needs_ref {
""
} else if is_mut {
"&mut "
} else {
"&"
};
span_lint_and_sugg(
cx,
GET_UNWRAP,
span,
&format!(
"called `.get{0}().unwrap()` on a {1}. Using `[]` is more clear and more concise",
mut_str, caller_type
),
"try this",
format!(
"{}{}[{}]",
borrow_str,
snippet_with_applicability(cx, get_args[0].span, "..", &mut applicability),
get_args_str
),
applicability,
);
}

32
clippy_lints/src/methods/implicit_clone.rs Normal file
View file

@ -0,0 +1,32 @@
use crate::utils::span_lint_and_sugg;
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::ExprKind;
use rustc_lint::LateContext;
use rustc_middle::ty::TyS;
use rustc_span::symbol::Symbol;
use super::IMPLICIT_CLONE;
use clippy_utils::is_diagnostic_assoc_item;
pub fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, trait_diagnostic: Symbol) {
if_chain! {
if let ExprKind::MethodCall(method_path, _, [arg], _) = &expr.kind;
let return_type = cx.typeck_results().expr_ty(&expr);
let input_type = cx.typeck_results().expr_ty(arg).peel_refs();
if let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
if let Some(ty_name) = input_type.ty_adt_def().map(|adt_def| cx.tcx.item_name(adt_def.did));
if TyS::same_type(return_type, input_type);
if is_diagnostic_assoc_item(cx, expr_def_id, trait_diagnostic);
then {
span_lint_and_sugg(
cx,IMPLICIT_CLONE,method_path.ident.span,
&format!("implicitly cloning a `{}` by calling `{}` on its dereferenced type", ty_name, method_path.ident.name),
"consider using",
"clone".to_string(),
Applicability::MachineApplicable
);
}
}
}

2
clippy_lints/src/methods/inefficient_to_string.rs
View file

@ -10,7 +10,7 @@ use rustc_middle::ty::{self, Ty};
use rustc_span::sym;
/// Checks for the `INEFFICIENT_TO_STRING` lint
pub fn lint<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, arg: &hir::Expr<'_>, arg_ty: Ty<'tcx>) {
pub fn check<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, arg: &hir::Expr<'_>, arg_ty: Ty<'tcx>) {
if_chain! {
if let Some(to_string_meth_did) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
if match_def_path(cx, to_string_meth_did, &paths::TO_STRING_METHOD);

2
clippy_lints/src/methods/inspect_for_each.rs
View file

@ -7,7 +7,7 @@ use crate::utils::{match_trait_method, paths, span_lint_and_help};
use super::INSPECT_FOR_EACH;
/// lint use of `inspect().for_each()` for `Iterators`
pub(super) fn lint<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, inspect_span: Span) {
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, inspect_span: Span) {
if match_trait_method(cx, expr, &paths::ITERATOR) {
let msg = "called `inspect(..).for_each(..)` on an `Iterator`";
let hint = "move the code from `inspect(..)` to `for_each(..)` and remove the `inspect(..)`";

43
clippy_lints/src/methods/into_iter_on_ref.rs Normal file
View file

@ -0,0 +1,43 @@
use crate::utils::{has_iter_method, match_trait_method, paths, span_lint_and_sugg};
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_middle::ty::{self, Ty};
use rustc_span::source_map::Span;
use rustc_span::symbol::Symbol;
use super::INTO_ITER_ON_REF;
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, self_ref_ty: Ty<'_>, method_span: Span) {
if !match_trait_method(cx, expr, &paths::INTO_ITERATOR) {
return;
}
if let Some((kind, method_name)) = ty_has_iter_method(cx, self_ref_ty) {
span_lint_and_sugg(
cx,
INTO_ITER_ON_REF,
method_span,
&format!(
"this `.into_iter()` call is equivalent to `.{}()` and will not consume the `{}`",
method_name, kind,
),
"call directly",
method_name.to_string(),
Applicability::MachineApplicable,
);
}
}
fn ty_has_iter_method(cx: &LateContext<'_>, self_ref_ty: Ty<'_>) -> Option<(Symbol, &'static str)> {
has_iter_method(cx, self_ref_ty).map(|ty_name| {
let mutbl = match self_ref_ty.kind() {
ty::Ref(_, _, mutbl) => mutbl,
_ => unreachable!(),
};
let method_name = match mutbl {
hir::Mutability::Not => "iter",
hir::Mutability::Mut => "iter_mut",
};
(ty_name, method_name)
})
}

30
clippy_lints/src/methods/iter_cloned_collect.rs Normal file
View file

@ -0,0 +1,30 @@
use crate::methods::derefs_to_slice;
use crate::utils::{is_type_diagnostic_item, span_lint_and_sugg};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_span::sym;
use super::ITER_CLONED_COLLECT;
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, iter_args: &'tcx [hir::Expr<'_>]) {
if_chain! {
if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::vec_type);
if let Some(slice) = derefs_to_slice(cx, &iter_args[0], cx.typeck_results().expr_ty(&iter_args[0]));
if let Some(to_replace) = expr.span.trim_start(slice.span.source_callsite());
then {
span_lint_and_sugg(
cx,
ITER_CLONED_COLLECT,
to_replace,
"called `iter().cloned().collect()` on a slice to create a `Vec`. Calling `to_vec()` is both faster and \
more readable",
"try",
".to_vec()".to_string(),
Applicability::MachineApplicable,
);
}
}
}

47
clippy_lints/src/methods/iter_count.rs Normal file
View file

@ -0,0 +1,47 @@
use crate::methods::derefs_to_slice;
use crate::utils::{is_type_diagnostic_item, match_type, paths, snippet_with_applicability, span_lint_and_sugg};
use rustc_errors::Applicability;
use rustc_hir::Expr;
use rustc_lint::LateContext;
use rustc_span::sym;
use super::ITER_COUNT;
pub(crate) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, iter_args: &'tcx [Expr<'tcx>], iter_method: &str) {
let ty = cx.typeck_results().expr_ty(&iter_args[0]);
let caller_type = if derefs_to_slice(cx, &iter_args[0], ty).is_some() {
"slice"
} else if is_type_diagnostic_item(cx, ty, sym::vec_type) {
"Vec"
} else if is_type_diagnostic_item(cx, ty, sym::vecdeque_type) {
"VecDeque"
} else if is_type_diagnostic_item(cx, ty, sym::hashset_type) {
"HashSet"
} else if is_type_diagnostic_item(cx, ty, sym::hashmap_type) {
"HashMap"
} else if match_type(cx, ty, &paths::BTREEMAP) {
"BTreeMap"
} else if match_type(cx, ty, &paths::BTREESET) {
"BTreeSet"
} else if match_type(cx, ty, &paths::LINKED_LIST) {
"LinkedList"
} else if match_type(cx, ty, &paths::BINARY_HEAP) {
"BinaryHeap"
} else {
return;
};
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
ITER_COUNT,
expr.span,
&format!("called `.{}().count()` on a `{}`", iter_method, caller_type),
"try",
format!(
"{}.len()",
snippet_with_applicability(cx, iter_args[0].span, "..", &mut applicability),
),
applicability,
);
}

68
clippy_lints/src/methods/iter_next_slice.rs Normal file
View file

@ -0,0 +1,68 @@
use crate::methods::derefs_to_slice;
use crate::utils::{get_parent_expr, higher, is_type_diagnostic_item, snippet_with_applicability, span_lint_and_sugg};
use if_chain::if_chain;
use rustc_ast::ast;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_span::symbol::sym;
use super::ITER_NEXT_SLICE;
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, iter_args: &'tcx [hir::Expr<'_>]) {
let caller_expr = &iter_args[0];
// Skip lint if the `iter().next()` expression is a for loop argument,
// since it is already covered by `&loops::ITER_NEXT_LOOP`
let mut parent_expr_opt = get_parent_expr(cx, expr);
while let Some(parent_expr) = parent_expr_opt {
if higher::for_loop(parent_expr).is_some() {
return;
}
parent_expr_opt = get_parent_expr(cx, parent_expr);
}
if derefs_to_slice(cx, caller_expr, cx.typeck_results().expr_ty(caller_expr)).is_some() {
// caller is a Slice
if_chain! {
if let hir::ExprKind::Index(ref caller_var, ref index_expr) = &caller_expr.kind;
if let Some(higher::Range { start: Some(start_expr), end: None, limits: ast::RangeLimits::HalfOpen })
= higher::range(index_expr);
if let hir::ExprKind::Lit(ref start_lit) = &start_expr.kind;
if let ast::LitKind::Int(start_idx, _) = start_lit.node;
then {
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
ITER_NEXT_SLICE,
expr.span,
"using `.iter().next()` on a Slice without end index",
"try calling",
format!("{}.get({})", snippet_with_applicability(cx, caller_var.span, "..", &mut applicability), start_idx),
applicability,
);
}
}
} else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(caller_expr), sym::vec_type)
|| matches!(
&cx.typeck_results().expr_ty(caller_expr).peel_refs().kind(),
ty::Array(_, _)
)
{
// caller is a Vec or an Array
let mut applicability = Applicability::MachineApplicable;
span_lint_and_sugg(
cx,
ITER_NEXT_SLICE,
expr.span,
"using `.iter().next()` on an array",
"try calling",
format!(
"{}.get(0)",
snippet_with_applicability(cx, caller_expr.span, "..", &mut applicability)
),
applicability,
);
}
}

38
clippy_lints/src/methods/iter_nth.rs Normal file
View file

@ -0,0 +1,38 @@
use crate::methods::derefs_to_slice;
use crate::methods::iter_nth_zero;
use crate::utils::{is_type_diagnostic_item, span_lint_and_help};
use rustc_hir as hir;
use rustc_lint::LateContext;
use rustc_span::symbol::sym;
use super::ITER_NTH;
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &hir::Expr<'_>,
nth_and_iter_args: &[&'tcx [hir::Expr<'tcx>]],
is_mut: bool,
) {
let iter_args = nth_and_iter_args[1];
let mut_str = if is_mut { "_mut" } else { "" };
let caller_type = if derefs_to_slice(cx, &iter_args[0], cx.typeck_results().expr_ty(&iter_args[0])).is_some() {
"slice"
} else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&iter_args[0]), sym::vec_type) {
"Vec"
} else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&iter_args[0]), sym::vecdeque_type) {
"VecDeque"
} else {
let nth_args = nth_and_iter_args[0];
iter_nth_zero::check(cx, expr, &nth_args);
return; // caller is not a type that we want to lint
};
span_lint_and_help(
cx,
ITER_NTH,
expr.span,
&format!("called `.iter{0}().nth()` on a {1}", mut_str, caller_type),
None,
&format!("calling `.get{}()` is both faster and more readable", mut_str),
);
}

Some files were not shown because too many files have changed in this diff Show more