Skip to content

Conversation

@MichaReiser
Copy link
Member

Summary

This PR adds support for parsing type: ignore comments that have an optional codes segment, e.g. type: ignore[a, b, c]. It also introduces the new knot: ignore[codes] suppression comment.

The logic is split into two parts:

  • A suppression comment parser that parses a single # type: ignore comment
  • The logic that creates one or multiple Suppressions for each comment.

I've decided to "flatten" ignore comments with multiple codes into multiple Suppression instances because I think it will simplify
the tracking of unused suppressions and it avoids a nested vector for the lint ids in Suppression.

Test Plan

Added md test

@MichaReiser MichaReiser added the ty Multi-file analysis & type inference label Dec 20, 2024
@MichaReiser MichaReiser force-pushed the micha/ignore-codes branch 4 times, most recently from b03fb8a to 5259a0f Compare December 20, 2024 10:45
@github-actions
Copy link
Contributor

github-actions bot commented Dec 20, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@MichaReiser MichaReiser marked this pull request as ready for review December 20, 2024 10:56
Copy link
Contributor

@carljm carljm left a comment

Choose a reason for hiding this comment

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

Sweet!


## Empty codes

An empty codes array suppresses no-diagnostics and is always useless
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this something that we should flag, or just let it pass silently?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch. I updated #15084 to correctly mark the suppression as unused

Comment on lines +294 to +295
if self.cursor.eat_if(char::is_whitespace) {
self.cursor.eat_while(char::is_whitespace);
Copy link
Member

Choose a reason for hiding this comment

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

Should we use is_python_whitespace?

Copy link
Member Author

Choose a reason for hiding this comment

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

Using is_whitespace should be fine because we're in a comment. This also matches the formatter's docstring handling

@MichaReiser MichaReiser merged commit 2f85749 into main Dec 23, 2024
21 checks passed
@MichaReiser MichaReiser deleted the micha/ignore-codes branch December 23, 2024 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants