Skip to content
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

Merged
merged 2 commits into from
Nov 24, 2019
Merged

Rework raw ident suggestions #66592

merged 2 commits into from
Nov 24, 2019

Conversation

estebank
Copy link
Contributor

Use heuristics to determine whethersuggesting raw identifiers is
appropriate.

Account for raw identifiers when printing a path in a use suggestion.

Fix #66126.

@rust-highfive
Copy link
Collaborator

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 20, 2019
@rust-highfive

This comment has been minimized.

@rust-highfive

This comment has been minimized.

TokenKind::Comma,
TokenKind::Semi,
TokenKind::ModSep,
TokenKind::OpenDelim(token::DelimToken::Brace),
Copy link
Member

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 { () => {} }

Copy link
Member

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 { ... })

@cramertj
Copy link
Member

r=me with travis fixed

@petrochenkov petrochenkov self-assigned this Nov 21, 2019
TokenKind::OpenDelim(token::DelimToken::Paren),
TokenKind::CloseDelim(token::DelimToken::Brace),
TokenKind::CloseDelim(token::DelimToken::Paren),
];
Copy link
Contributor

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.

Copy link
Contributor Author

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.

@petrochenkov petrochenkov removed their assignment Nov 22, 2019
Use heuristics to determine whethersuggesting raw identifiers is
appropriate.

Account for raw identifiers when printing a path in a `use` suggestion.
@estebank
Copy link
Contributor Author

@bors r=cramertj as per #66592 (comment)

@bors
Copy link
Contributor

bors commented Nov 24, 2019

📌 Commit 1803886 has been approved by cramertj

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 24, 2019
@bors
Copy link
Contributor

bors commented Nov 24, 2019

⌛ Testing commit 1803886 with merge 5a1d028...

bors added a commit that referenced this pull request Nov 24, 2019
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.
@bors
Copy link
Contributor

bors commented Nov 24, 2019

☀️ Test successful - checks-azure
Approved by: cramertj
Pushing 5a1d028 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 24, 2019
@bors bors merged commit 1803886 into rust-lang:master Nov 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Raw ident suggestion should only trigger if the code is otherwise well-formed
5 participants