[fix] [`redundant_closure_for_method_calls`] Suggest relative paths for local modules #11370

modelflat · 2023-08-21T13:31:55Z

Currently, redundant_closure_for_method_calls suggest incorrect paths when a method defined on a struct within inline mod is referenced (see the description in the aforementioned issue for an example; also see this playground link for the current-version output for the test cases added in this PR). It will now try to construct a relative path path to the module and suggest it instead.

changelog: [redundant_closure_for_method_calls] Fix incorrect path suggestions for types within local modules

rustbot · 2023-08-21T13:32:00Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Centri3 (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

Centri3 · 2023-08-21T14:27:33Z

Will review later, but can you extract get_ufcs_type_name to clippy_utils? There are quite a few lints that can benefit from this.

modelflat · 2023-08-22T12:29:56Z

@Centri3 Thanks for the suggestion! I moved get_ucfs_type_name to lib.rs in the utils. Considering the size of lib.rs I'd probably prefer to put it into a separate module, but I'm not sure which one is the most suitable... so I guess lib.rs it is 🙂

Centri3 · 2023-08-22T14:17:09Z

clippy_utils/src/lib.rs

+            // mod_a::sym  ::     ::
+            // (looking at mod_c)
+            EitherOrBoth::Right(r) => {
+                if let DefPathData::TypeNs(_) = r.data {


This would probably be better as a guard instead (same with the above)

Centri3 · 2023-08-22T14:25:16Z

clippy_utils/src/lib.rs

+                }
+                // Otherwise ignore
+            },
+            EitherOrBoth::Both(l, r) => match (l.data, r.data) {


afaict, this can be simplified by pushing l and r if they're TypeNs.

clippy_utils/src/lib.rs

Centri3 · 2023-08-22T14:33:33Z

clippy_utils/src/lib.rs

+                | rustc_ty::Dynamic(..)
+                | rustc_ty::Never
+                | rustc_ty::RawPtr(_)
+                | rustc_ty::Ref(..)
+                | rustc_ty::Slice(_)
+                | rustc_ty::Tuple(_) => format!("<{}>", EarlyBinder::bind(ty).instantiate(tcx, args)),


We need to handle these too, I'm pretty sure. This should recursively go through each nested type and get the relative path for each ADT.

Not an issue for the lint in question, but if somebody uses this on something like (super::A, super::super::B) it'll still be broken afaik.

bors · 2023-08-24T19:29:46Z

☔ The latest upstream changes (presumably #11398) made this pull request unmergeable. Please resolve the merge conflicts.

blyxyas · 2023-10-26T16:05:26Z

Seeing as Catherine is pretty busy (judging from her GitHub contribution graph) so I'll take from now on some of her reviews.

r? @blyxyas

blyxyas

Very good first iteration.

blyxyas · 2023-10-26T17:37:41Z

clippy_utils/src/lib.rs

+///
+/// Returned path can be either absolute (for methods defined non-locally), or relative (for local
+/// methods).
+pub fn get_ufcs_type_name<'tcx>(


This could have a much more descriptive name.

Suggested change

pub fn get_ufcs_type_name<'tcx>(

pub fn path_from_caller_to_method_type<'tcx>(

Or something like that.

blyxyas · 2023-10-28T11:59:02Z

tests/ui/eta.rs

@@ -399,3 +399,56 @@ fn _closure_with_types() {
    let _ = f2(|x: u32| f(x));
    let _ = f2(|x| -> u32 { f(x) });
 }
+
+// https://github.com/rust-lang/rust-clippy/issues/10854
+mod redundant_closure_for_method_calls_resolve_suggested_paths_to_relative {


Instead of having a module name so long, I think it would be best to name it issue_10854 and explain in a doc comment what it is supposed to test .

blyxyas · 2023-10-28T12:21:31Z

clippy_utils/src/lib.rs

+    }
+
+    repeat(String::from("super")).take(go_up_by).chain(path).join("::")


Seeing the result, I think that we should maybe cap go_up_by at like 2, AFAIK people prefer having crate::a::b than super::super::super::super::b in their codebase.

I really like this idea, implemented!

blyxyas · 2023-11-28T10:24:05Z

@modelflat Are you there?

modelflat · 2023-11-29T17:00:40Z

@blyxyas yep, hopefully I'll get some time to work on it in December

blyxyas · 2024-01-26T12:07:31Z

@modelflat Would you like for other person to finish this contribution?

modelflat · 2024-01-27T20:45:14Z

@blyxyas Hi! Sorry I completely forgot about this one. I've got some spare time now so can work on it. Pls take a look at the newest changes when you have time, I resolved conflicts and addressed your comments

blyxyas

LGTM! Could you squash these commits? ฅ*•ω•*ฅ♡

tests/ui/eta.rs

Fixes rust-lang#10854. Co-authored-by: Alejandra González <blyxyas@gmail.com>

blyxyas · 2024-01-29T11:40:57Z

@bors r+

bors · 2024-01-29T11:40:59Z

📌 Commit b3d5377 has been approved by blyxyas

It is now in the queue for this repository.

bors · 2024-01-29T11:42:06Z

⌛ Testing commit b3d5377 with merge 85d8082...

…-for-method-calls, r=blyxyas [fix] [`redundant_closure_for_method_calls`] Suggest relative paths for local modules Fixes #10854. Currently, `redundant_closure_for_method_calls` suggest incorrect paths when a method defined on a struct within inline mod is referenced (see the description in the aforementioned issue for an example; also see [this playground link](https://play.rust-lang.org/?version=stable&mode=release&edition=2021&gist=f7d3c5b2663c9bd3ab7abdb0bd38ee43) for the current-version output for the test cases added in this PR). It will now try to construct a relative path path to the module and suggest it instead. changelog: [`redundant_closure_for_method_calls`] Fix incorrect path suggestions for types within local modules

bors · 2024-01-29T11:48:32Z

💔 Test failed - checks-action_test

blyxyas · 2024-01-29T13:10:34Z

@bors retry

…-for-method-calls, r=blyxyas [fix] [`redundant_closure_for_method_calls`] Suggest relative paths for local modules Fixes #10854. Currently, `redundant_closure_for_method_calls` suggest incorrect paths when a method defined on a struct within inline mod is referenced (see the description in the aforementioned issue for an example; also see [this playground link](https://play.rust-lang.org/?version=stable&mode=release&edition=2021&gist=f7d3c5b2663c9bd3ab7abdb0bd38ee43) for the current-version output for the test cases added in this PR). It will now try to construct a relative path path to the module and suggest it instead. changelog: [`redundant_closure_for_method_calls`] Fix incorrect path suggestions for types within local modules

bors · 2024-01-29T13:11:41Z

⌛ Testing commit b3d5377 with merge 813d672...

bors · 2024-01-29T14:41:41Z

💥 Test timed out

blyxyas · 2024-01-29T14:52:50Z

@bors retry

bors · 2024-01-29T14:53:57Z

⌛ Testing commit b3d5377 with merge 3cd713a...

bors · 2024-01-29T15:06:52Z

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: blyxyas
Pushing 3cd713a to master...

bors · 2024-01-29T15:06:52Z

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: blyxyas
Pushing 3cd713a to master...

bors · 2024-01-29T15:07:51Z

👀 Test was successful, but fast-forwarding failed: 422 Changes must be made through a pull request.

rustbot assigned Centri3 Aug 21, 2023

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 21, 2023

Centri3 reviewed Aug 22, 2023

View reviewed changes

rustbot assigned blyxyas and unassigned Centri3 Oct 26, 2023

blyxyas reviewed Oct 28, 2023

View reviewed changes

blyxyas added the hacktoberfest-accepted For PRs that were accepted for hacktoberfest (and will be merged soon) label Oct 30, 2023

modelflat force-pushed the suggest-relpath-in-redundant-closure-for-method-calls branch from 0079622 to 7105961 Compare January 27, 2024 20:44

blyxyas removed the hacktoberfest-accepted For PRs that were accepted for hacktoberfest (and will be merged soon) label Jan 29, 2024

blyxyas approved these changes Jan 29, 2024

View reviewed changes

blyxyas reviewed Jan 29, 2024

View reviewed changes

tests/ui/eta.rs Outdated Show resolved Hide resolved

modelflat force-pushed the suggest-relpath-in-redundant-closure-for-method-calls branch from dc4146e to 0b7dd75 Compare January 29, 2024 11:32

Make redundant_closure_for_method_calls suggest relative paths

b3d5377

Fixes rust-lang#10854. Co-authored-by: Alejandra González <blyxyas@gmail.com>

modelflat force-pushed the suggest-relpath-in-redundant-closure-for-method-calls branch from 0b7dd75 to b3d5377 Compare January 29, 2024 11:35

bors merged commit 3cd713a into rust-lang:master Jan 29, 2024
5 checks passed

modelflat deleted the suggest-relpath-in-redundant-closure-for-method-calls branch January 29, 2024 17:57

Centri3 mentioned this pull request Feb 24, 2024

Add missing_transmute_annotations lint #12239

Merged

modelflat mentioned this pull request Mar 26, 2024

Cargo clippy --fix introduces compilation errors on rustc 1.79.0-nightly #12565

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[fix] [`redundant_closure_for_method_calls`] Suggest relative paths for local modules #11370

[fix] [`redundant_closure_for_method_calls`] Suggest relative paths for local modules #11370

modelflat commented Aug 21, 2023

rustbot commented Aug 21, 2023

Centri3 commented Aug 21, 2023

modelflat commented Aug 22, 2023

Centri3 Aug 22, 2023

Centri3 Aug 22, 2023

Centri3 Aug 22, 2023 •

edited

bors commented Aug 24, 2023

blyxyas commented Oct 26, 2023

blyxyas left a comment

blyxyas Oct 26, 2023

blyxyas Oct 28, 2023

blyxyas Oct 28, 2023

modelflat Jan 27, 2024

blyxyas commented Nov 28, 2023

modelflat commented Nov 29, 2023

blyxyas commented Jan 26, 2024

modelflat commented Jan 27, 2024

blyxyas left a comment

blyxyas commented Jan 29, 2024

bors commented Jan 29, 2024

bors commented Jan 29, 2024

bors commented Jan 29, 2024

blyxyas commented Jan 29, 2024

bors commented Jan 29, 2024

bors commented Jan 29, 2024

blyxyas commented Jan 29, 2024

bors commented Jan 29, 2024

bors commented Jan 29, 2024

bors commented Jan 29, 2024

bors commented Jan 29, 2024

	pub fn get_ufcs_type_name<'tcx>(
	pub fn path_from_caller_to_method_type<'tcx>(

		}

		repeat(String::from("super")).take(go_up_by).chain(path).join("::")

[fix] [redundant_closure_for_method_calls] Suggest relative paths for local modules #11370

[fix] [redundant_closure_for_method_calls] Suggest relative paths for local modules #11370

Conversation

modelflat commented Aug 21, 2023

rustbot commented Aug 21, 2023

Centri3 commented Aug 21, 2023

modelflat commented Aug 22, 2023

Centri3 Aug 22, 2023

Choose a reason for hiding this comment

Centri3 Aug 22, 2023

Choose a reason for hiding this comment

Centri3 Aug 22, 2023 • edited

Choose a reason for hiding this comment

bors commented Aug 24, 2023

blyxyas commented Oct 26, 2023

blyxyas left a comment

Choose a reason for hiding this comment

blyxyas Oct 26, 2023

Choose a reason for hiding this comment

blyxyas Oct 28, 2023

Choose a reason for hiding this comment

blyxyas Oct 28, 2023

Choose a reason for hiding this comment

modelflat Jan 27, 2024

Choose a reason for hiding this comment

blyxyas commented Nov 28, 2023

modelflat commented Nov 29, 2023

blyxyas commented Jan 26, 2024

modelflat commented Jan 27, 2024

blyxyas left a comment

Choose a reason for hiding this comment

blyxyas commented Jan 29, 2024

bors commented Jan 29, 2024

bors commented Jan 29, 2024

bors commented Jan 29, 2024

blyxyas commented Jan 29, 2024

bors commented Jan 29, 2024

bors commented Jan 29, 2024

blyxyas commented Jan 29, 2024

bors commented Jan 29, 2024

bors commented Jan 29, 2024

bors commented Jan 29, 2024

bors commented Jan 29, 2024

[fix] [`redundant_closure_for_method_calls`] Suggest relative paths for local modules #11370

[fix] [`redundant_closure_for_method_calls`] Suggest relative paths for local modules #11370

Centri3 Aug 22, 2023 •

edited