Repository navigation
destringlyfy lint infrastructure #164607
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: main
Are you sure you want to change the base?
destringlyfy lint infrastructure #164607
Changes from all commits
5afdb7a
123d20d
6a3df09
f6f8683
09a5060
a8a0f42
232a750
361557a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,9 @@ | ||
| use rustc_macros::{Decodable, Encodable, PrintAttribute, StableHash}; | ||
| use rustc_span::{Ident, Span, Symbol, sym}; | ||
| use rustc_span::{Span, Symbol, sym}; | ||
| use rustc_structures::LintName; | ||
|
|
||
| use crate::{HashIgnoredAttrId, PrintAttribute}; | ||
|
|
||
| #[derive(Clone, Copy, Debug, StableHash, Encodable, Decodable, PrintAttribute)] | ||
| pub enum LintCheckKind { | ||
| Allow, | ||
|
|
@@ -43,21 +45,7 @@ impl LintCheckKind { | |
| /// ``` | ||
| #[derive(Clone, Debug, StableHash, Encodable, Decodable, PrintAttribute)] | ||
| pub struct LintCheck { | ||
| /// The lint's tool name, if present. | ||
| pub tool: Option<Ident>, | ||
| /// The lint's name, minus the tool name if any. | ||
| /// | ||
| /// For example: | ||
| /// - `clippy::blah` -> `blah` | ||
| /// - `unstable_imports` -> `unstable_imports` | ||
| /// - `custom_tool::blah::bleh` -> `blah::bleh` | ||
| pub lint_name: Symbol, | ||
| /// The span of the lint name. | ||
| /// | ||
| /// For example `#[deny(foo, bar, reason = "reason")]` | ||
| /// produces multiple `LintCheck`s, one with a span pointing to `foo` | ||
| /// and another pointing to `bar`. | ||
| pub lint_span: Span, | ||
| pub lint_name: LintName, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My dream solution would be to get an
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. First exploration seems to show potential here, I think this should be possible! (with a few minor hacks)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
there are some reasons to not do this:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think that could either stay in the HIR by name if it's useful, and if it's not useful removed entirely.
The only information we'd need is from the The only significant problem I've encountered so far is that
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I got nerd-sniped too hard so this is part 1 of this approach: #164918
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fwiw, I think our approaches are actually mostly compatible. It's still possible to introduce a
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This PR also has better perf than #164918 (tho both are improvements) so doing both definitely makes sense |
||
| pub kind: LintCheckKind, | ||
| pub reason: Option<Symbol>, | ||
| /// Needed by `LintExpectationId` to track fulfilled expectations | ||
|
|
@@ -68,4 +56,10 @@ pub struct LintCheck { | |
| /// inside one attribute are all smooshed and flattened together, so | ||
| /// multiple `LintCheck`s can have the same `attr_span`. | ||
| pub attr_span: Span, | ||
| /// The span of the entire lint name. | ||
| /// | ||
| /// #[deny(foo::bar::baz)] | ||
| /// ^^^^^^^^^^^^^ | ||
| pub lint_span: Span, | ||
| pub tool_span: Option<Span>, | ||
| } | ||
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.
Maybe put this inside
LintNameas well, as it seems pretty related?