`never_loop`: suggest using an `if let` instead of a `for` loop
changelog: suggest using an `if let` statement instead of a `for` loop that [`never_loop`]s
Fixes#7537, r? `@camsteffen.`
Cleanup usage of `span_to_snippet` and `LintContext::sess`
- avoid using `SourceMap::span_to_snippet` directly and use `clippy_utils::source::snippet_opt` instead
- don't use `LintContext::sess()` on `EarlyContext`s which have a `sess` field directly available, saving the import of `LintContext`
changelog: none
Don't emit `too_many_lines` for closures
changelog: don't emit the [`too_many_lines`] lint inside closures to avoir duplicated diagnostics.
Fixes#7517.
rustc: Replace `HirId`s with `LocalDefId`s in `AccessLevels` tables
and passes using those tables - primarily privacy checking, stability checking and dead code checking.
All these passes work with definitions rather than with arbitrary HIR nodes.
r? `@cjgillot`
cc `@lambinoo` (#87487)
rfc3052 followup: Remove authors field from Cargo manifests
Since RFC 3052 soft deprecated the authors field, hiding it from
crates.io, docs.rs, and making Cargo not add it by default, and it is
not generally up to date/useful information for contributors, we may as well
remove it from crates in this repo.
Since RFC 3052 soft deprecated the authors field anyway, hiding it from
crates.io, docs.rs, and making Cargo not add it by default, and it is
not generally up to date/useful information, we should remove it from
crates in this repo.
Update lint documentation to use markdown headlines
This PR updates all lint documentation to use markdown headlines. It additionally removed the *Known problems* section for lints without any problems. I've double-checked all automatic replacements, but a second pair of eyes is definitely appreciated!
I wasn't sure when you wanted to switch to the new metadata collection tomorrow, I therefore prepared this PR today. And that's it this is a standalone PR to keep the other related PRs reviewable.
changelog: none
r? `@flip1995`
cc: #7172
Note: This should be merged with the other metadata collection related PRs.
Switch CI to new metadata collection
r? `@xFrednet`
Things we have to keep in mind:
- This removes the template files and the scripts used for deployment from the checkout. This was added in #5517. I don't think we ever needed those there. Not sure though.
- ~~As a result, we can't remove the python scripts yet. We have to wait until this hits a stable Clippy release.~~ I'll just break the next stable deploy and do it by hand once.
- This should be merged together with #7279. Me and `@xFrednet` will coordinate the switch
- ...?
I still have to try out some things:
- [x] Is it worth caching? Yes
- [x] ~~Is it worth to do a release build?~~ Nope
- [x] Does it actually work? With a few changes, yes
- [ ] ...?
changelog: Clippy now uses a lint to generate its documentation 🎉
Changes included:
- Minimum adaption to the new `lints.json` format
- Fixing filtering for the new `lints.json` format; hardcoding the
lint groups in the index
- Recreating the original doc styling for the new format
- Fixed sytax highlighting for rust,ignore code blocks
- Fixed markdown table extraction in the metadata collector and
fixed lint level output
- Adding the additional information row for lints
- Changed the website title to Clippy's lint list
- Flexing the website for mobile users
- Added (?) references for lint levels and groups
- Making deprecated lints look dead
- Removed JS code block language extraction in favor of a rust
implementation `rust-clippy#7352`
- Added the suspicious lint group to the lint list
- Remove trailing whitespaces from index.html
- Fix code highlighting
- Use default value if the docVersion is empty
Co-authored-by: Philipp Krones <hello@philkrones.com>
This updates all the deploy scripts and the deploy workflow.
The deploy workflow now runs the metadata collector to collect the lint
documentation. It also changes the files that are checked out in the
deploy workflow from master and adds an explanation why we have to do
this.
Fix docs of disallowed_type
Add ability to name primitive types without import path
Move primitive resolution to clippy_utils path_to_res fn
Refactor Res matching, fix naming and docs from review
Use tcx.def_path_str when emitting the lint
Use diagnostic items where possible
Clippy still uses a bunch of paths in places that could easily use already defined diagnostic items. This PR updates all references to such paths and also removes a bunch of them that are no longer needed after this cleanup.
Some paths are also used to construct new paths and can therefore not be removed that easily. I've added a doc comment to those instances that recommends the use of the diagnostic item where possible.
And that's it, cleaning crew signing off 🧹🗑️
---
changelog: none
(only internal improvements)
cc: #5393
Prefer a code snipped over formatting the self type (`new_without_default`)
Fixes: rust-lang/rust-clippy#7220
changelog: [`new_without_default`]: The `Default` impl block type doesn't use the full type path qualification
Have a nice day to everyone reading this 🙃
Some `clippy::author` improvements
changelog: none
* Use `Debug` instead of re-implementing it for some things
* Fix block trailing expression handing
* Don't double print on stmt/expr with `#[clippy::author]` attribute
similar_names: No longer suggest inserting or appending an underscore
changelog: [`similar_names`] lint no longer suggests to insert or add an underscore to "fix" too similar names
New lint: [`self_named_constructor`]
Adds the `self_named_constructor` lint for detecting when an implemented method has the same name as the type it is implemented for.
changelog: [`self_named_constructor`]
closes: #7142
Remove refs from Pat slices
Changes `PatKind::Or(&'hir [&'hir Pat<'hir>])` to `PatKind::Or(&'hir [Pat<'hir>])` and others. This is more consistent with `ExprKind`, saves a little memory, and is a little easier to use.
FP fix and documentation for `branches_sharing_code` lint
Closesrust-lang/rust-clippy#7369
Related rust-lang/rust-clippy#7452 I'm still thinking about the best way to fix this. I could simply add another visitor to ensure that the moved expressions don't modify values being used in the condition, but I'm not totally happy with this due to the complexity. I therefore only documented it for now
changelog: [`branches_sharing_code`] fixed false positive where block expressions would sometimes be ignored.
Fix internal `default_hash_types` lint to use resolved path
I run into false positives now and then (mostly in Clippy) when I want to name some util after HashMap.
Don't suggest doc(hidden) or unstable variants in wildcard lint
Clippy's wildcard lint would suggest doc(hidden) and unstable variants for non_exhaustive enums, even though those aren't part of the public interface (yet) and should only be matched on using a `_`, just like potential future additions to the enum. There was already some logic to exclude a *single* doc(hidden) variant. This extends that to all hidden variants, and also hides `#[unstable]` variants.
See https://github.com/rust-lang/rust/pull/85746#issuecomment-868886893
This PR includes https://github.com/rust-lang/rust-clippy/pull/7406 as the first commit.
Here's the diff that this PR adds on top of that PR: https://github.com/m-ou-se/rust-clippy/compare/std-errorkind...m-ou-se:doc-hidden-variants
---
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: No longer suggest unstable and doc(hidden) variants in wildcard lint. wildcard_enum_match_arm, match_wildcard_for_single_variants
This fixes a bug where match_wildcard_for_single_variants produced a
bad suggestion where besides the missing variant, one or more hidden
variants were left.
This also adds tests to the ui-tests match_wildcard_for_single_variants
and wildcard_enum_match_arm to make sure that the correct suggestion is
produced.
New lint: `disallowed_script_idents`
This PR implements a new lint to restrict locales that can be used in the code,
as proposed in #7376.
Current concerns / unresolved questions:
- ~~Mixed usage of `script` (as a Unicode term) and `locale` (as something that is easier to understand for the broad audience). I'm not sure whether these terms are fully interchangeable and whether in the current form it is more confusing than helpful.~~ `script` is now used everywhere.
- ~~Having to mostly copy-paste `AllowedScript`. Probably it's not a big problem, as the list of scripts is standardized and is unlikely to change, and even if we'd stick to the `unicode_script::Script`, we'll still have to implement custom deserialization, and I don't think that it will be shorter in terms of the amount of LoC.~~ `unicode::Script` is used together with a filtering deserialize function.
- Should we stick to the list of "recommended scripts" from [UAX #31](http://www.unicode.org/reports/tr31/#Table_Recommended_Scripts) in the configuration?
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: ``[`disallowed_script_idents`]``
r? `@Manishearth`
Improve lint message for match-same-arms lint
fixes#7331
Follow-up to #7377
This PR improves the lint message for `match-same-arms` lint and adds `todo!(..)` example to the lint docs.
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: None
Do not spawn blacklisted_name lint in test context
---
fixed#7305
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: `blacklisted_name` lint is not spawned in the test context anymore.
Fix detecting of the 'test' attribute
Update UI test to actually check that warning is not triggered in the test code
Fix approach for detecting the test module
Add nested test case
Remove code duplication by extracting 'is_test_module_or_function' into 'clippy_utils'
Cleanup the code
Add import_rename lint, this adds a field on the Conf struct
fixes#7276
changelog: Add ``[`import_rename`]`` a lint that enforces import renaming defined in the config file.
Fixed broken deploy script due to multiline configuration docs
The deploy script on master currently runs into an error (See [log](https://github.com/rust-lang/rust-clippy/runs/2865828873)) due to the new configuration documentation added in #7299. The current documentation collection for the configuration macro sadly doesn't support multiline doc comments. This will be changes in the future with the new metadata collector tracked in #7172 For now we have to use `<br>` inside doc comments to add paragraphs.
This PR restricts `define_Conf!` macro to single lines and adds a comment explaining the reasoning behind it. It also adjusted the actual document parsing to fix a bug. (The parsing was automatically stopping on the first curly bracket, even if it was part of a doc comment).
changelog: none
Problem:
for code like
````
fn main() {
println!("Hello, world!");
}
````
clippy will issue a warning to use a clippy.toml option instead:
````
warning: lint `clippy::wrong_pub_self_convention` has been removed: set the `avoid_breaking_exported_api` config option to `false` to enable the `wrong_self_convention` lint for public items
--> src/main.rs:2:9
|
2 | #![warn(clippy::wrong_pub_self_convention)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(renamed_and_removed_lints)]` on by default
````
But using the lint name as seen in the warning message
echo "avoid_breaking_exported_api = true\n" > clippy.toml
Will cause an error:
````
error: error reading Clippy's configuration file `/tmp/clippytest/clippy.toml`: unknown field `avoid_breaking_exported_api`, expected one of `avoid-breaking-exported-api`, ...
````
Replace the underscores with dashes in the deprecation message.
changelog: avoid_breaking_exported_api: suggest correct clippy config toml option in the deprecation message
Add macro_braces lint to check for irregular brace use in certain macros
The name is a bit long but this sounds good as `#[allow(unconventional_macro_braces)]` and it seems more clear that we are talking about the macro call not macro definitions, any feedback let me know. Thanks!
fixes#7278
changelog: Add ``[`unconventional_macro_braces`]`` lint that checks for uncommon brace usage with macros.
Remove some last remants of {push,pop}_unsafe!
These macros have already been removed, but there was still some code handling these macros. That code is now removed.
Rename unconventional -> nonstandard, add config field
Add standard_macro_braces fields so users can specify macro names and
brace combinations to lint for in the clippy.toml file.
Fix errors caused by nonstandard_macro_braces in other lint tests
Fix users ability to override the default nonstandard macro braces
Add type position macros impl `check_ty`
Remove requirement of fully qualified path for disallowed_method/type
changelog: Remove the need for fully qualified paths in disallowed_method and disallowed_type
After fixing this issue in [import_rename](https://github.com/rust-lang/rust-clippy/pull/7300#discussion_r650127720) I figured I'd fix it for these two.
If this does in fact fix the **Known problems:** section I was planning to remove them from both lints after confirmation.
Vec extend to append
This PR adds a check to suggest changes of vector from
```
vec.extend(other_vec.drain(..))
```
could be written as
```
vec![].append(&mut vec![]);
```
changelog: Add vec_extend_to_append lint
issue: #7209
Don't trigger `field_reassign_with_default` in macros
Fixes#7155
Producing a good suggestion for this lint is already hard when no macros
are involved. With macros the lint message and the suggestion are just
confusing. Since both, producing a good suggestion and figuring out if
this pattern can be re-written inside a macro is nearly impossible, just
bail out.
changelog: [`field_reassign_with_default`] No longer triggers in macros
---
No that our reviewing queue is under control, I want to start hacking on Clippy myself again. Starting with an easy issue to get back in :)
Add disallowed_type lint, this adds a field to the conf struct
Fixes#7277
changelog: Add ``[`disallowed_type`]`` a lint that can enforce banning types specified in the config.
Fix false positive on `semicolon_if_nothing_returned`
Currently the [`semicolon_if_nothing_returned`](https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_if_nothing_returned) lint fires in unwanted situations where a block only spans one line. An example of this was given in #7324. This code:
```rust
use std::mem::MaybeUninit;
use std::ptr;
fn main() {
let mut s = MaybeUninit::<String>::uninit();
let _d = || unsafe { ptr::drop_in_place(s.as_mut_ptr()) };
}
```
yields the following clippy error:
```
error: consider adding a `;` to the last statement for consistent formatting
--> src/main.rs:6:26
|
6 | let _d = || unsafe { ptr::drop_in_place(s.as_mut_ptr()) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add a `;` here: `ptr::drop_in_place(s.as_mut_ptr());`
|
= note: `-D clippy::semicolon-if-nothing-returned` implied by `-D clippy::pedantic`
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_if_nothing_returned
```
I updated the lint to check if the statement is inside an `unsafe` block, a closure or a normal block and if the block only spans one line, in that case the lint is not emitted.
This closes#7324.
changelog: enhanced semicolon if nothing returned according to #7324.
Refactoring identity function lints
I've noticed that we have several lints that all check for identity functions and each used their own check implementation. I moved the `is_expr_identity_function` function to `clippy_utils` and adapted all lints to reuse that one function. This should make the addition of new lints like this also easier in the future.
I've also moved the `map_identity` lint into the `methods` module. It's probably the best to review this PR by checking each commit individually. And that's it, have a great day 🙃
changelog: none
fix `while_let_on_iterator` suggestion in a closure
fixes: #7249
A future improvement would be to check if the closure is being used as `FnOnce`, in which case the original suggestion would be correct.
changelog: Suggest `&mut iter` inside a closure for `while_let_on_iterator`
Fix missing_docs_in_private_items false negative
changelog: Fix [`missing_docs_in_private_items`] false negative when the item has any `#[name = "value"]` attribute
Closes#7247 (decided not to use the rustc method since it calls `Session::check_name`, which is for rustc only)
Fix allow on some statement lints
changelog: Fix `#[allow(..)]` over statements for [`needless_collect`], [`short_circuit_statement`] and [`unnecessary_operation`]
Fixes#7171Fixes#7202
Add avoid_breaking_exported_api config option
changelog: Add `avoid_breaking_exported_api` config option for [`enum_variant_names`], [`large_types_passed_by_value`], [`trivially_copy_pass_by_ref`], [`unnecessary_wraps`], [`upper_case_acronyms`] and [`wrong_self_convention`].
changelog: Deprecates [`pub_enum_variant_names`] and [`wrong_pub_self_convention`] as the non-pub variants are now configurable.
changelog: Fix various false negatives for `pub` items that are not exported from the crate.
A couple changes to late passes in order to use `cx.access_levels.is_exported` rather than `item.vis.kind.is_pub`.
I'm not sure how to better document the config option or lints that are (not) affected (see comments in #6806). Suggestions are welcome. cc `@rust-lang/clippy`
I added `/clippy.toml` to use the config internally and `/tests/clippy.toml` to maintain a default config in ui tests.
Closes#6806Closes#4504
Fix invalid syntax in `from_iter_instead_of_collect` suggestion
First attempt at contributing, hopefully this is a good start and can be improved. :)
fixes#7259
changelog: [`from_iter_instead_of_collect`] fix invalid suggestion involving "as Trait"
Fix `redundant_closure` for `vec![]` macro in a closure with arguments
fixes: #7224
changelog: Fix `redundant_closure` for `vec![]` macro in a closure with arguments
This speeds up the metadata collection by 2-2.5x on my machine. During
metadata collection other lint passes don't have to be registered, only
the lints themselves.
Implement the new desugaring from `try_trait_v2`
~~Currently blocked on https://github.com/rust-lang/rust/issues/84782, which has a PR in https://github.com/rust-lang/rust/pull/84811~~ Rebased atop that fix.
`try_trait_v2` tracking issue: https://github.com/rust-lang/rust/issues/84277
Unfortunately this is already touching a ton of things, so if you have suggestions for good ways to split it up, I'd be happy to hear them. (The combination between the use in the library, the compiler changes, the corresponding diagnostic differences, even MIR tests mean that I don't really have a great plan for it other than trying to have decently-readable commits.
r? `@ghost`
~~(This probably shouldn't go in during the last week before the fork anyway.)~~ Fork happened.
Don't lint `multiple_inherent_impl` with generic arguments
fixes: #5772
changelog: Treat different generic arguments as different types in `multiple_inherent_impl`
This enables the same warnings that are enabled in `clippy_lints` also
in `clippy_utils` and `clippy_dev`. Then it makes sure, that the
`deny-warnings` feature is passed down to `clippy_lints` and
`clippy_utils` when compiling Clippy.
Remove powi, "square can be computed more efficiently"
powi(2) produces exactly the same native code as x * x
powi was part of the [`suboptimal_flops`] lint
fixes#7058
changelog: Remove powi [`suboptimal_flops`], "square can be computed more efficiently"
Add `needless_bitwise_bool` lint
fixes#6827fixes#1594
changelog: Add ``[`needless_bitwise_bool`]`` lint
Creates a new `bitwise_bool` lint to convert `x & y` to `x && y` when both `x` and `y` are booleans. I also had to adjust thh `needless_bool` lint slightly, and fix a couple failing dogfood tests. I made it a correctness lint as per flip1995's comment [here](https://github.com/rust-lang/rust-clippy/pull/3385#issuecomment-434715723), from a previous WIP attempt at this lint.
Fix FPs about generic args
Fix 2 false positives in [`use_self`] and [`useless_conversion`] lints, by taking into account generic args and comparing them.
Fixes: #7205Fixes: #7206
changelog: Fix FPs about generic args in [`use_self`] and [`useless_conversion`] lints
New lint: `unused_async`
changelog: Adds a lint, `unused_async`, which checks for async functions with no await statements
`unused_async` is a lint that reduces code smell and overhead by encouraging async functions to be refactored into synchronous functions.
Fixes#7176
### Examples
```rust
async fn get_random_number() -> i64 {
4 // Chosen by fair dice roll. Guaranteed to be random.
}
```
Could be written as:
```rust
fn get_random_number() -> i64 {
4 // Chosen by fair dice roll. Guaranteed to be random.
}
```
Something like this, however, should **not** be caught by clippy:
```rust
#[async_trait]
trait AsyncTrait {
async fn foo();
}
struct Bar;
#[async_trait]
impl AsyncTrait for Bar {
async fn foo() {
println!("bar");
}
}
```
Stop linting `else if let` pattern in [`option_if_let_else`] lint
For readability concerns, it is counterproductive to lint `else if let` pattern.
Unfortunately the suggested code is much less readable.
Fixes: #7006
changelog: stop linting `else if let` pattern in [`option_if_let_else`] lint
Remove CrateNum parameter for queries that only work on local crate
The pervasive `CrateNum` parameter is a remnant of the multi-crate rustc idea.
Using `()` as query key in those cases avoids having to worry about the validity of the query key.
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.
`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.
* 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.
Fix `--remap-path-prefix` not correctly remapping `rust-src` component paths and unify handling of path mapping with virtualized paths
This PR fixes#73167 ("Binaries end up containing path to the rust-src component despite `--remap-path-prefix`") by preventing real local filesystem paths from reaching compilation output if the path is supposed to be remapped.
`RealFileName::Named` introduced in #72767 is now renamed as `LocalPath`, because this variant wraps a (most likely) valid local filesystem path.
`RealFileName::Devirtualized` is renamed as `Remapped` to be used for remapped path from a real path via `--remap-path-prefix` argument, as well as real path inferred from a virtualized (during compiler bootstrapping) `/rustc/...` path. The `local_path` field is now an `Option<PathBuf>`, as it will be set to `None` before serialisation, so it never reaches any build output. Attempting to serialise a non-`None` `local_path` will cause an assertion faliure.
When a path is remapped, a `RealFileName::Remapped` variant is created. The original path is preserved in `local_path` field and the remapped path is saved in `virtual_name` field. Previously, the `local_path` is directly modified which goes against its purpose of "suitable for reading from the file system on the local host".
`rustc_span::SourceFile`'s fields `unmapped_path` (introduced by #44940) and `name_was_remapped` (introduced by #41508 when `--remap-path-prefix` feature originally added) are removed, as these two pieces of information can be inferred from the `name` field: if it's anything other than a `FileName::Real(_)`, or if it is a `FileName::Real(RealFileName::LocalPath(_))`, then clearly `name_was_remapped` would've been false and `unmapped_path` would've been `None`. If it is a `FileName::Real(RealFileName::Remapped{local_path, virtual_name})`, then `name_was_remapped` would've been true and `unmapped_path` would've been `Some(local_path)`.
cc `@eddyb` who implemented `/rustc/...` path devirtualisation
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`
This PR implements span quoting, allowing proc-macros to produce spans
pointing *into their own crate*. This is used by the unstable
`proc_macro::quote!` macro, allowing us to get error messages like this:
```
error[E0412]: cannot find type `MissingType` in this scope
--> $DIR/auxiliary/span-from-proc-macro.rs:37:20
|
LL | pub fn error_from_attribute(_args: TokenStream, _input: TokenStream) -> TokenStream {
| ----------------------------------------------------------------------------------- in this expansion of procedural macro `#[error_from_attribute]`
...
LL | field: MissingType
| ^^^^^^^^^^^ not found in this scope
|
::: $DIR/span-from-proc-macro.rs:8:1
|
LL | #[error_from_attribute]
| ----------------------- in this macro invocation
```
Here, `MissingType` occurs inside the implementation of the proc-macro
`#[error_from_attribute]`. Previosuly, this would always result in a
span pointing at `#[error_from_attribute]`
This will make many proc-macro-related error message much more useful -
when a proc-macro generates code containing an error, users will get an
error message pointing directly at that code (within the macro
definition), instead of always getting a span pointing at the macro
invocation site.
This is implemented as follows:
* When a proc-macro crate is being *compiled*, it causes the `quote!`
macro to get run. This saves all of the sapns in the input to `quote!`
into the metadata of *the proc-macro-crate* (which we are currently
compiling). The `quote!` macro then expands to a call to
`proc_macro::Span::recover_proc_macro_span(id)`, where `id` is an
opaque identifier for the span in the crate metadata.
* When the same proc-macro crate is *run* (e.g. it is loaded from disk
and invoked by some consumer crate), the call to
`proc_macro::Span::recover_proc_macro_span` causes us to load the span
from the proc-macro crate's metadata. The proc-macro then produces a
`TokenStream` containing a `Span` pointing into the proc-macro crate
itself.
The recursive nature of 'quote!' can be difficult to understand at
first. The file `src/test/ui/proc-macro/quote-debug.stdout` shows
the output of the `quote!` macro, which should make this eaier to
understand.
This PR also supports custom quoting spans in custom quote macros (e.g.
the `quote` crate). All span quoting goes through the
`proc_macro::quote_span` method, which can be called by a custom quote
macro to perform span quoting. An example of this usage is provided in
`src/test/ui/proc-macro/auxiliary/custom-quote.rs`
Custom quoting currently has a few limitations:
In order to quote a span, we need to generate a call to
`proc_macro::Span::recover_proc_macro_span`. However, proc-macros
support renaming the `proc_macro` crate, so we can't simply hardcode
this path. Previously, the `quote_span` method used the path
`crate::Span` - however, this only works when it is called by the
builtin `quote!` macro in the same crate. To support being called from
arbitrary crates, we need access to the name of the `proc_macro` crate
to generate a path. This PR adds an additional argument to `quote_span`
to specify the name of the `proc_macro` crate. Howver, this feels kind
of hacky, and we may want to change this before stabilizing anything
quote-related.
Additionally, using `quote_span` currently requires enabling the
`proc_macro_internals` feature. The builtin `quote!` macro
has an `#[allow_internal_unstable]` attribute, but this won't work for
custom quote implementations. This will likely require some additional
tricks to apply `allow_internal_unstable` to the span of
`proc_macro::Span::recover_proc_macro_span`.
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
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.
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)
needless_collect: Lint cases with type annotations for indirect usage and recognize `BinaryHeap`
fixes#7110
changelog: needless_collect: Lint cases with type annotations for indirect usage and recognize `BinaryHeap`.
Producing a good suggestion for this lint is already hard when no macros
are involved. With macros the lint message and the suggestion are just
confusing. Since both, producing a good suggestion and figuring out if
this pattern can be re-written inside a macro is nearly impossible, just
bail out.
[single_char_pattern] add strip_prefix and strip_suffix
Title says it all. Adjusted ui tests.
I added the second commit in case you don't like that I moved that table into `single_char_pattern.rs` directly. I don't see any reason why it shouldn't be in that file. It isn't used anywhere else.
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: add strip_prefix and strip_suffix to single_char_pattern lint
while_immutable_cond: check condition for mutation
This fixes#6689 by also checking the bindings mutated in the condition, whereas it was previously only checked in the loop body.
---
changelog: Fix FP in [`while_immutable_cond`] where mutation in the loop variable wasn't picked up.
`implicit_return` improvements
fixes: #6940
changelog: Fix `implicit_return` suggestion for async functions
changelog: Improve `implicit_return` suggestions when returning the result of a macro
changelog: Check for `break` expressions inside a loop which are then implicitly returned
changelog: Allow all diverging functions in `implicit_return`, not just panic functions
Remove leftover plugin conf_file code
changelog: none
Removes dead code that used to support the following syntax:
```rust
#![plugin(clippy(conf_file="path/to/clippy's/configuration"))]
```
RLS (and others?) will need to remove the `&[]` from `clippy_lints::read_conf(&[], sess)`.
r? `@flip1995`
Don't rebuild rustdoc and clippy after checking bootstrap
This works by unconditionally passing -Z unstable-options to the
compiler. This has no affect in practice since bootstrap doesn't use
`deny(rustc::internal)`.
Fixes https://github.com/rust-lang/rust/issues/82461.
r? ```@Mark-Simulacrum```
thread 'rustc' panicked at 'index out of bounds: the len is 0 but the index is 0', src/tools/clippy/clippy_lints/src/matches.rs:1595:53
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
I don't have a minimised testcase because I don't have time to reduce libstd down to a few lines right now.
Fix FN in `iter_cloned_collect` with a large array
fixes#6808
changelog: Fix FN in `iter_cloned_collect` with a large array
I spotted that [is_iterable_array](a362a4d1d0/clippy_lints/src/loops/explicit_iter_loop.rs (L67-L75)) function that `explicit_iter_loop` lint is using only works for array sizes <= 32.
There is this comment:
> IntoIterator is currently only implemented for array sizes <= 32 in rustc
I'm a bit confused, because I read that [IntoIterator for arrays](https://doc.rust-lang.org/src/core/array/mod.rs.html#194-201) with const generic `N` is stable since = "1.0.0". Although Const Generics MVP were stabilized in Rust 1.51.
Should I set MSRV for the current change? I will try to test with older compilers soon.
manual_unwrap_or: fix invalid code suggestion, due to macro expansion
fixes#6965
changelog: fix invalid code suggestion in `manual_unwrap_or` lint, due to macro expansion
extend `single_element_loop` to match `.iter()`
This extends `single_element_loop` to also match `[..].iter()` in the loop argument. Related to #7125, but not completely fixing it due to the lint only firing if the array expression contains a local variable.
---
changelog: none
`single_component_path_imports`: ignore `pub(crate) use some_macro;`
Fixes#7106
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: Ignore exporting a macro within a crate using `pub(crate) use some_macro;` for [`single_component_path_imports`]
Unused io amount detects `.read().ok()?`
fixes#7096
changelog: unused_io_amount now detect expertion like `.read().ok()?`, `.read().or_else(|err| ...)?` and similar expressions.
Better suggestions when returning macro calls.
Suggest changeing all the break expressions in a loop, not just the final statement.
Don't lint divergent functions.
Don't suggest returning the result of any divergent fuction.
Switch transmute_ptr_to_ptr to "pedantic" class.
Per discussion in https://github.com/rust-lang/rust-clippy/issues/6372, this lint has significant false positives.
changelog: transmute_ptr_to_ptr defaults to "allow".
Add lint to check for boolean comparison in assert macro calls
This PR adds a lint to check if an assert macro is using a boolean as "comparison value". For example:
```rust
assert_eq!("a".is_empty(), false);
```
Could be rewritten as:
```rust
assert!(!"a".is_empty());
```
PS: The dev guidelines are amazing. Thanks a lot for writing them!
changelog: Add `bool_assert_comparison` lint
useless use of format! should return function directly
fixes#7066
changelog: [`useless_format`] wraps the content in the braces when it's needed.
r? `@giraffate`
Allow allman style braces in `suspicious_else_formatting`
fixes: #3864
Indentation checks could be added as well, but the lint already doesn't check for it.
changelog: Allow allman style braces in `suspicious_else_formatting`
Fixing FPs for the `branches_sharing_code` lint
Fixes#7053Fixes#7054
And an additional CSS adjustment to support dark mode for every inline code. It currently only works in paragraphs, which was an oversight on my part 😅. [Current Example](https://rust-lang.github.io/rust-clippy/master/index.html#blacklisted_name)
This also includes ~50 lines of doc comments and is therefor not as big as the changes would indicate. 🐧
---
changelog: none
All of these bugs were introduced in this dev version and are therefor not worth a change log entry.
r? `@phansch`
cc: `@camsteffen` since you have a pretty good overview of the `SpanlessEq` implementation 🙃
Add `cloned_instead_of_copied` lint
Don't go cloning all willy-nilly.
Featuring a new `get_iterator_item_ty` util!
changelog: Add cloned_instead_of_copied lint
Closes#3870
Fix: redundant_pattern_matching drop order
Fixes#5746
A note about the change in drop order is added when the scrutinee (or any temporary in the expression) isn't known to be safe to drop in any order (i.e. doesn't implement the `Drop` trait, or contain such a type). There is a whitelist for some `std` types, but it's incomplete. Currently just `Vec<_>`, `Box<_>`, `Rc<_>` and `Arc<_>`, but only if the contained type is also safe to drop in any order.
Another lint for when the drop order changes could be added as allowed by default, but the drop order requirement is pretty subtle in this case. I think the note added to the lint should be enough to make someone think before applying the change.
changelog: Added a note to `redundant_pattern_matching` when the change in drop order might matter
