Skip to content

Issue 3381: fix WPS115 false-positive on StrEnum, IntEnum, IntFlag at…#3382

Merged
sobolevn merged 1 commit intowemake-services:masterfrom
NikitaSemenovAiforia:issue-3381
Apr 9, 2025
Merged

Issue 3381: fix WPS115 false-positive on StrEnum, IntEnum, IntFlag at…#3382
sobolevn merged 1 commit intowemake-services:masterfrom
NikitaSemenovAiforia:issue-3381

Conversation

@NikitaSemenovAiforia
Copy link
Copy Markdown
Contributor

…tributes (#3381)

I have made things!

  • I have double checked that there are no unrelated changes in this pull request (old patches, accidental config files, etc)
  • I have created at least one test case for the changes I have made
  • I have updated the documentation for the changes I have made
  • I have added my changes to the CHANGELOG.md

Related issues

Closes #3381

Copy link
Copy Markdown
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks, really good fix!

enum_with_primitive3 = 'class First({0}, EnumMeta): ...'
enum_with_primitive4 = 'class First({0}, enum.EnumType): ...'

repr_enum_with_primitive1 = 'class First({0}, enum.StrEnum): ...'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe let's use some other name? ReprEnum can be subclassed with a primitive. But, IntEnum can't.

Maybe something like concrete_enum?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

>>> import enum
>>> class A(int, enum.ReprEnum):
...     x = 1
...     y = 2
...     
>>> A.x, A.y
(<A.x: 1>, <A.y: 2>)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

renamed

from wemake_python_styleguide.logic.source import node_to_string

_ENUM_NAMES: Final = (
_REPR_ENUM_NAMES: Final = (
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
_REPR_ENUM_NAMES: Final = (
_CONCRETE_ENUM_NAMES: Final = (

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

renamed

'EnumType',
'EnumMeta',
'enum.Flag',
'Flag',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
'Flag',
'Flag',
'enum.ReprEnum',
'ReprEnum',

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added

return any(enum_base in string_bases for enum_base in base_names)


def has_repr_enum_base(defn: ast.ClassDef) -> bool:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it ever used?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed. but also had to remove has_enum_base cause it is not used any more.

Copy link
Copy Markdown
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Great, almost there!

concrete_enum_with_primitive3 = 'class First({0}, IntEnum): ...'
concrete_enum_with_primitive4 = 'class First({0}, enum.IntEnum): ...'
concrete_enum_with_primitive5 = 'class First({0}, IntFlag): ...'
concrete_enum_with_primitive6 = 'class First({0}, enum.IntFlag): ...'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please, also test that class First(PRIMITIVE, TextChoices): ...' and similar is not possible

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added

def has_regular_enum_base(defn: ast.ClassDef) -> bool:
"""Tells whether some class has `Enum` or similar class as its base.

Unlike ``has_enum_base`` it excluded `IntEnum`, `StrEnum`, `IntFlag`.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please, fix the doc here and in one more place: has_enum_base does not exist anymore.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed. also there was a typo in test name. fixed as well

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (01b87c9) to head (a6c5331).
Report is 94 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #3382   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          361       361           
  Lines        11830     11860   +30     
  Branches       808       808           
=========================================
+ Hits         11830     11860   +30     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@NikitaSemenovAiforia NikitaSemenovAiforia force-pushed the issue-3381 branch 2 times, most recently from 1b98149 to 84142a1 Compare April 9, 2025 10:54
Copy link
Copy Markdown
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Great work! Thank you!

@sobolevn sobolevn merged commit 03436d5 into wemake-services:master Apr 9, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Excluding Enum classes from the rule WPS115 misses StrEnum

3 participants