-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Description
Code
use crate::*;
use std::alloc::*;
use std::any::*;
use std::array::*;
use std::fmt::*;
use std::fs::*;
use std::io::*;
use std::iter::*;
use std::num::*;
use std::ops::*;
use std::sync::atomic::*;
use std::sync::*;
Desired output
warning: it's time to stop
--> /my/rust/module/with/too/many/globs.rs:LL:CC
|
LL | use crate::*;
| ^^^^^^^^
= note: using items introduced by this glob becomes ambiguous if upstream adds something using that name
|
LL | use std::alloc::*;
| ^^^^^^^^^^^^^
= note: globbing from your own crate and then from an upstream crate is a bad idea!
= help: actually name the items you want to use? please?
except maybe less sassy.
Rationale and extra context
If you glob import from an upstream crate once, it's okay. If you glob import from multiple local modules, that's probably okay if they all have non-glob imports. If you glob import from different modules in an upstream crate multiple times, or an upstream crate and then a local crate, you are running a very high risk of breaking yourself.
This is basically "how could we have avoided getting into this case?" for a regression on the current beta: #142427
I think that this is different from the clippy::wildcard_imports
lint. That clippy lint fires on basically any glob imports outside of a few special cases. Here we want to be smarter and identify when it poses a serious risk for upstream additions. In particular, we want to make it easier for std
to add new types and traits when it deems necessary without worrying too much about conflicts, so we could even choose to implement this lint only for globs from std
.
We might also have special cases (e.g. we may have to specially handle the preludes in some way) but we could allow basically any one glob always, and any combination of globs that have contents entirely subject to local control. This would let people actually use them where they are appropriate. There are also globs which are in-principle a closed set and we could give a pass, like enums (unless they are #[non_exhaustive]
), as globbing an enum is a common pattern.
The main risk, I think, aside from the usual social ones, would be that completely precise "are these transitive globs all local?" analysis may prove pretty challenging, so we may be better off accepting the occasional false positive. One saving grace is that the analysis only requires proving that we've globbed an upstream and a local crate, as we're not actually concerned whether the current compilation works out and are instead warning for semver reasons.
Rust Version
rustc 1.89.0-nightly (6ccd44760 2025-06-08)
binary: rustc
commit-hash: 6ccd4476036edfce364e6271f9e190ec7a2a1ff5
commit-date: 2025-06-08
host: x86_64-unknown-linux-gnu
release: 1.89.0-nightly
LLVM version: 20.1.5
Anything else?
No response
Activity
workingjubilee commentedon Jun 13, 2025
Note that I am deliberately ignoring that technically someone can shadow the implicit Rust prelude with their own Option, Result, Vec, etc. and break your code that way.
I assume that the mere act of having a glob-import from upstream is a willingness to subject yourself to deliberate horrors enacted by them.
hanna-kruppe commentedon Jun 13, 2025
I think a lint like this makes sense but given the many judgement calls involved (both in warning about this at all, and what exactly to warn about) and the very real possibility of false positives (even w.r.t. aforementioned judgement calls) I’d expect this to at least start out as clippy lint. It can still be uplifted later if experience suggests that’s desirable.
workingjubilee commentedon Jun 13, 2025
We have many lints that involve far more subjective and nitpicky judgement calls than anything in clippy, some of which have been in the language since before clippy even existed. I am proposing that rustc deliberately exercises the use of a lint that is functionally a future compatibility warning for all code that globs from std.
workingjubilee commentedon Jun 13, 2025
When I said that accepting the occasional false positive was acceptable, @hanna-kruppe, I said it because my fledgling understanding of the resolver is that we actually can get it precisely accurate, every time, but the cost may be egregious and negatively impact other aspects of the compiler. The analysis would only even start if the two-glob condition was met to begin with.
I understand the inclination to use clippy as a testbed for lints. I've even tittered about lints in rustc being unnecessarily opinionated and imprecise, like the ones in clippy often are. But the way that name resolution in this language works, this one is directly relevant to the health of developing the Rust standard library. And while it will involve judgement calls, because all code in the compiler does, it's really not about what code we would like to see. It's about discouraging behavior that will make it harder to add code to std without regressing half of crates.io.
And I unfortunately don't think that a clippy-initiated lint would ever get in good-enough condition to be uplifted. That's because I believe the primary heuristics involved in reducing the lint triggering on irrelevant cases will not be anywhere near precise enough to be useful to mitigate the impact if the change is not reviewed by people who work directly on the resolver.
hanna-kruppe commentedon Jun 13, 2025
It wasn’t clear to me from the initial description but it sounds like you want to define this lint to only trigger on glob imports from the standard library, not from other external crates? Framing it only as “future compatibility with standard library evolution” would be one way to make it less clippy-shaped and more obviously rustc’s domain. IMO that would be a shame because the semver hazard applies equally to other libraries, but if side-stepping tough questions like “what’s upstream for this purpose and what’s not” helps get this implemented sooner and with fewer problems, so be it.
workingjubilee commentedon Jun 14, 2025
Well, if there is consensus for a broader version, I think I would be happy to have that, but I'm also willing to haggle with butcher knife in hand to carve out parts that aren't strictly necessary to achieve the goal: make it so standard library additions don't break things as often, because people are aware when they run a risk, and will take steps to avoid negative events.
traviscross commentedon Jun 14, 2025
cc @rust-lang/lang
traviscross commentedon Jun 14, 2025
One thing maybe worth mentioning here is the connection to:
(This isn't an argument against doing what's proposed here necessarily. It's just something we should discuss.)
hanna-kruppe commentedon Jun 14, 2025
I haven’t followed the discussion there on the general problem, but FWIW in the specific case of serde_derive, you don’t need glob imports. Importing the traits as
serde::ser::Serialize
andserde::de::Deserialize
is sufficient since the derives are only re-exported from the crate root, not from those submodules.tgross35 commentedon Jun 17, 2025
Fyi I opened a clippy issue for this a while back rust-lang/rust-clippy#13961 and there is an open PR rust-lang/rust-clippy#14868.
I do think we should consider uplifting after that lands.
workingjubilee commentedon Jun 18, 2025
That is still not quite what I have in mind, @tgross35. I have in mind something that specifically detects the ambiguous case here, the overuse of glob imports:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=e7a0fef9d3c89d0ed5fa9196edbe5576
But thank you, that does reveal that libs has twice now requested this or similar lints:
Sign
is ambiguous #135672 (comment)proc_macro_totokens
#134707 (comment)However I am having doubts, after seeing the PR in progress, that the current PR will land as intended at the by-default lint level intended: rust-lang/rust-clippy#14868 (comment)
As does apparently the rest of the team, I believe this lint must be builtin to rustc. But I also believe strongly it must be significantly more conservative and precise than can be implemented by any passerby contributor without significant mentorship. I want to catch people we are almost certain are running a heavy risk and tell them that this doom has come for others, and it may come for them, too, someday.
std_wildcard_imports
rust-lang/rust-clippy#14868jieyouxu commentedon Jun 18, 2025
I'm very much in favor of linting against glob imports of std modules in the sense that we quite frequently discover resolve ambiguity regressions due to addition of even unstable standard library APIs. Immediate "refinements" I can think of (some of which are mentioned in rust-lang/rust-clippy#13961):
workingjubilee commentedon Jun 18, 2025
I think it's important that this land as warn-by-default or with the intent of rapidly promoting it to such if possible, even if it has to be absurdly conservative and cut-back to do so.
traviscross commentedon Jun 19, 2025
We discussed this today in lang triage. Thanks to @workingjubilee for joining us for that.
We were all sympathetic to the problem here and interested in seeing some solution happen. As @workingjubilee described for us, whatever lint we land on here is likely to have certain carve-outs to get the noise down to an acceptable level. The details are probably going to end up mattering, and we're interested in seeing this experimentation happen and for @workingjubilee to come back to us with a proposal and findings and analysis from a crater run.
One possible approach mentioned in the call was to add a way to flag, with attributes, whether or not a module is intended to be used with glob imports. Other approaches include various heuristics the lint could apply.