-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Disable use $crate::{self}
like use $crate
#146972
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
base: master
Are you sure you want to change the base?
Conversation
Failed to set assignee to
|
compiler/rustc_resolve/messages.ftl
Outdated
resolve_unnamed_crate_root_import = | ||
crate root imports need to be explicitly named: `use crate as name;` | ||
{$dollar -> | ||
[true] crate root imports need to be explicitly named: `use $crate as name;` |
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.
I don't think this is a valid suggestion. Per this, use $crate as x;
is not allowed.
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.
Though I suppose it is a separate question if that restriction makes sense if there is an as
. Is there a reason to also prevent that?
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.
Ah yes! Updated, now will also emit $crate may not be imported
for use $crate::{self}
things
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.
The whole pre-existing logic here is garbage (#119776 (comment)), and this PR adds more.
The only restriction that we really need here is for imports to always end up with a name (*) - #35612 (comment).
use super; // bad
use super as name; // good
use super::{self}; // bad
use super::{self as name}; // good
use self; // bad
use self as name; // good
use crate; // bad
use crate as name; // good
use crate::{self}; // bad
use crate::{self as name}; // good
use $crate; // bad
use $crate as name; // good
use $crate::{self}; // bad
use $crate::{self as name}; // good
// etc
Many of these examples do not work correctly atm for various reasons.
(*) The name can even be underscore _
, but it will just always be reported as unused in this case.
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.
More than a partial fixup, I'd rather want to see an exhaustive test listing the possible cases of imports using only path segment keywords (self
, super
, crate
, $crate
) in the input path with FIXMEs as a declaration of intent.
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.
@mu001999
Could you add such a test?
Adding some partial fixup on top of that would also be fine, but ideally this stuff just needs to be rewritten.
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.
for the case use $crate as name; // good
in the above examples, @ehuss said in rust-lang/reference#2010 (comment):
but for some reason use $crate as foo doesn't work either.
And I'd like to rewrite the whole logic
The previous wording for this restriction was pretty confusing to me. I don't remember what I was thinking when I wrote it, and I can't find any historical explanation either. `use` paths can use `$crate` as long as they have more than one segment (`use $crate::foo` is obviously OK). I have rewritten this to make it clear it is specifically about `use $crate`. One could say that restriction is already covered by the previous point that says `use crate;` requires an `as`, but for some reason `use $crate as foo` doesn't work either. So I have left this as a separate rule for now. cc rust-lang/rust#146972 (comment) for context.
ce2578f
to
49c425d
Compare
use $crate::{self}
like use $crate
Fixes #146967
we have hadUnnamedCrateRootImport
for things likeuse crate::self
, reusing it for$crate
seems good enoughEmit error
CrateImported
when we meetuse $crate::{self}
r? petrochenkov