Skip to content

Fix false positive when @property getter and setter are non-adjacent#20976

Open
PGrayCS wants to merge 2 commits intopython:masterfrom
PGrayCS:fix/non-adjacent-property-setter-1465
Open

Fix false positive when @property getter and setter are non-adjacent#20976
PGrayCS wants to merge 2 commits intopython:masterfrom
PGrayCS:fix/non-adjacent-property-setter-1465

Conversation

@PGrayCS
Copy link

@PGrayCS PGrayCS commented Mar 5, 2026

Fixes #1465.

Problem

fix_function_overloads in fastparse.py groups consecutive same-named decorators into an OverloadedFuncDef. When a @property getter and its @x.setter or @x.deleter are separated by other method definitions, the setter was left as an isolated Decorator instead of being part of the property's OverloadedFuncDef. This caused mypy to report false positives:

class A:
    @property
    def x(self) -> int:
        return 0

    def _other(self) -> None:  # <-- separates getter from setter
        pass

    @x.setter
    def x(self, value: int) -> None:
        pass
error: Name "x" already defined on line 2  [no-redef]
error: "Callable[[A], int]" has no attribute "setter"  [attr-defined]
error: Property "x" defined in "A" is read-only  [misc]

This has been open since 2015 and affects any class that has non-trivial method ordering (e.g. setter after a helper method, or after a class variable).

Fix

Add a second pass _merge_non_adjacent_property_overloads that runs after the existing consecutive-grouping loop. It:

  1. Scans the output list for property getters (@property) and records their positions.
  2. Scans again for any @x.setter or @x.deleter Decorator nodes whose getter was seen earlier in the same scope.
  3. Merges those stray setter/deleter nodes into the getter's OverloadedFuncDef (promoting a lone getter Decorator to an OverloadedFuncDef when needed) and removes them from their original positions.

The adjacent case is completely unaffected: if getter and setter are consecutive, the setter is already inside the OverloadedFuncDef after the first pass and won't appear as a standalone Decorator to be found by the second pass.

Tests

Three new test cases in test-data/unit/check-classes.test:

  • testPropertyWithNonAdjacentSetter — basic setter separated by a method
  • testPropertyWithNonAdjacentDeleter — deleter separated by a method
  • testPropertyWithNonAdjacentSetterAndDeleter — both setter and deleter non-adjacent

All 130 existing property-related tests continue to pass.

PGrayCS and others added 2 commits March 5, 2026 00:05
`fix_function_overloads` in fastparse.py groups consecutive same-named
decorators into an `OverloadedFuncDef`. When a property getter and its
setter/deleter are separated by other method definitions, the setter was
left as an isolated `Decorator` instead of being folded into the
property's `OverloadedFuncDef`.  This caused mypy to report:

  error: "Callable[[A], T]" has no attribute "setter"  [attr-defined]
  error: Name "x" already defined on line N  [no-redef]
  error: Property "x" defined in "A" is read-only  [misc]

Add a second pass `_merge_non_adjacent_property_overloads` that runs
after the existing loop.  It scans the output list for any `@x.setter`
or `@x.deleter` `Decorator` nodes whose property getter was seen earlier
in the same scope, and merges them into the getter's `OverloadedFuncDef`
(promoting a lone getter `Decorator` to an `OverloadedFuncDef` when
needed).  The adjacent case is unaffected because the setter/deleter is
already inside the `OverloadedFuncDef` after the first pass.

Fixes python#1465.
@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2026

Diff from mypy_primer, showing the effect of this PR on open source code:

discord.py (https://github.com/Rapptz/discord.py)
- discord/member.py:509: error: Name "status" already defined on line 496  [no-redef]
- discord/member.py:509: error: "Callable[[Member], Status]" has no attribute "setter"  [attr-defined]
- discord/shard.py:634: error: Property "status" defined in "Member" is read-only  [misc]
- discord/client.py:2168: error: Property "status" defined in "Member" is read-only  [misc]

@bzoracler
Copy link
Contributor

The original issue surprisingly looks extremely popular.

Forcing this through OverloadedFuncDef seems very complicated, and I wonder if there's a simpler way or more semantically correct way to do this. Reading through the current implementation, IMO a lot more edge cases need to be handled:

  • There is no reason not to support @<property name>.getter, although since mypy doesn't support this currently anyway, this could be a separate issue.
  • Does this successfully register both the setter and deleter?
    class A:
        @property
        def x(self) -> int: return 1
    
        def other(self) -> None: pass
    
        @x.setter
        def x(self, val: int) -> None: pass
    
        @x.deleter
        def x(self) -> None: pass
  • Does this successfully register the setter?
    import sys
    
    class A:
        @property
        def x(self) -> int: return 1
    
        def other(self) -> None: pass
        
        if sys.version_info >= (3, 10):
            @x.setter
            def x(self, val: int) -> None: pass
  • Does this prevent the setter and deleter from being registered?
    class A:
       @property
       def x(self) -> int: return 1
    
       x = lambda self: 1
    
       @x.setter
       def x(self, val: int) -> None: pass
    
       @x.deleter
       def x(self) -> None: pass
  • Does this successfully register both the setter and deleter?
    import sys
    from typing import Any
    
    class A:
       if sys.version_info >= (3, 12):
           @property
           def x(self) -> int: return 1
       else:
           @property
           def x(self) -> int | str: return 1
    
       def other(self) -> None: pass
    
       @x.setter
       def x(self, val: Any) -> None: pass
    
       @x.deleter
       def x(self) -> None: pass

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.

Property setter not accepted if not next to getter

2 participants