Skip to content

[flake8-pyi] Various improvements to PYI034#10807

Merged
AlexWaygood merged 1 commit intoastral-sh:mainfrom
AlexWaygood:metaclass-improvements
Apr 6, 2024
Merged

[flake8-pyi] Various improvements to PYI034#10807
AlexWaygood merged 1 commit intoastral-sh:mainfrom
AlexWaygood:metaclass-improvements

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.

@github-actions
Copy link
Contributor

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 force-pushed the metaclass-improvements branch from 44da4ef to a7fbe73 Compare April 6, 2024 21:52
@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
@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.

2 participants