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.
Cleanup path-to-local checks
changelog: none
It seemed like too much ceremony going on to check if an expression matches a variable. So I created two util functions `path_to_local(Expr) -> Option<HirId>` and `path_to_local_id(Expr, HirId) -> bool` to make this easier, and used them wherever applicable.
I changed logic in a few places to use `HirId` instead of `Symbol` where it was easy to do so. I believe this is more correct and may even fix some bugs.
I also removed some calls to `qpath_res`. This is not needed if you are only looking for a `Res::Local`.
As a note, I wanted to name the util functions in a way that encourages understanding of the HIR.
Refactor `PrimitiveTypeTable` for Clippy
I removed `PrimitiveTypeTable` and added `PrimTy::ALL` and `PrimTy::from_name` in its place. This allows Clippy to use `PrimTy::from_name` for the `builtin_type_shadow` lint, and a `const` list of primitive types is deleted from Clippy code (the goal). All changes should be a little faster, if anything.
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
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 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
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.
Improve safety of `LateContext::qpath_res`
This is my first rustc code change, inspired by hacking on clippy!
The first change is to clear cached `TypeckResults` from `LateContext` when visiting a nested item. I took a hint from [here](5e91c4ecc0/compiler/rustc_privacy/src/lib.rs (L1300)).
Clippy has a `qpath_res` util function to avoid a possible ICE in `LateContext::qpath_res`. But the docs of `LateContext::qpath_res` promise no ICE. So this updates the `LateContext` method to keep its promises, and removes the util function.
Related: rust-lang/rust-clippy#4545
CC ````````````@eddyb```````````` since you've done related work
CC ````````````@flip1995```````````` FYI
Make more traits of the From/Into family diagnostic items
Following traits are now diagnostic items:
- `From` (unchanged)
- `Into`
- `TryFrom`
- `TryInto`
This also adds symbols for those items:
- `into_trait`
- `try_from_trait`
- `try_into_trait`
Related: https://github.com/rust-lang/rust-clippy/pull/6620#discussion_r562482587
Refractor a few more types to `rustc_type_ir`
In the continuation of #79169, ~~blocked on that PR~~.
This PR:
- moves `IntVarValue`, `FloatVarValue`, `InferTy` (and friends) and `Variance`
- creates the `IntTy`, `UintTy` and `FloatTy` enums in `rustc_type_ir`, based on their `ast` and `chalk_ir` equilavents, and uses them for types in the rest of the compiler.
~~I will split up that commit to make this easier to review and to have a better commit history.~~
EDIT: done, I split the PR in commits of 200-ish lines each
r? `````@nikomatsakis````` cc `````@jackh726`````
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.
`manual_filter_map` and `manual_find_map`
changelog: Add `manual_filter_map` and replace `find_map` with `manual_find_map`
Replaces #6453Fixes#3188Fixes#4193
~Depends on #6567 (to fix an internal lint false positive)~
This replaces `filter_map` and `find_map` with `manual_filter_map` and `manual_find_map` respectively. However, `filter_map` is left in place since it is used for a variety of other cases. See discussion in #6453.
Following traits are now diagnostic items:
- `From` (unchanged)
- `Into`
- `TryFrom`
- `TryInto`
This also adds symbols for those items:
- `into_trait`
- `try_from_trait`
- `try_into_trait`
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`.
Fixes#81007
Previously, we would fail to collect tokens in the proper place when
only builtin attributes were present. As a result, we would end up with
attribute tokens in the collected `TokenStream`, leading to duplication
when we attempted to prepend the attributes from the AST node.
We now explicitly track when token collection must be performed due to
nomterminal parsing.
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.
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: 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.
Rework diagnostics for wrong number of generic args (fixes#66228 and #71924)
This PR reworks the `wrong number of {} arguments` message, so that it provides more details and contextual hints.
This makes it possible to pass the `Impl` directly to functions, instead
of having to pass each of the many fields one at a time. It also
simplifies matches in many cases.
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
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
Fix derive and macro related false positives in `field_reassign_with_default`
Closes#6545
changelog: Fix derive and macro related false positives in [`field_reassign_with_default`]
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
Clippy treated Rust 2021 as Rust 2015, because 2018 was checked with
`==` instead of `>=`. This fixes that, such that 2018-specific things
are also enabled for 2021.
- Adds optional default values to const generic parameters in the AST
and HIR
- Parses these optional default values
- Adds a `const_generics_defaults` feature gate
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
Move binder for dyn to each list item
This essentially changes `ty::Binder<&'tcx List<ExistentialTraitRef>>` to `&'tcx List<ty::Binder<ExistentialTraitRef>>`.
This is a first step in moving the `dyn Trait` representation closer to Chalk, which we've talked about in `@rust-lang/wg-traits.`
r? `@nikomatsakis`
Implement if-let match guards
Implements rust-lang/rfcs#2294 (tracking issue: #51114).
I probably should do a few more things before this can be merged:
- [x] Add tests (added basic tests, more advanced tests could be done in the future?)
- [x] Add lint for exhaustive if-let guard (comparable to normal if-let statements)
- [x] Fix clippy
However since this is a nightly feature maybe it's fine to land this and do those steps in follow-up PRs.
Thanks a lot `@matthewjasper` ❤️ for helping me with lowering to MIR! Would you be interested in reviewing this?
r? `@ghost` for now
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
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 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.
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`
Added known problem to comparison_chain docs
changelog: Added documentation to comparison_chain that explains a possible performance penalty, according to issue #5354
This is my first PR, I hope everything has been done correctly.
Fixes#5354
Add suspicious_operation_groupings lint
This is my (<del> currently WIP </del>) attempt to close#6039.
changelog: Added `suspicious_operation_groupings` lint.
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
rustc_ast currently has a few dependencies on rustc_lexer. Ideally, an AST
would not have any dependency its lexer, for minimizing unnecessarily
design-time dependencies. Breaking this dependency would also have practical
benefits, since modifying rustc_lexer would not trigger a rebuild of rustc_ast.
This commit does not remove the rustc_ast --> rustc_lexer dependency,
but it does remove one of the sources of this dependency, which is the
code that handles fuzzy matching between symbol names for making suggestions
in diagnostics. Since that code depends only on Symbol, it is easy to move
it to rustc_span. It might even be best to move it to a separate crate,
since other tools such as Cargo use the same algorithm, and have simply
contain a duplicate of the code.
This changes the signature of find_best_match_for_name so that it is no
longer generic over its input. I checked the optimized binaries, and this
function was duplicated at nearly every call site, because most call sites
used short-lived iterator chains, generic over Map and such. But there's
no good reason for a function like this to be generic, since all it does
is immediately convert the generic input (the Iterator impl) to a concrete
Vec<Symbol>. This has all of the costs of generics (duplicated method bodies)
with no benefit.
Changing find_best_match_for_name to be non-generic removed about 10KB of
code from the optimized binary. I know it's a drop in the bucket, but we have
to start reducing binary size, and beginning to tame over-use of generics
is part of that.
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`
Drop support for all cloudabi targets
`cloudabi` is a tier-3 target, and [it is no longer being maintained upstream][no].
This PR drops supports for cloudabi targets. Those targets are:
* aarch64-unknown-cloudabi
* armv7-unknown-cloudabi
* i686-unknown-cloudabi
* x86_64-unknown-cloudabi
Since this drops supports for a target, I'd like somebody to tag `relnotes` label to this PR.
Some other issues:
* The tidy exception for `cloudabi` crate is still remained because
* `parking_lot v0.9.0` and `parking_lot v0.10.2` depends on `cloudabi v0.0.3`.
* `parking_lot v0.11.0` depends on `cloudabi v0.1.0`.
[no]: https://github.com/NuxiNL/cloudabi#note-this-project-is-unmaintained
Fix handling of panic calls
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.
r? ``@flip1995``
cc ``@m-ou-se``
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.
Introduce `TypeVisitor::BreakTy`
Implements MCP rust-lang/compiler-team#383.
r? `@ghost`
cc `@lcnr` `@oli-obk`
~~Blocked on FCP in rust-lang/compiler-team#383.~~
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
Implement destructuring assignment for structs and slices
This is the second step towards implementing destructuring assignment (RFC: rust-lang/rfcs#2909, tracking issue: #71126). This PR is the second part of #71156, which was split up to allow for easier review.
Note that the first PR (#78748) is not merged yet, so it is included as the first commit in this one. I thought this would allow the review to start earlier because I have some time this weekend to respond to reviews. If ``@petrochenkov`` prefers to wait until the first PR is merged, I totally understand, of course.
This PR implements destructuring assignment for (tuple) structs and slices. In order to do this, the following *parser change* was necessary: struct expressions are not required to have a base expression, i.e. `Struct { a: 1, .. }` becomes legal (in order to act like a struct pattern).
Unfortunately, this PR slightly regresses the diagnostics implemented in #77283. However, it is only a missing help message in `src/test/ui/issues/issue-77218.rs`. Other instances of this diagnostic are not affected. Since I don't exactly understand how this help message works and how to fix it yet, I was hoping it's OK to regress this temporarily and fix it in a follow-up PR.
Thanks to ``@varkor`` who helped with the implementation, particularly around the struct rest changes.
r? ``@petrochenkov``
Do not collect tokens for doc comments
Doc comment is a single token and AST has all the information to re-create it precisely.
Doc comments are also responsible for majority of calls to `collect_tokens` (with `num_calls == 1` and `num_calls == 0`, cc https://github.com/rust-lang/rust/pull/78736).
(I also moved token collection into `fn parse_attribute` to deduplicate code a bit.)
r? `@Aaron1011`
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>
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`]
rustc_ast: Do not panic by default when visiting macro calls
Panicking by default made sense when we didn't have HIR or MIR and everything worked on AST, but now all AST visitors run early and majority of them have to deal with macro calls, often by ignoring them.
The second commit renames `visit_mac` to `visit_mac_call`, the corresponding structures were renamed earlier in https://github.com/rust-lang/rust/pull/69589.
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`
Fix example used in cargo_common_metadata
The previous example used in `cargo_common_metadata` included an authors field, even though the comment says it doesn't. And thus doesn't actually demonstrate an example of how the lint fails.
This removes that authors field from the _bad_ example and suggest to fix the _bad_ example by adding the authors field
changelog: Fix example used in `cargo_common_metadata`
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
Provide diagnostic suggestion in ExprUseVisitor Delegate
The [Delegate trait](981346fc07/compiler/rustc_typeck/src/expr_use_visitor.rs (L28-L38)) currently use `PlaceWithHirId` which is composed of Hir `Place` and the
corresponding expression id.
Even though this is an accurate way of expressing how a Place is used,
it can cause confusion during diagnostics.
Eg:
```
let arr : [String; 5];
let [a, ...] = arr;
^^^ E1 ^^^ = ^^E2^^
```
Here `arr` is moved because of the binding created E1. However, when we
point to E1 in diagnostics with the message `arr` was moved, it can be
confusing. Rather we would like to report E2 to the user.
Closes: https://github.com/rust-lang/project-rfc-2229/issues/20
r? `@ghost`
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
The [Delegate
trait](981346fc07/compiler/rustc_typeck/src/expr_use_visitor.rs (L28-L38))
currently use `PlaceWithHirId` which is composed of Hir `Place` and the
corresponding expression id.
Even though this is an accurate way of expressing how a Place is used,
it can cause confusion during diagnostics.
Eg:
```
let arr : [String; 5];
let [a, ...] = arr;
^^^ E1 ^^^ = ^^E2^^
```
Here `arr` is moved because of the binding created E1. However, when we
point to E1 in diagnostics with the message `arr` was moved, it can be
confusing. Rather we would like to report E2 to the user.
Closes: https://github.com/rust-lang/project-rfc-2229/issues/20
Implement rustc side of report-future-incompat
cc https://github.com/rust-lang/rust/issues/71249
This is an alternative to `@pnkfelix's` initial implementation in https://github.com/pnkfelix/rust/commits/prototype-rustc-side-of-report-future-incompat (mainly because I started working before seeing that branch 😄 ).
My approach outputs the entire original `Diagnostic`, in a way that is compatible with incremental compilation. This is not yet integrated with compiletest, but can be used manually by passing `-Z emit-future-incompat-report` to `rustc`.
Several changes are made to support this feature:
* The `librustc_session/lint` module is moved to a new crate `librustc_lint_defs` (name bikesheddable). This allows accessing lint definitions from `librustc_errors`.
* The `Lint` struct is extended with an `Option<FutureBreakage>`. When present, it indicates that we should display a lint in the future-compat report. `FutureBreakage` contains additional information that we may want to display in the report (currently, a `date` field indicating when the crate will stop compiling).
* A new variant `rustc_error::Level::Allow` is added. This is used when constructing a diagnostic for a future-breakage lint that is marked as allowed (via `#[allow]` or `--cap-lints`). This allows us to capture any future-breakage diagnostics in one place, while still discarding them before they are passed to the `Emitter`.
* `DiagnosticId::Lint` is extended with a `has_future_breakage` field, indicating whether or not the `Lint` has future breakage information (and should therefore show up in the report).
* `Session` is given access to the `LintStore` via a new `SessionLintStore` trait (since `librustc_session` cannot directly reference `LintStore` without a cyclic dependency). We use this to turn a string `DiagnosticId::Lint` back into a `Lint`, to retrieve the `FutureBreakage` data.
Currently, `FutureBreakage.date` is always set to `None`. However, this could potentially be interpreted by Cargo in the future.
I've enabled the future-breakage report for the `ARRAY_INTO_ITER` lint, which can be used to test out this PR. The intent is to use the field to allow Cargo to determine the date of future breakage (as described in [RFC 2834](https://github.com/rust-lang/rfcs/blob/master/text/2834-cargo-report-future-incompat.md)) without needing to parse the diagnostic itself.
cc `@pnkfelix`
- 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
Hex bin digit grouping
This revives and updates an old pr (#3391) for the current API.
Closes#2538.
---
*Please keep the line below*
changelog: Add [`unusual_byte_groupings`] lint.
Remove redundant `expect_local()` call
The field `owner` of `HirId` is `LocalDefId` and `hir_id.owner.to_def_id().expect_local()` is redundant. I wonder they were introduced in some rustups.
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
Split out statement attributes changes from #78306
This is the same as PR https://github.com/rust-lang/rust/pull/78306, but `unused_doc_comments` is modified to explicitly ignore statement items (which preserves the current behavior).
This shouldn't have any user-visible effects, so it can be landed without lang team discussion.
---------
When the 'early' and 'late' visitors visit an attribute target, they
activate any lint attributes (e.g. `#[allow]`) that apply to it.
This can affect warnings emitted on sibiling attributes. For example,
the following code does not produce an `unused_attributes` for
`#[inline]`, since the sibiling `#[allow(unused_attributes)]` suppressed
the warning.
```rust
trait Foo {
#[allow(unused_attributes)] #[inline] fn first();
#[inline] #[allow(unused_attributes)] fn second();
}
```
However, we do not do this for statements - instead, the lint attributes
only become active when we visit the struct nested inside `StmtKind`
(e.g. `Item`).
Currently, this is difficult to observe due to another issue - the
`HasAttrs` impl for `StmtKind` ignores attributes for `StmtKind::Item`.
As a result, the `unused_doc_comments` lint will never see attributes on
item statements.
This commit makes two interrelated fixes to the handling of inert
(non-proc-macro) attributes on statements:
* The `HasAttr` impl for `StmtKind` now returns attributes for
`StmtKind::Item`, treating it just like every other `StmtKind`
variant. The only place relying on the old behavior was macro
which has been updated to explicitly ignore attributes on item
statements. This allows the `unused_doc_comments` lint to fire for
item statements.
* The `early` and `late` lint visitors now activate lint attributes when
invoking the callback for `Stmt`. This ensures that a lint
attribute (e.g. `#[allow(unused_doc_comments)]`) can be applied to
sibiling attributes on an item statement.
For now, the `unused_doc_comments` lint is explicitly disabled on item
statements, which preserves the current behavior. The exact locatiosn
where this lint should fire are being discussed in PR #78306
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
When the 'early' and 'late' visitors visit an attribute target, they
activate any lint attributes (e.g. `#[allow]`) that apply to it.
This can affect warnings emitted on sibiling attributes. For example,
the following code does not produce an `unused_attributes` for
`#[inline]`, since the sibiling `#[allow(unused_attributes)]` suppressed
the warning.
```rust
trait Foo {
#[allow(unused_attributes)] #[inline] fn first();
#[inline] #[allow(unused_attributes)] fn second();
}
```
However, we do not do this for statements - instead, the lint attributes
only become active when we visit the struct nested inside `StmtKind`
(e.g. `Item`).
Currently, this is difficult to observe due to another issue - the
`HasAttrs` impl for `StmtKind` ignores attributes for `StmtKind::Item`.
As a result, the `unused_doc_comments` lint will never see attributes on
item statements.
This commit makes two interrelated fixes to the handling of inert
(non-proc-macro) attributes on statements:
* The `HasAttr` impl for `StmtKind` now returns attributes for
`StmtKind::Item`, treating it just like every other `StmtKind`
variant. The only place relying on the old behavior was macro
which has been updated to explicitly ignore attributes on item
statements. This allows the `unused_doc_comments` lint to fire for
item statements.
* The `early` and `late` lint visitors now activate lint attributes when
invoking the callback for `Stmt`. This ensures that a lint
attribute (e.g. `#[allow(unused_doc_comments)]`) can be applied to
sibiling attributes on an item statement.
For now, the `unused_doc_comments` lint is explicitly disabled on item
statements, which preserves the current behavior. The exact locatiosn
where this lint should fire are being discussed in PR #78306
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
BTreeMap: refactor Entry out of map.rs into its own file
btree/map.rs is approaching the 3000 line mark, splitting out the entry
code buys about 500 lines of headroom.
I've created this PR because the changes I've made in #77438 will push `map.rs` over the 3000 line limit and cause tidy to complain.
I picked `Entry` to factor out because it feels less tightly coupled to the rest of `BTreeMap` than the various iterator implementations.
Related: #60302
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
The wrapper type led to tons of target.target
across the compiler. Its ptr_width field isn't
required any more, as target_pointer_width
is already present in parsed form.
Fix two ICEs caused by ty.is_{sized,freeze}
Fixes#6153
Properly fixes#6139
The test case in #6153 is kind of weird. Even removing one of the arguments of the `foo` function prevented the ICE. I think this test case is actually minimal.
changelog: none
Ref style link false positive
This PR does not fix#5834, but it records the problematic link text as a **known problem** inside `doc_markdown` lint.
r? `@flip1995`
- [ ] Followed [lint naming conventions][lint_naming]
- [ ] Added passing UI tests (including committed `.stderr` file)
- [x] `cargo test` passes locally
- [x] Executed `cargo dev update_lints`
- [ ] Added lint documentation
- [x] Run `cargo dev fmt`
---
*Please keep the line below*
changelog: Document problematic link text style as known problem for `doc_markdown`
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.
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
clippy_lints: Do not warn against Box parameter in C FFI
changelog: [`boxed_local`]: don't lint in `extern fn` arguments
Fixes#5542.
When using C FFI, to handle pointers in parameters it is needed to
declare them as `Box` in its Rust-side signature. However, the current
linter warns against the usage of Box stating that "local variable
doesn't need to be boxed here".
This commit fixes it by ignoring functions whose Abi is C.
Downgrade string_lit_as_bytes to nursery
Between #1402 (regarding `to_owned`) and #4494 (regarding `impl Read`), as well as other confusion I've seen hit in my work codebase involving string_lit_as_bytes (`"...".as_bytes().into()`), I don't think this lint is at a quality to be enabled by default.
I would consider re-enabling this lint after it is updated to understand when the surrounding type information is sufficient to unsize `b"..."` to &\[u8\] without causing a type error.
As currently implemented, this lint is pushing people to write `&b"_"[..]` which is not an improvement over `"_".as_bytes()` as far as I am concerned.
---
changelog: Remove string_lit_as_bytes from default set of enabled lints
Fix unicode regexen with bytes::Regex
fixes#6005
The rationale for this is that since we wrote that lint, `bytes::Regex` was extended to be able to use unicode character classes.
---
changelog: [`invalid_regex`]: allow unicode character classes in bytes regex.
Fixes#5542.
When using C FFI, to handle pointers in parameters it is needed to
declare them as `Box` in its Rust-side signature. However, the current
linter warns against the usage of Box stating that "local variable
doesn't need to be boxed here".
This commit fixes it by ignoring functions whose Abi is Cdecl.
Downgrade rc_buffer to restriction
I think Arc\<Vec\<T\>\> and Arc\<String\> and similar are a totally reasonable data structure, as observed by others in the comments on [#6044](https://github.com/rust-lang/rust-clippy/pull/6044#event-3799579830) as well. Doing `Arc::make_mut(&mut self.vec).push(...)` or `Arc::make_mut(&mut self.string).push_str("...")` is a terrific and well performing copy-on-write pattern. Linting this with an enabled-by-default <kbd>performance</kbd> lint strikes me as an unacceptable false positive balance.
As of #6090 the documentation of this lint now contains:
> **Known problems:** This pattern can be desirable ...
which should indicate that we shouldn't be linting against correct, reasonable, well-performing patterns with an enabled-by-default lint.
Mentioning #6044, #6090.
r? `@yaahc,` who reviewed the lint.
---
changelog: Remove rc_buffer from default set of enabled lints
Fix LitKind's byte buffer to use refcounted slice
While working on adding a new lint for clippy (see https://github.com/rust-lang/rust-clippy/pull/6044) for avoiding shared ownership of "mutable buffer" types (such as using `Rc<Vec<T>>` instead of `Rc<[T]>`), I noticed a type exported from rustc_ast and used by clippy gets caught by the lint. This PR fixes the exported type.
This PR includes the actual change to clippy too, but I will open a PR directly against clippy for that part (although it will currently fail to build there).
unnecessary sort by: avoid dereferencing the suggested closure parameter
This change tries to simplify the solution for problematic cases but is less restrictive than #6006.
* We can't dereference shared references to non-Copy types, so the new suggestion does not do that. Note that this implies that the suggested closure parameter will be a reference.
* We can't take a reference to the closure parameter in the returned key, so we don't lint in those cases. This can happen either because the key borrows from the parameter (e.g. `|a| a.borrows()`), or because we suggest `|a| Reverse(a)`. If we did we would hit this error:
```
error: lifetime may not live long enough
--> /home/ebroto/src/ebroto-clippy/tests/ui/unnecessary_sort_by.fixed:19:25
|
19 | vec.sort_by_key(|b| Reverse(b));
| -- ^^^^^^^^^^ returning this value requires that `'1` must outlive `'2`
| ||
| |return type of closure is Reverse<&'2 isize>
| has type `&'1 isize`
error: aborting due to previous error
```
Note that Clippy does not currently have the (MIR-based) machinery necessary to check that what is borrowed is actually the closure parameter.
changelog: [`unnecessary_sort_by`]: avoid dereferencing the suggested closure parameter
Fixes#6001
Do not lint float fractions in `mistyped_literal_suffixes`
As suggested in https://github.com/rust-lang/rust-clippy/issues/4706#issuecomment-544797928, the fractional part is now ignored (the integer part is checked instead).
Fixes: #4706
changelog: `mistyped_literal_suffixes` no longer warns on the fractional part of a float (e.g. 713.23_64)
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. Note that 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).
Lint for invisible Unicode characters other than ZWSP
This PR extends the existing `zero_width_space` lint to look for other invisible characters as well (in this case, `\\u{ad}` soft hyphen.
I feel like this lint is the logical place to add the check, but I also realize the lint name is not particularly flexible, but I also understand that it shouldn't be renamed for compatibility reasons.
Open questions:
- What other characters should trigger the lint?
- What should be done with the lint name?
- How to indicate the change in functionality?
Motivation behind this PR: https://github.com/rust-lang/rust/issues/77417 - I managed to shoot myself in the foot by an invisible character pasted into my test case.
changelog: rename [`zero_width_space`] to [`invisible_characters`] and add SHY and WJ to the list.
Don't emit a lint for the suggestion leading to errors in `needless_range_loop`
Fix#5945
changelog: Don't emit a lint for the suggestion leading to errors in `needless_range_loop`
New Lint: disallowed_method
Fixes https://github.com/rust-lang/rust-clippy/issues/6073
I added a `disallowed_method` lint that works similar to `blacklisted_name`, e.g allows config-defined disallowed method calls.
This lint allows advanced users to patch in undesired method calls for specific projects. It basically uses the DefId of method calls and matches it to parsed strings. Is there an alternative approach that could use more easily digestible configuration strings?
All tests pass as expected except for a linting error on `ui/redundant_pattern_matching_option`, which I am not sure how to resolve since I don't think I changed anything affecting it.
changelog: Add disallowed_method lint to warn on user-defined method calls
Fix FP in `print_stdout`
Fix#6041
This lint shouldn't be emitted in `build.rs` as `println!` and `print!` are used for the build script.
changelog: none
Update Clippy
Bi-weekly Clippy update.
This includes a `Cargo.lock` update (d445493479711389f4dea3a0f433041077ba2088), so probably needs `rollup=never`.
r? `@Manishearth`
Allow a unique name to be assigned to dataflow graphviz output
Previously, if the same analysis were invoked multiple times in a single compilation session, the graphviz output for later runs would overwrite that of previous runs. Allow callers to add a unique identifier to each run so this can be avoided.
Revert: or_fun_call should lint calls to `const fn`s with no args
The changes in #5889 and #5984 were done under the incorrect assumption that a `const fn` with no args was guaranteed to be evaluated at compile time. A `const fn` is only guaranteed to be evaluated at compile time if it's inside a const context (the initializer of a `const` or a `static`).
See this [zulip conversation](https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/Common.20misconception.3A.20.60const.20fn.60.20and.20its.20effect.20on.20codegen/near/208059113) for more details on this common misconception.
Given that none of the linted methods by `or_fun_call` can be called in const contexts, the lint should make no exceptions.
changelog: [`or_fun_call`] lints again calls to `const fn` with no args
Fix a FP in `explicit_counter_loop`
Fixes#4677 and #6074
Fix a false positive in `explicit_counter_loop` where the loop counter is used after incremented, adjust the test so that counters are incremented at the end of the loop and add the test for this false positive.
---
changelog: Fix a false positive in `explicit_counter_loop` where the loop counter is used after incremented
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
Fix a false positive in `explicit_counter_loop` where the loop counter is used after incremented,
adjust the test so that counters are incremented at the end of the loop
and add the test for this false positive.
Stabilize some Result methods as const
Stabilize the following methods of Result as const:
- `is_ok`
- `is_err`
- `as_ref`
A test is also included, analogous to the test for `const_option`.
These methods are currently const under the unstable feature `const_result` (tracking issue: #67520).
I believe these methods to be eligible for stabilization because of the stabilization of #49146 (Allow if and match in constants) and the trivial implementations, see also: [PR#75463](https://github.com/rust-lang/rust/pull/75463) and [PR#76135](https://github.com/rust-lang/rust/pull/76135).
Note: these methods are the only methods currently under the `const_result` feature, thus this PR results in the removal of the feature.
Related: #76225
* stop linting associated types and generic type parameters
* start linting ones in trait impls
whose corresponding definitions in the traits are generic
* remove the `is_copy` check
as presumably the only purpose of it is to allow
generics with `Copy` bounds as `Freeze` is internal
and generics are no longer linted
* remove the term 'copy' from the tests
as being `Copy` no longer have meaning
option_if_let_else - distinguish pure from impure else expressions
Addresses partially #5821.
changelog: improve the lint `option_if_let_else`. Suggest `map_or` or `map_or_else` based on the else expression purity.
Extend invalid_atomic_ordering for compare_exchange{,_weak} and fetch_update
changelog: The invalid_atomic_ordering lint can now detect misuse of `compare_exchange`, `compare_exchange_weak`, and `fetch_update`.
---
I was surprised not to find an issue or existing support here, since these are the functions which are always hardest to get the ordering right on for me (as the allowed orderings for `fail` depend on the `success` parameter).
I believe this lint now covers all atomic methods which care about their ordering now, but I could be wrong.
Hopefully I didn't forget to do anything for the PR!
{print,write}-with-newline: do not suggest empty format string
changelog: do not suggest empty format strings in `print-with-newline` and `write-with-newline`
clarify margin of error in wording of float comparison operator lint messages
fixes#6040
changelog: change wording of float comparison operator to make margin of error less ambiguous
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
Attach tokens to all AST types used in `Nonterminal`
We perform token capturing when we have outer attributes (for nonterminals that support attributes - e.g. `Stmt`), or when we parse a `Nonterminal` for a `macro_rules!` argument. The full list of `Nonterminals` affected by this PR is:
* `NtBlock`
* `NtStmt`
* `NtTy`
* `NtMeta`
* `NtPath`
* `NtVis`
* `NtLiteral`
Of these nonterminals, only `NtStmt` and `NtLiteral` (which is actually just an `Expr`), support outer attributes - the rest only ever have token capturing perform when they match a `macro_rules!` argument.
This makes progress towards solving https://github.com/rust-lang/rust/issues/43081 - we now collect tokens for everything that might need them. However, we still need to handle `#[cfg]`, inner attributes, and misc pretty-printing issues (e.g. #75734)
I've separated the changes into (mostly) independent commits, which could be split into individual PRs for each `Nonterminal` variant. The purpose of having them all in one PR is to do a single Crater run for all of them.
Most of the changes in this PR are trivial (adding `tokens: None` everywhere we construct the various AST structs). The significant changes are:
* `ast::Visibility` is changed from `type Visibility = Spanned<VisibilityKind>` to a `struct Visibility { kind, span, tokens }`.
* `maybe_collect_tokens` is made generic, and used for both `ast::Expr` and `ast::Stmt`.
* Some of the statement-parsing functions are refactored so that we can capture the trailing semicolon.
* `Nonterminal` and `Expr` both grew by 8 bytes, as some of the structs which are stored inline (rather than behind a `P`) now have an `Option<TokenStream>` field. Hopefully the performance impact of doing this is negligible.
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
Improve the "known problems" section of `interior_mutable_key`
* Remove the mention to `Rc` and `Arc` as these are `Freeze` (despite my intuition) so the lint correctly handles already.
* Instead, explain what could cause a false positive, and mention `bytes` as an example.
---
changelog: Improved the "known problems" section of `interior_mutable_key`
improve the suggestion of the lint `unit-arg`
Fixes#5823Fixes#6015
Changes
```
help: move the expression in front of the call...
|
3 | g();
|
help: ...and use a unit literal instead
|
3 | o.map_or((), |i| f(i))
|
```
into
```
help: move the expression in front of the call and replace it with the unit literal `()`
|
3 | g();
| o.map_or((), |i| f(i))
|
```
changelog: improve the suggestion of the lint `unit-arg`
Add CONST_ITEM_MUTATION lint
Fixes#74053Fixes#55721
This PR adds a new lint `CONST_ITEM_MUTATION`.
Given an item `const FOO: SomeType = ..`, this lint fires on:
* Attempting to write directly to a field (`FOO.field = some_val`) or
array entry (`FOO.array_field[0] = val`)
* Taking a mutable reference to the `const` item (`&mut FOO`), including
through an autoderef `FOO.some_mut_self_method()`
The lint message explains that since each use of a constant creates a
new temporary, the original `const` item will not be modified.
* Remove the mention to `Rc` and `Arc` as these are `Freeze`
so the lint correctly handles already.
* Instead, explain what could cause a false positive,
and mention `bytes` as an example.
We no longer lint assignments to const item fields in the
`temporary_assignment` lint, since this is now covered by the
`CONST_ITEM_MUTATION` lint.
Additionally, we `#![allow(const_item_mutation)]` in the
`borrow_interior_mutable_const.rs` test. Clippy UI tests are run with
`-D warnings`, which seems to cause builtin lints to prevent Clippy
lints from running.
Support dataflow problems on arbitrary lattices
This PR implements last of the proposed extensions I mentioned in the design meeting for the original dataflow refactor. It extends the current dataflow framework to work with arbitrary lattices, not just `BitSet`s. This is a prerequisite for dataflow-enabled MIR const-propagation. Personally, I am skeptical of the usefulness of doing const-propagation pre-monomorphization, since many useful constants only become known after monomorphization (e.g. `size_of::<T>()`) and users have a natural tendency to hand-optimize the rest. It's probably worth exprimenting with, however, and others have shown interest cc `@rust-lang/wg-mir-opt.`
The `Idx` associated type is moved from `AnalysisDomain` to `GenKillAnalysis` and replaced with an associated `Domain` type that must implement `JoinSemiLattice`. Like before, each `Analysis` defines the "bottom value" for its domain, but can no longer override the dataflow join operator. Analyses that want to use set intersection must now use the `lattice::Dual` newtype. `GenKillAnalysis` impls have an additional requirement that `Self::Domain: BorrowMut<BitSet<Self::Idx>>`, which effectively means that they must use `BitSet<Self::Idx>` or `lattice::Dual<BitSet<Self::Idx>>` as their domain.
Most of these changes were mechanical. However, because a `Domain` is no longer always a powerset of some index type, we can no longer use an `IndexVec<BasicBlock, GenKillSet<A::Idx>>>` to store cached block transfer functions. Instead, we use a boxed `dyn Fn` trait object. I discuss a few alternatives to the current approach in a commit message.
The majority of new lines of code are to preserve existing Graphviz diagrams for those unlucky enough to have to debug dataflow analyses. I find these diagrams incredibly useful when things are going wrong and considered regressing them unacceptable, especially the pretty-printing of `MovePathIndex`s, which are used in many dataflow analyses. This required a parallel `fmt` trait used only for printing dataflow domains, as well as a refactoring of the `graphviz` module now that we cannot expect the domain to be a `BitSet`. Some features did have to be removed, such as the gen/kill display mode (which I didn't use but existed to mirror the output of the old dataflow framework) and line wrapping. Since I had to rewrite much of it anyway, I took the opportunity to switch to a `Visitor` for printing dataflow state diffs instead of using cursors, which are error prone for code that must be generic over both forward and backward analyses. As a side-effect of this change, we no longer have quadratic behavior when writing graphviz diagrams for backward dataflow analyses.
r? `@pnkfelix`
Fix FP in `same_item_push`
Don't emit a lint when the pushed item doesn't have Clone trait
Fix#5979
changelog: Fix FP in `same_item_push` not to emit a lint when the pushed item doesn't have Clone trait
useless_attribute: Permit wildcard_imports and enum_glob_use
Fixes#5918
changelog: `useless_attribute`: Permit `wildcard_imports` and `enum_glob_use` on `use` items
default_trait_access: Fix wrong suggestion
https://github.com/rust-lang/rust-clippy/issues/5975#issuecomment-683751131
> I think the underlying problem is clippy suggests code with complete parameters, not clippy triggers this lint even for complex types. AFAIK, If code compiles with `Default::default`, it doesn't need to specify any parameters, as type inference is working. (So, in this case, `default_trait_access` should suggest `RefCell::default`.)
Fixes#5975Fixes#5990
changelog: `default_trait_access`: fixed wrong suggestion
Add a lint for an async block/closure that yields a type that is itself awaitable.
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.
});
See the two XXXkhuey comments and the unfixed `_l` structure for things that need more thought.
*Please keep the line below*
changelog: none
or_fn_call: ignore nullary associated const fns
The fix in #5889 was missing associated functions.
changelog: Ignore also `const fn` methods in [`or_fun_call`]
Fixes#5693
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.
});
Fix FP in `to_string_in_display`
Don't emit a lint when `.to_string()` on anything that is not `self`
Fix#5967
changelog: Fix FP in `to_string_in_display` when calling `.to_string()` on anything that is not `self`
Corrects the float_equality_without_abs lint
Fixes an issue in the `float_equality_without_abs` lint. The lint suggestion was configured in a way that it lints the whole error and not just the subtraction part. In the current configuration the lint would suggest to change the expression in a wrong way, e.g.
```rust
let _ = (a - b) < f32::EPSILON; // before
let _ = (a - b).abs(); // after
```
This was dicovered by @flip1995. (See discussion of PR #5952).
Also the suggestion is now formatted via `utils::sugg`.
changelog: none
Fix `let_and_return` bad suggestion
Add a cast to the suggestion when the return expression has adjustments.
These adjustments are lost when the suggestion is applied.
This is similar to the problem in issue #4437.
Closes#5729
changelog: Fix `let_and_return` bad suggestion
Fix fp in `borrow_interior_mutable_const`
fixes#5796
changelog: fix false positive in `borrow_interior_mutable_const` when referencing a field behind a pointer.
Fix incorrect suggestion when `clone_on_ref_ptr` is triggered in macros
In the lint `clone_on_ref_ptr`, if the `span` is in a macro, don't expand it for suggestion.
Fixes: #2076
changelog: none
r? @ebroto
Add a cast to the suggestion when the return expression has adjustments.
These adjustments are lost when the suggestion is applied.
This is similar to the problem in issue #4437.
Closes#5729
Don't lint if it has always inline attribute
Don't trigger the lint `trivially_copy_pass_by_ref` if it has `#[inline(always)]` attribute.
Note: I am not particularly familiar with `inline` impacts, so I implemented this the way that if only `#[inline]` attribute is here (without `always`), the lint will still trigger. Also, it will still trigger if it has `#[inline(never)]`.
Just tell me if it sounds too much conservative.
Fixes: #5876
changelog: none
Prelude imports are exempt from wildcard import warnings. Until now only
imports of the form
```
use ...::prelude::*;
```
were considered. This change makes it so that the segment `prelude` can
show up anywhere, for instance:
```
use ...::prelude::v1::*;
```
Fixes#5917
Fix false positive in `PRECEDENCE` lint
Extend the lint to handle chains of methods combined with unary negation.
Closes#5924
changelog: Fix false negative in `PRECEDENCE` lint
This extends PR #73293 to handle patterns (Pat). Unlike expressions,
patterns do not support custom attributes, so we only need to capture
tokens during macro_rules! argument parsing.
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;
}
```
Add unnecessary lazy evaluation lint
changelog: Add [`unnecessary_lazy_evaluations`] lint that checks for usages of `unwrap_or_else` and similar functions that can be simplified.
Closes#5715
should_impl_trait - ignore methods with lifetime params
Fixes: #5617
changelog: don't lint should_implement_trait when an `Iterator::next` case has explicit parameters
