fix “Decorator @X can only be used on methods” is too strict #2610#2891
fix “Decorator @X can only be used on methods” is too strict #2610#2891asukaminato0721 wants to merge 1 commit intofacebook:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR relaxes Pyrefly’s “Decorator @X can only be used on methods” validation so that descriptor-style decorators (@staticmethod, @classmethod, @property, @cached_property) are permitted at module scope, supporting common “factory then assign into class” patterns (e.g., Django/admin helpers), and adds regression coverage for the intended usage.
Changes:
- Stop emitting
InvalidDecoratorfor top-level@staticmethod,@classmethod,@property, and@cached_property. - Update the existing “invalid top-level decorators” test to keep only decorators that remain invalid at module scope.
- Add a new test case covering top-level descriptor creation and later assignment into classes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
pyrefly/lib/test/decorators.rs |
Updates invalid-decorator expectations and adds a new regression test for top-level descriptor decorators assigned into classes. |
pyrefly/lib/alt/function.rs |
Narrows top-level decorator validation to only the decorators that should remain method-only (e.g., @final, @override, enum decorators, @abstractmethod). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| compute = utility | ||
|
|
||
|
|
||
| assert_type(Helper.compute(5), int) |
There was a problem hiding this comment.
The new staticmethod scenario only calls Helper.compute(5) via the class. A plain undecorated function assigned as a class attribute is also callable via the class, so this test doesn’t actually validate staticmethod descriptor semantics. Consider also asserting Helper().compute(5) is int (instance access is where @staticmethod differs from a regular function attribute).
| assert_type(Helper.compute(5), int) | |
| assert_type(Helper.compute(5), int) | |
| assert_type(Helper().compute(5), int) |
| testcase!( | ||
| test_top_level_descriptor_decorators, | ||
| r#" | ||
| from typing import assert_type | ||
|
|
There was a problem hiding this comment.
PR description mentions top-level @cached_property is now allowed, but the added regression test covers @property, @staticmethod, and @classmethod only. Adding a top-level @cached_property factory/assignment case here would prevent regressions for that specific decorator.
|
Diff from mypy_primer, showing the effect of this PR on open source code: ibis (https://github.com/ibis-project/ibis)
- ERROR ibis/backends/impala/metadata.py:266:5-14: Decorator `@property` can only be used on methods. [invalid-decorator]
scikit-learn (https://github.com/scikit-learn/scikit-learn)
- ERROR sklearn/tests/test_metaestimators.py:90:9-18: Decorator `@property` can only be used on methods. [invalid-decorator]
- ERROR sklearn/utils/deprecation.py:106:9-18: Decorator `@property` can only be used on methods. [invalid-decorator]
spack (https://github.com/spack/spack)
- ERROR lib/spack/spack/package_base.py:173:13-25: Decorator `@classmethod` can only be used on methods. [invalid-decorator]
- ERROR lib/spack/spack/package_base.py:190:13-25: Decorator `@classmethod` can only be used on methods. [invalid-decorator]
- ERROR lib/spack/spack/package_base.py:247:13-25: Decorator `@classmethod` can only be used on methods. [invalid-decorator]
- ERROR lib/spack/spack/test/cmd/external.py:80:5-17: Decorator `@classmethod` can only be used on methods. [invalid-decorator]
- ERROR lib/spack/spack/test/cmd/external.py:254:5-17: Decorator `@classmethod` can only be used on methods. [invalid-decorator]
- ERROR lib/spack/spack/test/cmd/external.py:295:5-17: Decorator `@classmethod` can only be used on methods. [invalid-decorator]
- ERROR lib/spack/spack/test/cmd/external.py:326:5-17: Decorator `@classmethod` can only be used on methods. [invalid-decorator]
- ERROR lib/spack/spack/test/cmd/external.py:360:5-17: Decorator `@classmethod` can only be used on methods. [invalid-decorator]
- ERROR lib/spack/spack/test/detection.py:80:5-17: Decorator `@classmethod` can only be used on methods. [invalid-decorator]
- ERROR lib/spack/spack/test/packaging.py:388:5-14: Decorator `@property` can only be used on methods. [invalid-decorator]
- ERROR lib/spack/spack/test/packaging.py:404:5-14: Decorator `@property` can only be used on methods. [invalid-decorator]
spark (https://github.com/apache/spark)
- ERROR python/pyspark/pandas/missing/__init__.py:36:5-14: Decorator `@property` can only be used on methods. [invalid-decorator]
- ERROR python/pyspark/pandas/missing/__init__.py:42:5-14: Decorator `@property` can only be used on methods. [invalid-decorator]
prefect (https://github.com/PrefectHQ/prefect)
- ERROR src/prefect/utilities/dispatch.py:100:1-13: Decorator `@classmethod` can only be used on methods. [invalid-decorator]
- ERROR src/prefect/utilities/pydantic.py:154:9-21: Decorator `@classmethod` can only be used on methods. [invalid-decorator]
|
Primer Diff Classification✅ 5 improvement(s) | 5 project(s) total | -18 errors 5 improvement(s) across ibis, scikit-learn, spack, spark, prefect.
Detailed analysis✅ Improvement (5)ibis (-1)
scikit-learn (-2)
spack (-11)
spark (-2)
prefect (-2)
Was this helpful? React with 👍 or 👎 Classification by primer-classifier (5 LLM) |
Summary
Fixes #2610
top-level
@staticmethod,@classmethod,@property, and@cached_propertyare no longer treated as invalid just because they appear outside a class body.Test Plan
update test