Skip to content

FCW Lint when using an ambiguously glob imported trait #149058

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
Open
LorrensP-2158466 wants to merge 8 commits into
base: main
Choose a base branch
from
Open

FCW Lint when using an ambiguously glob imported trait #149058

LorrensP-2158466 wants to merge 8 commits into from
+446 −20

Conversation

@LorrensP-2158466
Copy link
Contributor

@LorrensP-2158466 LorrensP-2158466 commented Nov 18, 2025 •
edited by rustbot
Loading

Related to #147992.

Report a lint when using an ambiguously glob import trait, this is a FCW because this should not be allowed.

r? @petrochenkov

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 18, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@petrochenkov petrochenkov mentioned this pull request Nov 18, 2025
Closed
petrochenkov
petrochenkov reviewed Nov 18, 2025
View reviewed changes
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 18, 2025
petrochenkov
petrochenkov reviewed Nov 19, 2025
View reviewed changes
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 19, 2025
@LorrensP-2158466 LorrensP-2158466 changed the title resolve: Consider all traits ambiguously glob imported to be in scope Lint when using an ambiguously glob imported trait Nov 19, 2025
@LorrensP-2158466 LorrensP-2158466 changed the title Lint when using an ambiguously glob imported trait FCW Lint when using an ambiguously glob imported trait Nov 19, 2025
@LorrensP-2158466
Copy link
Contributor Author

LorrensP-2158466 commented Nov 19, 2025

Addressed your comments and updated pr title and description.

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 19, 2025
LorrensP-2158466
LorrensP-2158466 commented Nov 19, 2025
View reviewed changes
compiler/rustc_hir_typeck/src/method/confirm.rs Outdated
Comment on lines 735 to 740
self.tcx.node_lint(AMBIGUOUS_TRAIT_GLOB_IMPORTS, segment.hir_id, |diag| {
diag.primary_message(format!("Use of ambiguously glob imported trait `{trait_name}`"))
.span(segment.ident.span)
.span_label(import_span, format!("`{trait_name}`imported ambiguously here"))
.help(format!("Import `{trait_name}` explicitly"));
});
Copy link
Contributor Author

@LorrensP-2158466 LorrensP-2158466 Nov 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to add some kind of suggestion like:

Consider importing `{trait_name}` directly:
+ use m1::{trait_name};

petrochenkov
petrochenkov reviewed Nov 20, 2025
View reviewed changes
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 20, 2025
This was referenced Nov 20, 2025
Closed
Closed
@rustbot rustbot assigned lcnr and unassigned petrochenkov Nov 20, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@LorrensP-2158466
Copy link
Contributor Author

LorrensP-2158466 commented Nov 20, 2025

What am I doing 🤦 🤦 🤦

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@petrochenkov petrochenkov mentioned this pull request Dec 3, 2025
Open
@petrochenkov petrochenkov self-assigned this Dec 3, 2025
@petrochenkov petrochenkov added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 3, 2025
@petrochenkov
Copy link
Contributor

petrochenkov commented Dec 3, 2025

The implementation looks good to me, so I'll just start crater.
@bors try

rust-bors bot added a commit that referenced this pull request Dec 3, 2025
@rust-bors
Auto merge of #149058 - LorrensP-2158466:amb-trait-lint, r=<try>
47eaa3b 
FCW Lint when using an ambiguously glob imported trait
@rust-bors

This comment has been minimized.

Sign in to view
@rust-bors
Copy link

rust-bors bot commented Dec 3, 2025

☀️ Try build successful (CI)
Build commit: 47eaa3b (47eaa3b85fbe5fa2b3b731b5fecdc5954cbfc268, parent: 568b11762723b001bfa693d0f21c5dad01d4e813)

@petrochenkov
Copy link
Contributor

petrochenkov commented Dec 3, 2025

@craterbot check

@craterbot
Copy link
Collaborator

craterbot commented Dec 3, 2025

👌 Experiment pr-149058 created and queued.
🤖 Automatically detected try build 47eaa3b
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@bors
Copy link
Collaborator

bors commented Dec 3, 2025

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

@LorrensP-2158466
Copy link
Contributor Author

LorrensP-2158466 commented Dec 3, 2025

Will crater fail because of this merge conflict?

@petrochenkov
Copy link
Contributor

petrochenkov commented Dec 3, 2025

No

@petrochenkov petrochenkov mentioned this pull request Dec 6, 2025
Merged
@craterbot
Copy link
Collaborator

craterbot commented Dec 11, 2025

🚧 Experiment pr-149058 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

craterbot commented Dec 13, 2025

🎉 Experiment pr-149058 is completed!
📊 5 regressed and 5 fixed (751500 total)
📊 2125 spurious results on the retry-regessed-list.txt, consider a retry1 if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Footnotes

  1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-149058/retry-regressed-list.txt ↩

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Dec 13, 2025
@petrochenkov
Copy link
Contributor

petrochenkov commented Dec 13, 2025

Sigh, I didn't notice that the lint is not set to deny-by-default, so we don't see how often it was hit from the regression list.

The crater run is not completely wasted though, could you download all the crater logs, including the passing ones, and grep them for ambiguous_glob_imported_trait manually?
And also debug the 5 regressed cases from the report above.
@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 13, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 13, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@petrochenkov petrochenkov petrochenkov left review comments

Assignees

@petrochenkov petrochenkov

@lcnr lcnr

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@LorrensP-2158466 @rust-log-analyzer @rustbot @petrochenkov @craterbot @bors @lcnr