Refactor: Use rustc's `match_def_path`
This replaces our match_def_path implementation with the rustc one.
Note that we can't just use it in all call sites because of the
`&[&str]` / `&[Symbol]` difference in Clippy/rustc.
changelog: none
This replaces our match_def_path implementation with the rustc one.
Note that we can't just use it in all call sites because of the
`&[&str]` / `&[Symbol]` difference in Clippy/rustc.
Make use of more diagnostic items
This makes use of some (not all) already existing diagnostic items. Specifically:
* 79982a2: `core::mem::uninitialized`, `core::mem::zeroed`, `alloc::sync::Arc`, `alloc::sync::Rc`
* 83874d0: `Option` and `Result`
cc #5393
changelog: none
Fixes#5405: redundant clone false positive with arrays
Check whether slice elements implement Copy before suggesting to drop
the clone method
changelog: add a check for slice indexing on redundant_clone lint
Update documentation for new_ret_no_self
changelog: Update documentation for lint new_ret_no_self to reflect that the return type must only contain `Self`, not be `Self`
The lint was changed to be more lenient than the documentation implies in PR #3338 (Related issue #3313)
Change the existing hex bit mask (`0x1111`) to a binary one (`0b1111`).
The former does not seem to have anything to do with trailing zeros and is
probably a typo.
This change adds a check to the `inconsistent_digit_grouping` to add a check for
NumericLiterals that follow the UUID format of 8-4-4-4-12.
If the NumericLiteral matches the UUID format, no further inconsistent grouping checks
will be performed.
Closes#5431
Check for clone-on-copy in argument positions
Earlier if arguments to method calls matched the above pattern they were
not reported. This patch ensures such arguments are checked as well.
Fixes#5436
changelog: apply clone_on_copy lint to func args as well
Earlier if arguments to method calls matched the above pattern they were
not reported. This patch ensures such arguments are checked as well.
Fixes#5436
Downgrade implicit_hasher to pedantic
From the [documentation](https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher), this lint is intended to suggest:
```diff
- pub fn foo(map: &mut HashMap<i32, i32>) { }
+ pub fn foo<S: BuildHasher>(map: &mut HashMap<i32, i32, S>) { }
```
I think this is pedantic. I get that this lint can benefit core libraries like serde, but that's exactly the use case for pedantic lints; a library like serde will [enable clippy_pedantic](fd6741f4b0/src/lib.rs (L304)) and take the time to go through everything possible. Similar for libraries doing a libz blitz style checkup before committing to a 1.0 release; it would make sense to run through all the available pedantic lints then.
But otherwise, for most codebases and certainly for industrial codebases, the above suggested change just makes the codebase more obtuse for questionable benefit.
changelog: Remove implicit_hasher from default set of enabled lints
Check fn header along with decl when suggesting to implement trait
When checking for functions that are potential candidates for trait
implementations check the function header to make sure modifiers like
asyncness, constness and safety match before triggering the lint.
Fixes#5413, #4290
changelog: check fn header along with decl for should_implement_trait
When checking for functions that are potential candidates for trait
implementations check the function header to make sure modifiers like
asyncness, constness and safety match before triggering the lint.
Fixes#5413, #4290
Downgrade unreadable_literal to pedantic
As motivated by #5418. This is the top most commonly suppressed Clippy style lint, which indicates that the community has decided they don't share Clippy's opinion on the best style of this.
I've left the lint in as pedantic, though it could be that "restriction" would be better -- I can see this lint being useful as an opt-in restriction in some codebases.
changelog: Remove unreadable_literal from default set of enabled lints
Add new lint for `Result<T, E>.map_or(None, Some(T))`
Fixes#5414
PR Checklist
---
- [x] Followed lint naming conventions (the name is a bit awkward, but it seems to conform)
- [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
`Result<T, E>` has an [`ok()`](https://doc.rust-lang.org/std/result/enum.Result.html#method.ok) method that adapts a `Result<T,E>` into an `Option<T>`.
It's possible to get around this adapter by writing `Result<T,E>.map_or(None, Some)`.
This lint is implemented as a new variant of the existing [`option_map_none` lint](https://github.com/rust-lang/rust-clippy/pull/2128)
Downgrade inefficient_to_string to pedantic
From the [documentation](https://rust-lang.github.io/rust-clippy/master/index.html#inefficient_to_string):
> ```diff
> - ["foo", "bar"].iter().map(|s| s.to_string());
>
> + ["foo", "bar"].iter().map(|&s| s.to_string());
> ```
I feel like saving 10 nanoseconds from the formatting machinery isn't worth asking the programmer to insert extra `&` / `*` noise in the *vast* majority of cases. This is a pedantic lint.
changelog: Remove inefficient_to_string from default set of enabled lints
Downgrade trivially_copy_pass_by_ref to pedantic
The rationale for this lint is documented as:
> In many calling conventions instances of structs will be passed through registers if they fit into two or less general purpose registers.
I think the purported performance benefits of clippy's recommendation are overstated. This isn't worth asking people to sprinkle code with more `*``*``&``*``&` to chase the alleged performance.
This should be a pedantic lint that is disabled by default and opted in if some specific performance sensitive codebase determines that it is worthwhile.
As a reminder, a typical place that a reference to a primitive would come up is if the function is used as a filter. Triggering a performance-oriented lint on this type of code is the definition of pedantic.
```rust
fn filter(_n: &i32) -> bool {
true
}
fn main() {
let v = vec![1, 2, 3];
v.iter().copied().filter(filter).for_each(drop);
}
```
```console
warning: this argument (4 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
--> src/main.rs:1:15
|
1 | fn filter(_n: &i32) -> bool {
| ^^^^ help: consider passing by value instead: `i32`
```
changelog: Remove trivially_copy_pass_by_ref from default set of enabled lints
Downgrade let_unit_value to pedantic
Given that the false positive in #1502 is marked E-hard and I don't have much hope of it getting fixed, I think it would be wise to disable this lint by default. I have had to suppress this lint in every substantial codebase (\>100k line) I have worked in. Any time this lint is being triggered, it's always the false positive case.
The motivation for this lint is documented as:
> A unit value cannot usefully be used anywhere. So binding one is kind of pointless.
with this example:
> ```rust
> let x = {
> 1;
> };
> ```
Sure, but the author would find this out via an unused_variable warning or from `x` not being the type that they need further down. If there ends up being a type error on `x`, clippy's advice isn't going to help get the code compiling because it can only run if the code already compiles.
changelog: Remove let_unit_value from default set of enabled lints
Fix update_lints
This fixes a bug in update_lints, where `internal` lints were not registered properly. This also cleans up some code. For example: The code generation functions no longer filter the lints the are given. This is now the task of the caller. This way, it is more obvious in the `replace_in_file` calls which lints will be included in which part of a file.
This also turns the lint modules private. There is no need for them to be public, since shared code should be in the utils module anyway.
And last but not least, this fixes the `register_lints` code generation, so also internal lints get registered.
changelog: none
Result<T, E> has an `ok()` method that adapts a Result<T,E> into an Option<T>.
It's possible to get around this adapter by writing Result<T,E>.map_or(None, Some).
This lint is implemented as a new variant of the existing
[`option_map_none` lint](https://github.com/rust-lang/rust-clippy/pull/2128)
useless Rc<Rc<T>>, Rc<Box<T>>, Rc<&T>, Box<&T>
refers to #2394
changelog: Add lints for Rc<Rc<T>> and Rc<Box<T>> and Rc<&T>, Box<&T>
this is based on top of another change #5310 so probably should go after that one.
Downgrade option_option to pedantic
Based on a search of my work codebase (\>500k lines) for `Option<Option<`, it looks like a bunch of reasonable uses to me. The documented motivation for this lint is:
> an optional optional value is logically the same thing as an optional value but has an unneeded extra level of wrapping
which seems a bit bogus in practice. For example a typical usage would look like:
```rust
let mut host: Option<String> = None;
let mut port: Option<i32> = None;
let mut payload: Option<Option<String>> = None;
for each field {
match field.name {
"host" => host = Some(...),
"port" => port = Some(...),
"payload" => payload = Some(...), // can be null or string
_ => return error,
}
}
let host = host.ok_or(...)?;
let port = port.ok_or(...)?;
let payload = payload.ok_or(...)?;
do_thing(host, port, payload)
```
This lint seems to fit right in with the pedantic group; I don't think linting on occurrences of `Option<Option<T>>` by default is justified.
---
changelog: Remove option_option from default set of enabled lints
Lint unnamed address comparisons
Functions and vtables have an insignificant address. Attempts to compare such addresses will lead to very surprising behaviour. For example: addresses of different functions could compare equal; two trait object pointers representing the same object and the same type could be unequal.
Lint against unnamed address comparisons to avoid issues like those in rust-lang/rust#69757 and rust-lang/rust#54685.
changelog: New lints: [`fn_address_comparisons`] [#5294](https://github.com/rust-lang/rust-clippy/pull/5294), [`vtable_address_comparisons`] [#5294](https://github.com/rust-lang/rust-clippy/pull/5294)
`unused_self` false positive
fixes#5351
Remove the for loop in `unused_self` so that lint enabled for one method doesn't trigger on another method.
changelog: Fix false positive in `unused_self` around lint gates on impl items
Move verbose_file_reads to restriction
cc #5368
Using `File::read` instead of `fs::read_to_end` does make sense in multiple cases, so this lint is rather restriction, than complexity
changelog: Move [`verbose_file_reads`] to restriction
Lint for `pub(crate)` items that are not crate visible due to the visibility of the module that contains them
changelog: Add `redundant_pub_crate` lint
Closes#5274.
Fix single binding closure
Fix the `match_single_binding` lint when triggered inside a closure.
Fixes: #5347
changelog: Improve suggestion for [`match_single_binding`]
Improvement: Don't show function body in needless_lifetimes
Changes the span on which the lint is reported to point to only the
function return type instead of the entire function body.
Fixes#5284
changelog: none
Extend `redundant_clone` to the case that cloned value is not consumed
Fixes#5303.
---
changelog: Extend `redundant_clone` to the case that cloned value is not consumed
add lint on File::read_to_string and File::read_to_end
Adds lint `verbose_file_reads` which checks for use of File::read_to_end and File::read_to_string.
Closes https://github.com/rust-lang/rust-clippy/issues/4916
changelog: add lint on File::{read_to_end, read_to_string}
redundant_pattern: take binding (ref, ref mut) into account in suggestion
fixes#5271
changelog: redundant_pattern: take binding (ref, ref mut) into account in suggestion (#5271)
Fix match single binding when in a let stmt
Fix bad suggestion when `match_single_binding` lints when inside a local (let) statement.
Fixes#5267
changelog: none
Resolve false positives of unnecessary_cast for non-decimal integers
This PR resolves false positives of `unnecessary_cast` for hexadecimal integers to floats and adds a corresponding test case.
Fixes: #5220
changelog: none
Add lint for .. use in fully binded struct
This PR adds the lint `match-wild-in-fully-binded-struct` to prevent the use of the `..` pattern when all fields of the struct are already binded.
Fixes: #638
changelog: Add [`rest_pat_in_fully_bound_structs`] lint to warn against the use of `..` in fully binded struct
Detect usage of custom floating-point abs implementation
Closes#5224
changelog: Enhance [`suboptimal_flops`] lint to detect manual implementations of the `abs` method
Whitelist unused attribute for use items.
This PR whitelists the `unused` attribute with `use` items and adds a corresponding test case.
Fixes: #5229
changelog: none
Add lint to detect floating point operations that can be computed more
accurately at the cost of performance. `cbrt`, `ln_1p` and `exp_m1`
library functions call their equivalent cmath implementations which is
slower but more accurate so moving checks for these under this new lint.
Merge the accuracy and efficiency lints into a single lint that
checks for improvements to accuracy, efficiency and readability
of floating-point expressions.
Move check for lossy whole-number floats out of `excessive_precision`
changelog: Add new lint `lossy_float_literal` to detect lossy whole number float literals and move it out of `excessive_precision` again.
Fixes#5201
Make it possible to correctly indent snippet_block snippets
This adds a `indent_relative_to` arg to the `{snippet,expr}_block` functions. This makes it possible to keep the correct indentation of block like suggestions.
In addition, this makes the `trim_multiline` function private and adds a `indent_of` function, to get the indentation of the first line of a span.
The suggestion of `needless_continue` cannot be made auto applicable, since it would be also necessary to remove code following the linted expression. (Well, maybe it is possible, but I don't know how to do it. Expanding the suggestion span to the last expression, that should be removed didn't work)
changelog: Improve suggestions, when blocks of code are involved
Port mitsuhiko's excessive bools lints
Closes#4 .
changelog: add `struct_excessive_bools` and `fn_params_excessive_bools` lints.
I moved is_trait_impl_item check because at first I implemented it as a late pass for some reason but then I realized it's actually an early lint. But it's a useful function to have, should I move it into a separate pr?
Do not lint `unnecessary_unwrap` in external macros
Fixes#5131
I think we shouldn't lint `{panicking, unnecessary}_unwrap` in macros, not just `assert!`.
changelog: Fix false positive in `unnecessary_unwrap`
Use `checked_sub` to avoid index out of bounds
(Fixes) #4681 (possibly)
The issue likely occurs due to `lit_snip.len() < suffix.len() + 1`. You can see similar backtrace to change it to `lit_snip.len() - suffix.len() - 1000` or something then run `cargo test --release`.
But I couldn't come up with the test so I'd leave the issue open if we want.
changelog: Fix potential ICE in `misc_early`
Tweak documentation in `multiple_crate_versions`
This example isn't reproducible now since `ctrlc` upgrades `winapi` to `0.3.x` in `3.1.1`. We should pin their versions to trigger lint correctly.
changelog: none
Fix syntax highlighting of code fences
The documentation for RESULT_EXPECT_USED includes this code:
let res: Result<usize, ()> = Ok(1);
res?;
# Ok::<(), ()>(())
Because the code fence didn't start with `rust`, the code wasn't highlighted and the line starting with `#` was displayed on the website. This is now fixed.
EDIT: I noticed that highlighting for some other lints is broken as well. It only works if the code fence looks like this:
````markdown
```rust
// ..
```
````
However, many code blocks were ignored. I un-ignored most code blocks and made them compile by adding hidden code with `#`. While doing so, I found two mistakes:
```rust
opt.map_or(None, |a| a + 1)
// instead of
opt.map_or(None, |a| Some(a + 1))
```
and
```rust
fn as_str(self) -> &str
// instead of
fn as_str(self) -> &'static str
```
changelog: none
The documentation for RESULT_EXPECT_USED includes this code:
let res: Result<usize, ()> = Ok(1);
res?;
# Ok::<(), ()>(())
Because the code fence didn't start with `rust`, the code wasn't highlighted and the line starting with `#` was displayed on the website. This is now fixed.
Clean up `span_lint` in `methods/mod.rs`
Uses `span_help_and_lint` instead of `span_lint` and `span_lint_and_sugg` instead of `span_lint_and_then`.
changelog: none
dont fire `possible_missing_comma` if intendation is present
Closes#4399
changelog: dont fire `possible_missing_comma` if intendation is present
I suspect there is already a utils function for indentation (but searching indent didn't yield a function for that), and my solution is certainly not universal, but it's probably the best we can do.
Detect usage of invalid atomic ordering in memory fences
Detect usage of `core::sync::atomic::{fence, compiler_fence}` with `Ordering::Relaxed` and suggest valid alternatives.
changelog: Extend `invalid_atomic_ordering` to lint memory fences
Fixes#5026
Avoid mut_key on types of unknown layout
This fixes#5020 by requiring a known layout for the key type before linting. Edit: This fixes#5043, too.
changelog: none
Keep the ordering in `nonminimal_bool` lint
I believe it shouldn't cause any regression but feel free to let me know if you have a doubtful example.
Also, splits up `booleans` ui test.
Fixes#5045
changelog: none
Match wild err arm improvements
This lint should trigger on other identifiers which have `_` prefix (such as `_e`) and only if they are unused in the panic block.
_Note_: the `is_unused` function is greatly inspired from `pat_is_wild` function in [loops lints](43ac9416d9/clippy_lints/src/loops.rs (L1689)).
I've been considering doing some refactoring, maybe in utils. Maybe this PR or a new one. What do you think ?
fixes#5024
changelog: none
New lint: pats_with_wild_match_arm
Wildcard use with other pattern in same match arm.
The wildcard covers other(s) pattern(s) as it will match anyway.
changelog: add new lint when multiple patterns (including wildcard) are used in a match arm.
Fixes#4640.
Fix redundant_clone lint not working with PathBuf and OsString
https://github.com/rust-lang/rust-clippy/pull/4825 diabled MIR optimization in clippy, including `rustc_mir::transform::InstCombine` which reduces `&(*x)` to `x`. This PR tries to unwrap `&*` when looking into `mir::Rvalue`s.
Fixes#5014.
---
changelog: fixed `redundant_clone` lint not working with `PathBuf` and `OsString`
Move `transmute_float_to_int` lint to `complexity`
`transmute_float_to_int` lint was accidentally added to nursery so moving it to the complexity group.
changelog: Move `transmute_float_to_int` out of nursery
Detect usage of invalid atomic ordering modes such as
`Ordering::{Release, AcqRel}` in atomic loads and
`Ordering::{Acquire, AcqRel}` in atomic stores.
Use `mem::take` instead of `mem::replace` when applicable
`std::mem::take` can be used to replace a value of type `T` with `T::default()` instead of `std::mem::replace`.
Fixes issue #4871
changelog: Added lint for [`mem_replace_with_default`]
Fix ICE on `unsound_collection_transmute`
Fixes#4968
Check if `Ty`s are normalizable. It might show hidden false negative, I'm not sure.
Also, the regression tests are placed on two dirs, so move them to `/crashes`. I think it will be easier to find the right place.
changelog: Fix ICE on `unsound_collection_transmute`
Currently `replace_consts` lint applies within match patterns but
the suggestion is incorrect as function calls are disallowed in
them. To fix this we prevent the lint from firing within patterns.
Fix `replace_consts` lint documentation
`replace_consts` lint no longer lints for the usage of
`ATOMIC_{SIZE}_INIT` and `ONCE_INIT` so removing any
occurences of them in the documentation.
changelog: Update `replace_consts` lint documentation
new lint: mutable_key_type
This fixes#732 - well, partly, it doesn't adress `Hash` impls, but the use of mutable types as map keys or set members
changelog: add `mutable_key_type` lint
r? @flip1995
Add `let_underscore_must_use` lint
changelog: closes#4812 , added a new `let_underscore_must_use` lint, moved `is_must_use_ty` to utils, added `is_must_use_fn` util function
Fix `map_clone` false positive
Don't lint when the item type is not a reference. `copied` only applies to references.
changelog: Fix `map_clone` false positive
Currently the cognitive complexity lint spans the entire function
body making it really difficult to read and refactor the code in
editors. To fix this we reduce the lint span to the function name.
Move `iterator_step_by_zero` and correct the documentation
Move `iterator_step_by_zero` and correct the documentation.
changelog: Corrected `iterator_step_by_zero` documentation
Currently `cmp_nan` lint doesn't detect comparisons with NaN's
if the operands are consts variables so to fix this we evaluate
the const variables first before testing for NaN.
Fix false positive with cast_sign_loss lint
`cast_sign_loss` lint incorrectly suggests that the result of `checked_abs`, `rem_euclid` and `checked_rem_euclid` cannot be casted to an unsigned integer without loss.
Fixes#4818#4764#4743
changelog: Fix false positives in `cast_sign_loss` lint
The Rust Book recommends that functions that return a `Result` type have
a doc comment with an `# Errors` section describing the kind of errors
that can be returned
(https://doc.rust-lang.org/book/ch14-02-publishing-to-crates-io.html#commonly-used-sections).
This change adds a lint to enforce this. The lint is allow by default;
it can be enabled with `#![warn(clippy::missing_errors_doc)]`.
Closes#4854.
`cast_sign_loss` lint incorrectly suggests that the result of
`checked_abs`, `rem_euclid` and `checked_rem_euclid` cannot
be casted to an unsigned integer without loss.
Fix FP in manual_swap lint with slice-like types
Fixes#4853
changelog: Fix FP in [`manual_swap`] lint with slice-like types and make it auto applicable
This utilizes https://github.com/rust-lang/rust/pull/60584 by setting
our own `panic_hook` and pointing to our own issue tracker instead of
the rustc issue tracker.
This also adds a new internal lint to test the ICE message.
**Potential downsides**
* This essentially copies rustc's `report_ice` function as
`report_clippy_ice`. I think that's how it's meant to be implemented, but
maybe @jonas-schievink could have a look as well =)
The downside of more-or-less copying this function is that we have to
maintain it as well now.
The original function can be found [here][original].
* `driver` now depends directly on `rustc` and `rustc_errors`
Closes#2734
[original]: 59367b074f/src/librustc_driver/lib.rs (L1185)
Move use_self to nursery
Closes#4859
We have a lot of false positives in this lint, so I think it makes sense
to move this to the nursery until they are resolved.
changelog: Move `use_self` lint to nursery, due to many false positives
Closes#4859
We have a lot of false positives in this lint, so I think it makes sense
to move this to the nursery until they are resolved.
changelog: Move `use_self` lint to nursery, due to many false positives
Deny warnings in CI
Removes the `debugging` feature, that wasn't used anymore and adds/enables the `deny-warnings` feature for every sub-crate of Clippy.
changelog: none
Literal Representation Restructure
This pull request restructures the literal_representation module to be easier to understand and maintain. I split the changes into a lot of commits to make reviewing easier.
changelog: none
Fix false positive in derive_hash_xor_eq
This fixes a false positive in derive_hash_xor_eq where the lint was
triggering on user-defined traits called `Hash`.
changelog: Fix false positive in `derive_hash_xor_eq`
Fixes#4658
Fix check_infinite_loop (while_immutable_condition) by checking for break or return inside loop body
changelog: Fix check_infinite_loop (while_immutable_condition) by checking for break or return inside loop body
fixes#4648
Run ./util/dev
Revert changelog entry
Rename lint to same_functions_in_if_condition and add a doc example
Add testcases with different arg in fn invocation
added documentation
minor style fix
change as to ::from
add ignore to doc
include threshold in lint message/make suggestion more apparent/use Scalar api instead of matching
style fix
shange snippet_opt to snippet
Only store valid suffixes (and not mistyped suffixes) in DigitInfo.
Check for mistyped suffixes later and not when DigitInfo is created.
This opens the door to more sophisticated mistyped suffix checks later.
Fix false positive in explicit_counter_loop lint
When the counter was used in a closure after the loop the lint didn't detect the
usage of the counter correctly.
changelog: Fix false positive in `explicit_counter_loop`
Fixes#4732