Skip to content

Commit cd2155e

Browse files
ManishearthManishearth
committed
Update clippy
1 parent f887a40 commit cd2155e

33 files changed

+692
-311
lines changed

‎CONTRIBUTING.md

Lines changed: 3 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,6 @@ All contributors are expected to follow the [Rust Code of Conduct](http://www.ru
1414
* [Getting started](#getting-started)
1515
* [Finding something to fix/improve](#finding-something-to-fiximprove)
1616
* [Writing code](#writing-code)
17-
* [Author lint](#author-lint)
18-
* [Documentation](#documentation)
19-
* [Running test suite](#running-test-suite)
20-
* [Running rustfmt](#running-rustfmt)
21-
* [Testing manually](#testing-manually)
2217
* [How Clippy works](#how-clippy-works)
2318
* [Fixing nightly build failures](#fixing-build-failures-caused-by-rust)
2419
* [Issue and PR Triage](#issue-and-pr-triage)
@@ -73,121 +68,15 @@ an AST expression). `match_def_path()` in Clippy's `utils` module can also be us
7368

7469
## Writing code
7570

76-
Clippy depends on the current git master version of rustc, which can change rapidly. Make sure you're
77-
working near rust-clippy's master, and use the `setup-toolchain.sh` script to configure the appropriate
78-
toolchain for this directory.
79-
80-
[Llogiq's blog post on lints](https://llogiq.github.io/2015/06/04/workflows.html) is a nice primer
81-
to lint-writing, though it does get into advanced stuff. Most lints consist of an implementation of
82-
`LintPass` with one or more of its default methods overridden. See the existing lints for examples
83-
of this.
71+
Have a look at the [docs for writing lints](doc/adding_lints.md) for more details. [Llogiq's blog post on lints](https://llogiq.github.io/2015/06/04/workflows.html) is also a nice primer
72+
to lint-writing, though it does get into advanced stuff and may be a bit
73+
outdated.
8474

8575
If you want to add a new lint or change existing ones apart from bugfixing, it's
8676
also a good idea to give the [stability guarantees][rfc_stability] and
8777
[lint categories][rfc_lint_cats] sections of the [Clippy 1.0 RFC][clippy_rfc] a
8878
quick read.
8979

90-
### Author lint
91-
92-
There is also the internal `author` lint to generate Clippy code that detects the offending pattern. It does not work for all of the Rust syntax, but can give a good starting point.
93-
94-
First, create a new UI test file in the `tests/ui/` directory with the pattern you want to match:
95-
96-
```rust
97-
// ./tests/ui/my_lint.rs
98-
fn main() {
99-
#[clippy::author]
100-
let arr: [i32; 1] = [7]; // Replace line with the code you want to match
101-
}
102-
```
103-
104-
Now you run `TESTNAME=ui/my_lint cargo uitest` to produce
105-
a `.stdout` file with the generated code:
106-
107-
```rust
108-
// ./tests/ui/my_lint.stdout
109-
110-
if_chain! {
111-
if let ExprKind::Array(ref elements) = stmt.node;
112-
if elements.len() == 1;
113-
if let ExprKind::Lit(ref lit) = elements[0].node;
114-
if let LitKind::Int(7, _) = lit.node;
115-
then {
116-
// report your lint here
117-
}
118-
}
119-
```
120-
121-
If the command was executed successfully, you can copy the code over to where you are implementing your lint.
122-
123-
### Documentation
124-
125-
Please document your lint with a doc comment akin to the following:
126-
127-
```rust
128-
/// **What it does:** Checks for ... (describe what the lint matches).
129-
///
130-
/// **Why is this bad?** Supply the reason for linting the code.
131-
///
132-
/// **Known problems:** None. (Or describe where it could go wrong.)
133-
///
134-
/// **Example:**
135-
///
136-
/// ```rust
137-
/// // Bad
138-
/// Insert a short example of code that triggers the lint
139-
///
140-
/// // Good
141-
/// Insert a short example of improved code that doesn't trigger the lint
142-
/// ```
143-
```
144-
145-
Once your lint is merged it will show up in the [lint list](https://rust-lang.github.io/rust-clippy/master/index.html)
146-
147-
### Running test suite
148-
149-
Use `cargo test` to run the whole testsuite.
150-
151-
If you don't want to wait for all tests to finish, you can also execute a single test file by using `TESTNAME` to specify the test to run:
152-
153-
```bash
154-
TESTNAME=ui/empty_line_after_outer_attr cargo uitest
155-
```
156-
157-
Clippy uses UI tests. UI tests check that the output of the compiler is exactly as expected.
158-
Of course there's little sense in writing the output yourself or copying it around.
159-
Therefore you should use `tests/ui/update-all-references.sh` (after running
160-
`cargo test`) and check whether the output looks as you expect with `git diff`. Commit all
161-
`*.stderr` files, too.
162-
163-
If the lint you are working on is making use of structured suggestions, the
164-
test file should include a `// run-rustfix` comment at the top. This will
165-
additionally run [rustfix](https://github.com/rust-lang-nursery/rustfix) for
166-
that test. Rustfix will apply the suggestions from the lint to the code of the
167-
test file and compare that to the contents of a `.fixed` file.
168-
169-
Use `tests/ui/update-all-references.sh` to automatically generate the
170-
`.fixed` file after running `cargo test`.
171-
172-
### Running rustfmt
173-
174-
[Rustfmt](https://github.com/rust-lang/rustfmt) is a tool for formatting Rust code according
175-
to style guidelines. The code has to be formatted by `rustfmt` before a PR will be merged.
176-
177-
It can be installed via `rustup`:
178-
```bash
179-
rustup component add rustfmt
180-
```
181-
182-
Use `cargo fmt --all` to format the whole codebase.
183-
184-
### Testing manually
185-
186-
Manually testing against an example file is useful if you have added some
187-
`println!`s and test suite output becomes unreadable. To try Clippy with your
188-
local modifications, run `env CLIPPY_TESTS=true cargo run --bin clippy-driver -- -L ./target/debug input.rs`
189-
from the working copy root.
190-
19180
## How Clippy works
19281

19382
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: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,13 +107,13 @@ script:
107107
- cargo clippy
108108
# if you want the build job to fail when encountering warnings, use
109109
- cargo clippy -- -D warnings
110-
# in order to also check tests and none-default crate features, use
110+
# in order to also check tests and non-default crate features, use
111111
- cargo clippy --all-targets --all-features -- -D warnings
112112
- cargo test
113113
# etc.
114114
```
115115

116-
It might happen that Clippy is not available for a certain nightly release.
116+
If you are on nightly, It might happen that Clippy is not available for a certain nightly release.
117117
In this case you can try to conditionally install Clippy from the git repo.
118118

119119
```yaml

‎ci/base-tests.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ echo "Running clippy base tests"
44

55
PATH=$PATH:./node_modules/.bin
66
if [ "$TRAVIS_OS_NAME" == "linux" ]; then
7-
remark -f *.md > /dev/null
7+
remark -f *.md -f doc/*.md > /dev/null
88
fi
99
# build clippy in debug mode and run tests
1010
cargo build --features debugging

‎clippy_dev/src/main.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ extern crate clap;
22
extern crate clippy_dev;
33
extern crate regex;
44

5-
use clap::{App, Arg, SubCommand};
5+
use clap::{App, AppSettings, Arg, SubCommand};
66
use clippy_dev::*;
77

88
#[derive(PartialEq)]
@@ -13,9 +13,11 @@ enum UpdateMode {
1313

1414
fn main() {
1515
let matches = App::new("Clippy developer tooling")
16+
.setting(AppSettings::SubcommandRequiredElseHelp)
1617
.subcommand(
1718
SubCommand::with_name("update_lints")
18-
.about(
19+
.about("Updates lint registration and information from the source code")
20+
.long_about(
1921
"Makes sure that:\n \
2022
* the lint count in README.md is correct\n \
2123
* the changelog contains markdown link references at the bottom\n \

‎clippy_lints/src/assign_ops.rs

Lines changed: 56 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
use crate::utils::{get_trait_def_id, implements_trait, snippet_opt, span_lint_and_then, SpanlessEq};
1+
use crate::utils::{
2+
get_trait_def_id, implements_trait, snippet_opt, span_lint_and_then, trait_ref_of_method, SpanlessEq,
3+
};
24
use crate::utils::{higher, sugg};
35
use if_chain::if_chain;
46
use rustc::hir;
@@ -68,52 +70,16 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssignOps {
6870
match &expr.node {
6971
hir::ExprKind::AssignOp(op, lhs, rhs) => {
7072
if let hir::ExprKind::Binary(binop, l, r) = &rhs.node {
71-
if op.node == binop.node {
72-
let lint = |assignee: &hir::Expr, rhs_other: &hir::Expr| {
73-
span_lint_and_then(
74-
cx,
75-
MISREFACTORED_ASSIGN_OP,
76-
expr.span,
77-
"variable appears on both sides of an assignment operation",
78-
|db| {
79-
if let (Some(snip_a), Some(snip_r)) =
80-
(snippet_opt(cx, assignee.span), snippet_opt(cx, rhs_other.span))
81-
{
82-
let a = &sugg::Sugg::hir(cx, assignee, "..");
83-
let r = &sugg::Sugg::hir(cx, rhs, "..");
84-
let long =
85-
format!("{} = {}", snip_a, sugg::make_binop(higher::binop(op.node), a, r));
86-
db.span_suggestion(
87-
expr.span,
88-
&format!(
89-
"Did you mean {} = {} {} {} or {}? Consider replacing it with",
90-
snip_a,
91-
snip_a,
92-
op.node.as_str(),
93-
snip_r,
94-
long
95-
),
96-
format!("{} {}= {}", snip_a, op.node.as_str(), snip_r),
97-
Applicability::MachineApplicable,
98-
);
99-
db.span_suggestion(
100-
expr.span,
101-
"or",
102-
long,
103-
Applicability::MachineApplicable, // snippet
104-
);
105-
}
106-
},
107-
);
108-
};
109-
// lhs op= l op r
110-
if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, l) {
111-
lint(lhs, r);
112-
}
113-
// lhs op= l commutative_op r
114-
if is_commutative(op.node) && SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, r) {
115-
lint(lhs, l);
116-
}
73+
if op.node != binop.node {
74+
return;
75+
}
76+
// lhs op= l op r
77+
if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, l) {
78+
lint_misrefactored_assign_op(cx, expr, *op, rhs, lhs, r);
79+
}
80+
// lhs op= l commutative_op r
81+
if is_commutative(op.node) && SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, r) {
82+
lint_misrefactored_assign_op(cx, expr, *op, rhs, lhs, l);
11783
}
11884
}
11985
},
@@ -140,13 +106,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssignOps {
140106
};
141107
// check that we are not inside an `impl AssignOp` of this exact operation
142108
let parent_fn = cx.tcx.hir().get_parent_item(e.hir_id);
143-
let parent_impl = cx.tcx.hir().get_parent_item(parent_fn);
144-
// the crate node is the only one that is not in the map
145109
if_chain! {
146-
if parent_impl != hir::CRATE_HIR_ID;
147-
if let hir::Node::Item(item) = cx.tcx.hir().get_by_hir_id(parent_impl);
148-
if let hir::ItemKind::Impl(_, _, _, _, Some(trait_ref), _, _) =
149-
&item.node;
110+
if let Some(trait_ref) = trait_ref_of_method(cx, parent_fn);
150111
if trait_ref.path.def.def_id() == trait_id;
151112
then { return; }
152113
}
@@ -234,6 +195,48 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssignOps {
234195
}
235196
}
236197

198+
fn lint_misrefactored_assign_op(
199+
cx: &LateContext<'_, '_>,
200+
expr: &hir::Expr,
201+
op: hir::BinOp,
202+
rhs: &hir::Expr,
203+
assignee: &hir::Expr,
204+
rhs_other: &hir::Expr,
205+
) {
206+
span_lint_and_then(
207+
cx,
208+
MISREFACTORED_ASSIGN_OP,
209+
expr.span,
210+
"variable appears on both sides of an assignment operation",
211+
|db| {
212+
if let (Some(snip_a), Some(snip_r)) = (snippet_opt(cx, assignee.span), snippet_opt(cx, rhs_other.span)) {
213+
let a = &sugg::Sugg::hir(cx, assignee, "..");
214+
let r = &sugg::Sugg::hir(cx, rhs, "..");
215+
let long = format!("{} = {}", snip_a, sugg::make_binop(higher::binop(op.node), a, r));
216+
db.span_suggestion(
217+
expr.span,
218+
&format!(
219+
"Did you mean {} = {} {} {} or {}? Consider replacing it with",
220+
snip_a,
221+
snip_a,
222+
op.node.as_str(),
223+
snip_r,
224+
long
225+
),
226+
format!("{} {}= {}", snip_a, op.node.as_str(), snip_r),
227+
Applicability::MachineApplicable,
228+
);
229+
db.span_suggestion(
230+
expr.span,
231+
"or",
232+
long,
233+
Applicability::MachineApplicable, // snippet
234+
);
235+
}
236+
},
237+
);
238+
}
239+
237240
fn is_commutative(op: hir::BinOpKind) -> bool {
238241
use rustc::hir::BinOpKind::*;
239242
match op {

‎clippy_lints/src/attrs.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
33
use crate::reexport::*;
44
use crate::utils::{
5-
in_macro, last_line_of_span, match_def_path, opt_def_id, paths, snippet_opt, span_lint, span_lint_and_sugg,
6-
span_lint_and_then, without_block_comments,
5+
in_macro, last_line_of_span, match_def_path, paths, snippet_opt, span_lint, span_lint_and_sugg, span_lint_and_then,
6+
without_block_comments,
77
};
88
use if_chain::if_chain;
99
use rustc::hir::*;
@@ -396,7 +396,7 @@ fn is_relevant_expr(tcx: TyCtxt<'_, '_, '_>, tables: &ty::TypeckTables<'_>, expr
396396
ExprKind::Ret(None) | ExprKind::Break(_, None) => false,
397397
ExprKind::Call(path_expr, _) => {
398398
if let ExprKind::Path(qpath) = &path_expr.node {
399-
if let Some(fun_id) = opt_def_id(tables.qpath_def(qpath, path_expr.hir_id)) {
399+
if let Some(fun_id) = tables.qpath_def(qpath, path_expr.hir_id).opt_def_id() {
400400
!match_def_path(tcx, fun_id, &paths::BEGIN_PANIC)
401401
} else {
402402
true

‎clippy_lints/src/default_trait_access.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use rustc::ty;
55
use rustc::{declare_tool_lint, lint_array};
66
use rustc_errors::Applicability;
77

8-
use crate::utils::{any_parent_is_automatically_derived, match_def_path, opt_def_id, paths, span_lint_and_sugg};
8+
use crate::utils::{any_parent_is_automatically_derived, match_def_path, paths, span_lint_and_sugg};
99

1010
declare_clippy_lint! {
1111
/// **What it does:** Checks for literal calls to `Default::default()`.
@@ -47,7 +47,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DefaultTraitAccess {
4747
if let ExprKind::Call(ref path, ..) = expr.node;
4848
if !any_parent_is_automatically_derived(cx.tcx, expr.hir_id);
4949
if let ExprKind::Path(ref qpath) = path.node;
50-
if let Some(def_id) = opt_def_id(cx.tables.qpath_def(qpath, path.hir_id));
50+
if let Some(def_id) = cx.tables.qpath_def(qpath, path.hir_id).opt_def_id();
5151
if match_def_path(cx.tcx, def_id, &paths::DEFAULT_TRAIT_METHOD);
5252
then {
5353
match qpath {

‎clippy_lints/src/drop_forget_ref.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::utils::{is_copy, match_def_path, opt_def_id, paths, span_note_and_lint};
1+
use crate::utils::{is_copy, match_def_path, paths, span_note_and_lint};
22
use if_chain::if_chain;
33
use rustc::hir::*;
44
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
@@ -124,7 +124,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
124124
if let ExprKind::Call(ref path, ref args) = expr.node;
125125
if let ExprKind::Path(ref qpath) = path.node;
126126
if args.len() == 1;
127-
if let Some(def_id) = opt_def_id(cx.tables.qpath_def(qpath, path.hir_id));
127+
if let Some(def_id) = cx.tables.qpath_def(qpath, path.hir_id).opt_def_id();
128128
then {
129129
let lint;
130130
let msg;

‎clippy_lints/src/explicit_write.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::utils::{is_expn_of, match_def_path, opt_def_id, resolve_node, span_lint, span_lint_and_sugg};
1+
use crate::utils::{is_expn_of, match_def_path, resolve_node, span_lint, span_lint_and_sugg};
22
use if_chain::if_chain;
33
use rustc::hir::*;
44
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
@@ -53,7 +53,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
5353
if let ExprKind::Call(ref dest_fun, _) = write_args[0].node;
5454
if let ExprKind::Path(ref qpath) = dest_fun.node;
5555
if let Some(dest_fun_id) =
56-
opt_def_id(resolve_node(cx, qpath, dest_fun.hir_id));
56+
resolve_node(cx, qpath, dest_fun.hir_id).opt_def_id();
5757
if let Some(dest_name) = if match_def_path(cx.tcx, dest_fun_id, &["std", "io", "stdio", "stdout"]) {
5858
Some("stdout")
5959
} else if match_def_path(cx.tcx, dest_fun_id, &["std", "io", "stdio", "stderr"]) {

0 commit comments

Comments
 (0)