-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Rework raw ident suggestions #66592
Rework raw ident suggestions #66592
Conversation
r? @cramertj (rust_highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
3ef2b96
to
17ff1a7
Compare
This comment has been minimized.
This comment has been minimized.
TokenKind::Comma, | ||
TokenKind::Semi, | ||
TokenKind::ModSep, | ||
TokenKind::OpenDelim(token::DelimToken::Brace), |
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 think this should also include Bracket
-- e.g.:
macro_rules! async { () => {} }
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.
(but it is a heuristic, and as such it may be better not to suggest changing async { ... }
)
r=me with travis fixed |
TokenKind::OpenDelim(token::DelimToken::Paren), | ||
TokenKind::CloseDelim(token::DelimToken::Brace), | ||
TokenKind::CloseDelim(token::DelimToken::Paren), | ||
]; |
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.
Code like this is pure technical debt, and my preferred solution would be to remove the diagnostic (#66126 (comment)), especially given that raw identifiers are a compatibility feature that should never be recommended in general.
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 think that with this new restriction we'll only be presenting this suggestion in the very rare cases where the intent was there.
Use heuristics to determine whethersuggesting raw identifiers is appropriate. Account for raw identifiers when printing a path in a `use` suggestion.
17ff1a7
to
1803886
Compare
@bors r=cramertj as per #66592 (comment) |
📌 Commit 1803886 has been approved by |
Rework raw ident suggestions Use heuristics to determine whethersuggesting raw identifiers is appropriate. Account for raw identifiers when printing a path in a `use` suggestion. Fix #66126.
☀️ Test successful - checks-azure |
Use heuristics to determine whethersuggesting raw identifiers is
appropriate.
Account for raw identifiers when printing a path in a
use
suggestion.Fix #66126.