bors [Mon, 24 May 2021 16:18:19 +0000 (16:18 +0000)]
Auto merge of #7267 - camsteffen:sphash-improvements, r=Manishearth
Some SpanlessHash improvements
changelog: none
* Use `mem::discriminant().hash()` instead of `stable_hash` for simple enums and then use `FxHasher` instead of `StableHasher`. We don't use any StableHash features.
* Use `UnHashMap` for maps keyed by spanless hash values.
bors [Mon, 24 May 2021 00:20:17 +0000 (00:20 +0000)]
Auto merge of #7255 - whatisaphone:feat/similar-names-wparam-lparam, r=giraffate
Allow wparam and lparam in similar_names
`wparam` and `lparam` are often used as generic parameter names in win32 (for example [WindowProc](https://docs.microsoft.com/en-us/previous-versions/windows/desktop/legacy/ms633573(v=vs.85))). This PR adds them to the similar_names exception list.
changelog: [`similar_names`] don't treat wparam and lparam as similar
bors [Fri, 21 May 2021 14:53:04 +0000 (14:53 +0000)]
Auto merge of #7105 - Jarcho:needless_borrow_pat, r=camsteffen
fix `needless_borrow` suggestion
fixes: #2610
While I'm working on this, should needless_borrow be split into two? One lint for expressions and another for patterns. In expression it only lints when the compiler inserts a dereference, but for patterns it's whenever a double reference is created. I think at least the case where a double reference is needed should be split into a new lint as it's not 'needless', it can just be done without a ref binding.
For illustration:
```rust
fn foo(x: &&str) {}
match Some("test") {
// ref binding is useless here
Some(ref x) => *x,
_ => (),
}
match Some("test") {
// ref binding is useless here
Some(ref x) => x.len(),
_ => (),
}
match Some("test") {
// double reference is needed, but could be `Some(x) => foo(&x)`
Some(ref x) => foo(x),
_ => (),
}
```
changelog: Improve the suggestion for `needless_borrow` in patterns to change all usage sites as needed.
changelog: Add lint `ref_binding_to_reference`
Jason Newcomb [Sun, 18 Apr 2021 00:35:39 +0000 (20:35 -0400)]
Improve `needless_borrow` lint
Suggest changing usages of ref bindings to match the new type
Split out some cases into new lint `ref_binding_to_reference`
bors [Thu, 20 May 2021 12:32:06 +0000 (12:32 +0000)]
Auto merge of #7253 - flip1995:shrink-monster, r=xFrednet
Early return from LintPass registration when collecting metadata
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.
flip1995 [Thu, 20 May 2021 11:57:52 +0000 (13:57 +0200)]
Early return from LintPass registration when collecting metadata
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.
bors [Thu, 20 May 2021 10:07:36 +0000 (10:07 +0000)]
Auto merge of #7252 - flip1995:rustup, r=flip1995
Rustup
r? `@ghost`
cc `@xFrednet` There was a change to the `rustc_span::FileName` removing the `Display` impl for it. I adapted the metadata collector to compile with that change. I'm not sure if I changed the behavior with this. The path to the string is now printed relative to the `clippy_lints` dir. So for example `src/swap.rs`. I think this should be fine, but probably something to be aware of.
bors [Wed, 19 May 2021 15:23:58 +0000 (15:23 +0000)]
Auto merge of #7246 - xFrednet:7172-add-lint-level-to-output, r=flip1995
Adding the default lint level to the metadata collection
I noticed while working on the website adaption that the lint groups still had the `clippy::` prefix in the JSON output. This PR removes this prefix and adds a `level` field to each lint and with that simplifies the website display and saves performance.
The deprecated lints get are assigned to the level `none`. This is a bit different in comparison to the current lint list, but I believe that this will look better overall. Unless there is any argument against this :).
That's it just a small baby PR in comparison to the original monster ^^
---
See: #7172 for the full metadata collection to-do list or to suggest a new feature in connection to it.
bors [Tue, 18 May 2021 20:50:01 +0000 (20:50 +0000)]
Auto merge of #84767 - scottmcm:try_trait_actual, r=lcnr
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.
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.
Jason Newcomb [Thu, 15 Apr 2021 16:20:43 +0000 (12:20 -0400)]
Improve `multiple_inherent_impl` lint
Treat different generic arguments as different types.
Allow the lint to be ignored on the type definition, or any impl blocks.
In addition it forwards the `deny-warnings` feature to those sub-crates, so we don't miss warnings that then become a problem during sync. (I wanted to fix that before, but forgot about it, so thanks for pointing it out `@xFrednet!)`
flip1995 [Tue, 18 May 2021 09:01:00 +0000 (11:01 +0200)]
Deny warning in every main sub-crate
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.
bors [Tue, 18 May 2021 08:24:04 +0000 (08:24 +0000)]
Auto merge of #7240 - flip1995:rollup-6nwjgyp, r=flip1995
Rollup of 3 pull requests
Successful merges:
- #7235 (Fix another manual_unwrap_or deref FP)
- #7237 (Add the command to add upstream remote)
- #7239 (CI: update rustup before installing the toolchain on windows)
bors [Mon, 17 May 2021 18:57:14 +0000 (18:57 +0000)]
Auto merge of #7133 - arya-k:master, r=llogiq
Add `needless_bitwise_bool` lint
fixes #6827
fixes #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.
bors [Sun, 16 May 2021 21:02:02 +0000 (21:02 +0000)]
Auto merge of #7214 - xFrednet:7197-collecting-configuration, r=flip1995,camsteffen
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 :upside_down_face:
---
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.
bors [Sun, 16 May 2021 16:57:15 +0000 (16:57 +0000)]
Auto merge of #7217 - xFrednet:7172-metadata-collection-cargo-alias, r=flip1995
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 :upside_down_face:
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.
bors [Thu, 13 May 2021 16:35:42 +0000 (16:35 +0000)]
Auto merge of #7188 - mgacek8:issue7164_needless_collect_FP, r=xFrednet,flip1995
`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
bors [Thu, 13 May 2021 13:55:47 +0000 (13:55 +0000)]
Auto merge of #7095 - Y-Nak:match_single_binding, r=giraffate
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.
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.
Jason Newcomb [Wed, 24 Mar 2021 13:32:29 +0000 (09:32 -0400)]
Improvements to `while_let_on_iterator`
* 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.
bors [Wed, 12 May 2021 11:05:56 +0000 (11:05 +0000)]
Auto merge of #83813 - cbeuw:remap-std, r=michaelwoerister
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
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 :upside_down_face:
Aaron Hill [Sun, 2 Aug 2020 23:52:16 +0000 (19:52 -0400)]
Implement span quoting for proc-macros
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`.
bors [Tue, 11 May 2021 12:44:58 +0000 (12:44 +0000)]
Auto merge of #7193 - wchargin:wchargin-inconsistent-struct-constructor-pedantic, r=camsteffen
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.
projects / rust.git / log