Skip to content

Commit 40084c2

Browse files
matthiaskrgrmatthiaskrgr
committed
submodules: update clippy from 29bf75c to 1df5766
Fixes clippy toolstate. Changes: ```` Remove -preview suffix from README rustup clippy build with latest rustc (breakage due to rust-lang/rust@08f8fae ) Forgot to remove some debugging code ... Improved code noted by clippy. Fix bug in `implicit_return`. Bug was already covered by test, but test was not checked for. fix rust-lang#3482 and add ui test for it Don't change current working directory of cargo tests Use cargo's "PROFILE" envvar and set CLIPPY_DOGFOOD Use dogfood_runner for deterministic test ordering Remove unnecessary documentation Fix dogfood tests. Added additional reasoning to `Why is this bad?`. Added comment to explain usage of MIR. Renamed to `implicit_return`. Covered all other kinds besides `ExprKind::Lit`. Added check for replacing `break` with `return`. Appeasing the Test Gods. Seems I'm not smart enough to run the tests locally before committing. Renamed `forced_return` to `missing_returns`. Better clarification in the docs. Ran `update_lints`. Added `FORCED_RETURN` lint. ````
1 parent e1488aa commit 40084c2

13 files changed

+316
-52
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -706,6 +706,7 @@ All notable changes to this project will be documented in this file.
706706
[`if_same_then_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else
707707
[`ifs_same_cond`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond
708708
[`implicit_hasher`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher
709+
[`implicit_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_return
709710
[`inconsistent_digit_grouping`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_digit_grouping
710711
[`indexing_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing
711712
[`ineffective_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#ineffective_bit_mask

‎CONTRIBUTING.md

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -168,18 +168,6 @@ Manually testing against an example file is useful if you have added some
168168
local modifications, run `env CLIPPY_TESTS=true cargo run --bin clippy-driver -- -L ./target/debug input.rs`
169169
from the working copy root.
170170

171-
### Linting Clippy with your local changes
172-
173-
Clippy CI only passes if all lints defined in the version of the Clippy being
174-
tested pass (that is, don’t report any suggestions). You can avoid prolonging
175-
the CI feedback cycle for PRs you submit by running these lints yourself ahead
176-
of time and addressing any issues found:
177-
178-
```
179-
cargo build
180-
`pwd`/target/debug/cargo-clippy clippy --all-targets --all-features -- -D clippy::all -D clippy::internal -D clippy::pedantic
181-
```
182-
183171
### How Clippy works
184172

185173
Clippy is a [rustc compiler plugin][compiler_plugin]. The main entry point is at [`src/lib.rs`][main_entry]. In there, the lint registration is delegated to the [`clippy_lints`][lint_crate] crate.

‎README.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
99

10-
[There are 289 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
10+
[There are 290 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
1111

1212
We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
1313

@@ -66,7 +66,7 @@ rustup update
6666
Once you have rustup and the latest stable release (at least Rust 1.29) installed, run the following command:
6767

6868
```terminal
69-
rustup component add clippy-preview
69+
rustup component add clippy
7070
```
7171

7272
Now you can run Clippy by invoking `cargo clippy`.
@@ -95,7 +95,7 @@ rust:
9595
- stable
9696
- beta
9797
before_script:
98-
- rustup component add clippy-preview
98+
- rustup component add clippy
9999
script:
100100
- cargo clippy
101101
# if you want the build job to fail when encountering warnings, use
@@ -114,7 +114,7 @@ language: rust
114114
rust:
115115
- nightly
116116
before_script:
117-
- rustup component add clippy-preview --toolchain=nightly || cargo install --git https://github.com/rust-lang/rust-clippy/ --force clippy
117+
- rustup component add clippy --toolchain=nightly || cargo install --git https://github.com/rust-lang/rust-clippy/ --force clippy
118118
# etc
119119
```
120120

‎ci/base-tests.sh

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -27,23 +27,3 @@ cd clippy_dev && cargo test && cd ..
2727
# Perform various checks for lint registration
2828
./util/dev update_lints --check
2929
cargo +nightly fmt --all -- --check
30-
31-
# Add bin to PATH for windows
32-
PATH=$PATH:$(rustc --print sysroot)/bin
33-
34-
CLIPPY="`pwd`/target/debug/cargo-clippy clippy"
35-
# run clippy on its own codebase...
36-
${CLIPPY} --all-targets --all-features -- -D clippy::all -D clippy::internal -Dclippy::pedantic
37-
# ... and some test directories
38-
for dir in clippy_workspace_tests clippy_workspace_tests/src clippy_workspace_tests/subcrate clippy_workspace_tests/subcrate/src clippy_dev rustc_tools_util
39-
do
40-
cd ${dir}
41-
${CLIPPY} -- -D clippy::all -D clippy::pedantic
42-
cd -
43-
done
44-
45-
46-
# test --manifest-path
47-
${CLIPPY} --manifest-path=clippy_workspace_tests/Cargo.toml -- -D clippy::all
48-
cd clippy_workspace_tests/subcrate && ${CLIPPY} --manifest-path=../Cargo.toml -- -D clippy::all && cd ../..
49-
set +x

‎clippy_lints/src/block_in_if_condition.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ impl<'a, 'tcx: 'a> Visitor<'tcx> for ExVisitor<'a, 'tcx> {
7070
if let ExprKind::Closure(_, _, eid, _, _) = expr.node {
7171
let body = self.cx.tcx.hir.body(eid);
7272
let ex = &body.value;
73-
if matches!(ex.node, ExprKind::Block(_, _)) {
73+
if matches!(ex.node, ExprKind::Block(_, _)) && !in_macro(body.value.span) {
7474
self.found_block = Some(ex);
7575
return;
7676
}

‎clippy_lints/src/implicit_return.rs

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution.
3+
//
4+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
5+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
6+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
7+
// option. This file may not be copied, modified, or distributed
8+
// except according to those terms.
9+
10+
use crate::rustc::hir::{intravisit::FnKind, Body, ExprKind, FnDecl};
11+
use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
12+
use crate::rustc::{declare_tool_lint, lint_array};
13+
use crate::rustc_errors::Applicability;
14+
use crate::syntax::{ast::NodeId, source_map::Span};
15+
use crate::utils::{snippet_opt, span_lint_and_then};
16+
17+
/// **What it does:** Checks for missing return statements at the end of a block.
18+
///
19+
/// **Why is this bad?** Actually omitting the return keyword is idiomatic Rust code. Programmers
20+
/// coming from other languages might prefer the expressiveness of `return`. It's possible to miss
21+
/// the last returning statement because the only difference is a missing `;`. Especially in bigger
22+
/// code with multiple return paths having a `return` keyword makes it easier to find the
23+
/// corresponding statements.
24+
///
25+
/// **Known problems:** None.
26+
///
27+
/// **Example:**
28+
/// ```rust
29+
/// fn foo(x: usize) {
30+
/// x
31+
/// }
32+
/// ```
33+
/// add return
34+
/// ```rust
35+
/// fn foo(x: usize) {
36+
/// return x;
37+
/// }
38+
/// ```
39+
declare_clippy_lint! {
40+
pub IMPLICIT_RETURN,
41+
restriction,
42+
"use a return statement like `return expr` instead of an expression"
43+
}
44+
45+
pub struct Pass;
46+
47+
impl Pass {
48+
fn expr_match(cx: &LateContext<'_, '_>, expr: &rustc::hir::Expr) {
49+
match &expr.node {
50+
// loops could be using `break` instead of `return`
51+
ExprKind::Block(block, ..) | ExprKind::Loop(block, ..) => {
52+
if let Some(expr) = &block.expr {
53+
Self::expr_match(cx, expr);
54+
}
55+
// only needed in the case of `break` with `;` at the end
56+
else if let Some(stmt) = block.stmts.last() {
57+
if let rustc::hir::StmtKind::Semi(expr, ..) = &stmt.node {
58+
Self::expr_match(cx, expr);
59+
}
60+
}
61+
},
62+
// use `return` instead of `break`
63+
ExprKind::Break(.., break_expr) => {
64+
if let Some(break_expr) = break_expr {
65+
span_lint_and_then(cx, IMPLICIT_RETURN, expr.span, "missing return statement", |db| {
66+
if let Some(snippet) = snippet_opt(cx, break_expr.span) {
67+
db.span_suggestion_with_applicability(
68+
expr.span,
69+
"change `break` to `return` as shown",
70+
format!("return {}", snippet),
71+
Applicability::MachineApplicable,
72+
);
73+
}
74+
});
75+
}
76+
},
77+
ExprKind::If(.., if_expr, else_expr) => {
78+
Self::expr_match(cx, if_expr);
79+
80+
if let Some(else_expr) = else_expr {
81+
Self::expr_match(cx, else_expr);
82+
}
83+
},
84+
ExprKind::Match(_, arms, ..) => {
85+
for arm in arms {
86+
Self::expr_match(cx, &arm.body);
87+
}
88+
},
89+
// skip if it already has a return statement
90+
ExprKind::Ret(..) => (),
91+
// everything else is missing `return`
92+
_ => span_lint_and_then(cx, IMPLICIT_RETURN, expr.span, "missing return statement", |db| {
93+
if let Some(snippet) = snippet_opt(cx, expr.span) {
94+
db.span_suggestion_with_applicability(
95+
expr.span,
96+
"add `return` as shown",
97+
format!("return {}", snippet),
98+
Applicability::MachineApplicable,
99+
);
100+
}
101+
}),
102+
}
103+
}
104+
}
105+
106+
impl LintPass for Pass {
107+
fn get_lints(&self) -> LintArray {
108+
lint_array!(IMPLICIT_RETURN)
109+
}
110+
}
111+
112+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
113+
fn check_fn(
114+
&mut self,
115+
cx: &LateContext<'a, 'tcx>,
116+
_: FnKind<'tcx>,
117+
_: &'tcx FnDecl,
118+
body: &'tcx Body,
119+
_: Span,
120+
_: NodeId,
121+
) {
122+
let def_id = cx.tcx.hir.body_owner_def_id(body.id());
123+
let mir = cx.tcx.optimized_mir(def_id);
124+
125+
// checking return type through MIR, HIR is not able to determine inferred closure return types
126+
if !mir.return_ty().is_unit() {
127+
Self::expr_match(cx, &body.value);
128+
}
129+
}
130+
}

‎clippy_lints/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ pub mod functions;
126126
pub mod identity_conversion;
127127
pub mod identity_op;
128128
pub mod if_not_else;
129+
pub mod implicit_return;
129130
pub mod indexing_slicing;
130131
pub mod infallible_destructuring_match;
131132
pub mod infinite_iter;
@@ -371,6 +372,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
371372
reg.register_late_lint_pass(box unicode::Unicode);
372373
reg.register_late_lint_pass(box strings::StringAdd);
373374
reg.register_early_lint_pass(box returns::ReturnPass);
375+
reg.register_late_lint_pass(box implicit_return::Pass);
374376
reg.register_late_lint_pass(box methods::Pass);
375377
reg.register_late_lint_pass(box map_clone::Pass);
376378
reg.register_late_lint_pass(box shadow::Pass);
@@ -485,6 +487,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
485487
arithmetic::FLOAT_ARITHMETIC,
486488
arithmetic::INTEGER_ARITHMETIC,
487489
else_if_without_else::ELSE_IF_WITHOUT_ELSE,
490+
implicit_return::IMPLICIT_RETURN,
488491
indexing_slicing::INDEXING_SLICING,
489492
inherent_impl::MULTIPLE_INHERENT_IMPL,
490493
literal_representation::DECIMAL_LITERAL_REPRESENTATION,

‎clippy_lints/src/use_self.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use crate::rustc::ty;
1414
use crate::rustc::{declare_tool_lint, lint_array};
1515
use crate::rustc_errors::Applicability;
1616
use crate::syntax::ast::NodeId;
17-
use crate::syntax_pos::symbol::keywords::SelfType;
17+
use crate::syntax_pos::symbol::keywords::SelfUpper;
1818
use crate::utils::{in_macro, span_lint_and_sugg};
1919
use if_chain::if_chain;
2020

@@ -226,7 +226,7 @@ struct UseSelfVisitor<'a, 'tcx: 'a> {
226226

227227
impl<'a, 'tcx> Visitor<'tcx> for UseSelfVisitor<'a, 'tcx> {
228228
fn visit_path(&mut self, path: &'tcx Path, _id: HirId) {
229-
if self.item_path.def == path.def && path.segments.last().expect(SEGMENTS_MSG).ident.name != SelfType.name() {
229+
if self.item_path.def == path.def && path.segments.last().expect(SEGMENTS_MSG).ident.name != SelfUpper.name() {
230230
span_use_self_lint(self.cx, path);
231231
}
232232

‎clippy_lints/src/utils/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -970,7 +970,7 @@ pub fn opt_def_id(def: Def) -> Option<DefId> {
970970

971971
pub fn is_self(slf: &Arg) -> bool {
972972
if let PatKind::Binding(_, _, name, _) = slf.pat.node {
973-
name.name == keywords::SelfValue.name()
973+
name.name == keywords::SelfLower.name()
974974
} else {
975975
false
976976
}

‎tests/dogfood.rs

Lines changed: 51 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,57 @@ fn dogfood() {
1212
if option_env!("RUSTC_TEST_SUITE").is_some() || cfg!(windows) {
1313
return;
1414
}
15-
let root_dir = std::env::current_dir().unwrap();
16-
for d in &[".", "clippy_lints", "rustc_tools_util", "clippy_dev"] {
17-
std::env::set_current_dir(root_dir.join(d)).unwrap();
18-
let output = std::process::Command::new("cargo")
19-
.arg("run")
20-
.arg("--bin")
21-
.arg("cargo-clippy")
22-
.arg("--all-features")
23-
.arg("--manifest-path")
24-
.arg(root_dir.join("Cargo.toml"))
25-
.args(&["--", "-W clippy::internal -W clippy::pedantic"])
26-
.env("CLIPPY_DOGFOOD", "true")
15+
let root_dir = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"));
16+
let clippy_cmd = std::path::Path::new(&root_dir)
17+
.join("target")
18+
.join(env!("PROFILE"))
19+
.join("cargo-clippy");
20+
21+
let output = std::process::Command::new(clippy_cmd)
22+
.current_dir(root_dir)
23+
.env("CLIPPY_DOGFOOD", "1")
24+
.arg("clippy")
25+
.arg("--all-targets")
26+
.arg("--all-features")
27+
.arg("--")
28+
.args(&["-D", "clippy::all"])
29+
.args(&["-D", "clippy::internal"])
30+
.args(&["-D", "clippy::pedantic"])
31+
.output()
32+
.unwrap();
33+
println!("status: {}", output.status);
34+
println!("stdout: {}", String::from_utf8_lossy(&output.stdout));
35+
println!("stderr: {}", String::from_utf8_lossy(&output.stderr));
36+
37+
assert!(output.status.success());
38+
}
39+
40+
#[test]
41+
fn dogfood_tests() {
42+
if option_env!("RUSTC_TEST_SUITE").is_some() || cfg!(windows) {
43+
return;
44+
}
45+
let root_dir = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"));
46+
let clippy_cmd = std::path::Path::new(&root_dir)
47+
.join("target")
48+
.join(env!("PROFILE"))
49+
.join("cargo-clippy");
50+
51+
for d in &[
52+
"clippy_workspace_tests",
53+
"clippy_workspace_tests/src",
54+
"clippy_workspace_tests/subcrate",
55+
"clippy_workspace_tests/subcrate/src",
56+
"clippy_dev",
57+
"rustc_tools_util",
58+
] {
59+
let output = std::process::Command::new(&clippy_cmd)
60+
.current_dir(root_dir.join(d))
61+
.env("CLIPPY_DOGFOOD", "1")
62+
.arg("clippy")
63+
.arg("--")
64+
.args(&["-D", "clippy::all"])
65+
.args(&["-D", "clippy::pedantic"])
2766
.output()
2867
.unwrap();
2968
println!("status: {}", output.status);

0 commit comments

Comments
 (0)