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.
This should make Clippy more resilient and will unblock #78343.
This PR is made against rust-lang/rust to avoid the need for a subtree
sync at @flip1995's suggestion in rust-lang/rust-clippy#6310.
Add a case to `lint_search_is_some` to handle searching strings
Fixes: #6010
This adds a lint which recommends using `contains()` instead of `find()` followed by `is_some()` on strings as suggested in #6010.
This was added as an additional case to
5af88e3c2d/clippy_lints/src/methods/mod.rs (L3037)
I would really appreciate any comments/suggestions for my code!
changelog: Added case to `lint_search_is_some` to handle searching strings
add error_occured field to ConstQualifs,
fix#76064
I wasn't sure what `in_return_place` actually did and not sure why it returns `ConstQualifs` while it's sibling functions return `bool`. So I tried to make as minimal changes to the structure as possible. Please point out whether I have to refactor it or not.
r? `@oli-obk`
cc `@RalfJung`
instead of `find()` follows by `is_some()` on strings
Update clippy_lints/src/find_is_some_on_strs.rs
Co-authored-by: Takayuki Nakata <f.seasons017@gmail.com>
Update clippy_lints/src/methods/mod.rs
Co-authored-by: Philipp Krones <hello@philkrones.com>
Remove `allow` in `option_option` lint test
As it is not triggering locally anymore, I propose to remove `#[allow(clippy::option_option)]` from the test.
The goal is also to see what happens on CI.
closes: #4298
changelog: none
Add `let_underscore_drop`
This line generalizes `let_underscore_lock` (#5101) to warn about any initializer expression that implements `Drop`.
So, for example, the following would generate a warning:
```rust
struct Droppable;
impl Drop for Droppable {
fn drop(&mut self) {}
}
let _ = Droppable;
```
I tried to preserve the original `let_underscore_lock` functionality in the sense that the warning generated for
```rust
let _ = mutex.lock();
```
should be unchanged.
*Please keep the line below*
changelog: Add lint [`let_underscore_drop`]
Fix bad suggestions for `deref_addrof` and `try_err` lints
Fix bad suggestions when in macro expansion for `deref_addrof` and `try_err` lints.
Fixes: #6234Fixes: #6242Fixes: #6237
changelog: none
r? `@llogiq`
We skip the lint if the `loop {}` is in the `#[panic_handler]` as the
main recommendation we give is to panic, which obviously isn't
possible in a panic handler.
Signed-off-by: Joe Richey <joerichey@google.com>
Check when `from_utf8` is called from sliced byte array from string
---
*Please keep the line below*
changelog: Fix#5487: Add linter to check when `from_utf8` is called from sliced byte array from string.
"Respect" enums in `interior_mutable_const`
fixes#3962fixes#3825
Hello,
It might not be a good idea to submit another relatively large PR while I have an opened PR; but, I've finished this anyway. This may be able to wait for months.
Note: the code uses the MIR interpreter, which the author of #3962 thought unlikely to be a solution. This might be over-engineering; but, I think it's important to be able to work with the 'http' crate (#3825). (And, I don't want to write a MIR visitor)
---
changelog: fix a false positive in two `interior_mutable_const` lints where a constant with enums gets linted
even if it uses a clearly unfrozen variant
Add lint for 'field_reassign_with_default` #568
changelog: Add lint for field_reassign_with_default that checks if mutable object + field modification is used to edit a binding initialized with Default::default() instead of struct constructor.
Fixes#568
Notes:
- Checks for reassignment of one or more fields of a binding initialized with Default::default().
- Implemented using EarlyLintPass, might be future proofed better with LateLintPass.
- Does not trigger if Default::default() is used via another type implementing Default.
- This is a re-open of [PR#4761](https://github.com/rust-lang/rust-clippy/pull/4761), but I couldn't figure out how to re-open that one so here's a new one with the requested changes :S
Use const sym where possible
I ran a regex search and replace to use const `sym` values where possible. This should give some performance boost by avoiding string interning at runtime.
Con: It is not as consistent as always using `sym!`.
I also changed an internal lint to suggest using `sym::{}`, making an assumption that this will always work for diagnostic items.
changelog: none
Add lint 'ref_option_ref' #1377
This lint checks for usage of `&Option<&T>` which can be simplified as `Option<&T>` as suggested in #1377.
This WIP PR is here to get feedback on the lint as there's more cases to be handled:
* statics/consts,
* associated types,
* type alias,
* function/method parameter/return,
* ADT definitions (struct/tuple struct fields, enum variants)
changelog: Add 'ref_option_ref' lint
Add lint: from_iter_instead_of_collect
Fixes#5679
This implements lint for `::from_iter()` from #5679 not the general issue (`std::ops::Add::add`, etc.).
This lint checks if expression is function call with `from_iter` name and if it's implementation of the `std::iter::FromIterator` trait.
changelog: Introduce from_iter_instead_of_collect lint
single_char_insert_str: lint using insert_str() on single-char literals and suggest insert()
Fixes#6026
changelog: add single_char_insert_str lint which lints using string.insert_str() with single char literals and suggests string.insert() with a char
- Implement `field_reassign_with_default` as a `LateLintPass`
- Avoid triggering `default_trait_access` on a span already linted by
`field_reassigned_with_default`
- Merge `default_trait_access` and `field_reassign_with_default` into
`Default`
- Co-authored-by: Eduardo Broto <ebroto@tutanota.com>
- Fixes#568
Update the existing arithmetic lint
re: #6209
Updates the lint to not the error message if RHS of binary operation `/` of `%` is a literal/constant that is not `0` or `-1`, as suggested [here](https://github.com/rust-lang/rust-clippy/issues/6209#issuecomment-715624354)
changelog: Expand [`integer_arithmetic`] to work with RHS literals and constants
fix the error-causing suggestion of 'borrowed_box'
Fixes#3128
Fix the suggestion of 'borrowed_box', which causes a syntax error because it misses necessary parentheses.
---
changelog: Fix the error-causing suggestion of 'borrowed_box'
Add lint for comparing to empty slices instead of using .is_empty()
Hey first time making a clippy lint
I added the implementation of the lint the `len_zero` since it shared a lot of the code, I would otherwise have to rewrite. Just tell me if the lint should use it's own file instead
changelog: Add lint for comparing to empty slices
Fixes#6217
Lint items after statements in local macro expansions
The items_after_statements lint was skipping all expansions. Instead
we should still lint local macros.
Fixes#578
---
*Please keep the line below*
changelog: The items_after_statements now applies to local macro expansions
No lint in macro for `toplevel_ref_arg`
Do not lint when the span is from a macro.
Question: shouldn't we extend this for external macros also ?
Fixes: #5849
changelog: none
No lint with `cfg!` and fix sugg for macro in `needless_bool` lint
Don't lint if `cfg!` macro is one of the operand.
Fix suggestion when the span originated from a macro, using `hir_with_macro_callsite`.
Fixes: #3973
changelog: none
New lint: manual-range-contains
This fixes#1110, at least for the contains-suggesting part.
- \[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`
---
changelog: new lint: manual-range-contains
Add lint for `&mut Mutex::lock`
Fixes#1765
changelog: Add lint [`mut_mutex_lock`] for `&mut Mutex::lock` and suggests using `&mut Mutex::get_mut` instead.
Update empty_loop documentation/message.
Originally part of #6161, but now this PR only deals with `std` crates
This change:
- Updates the `std` message .
- Updates the docs to mention how the busy loops should be fixed
- Gives examples of how to do this for `no_std` targets
- Updates the tests/stderr files to test this change.
changelog: Update `empty_loop` lint documentation
Add new lint for undropped ManuallyDrop values
Adds a new lint for the following code:
```rust
struct S;
impl Drop for S {
fn drop(&mut self) {
println!("drip drop");
}
}
fn main() {
// This will not drop the `S`!!!
drop(std::mem::ManuallyDrop::new(S));
unsafe {
// This will.
std::mem::ManuallyDrop::drop(&mut std::mem::ManuallyDrop::new(S));
}
}
```
The inner value of a `ManuallyDrop` will not be dropped unless the proper, unsafe drop function is called on it. This lint makes sure that a user does not accidently use the wrong function and forget to drop a `ManuallyDrop` value.
Fixes#5581.
---
*Please keep the line below*
changelog: none
Add lint for holding RefCell Ref across an await
Fixes#6008
This introduces the lint await_holding_refcell_ref. For async functions, we iterate
over all types in generator_interior_types and look for `core::cell::Ref` or `core::cell::RefMut`. If we find one then we emit a lint.
Heavily cribs from: https://github.com/rust-lang/rust-clippy/pull/5439
changelog: introduce the await_holding_refcell_ref lint
Lint unnecessary int-to-int and float-to-float casts
This is an implementation of a lint that detects unnecessary casts of number literals, as discussed here:
https://github.com/rust-lang/rust-clippy/issues/6116
---
changelog: lint unnecessary as-casts of literals when they could be written using literal syntax.
Refactor trivially_copy_pass_by_ref and the new lint into pass_by_ref_or_value module
Update stderr of conf_unknown_key test
Rename lint to large_types_passed_by_value
Increase `pass_by_value_size_limit` default value to 256
Improve rules for `large_types_passed_by_value`
Improve tests for `large_types_passed_by_value`
Improve documentation for `large_types_passed_by_value`
Make minor corrections to pass_by_ref_or_value.rs suggested by clippy itself
Fix `large_types_passed_by_value` example and improve docs
pass_by_ref_or_value: Tweak check for mut annotation in params
large_types_passed_by_value: add tests for pub trait, trait impl and inline attributes
We also update the documentation to note that the remediations are
different for `std` and `no_std` crates.
Signed-off-by: Joe Richey <joerichey@google.com>
Identical arguments on assert macro family
Lint when identical args are used on `assert_eq!`, `debug_assert_eq!`, `assert_ne!` and `debug_assert_ne!` macros.
Added to the lint `eq_op`.
Common functions added to `utils/higher.rs`
Fixes: #3574Fixes: #4694
changelog: Lint on identical args when calling `assert_eq!`, `debug_assert_eq!`, `assert_ne!` and `debug_assert_ne!` macros
Sync from rust
Fix rustc breakage by running:
```rust
git subtree push -P src/tools/clippy git@github.com:josephlr/rust-clippy sync-from-rust
```
and then adding a commit that runs `cargo dev fmt`
---
changelog: none
Expands `manual_memcpy` to lint ones with loop counters
Closes#1670
This PR expands `manual_memcpy` to lint ones with loop counters as described in https://github.com/rust-lang/rust-clippy/issues/1670#issuecomment-293280204
Although the current code is working, I have a couple of questions and concerns.
~~Firstly, I manually implemented `Clone` for `Sugg` because `AssocOp` lacks `Clone`. As `AssocOp` only holds an enum, which is `Copy`, as a value, it seems `AssocOp` can be `Clone`; but, I was not sure where to ask it. Should I make a PR to `rustc`?~~ The [PR]( https://github.com/rust-lang/rust/pull/73629) was made.
Secondly, manual copying with loop counters are likely to trigger `needless_range_loop` and `explicit_counter_loop` along with `manual_memcpy`; in fact, I explicitly allowed them in the tests. Is there any way to disable these two lints when a code triggers `manual_memcpy`?
And, another thing I'd like to note is that `Sugg` adds unnecessary parentheses when expressions with parentheses passed to its `hir` function, as seen here:
```
error: it looks like you're manually copying between slices
--> $DIR/manual_memcpy.rs:145:14
|
LL | for i in 3..(3 + src.len()) {
| ^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[3..((3 + src.len()))].clone_from_slice(&src[..((3 + src.len()) - 3)])
```
However, using the `hir` function is needed to prevent the suggestion causing errors when users use bitwise operations; and also this have already existed, for example: `verbose_bit_mask`. Thus, I think this is fine.
changelog: Expands `manual_memcpy` to lint ones with loop counters
Preserve raw strs for: format!(s) to s.to_string() lint
fixes#6142
clippy::useless_format will keep the source's string (after converting {{ and }} to { and }) when suggesting a change from format!() to .to_string() usage. Ie:
| let s = format!(r#""hello {{}}""#);
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `r#""hello {}""#.to_string()`
changelog: [`useless_format`]: preserve raw string literals when no arguments to `format!()` are provided.
