* 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
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.
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.
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`
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`
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
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`.
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`.
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
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.
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
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
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.
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
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
- 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
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
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.
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
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
New lint: Recommend using `ptr::eq` when possible
This is based almost entirely on the code available in the previous PR #4596. I merely updated the code to make it compile.
Fixes#3661.
- [ ] I'm not sure about the lint name, but it was the one used in the original PR.
- [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: none
Add `rc_buffer` lint for checking Rc<String> and friends
Fixes#2623
This is a bit different from the original PR attempting to implement this type of lint. Rather than linting against converting into the unwanted types, this PR lints against declaring the unwanted type in a struct or function definition.
I'm reasonably happy with what I have here, although I used the fully qualified type names for the Path and OsString suggestions, and I'm not sure if I should have just used the short versions instead, even if they might not have been declared via use.
Also, I don't know if "buffer type" is the best way to put it or not. Alternatively I could call it a "growable type" or "growable buffer type", but I was thinking of PathBuf when I started making the lint.
changelog: Add `rc_buffer` lint
Add map_err_ignore lint
In a large code base a lot of times errors are ignored by using something like:
```rust
foo.map_err(|_| Some::Enum)?;
```
This drops the original error in favor of a enum that will not have the original error's context. This lint helps catch throwing away the original error in favor of an enum without its context.
---
*Please keep the line below*
changelog: Added map_err_ignore lint
Add a new lint, `manual-strip`, that suggests using the `str::strip_prefix`
and `str::strip_suffix` methods introduced in Rust 1.45 when the same
functionality is performed 'manually'.
Closes#5734
Add lint panic in result
### Change
Adding a new "restriction" lint that will emit a warning when using "panic", "unimplemented" or "unreachable" in a function of type option/result.
### Motivation
Some codebases must avoid crashes at all costs, and hence functions of type option/result must return an error instead of crashing.
### Test plan
Running:
TESTNAME=panic_in_result cargo uitest ---
changelog: none
This catches bugs of the form
tokio::spawn(async move {
let f = some_async_thing();
f // Oh no I forgot to await f so that work will never complete.
});
Warn about assignments where left-hand side place expression is the same
as right-hand side value expression. For example, warn about assignment in:
```rust
pub struct Event {
id: usize,
x: i32,
y: i32,
}
pub fn copy_position(a: &mut Event, b: &Event) {
a.x = b.x;
a.y = a.y;
}
```
