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

Write lint for usage of edition-gated keywords #49716

Closed
Manishearth opened this issue Apr 5, 2018 · 6 comments · Fixed by #52375
Closed

Write lint for usage of edition-gated keywords #49716

Manishearth opened this issue Apr 5, 2018 · 6 comments · Fixed by #52375
Assignees
Labels
A-edition-2018 Area: The 2018 edition A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-epoch Working group: Epoch (2018) management

Comments

@Manishearth
Copy link
Member

After #49611 lands, attempting to use async as an identifier will work if on the 2015 edition, but not on the 2018 edition.

So that editions can be rustfixed, we need the ability to lint this and suggest fixes.

@Manishearth Manishearth added E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. P-medium Medium priority WG-epoch Working group: Epoch (2018) management labels Apr 5, 2018
@Manishearth
Copy link
Member Author

This is trickier than usual. We want to run this lint for code actually specified in this crate -- ideally in the place where we lex the tokens. So code from a macro from a different crate should not trigger the lint, but code within a macro def/invocation from this crate should.

However, the lint levels aren't available to the lexer. We can persist this info till parsing and lint there, but the lint levels aren't available there either.

The node IDs used during parsing are dummy ids, so we cannot stash the lint info there and emit it lated.

The way I see it, we have the following options:

  • Emit lints during lexing, somehow figure out lint levels (afaict this may be fundamentally impossible)
  • Emit lints during parsing, somehow figure out lint levels (doable, but we're kinda reinventing the lint framework)
  • Emit lints as an extra-early lint pass running before macro expansion: By now the spans of the tokens within macro defs and invocations are missing, so this is hard to do well. We can still do it but the suggestions for macros won't work.
  • Emit lints as a regular early or late pass: Macro invocations are now missing completely.

thoughts? @nikomatsakis @oli-obk @eddyb

@oli-obk
Copy link
Contributor

oli-obk commented Apr 6, 2018

We could stash the lints with their span instead of an ID. This is doable in the lexer. Then, later, we can run a normal early lint pass which only does lint level checking and runs through the list of stashed lints and checks whether their span is a subset of any span it encounters.

Not sure whether those spans would still match after expansion, so maybe we need that pre expansion lint pass

@rcoh
Copy link
Contributor

rcoh commented Apr 17, 2018

Hey, I'd like to take a stab at this, if someone wouldn't mind pointing me in the right direction.

  • By "lint level" do you just mean what the current lint configuration is?

@oli-obk just so I understand what you're suggesting: We add the ability to persist partial-lint information in the tokens emitted by the lexer, then later pull the lint information back out again once we have actual ids for things?

@Manishearth
Copy link
Member Author

By "lint level" do you just mean what the current lint configuration is?

The lint level is whether or not a particular lint is allow/warn/deny. We'd be doing this for a single lint. We could keep track of info for all lints but that's not necessary.

Note that #49611 hasn't landed yet and @petrochenkov is redoing the approach, so there's a chance the approach for this issue won't work anymore. But I don't think that's going to be the case, so you can go ahead with writing the framework code without writing the lint itself.

@XAMPPRocky XAMPPRocky added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 29, 2018
@nrc nrc mentioned this issue Jul 4, 2018
15 tasks
@alexcrichton alexcrichton added this to the Rust 2018 Preview 2 milestone Jul 11, 2018
@Mark-Simulacrum Mark-Simulacrum added P-high High priority and removed P-medium Medium priority labels Jul 11, 2018
@pnkfelix
Copy link
Member

visiting for triage

assigning to @oli-obk to make sure that this gets mentoring instructions (or outright implemented), hopefully before Preview 2.

@alexcrichton
Copy link
Member

A PR for this is now at #52375

bors added a commit that referenced this issue Jul 18, 2018
…anishearth

Lint `async` identifiers in 2018 preparation mode

r? @Manishearth

fixes #49716
@fmease fmease added A-edition-2018 Area: The 2018 edition and removed A-edition-2018-lints labels Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-edition-2018 Area: The 2018 edition A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-epoch Working group: Epoch (2018) management
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants