Skip to content

fix “Decorator @X can only be used on methods” is too strict #2610#2891

Open
asukaminato0721 wants to merge 1 commit intofacebook:mainfrom
asukaminato0721:2610
Open

fix “Decorator @X can only be used on methods” is too strict #2610#2891
asukaminato0721 wants to merge 1 commit intofacebook:mainfrom
asukaminato0721:2610

Conversation

@asukaminato0721
Copy link
Contributor

Summary

Fixes #2610

top-level @staticmethod, @classmethod, @property, and @cached_property are no longer treated as invalid just because they appear outside a class body.

Test Plan

update test

Copilot AI review requested due to automatic review settings March 25, 2026 03:08
@meta-cla meta-cla bot added the cla signed label Mar 25, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

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 InvalidDecorator for 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)
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

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).

Suggested change
assert_type(Helper.compute(5), int)
assert_type(Helper.compute(5), int)
assert_type(Helper().compute(5), int)

Copilot uses AI. Check for mistakes.
Comment on lines +262 to +266
testcase!(
test_top_level_descriptor_decorators,
r#"
from typing import assert_type

Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link

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]

@github-actions
Copy link

Primer Diff Classification

✅ 5 improvement(s) | 5 project(s) total | -18 errors

5 improvement(s) across ibis, scikit-learn, spack, spark, prefect.

Project Verdict Changes Error Kinds Root Cause
ibis ✅ Improvement -1 invalid-decorator pyrefly/lib/alt/function.rs
scikit-learn ✅ Improvement -2 invalid-decorator pyrefly/lib/alt/function.rs
spack ✅ Improvement -11 invalid-decorator pyrefly/lib/alt/function.rs
spark ✅ Improvement -2 invalid-decorator pyrefly/lib/alt/function.rs
prefect ✅ Improvement -2 invalid-decorator pyrefly/lib/alt/function.rs
Detailed analysis

✅ Improvement (5)

ibis (-1)

This is a clear improvement. The _get_meta function on line 265 is a property factory — it applies @property to a locally-defined function f and returns the resulting property descriptor. The property is then used as a class attribute on TableMetadata (e.g., create_time = _get_meta('info', 'CreateTime') on line 315). This is a valid and well-established Python pattern. Pyrefly was incorrectly treating @property outside a class body as always invalid, which is too strict. The PR correctly relaxes this restriction.
Attribution: The change in pyrefly/lib/alt/function.rs removed the SpecialDecorator::Property(name) (along with StaticMethod, ClassMethod, and CachedProperty) from the match arms that generate the 'can only be used on methods' error. This means these decorators are no longer flagged as invalid when used outside a class body. The test file pyrefly/lib/test/decorators.rs confirms this intent with the new test_top_level_descriptor_decorators test case that validates @property, @staticmethod, and @classmethod used outside class bodies in factory patterns.

scikit-learn (-2)

Both removed errors were false positives. @property is a descriptor that can be legitimately created outside a class body and later assigned as a class attribute. In test_metaestimators.py, the hides() function uses @property on an inner function wrapper to create a property descriptor, which hides() then returns. When @hides is used as a decorator on SubEstimator methods (e.g., @hides on inverse_transform), the method function is passed to hides(), which wraps it in a property descriptor. The result is that the class attribute becomes a property. The @property on line 90 is inside a regular function, not a class body, but this is valid Python - it creates a property object that is later assigned as a class attribute. In deprecation.py, _decorate_property() similarly uses @property inside a method to wrap a property's getter in a new property that emits deprecation warnings, returning the new property descriptor to be assigned as a class attribute. Both are standard Python patterns for programmatically creating property descriptors. The PR correctly relaxes the overly strict check that required @property (and similar descriptors) to appear only inside class bodies.
Attribution: The change in pyrefly/lib/alt/function.rs removed SpecialDecorator::Property(name) (along with StaticMethod, ClassMethod, and CachedProperty) from the match arms in the function that reports 'can only be used on methods' errors. This means these four decorators are no longer flagged as invalid when used outside a class body. The test file pyrefly/lib/test/decorators.rs confirms the intent: the old test expected @property and @staticmethod etc. to produce errors at top level, while the new test (test_top_level_descriptor_decorators) explicitly validates that @property, @staticmethod, and @classmethod work correctly at top level with descriptors being assigned to class attributes.

spack (-11)

These 11 removed invalid-decorator errors were all false positives. The @classmethod decorator (and @staticmethod, @property) can legitimately be used outside a class body to create descriptor objects that are later assigned to classes. This is a common pattern in metaclasses (as seen in DetectablePackageMeta.__init__) and in test code. Pyrefly was incorrectly restricting these decorators to only appear inside class bodies, which is not a Python language requirement. The PR correctly relaxes this restriction.
Attribution: The change in pyrefly/lib/alt/function.rs removed the SpecialDecorator::StaticMethod, SpecialDecorator::ClassMethod, SpecialDecorator::Property, and SpecialDecorator::CachedProperty cases from the match arm that generates 'can only be used on methods' errors. This directly stops pyrefly from flagging @staticmethod, @classmethod, @property, and @cached_property when used outside class bodies. The test file pyrefly/lib/test/decorators.rs confirms the intent by removing the expected errors for these decorators at the top level and adding a new test test_top_level_descriptor_decorators that validates the pattern works correctly.

spark (-2)

The analysis is factually correct. The @property decorator creates a descriptor object and can indeed be applied to any function in Python - it's not restricted to class bodies at runtime. The code shows @property being used inside a factory function unsupported_property() to create property descriptor objects that will later be assigned to class attributes. This is a valid and common Python pattern (used in PySpark and other libraries). Pyrefly's [invalid-decorator] error was incorrectly flagging these as invalid because it enforced a rule that @property can only be used on methods (i.e., inside class bodies), when in fact Python allows @property to be used anywhere. The property objects created here are returned from the factory function and presumably assigned as class attributes elsewhere, where they function correctly as descriptors. Both errors at lines 36 and 42 are false positives that should be removed.
Attribution: The change in pyrefly/lib/alt/function.rs removed SpecialDecorator::Property(name) (along with StaticMethod, ClassMethod, and CachedProperty) from the match arms that generate the 'can only be used on methods' error. This directly caused the two invalid-decorator errors on @property in pyspark/pandas/missing/__init__.py to be removed. The test case test_top_level_descriptor_decorators in pyrefly/lib/test/decorators.rs confirms the intended behavior — @property outside a class body is now allowed.

prefect (-2)

These were false positives. @classmethod and @staticmethod are built-in functions that return descriptor objects. Using them outside a class body is a valid Python pattern — the resulting descriptor is typically assigned to a class attribute later. Both instances in prefect follow this exact pattern: the classmethod descriptors are created at module/function level and then attached to classes via setattr. Pyrefly was incorrectly restricting these decorators to class bodies only. Removing these errors is an improvement.
Attribution: The change in pyrefly/lib/alt/function.rs removed SpecialDecorator::StaticMethod, SpecialDecorator::ClassMethod, SpecialDecorator::Property, and SpecialDecorator::CachedProperty from the match arms in the function that reports 'can only be used on methods' errors. This directly caused the two invalid-decorator errors on prefect to be removed. The test file pyrefly/lib/test/decorators.rs confirms the intent: the test test_invalid_top_level_function_decorators no longer expects errors for @staticmethod, @classmethod, @abstractstaticmethod, or @property, and a new test test_top_level_descriptor_decorators validates that these decorators work correctly at module level.


Was this helpful? React with 👍 or 👎

Classification by primer-classifier (5 LLM)

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.

“Decorator @X can only be used on methods” is too strict

2 participants