* Added expression check for shared_code_in_if_blocks
* Finishing touches for the shared_code_in_if_blocks lint
* Applying PR suggestions
* Update lints yay
* Moved test into subfolder
Fix `redundant_clone` fp
fixes: #5973fixes: #5595fixes: #6998
changelog: Fix `redundant_clone` fp where the cloned value is modified while the clone is in use.
Lint: filter(Option::is_some).map(Option::unwrap)
Fixes#6061
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog:
* add new lint for filter(Option::is_some).map(Option::unwrap)
First Rust PR, so I'm sure I've violated some idioms. Happy to change anything.
I'm getting one test failure locally -- a stderr diff for `compile_test`. I'm having a hard time seeing how I could be causing it, so I'm tentatively opening this in the hopes that it's an artifact of my local setup against `rustc`. Hoping it can at least still be reviewed in the meantime.
I'm gathering that since this is a method lint, and `.filter(...).map(...)` is already checked, the means of implementation needs to be a little different, so I didn't exactly follow the setup boilerplate. My way of checking for method calls seems a little too direct (ie, "is the second element of the expression literally the path for `Option::is_some`?"), but it seems like that's how some other lints work, so I went with it. I'm assuming we're not concerned about, eg, closures that just end up equivalent to `Option::is_some` by eta reduction.
disable upper_case_acronyms for pub items - enum edition
Fixes https://github.com/rust-lang/rust-clippy/issues/6803 (again... 😅 )
My previous fix did not work for enums because enum variants were checked separately in the `check_variant` function but it looks like we can't use that because we can't tell if the enum the variants belong to is declared as public or not (it always said `Inherited` for me)
I went and special-cased enums and iterated over all the variants "manually", but only, if the enums is not public.
---
changelog: fix upper_case_acronyms still firing on public enums (#6803)
Refactor types
r? `@flip1995`
This is the last PR to close#6724🎉
Also, this fixes#6936.
changelog: `vec_box`: Fix FN in `const` or `static`
changelog: `linkedlist`: Fix FN in `const` or `static`
changelog: `option_option`: Fix FN in `const` or `static`
Improve `clone_on_copy`
This also removes the `clone_on_copy_mut` test as the same thing is covered in the `clone_on_copy` test.
changelog: `copy_on_clone` lint on chained method calls taking self by value
changelog: `copy_on_clone` only lint when using the `Clone` trait
changelog: `copy_on_clone` correct suggestion when the cloned value is a macro call.
Lint on `_.clone().method()` when method takes self by value
Set applicability correctly
Correct suggestion when the cloned value is a macro call. e.g. `m!(x).clone()`
Don't lint when not using the `Clone` trait
Improve `expl_impl_clone_on_copy`
fixes: #1254
changelog: Check to see if the generic constraints are the same as if using derive for `expl_impl_clone_on_copy`
`len_without_is_empty` improvements
fixes: #6958fixes: #6972
changelog: Check the return type of `len`. Only integral types, or an `Option` or `Result` wrapping one.
changelog: Ensure the return type of `is_empty` matches. e.g. `Option<usize>` -> `Option<bool>`
Check the return type of `len`. Only integral types, or an `Option` or `Result` wrapping one.
Ensure the return type of `is_empty` matches. e.g. `Option<usize>` -> `Option<bool>`
When the character next to `{}` is "shifted" (when mapping a byte index
in the format string to span) we should avoid shifting the span end
index, so first map the index of `}` to span, then bump the span,
instead of first mapping the next byte index to a span (which causes
bumping the end span too much).
Regression test added.
Fixes#83344
Fix bad suggestion when a reborrow might be required
Fix bad suggestion when the value being sliced is a macro call
Don't lint inside of a macro due to the previous context sensitive changes
search_is_some: add checking for `is_none()`
fixes: #6815
changelog: search_is_some: add checking for `is_none()`.
To be honest I don't know what is the process of renaming the lints. Appreciate any feedback if that needs to be handled differently. Thanks!
Fix bad suggestion for `match_single_binding` lint
Fix a bad suggestion that needs curly braces when the target `match` is the body of an arm.
Fixes#6572
changelog: none
`match_wildcard` improvements
fixes: #6604fixes: #5733fixes: #6862#5733 is only fixed in the normal case, if different paths are used for the variants then the same problem will occur. It's cause by `def_path_str` returning an utterly useless result. I haven't dug into why yet.
For #6604 there should be some discussion before accepting this. It's easy enough to change the message rather than disable the lint for `Option` and `Result`.
changelog: Attempt to find a common path prefix for `match_wildcard_for_single_variants` and `wildcard_enum_match_arm`
changelog: Don't lint op `Option` and `Result` for `match_wildcard_for_single_variants` and `wildcard_enum_match_arm`
changelog: Consider `or` patterns and `Self` prefix for `match_wildcard_for_single_variants` and `wildcard_enum_match_arm`
Don't lint on `Result` and `Option` types.
Considers `or` patterns.
Considers variants prefixed with `Self`
Suggestions will try to find a common prefix rather than just using the full path
Write literal suggestion
fixes: #6768
changelog: Add suggestion to `write_literal` and `print_literal` lints
changelog: Change `use_debug` to point only at the format string
wrong_self_convention: fix lint in case of `to_*_mut` method
fixes#6758
changelog: wrong_self_convention: fix lint in case of `to_*_mut` method. When a method starts with `to_` and ends with `_mut`, clippy expects a `&mut self` parameter, otherwise `&self`.
Any feedback is welcome. I was also thinking if shouldn't we treat `to_` the same way as `as_`. Namely to accept `self` taken: `&self` or `&mut self`.
replace span_lint with span_lint_and_sugg along with error message
fixes: #6874
changelog: none
apologies if this may not be the most idiomatic way of doing it, any advice on changes (if any) would be greatly appreciated.
mem_replace_with_default: recognize some std library ctors
fixes#6562
changelog: mem_replace_with_default: recognize some common constructors equivalent to `Default::default()`
Fix suggestion for `explicit_deref_methods`. Sometimes `&**` is needed, sometimes nothing is needed.
Allow `explicit_deref_methods` to trigger in a few new contexts.
`explicit_deref_methods` will now consider ufcs calls
Fix false positives on procedural macros of `missing_inline_in_public_items` lint
Fixes#6486.
changelog: Fix false positives on procedural macros of `missing_inline_in_public_items` lint.
Fix ICE 6840 - make is_normalizable more strict
fixes#6840
make `is_normalizable` more strict, which should catch this ICE and related cases
changelog: Fix ICE in [`zero_sized_map_values`]
Let Cargo track CLIPPY_ARGS
This PR makes `clippy-driver` emit `CLIPPY_ARGS` in its `dep-info` output.
Just like #6441, this allows this workflow to work:
```shell
cargo clippy # warning: empty `loop {}` wastes CPU cycles
cargo clippy -- -A clippy::empty_loop # no warnings emitted
```
But without rebuilding all dependencies.
cc https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/CLIPPY_ARGS.20is.20not.20tracked.20by.20Cargo/near/228599088
changelog: Cargo now re-runs Clippy if arguments after `--` provided to `cargo clippy` are changed.
Compare empty blocks for equality based on tokens
fixes: #1390
This only considers empty blocks for now, though we should also catch something like this:
```rust
match 0 {
0 => {
do_something();
trace!(0);
0
}
1 => {
do_something();
trace!(1);
1
}
x => x,
}
```
As far as I can tell there aren't any negative effects on other lints. These blocks only happen to be the same for a given compilation, not all compilations.
changelog: Fix `match_on_same_arms` and others. Only consider empty blocks equal if the tokens contained are the same.
Fix `manual_map` false positives
fixes: #6795fixes: #6797fixes: #6811fixes: #6819
changelog: Fix false positives for `manual_map` when `return`, `break`, `continue`, `yield`, `await`, and partially moved values are used.
changelog: Don't expand macros in suggestions for `manual_map`
lint message should not start with uppercase letters
lint messages should not have punctuation at the end of the last line
https://rustc-dev-guide.rust-lang.org/diagnostics.html#diagnostic-structure
The test reads through all the .stderr files in the testsuit and checks lint messages that start with "help: ", "error: " etc.
There is also an exception list for special messages that are deemed acceptable.
changelog: make sure lint messages conform with the rustc dev guide and add test
Fix false positive for unit_arg lint
Fixes#6447
To avoid false positives don't complain about unit args when they come from a path expression, e.g. a local variable.
**Note:** This is my first contribution to Clippy, so I might have messed up somewhere. Any feedback is welcome and I'm happy to work out any kinks.
---
changelog: Do not lint unit arguments when they come from a path expression.
or_fun_call: fix suggestion for `or_insert(vec![])`
fixes#6748
changelog: or_fun_call: fix suggestion for `or_insert(vec![])` on `std::collections::hash_map::Entry` or `std::collections::btree_map::Entry`
Applies for `std::collections::hash_map::Entry` and `std::collections::btree_map::Entry`
Example:
Previously, for the following code:
`let _ = hash_map.entry("test".to_owned()).or_insert(vec![]);`
clippy would suggest to use:
`or_insert_with(vec![])`, which causes a compiler error (E0277).
Now clippy suggests:
`or_insert_with(Vec::new)`
move upper_case_acronyms back to style, but make the default behaviour less aggressive by default (can be unleashed via config option)
Previous discussion in the bi-weekly clippy meeting for reference: https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Meeting.202021-02-23/near/227458019
Move the `upper_case_acronyms` lint back to the style group.
Only warn on fully-capitalized names by default.
Add add a clippy-config option `upper-case-acronyms-aggressive: true/false` to enabled more aggressive linting on
all substrings that could be capitalized acronyms.
---
changelog: reenable upper_case_acronyms by default but make the more aggressive linting opt-in via config option
Inconsistent struct constructor
fixes: #6352
r? `@matthiaskrgr`
I added the lint that checks for the struct constructors where the order of the field init shorthands is inconsistent with that in the struct definition.
changelog: Add style lint: `inconsistent_struct_constructor`
Fix FP in inherent_to_string when the function has generic parameters
Minimal example of the false positive:
````
struct G;
impl G {
fn to_string<const _N: usize>(&self) -> String {
"G.to_string()".to_string()
}
}
fn main() {
let g = G;
g.to_string::<1>();
}
````
Clippy emits an `inherent_to_string` warning, and suggests that we implement `Display` for `G` instead. However, this is not possible, since the generic parameter _N only exists in this function, not in `G` itself. This particular example uses const generics, which is where the issue is most likely to come up, but this PR skips the lint if the `to_string` function has any kind of generic parameters.
changelog: Fix FP in `inherent_to_string`
Teach SpanlessEq binding IDs
changelog: Fix collapsible_match false positive
Fixes#6740
This PR changes the way `SpanlessEq` determines whether two local variables are the same. Instead of checking that the names match, it checks that the `HirId`s match. If local bindings are declared within the expressions that are being compared, `SpanlessEq` will remember bindings that correspond to each other in a `FxHashMap<HirId, HirId>`. This makes `SpanlessEq` more flexible while also fixing false positives.
Example: `{ let x = 1; x + 2 }` is equal to `{ let y = 1; y + 2 }`.
CC `@xFrednet` I think this will resolve some concerns in #6463
Add the from_str_radix_10 lint
changelog: added the new `from_str_radix_10` which sometimes replaces calls to `primitive::from_str_radix` to `str::parse`
This is ready to be merged, although maybe the category should be `pedantic` instead of `style`? I'm not sure where it fits better.
Closes#6713.
Fix for issue 6640
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: unnecessary_wraps will now suggest to remove unnecessary wrapped return unit type, like Option<()>
fixes#6640
New lint: default_numeric_fallback
fixes#6064
r? `@flip1995`
As we discussed in [here](https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Issue.20.236064/near/224647188) and [here](https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Issue.20clippy.236064/near/224746333), I start implementing this lint from the strictest version.
In this PR, I'll allow the below two cases to pass the lint to reduce FPs.
1. Appearances of unsuffixed numeric literals in `Local` if `Local` has a type annotation, for example:
```rust
// Good.
let x: i32 = 1;
// Also good.
let x: (i32, i32) = if cond {
(1, 2)
} else {
(2, 3)
};
```
2. Appearances of unsuffixed numeric literals in args of `Call` or `MethodCall` if corresponding arguments of their signature have concrete types, for example:
```rust
fn foo_mono(x: i32) -> i32 {
x
}
fn foo_poly<T>(t: T) -> t {
t
}
// Good.
let x = foo_mono(13);
// Still bad.
let x: i32 = foo_poly(13);
```
changelog: Added restriction lint: `default_numeric_fallback`
These updates allow us to specify multiple testnames for `TESTNAME`.
The new version of compiletest-rs also includes `bless` support, but is
not enabled with this PR.
Do not lint when the closure is called using an iterator
Fix FP when the closure is used in an iterator for `blocks_in_if_conditions` lint
FIxes: #1141
changelog: none
Rework use_self impl based on ty::Ty comparison #3410 | Take 2
This builds on top of #5531
I already reviewed and approved the commits by `@montrivo.` So only the review of my commits should be necessary.
I would also appreciate your review `@montrivo,` since you are familiar with the challenges here.
Fixes#3410 and Fixes#4143 (same problem)
Fixes#2843Fixes#3859Fixes#4734 and fixes#6221Fixes#4305Fixes#5078 (even at expression level now 🎉)
Fixes#3881 and Fixes#4887 (same problem)
Fixes#3909
Not yet: #4140 (test added)
All the credit for the fixes goes to `@montrivo.` I only refactored and copy and pasted his code.
changelog: rewrite [`use_self`] lint and fix multiple (8) FPs. One to go.
Fix suggestions that need parens in `from_iter_instead_of_collect` lint
Fixes broken suggestions that need parens (i.e.: range)
Fixes: #6648
changelog: none
This renames the variants in HIR UnOp from
enum UnOp {
UnDeref,
UnNot,
UnNeg,
}
to
enum UnOp {
Deref,
Not,
Neg,
}
Motivations:
- This is more consistent with the rest of the code base where most enum
variants don't have a prefix.
- These variants are never used without the `UnOp` prefix so the extra
`Un` prefix doesn't help with readability. E.g. we don't have any
`UnDeref`s in the code, we only have `UnOp::UnDeref`.
- MIR `UnOp` type variants don't have a prefix so this is more
consistent with MIR types.
- "un" prefix reads like "inverse" or "reverse", so as a beginner in
rustc code base when I see "UnDeref" what comes to my mind is
something like "&*" instead of just "*".
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
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`
[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints
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`
Adds a new lint that checks if there is a semicolon on the last block statement if it returns nothing
changelog: Added a new lint: `SEMICOLON_IF_NOTHING_RETURNED`
fixes#6467
Adds the `SEMICOLON_IF_NOTHING_RETURNED` lint and therefore closes#6467.
This commit adds a new lint named `filter_map_identity`. This lint is
the same as `flat_map_identity` except that it checks for `filter_map`.
Closes#6643
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
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`.
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`.
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.
ref: https://github.com/rust-lang/rust-clippy/pull/4231
changelog: none
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.
ref: https://github.com/rust-lang/rust-clippy/pull/4231
Fix let_and_return false positive
The issue:
See this Rust playground link: https://play.rust-lang.org/?edition=2018&gist=12cb5d1e7527f8c37743b87fc4a53748
Run the above with clippy to see the following warning:
```
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.
changelog: none
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: c5273bdfb2 That change is blocked on figuring out what to do with https://github.com/rust-lang/rust/issues/80846 first.
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
The issue:
See this Rust playground link: https://play.rust-lang.org/?edition=2018&gist=12cb5d1e7527f8c37743b87fc4a53748
Run the above with clippy to see the following warning:
```
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.
Similar names ignore underscore prefixed names
changelog: Ignore underscore-prefixed names for similar_names
IMO, this lint is not very helpful for underscore-prefixed variables. Usually they are unused or are just there to ignore part of a destructuring.
Fix formatting for removed lints
- Don't add backticks for the reason a lint was removed. This is almost
never a code block, and when it is the backticks should be in the reason
itself.
- Don't assume clippy is the only tool that needs to be checked for
backwards compatibility
I split this out of https://github.com/rust-lang/rust/pull/80527/ because it kept causing tests to fail, and it's a good change to have anyway.
r? `@flip1995`
Doc markdown
I added "WebGL" along the lines of the existing "OpenGL" to the whitelist of `doc_markdown` as I found this to be a pretty common term.
(this is a follow-up of the now closed https://github.com/rust-lang/rust-clippy/pull/6388)
changelog: Whitelist "WebGL" in `doc_markdown`.
Fix false positive in write_literal and print_literal (numeric literals)
changelog: No longer lint numeric literals in [`write_literal`] and [`print_literal`].
Fixes#6335
Fix the reversed suggestion message of `stable_sort_primitive`.
Now Clippy emits `stable_sort_primitive` warning as follows:
```
warning: used sort instead of sort_unstable to sort primitive type `usize`
--> src\asm.rs:41:13
|
41 | self.successors.sort();
| ^^^^^^^^^^^^^^^^^^^^^^ help: try: `self.successors.sort_unstable()`
|
= note: `#[warn(clippy::stable_sort_primitive)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive
```
I think the position of `sort` and `sort_unstable` in the first line should be reversed.
changelog: Fix the reversed suggestion message of `stable_sort_primitive`.
Fix path_to_res for enum inherent items
changelog: none
I tried to add `Option::is_some` to the paths but got a false positive from the invalid paths lint. Turns out the `path_to_res` function does not find inherent impls for enums. I fixed this and took the liberty to do some additional cleanup in the method.
It is fairly common to divide some length in bytes by the byte-size of a
single element before creating a `from_raw_parts` slice or similar
operation. This lint would erroneously disallow such expressions.
Just in case, instead of simply disabling this lint in the RHS of a
division, keep track of the inversion and enable it again on recursive
division.
An upcoming test case for new expresssion variants make the stderr file
go over 200 lines. Split this test case in two to have a clear
distinction between checking whether the lint is still applying on
all the functions with member counts as argument, versus validating
various member-count expressions that may or may not be invalid.
New Lint: inspect_then_for_each
**Work In Progress**
This PR addresses [Issue 5209](https://github.com/rust-lang/rust-clippy/issues/5209) and adds a new lint called `inspect_then_for_each`.
Current seek some guidance.
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`
[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints
---
changelog: Add [`inspect_for_each`] lint for the use of `inspect().for_each()` on `Iterators`.
- Don't add backticks for the reason a lint was removed. This is almost
never a code block, and when it is the backticks should be in the reason
itself.
- Don't assume clippy is the only tool that needs to be checked for
backwards compatibility
New lint: redundant_slicing
changelog: Added lint: `redundant_slicing`
fixes#6519
This will trigger on any type which implements `Index<RangeFull>` that returns the input type. This would be a false positive if the implementation does something other than return itself, but I'm not sure why you would ever want to do that.
Fix the ICE 6539
Fixes#6539
It happened because `zero_sized_map_values` used `layout_of` with types from type aliases, which is essentially the same as the ICE 4968.
---
changelog: Fix an ICE in `zero_sized_map_values`
Fix FP with empty return for `needless_return` lint
This fixes a false positive in `needless_return` lint, when triggered in a closure using `return` statement without value.
Fixes: #6501
changelog: none
Case sensitive file extensions
Closes#6425
Looks for ends_with methods calls with case sensitive extension comparisons.
changelog: Add new lint that warns about case-sensitive file extension comparisons.
Catch `pointer::cast` too in `cast_ptr_alignment`
Fixes#4708
Although there were some discussion in the issue, this PR implements the original feature. I think `cast_ptr_alignment` should exist as it is, separated from `ptr_as_ptr`.
---
changelog: Extend `cast_ptr_alignment` lint for the `pointer::cast` method
Change env var used for testing Clippy
This changes the variable used for testing Clippy in the internal test
suite:
```
CLIPPY_TESTS -> __CLIPPY_INTERNAL_TESTS
```
`CLIPPY_TESTS` is understandably used in environments of Clippy users,
so we shouldn't use it in our test suite.
changelog: Fix oversight which caused Clippy to lint deps in some environments.
Once again fixes https://github.com/rust-lang/rust-clippy/issues/3874
Fix FP for `boxed_local` lint in default trait fn impl
Fix FP on default trait function implementation on `boxed_local` lint.
Maybe I checked too much when looking if `self` is carrying `Self` in its bound type.
I can't find a good test case for this, so it could be too much conservative.
Let me know if you think only detecting `self` parameter is enough.
Fixes: #4804
changelog: none
This changes the variable used for testing Clippy in the internal test
suite:
```
CLIPPY_TESTS -> __CLIPPY_INTERNAL_TESTS
```
`CLIPPY_TESTS` is understandably used in environments of Clippy users,
so we shouldn't use it in our test suite.
Add a new lint `ptr_as_ptr`
This PR adds a new lint `ptr_as_ptr` which checks for `as` casts between raw pointers without changing its mutability and suggest replacing it with `pointer::cast`. Closes#5890.
Open question: should this lint be `pedantic` or `style`? I set it `pedantic` for now because the original post suggests using it, but I think the lint also fits well to `style`.
---
changelog: New lint `ptr_as_ptr`
Fix: Empty enum never type suggested only if the feature is enabled
This PR addresses [Issue 6422](https://github.com/rust-lang/rust-clippy/issues/6422). Instead of always recommending `never type` for empty enums, Clippy would only recommend [the lint](https://rust-lang.github.io/rust-clippy/master/index.html#empty_enum) if [LatePass.TyCtxt](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/context/struct.TyCtxt.html) has `features().never_type` enabled.
- \[ ] 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: Only trigger [`empty_enum`] lint if `never_type` feature is enabled.
New lint: vec_init_then_push
fixes: #1483
This will trigger on `new`, `default`, and `with_capacity` when the given capacity is less than or equal to the number of push calls. Is there anything else this should trigger on?
changelog: Added lint: `vec_init_then_push`
This splits up clippy::collapsible_if into collapsible_if for
if x {
if y { }
}
=>
if x && y { }
and collapsible_else_if for
if x {
} else {
if y { }
}
=>
if x {
} else if y {
}
so that we can lint for only the latter but not the first if we desire.
changelog: collapsible_if: split up linting for if x {} else { if y {} } into collapsible_else_if lint
Ensure `Copy` exception in trait definition for `wrong_self_conventio…
Add a test case to ensure `Copy` exception is preserved also in trait definition, when passing `self` by value.
Follow up of #6316
changelog: none
Reassign default private
changelog: fix field_reassign_with_default false positive
* Fix#6344
* Fix assumption that `field: Default::default()` is the same as `..Default::default()`
* Cleanup some redundant logic
Added from_over_into lint
Closes#6456
Added a lint that searches for implementations of `Into<..>` and suggests to implement `From<..>` instead, as it comes with a default implementation of `Into`. Category: style.
changelog: added `from_over_into` lint
Lint also in trait def for `wrong_self_convention`
Extends `wrong_self_convention` to lint also in trait definition.
By the way, I think the `wrong_pub_self_convention` [example](dd826b4626/clippy_lints/src/methods/mod.rs (L197)) is misleading.
On [playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=32615ab3f6009e7e42cc3754be0ca17f), it fires `wrong_self_convention`, so the example (or the lint maybe?) needs to be reworked.
The difference with `wrong_self_convention` [example](dd826b4626/clippy_lints/src/methods/mod.rs (L172)) is mainly the `pub` keyword on the method `as_str`, but the lint doesn't use the function visibility as condition to choose which lint to fire (in fact it uses the visibility of the impl item).
fixes: #6307
changelog: Lint `wrong_self_convention` lint in trait def also
needless_doctest_main: handle correctly parse errors
Before this change, finding an error when parsing a doctest would make Clippy exit without emitting an error. Now we properly catch a fatal error and ignore it.
Also, if a doctest specifies an edition in the info line, it will be used when parsing it.
changelog: needless_doctest_main: handle correctly parse errors
Fixes#6022
make MIR graphviz generation use gsgdt
gsgdt [https://crates.io/crates/gsgdt] is a crate which provides an
interface for stringly typed graphs. It also provides generation of
graphviz dot format from said graph.
This is the first in a series of PRs on moving graphviz code out of rustc into normal crates and then implementating graph diffing on top of these crates.
r? `@oli-obk`
Fixing a false positive for the `match_single_binding` lint #5552
This is a fix for a false positive in the `match_single_binding` lint when using `#[cfg()]` on a branch. It is sadly a bit hacky but maybe the best solution as rust removes the other branch from the AST before we can even validate it. This fix looks at the code snippet itself and returns if it includes another thick arrow `=>` besides the one matching arm we found. This can again cause false negatives if someone has the following code:
```rust
match x {
// => <-- Causes a false negative
_ => 1,
}
```
I thought about making the code more complex and maybe validating against other things like the `#[cfg()]` macro but I believe that this is the best solution. This has basically switched the issue from a false positive to a false negative in a very specific case.
I'm happy to make some changes if you have any suggestions 🙃.
---
Fixes#5552
changelog: Fixed a false positive in the `match_single_binding` lint with `#[cfg()]` macro
📌 Pin Clippy to a nightly 📌
changelog: Pin Clippy to a specific nightly version (No more master/custom toolchain required to compile Clippy)
Addresses partially #5561. As proposed there in [this comment](https://github.com/rust-lang/rust-clippy/issues/5561#issuecomment-623109095), this kicks off the process, to help us get acquainted with how the syncs should work, before working on improving the tooling.
Open questions:
* When performing a rustup, we will need to exclude the commits that were merged that same day, or else wait until that nightly is released. I did not update the documentation about this part, mainly because I'm not sure about how to do that.
* When should we perform the rustups now? My first idea is to do it at the same time we do the clippyups, to have a clear cadence and to avoid the two copies of the repo to diverge enough to make the process painful.
* Who does the rustups now? If we follow my previous idea and do both rustup and clippyup at the same time, it would be more work for `@flip1995` who currently does the clippyups. I would prefer to establish some kind of rotation to spead the work. Other ideas?
* I'm not sure if this affects the release process in any way.
* ???
`@rust-lang/clippy` thoughts?
r? `@flip1995`
Add MSRV to more lints specified in #6097
add MSRV to more lints specified in #6097
add instructions for adding msrv in other lints
update tests
- [x] `redundant_field_names` requires Rust 1.17 due to suggest feature stablized in that version.
- [x] `redundant_static_lifetimes` requires Rust 1.17 due to suggest feature stablized in that version.
- [x] `filter_map_next` requires Rust 1.30 due to suggest `Iterator::find_map`.
- [x] `checked_conversions` requires Rust 1.34 due to suggest `TryFrom`.
- [x] `match_like_matches_macro` requires Rust 1.42 due to suggest `matches!`. Addressed in #6201
- [x] `manual_strip` requires Rust 1.45 due to suggest `str::{strip_prefix, strip_suffix}`. Addressed in #6201
- [x] `option_as_ref_deref` requires Rust 1.40 due to suggest `Option::{as_deref, as_deref_mut}`. Addressed in #6201
- [x] `manual_non_exhaustive` requires Rust 1.40 due to suggest `#[non_exhaustive]`. Addressed in #6201
- [x] `manual_range_contains` requires Rust 1.35 due to suggest `Range*::contains`.
- [x] `use_self` requires Rust 1.37 due to suggest `Self::Variant on enum`.
- [x] `mem_replace_with_default` requires Rust 1.40 due to suggest `mem::take`.
- [x] `map_unwrap_or` requires Rust 1.41 due to suggest `Result::{map_or, map_or_else}`.
- [x] `missing_const_for_fn` requires Rust 1.46 due to `match/if/loop in const fn` needs that version.
changelog: Add MSRV config to more lints. ^This is now the complete list, AFAWK
Add --no-deps option to avoid running on path dependencies in workspaces
Since rust-lang/cargo#8758 has hit nightly, this allows us to address the second bullet point and [the concern related to `--fix`](https://github.com/rust-lang/cargo/issues/8143#issuecomment-619289546) in the [RUSTC_WORKSPACE_WRAPPER tracking issue](https://github.com/rust-lang/cargo/issues/8143).
As a reminder stabilizing that env var will solve #4612 (Clippy not running after `cargo check` in stable) and would allow to stabilize the `--fix` option in Clippy.
changelog: Add `--no-deps` option to avoid running on path dependencies in workspaces
Fixes#3025
Add lint print_stderr
Resolves#6348
Almost identical to print_stdout, this lint applies to the `eprintln!` and `eprint!` macros rather than `println!` and `print!`.
changelog: Add new lint [`print_stderr`]. [`println_empty_string`] and [`print_with_newline`] now apply to `eprint!()` and `eprintln!()` respectively.
Added a lint-fraction-readability flag to the configuration
This adds an option to disable the `unreadable_literal` lint for floats with a longer fraction. This allows users to write `0.100200300` without getting a warning. Fixes#4176
I have some open questions about this PR:
1. I've named the option `lint-fraction-readability` is this a good name or should I rename it to something else?
2. What should the default configuration value be?
* The current default value is `true` as this was also the previous default.
3. Do I have to document this new option somewhere else or will it be extracted from the code comment?
4. The current fix option will also rewrite the fraction if the integer part violates the `unreadable_literal` lint it would otherwise also trigger the `inconsistent_digit_grouping` lint. Is this also okay?
* `1.100200300` will be unaffected by the fix function
* `100200300.100200300` will be effected and fixed to `100_200_300.100_200_300`
---
The project needed some getting used to but I'm happy with the result. A big thank you to `@flip1995` for giving me some pointers for this implementation and to everyone for the great introduction documentation!
---
changelog: Added the `unreadable-literal-lint-fractions` configuration to disable the `unreadable_literal` lint for fractions
Moved map_err_ignore to restriction and updated help message
This MR moves map_err_ignore lint from `pedantic` to the `restriction` category of lints and updates the help message to give the user an option to ignore the lint by naming the closure variable e.g. `.map_err(|_ignored| ...`
---
changelog: move map_err_ignore to restriction category
Specifically ptr::{sub, wrapping_sub, add, wrapping_add, offset, wrapping_offset} and slice::{from_raw_parts, from_raw_parts_mut}
The lint now also looks for size_of calls through casts (Since offset takes an isize)
Also fix review comments:
- Use const arrays and iterate them for the method/function names
- merge 2 if_chain's into one using a rest pattern
- remove unnecessary unsafe block in test
And make the lint only point to the count expression instead of the entire function call
Add Collapsible match lint
changelog: Add collapsible_match lint
Closes#1252Closes#2521
This lint finds nested `match` or `if let` patterns that can be squashed together. It is designed to be very conservative to only find cases where merging the patterns would most likely reduce cognitive complexity.
Example:
```rust
match result {
Ok(opt) => match opt {
Some(x) => x,
_ => return,
}
_ => return,
}
```
to
```rust
match result {
Ok(Some(x)) => x,
_ => return,
}
```
These criteria must be met for the lint to fire:
* The inner match has exactly 2 branches.
* Both the outer and inner match have a "wild" branch like `_ => ..`. There is a special case for `None => ..` to also be considered "wild-like".
* The contents of the wild branches are identical.
* The binding which "links" the matches is never used elsewhere.
Thanks to the hir, `if let`'s are easily included with this lint since they are desugared into equivalent `match`'es.
I think this would fit into the style category, but I would also understand changing it to pedantic.
add `internal-lints` feature to enable clippys internal lints (off by default)
This PR moves the internal lint tests into a new subdirectory (I couldn't find a different way to compile-time-conditionally exclude them from compiletest) and only builds and tests internal lints if the `internal-lints` feature is enabled.
Fixes#6306
changelog: put internal lints behind a feature ("internal-lints")
Update error to reflect that integer literals can have float suffixes
For example, `1` is parsed as an integer literal, but it can be turned
into a float with the suffix `f32`. Now the error calls them "numeric
literals" and notes that you can add a float suffix since they can be
either integers or floats.
Part of #68490.
Care has been taken to leave the old consts where appropriate, for testing backcompat regressions, module shadowing, etc. The intrinsics docs were accidentally referring to some methods on f64 as std::f64, which I changed due to being contrary with how we normally disambiguate the shadow module from the primitive. In one other place I changed std::u8 to std::ops since it was just testing path handling in macros.
For places which have legitimate uses of the old consts, deprecated attributes have been optimistically inserted. Although currently unnecessary, they exist to emphasize to any future deprecation effort the necessity of these specific symbols and prevent them from being accidentally removed.
Enhance `redundant_pattern_matching` to also lint on `std::net::IpAddr`
Follow-up to #6339
r? `@ebroto`
(note: also contains a small cleanup of the other ui tests)
changelog: Enhance [`redundant_pattern_matching`] to also lint on `std::net::IpAddr`
Add suspicious_operation_groupings lint
This is my (<del> currently WIP </del>) attempt to close#6039.
changelog: Added `suspicious_operation_groupings` lint.
For example, `1` is parsed as an integer literal, but it can be turned
into a float with the suffix `f32`. Now the error calls them "numeric
literals" and notes that you can add a float suffix since they can be
either integers or floats.
run `cargo dev new_lint --category correctness --name suspicious_chained_operators --pass early`
add (currently failing) tests for suspicious_chained_operators
add some tests to answer a question that came up during implementation
write usage code for functions we'll need to find or create
Complete left-right tracking TODO
get it compiling with several `todo!` invocations.
refactor to a set of incomplete functions that don't expect to be able to edit a `Span`
create placeholder for `suggestion_with_swapped_ident` function and correct some comments
add `inside_larger_boolean_expression` test
fill out `get_ident` and `suggestion_with_swapped_ident`
Implementi the `IdentIter`
start on implementing the `IdentIter`
handle the `ExprKind::Path` case in `IdentIter`
on second thought, make the iterator type dynamic so we don't need an explicit type for each one we will need
handle `ExprKind::MacCall` in `IdentIter`
Try handling `box x` expressions
restructure `IdentIter`
set `self.done` when returning `None`
Handle `ExprKind::Array`
reduce duplication with a macro that we expect to use several more times
handle ExprKind::Call
add `new_p` convenience method
handle `MethodCall`
handle `Tup` and `Binary`
handle `Unary`
simplify by not returning an additional `Expr` from the `IdentIter`
add cross product test against false positives
rename suspicious_chained_operators to suspicious_operation_groupings within files
For the record, the exact commands run were:
find . -type f -name "*.md" -exec sed -i 's/suspicious_chained_operators/suspicious_operation_groupings/g' {} +
find . -type f -name "*.rs" -exec sed -i 's/suspicious_chained_operators/suspicious_operation_groupings/g' {} +
find . -type f -name "*.rs" -exec sed -i 's/SUSPICIOUS_CHAINED_OPERATORS/SUSPICIOUS_OPERATION_GROUPINGS/g' {} +
find . -type f -name "*.rs" -exec sed -i 's/SuspiciousChainedOperators/SuspiciousOperationGroupings/g' {} +
Also:
rename file to match module name
rename test file to match lint name
start implementing `IdentDifference` creation
add `IdentIter` utility
use `ident_iter::IdentIter`
fix bug in `suggestion_with_swapped_ident`
add `inside_if_statements` test
implement `Add` `todo`s
register `SuspiciousOperationGroupings` lint pass
fill in `chained_binops`, and fill in a stopgap version of `ident_difference_expr`, but then notice that the lint does not seem to ever be run in the tests
run `cargo dev update_lints` and not that the `suspicious_operation_groupings` lint still does not seem to be run
fix base index incrementing bug
fix paired_identifiers bug, and remove ident from `Single`
change help prefix and note our first successful lint messages!
add odd_number_of_pairs test
get the `non_boolean_operators` test passing, with two copies of the error message
extract `is_useless_with_eq_exprs` so we can know when `eq_op` will already handle something
add `not_caught_by_eq_op` tests since `s1.b * s1.b` was (reasonably) not caught by `eq_op`
cover the case where the change should be made on either side of the expression with `not_caught_by_eq_op` tests
produce the expected suggestion on the `not_caught_by_eq_op_middle_change_left` test
confirm that the previous tests still pass and update references
fix early continue bug and get `not_caught_by_eq_op_middle_change_right` passing
note that `not_caught_by_eq_op_start` already passes
fix bugs based on misunderstanding of what `Iterator::skip` does, and note that `not_caught_by_eq_op_end` now passes
add several parens tests and make some of them pass
handle parens inside `chained_binops_helper` and note that this makes several tests pass
get `inside_larger_boolean_expression_with_unsorted_ops` test passing by extracting out `check_same_op_binops` function
also run `cargo dev fmt`
note that `inside_function_call` already passes
add another `if_statement` test
remove the matching op requirement, making `inside_larger_boolean_expression_with_unsorted_ops` pass
prevent non-change suggestions from being emitted
get the `Nested` tests passing, and remove apparently false note about eq_op
add a test to justify comment in `ident_difference_expr_with_base_location` but find that the failure mode seems different than expected
complete `todo` making `do_not_give_bad_suggestions_for_this_unusual_expr` pass and add some more tests that already pass
add test to `eq_op`
note that `inside_fn_with_similar_expression` already passes
fix `inside_an_if_statement` and note that it already passes
attempt to implement if statement extraction and notice that we don't seem to handle unary ops correctly
add `maximum_unary_minus_right_tree` test and make it pass
add two tests and note one of them passes
filter out unary operations in several places, and find that the issue seems to be that we don't currently recognize the error in `multiple_comparison_types_and_unary_minus` even so.
remove filtering that was causing bad suggestions
remove tests that were deemed too much for now
run `cargo dev fmt`
correct eq_op post-merge
fill out the description and delete debugging code
run `cargo dev update_lints`
update eq_op references
add parens to work around rustfmt issue #3666 and run rustfmt
https://github.com/rust-lang/rustfmt/issues/3666#issuecomment-714612257
update references after formatting
fix dogfood issues
fix multi-cursor edit
fix missed dogfood error
fix more dogfood pedantic issues, including function length
even more nesting
insert hidden definition of Vec3 so docs compile
add spaces to second struct def
reword test description comment
Co-authored-by: llogiq <bogusandre@gmail.com>
add local `use BinOpKind::*;`
Apply suggestions from code review
Co-authored-by: llogiq <bogusandre@gmail.com>
switch `SUSPICIOUS_OPERATION_GROUPINGS` to a style lint
run `cargo dev update_lints`
put both usages of `op_types` in the same closure to satisfy `borrowck`
fix compile error
Change `redundant_pattern_matching` to also lint `std::task::Poll`
`reduntant_pattern_matching` currently lints pattern matching on `Option` and `Result` where the `is_variant` utility methods could be used instead: `is_some`, `is_none`, `is_ok`, `is_err`. This PR extends this behaviour to `std::task::Poll`, suggesting the methods `is_pending` and `is_ready`.
Motivation: The current description of `redundant_pattern_matching` mentions
> It's more concise and clear to just use the proper utility function
which in my mind applies to `Poll` as well.
changelog: Enhance [`redundant_pattern_matching`] to also lint on `std::task::Poll`
Qualify `panic!` as `core::panic!` in non-built-in `core` macros
Fixes#78333.
-----
Otherwise code like this
#![no_implicit_prelude]
fn main() {
::std::todo!();
::std::unimplemented!();
}
will fail to compile, which is unfortunate and presumably unintended.
This changes many invocations of `panic!` in a `macro_rules!` definition
to invocations of `$crate::panic!`, which makes the invocations hygienic.
Note that this does not make the built-in macro `assert!` hygienic.
Otherwise code like this
#![no_implicit_prelude]
fn main() {
::std::todo!();
::std::unimplemented!();
}
will fail to compile, which is unfortunate and presumably unintended.
This changes many invocations of `panic!` in a `macro_rules!` definition
to invocations of `$crate::panic!`, which makes the invocations hygienic.
Note that this does not make the built-in macro `assert!` hygienic.
