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 2 commits 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.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
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