-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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.
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
Expand internal lint unnecessary_def_path
#9566
Conversation
r? @dswij (rust-highfive has picked a reviewer for you, use r? to override) |
6d30b1d
to
69fc1ab
Compare
There is a lint for this, it's just currently broken. There is a pr to fix said lint (#7962). Splitting the file up is definitely worth it. |
Perhaps there is room for both approaches? If I understand correctly, I would expect cases like this to be hard for the former, but catchable by the latter: https://github.com/rust-lang/rust-clippy/pull/9566/files#diff-99cb6bebcf4ed3a60e0e046a65b7a39631a9ce18589a91e9e668c0e0fd6cd9abL102-L105 That said, it would certainly be better that the latter not fire when the former does. So perhaps we could push #7962 through and then look to merge them? What is needed to push #7962 through? |
Ahh, that is a little different. I think it would be better to be part of |
☔ The latest upstream changes (presumably #9571) made this pull request unmergeable. Please resolve the merge conflicts. |
7e413ae
to
7d31846
Compare
diagnostic_item_path
unnecessary_def_path
6e071bf
to
bb72e43
Compare
I think this is ready for review, @dswij (cc: @Jarcho). Instead of adding a new lint, the changes now expand The last commit is just formatting, and could be omitted. |
☔ The latest upstream changes (presumably #9629) made this pull request unmergeable. Please resolve the merge conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for this! Can you help to rebase? I think this is good to merge afterward.
bb72e43
to
5dc54c6
Compare
The best way I know to verify that a rebase was done correctly is to diff the old patch against the new. So here is a diff of patches 2c8e473:bb72e43 and 50f192f:5dc54c6 (the interesting lines are the 217c217
< index 05d927dbe..19935bd4d 100644
---
> index de1253c85..5609a4dc9 100644
275c275
< index 2dcefd787..da33ad299 100644
---
> index ebb0f14fe..893410dbf 100644
278c278
< @@ -529,17 +529,23 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
---
> @@ -528,17 +528,23 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
503c503
< index cfcf9596c..265ac7bba 100644
---
> index 78c1b33ed..fb92779be 100644
560,563c560,562
< - let trait_def_id = match get_trait_def_id(cx, trait_path) {
< + let trait_def_id = match cx.tcx.get_diagnostic_item(trait_sym) {
< Some(did) => did,
< None => return false,
---
> - let Some(trait_def_id) = get_trait_def_id(cx, trait_path) else {
> + let Some(trait_def_id) = cx.tcx.get_diagnostic_item(trait_sym) else {
> return false
564a564
> implements_trait(cx, ty, trait_def_id, &[parent_ty.into()])
802c802
< index 85bcbc7ad..71f6c9909 100644
---
> index 0d908bf2a..71f6c9909 100644
805c805
< @@ -1,1566 +1,12 @@
---
> @@ -1,1560 +1,12 @@
2217,2220c2217
< - let then_block = match then.kind {
< - ExprKind::Block(block, _) => block,
< - _ => return,
< - };
---
> - let ExprKind::Block(then_block, _) = then.kind else { return };
2225,2228c2222
< - let if_chain_span = match if_chain_span {
< - None => return,
< - Some(span) => span,
< - };
---
> - let Some(if_chain_span) = if_chain_span else { return };
2774c2768
< index 000000000..81cfd1f7e
---
> index 000000000..883a5c08e
2777c2771
< @@ -0,0 +1,170 @@
---
> @@ -0,0 +1,164 @@
2826,2829c2820
< + let then_block = match then.kind {
< + ExprKind::Block(block, _) => block,
< + _ => return,
< + };
---
> + let ExprKind::Block(then_block, _) = then.kind else { return };
2834,2837c2825
< + let if_chain_span = match if_chain_span {
< + None => return,
< + Some(span) => span,
< + };
---
> + let Some(if_chain_span) = if_chain_span else { return };
4211c4199
< index e6492d762..f9a9c89de 100644
---
> index dbe75b43c..cbc2fde69 100644
4214c4202
< @@ -1763,6 +1763,7 @@ pub fn any_parent_is_automatically_derived(tcx: TyCtxt<'_>, node: HirId) -> bool
---
> @@ -1766,6 +1766,7 @@ pub fn any_parent_is_automatically_derived(tcx: TyCtxt<'_>, node: HirId) -> bool
4222c4210
< @@ -1780,6 +1781,22 @@ pub fn match_function_call<'tcx>(
---
> @@ -1783,6 +1784,22 @@ pub fn match_function_call<'tcx>(
4246c4234
< index 13938645f..f54d7e220 100644
---
> index 78adc4536..bc8514734 100644
4329c4317,4318
< @@ -131,14 +107,8 @@ pub const REGEX_BYTES_SET_NEW: [&str; 5] = ["regex", "re_set", "bytes", "RegexSe
---
> @@ -125,14 +101,8 @@ pub const REGEX_BYTES_NEW: [&str; 4] = ["regex", "re_bytes", "Regex", "new"];
> pub const REGEX_BYTES_SET_NEW: [&str; 5] = ["regex", "re_set", "bytes", "RegexSet", "new"];
4331d4319
< #[expect(clippy::invalid_paths)] // internal lints do not know about all external crates The changes are to account for these: Thank you very much for the review. |
Thank you for the changes and the cleanup! @bors r+ |
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
同步社区最新master分支代码 Created-by: lizheng 30020856 Author-id: 11958 MR-id: 457385 Commit-by: bors;Michael Krasnitski;Mara Bos;KaDiWa;Alex Macleod;Manish Goregaokar;Tyler Weaver;Niki4tap;koka;Sylvain Desodt;ksaleem;Philipp Krones;Collin Styles;xFrednet;Martin Fischer;Evan Typanski;Vincenzo Palazzo;Samuel Moelius;chansuke;Michael Woerister;Kyle Matsuda;Maria José Solano;Michael Goulet;Yuki Okushi;Jason Newcomb;Joshua Nelson;Oli Scherer;Robert Bastian;navh;asquared31415;Andre Bogus;Albert Larsan;dswij;Raiki Tamura;Caio;blyxyas;Robin Schroer;Kyle Huey;Eric Wu;Nilstrieb;Andy Russell;Max Baumann;Ardis Lu;Trevor Gross;Ray Redondo;Matthias Krüger;Esteban Küber;Lukas Lueg;alexey semenyuk;dboso;Samuel Tardieu;feniljain;Gary Guo;Nicholas Nethercote;Fridtjof Stoldt;naosense;alex-semenyuk;Taiki Endo;Hannah Town;Jakob Degen;Lukas Wirth;Vadim Petrochenkov;mdgaziur;Eric Huss;Yuri Astrakhan;kraktus;Ralf Jung;Santiago Pastorino;Camille GILLOT;hkalbasi;Arpad Borsos;Maybe Waffle;ozkanonur;Sosthène Guédon;Deadbeef;Kartavya Vashishtha;Aphek;Nadir Fejzic;Lukas Markeffsky;hrxi;clubby789;yukang;Ryan Scheidter;Grachev Mikhail;Elliot Bobrow;Dylan DPC;Steven Casper;bebecue;Trevor Arjeski;Onur Özkan;Cameron Steffen;Guillaume Gomez;Matthew Ingwersen;Alex ✨ Cosmic Princess ✨;dswijj;mejrs;Rageking8;Alex;est31;oxalica;JT;Doru-Florin Blanzeanu;Andreu Botella;royrustdev;flip1995;lcnr;Kevin Per;Josh Stone;TennyZhuang;Marijn Schouten;Steven Nguyen;Cody;Urgau;ouz-a;Nahua Kang;Felix Kohlgrüber Merged-by: wangqilin 00349210 E2E-issues: Description: add syntax-tree-patterns RFC, Remove if_chain from equatable_if_let, Lint suggests matches macro if PartialEq trait is not implemented, Run cargo dev bless to update fixes & stderr, Merge commit 'ac0e10aa68325235069a842f47499852b2dee79e' into clippyup, Remove `mir::CastKind::Misc`, Merge commit '8f1ebdd18bdecc621f16baaf779898cc08cc2766' into clippyup, Introduce TypeErrCtxt TypeErrCtxt optionally has a TypeckResults so that InferCtxt doesn't need to., Change InferCtxtBuilder from enter to build, make const_err a hard error, Auto merge of #102091 - RalfJung:const_err, r=oli-obk make const_err a hard error This lint has been deny-by-default with future incompat wording since [Rust 1.51](rust-lang/rust#80394) and the stable release of this week starts showing it in cargo's future compat reports. I can't wait to finally get rid of at least some of the mess in our const-err-reporting-code. ;) r? `@oli-obk` Fixes rust-lang/rust#71800 Fixes rust-lang/rust#100114, Auto merge of rust-lang#2583 - RalfJung:rustup, r=oli-obk initial josh subtree sync This demonstrates what a josh-based rustup would look like with my patched josh. To create it I did ``` git fetch http://localhost:8000/rust-lang/rust.git:start=75dd959a3a40eb5b4574f8d2e23aa6efbeb33573[:prefix=src/tools/miri]:/src/tools/miri.git master git merge FETCH_HEAD ./rustup-toolchain HEAD && ./miri fmt git commit -am rustup ``` Unlike the [previous attempt](rust-lang/miri#2554), this does not add a new root commit to the repo. Once we merge this, we committed to using josh for subtree syncing, and in particular a version of josh that includes josh-project/josh#961 (or something compatible)., Stabilize half_open_range_patterns, Rollup merge of #102675 - ouz-a:mir-technical-debt, r=oli-obk Remove `mir::CastKind::Misc` As discussed in #97649 `mir::CastKind::Misc` is not clear, this PR addresses that by creating a new enum variant for every valid cast. r? ````@oli-obk````, [`unnecessary_cast`] Do not lint negative hexadecimal literals when cast as float Floats cannot be expressed as hexadecimal literals, ImplItemKind::TyAlias => ImplItemKind::Type, merge rustc history, Fix clippy tests that trigger `for_loop_over_fallibles` lint, fixup lint name, deprecate `clippy::for_loops_over_fallibles`, Rollup merge of #102829 - compiler-errors:rename-impl-item-kind, r=TaKO8Ki rename `ImplItemKind::TyAlias` to `ImplItemKind::Type` The naming of this variant seems inconsistent given that this is not really a "type alias", and the associated type variant for `TraitItemKind` is just called `Type`., Rollup merge of #102275 - Urgau:stabilize-half_open_range_patterns, r=cjgillot Stabilize `half_open_range_patterns` This PR stabilize `feature(half_open_range_patterns)`: ``` Allows using `..=X` as a pattern. ``` And adds a new `feature(half_open_range_patterns_in_slices)` for the slice part, rust-lang/rust#102275 (comment). The FCP was completed in rust-lang/rust#67264., Rename AssocItemKind::TyAlias to AssocItemKind::Type, Rollup merge of #99696 - WaffleLapkin:uplift, r=fee1-dead Uplift `clippy::for_loops_over_fallibles` lint into rustc This PR, as the title suggests, uplifts [`clippy::for_loops_over_fallibles`] lint into rustc. This lint warns for code like this: ```rust for _ in Some(1) {} for _ in Ok::<_, ()>(1) {} ``` i.e. directly iterating over `Option` and `Result` using `for` loop. There are a number of suggestions that this PR adds (on top of what clippy suggested): 1. If the argument (? is there a better name for that expression) of a `for` loop is a `.next()` call, then we can suggest removing it (or rather replacing with `.by_ref()` to allow iterator being used later) ```rust for _ in iter.next() {} // turns into for _ in iter.by_ref() {} ``` 2. (otherwise) We can suggest using `while let`, this is useful for non-iterator, iterator-like things like [async] channels ```rust for _ in rx.recv() {} // turns into while let Some(_) = rx.recv() {} ``` 3. If the argument type is `Result<impl IntoIterator, _>` and the body has a `Result<_, _>` type, we can suggest using `?` ```rust for _ in f() {} // turns into for _ in f()? {} ``` 4. To preserve the original behavior and clear intent, we can suggest using `if let` ```rust for _ in f() {} // turns into if let Some(_) = f() {} ``` (P.S. `Some` and `Ok` are interchangeable depending on the type) I still feel that the lint wording/look is somewhat off, so I'll be happy to hear suggestions (on how to improve suggestions :D)! Resolves #99272 [`clippy::for_loops_over_fallibles`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loops_over_fallibles, Rollup merge of #102868 - compiler-errors:rename-assoc-tyalias-to-ty, r=TaKO8Ki Rename `AssocItemKind::TyAlias` to `AssocItemKind::Type` Thanks `@camsteffen` for catching this in ast too, cc rust-lang/rust#102829 (comment), merge rustc history, Fix unclosed HTML tag in clippy doc, fix `box-default` ignoring trait objects' types, Fix allow_attributes_without_reason applying to external crate macros Previously the `clippy::allow_attributes_without_reason` lint would apply to external crate macros. Many macros in the Rust ecosystem include these `allow` attributes without adding a reason, making this lint pretty much unusable in any sizable Rust project. This commit fixes that by adding a check to the lint if the attribute is from an external crate macro and returning early., Fix bug in `referent_used_exactly_once`, merge rustc history, `default_numeric_fallback` do not lint on constants, refactor `default_numeric_fallback` We only need to store if the literal binding has an explicit type bound or not, Book: Small grammar + link a11y change, Remove CastCheckResult since it's unused, add missing comma, Auto merge of rust-lang#9644 - hkBst:patch-1, r=flip1995 add missing comma changelog: none, Auto merge of rust-lang#9643 - icecream17:patch-1, r=flip1995 Book: Small grammar + link a11y change *Please write a short comment explaining your change (or "none" for internal only changes)* changelog: none --- Very minor For the link accessibility change, `here` and related don't provide context for screen readers who are reading a list of links. (Random supporting google links) https://www.w3.org/QA/Tips/noClickHere https://usability.yale.edu/web-accessibility/articles/links, Don't lint `ptr_arg` when used as an incompatible trait object, Auto merge of rust-lang#9645 - Jarcho:ptr_arg_9542, r=llogiq Don't lint `ptr_arg` when used as an incompatible trait object fixes rust-lang#9542 changelog: [`ptr_arg`](https://rust-lang.github.io/rust-clippy/master/#ptr_arg): Don't lint when used as an incompatible trait object, Add a suggestion and a note about orphan rules for `from_over_into`, Auto merge of rust-lang#9649 - Alexendoo:from-over-into-suggestion, r=llogiq Add a suggestion and a note about orphan rules for `from_over_into` Adds a machine applicable suggestion to convert the `Into` impl into a `From` one to `from_over_into` Also adds a note explaining that `impl From<Local> for Foreign` is fine if the `Into` type is foreign Closes rust-lang#7444 Addresses half of rust-lang#9638 changelog: [`from_over_into`] Add a suggestion and a note about orphan rules, Separate internal lints by pass, Move some things around, Expand `unnecessary_def_path` lint, Fix adjacent code, Format affected files, `explicit_ty_bound` code golf, [`zero_prefixed_literal`] Do not advise to use octal form if not possible, Enable test no_std_main_recursion, fix `box-default` linting `no_std` non-boxes, Auto merge of rust-lang#9655 - llogiq:unbox-default, r=dswij fix `box-default` linting `no_std` non-boxes This fixes rust-lang#9653 by doing the check against the `Box` type correctly even if `Box` isn't there, as in `no_std` code. Thanks to `@lukas-code` for opening the issue and supplying a reproducer! --- changelog: none, Auto merge of rust-lang#9636 - kraktus:numeric-fallback, r=dswij [`default_numeric_fallback`] do not lint on constants fix rust-lang#9632 changelog:[`default_numeric_fallback`] do not lint on constants, Auto merge of rust-lang#9566 - smoelius:diagnostic-item-path, r=dswij Expand internal lint `unnecessary_def_path` This PR does essentially two things: * Separates the internal lints into modules by pass. (`internal_lints.rs` was over 1400 lines, which is a little unruly IMHO.) * ~Adds a new~ Expands the `unnecessary_def_path` internal lint to flag hardcoded paths to diagnostic and language items. My understanding is that the latter is currently done by reviewers. Automating this process should make things easier for both reviewers and contributors. I could make the first bullet a separate PR, or remove it entirely, if desired. changelog: Add internal lint `diagnostic_item_path`, Add new lint `partial_pub_fields` Signed-off-by: TennyZhuang <[email protected]>, fix dogfood Signed-off-by: TennyZhuang <[email protected]>, add many tests Signed-off-by: TennyZhuang <[email protected]>, fix a doctest Signed-off-by: TennyZhuang <[email protected]>, Auto merge of rust-lang#9658 - TennyZhuang:partial-pub-fields, r=llogiq Add new lint `partial_pub_fields` Signed-off-by: TennyZhuang <[email protected]> *Please write a short comment explaining your change (or "none" for internal only changes)* changelog: `partial_pub_fields`: new lint to disallow partial fields of a struct be pub Resolve rust-lang#9604, Auto merge of rust-lang#9652 - kraktus:octo_89, r=xFrednet [`zero_prefixed_literal`] Do not advise to use octal form if not possible fix rust-lang#9651 changelog: [`zero_prefixed_literal`] Do not advise to use octal form if not possible, Auto merge of rust-lang#9609 - kraktus:hexa_f32, r=giraffate [`unnecessary_cast`] Do not lint negative he See merge request innersource/rust/toolset/rust-clippy!8
This PR does essentially two things:
internal_lints.rs
was over 1400 lines, which is a little unruly IMHO.)Adds a newExpands theunnecessary_def_path
internal lint to flag hardcoded paths to diagnostic and language items.My understanding is that the latter is currently done by reviewers. Automating this process should make things easier for both reviewers and contributors.
I could make the first bullet a separate PR, or remove it entirely, if desired.
changelog: Add internal lint
diagnostic_item_path