Metadata collection monster searching for Clippy's configuration options
This PR teaches our lovely metadata collection monster which configurations are available inside Clippy. It then adds a new *Configuration* section to the lint documentation.
---
The implementation uses the `define_Conf!` macro to create a vector of metadata during compilation. This enables easy collection and parsing without the need of searching for the struct during a lint-pass (and it's quite elegant IMO). The information is then parsed into an intermediate struct called `ClippyConfiguration` which will be saved inside the `MetadataCollector` struct itself. It is currently only used to generate the *Configuration* section in the lint documentation, but I'm thinking about adding an overview of available configurations to the website. Saving them in this intermediate state without formatting them right away enables this in the future.
The new parsing will also allow us to have a documentation that spans over multiple lines in the future. For example, this will be valid when the old script has been removed:
```rust
/// Lint: BLACKLISTED_NAME.
/// The list of blacklisted names to lint about. NB: `bar` is not here since it has legitimate uses
(blacklisted_names: Vec<String> = ["foo", "baz", "quux"].iter().map(ToString::to_string).collect())
```
The deprecation reason is also currently being collected but not used any further and that's basically it.
---
See: #7172 for the full metadata collection to-do list or to suggest a new feature in connection to it 🙃
---
changelog: none
r? `@flip1995`
cc `@camsteffen` It would be great if you could also review this PR as you have recently worked on Clippy's `define_Conf!` macro.
Add `cargo collect-metadata` as an cargo alias for the metadata collection lint
This PR adds a new alias to run the metadata collection monster on `clippy_lints`. I'm currently using it to create the `metadata_collection.json` file and I plan to use it in the `deply.sh` script. Having it as a new alias enables us to simply use:
```sh
cargo collect-metadata
```
It sometimes requires running `cargo clean` before collecting the metadata due to caching. I'm still debating if I should include a cargo clean as part of the `run_metadata_collection_lint` test or not. Input on this would be greatly appreciated 🙃
That's it, just a small change that can be reviewed and merged in parallel to #7214.
---
See: #7172 for the full metadata collection to-do list or to suggest a new feature in connection to it.
---
changelog: none
r? `@flip1995` btw. feel free to pass these PRs one to other team members as well if you want.
`needless_collect` enhancements
fixes#7164
changelog: `needless_collect`: For `BTreeMap` and `HashMap` lint only `is_empty`, as `len` might produce different results than iter's `count`
changelog: `needless_collect`: Lint `LinkedList` and `BinaryHeap` in direct usage case as well
Trigger [`wrong_self_convention`] only if it has implicit self
Lint [`wrong_self_convention`] only if the impl or trait has `self` _per sé_.
Fixes: #7179
changelog: trigger [`wrong_self_convention`] only if it has implicit self
match_single_binding: Fix invalid suggestion when match scrutinee has side effects
fixes#7094
changelog: `match_single_binding`: Fix invalid suggestion when match scrutinee has side effects
---
`Expr::can_have_side_effects` is used to determine the scrutinee has side effects, while this method is a little bit conservative for our use case. But I'd like to use it to avoid reimplementation of the method and too much heuristics. If you think this is problematic, then I'll implement a custom visitor to address it.
`while_let_on_iterator` Improvements
fixes: #6491fixes: #6231fixes: #5844fixes: #1924fixes: #1033
The check for whether a field can be borrowed should probably be moved to utils at some point, but it would require some cleanup work and knowing what parts can actually be shared.
changelog: Suggest `&mut iter` when the iterator is used after the loop.
changelog: Suggest `&mut iter` when the iterator is a field in a struct.
changelog: Don't lint when the iterator is a field in a struct, and the struct is used in the loop.
changelog: Lint when the loop is nested in another loop, but suggest `&mut iter` unless the iterator is from a local declared inside the loop.
* Suggest `&mut iter` when the iterator is used after the loop.
* Suggest `&mut iter` when the iterator is a field in a struct.
* Don't lint when the iterator is a field in a struct, and the struct is
used in the loop.
* Lint when the loop is nested in another loop, but suggest `&mut iter`
unless the iterator is from a local declared inside the loop.
Metadata collection monster eating deprecated lints
This adds the collection of deprecated lints to the metadata collection monster. The JSON output has the same structure with the *new* lint group "DEPRECATED". Here is one of fourteen examples it was able to dig up in Clippy's code:
```JSON
{
"id": "assign_op_pattern",
"id_span": {
"path": "src/assign_ops.rs",
"line": 34
},
"group": "clippy::style",
"docs": " **What it does:** Checks for `a = a op b` or `a = b commutative_op a` patterns.\n\n **Why is this bad?** These can be written as the shorter `a op= b`.\n\n **Known problems:** While forbidden by the spec, `OpAssign` traits may have\n implementations that differ from the regular `Op` impl.\n\n **Example:**\n ```rust\n let mut a = 5;\n let b = 0;\n // ...\n // Bad\n a = a + b;\n\n // Good\n a += b;\n ```\n",
"applicability": {
"is_multi_part_suggestion": false,
"applicability": "MachineApplicable"
}
}
```
And you! Yes you! Sir or Madam can get all of this **for free** in Clippy if this PR gets merged. (Sorry for the silliness ^^)
---
See: #7172 for the full metadata collection to-do list or to suggest a new feature in connection to it 🙃
---
changelog: none
r? `@flip1995`
Move `inconsistent_struct_constructor` to pedantic
The whole point of named fields is that we don't have to worry about
order. The names, not the position, communicate the information, so
worrying about consistency for consistency's sake is pedantic to a *T*.
Cf. #7192.
changelog: [`inconsistent_struct_constructor`] is moved to pedantic.
wchargin-branch: inconsistent-struct-constructor-pedantic
Fix stack overflow issue in `redundant_pattern_matching`
Fixes#7169
~~cc `@Jarcho` Since tomorrow is release day and we need to get this also fixed in beta, I'll just revert the PR instead of looking into the root issue. Your changes are good, so if you have an idea what could cause this stack overflow and know how to fix it, please open a PR that reverts this revert with a fix.~~
r? `@llogiq`
changelog: none (fixes stack overflow, but this was introduced in this release cycle)
The whole point of named fields is that we don't have to worry about
order. The names, not the position, communicate the information, so
worrying about consistency for consistency's sake is pedantic to a *T*.
Fixes#7192.
wchargin-branch: inconsistent-struct-constructor-pedantic
wchargin-source: 4fe078a21c77ceb625e58fa3b90b613fc4fa6a76
Fix needless_quesiton_mark false positive
changelog: Fix [`needless_question_mark`] false positive where the inner value is implicity dereferenced by the question mark.
Fixes#7107
Handle write!(buf, "\n") case better
Make `write!(buf, "\n")` suggest `writeln!(buf)` by removing
the trailing comma from `writeln!(buf, )`.
changelog: [`write_with_newline`] suggestion on only "\n" improved
For `to_*` variant don't lint in trait impl taking `self` when non-`Copy` type
Lint name: `wrong_self_convention`.
It relaxes rules for `to_*` variant, so it doesn't lint in trait definitions and implementations anymore.
Although, non-`Copy` type implementing trait's `to_*` method taking
`self` feels not good (consumes ownership, so should be rather named `into_`), it would be better if this case was a pedantic lint (allow-by-default) instead.
More information in the discussion with `@flip1995` [here](https://github.com/rust-lang/rust-clippy/pull/7002#discussion_r627363450)
changelog: `wrong_self_convention`: For `to_*` variant don't lint in trait impl taking `self` when non-`Copy` type
r? `@flip1995`
Fix duplicated "Rust 1.52" version section header
The most recent changelog update 037ddf282b accompanying the 1.52 release added a second "Rust 1.52" section header, with the result that the Rust release announcement https://blog.rust-lang.org/2021/05/06/Rust-1.52.0.html is linking to the "current beta" changelog section for Clippy rather than the stable changelog. I don't know the release process but based on previous changes to this file, I assume the correct thing to do is to mark the topmost section as being for Rust 1.53, not 1.52.
changelog: none
The most recent changelog update 037ddf282b accompanying the 1.52 release added a second "Rust 1.52" section header, with the result that the Rust release announcement https://blog.rust-lang.org/2021/05/06/Rust-1.52.0.html is linking to the "current beta" changelog section for Clippy rather than the stable changelog. I don't know the release process but based on previous changes to this file, I assume the correct thing to do is to mark the topmost section as being for Rust 1.53, not 1.52.
Make `write!(buf, "\n")` suggest `writeln!(buf)` by removing
the trailing comma from `writeln!(buf, )`.
changelog: [`write_with_newline`] suggestion on only "\n" improved
It relaxes rules for `to_*` variant, so it doesn't lint in trait definitions
and implementations anymore.
Although, non-`Copy` type implementing trait's `to_*` method taking
`self` feels not good (consumes ownership, so should be rather named `into_`), it would be better if this case was a pedantic lint (allow-by-default) instead.
Refactor: arrange lints in misc_early module
This PR arranges misc_early lints so that they can be accessed more easily.
Basically, I refactored them following the instruction described in #6680.
cc: `@Y-Nak,` `@flip1995,` `@magurotuna`
changelog: Move lints in misc_early module into their own modules.
