Fixes#4226
This introduces the lint await_holding_lock. For async functions, we iterate
over all types in generator_interior_types and look for types named MutexGuard,
RwLockReadGuard, or RwLockWriteGuard. If we find one then we emit a lint.
If let else mutex
changelog: Adds lint to catch incorrect use of `Mutex::lock` in `if let` expressions with lock calls in any of the blocks.
closes: #5219
Add lint on large non scalar const
This PR adds the new lint `non_scalar_const` that aims to warn against `const` declaration of large arrays. For performance, because of inlining, large arrays should be preferably declared as `static`.
Note: i made this one to warn on all const arrays, whether they are in a body function or not. I don't know if this is really necessary, i could just reduce this lint to variables out of function scope.
Fixes: #400
changelog: add new lint for large non-scalar types declared as const
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
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
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)
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.
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
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.
Detect usage of invalid atomic ordering modes such as
`Ordering::{Release, AcqRel}` in atomic loads and
`Ordering::{Acquire, AcqRel}` in atomic stores.
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
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.
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)
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
Specifically, this revises the clippy integration to utilize a new
callback to register its lints, as the prior editing of lint store in
Session is no longer possible.
This lint will complain when you put a mutable function/method call
inside a `debug_assert` macro, because it will not be executed in
release mode, therefore it will change the execution flow, which is not
wanted.
Add `inefficient_to_string` lint
Closes#4586
changelog: Add `inefficient_to_string` lint, which checks for calling `to_string` on `&&str`, which would bypass the `str`'s specialization
`must_use_unit` lints unit-returning functions with a `#[must_use]`
attribute, suggesting to remove it.
`double_must_use` lints functions with a plain `#[must_use]`
attribute, but which return a type which is already `#[must_use]`,
so the attribute has no benefit.
`must_use_candidate` is a pedantic lint that lints functions and
methods that return some non-unit type that is not already
`#[must_use]` and suggests to add the annotation.
Lints when, on the RHS of a BinOp, there is a UnOp without a space
before the operator but with a space after (e.g. foo >- 1).
Signed-off-by: Nikos Filippakis <nikolaos.filippakis@cern.ch>
account for doc visibility
This fixes#4608.
Also I noticed that the lint failed to look at trait and impl items. There's a small bit of fallout in the code, too, but not enough to warrant its own commit.
changelog: check docs of trait items and impl items, also make `missing_safety_doc` account for visibility
Changes cast-lossless to a pedantic lint
As discussed in #4528, this moves the cast-lossless lint from `all` to `pedantic`.
I couldn't tell from description alone if it should also be removed from the complexity category, so I left it as part of complexity for now. I didn't see any impact to the tests from this change, but I could be wrong (as this is my first PR).
fixes#4528
changelog: Moves cast-lossless from default to checking only as a `pedantic` lint.
Improvements to `type_repetition_in_bounds`
Improvements to the `type_repetition_in_bounds` trait based on feedback from #4380#4326#4323
Currently just make it pedantic. Hopefully, more to come
changelog: move `type_repetition_in_bounds` to `pedantic`
Update lint deprecation for tool lints
changelog: Allow tool lints (`clippy::*`) to be deprecated
Our lint deprecation previously didn't work for tool lints, because
`register_removed` was registering lints to be removed _without_ the
`clippy` prefix.
Fixes#4349
Our lint deprecation previously didn't work for tool lints, because
`register_removed` was registering lints to be removed _without_ the
`clippy` prefix.
This lint adds warning if types are redundantly repeated in trait bounds i.e. `T: Copy, T: Clone` instead of `T: Copy + Clone`. This is a late pass trait lint and has necessitated the addition of code to allow hashing of TyKinds without taking into account Span information.
Move the method checking into a new lint called
`redundant_closures_for_method_calls` and put it in the pedantic group.
This aspect of the lint seems more controversial than the rest.
cc #3942
* Late Lint pass, catches:
* One liner: 0 -> null -> transmute
* One liner: std:null() -> transmute
* Const (which resolves to null) -> transmute
* UI Test case for Lint
* Updated test for issue 3849, because now the lint that code generated is in Clippy.
* Expanded `const.rs` miri-based Constant Folding code, to cover
raw pointers
* Ran automatic naming update
* Formalized rename of `cyclomatic_complexity` to `cognitive_complexity`
** Added the rename to `lib.rs`
** Added rename test
* Added warning for deprecated key `cyclomatic_complexity_threshold` and tests for it
* Added deprecation status for Clippy's builtin attribute
* Updated tests for new builtin attribute renaming
**What it does:** Checks for generics with `std::ops::Drop` as bounds.
**Why is this bad?** `Drop` bounds do not really accomplish anything.
A type may have compiler-generated drop glue without implementing the
`Drop` trait itself. The `Drop` trait also only has one method,
`Drop::drop`, and that function is by fiat not callable in user code.
So there is really no use case for using `Drop` in trait bounds.
**Known problems:** None.
**Example:**
```rust
fn foo<T: Drop>() {}
```
* master: (58 commits)
Rustfmt all the things
Don't make decisions on values that don't represent the decision
Improving comments.
Rustup
Added rustfix to the test.
Improve span shortening.
Added "make_return" and "blockify" convenience methods in Sugg and used them in "needless_bool".
Actually check for constants.
Fixed potential mistakes with nesting. Added tests.
formatting fix
Update clippy_lints/src/needless_bool.rs
formatting fix
Fixing typo in CONTRIBUTING.md
Fix breakage due to rust-lang/rust#57651
needless bool lint suggestion is wrapped in brackets if it is an "else" clause of an "if-else" statement
Fix automatic suggestion on `use_self`.
Remove negative integer literal checks.
Fix `implicit_return` false positives.
Run rustfmt
Fixed breakage due to rust-lang/rust#57489
...
Merge new_without_default_derive into new_without_default
Closes#3525, deprecating new_without_default_derive and moving both lints into new_without_default.
I believe if the user already decided to put underscores in their
literal, Clippy should be willing to believe that they put a number of
underscores that they felt was readable.
This lint looks for:
let mut vec = Vec::with_capacity(len);
vec.set_len(len);
The suggested replacement is `vec![0; len]`.
This is far too opinionated to be a deny-by-default lint because the performance
characteristics of the suggested replacement are totally different.
I am not convinced that this lint has value beyond what deny(unsafe_code) gives
you. Unsafe code is unsafe but please don't deny-by-default lint it if that's
the only reason.
Warning was:
warning: the feature `macro_at_most_once_rep` has been stable since 1.32.0 and no longer requires an attribute to enable
--> clippy_lints/src/lib.rs:19:12
|
19 | #![feature(macro_at_most_once_rep)]
| ^^^^^^^^^^^^^^^^^^^^^^
|
= note: #[warn(stable_features)] on by default
Add lint to detect slow zero-filled vector initialization. It detects
when a vector is zero-filled with extended with `repeat(0).take(len)`
or `resize(len, 0)`.
This zero-fillings are usually slower than simply using `vec![0; len]`.
I noticed that I suppress this lint in many of my projects.
https://github.com/search?q=needless_pass_by_value+user%3Adtolnay&type=Codehttps://github.com/search?q=needless_pass_by_value+user%3Aserde-rs&type=Code
Upon further inspection, this lint has a *long* history of false
positives (and several remaining).
Generally I feel that this lint is the definition of pedantic and should
not be linted by default.
#[derive(Debug)]
enum How {
ThisWay,
ThatWay,
}
// Are we really better off forcing the call sites to write f(&_)...?
fn f(how: How) {
println!("You want to do it {:?}", how);
}
fn main() {
f(How::ThatWay);
}
This finishes up the rewrite of `update_lints.py` in Rust. More
specifically, this
* adds the `--check` flag and handling to clippy_dev
* tracks file changes over the different calls to `replace_region_in_file`
* only writes changes to files if the `--check` flag is *not* used
* runs `./util/dev update_lints --check` on CI instead of the old script
* replaces usage of the `update_lints.py` script with an error
`./util/dev update_lints` behaves 99% the same as the python script.
The only difference that I'm aware of is an ordering change to
`clippy_lints/src/lib.rs` because underscores seem to be sorted
differently in Rust and in Python.
🏁
3388: RIIR update lints: Generate deprecated lints r=phansch a=phansch
The update script now also generates the 'register_removed' section in
`clippy_lints/src/lib.rs`.
Also, instead of using `let mut store ...`, I added a new identifier
line so that the replacement will continue to work in case `let mut
store ...` ever changes.
cc #2882
Co-authored-by: Philipp Hansch <dev@phansch.net>
The update script now also generates the 'register_removed' section in
`clippy_lints/src/lib.rs`.
Also, instead of using `let mut store ...`, I added a new identifier
line so that the replacement will continue to work in case `let mut
store ...` ever changes.
3349: Fixes#3347: Lint for wildcard dependencies in Cargo.toml r=ordovicia a=ordovicia
Add a lint for wildcard dependencies in Cargo.toml.
How should I write a test for this lint?
Fixes#3347
Co-authored-by: Hidehito Yabuuchi <hdht.ybuc@gmail.com>
Handles cases of `.collect().len()`, `.collect().is_empty()`, and
`.collect().contains()`. This lint is intended to be generic enough to
be added to at a later time with other similar patterns that could be
optimized.
Closes#3034
Make explicit_iter_loop and explicit_into_iter_loop allow-by-default, so
that people can turn them on if they want to enforce that style; avoid
presenting them as *the* idiomatic Rust style, rather than just *a* style.
Recategorize `range_plus_one` and `range_minus_one` to `complexity`.
This moves `range_plus_one` out of the nursery as the inclusive range
syntax is now stable. Both are moved to `complexity` as it is more
consistent with other lints such as `int_plus_one`.
Warning was:
warning: the feature `macro_vis_matcher` has been stable since 1.29.0 and no longer requires an attribute to enable
--> src/lib.rs:4:12
|
4 | #![feature(macro_vis_matcher)]
| ^^^^^^^^^^^^^^^^^
|
= note: #[warn(stable_features)] on by default
The changes reflected in this commit are as follows:
- Revised `IndexingSlicingPass` struct name to IndexingSlicing for consistency with the rest of the code base.
- Revised match arm condition to use `(..)` shorthand in favor of `(_, _, _)`.
- Restored a couple telling variable names.
- Calls to `cx.span_lint` were revised to use `utils::span_help_and_lint`.
- Took a stab at refactoring some generalizable calls to `utils::span_help_and_lint` to minimize duplicate code.
- Revised INDEXING_SLICING declaration to pedantic rather than restriction.
- Added `&x[0..].get(..3)` to the test cases.
This commit renames instances of `array_indexing` to `indexing_slicing` and moves the `indexing_slicing` lint to the `clippy_pedantic` group. The justification for this commit's changes are detailed in the previous commit's message.
Hey there clippy team! I've made some assumptions in this PR and I'm not at all certain they'll look like the right approach to you. I'm looking forward to any feedback or revision requests you have, thanks!
Prior to this commit the `indexing_slicing` lint was limited to indexing/slicing operations on arrays. This meant that the scope of a really useful lint didn't include vectors. In order to include vectors in the `indexing_slicing` lint a few steps were taken.
The `array_indexing.rs` source file in `clippy_lints` was renamed to `indexing_slicing.rs` to more accurately reflect the lint's new scope. The `OUT_OF_BOUNDS_INDEXING` lint persists through these changes so if we can know that a constant index or slice on an array is in bounds no lint is triggered.
The `array_indexing` tests in the `tests/ui` directory were also extended and moved to `indexing_slicing.rs` and `indexing_slicing.stderr`.
The `indexing_slicing` lint was moved to the `clippy_pedantic` lint group.
A specific "Consider using" string was added to each of the `indexing_slicing` lint reports.
At least one of the test scenarios might look peculiar and I'll leave it up to y'all to decide if it's palatable. It's the result of indexing the array `x` after `let x = [1, 2, 3, 4];`
```
error: slicing may panic. Consider using `.get(..n)`or `.get_mut(..n)`instead
--> $DIR/indexing_slicing.rs:23:6
|
23 | &x[0..][..3];
| ^^^^^^^^^^^
```
The error string reports only on the second half's range-to, because the range-from is in bounds!
Again, thanks for taking a look.
Closes#2536
