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

[flake8-pyi] Various improvements to PYI034 #10807

Merged
merged 1 commit into from Apr 6, 2024

Conversation

AlexWaygood
Copy link
Member

Summary

Currently, the logic for PYI034 is pretty-much copied straight from the original Y034 implementation in flake8-pyi. As such, the implementation reflects the limited semantic analysis that flake8-pyi is capable of -- but Ruff, with its more sophisticated semantic model, can do better here.

Specific improvements made:

  • is_metaclass now recognises that subclasses of subclasses of type/enum.EnumMeta/abc.ABCMeta are also metaclasses. (The current logic only recognises direct subclasses of these subclasses as being metaclasses. Any metaclass should be excluded from PYI034, as PEP 673 specifies that no methods in metaclasses may use Self in annotations.)
  • Subclasses of subclasses of Iterator[T] are also flagged if their __iter__ methods are annotated as returning Iterator[T] rather than Self. Currently only direct subclasses of Iterator are flagged.
  • Subclasses of subclasses of AsyncIterator[T] are also flagged if their __aiter__ methods are annotated as returning AsyncIterator[T] rather than Self. Currently only direct subclasses of AsyncIterator are flagged.

Test Plan

cargo test.

Copy link

github-actions bot commented Apr 6, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@AlexWaygood AlexWaygood added the rule Implementing or modifying a lint rule label Apr 6, 2024
.args
.iter()
.any(|expr| is_metaclass_base(expr, semantic))
analyze::class::any_qualified_name(class_def, semantic, &|qualified_name| {
Copy link
Member

Choose a reason for hiding this comment

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

IIRC this will recursively resolve subclasses (within the file). I'm not sure if that's intended in those case or not given the bullet in your PR summary.

Copy link
Member Author

Choose a reason for hiding this comment

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

Superclasses rather than subclasses, right? ;)

But yup, that's what I want! Sorry if theh PR summary was unclear

Copy link
Member

Choose a reason for hiding this comment

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

Uhh yes superclasses.

Copy link
Member

Choose a reason for hiding this comment

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

Oh right, "The current logic" refers to main, not the current logic of this PR. Of course.

Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

Great!

@AlexWaygood AlexWaygood merged commit 47e0cb8 into astral-sh:main Apr 6, 2024
17 checks passed
@AlexWaygood AlexWaygood deleted the metaclass-improvements branch April 6, 2024 23:15
Glyphack pushed a commit to Glyphack/ruff that referenced this pull request Apr 12, 2024
More accurately identify whether a class is a metaclass, a subclass of `collections.abc.Iterator`, or a subclass of `collections.abc.AsyncIterator`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants