Skip to content

Fix false positive bad-class-definition for class methods#3261

Open
QEDady wants to merge 1 commit intofacebook:mainfrom
QEDady:fix-dataclass-class-method
Open

Fix false positive bad-class-definition for class methods#3261
QEDady wants to merge 1 commit intofacebook:mainfrom
QEDady:fix-dataclass-class-method

Conversation

@QEDady
Copy link
Copy Markdown

@QEDady QEDady commented Apr 28, 2026

Summary

Pyrefly incorrectly extracts dataclass fields from class method assignments (e.g., inside @classmethod or __init_subclass__). At runtime, these assignments are ignored by Python's dataclasses.dataclass mechanism.

This leads to a false positive bad-class-definition error when a subclass defines a field without a default value.

To fix this, I introduced the new variant ClassMethod to the ClassFieldInitialization enum to explicitly track fields initialized inside class methods. These fields are now filtered out during dataclass field extraction, which aligns Pyrefly's behavior with standard Python dataclass semantics.

Fixes #3259

Test Plan

I added two new integration tests to pyrefly/lib/test/dataclass_transform.rs:

  1. test_class_method_field_ignored_by_dataclass: Verifies that fields defined in regular class methods are ignored by dataclass synthesis.
  2. test_init_subclass_field_ignored_by_dataclass: Verifies that fields defined in __init_subclass__ are ignored by dataclass synthesis.

I ran test.py and it didn't have any failure.

@meta-cla
Copy link
Copy Markdown

meta-cla Bot commented Apr 28, 2026

Hi @QEDady!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

@meta-cla
Copy link
Copy Markdown

meta-cla Bot commented Apr 28, 2026

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@meta-cla meta-cla Bot added the cla signed label Apr 28, 2026
@meta-cla
Copy link
Copy Markdown

meta-cla Bot commented Apr 28, 2026

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@NathanTempest
Copy link
Copy Markdown
Contributor

Welcome to the project and thank you for your contribution @QEDady!

The fix correctly identifies the bug (Python's @DataClass ignores cls.x = ... assignments inside class methods, but pyrefly was treating them as fields). The 2 test cases are well-chosen.

Tagging @rechen to review this PR. Here are a some key changes to consider while reviewing this PR:

  • is_assigned_in_method at class_field.rs (around line 1125) still uses matches!(_, Method) and isn't updated for the new ClassMethod variant. Worth checking if this getter should consider class methods as "assigned in a method" too.

  • Literal promotion at line ~1782 currently only triggers for ClassFieldInitialization::Method. After this PR, fields like cls.x: Literal["foo"] = "foo" inside a classmethod won't get literal preservation. Likely a minor edge case but worth confirming.

  • Design: adding a new variant means every match site has to handle it, and a few got missed. An alternative would be to check the method kind at the dataclass extraction site itself (a more localized change). Curious which approach you'd prefer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bad-class-definition due to Class Method Assignments when using dataclass_transform()

3 participants