Skip to content

fix(adding_lints): usage of early vs late lint pass #13955

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

Merged
merged 2 commits into from
Jan 13, 2025

Conversation

aaron-ang
Copy link
Contributor

@aaron-ang aaron-ang commented Jan 6, 2025

changelog: none

This PR fixes explaining the difference in usage between early and late lint passes in the book.

@rustbot
Copy link
Collaborator

rustbot commented Jan 6, 2025

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @flip1995 (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 6, 2025
@samueltardieu samueltardieu added the A-documentation Area: Adding or improving documentation label Jan 7, 2025
@aaron-ang
Copy link
Contributor Author

@y21 I have updated the paragraph.

Comment on lines +305 to +306
defaults to the recommended `LateLintPass`, but you can specify `--pass=early` if your lint
only needs AST level analysis.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still seems to flip the meaning, if only AST information is needed we'd still prefer it to be a late lint pass

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that EarlyLintPass is a subset of LateLintPass. In what cases should we use EarlyLintPass, then? When editing this paragraph, I considered the context in which it is written. The author uses an example of an EarlyLintPass and explains that "we only have to deal with the AST and don't have to deal with the type system at all."

If this edit still seems to flip the meaning, we should consider editing other parts of the section.

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 the new formulation is correct. Why would we prefer a LateLintPass if only AST information are required?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what cases should we use EarlyLintPass, then?

It's not a strict subset, there's occasionally information that is present in the AST but not in the HIR. IIRC some information about use declarations aren't preserved in the HIR for example

Why would we prefer a LateLintPass if only AST information are required?

Because when either a late or early pass would work there's no advantage to going with an early pass, yet lints often find themselves needing type information further down the line. Types aside many util functions are written for late passes

Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@flip1995 flip1995 added this pull request to the merge queue Jan 13, 2025
Merged via the queue into rust-lang:master with commit 0db6411 Jan 13, 2025
11 checks passed
@aaron-ang aaron-ang deleted the patch-1 branch January 21, 2025 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documentation Area: Adding or improving documentation S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants