Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move our paths over to diagnostic items #5393

Open
Manishearth opened this issue Mar 30, 2020 · 5 comments
Open

Move our paths over to diagnostic items #5393

Manishearth opened this issue Mar 30, 2020 · 5 comments
Labels
C-an-interesting-project Category: Interesting projects, that usually are more involved design/code wise. C-tracking-issue Category: Tracking Issue E-medium Call for participation: Medium difficulty level problem and requires some initial experience.

Comments

@Manishearth
Copy link
Member

Currently we have a ton of hardcoded paths for stdlib types.

Rustc has a way of tagging types for diagnostics via a "diagnostic item" API (rust-lang/rust#60966). We should go through rustc and tag most of these diagnostic items, and replace the hardcoded paths with hardcoded diagnostic item names instead. We can still keep the path framework around for rapid prototyping, but have a comment recommending people switch to diagnostic items whenever possible.
@Manishearth Manishearth added E-medium Call for participation: Medium difficulty level problem and requires some initial experience. C-an-interesting-project Category: Interesting projects, that usually are more involved design/code wise. labels Mar 30, 2020
@flip1995 flip1995 added the C-tracking-issue Category: Tracking Issue label Mar 30, 2020
@flip1995
Copy link
Member

Marking it as tracking issue and pinning it, since this is probably done in multiple steps.

@flip1995 flip1995 pinned this issue Mar 30, 2020
@phansch phansch mentioned this issue Apr 10, 2020
Merged
bors added a commit that referenced this issue Apr 14, 2020
@bors
Auto merge of #5449 - phansch:diagnostic-items, r=matthiaskrgr
81b3e70 
Make use of more diagnostic items

This makes use of some (not all) already existing diagnostic items. Specifically:

* 79982a2: `core::mem::uninitialized`, `core::mem::zeroed`, `alloc::sync::Arc`, `alloc::sync::Rc`
* 83874d0: `Option` and `Result`

cc #5393

changelog: none
@phansch phansch mentioned this issue Apr 22, 2020
Merged
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Apr 22, 2020
@Dylan-DPC
Rollup merge of rust-lang#71414 - phansch:more-diagnostic-items, r=Ma…
01fdc88 
…nishearth

More diagnostic items for Clippy usage

This adds a couple of more diagnostic items to be used in Clippy.
I chose these particular ones because they were the types which we seem
to check for the most in Clippy. I'm not sure if the `cfg_attr(not(test))`
is needed, but it was also used for `Vec` and a few other types.

cc rust-lang/rust-clippy#5393

r? @Manishearth
phansch added a commit to phansch/rust-clippy that referenced this issue Apr 23, 2020
@phansch
More diagnostic items
c3acaf3 
In particular for:

* `VecDeque`
* `String`
* `Mutex`
* `HashMap`
* `HashSet`

cc rust-lang/rust#71414 rust-lang#5393
@phansch phansch mentioned this issue Apr 23, 2020
Merged
bors added a commit that referenced this issue Apr 23, 2020
@bors
Auto merge of #5509 - phansch:more-diagnostic-items2, r=matthiaskrgr
855395b 
Use more diagnostic items

In particular for:

* `VecDeque`
* `String`
* `Mutex`
* `HashMap`
* `HashSet`

cc rust-lang/rust#71414 #5393

---

changelog: none
bors added a commit that referenced this issue Apr 23, 2020
@bors
Auto merge of #5509 - phansch:more-diagnostic-items2, r=matthiaskrgr
1b7ed5c 
Use more diagnostic items

In particular for:

* `VecDeque`
* `String`
* `Mutex`
* `HashMap`
* `HashSet`

cc rust-lang/rust#71414 #5393

---

changelog: none
phansch added a commit to phansch/rust-clippy that referenced this issue Apr 26, 2020
@phansch
More diagnostic items
0480ff8 
In particular for:

* `VecDeque`
* `String`
* `Mutex`
* `HashMap`
* `HashSet`

cc rust-lang/rust#71414 rust-lang#5393
bors added a commit that referenced this issue Apr 26, 2020
@bors
Auto merge of #5509 - phansch:more-diagnostic-items2, r=<try>
ae0576a 
Use more diagnostic items

In particular for:

* `VecDeque`
* `String`
* `Mutex`
* `HashMap`
* `HashSet`

cc rust-lang/rust#71414 #5393

---

changelog: none
bors added a commit that referenced this issue Apr 26, 2020
@bors
Auto merge of #5509 - phansch:more-diagnostic-items2, r=<try>
90a9e88 
Use more diagnostic items

In particular for:

* `VecDeque`
* `String`
* `Mutex`
* `HashMap`
* `HashSet`

cc rust-lang/rust#71414 #5393

---

changelog: none
bors added a commit that referenced this issue Apr 26, 2020
@bors
Auto merge of #5509 - phansch:more-diagnostic-items2, r=matthiaskrgr
5c12f2b 
Use more diagnostic items

In particular for:

* `VecDeque`
* `String`
* `Mutex`
* `HashMap`
* `HashSet`

cc rust-lang/rust#71414 #5393

---

changelog: none
ThibsG pushed a commit to ThibsG/rust-clippy that referenced this issue May 2, 2020
@phansch @ThibsG
More diagnostic items
69dad2d 
In particular for:

* `VecDeque`
* `String`
* `Mutex`
* `HashMap`
* `HashSet`

cc rust-lang/rust#71414 rust-lang#5393
@esamudera esamudera mentioned this issue Jun 9, 2020
Merged
@Ryan1729 Ryan1729 mentioned this issue Jul 27, 2020
Merged
bors added a commit that referenced this issue Jul 28, 2020
@bors
Auto merge of #5848 - Ryan1729:add-derive_ord_xor_partial_ord-lint, r…
c896119 
…=matthiaskrgr

Add derive_ord_xor_partial_ord lint

Fix #1621

Some remarks:
This PR follows the example of the analogous derive_hash_xor_partial_eq lint where possible.
I initially tried using the `match_path` function to identify `Ord` implementation like the derive_hash_xor_partial_eq lint currently does, for `Hash` implementations but that didn't work.

Specifically, the structs at the top level were getting paths that matched `&["$crate", "cmp", "Ord"]` instead of `&["std", "cmp", "Ord"]`. While trying to figure out what to do instead I saw the comment at the top of [clippy_lints/src/utils/paths.rs](https://github.com/rust-lang/rust-clippy/blob/f5d429cd762423901f946abd800dc2cd91366ccf/clippy_lints/src/utils/paths.rs#L5) that mentioned [this issue](#5393) and suggested to use diagnostic items instead of hardcoded paths whenever possible. I looked for a way to identify `Ord` implementations with diagnostic items, but (possibly because this was the first time I had heard of diagnostic items,) I was unable to find one.

Eventually I tried using `get_trait_def_id` and comparing `DefId` values directly and that seems to work as expected. Maybe there's a better approach however?

changelog: new lint: derive_ord_xor_partial_ord
bors added a commit that referenced this issue Jul 29, 2020
@bors
Auto merge of #5848 - Ryan1729:add-derive_ord_xor_partial_ord-lint, r…
4bc4fd9 
…=matthiaskrgr

Add derive_ord_xor_partial_ord lint

Fix #1621

Some remarks:
This PR follows the example of the analogous derive_hash_xor_partial_eq lint where possible.
I initially tried using the `match_path` function to identify `Ord` implementation like the derive_hash_xor_partial_eq lint currently does, for `Hash` implementations but that didn't work.

Specifically, the structs at the top level were getting paths that matched `&["$crate", "cmp", "Ord"]` instead of `&["std", "cmp", "Ord"]`. While trying to figure out what to do instead I saw the comment at the top of [clippy_lints/src/utils/paths.rs](https://github.com/rust-lang/rust-clippy/blob/f5d429cd762423901f946abd800dc2cd91366ccf/clippy_lints/src/utils/paths.rs#L5) that mentioned [this issue](#5393) and suggested to use diagnostic items instead of hardcoded paths whenever possible. I looked for a way to identify `Ord` implementations with diagnostic items, but (possibly because this was the first time I had heard of diagnostic items,) I was unable to find one.

Eventually I tried using `get_trait_def_id` and comparing `DefId` values directly and that seems to work as expected. Maybe there's a better approach however?

changelog: new lint: derive_ord_xor_partial_ord
flip1995 added a commit to flip1995/rust-clippy that referenced this issue Aug 4, 2020
@flip1995
Rollup merge of rust-lang#5848 - Ryan1729:add-derive_ord_xor_partial_…
e2d2e0e 
…ord-lint, r=matthiaskrgr

Add derive_ord_xor_partial_ord lint

Fix rust-lang#1621

Some remarks:
This PR follows the example of the analogous derive_hash_xor_partial_eq lint where possible.
I initially tried using the `match_path` function to identify `Ord` implementation like the derive_hash_xor_partial_eq lint currently does, for `Hash` implementations but that didn't work.

Specifically, the structs at the top level were getting paths that matched `&["$crate", "cmp", "Ord"]` instead of `&["std", "cmp", "Ord"]`. While trying to figure out what to do instead I saw the comment at the top of [clippy_lints/src/utils/paths.rs](https://github.com/rust-lang/rust-clippy/blob/f5d429cd762423901f946abd800dc2cd91366ccf/clippy_lints/src/utils/paths.rs#L5) that mentioned [this issue](rust-lang#5393) and suggested to use diagnostic items instead of hardcoded paths whenever possible. I looked for a way to identify `Ord` implementations with diagnostic items, but (possibly because this was the first time I had heard of diagnostic items,) I was unable to find one.

Eventually I tried using `get_trait_def_id` and comparing `DefId` values directly and that seems to work as expected. Maybe there's a better approach however?

changelog: new lint: derive_ord_xor_partial_ord
flip1995 added a commit to flip1995/rust-clippy that referenced this issue Aug 4, 2020
@flip1995
Rollup merge of rust-lang#5848 - Ryan1729:add-derive_ord_xor_partial_…
888067c 
…ord-lint, r=matthiaskrgr

Add derive_ord_xor_partial_ord lint

Fix rust-lang#1621

Some remarks:
This PR follows the example of the analogous derive_hash_xor_partial_eq lint where possible.
I initially tried using the `match_path` function to identify `Ord` implementation like the derive_hash_xor_partial_eq lint currently does, for `Hash` implementations but that didn't work.

Specifically, the structs at the top level were getting paths that matched `&["$crate", "cmp", "Ord"]` instead of `&["std", "cmp", "Ord"]`. While trying to figure out what to do instead I saw the comment at the top of [clippy_lints/src/utils/paths.rs](https://github.com/rust-lang/rust-clippy/blob/f5d429cd762423901f946abd800dc2cd91366ccf/clippy_lints/src/utils/paths.rs#L5) that mentioned [this issue](rust-lang#5393) and suggested to use diagnostic items instead of hardcoded paths whenever possible. I looked for a way to identify `Ord` implementations with diagnostic items, but (possibly because this was the first time I had heard of diagnostic items,) I was unable to find one.

Eventually I tried using `get_trait_def_id` and comparing `DefId` values directly and that seems to work as expected. Maybe there's a better approach however?

changelog: new lint: derive_ord_xor_partial_ord
@tesuji
Copy link
Contributor

tesuji commented Sep 15, 2020 •
edited
Loading

Could we use lang_items if possible ?

For example: begin_panic has lang_items but no diagnostic item.
https://github.com/rust-lang/rust/blob/c1589cc819ba7cf289c3ccbab30c215f0a6ba7d7/library/std/src/panicking.rs#L491-L499

@flip1995
Copy link
Member

If there is a LangItem we should use it. But this issue is rather about adding #[rustc_diagnostic_item(..)] in rustc and then use it in Clippy. We can do this easier now that Clippy is a subtree in rustc.

@boxdot boxdot mentioned this issue Nov 23, 2020
Closed
@anall anall mentioned this issue Feb 27, 2021
Merged
bors added a commit that referenced this issue Mar 8, 2021
@bors
Auto merge of #6807 - anall:feature/use_new_diagnostics, r=Manishearth
d02ca3b 
migrate paths to newly-added diagnostic items

This gets rid of the following paths:
  * OS_STRING
  * TO_OWNED
  * TO_STRING

Removes some usages of:
 * PATH_BUF

Per #5393

also removes unneeded `is_ty_param_path` from `clippy_lints::types` and relocates `is_ty_param_lang_item` and `is_ty_param_diagnostic_item` to `clippy_utils`.

changelog: none
@Y-Nak Y-Nak mentioned this issue Mar 12, 2021
Merged
bors added a commit that referenced this issue Jul 27, 2021
@bors
Auto merge of #7466 - xFrednet:5393-use-more-diagnostic-items, r=flip…
ceb7a86 
…1995

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
@xFrednet xFrednet mentioned this issue Aug 11, 2021
Closed
@xFrednet xFrednet mentioned this issue Aug 26, 2021
Merged
@camsteffen camsteffen mentioned this issue Oct 7, 2021
Closed
@Alexendoo Alexendoo mentioned this issue Feb 12, 2022
Merged
bors added a commit that referenced this issue Feb 12, 2022
@bors
Auto merge of #8421 - Alexendoo:paths, r=llogiq
c37d6ee 
Replace a few paths with diagnostic items

A fairly small change towards #5393

changelog: none
@Alexendoo Alexendoo mentioned this issue Feb 25, 2022
Merged
bors added a commit that referenced this issue Feb 25, 2022
@bors
Auto merge of #8474 - Alexendoo:paths, r=Manishearth
4417f78 
Replace some more paths with diagnostic items

cc #5393

Replaces the macro & mem paths, and catches a couple others that were unused

changelog: none
eggyal pushed a commit to eggyal/copse that referenced this issue Jan 9, 2023
@JohnTitor
Rollup merge of #87170 - xFrednet:clippy-5393-add-diagnostic-items, r…
657ff25 
…=Manishearth,oli-obk

Add diagnostic items for Clippy

This adds a bunch of diagnostic items to `std`/`core`/`alloc` functions, structs and traits used in Clippy. The actual refactorings in Clippy to use these items will be done in a different PR in Clippy after the next sync.

This PR doesn't include all paths Clippy uses, I've only gone through the first 85 lines of Clippy's [`paths.rs`](https://github.com/rust-lang/rust-clippy/blob/ecf85f4bdc319f9d9d853d1fff68a8a25e64c7a8/clippy_utils/src/paths.rs) (after rust-lang/rust-clippy#7466) to get some feedback early on. I've also decided against adding diagnostic items to methods, as it would be nicer and more scalable to access them in a nicer fashion, like adding a `is_diagnostic_assoc_item(did, sym::Iterator, sym::map)` function or something similar (Suggested by `@camsteffen` [on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/147480-t-compiler.2Fwg-diagnostics/topic/Diagnostic.20Item.20Naming.20Convention.3F/near/225024603))

There seems to be some different naming conventions when it comes to diagnostic items, some use UpperCamelCase (`BinaryHeap`) and some snake_case (`hashmap_type`). This PR uses UpperCamelCase for structs and traits and snake_case with the module name as a prefix for functions. Any feedback on is this welcome.

cc: rust-lang/rust-clippy#5393

r? `@Manishearth`
@xFrednet xFrednet mentioned this issue Jan 14, 2023
Closed
@CBSpeir CBSpeir mentioned this issue Apr 23, 2024
Merged
fmease added a commit to fmease/rust that referenced this issue Apr 24, 2024
@fmease
Rollup merge of rust-lang#124308 - CBSpeir:diagnostic-item-enumerate,…
a3e8852 
… r=compiler-errors

Add diagnostic item for `std::iter::Enumerate`

This adds a diagnostic item for `std::iter::Enumerate`.  The change will be used by the clippy `unused_enumerate_index` lint to move away from type paths to using diagnostic items.

see: rust-lang/rust-clippy#5393
fmease added a commit to fmease/rust that referenced this issue Apr 24, 2024
@fmease
Rollup merge of rust-lang#124308 - CBSpeir:diagnostic-item-enumerate,…
6671edb 
… r=compiler-errors

Add diagnostic item for `std::iter::Enumerate`

This adds a diagnostic item for `std::iter::Enumerate`.  The change will be used by the clippy `unused_enumerate_index` lint to move away from type paths to using diagnostic items.

see: rust-lang/rust-clippy#5393
fmease added a commit to fmease/rust that referenced this issue Apr 24, 2024
@fmease
Rollup merge of rust-lang#124308 - CBSpeir:diagnostic-item-enumerate,…
eaeaeb9 
… r=compiler-errors

Add diagnostic item for `std::iter::Enumerate`

This adds a diagnostic item for `std::iter::Enumerate`.  The change will be used by the clippy `unused_enumerate_index` lint to move away from type paths to using diagnostic items.

see: rust-lang/rust-clippy#5393
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Apr 24, 2024
@rust-timer
Unrolled build for rust-lang#124308
1e4f7d7 
Rollup merge of rust-lang#124308 - CBSpeir:diagnostic-item-enumerate, r=compiler-errors

Add diagnostic item for `std::iter::Enumerate`

This adds a diagnostic item for `std::iter::Enumerate`.  The change will be used by the clippy `unused_enumerate_index` lint to move away from type paths to using diagnostic items.

see: rust-lang/rust-clippy#5393
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Apr 25, 2024
@fmease
Rollup merge of #124308 - CBSpeir:diagnostic-item-enumerate, r=compil…
f0a4fd8 
…er-errors

Add diagnostic item for `std::iter::Enumerate`

This adds a diagnostic item for `std::iter::Enumerate`.  The change will be used by the clippy `unused_enumerate_index` lint to move away from type paths to using diagnostic items.

see: rust-lang/rust-clippy#5393
This was referenced Apr 29, 2024
Merged
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue May 1, 2024
@matthiaskrgr
Rollup merge of rust-lang#124542 - CBSpeir:diagnostic-item-enumerate-…
13cd193 
…method, r=scottmcm

Add diagnostic item for `std::iter::Iterator::enumerate`

Adds a diagnostic item for the `std::iter:Iterator::enumerate` trait method. This change, along with PR rust-lang#124308, will be used by the clippy `unused_enumerate_index` lint to move away from paths to using diagnostic items.

see: rust-lang/rust-clippy#5393
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue May 1, 2024
@matthiaskrgr
Rollup merge of rust-lang#124542 - CBSpeir:diagnostic-item-enumerate-…
38cbad9 
…method, r=scottmcm

Add diagnostic item for `std::iter::Iterator::enumerate`

Adds a diagnostic item for the `std::iter:Iterator::enumerate` trait method. This change, along with PR rust-lang#124308, will be used by the clippy `unused_enumerate_index` lint to move away from paths to using diagnostic items.

see: rust-lang/rust-clippy#5393
rust-timer added a commit to rust-lang-ci/rust that referenced this issue May 1, 2024
@rust-timer
Unrolled build for rust-lang#124542
21c0432 
Rollup merge of rust-lang#124542 - CBSpeir:diagnostic-item-enumerate-method, r=scottmcm

Add diagnostic item for `std::iter::Iterator::enumerate`

Adds a diagnostic item for the `std::iter:Iterator::enumerate` trait method. This change, along with PR rust-lang#124308, will be used by the clippy `unused_enumerate_index` lint to move away from paths to using diagnostic items.

see: rust-lang/rust-clippy#5393
bors added a commit that referenced this issue May 2, 2024
@bors
Auto merge of #12743 - CBSpeir:remove-paths-vec_resize, r=y21
c369183 
Remove `dead_code` paths

The following paths are `dead_code` and can be removed:

### `clippy_utils::paths::VEC_RESIZE`
* Introduced when `vec_resize_to_zero` lint added in PR #5637
* No longer used after commit 8acc4d2
### `clippy_utils::paths::SLICE_GET`
* Introduced when `get_first` lint added in PR #8882
* No longer used after commit a8d80d5
### `clippy_utils::paths::STR_BYTES`
* Introduced when `bytes_count_to_len` lint added in PR #8711
* No longer used after commit ba6a459

When the lints were moved into the `Methods` lint pass, they switched from using paths to diagnostic items.  However, the paths were never removed.  This occurred in PR #8957.

This relates to issue #5393

changelog: none
This was referenced Sep 18, 2024
Merged
Closed
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Sep 18, 2024
@workingjubilee
Rollup merge of rust-lang#130522 - GnomedDev:clippy-manual-retain-pat…
457c91c 
…hs, r=compiler-errors

[Clippy] Swap `manual_retain` to use diagnostic items instead of paths

Part of rust-lang/rust-clippy#5393, just a chore.
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Sep 18, 2024
@workingjubilee
Rollup merge of rust-lang#130522 - GnomedDev:clippy-manual-retain-pat…
4bd9de5 
…hs, r=compiler-errors

[Clippy] Swap `manual_retain` to use diagnostic items instead of paths

Part of rust-lang/rust-clippy#5393, just a chore.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 19, 2024
@rust-timer
Unrolled build for rust-lang#130522
6b0efb2 
Rollup merge of rust-lang#130522 - GnomedDev:clippy-manual-retain-paths, r=compiler-errors

[Clippy] Swap `manual_retain` to use diagnostic items instead of paths

Part of rust-lang/rust-clippy#5393, just a chore.
@GnomedDev GnomedDev mentioned this issue Sep 19, 2024
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 19, 2024
@matthiaskrgr
Rollup merge of rust-lang#130553 - GnomedDev:remove-clippy-paths, r=c…
569153a 
…ompiler-errors

[Clippy] Get rid of most `std` `match_def_path` usage, swap to diagnostic items.

Part of rust-lang/rust-clippy#5393.

This was going to remove all `std` paths, but `SeekFrom` has issues being cleanly replaced with a diagnostic item as the paths are for variants, which currently cannot be diagnostic items.

This also, as a last step, categories the paths to help with future path removals.
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 19, 2024
@rust-timer
Unrolled build for rust-lang#130553
0b28e76 
Rollup merge of rust-lang#130553 - GnomedDev:remove-clippy-paths, r=compiler-errors

[Clippy] Get rid of most `std` `match_def_path` usage, swap to diagnostic items.

Part of rust-lang/rust-clippy#5393.

This was going to remove all `std` paths, but `SeekFrom` has issues being cleanly replaced with a diagnostic item as the paths are for variants, which currently cannot be diagnostic items.

This also, as a last step, categories the paths to help with future path removals.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-an-interesting-project Category: Interesting projects, that usually are more involved design/code wise. C-tracking-issue Category: Tracking Issue E-medium Call for participation: Medium difficulty level problem and requires some initial experience.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@Manishearth @flip1995 @tesuji @xFrednet