Skip to content

Report redefined instance attributes across methods - #5392

Closed
mangeshraut712 wants to merge 18 commits into
meta:mainfrom
mangeshraut712:fix/redef-instance-attr-5387
Closed

Report redefined instance attributes across methods#5392
mangeshraut712 wants to merge 18 commits into
meta:mainfrom
mangeshraut712:fix/redef-instance-attr-5387

Conversation

@mangeshraut712

@mangeshraut712 mangeshraut712 commented Oct 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes #5387.

Summary

Emit a redefinition diagnostic when an instance attribute is annotated in more than one method. The binder preserves method-level annotations in source order before field merging.

Test Plan

  • cargo test -p pyrefly test_self_attribute_redefinition_across_methods
  • python3 test.py --no-test --no-tensor-shapes --no-conformance --no-jsonschema --no-extension

@mangeshraut712
Test reannotated instance attributes across methods
7493584
@mangeshraut712
Report duplicate instance attribute annotations
c5d3243 
Compare per-method annotations in source order before method fields are merged.
@meta-cla meta-cla Bot added the cla signed label Oct 26, 2026
@mangeshraut712 mangeshraut712 changed the title Report duplicate instance attribute annotations across methods Report redefined instance attributes across methods Oct 26, 2026
@meta-codesync

meta-codesync Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

This pull request has been imported. If you are a Meta employee, you can view this in D127579684. (Because this pull request was imported automatically, there will not be any future comments.)

@github-actions

This comment has been minimized.

Sign in to view
@mangeshraut712
fix(test): portable coverage pipe-close on macOS
915a1d0 
Same as meta#5390: BSD head rejects -n 0; use dd bs=1 count=0.
@github-actions github-actions Bot added size/s and removed size/s labels Oct 26, 2026
@github-actions

This comment has been minimized.

Sign in to view
@github-actions github-actions Bot added size/s and removed size/s labels Oct 27, 2026
@github-actions github-actions Bot added size/s and removed size/s labels Oct 27, 2026
@github-actions

This comment has been minimized.

Sign in to view
@mangeshraut712

mangeshraut712 commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

CI is green (27 checks). Reports redefined instance attributes across methods. Please review and merge when you can.

@github-actions
github-actions Bot requested a review from stroxler October 27, 2026 09:11
@github-actions github-actions Bot added size/s and removed size/s labels Oct 27, 2026
@github-actions

This comment has been minimized.

Sign in to view
@github-actions github-actions Bot added size/s and removed size/s labels Oct 27, 2026
@github-actions

This comment has been minimized.

Sign in to view
@mangeshraut712
Merge branch 'upstream/main' into fix/redef-instance-attr-5387
bb3bf0f
@mangeshraut712
Solve-time redefinition check and consistent receiver scoping
c348093 
Instance attribute redefinitions across methods are now compared at solve
time rather than using syntactic AST equality, comparing against the declaration
selected by constructor-prioritized field merging. Receiver ownership now
distinguishes local/non-receiver parameters from captured outer receivers,
treats intervening class scopes as transparent, and forbids Final-write
permissions across nested method boundaries. Finally, staticmethods skip
field inference in unannotated or @no_type_check bodies.
@github-actions github-actions Bot added size/l and removed size/m labels Nov 2, 2026
@mangeshraut712

mangeshraut712 commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

@stroxler I merged main (bb3bf0f) and pushed c348093 for the three points.

  1. The syntactic ComparableExpr check is gone. Field merging still picks the declaration, with the constructor taking priority. Each other method's annotation is compared against that one at solve time, after the types are resolved. Reordered unions and aliases are now accepted. The same spelling resolving to different types in different scopes gets flagged. A helper defined before __init__ gets the blame now, not the constructor. Class-body declarations stay out of this check, so the existing reannotation rules, including Literal narrowing, are unchanged.
  2. Attribute recording and the Final-write permission check now use the same receiver lookup. That lookup looks through class, annotation, and type-alias scopes, so an intervening class doesn't shadow a captured receiver. It also follows nonlocal self. A staticmethod param named self or any other non-receiver param shadows the receiver. If the lookup has to cross a function boundary, it returns no permission, so nested functions and methods can't write Final fields.
  3. Skipped bodies (@no_type_check, or check-unannotated-defs = false) no longer run SelfAttrNames on staticmethods.

There are new tests for each point (constructor ordering, unions/aliases, scoped aliases, Literal narrowing, nested receiver ownership, nested Final permissions, and both skipped-body staticmethod modes). The attributes, untyped_def_behaviors and binding tests pass locally.

@mangeshraut712
Keep Final-write permission in closures inside constructors
ff5acb1 
Only nested methods drop the enclosing receiver's permissions; plain
closures defined in __init__ keep them, matching the existing
test_assign_final_attr_in_nested_function_in_constructor behavior.
@mangeshraut712

mangeshraut712 commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up in ff5acb1. c348093 broke the existing test_assign_final_attr_in_nested_function_in_constructor: it also stopped plain closures in __init__ (def inner(): self.x = 1) from setting Final fields, and that is deliberately allowed today to match mypy. The receiver lookup now drops the permission only when it crosses a nested method. Plain closures, including the nonlocal self form, keep it. I updated test_final_attribute_nested_permissions to cover both.

@stroxler I took "must not cross a nested-method boundary" to mean methods only, not closures. If you meant closures too, let me know and I'll flip it and update the old test.

@github-actions github-actions Bot added size/l and removed size/l labels Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

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

============================================================
SUMMARY
============================================================
Total: +0 new errors, -31 fixed errors
By preset: +0/-2 (default), +0/-2 (strict)

Projects with changes (15):
  tornado: +0 -2
  psycopg: +0 -1
  jax: +0 -1
  prefect: +0 -1
  spark: +0 -2
  freqtrade: +0 -7
  pwndbg: +0 -2
  manticore: +0 -1
  mitmproxy: +0 -4
  cloud-init: +0 -1
  strawberry: +0 -1
  hydpy: +0 -4
  anyio: +0 -2
  meson: +0 -1
  dd-trace-py: +0 -1
============================================================

FULL DIFF DETAILS
------------------------------------------------------------

tornado (https://github.com/tornadoweb/tornado)
- ERROR tornado/test/websocket_test.py:1244:12-22: Attribute `drop_pongs` is implicitly defined by assignment in method `install_hook`, which is not a constructor [implicitly-defined-attribute]
- ERROR tornado/test/websocket_test.py:1245:12-26: Attribute `pongs_received` is implicitly defined by assignment in method `install_hook`, which is not a constructor [implicitly-defined-attribute]

psycopg (https://github.com/psycopg/psycopg)
- ERROR psycopg_pool/psycopg_pool/pool.py:569:18-28: Attribute `autocommit` is implicitly defined by assignment in method `check_connection`, which is not a constructor [implicitly-defined-attribute]

jax (https://github.com/google/jax)
- ERROR jax/_src/export/shape_poly_decision.py:110:13-36: Attribute `_decision_initial_state` is implicitly defined by assignment in method `build`, which is not a constructor [implicitly-defined-attribute]

prefect (https://github.com/PrefectHQ/prefect)
- ERROR src/prefect/workers/_cleanup.py:444:15-31: Attribute `owns_reservation` is implicitly defined by assignment in method `_lose_reservation`, which is not a constructor [implicitly-defined-attribute]

spark (https://github.com/apache/spark)
- ERROR python/pyspark/pandas/internal.py:1193:21-26: Attribute `index` is implicitly defined by assignment in method `restore_index`, which is not a constructor [implicitly-defined-attribute]
- ERROR python/pyspark/pandas/internal.py:1205:17-24: Attribute `columns` is implicitly defined by assignment in method `restore_index`, which is not a constructor [implicitly-defined-attribute]

freqtrade (https://github.com/freqtrade/freqtrade)
- ERROR freqtrade/persistence/custom_data.py:95:18-23: Attribute `value` is implicitly defined by assignment in method `_convert_custom_data`, which is not a constructor [implicitly-defined-attribute]
- ERROR freqtrade/persistence/custom_data.py:103:26-51: The type of this attribute is an explicit `Any` [explicit-any-use]
- ERROR freqtrade/resolvers/strategy_resolver.py:136:22-33: Attribute `minimal_roi` is implicitly defined by assignment in method `_normalize_attributes`, which is not a constructor [implicitly-defined-attribute]
- ERROR freqtrade/resolvers/strategy_resolver.py:136:36-141:14: This expression's type is partially unknown; it contains an implicit `Any`. Please provide an explicit type annotation. [partially-unknown-type]
- ERROR freqtrade/resolvers/strategy_resolver.py:143:22-30: Attribute `stoploss` is implicitly defined by assignment in method `_normalize_attributes`, which is not a constructor [implicitly-defined-attribute]
- ERROR freqtrade/resolvers/strategy_resolver.py:145:22-37: Attribute `max_open_trades` is implicitly defined by assignment in method `_normalize_attributes`, which is not a constructor [implicitly-defined-attribute]
- ERROR freqtrade/resolvers/strategy_resolver.py:244:22-46: Attribute `_ft_stop_uses_after_fill` is implicitly defined by assignment in method `validate_strategy`, which is not a constructor [implicitly-defined-attribute]

pwndbg (https://github.com/pwndbg/pwndbg)
- ERROR pwndbg/commands/__init__.py:304:20-24: Attribute `prog` is implicitly defined by assignment in method `initialize_parser_recursively`, which is not a constructor [implicitly-defined-attribute]
- ERROR pwndbg/commands/__init__.py:378:16-31: Attribute `formatter_class` is implicitly defined by assignment in method `initialize_parser_recursively`, which is not a constructor [implicitly-defined-attribute]

manticore (https://github.com/trailofbits/manticore)
- ERROR manticore/native/cpu/cpufactory.py:34:13-16: Attribute `cpu` is implicitly defined by assignment in method `get_cpu`, which is not a constructor [implicitly-defined-attribute]

mitmproxy (https://github.com/mitmproxy/mitmproxy)
- ERROR docs/src/examples/contrib/ntlm_upstream_proxy.py:96:18-30: Attribute `ntlm_context` is implicitly defined by assignment in method `running`, which is not a constructor [implicitly-defined-attribute]
- ERROR docs/src/examples/contrib/ntlm_upstream_proxy.py:98:18-22: Attribute `flow` is implicitly defined by assignment in method `running`, which is not a constructor [implicitly-defined-attribute]
- ERROR examples/contrib/ntlm_upstream_proxy.py:96:18-30: Attribute `ntlm_context` is implicitly defined by assignment in method `running`, which is not a constructor [implicitly-defined-attribute]
- ERROR examples/contrib/ntlm_upstream_proxy.py:98:18-22: Attribute `flow` is implicitly defined by assignment in method `running`, which is not a constructor [implicitly-defined-attribute]

cloud-init (https://github.com/canonical/cloud-init)
- ERROR tests/unittests/sources/test_lxd.py:194:18-35: Attribute `_crawled_metadata` is implicitly defined by assignment in method `test_provided_network_config`, which is not a constructor [implicitly-defined-attribute]

strawberry (https://github.com/strawberry-graphql/strawberry)
- ERROR strawberry/schema/base.py:120:19-26: Protocol variables must be explicitly declared in the class body [protocol-implicitly-defined-attribute]

hydpy (https://github.com/hydpy-dev/hydpy)
- ERROR hydpy/models/lland/lland_model.py:3705:23-30: Attribute `idx_hru` is implicitly defined by assignment in method `__call__`, which is not a constructor [implicitly-defined-attribute]
- ERROR hydpy/models/lland/lland_model.py:4255:23-30: Attribute `idx_hru` is implicitly defined by assignment in method `__call__`, which is not a constructor [implicitly-defined-attribute]
- ERROR hydpy/models/lland/lland_model.py:4439:23-30: Attribute `idx_hru` is implicitly defined by assignment in method `__call__`, which is not a constructor [implicitly-defined-attribute]
- ERROR hydpy/models/sw1d/sw1d_model.py:689:19-27: Attribute `timeleft` is implicitly defined by assignment in method `__call__`, which is not a constructor [implicitly-defined-attribute]

anyio (https://github.com/agronholm/anyio)
- ERROR src/anyio/_backends/_trio.py:740:17-21: Attribute `args` is implicitly defined by assignment in method `_convert_runtime_error_msg`, which is not a constructor [implicitly-defined-attribute]
- ERROR src/anyio/_backends/_trio.py:740:17-21: Object of class `Lock` has no attribute `args` (not declared in `__slots__`) [missing-attribute]

meson (https://github.com/mesonbuild/meson)
- ERROR mesonbuild/msubprojects.py:146:17-25: Attribute `releases` is implicitly defined by assignment in method `pre_update_wrapdb`, which is not a constructor [implicitly-defined-attribute]

dd-trace-py (https://github.com/DataDog/dd-trace-py)
- ERROR ddtrace/llmobs/_integrations/openai.py:179:14-18: Attribute `name` is implicitly defined by assignment in method `_llmobs_set_tags_from_tool`, which is not a constructor [implicitly-defined-attribute]

@mangeshraut712
Merge branch 'main' into fix/redef-instance-attr-5387
10fa7c1 
Keep the redefinition return alongside upstream's UnsetNodeId
uninitialized-field tracking in finish_class_and_get_field_definitions.
@github-actions github-actions Bot added size/l and removed size/l labels Nov 3, 2026
@github-actions

This comment has been minimized.

Sign in to view
@github-actions github-actions Bot added size/l and removed size/l labels Nov 3, 2026
@github-actions

This comment has been minimized.

Sign in to view
@github-actions github-actions Bot added size/l and removed size/l labels Nov 4, 2026
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

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

============================================================
SUMMARY
============================================================
Total: +0 new errors, -31 fixed errors
By preset: +0/-2 (default), +0/-2 (strict)

Projects with changes (15):
  tornado: +0 -2
  psycopg: +0 -1
  jax: +0 -1
  prefect: +0 -1
  spark: +0 -2
  freqtrade: +0 -7
  pwndbg: +0 -2
  manticore: +0 -1
  mitmproxy: +0 -4
  cloud-init: +0 -1
  strawberry: +0 -1
  hydpy: +0 -4
  anyio: +0 -2
  meson: +0 -1
  dd-trace-py: +0 -1
============================================================

FULL DIFF DETAILS
------------------------------------------------------------

tornado (https://github.com/tornadoweb/tornado)
- ERROR tornado/test/websocket_test.py:1244:12-22: Attribute `drop_pongs` is implicitly defined by assignment in method `install_hook`, which is not a constructor [implicitly-defined-attribute]
- ERROR tornado/test/websocket_test.py:1245:12-26: Attribute `pongs_received` is implicitly defined by assignment in method `install_hook`, which is not a constructor [implicitly-defined-attribute]

psycopg (https://github.com/psycopg/psycopg)
- ERROR psycopg_pool/psycopg_pool/pool.py:569:18-28: Attribute `autocommit` is implicitly defined by assignment in method `check_connection`, which is not a constructor [implicitly-defined-attribute]

jax (https://github.com/google/jax)
- ERROR jax/_src/export/shape_poly_decision.py:110:13-36: Attribute `_decision_initial_state` is implicitly defined by assignment in method `build`, which is not a constructor [implicitly-defined-attribute]

prefect (https://github.com/PrefectHQ/prefect)
- ERROR src/prefect/workers/_cleanup.py:444:15-31: Attribute `owns_reservation` is implicitly defined by assignment in method `_lose_reservation`, which is not a constructor [implicitly-defined-attribute]

spark (https://github.com/apache/spark)
- ERROR python/pyspark/pandas/internal.py:1193:21-26: Attribute `index` is implicitly defined by assignment in method `restore_index`, which is not a constructor [implicitly-defined-attribute]
- ERROR python/pyspark/pandas/internal.py:1205:17-24: Attribute `columns` is implicitly defined by assignment in method `restore_index`, which is not a constructor [implicitly-defined-attribute]

freqtrade (https://github.com/freqtrade/freqtrade)
- ERROR freqtrade/persistence/custom_data.py:95:18-23: Attribute `value` is implicitly defined by assignment in method `_convert_custom_data`, which is not a constructor [implicitly-defined-attribute]
- ERROR freqtrade/persistence/custom_data.py:103:26-51: The type of this attribute is an explicit `Any` [explicit-any-use]
- ERROR freqtrade/resolvers/strategy_resolver.py:136:22-33: Attribute `minimal_roi` is implicitly defined by assignment in method `_normalize_attributes`, which is not a constructor [implicitly-defined-attribute]
- ERROR freqtrade/resolvers/strategy_resolver.py:136:36-141:14: This expression's type is partially unknown; it contains an implicit `Any`. Please provide an explicit type annotation. [partially-unknown-type]
- ERROR freqtrade/resolvers/strategy_resolver.py:143:22-30: Attribute `stoploss` is implicitly defined by assignment in method `_normalize_attributes`, which is not a constructor [implicitly-defined-attribute]
- ERROR freqtrade/resolvers/strategy_resolver.py:145:22-37: Attribute `max_open_trades` is implicitly defined by assignment in method `_normalize_attributes`, which is not a constructor [implicitly-defined-attribute]
- ERROR freqtrade/resolvers/strategy_resolver.py:244:22-46: Attribute `_ft_stop_uses_after_fill` is implicitly defined by assignment in method `validate_strategy`, which is not a constructor [implicitly-defined-attribute]

pwndbg (https://github.com/pwndbg/pwndbg)
- ERROR pwndbg/commands/__init__.py:304:20-24: Attribute `prog` is implicitly defined by assignment in method `initialize_parser_recursively`, which is not a constructor [implicitly-defined-attribute]
- ERROR pwndbg/commands/__init__.py:378:16-31: Attribute `formatter_class` is implicitly defined by assignment in method `initialize_parser_recursively`, which is not a constructor [implicitly-defined-attribute]

manticore (https://github.com/trailofbits/manticore)
- ERROR manticore/native/cpu/cpufactory.py:34:13-16: Attribute `cpu` is implicitly defined by assignment in method `get_cpu`, which is not a constructor [implicitly-defined-attribute]

mitmproxy (https://github.com/mitmproxy/mitmproxy)
- ERROR docs/src/examples/contrib/ntlm_upstream_proxy.py:96:18-30: Attribute `ntlm_context` is implicitly defined by assignment in method `running`, which is not a constructor [implicitly-defined-attribute]
- ERROR docs/src/examples/contrib/ntlm_upstream_proxy.py:98:18-22: Attribute `flow` is implicitly defined by assignment in method `running`, which is not a constructor [implicitly-defined-attribute]
- ERROR examples/contrib/ntlm_upstream_proxy.py:96:18-30: Attribute `ntlm_context` is implicitly defined by assignment in method `running`, which is not a constructor [implicitly-defined-attribute]
- ERROR examples/contrib/ntlm_upstream_proxy.py:98:18-22: Attribute `flow` is implicitly defined by assignment in method `running`, which is not a constructor [implicitly-defined-attribute]

cloud-init (https://github.com/canonical/cloud-init)
- ERROR tests/unittests/sources/test_lxd.py:194:18-35: Attribute `_crawled_metadata` is implicitly defined by assignment in method `test_provided_network_config`, which is not a constructor [implicitly-defined-attribute]

strawberry (https://github.com/strawberry-graphql/strawberry)
- ERROR strawberry/schema/base.py:120:19-26: Protocol variables must be explicitly declared in the class body [protocol-implicitly-defined-attribute]

hydpy (https://github.com/hydpy-dev/hydpy)
- ERROR hydpy/models/lland/lland_model.py:3705:23-30: Attribute `idx_hru` is implicitly defined by assignment in method `__call__`, which is not a constructor [implicitly-defined-attribute]
- ERROR hydpy/models/lland/lland_model.py:4255:23-30: Attribute `idx_hru` is implicitly defined by assignment in method `__call__`, which is not a constructor [implicitly-defined-attribute]
- ERROR hydpy/models/lland/lland_model.py:4439:23-30: Attribute `idx_hru` is implicitly defined by assignment in method `__call__`, which is not a constructor [implicitly-defined-attribute]
- ERROR hydpy/models/sw1d/sw1d_model.py:689:19-27: Attribute `timeleft` is implicitly defined by assignment in method `__call__`, which is not a constructor [implicitly-defined-attribute]

anyio (https://github.com/agronholm/anyio)
- ERROR src/anyio/_backends/_trio.py:740:17-21: Attribute `args` is implicitly defined by assignment in method `_convert_runtime_error_msg`, which is not a constructor [implicitly-defined-attribute]
- ERROR src/anyio/_backends/_trio.py:740:17-21: Object of class `Lock` has no attribute `args` (not declared in `__slots__`) [missing-attribute]

meson (https://github.com/mesonbuild/meson)
- ERROR mesonbuild/msubprojects.py:146:17-25: Attribute `releases` is implicitly defined by assignment in method `pre_update_wrapdb`, which is not a constructor [implicitly-defined-attribute]

dd-trace-py (https://github.com/DataDog/dd-trace-py)
- ERROR ddtrace/llmobs/_integrations/openai.py:179:14-18: Attribute `name` is implicitly defined by assignment in method `_llmobs_set_tags_from_tool`, which is not a constructor [implicitly-defined-attribute]

@meta-codesync meta-codesync Bot closed this in d0fb354 Nov 4, 2026
@meta-codesync meta-codesync Bot added the Merged label Nov 4, 2026
@meta-codesync

meta-codesync Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

This pull request has been merged in d0fb354.

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

Reviewers

@stroxler stroxler stroxler requested changes

Assignees

@stroxler stroxler

Labels

cla signed Merged size/l

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Redefinition of instance variable.

2 participants

@mangeshraut712 @stroxler