bors [Mon, 8 Feb 2021 06:02:29 +0000 (06:02 +0000)]
Auto merge of #6696 - dtolnay-contrib:regex, r=Manishearth
Downgrade trivial_regex to nursery
See #6690. I think there is still value in a trivial_regex lint, but only if clippy can tell that the regex is only ever constructed and applied to a single input.
```rust
let regex = Regex::new("trivial_regex")?;
println!("{}", regex.is_match(s));
// `regex` never used again
```
---
changelog: remove `trivial_regex` from default set of enabled lints
bors [Sun, 7 Feb 2021 13:25:00 +0000 (13:25 +0000)]
Auto merge of #6686 - matthiaskrgr:lintcheck_git, r=flip1995
lintcheck: support git sources
This adds support for git sources in `cargo dev-lintcheck`
You can add a git source to `clippy_dev/lintcheck_crates.toml` by having a `git_url` and a `git_hash` key instead of the `versions` array.
The repo will the be cloned and checked out to the requested commit before checking it with clippy.
bors [Sun, 7 Feb 2021 11:15:44 +0000 (11:15 +0000)]
Auto merge of #6674 - phlip9:disallowed_functions, r=llogiq
disallowed_methods: Support functions in addition to methods
## Context:
Hey all! I have a particular use case where I'd like to ban certain functions in a code base I work on. For example, I want to ban `Instant::now()` (among others) as we have a time service for mocking time in deterministic simulation tests. Obviously, it doesn't make sense to include a lint like this for all clippy users. Imagine my excitement when I spotted the new `disallowed_methods` lint in clippy--perfect! Unfortunately, after playing around with it for a bit, I was frustrated to realize that it didn't support functions like `Instant::now()`, so I've added support for them in this PR.
It might also make sense to rename the lint from `disallowed_methods` -> `disallowed_functions`, though I've held off from including that rename in this change, since I'm unsure of clippy's breaking change policy.
## Change
Support functions in addition to methods. In other words, support:
`disallowed_methods = ["alloc::vec::Vec::new"]` (a function) in addition to
`disallowed_methods = ["alloc::vec::Vec::leak"]` (a method).
Improve the documentation to clarify that users must specify the full qualified path for each disallowed function, which can be confusing for reexports. Include an example `clippy.toml`.
Simplify the actual lint pass so we can reuse `utils::fn_def_id`.
changelog: disallowed_method: Now supports functions in addition to methods
bors [Sun, 7 Feb 2021 09:55:32 +0000 (09:55 +0000)]
Auto merge of #6685 - magurotuna:filter_map_identity, r=phansch
Add new lint `filter_map_identity`
<!--
Thank you for making Clippy better!
We're collecting our changelog from pull request descriptions.
If your PR only includes internal changes, you can just write
`changelog: none`. Otherwise, please write a short comment
explaining your change.
If your PR fixes an issue, you can add "fixes #issue_number" into this
PR description. This way the issue will be automatically closed when
your PR is merged.
If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.
- \[x] Followed [lint naming conventions][lint_naming]
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[x] Executed `cargo dev update_lints`
- \[x] Added lint documentation
- \[x] Run `cargo dev fmt`
Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.
Delete this line and everything above before opening your PR.
-->
This commit adds a new lint named filter_map_identity.
This lint is the same as `flat_map_identity` except that it checks for the usage of `filter_map`.
---
Closes #6643
changelog: Added a new lint: `filter_map_identity`
bors [Sun, 7 Feb 2021 08:40:41 +0000 (08:40 +0000)]
Auto merge of #6660 - camsteffen:path-to-local, r=llogiq
Cleanup path-to-local checks
changelog: none
It seemed like too much ceremony going on to check if an expression matches a variable. So I created two util functions `path_to_local(Expr) -> Option<HirId>` and `path_to_local_id(Expr, HirId) -> bool` to make this easier, and used them wherever applicable.
I changed logic in a few places to use `HirId` instead of `Symbol` where it was easy to do so. I believe this is more correct and may even fix some bugs.
I also removed some calls to `qpath_res`. This is not needed if you are only looking for a `Res::Local`.
As a note, I wanted to name the util functions in a way that encourages understanding of the HIR.
bors [Sat, 6 Feb 2021 13:38:36 +0000 (13:38 +0000)]
Auto merge of #6684 - matthiaskrgr:versiontest_dont_break_upstream, r=flip1995
tests: ignore check_that_clippy_has_the_same_major_version_as_rustc()inside the rustc repo
Do not check if clippy version matches rustc version when runnning tests inside the rustc repo.
This makes sure that upstream rustc maintainers do not have to deal with our test failing/mismatching versions
when the rustc version bump is happening.
cc #6683
We already do the "don't run inside the rustc-repo" workaround for the dogfood test:
https://github.com/rust-lang/rust-clippy/blob/a507c27660d05f37307369d30bee9e82ce3a11e1/tests/dogfood.rs#L16
Matthias Krüger [Fri, 5 Feb 2021 23:56:54 +0000 (00:56 +0100)]
tests: ignore check_that_clippy_has_the_same_major_version_as_rustc() inside the rustc repo.
Do not check if clippy version matches rustc version when runnning tests inside the rustc repo.
This makes sure that upstream rustc maintainers do not have to deal with our test failing/mismatching versions
when the rustc version bump is happening.
cc #6683
bors [Fri, 5 Feb 2021 17:48:46 +0000 (17:48 +0000)]
Auto merge of #6679 - flip1995:ra_setup_abs_path, r=Manishearth
Use absolute path to Rust repo in ra_setup
This will convert the path to the Rust repo to an absolute path. This is
important for the clippy_lints/Cargo.toml file. Otherwise if a relative
path is passed, rst-analyzer won't find the Rust repo, because it starts
the relative path search from the clippy_lints dir, not the
rust-clippy dir where the ra_setup command was run from.
flip1995 [Fri, 5 Feb 2021 16:23:04 +0000 (17:23 +0100)]
Use absolute path to Rust repo in ra_setup
This will convert the path to the Rust repo to an absolute path. This is
important for the clippy_lints/Cargo.toml file. Otherwise if a relative
path is passed, rst-analyzer won't find the Rust repo, because it starts
the relative path search from the clippy_lints dir, not the
rust-clippy dir where the ra_setup command was run from.
Philip Hayes [Sat, 30 Jan 2021 06:18:56 +0000 (22:18 -0800)]
Support free functions in disallowed-methods lint
In other words, support:
`disallowed_methods = ["alloc::vec::Vec::new"]` (a free function) in
addition to
`disallowed_methods = ["alloc::vec::Vec::leak"]` (a method).
Improve the documentation to clarify that users must specify the full
qualified path for each disallowed function, which can be confusing for
reexports. Include an example `clippy.toml`.
Simplify the actual lint pass so we can reuse `utils::fn_def_id`.
bors [Thu, 4 Feb 2021 10:03:45 +0000 (10:03 +0000)]
Auto merge of #6646 - nahuakang:for_loops_over_options_or_results, r=flip1995
New Lint: Manual Flatten
This is a draft PR for [Issue 6564](https://github.com/rust-lang/rust-clippy/issues/6564).
r? `@camsteffen`
- \[x] Followed [lint naming conventions][lint_naming]
- \[x] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[x] Executed `cargo dev update_lints`
- \[x] Added lint documentation
- \[x] Run `cargo dev fmt`
---
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: Add new lint [`manual_flatten`] to check for loops over a single `if let` expression with `Result` or `Option`.
bors [Wed, 3 Feb 2021 15:39:37 +0000 (15:39 +0000)]
Auto merge of #6669 - magurotuna:fix-test-name, r=flip1995
Fix file names of flat_map_identity test
This patch fixes the file names of the `flat_map_identity` test.
Previously, their names were started with `unnecessary_flat_map` even though the lint rule name is `flat_map_identity`. This inconsistency happened probably because the rule name was changed during the discussion in the PR where this rule was introduced.
Yusuke Tanaka [Wed, 3 Feb 2021 15:06:26 +0000 (00:06 +0900)]
Fix file names of flat_map_identity test
This commit fixes the file names of the `flat_map_identity` test.
Previously, their names were started with `unnecessary_flat_map` even
though the lint rule name is `flat_map_identity`. This inconsistency
happened probably because the rule name was changed during the
discussion in the PR where this rule was introduced.
Jack Huey [Tue, 2 Feb 2021 21:01:33 +0000 (16:01 -0500)]
Rollup merge of #81260 - vn971:restore-editorconfig, r=Mark-Simulacrum
Add .editorconfig
This adds a .editorconfig file to rust-lang/rust, matching Clippy's. It's not clear that this will benefit many people, but the cost is low and the rewards are potentially meaningful.
bors [Tue, 2 Feb 2021 18:05:35 +0000 (18:05 +0000)]
Auto merge of #6639 - xFrednet:0000-configuration-documentation-some-nits, r=flip1995
Updated some NITs in the documentation from #6630
I've implemented the two suggestions from #6630 that were added after the merge. This PR also changes the example code to use `register_*_pass` instead of `register_late_pass`. I'm not sure if this is better or worse, but it makes it clearer in my opinion. Let me know if I should change it back.
Vasili Novikov [Fri, 22 Jan 2021 08:51:23 +0000 (09:51 +0100)]
Add .editorconfig
Editorconfig is a lightweight specification that
helps maintaining consistent coding/formatting style
accross editors, especially those editors
that are not explicitly aware of Rust and rustfmt.
bors [Tue, 2 Feb 2021 15:37:24 +0000 (15:37 +0000)]
Auto merge of #6523 - brightly-salty:missing-panic-doc, r=flip1995
Add new lint "missing_panics_doc"
fixes #1974
changelog: Added the "missing_panics_doc" lint which lints when public functions that may panic are missing "# Panics" in their doc comment
```
warning: returning the result of a `let` binding from a block
--> src/main.rs:24:5
|
23 | let value = Foo::new(&x).value();
| --------------------------------- unnecessary `let` binding
24 | value
| ^^^^^
|
= note: `#[warn(clippy::let_and_return)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
help: return the expression directly
|
23 |
24 | Foo::new(&x).value()
|
```
Implementing the suggested fix, removing the temporary let binding,
yields a compiler error:
```
error[E0597]: `x` does not live long enough
--> src/main.rs:23:14
|
23 | Foo::new(&x).value()
| ---------^^-
| | |
| | borrowed value does not live long enough
| a temporary with access to the borrow is created here ...
24 | }
| -
| |
| `x` dropped here while still borrowed
| ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `Foo`
|
= note: the temporary is part of an expression at the end of a block;
consider forcing this temporary to be dropped sooner, before the block's local variables are dropped
help: for example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block
|
23 | let x = Foo::new(&x).value(); x
| ^^^^^^^ ^^^
```
The fix:
Of course, clippy looks like it should already handle this edge case;
however, it appears `utils::fn_def_id` is not returning a `DefId` for
`Foo::new`. Changing the `qpath_res` lookup to use the child Path
`hir_id` instead of the parent Call `hir_id` fixes the issue.
bors [Mon, 1 Feb 2021 10:25:31 +0000 (10:25 +0000)]
Auto merge of #80851 - m-ou-se:panic-2021, r=petrochenkov
Implement Rust 2021 panic
This implements the Rust 2021 versions of `panic!()`. See https://github.com/rust-lang/rust/issues/80162 and https://github.com/rust-lang/rfcs/pull/3007.
It does so by replacing `{std, core}::panic!()` by a bulitin macro that expands to either `$crate::panic::panic_2015!(..)` or `$crate::panic::panic_2021!(..)` depending on the edition of the caller.
This does not yet make std's panic an alias for core's panic on Rust 2021 as the RFC proposes. That will be a separate change: https://github.com/rust-lang/rust/pull/80879/commits/c5273bdfb266c35e8eab9413aa8d58d27fdbe114 That change is blocked on figuring out what to do with https://github.com/rust-lang/rust/issues/80846 first.
bors [Sun, 31 Jan 2021 15:09:55 +0000 (15:09 +0000)]
Auto merge of #6603 - ThibsG:MatchOverlappingArm5986, r=flip1995
Do not lint when range is completely included into another one
This fix has been developed following this [comment](https://github.com/rust-lang/rust-clippy/issues/5986#issuecomment-703313548).
So this will be linted:
```
|----------|
|-----------|
```
Now this won't be linted:
```
|---|
|--------------------|
```
and this will still lint:
```
|--------|
|--------------|
```
Fixes: #5986
changelog: Fix FPs in match_overlapping_arm, when first arm is completely included in second arm
bors [Sun, 31 Jan 2021 14:56:53 +0000 (14:56 +0000)]
Auto merge of #6656 - phansch:command-failed-print-stderr, r=flip1995
clippy_dev: Pass stderr to CommandFailed
This improves error reporting when running `rustfmt` fails for some reason, as seen [on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Issue.20with.20rustfmt). It will now include the stderr output in the `CliError::CommandFailed` error.
bors [Sat, 30 Jan 2021 16:49:45 +0000 (16:49 +0000)]
Auto merge of #6654 - flip1995:no_lazy_static_regex, r=flip1995
No lazy static regex
r? `@llogiq`
#6500
regex is unnecessary for this lint (https://github.com/rust-lang/rust-clippy/pull/6500#discussion_r558555071)
lazy_static is unnecessary. The std lazy feature should be used instead.
```
warning: returning the result of a `let` binding from a block
--> src/main.rs:24:5
|
23 | let value = Foo::new(&x).value();
| --------------------------------- unnecessary `let` binding
24 | value
| ^^^^^
|
= note: `#[warn(clippy::let_and_return)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
help: return the expression directly
|
23 |
24 | Foo::new(&x).value()
|
```
Implementing the suggested fix, removing the temporary let binding,
yields a compiler error:
```
error[E0597]: `x` does not live long enough
--> src/main.rs:23:14
|
23 | Foo::new(&x).value()
| ---------^^-
| | |
| | borrowed value does not live long enough
| a temporary with access to the borrow is created here ...
24 | }
| -
| |
| `x` dropped here while still borrowed
| ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `Foo`
|
= note: the temporary is part of an expression at the end of a block;
consider forcing this temporary to be dropped sooner, before the block's local variables are dropped
help: for example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block
|
23 | let x = Foo::new(&x).value(); x
| ^^^^^^^ ^^^
```
The fix:
Of course, clippy looks like it should already handle this edge case;
however, it appears `utils::fn_def_id` is not returning a `DefId` for
`Foo::new`. Changing the `qpath_res` lookup to use the child Path
`hir_id` instead of the parent Call `hir_id` fixes the issue.
Yuki Okushi [Fri, 29 Jan 2021 00:17:32 +0000 (09:17 +0900)]
Rollup merge of #81176 - camsteffen:qpath-res, r=oli-obk
Improve safety of `LateContext::qpath_res`
This is my first rustc code change, inspired by hacking on clippy!
The first change is to clear cached `TypeckResults` from `LateContext` when visiting a nested item. I took a hint from [here](https://github.com/rust-lang/rust/blob/5e91c4ecc09312d8b63d250a432b0f3ef83f1df7/compiler/rustc_privacy/src/lib.rs#L1300).
Clippy has a `qpath_res` util function to avoid a possible ICE in `LateContext::qpath_res`. But the docs of `LateContext::qpath_res` promise no ICE. So this updates the `LateContext` method to keep its promises, and removes the util function.
Related: rust-lang/rust-clippy#4545
CC ````````````@eddyb```````````` since you've done related work
CC ````````````@flip1995```````````` FYI
Yuki Okushi [Thu, 28 Jan 2021 06:09:02 +0000 (15:09 +0900)]
Rollup merge of #79951 - LeSeulArtichaut:ty-ir, r=nikomatsakis
Refractor a few more types to `rustc_type_ir`
In the continuation of #79169, ~~blocked on that PR~~.
This PR:
- moves `IntVarValue`, `FloatVarValue`, `InferTy` (and friends) and `Variance`
- creates the `IntTy`, `UintTy` and `FloatTy` enums in `rustc_type_ir`, based on their `ast` and `chalk_ir` equilavents, and uses them for types in the rest of the compiler.
~~I will split up that commit to make this easier to review and to have a better commit history.~~
EDIT: done, I split the PR in commits of 200-ish lines each
bors [Tue, 26 Jan 2021 06:58:04 +0000 (06:58 +0000)]
Auto merge of #6469 - matthiaskrgr:clippy_dev_crater, r=flip1995
add "cargo dev crater" to run clippy on a fixed set of crates and diff the lint warnings
`cargo dev crater` now does the following:
build clippy in debug mode
for a fixed set of crates:
download and extract the crate
run compiled clippy on the crate
dump the warnings into a file that is inside the repo
We can then do a "git diff" and see what effects our clippy changes had on a tiny fraction of the rust ecosystem and can see when an change unexpectedly added or silenced a lot of warnings.
Checking all the crates took less than 5 minutes on my system.
Should help with https://github.com/rust-lang/rust-clippy/issues/6429
---
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: extend cargo dev to run clippy against a fixed set of crates and compare warnings
I'm not sure if the guide is written in the best way. Style suggestions are appreciated. :upside_down_face:
---
Again a big **thank you** for everyone who helped to collect the abbreviation list over on [zulip]. I had a lot of fun, and it was also very informative. Keep up the good work :upside_down_face: