Some "parenthesis" and "parentheses" fixes
"Parenthesis" is the singular (e.g. one `(` or one `)`) and "parentheses" is the plural (multiple `(` or `)`s) and this is not hard to mix up so here are some fixes for that.
Inspired by #89958
Add `format_in_format_args` and `to_string_in_format_args` lints
Fixes#7667 and #7729
I put these in `perf` since that was one of `@jplatte's` suggestions, and `redundant_clone` (which I consider to be similar) lives there as well.
However, I am open to changing the category or anything else.
r? `@camsteffen`
changelog: Add `format_in_format_args` and `to_string_in_format_args` lints
Do not expand macros in equatable_if_let suggestion
Fixes#7781
Let's use Hacktoberfest as a motivation to start contributing PRs myself again :)
changelog: [`equatable_if_let`]: No longer expands macros in the suggestion
Allow giving reasons for `disallowed_types`
Similar to #7609 but for the `disallowed_type` lint. The permitted form of configuration is the same as for `disallowed_methods`.
changelog: Allow giving reasons for [`disallowed_type`]
Fix FP in external macros for `mut_mut` lint
Fix FP in `mut_mut` lint when type is defined in external macros.
fixes: #6922
changelog: [`mut_mut`] Fix FP when type is defined in external macros
Refactor `clippy::match_ref_pats` to check for multiple reference patterns
fixes#7740
When there is only one pattern, to begin with, i.e. a single deref(`&`), then in such cases we suppress `clippy::match_ref_pats`.
This is done by checking the count of the reference pattern and emitting `clippy::match_ref_pats` when more than one pattern is present.
Removed certain errors in the `stderr` tests as they would not be triggered.
changelog: Refactor `clippy::match_ref_pats` to check for multiple reference patterns
Before this lint didn't trigger on macros. With rust-lang/rust#88175
this isn't enough anymore. In this PR a `WhileLoop` desugaring kind was
introduced. This overrides the span of expanded expressions when
lowering the while loop. So if a while loop is in a macro, the
expressions that it expands to are no longer marked with
`ExpnKind::Macro`, but with `ExpnKind::Desugaring`. In general, this is
the correct behavior and the same that is done for `ForLoop`s. It just
tripped up this lint.
Restriction lint for function pointer casts
The existing lints for function pointer casts cover the cases where a cast is non-portable or would result in truncation, however there's currently no way to forbid numeric function pointer casts entirely.
I've added a new lint `fn_to_numeric_cast_any`, which allows one to ban _all_ numeric function pointer casts, including to `usize`. This is useful if you're writing high-level Rust and want to opt-out of potentially surprising behaviour, avoiding silent bugs from forgotten parentheses, e.g.
```rust
fn foo() -> u32 {
10
}
fn main() {
let _ = foo as usize; // oops, forgot to call foo and got a random address instead!
}
```
~~I'm open to suggestions for the naming of the lint, because `fn_to_numeric_cast_any` is a bit clunky. Ideally I'd call this lint `fn_to_numeric_cast`, but that name is already taken for the more specific lint~~. We've stuck with `fn_to_numeric_cast_any` to avoid renaming the existing lint, or choosing a different name that's too generic (like `fn_cast`).
I'm also open to changing the suggestion behaviour, as adding parentheses is only one of many possible ways to fix the lint.
changelog: add ``[`fn_to_numeric_cast_any`]`` restriction lint
Make `shadow_reuse` suggestion less verbose
Closes#7764
Make `shadow_reuse` suggestion less verbose.
changelog: [`shadow_reuse`] does not warn shadowing statement
fix bug for large_enum_variants
Fix the discussion problem in the issue of https://github.com/rust-lang/rust-clippy/issues/7666#issuecomment-919654291
About the false positive problem of case:
```rust
enum LargeEnum6 {
A,
B([u8;255]),
C([u8;200]),
}
```
changelog: Fix largest_enum_variant wrongly identifying the second largest variant.
Rustup
This needs a review this time. Especially 521bf8f0fa cc `@camsteffen` I think this is necessary now, because `itertools` is no longer a dependency of `clippy_utils` and therefore this path can't be found 🤔
( I forgot about the sync last week. I should get to document this process better, so other people can do it when I'm not around )
changelog: none
bump clippy crates to edition 2021
Also helps with dogfooding edition 2021 a bit. :)
Tests passed locally.
---
changelog: bump edition from 2018 to 2021
Demote float_cmp to pedantic
See this issue: https://github.com/rust-lang/rust-clippy/issues/7666
This is one of the most frequently suppressed lints. It is deny-by-default. It is not actually clearly wrong, as there are many instances where direct float comparison is actually desirable. It is only after operating on floats that they may lose precision, and that depends greatly on the operation. As most correctness lints have a much higher standard of error, being based on hard and fast binary logic, this should not be amongst them.
A linter is not a substitute for observing the math carefully and running tests, and doing the desirable thing is even more likely to lead one to want exact comparisons.
changelog: Demote [`float_cmp`] from correctness to pedantic lints
Be explicit about using Binder::dummy
This is somewhat of a late followup to the binder refactor PR. It removes `ToPredicate` and `ToPolyTraitImpls` that hide the use of `Binder::dummy`. While this does make code a bit more verbose, it allows us be more careful about where we create binders.
Another alternative here might be to add a new trait `ToBinder` or something with a `dummy()` fn. Which could still allow grepping but allows doing something like `trait_ref.dummy()` (but I also wonder if longer-term, it would be better to be even more explicit with a `bind_with_vars(ty::List::empty())` *but* that's not clear yet.
r? ``@nikomatsakis``
Don't lint `suspicious_else_formatting` inside proc-macros
fixes: #7650
I'll add a test for this one soon.
changelog: Don't lint `suspicious_else_formatting` inside proc-macros
Expand box_vec lint to box_collection
fixed#7451
changelog: Expand `box_vec` into [`box_collection`], and have it error on all sorts of boxed collections
Change `while_let_on_iterator` suggestion to use `by_ref()`
It came up in the discussion #7659 that suggesting `iter.by_ref()` is a clearer suggestion than `&mut iter`. I personally think they're equivalent, but if `by_ref()` is clearer to people then that should be the suggestion.
changelog: Change `while_let_on_iterator` suggestion when using `&mut` to use `by_ref()`
New lint: `same_name_method`
changelog: ``[`same_name_method`]``
fix: https://github.com/rust-lang/rust-clippy/issues/7632
It only compares a method in `impl` with another in `impl trait for`
It doesn't lint two methods in two traits.
I'm not sure my approach is the best way. I meet difficulty in other approaches.
Fix various redundant_closure bugs
changelog: Fix various false negatives and false positives for [`redundant_closure`]
Closes#3071Closes#4002
This lint is full of weird nuances and this is basically a re-write to tighten up the logic.
Encode spans relative to the enclosing item
The aim of this PR is to avoid recomputing queries when code is moved without modification.
MCP at https://github.com/rust-lang/compiler-team/issues/443
This is achieved by :
1. storing the HIR owner LocalDefId information inside the span;
2. encoding and decoding spans relative to the enclosing item in the incremental on-disk cache;
3. marking a dependency to the `source_span(LocalDefId)` query when we translate a span from the short (`Span`) representation to its explicit (`SpanData`) representation.
Since all client code uses `Span`, step 3 ensures that all manipulations
of span byte positions actually create the dependency edge between
the caller and the `source_span(LocalDefId)`.
This query return the actual absolute span of the parent item.
As a consequence, any source code motion that changes the absolute byte position of a node will either:
- modify the distance to the parent's beginning, so change the relative span's hash;
- dirty `source_span`, and trigger the incremental recomputation of all code that
depends on the span's absolute byte position.
With this scheme, I believe the dependency tracking to be accurate.
For the moment, the spans are marked during lowering.
I'd rather do this during def-collection,
but the AST MutVisitor is not practical enough just yet.
The only difference is that we attach macro-expanded spans
to their expansion point instead of the macro itself.
Fix result order for `manual_split_once` when `rsplitn` is used
fixes: #7656
changelog: Fix result order for `manual_split_once` when `rsplitn` is used
rustc: use more correct span data in for loop desugaring
Fixes#82462
Before:
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
|
LL | for x in DroppingSlice(&*v).iter(); {
| +
After:
help: consider adding semicolon after the expression so its temporaries are dropped sooner, before the local variables declared by the block are dropped
|
LL | };
| +
This seems like a reasonable fix: since the desugared "expr_drop_temps_mut" contains the entire desugared loop construct, its span should contain the entire loop construct as well.
Add new lint `iter_not_returning_iterator`
Add new lint [`iter_not_returning_iterator`] to detect method `iter()` or `iter_mut()` returning a type not implementing `Iterator`
changelog: Add new lint [`iter_not_returning_iterator`]
Avoid slice indexing in Clippy (down with the ICEs)
While working on #7569 I got about 23 lint reports where we can avoid slice indexing by destructing the slice early. This is a preparation PR to avoid fixing them in the lint PR. (The implementation already takes about 300 lines without tests 😅). Either way, this should hopefully be easy to review 🙃
---
changelog: none
Provide `layout_of` automatically (given tcx + param_env + error handling).
After #88337, there's no longer any uses of `LayoutOf` within `rustc_target` itself, so I realized I could move the trait to `rustc_middle::ty::layout` and redesign it a bit.
This is similar to #88338 (and supersedes it), but at no ergonomic loss, since there's no funky `C: LayoutOf<Ty = Ty>` -> `Ty: TyAbiInterface<C>` generic `impl` chain, and each `LayoutOf` still corresponds to one `impl` (of `LayoutOfHelpers`) for the specific context.
After this PR, this is what's needed to get `trait LayoutOf` (with the `layout_of` method) implemented on some context type:
* `TyCtxt`, via `HasTyCtxt`
* `ParamEnv`, via `HasParamEnv`
* a way to transform `LayoutError`s into the desired error type
* an error type of `!` can be paired with having `cx.layout_of(...)` return `TyAndLayout` *without* `Result<...>` around it, such as used by codegen
* this is done through a new `LayoutOfHelpers` trait (and so is specifying the type of `cx.layout_of(...)`)
When going through this path (and not bypassing it with a manual `impl` of `LayoutOf`), the end result is that only the error case can be customized, the query itself and the success paths are guaranteed to be uniform.
(**EDIT**: just noticed that because of the supertrait relationship, you cannot actually implement `LayoutOf` yourself, the blanket `impl` fully covers all possible context types that could ever implement it)
Part of the motivation for this shape of API is that I've been working on querifying `FnAbi::of_*`, and what I want/need to introduce for that looks a lot like the setup in this PR - in particular, it's harder to express the `FnAbi` methods in `rustc_target`, since they're much more tied to `rustc` concepts.
r? `@nagisa` cc `@oli-obk` `@bjorn3`
Path remapping: Make behavior of diagnostics output dependent on presence of --remap-path-prefix.
This PR fixes a regression (#87745) with `--remap-path-prefix` where the flag stopped causing diagnostic messages to be remapped as well. The regression was introduced in https://github.com/rust-lang/rust/pull/83813 where we erroneously assumed that remapping of diagnostic messages was not desired anymore (because #70642 partially undid that functionality with nobody objecting).
The issue is fixed by making `--remap-path-prefix` remap diagnostic messages again, including for paths that have been remapped in upstream crates (e.g. the standard library). This means that "sysroot-localization" (implemented in #70642) is also disabled if `rustc` is invoked with `--remap-path-prefix`. The assumption is that once someone starts explicitly remapping paths they also don't want paths to their local Rust installation in their build output.
In the future we might want to give more fine-grained control over this behavior via compiler flags (see https://github.com/rust-lang/rfcs/pull/3127 for a related RFC). For now this PR is intended as a regression fix.
This PR is an alternative to https://github.com/rust-lang/rust/pull/88191, which makes diagnostic messages be remapped unconditionally. That approach, however, would effectively revert #70642.
Fixes https://github.com/rust-lang/rust/issues/87745.
cc `@cbeuw`
r? `@ghost`
Fix `option_if_let_else`
fixes: #5822fixes: #6737fixes: #7567
The inference from #6137 still exists so I'm not sure if this should be moved from the nursery. Before doing that though I'd almost want to see this split into two lints. One suggesting `map_or` and the other suggesting `map_or_else`.
`map_or_else` tends to have longer expressions for both branches so it doesn't end up much shorter than a match expression in practice. It also seems most people find it harder to read. `map_or` at least has the terseness benefit of being on one line most of the time, especially when the `None` branch is just a literal or path expression.
changelog: `break` and `continue` statments local to the would-be closure are allowed in `option_if_let_else`
changelog: don't lint in const contexts in `option_if_let_else`
changelog: don't lint when yield expressions are used in `option_if_let_else`
changelog: don't lint when the captures made by the would-be closure conflict with the other branch in `option_if_let_else`
changelog: don't lint when a field of a local is used when the type could be pontentially moved from in `option_if_let_else`
changelog: in some cases, don't lint when scrutinee expression conflicts with the captures of the would-be closure in `option_if_let_else`
Add module_style lint to style
changelog: Add new [`module_style`] style lint
This is a configurable (no mod file/mod file) lint that determines if `mod.rs` is used consistently or if `mod.rs` is never used (using the new mod layout).
Don't report function calls as unnecessary operation if used in array index
Attempts to fix: #7412
changelog: Don't report function calls used in indexing as unnecessary operation. [`unnecessary_operation`]
Add tests for disallowed_mod in ui-cargo test section
Use correct algorithm to determine if mod.rs is missing
Move to two lints and remove config option
Switch lint names so they read "warn on ..."
Emit the same help info for self_named_mod_file warnings
Bail when both lints are Allow
Reword help message for both module_style lints
Add new lint `negative_feature_names` and `redundant_feature_names`
Add new lint [`negative_feature_names`] to detect feature names with prefixes `no-` or `not-` and new lint [`redundant_feature_names`] to detect feature names with prefixes `use-`, `with-` or suffix `-support`
changelog: Add new lint [`negative_feature_names`] and [`redundant_feature_names`]
New lint `manual_split_once`
This is a WIP because it still needs to recognize more patterns. Currently handles:
```rust
s.splitn(2, ' ').next();
s.splitn(2, ' ').nth(0)
s.splitn(2, ' ').nth(1);
s.splitn(2, ' ').skip(0).next();
s.splitn(2, ' ').skip(1).next();
s.splitn(2, ' ').next_tuple(); // from itertools
// as well as `unwrap()` and `?` forms
```
Still to do:
```rust
let mut iter = s.splitn(2, ' ');
(iter.next().unwrap(), iter.next()?)
let mut iter = s.splitn(2, ' ');
let key = iter.next().unwrap();
let value = iter.next()?;
```
Suggestions on other patterns to check for would be useful. I've done a search on github for uses of `splitn`. Still have yet to actually look through the results.
There's also the question of whether the lint shouold trigger on all uses of `splitn` with two values, or only on recognized usages. The former could have false positives where it couldn't be replaced, but I'm not sure how common that would be.
changelog: Add lint `manual_split_once`
* `break` and `continue` statments local to the would-be closure are allowed
* don't lint in const contexts
* don't lint when yield expressions are used
* don't lint when the captures made by the would-be closure conflict with the other branch
* don't lint when a field of a local is used when the type could be pontentially moved from
* in some cases, don't lint when scrutinee expression conflicts with the captures of the would-be closure
Uplift the invalid_atomic_ordering lint from clippy to rustc
This is mostly just a rebase of https://github.com/rust-lang/rust/pull/79654; I've copy/pasted the text from that PR below.
r? `@lcnr` since you reviewed the last one, but feel free to reassign.
---
This is an implementation of https://github.com/rust-lang/compiler-team/issues/390.
As mentioned, in general this turns an unconditional runtime panic into a (compile time) lint failure. It has no false positives, and the only false negatives I'm aware of are if `Ordering` isn't specified directly and is comes from an argument/constant/whatever.
As a result of it having no false positives, and the alternative always being strictly wrong, it's on as deny by default. This seems right.
In the [zulip stream](https://rust-lang.zulipchat.com/#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Uplift.20the.20.60invalid_atomic_ordering.60.20lint.20from.20clippy/near/218483957) `@joshtriplett` suggested that lang team should FCP this before landing it. Perhaps libs team cares too?
---
Some notes on the code for reviewers / others below
## Changes from clippy
The code is changed from [the implementation in clippy](68cf94f6a6/clippy_lints/src/atomic_ordering.rs) in the following ways:
1. Uses `Symbols` and `rustc_diagnostic_item`s instead of string literals.
- It's possible I should have just invoked Symbol::intern for some of these instead? Seems better to use symbol, but it did require adding several.
2. The functions are moved to static methods inside the lint struct, as a way to namespace them.
- There's a lot of other code in that file — which I picked as the location for this lint because `@jyn514` told me that seemed reasonable.
3. Supports unstable AtomicU128/AtomicI128.
- I did this because it was almost easier to support them than not — not supporting them would have (ideally) required finding a way not to give them a `rustc_diagnostic_item`, which would have complicated an already big macro.
- These don't have tests since I wasn't sure if/how I should make tests conditional on whether or not the target has the atomic... This is to a certain extent an issue of 64bit atomics too, but 128-bit atomics are much less common. Regardless, the existing tests should be *more* than thorough enough here.
4. Minor changes like:
- grammar tweaks ("loads cannot have `Release` **and** `AcqRel` ordering" => "loads cannot have `Release` **or** `AcqRel` ordering")
- function renames (`match_ordering_def_path` => `matches_ordering_def_path`),
- avoiding clippy-specific helper methods that don't exist in rustc_lint and didn't seem worth adding for this case (for example `cx.struct_span_lint` vs clippy's `span_lint_and_help` helper).
## Potential issues
(This is just about the code in this PR, not conceptual issues with the lint or anything)
1. I'm not sure if I should have used a diagnostic item for `Ordering` and its variants (I couldn't figure out how really, so if I should do this some pointers would be appreciated).
- It seems possible that failing to do this might possibly mean there are more cases this lint would miss, but I don't really know how `match_def_path` works and if it has any pitfalls like that, so maybe not.
2. I *think* I deprecated the lint in clippy (CC `@flip1995` who asked to be notified about clippy changes in the future in [this comment](https://github.com/rust-lang/rust/pull/75671#issuecomment-718731659)) but I'm not sure if I need to do anything else there.
- I'm kind of hoping CI will catch if I missed anything, since `x.py test src/tools/clippy` fails with a lot of errors with and without my changes (and is probably a nonsense command regardless). Running `cargo test` from src/tools/clippy also fails with unrelated errors that seem like refactorings that didnt update clippy? So, honestly no clue.
3. I wasn't sure if the description/example I gave good. Hopefully it is. The example is less thorough than the one from clippy here: https://rust-lang.github.io/rust-clippy/master/index.html#invalid_atomic_ordering. Let me know if/how I should change it if it needs changing.
4. It pulls in the `if_chain` crate. This crate was already used in clippy, and seems like it's used elsewhere in rustc, but I'm willing to rewrite it to not use this if needed (I'd prefer not to, all things being equal).
- Deprecate clippy::invalid_atomic_ordering
- Use rustc_diagnostic_item for the orderings in the invalid_atomic_ordering lint
- Reduce code duplication
- Give up on making enum variants diagnostic items and just look for
`Ordering` instead
I ran into tons of trouble with this because apparently the change to
store HIR attrs in a side table also gave the DefIds of the
constructor instead of the variant itself. So I had to change
`matches_ordering` to also check the grandparent of the defid as well.
- Rename `atomic_ordering_x` symbols to just the name of the variant
- Fix typos in checks - there were a few places that said "may not be
Release" in the diagnostic but actually checked for SeqCst in the lint.
- Make constant items const
- Use fewer diagnostic items
- Only look at arguments after making sure the method matches
This prevents an ICE when there aren't enough arguments.
- Ignore trait methods
- Only check Ctors instead of going through `qpath_res`
The functions take values, so this couldn't ever be anything else.
- Add if_chain to allowed dependencies
- Fix grammar
- Remove unnecessary allow
Manual map 7413
fixes: #7413
This only fixes the specific problem from #7413, not the general case. The full fix requires interacting with the borrow checker to determine the lifetime of all the borrows made in the function. I'll open an issue about it later.
changelog: Don't suggest using `map` when the option is borrowed in the match, and also consumed in the arm.
changelog: Locals declared within the would-be closure will not prevent the closure from being suggested in `manual_map` and `map_entry`
Add `unwrap_or_else_default` lint
---
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: Add a new [`unwrap_or_else_default`] style lint. This will catch `unwrap_or_else(Default::default)` on Result and Option and suggest `unwrap_or_default()` instead.
`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)
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`
