Skip to content

Report possibly uninitialized attributes defined in class bodies - #5344

Closed
asukaminato0721 wants to merge 3 commits into
meta:mainfrom
asukaminato0721:split-class-body-initialization
Closed

asukaminato0721 wants to merge 3 commits into
meta:mainfrom
asukaminato0721:split-class-body-initialization

Conversation

@asukaminato0721

@asukaminato0721 asukaminato0721 commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

A class attribute assigned in only one runtime branch is treated as always available. Add possibly-uninitialized-attribute at the definition when a completing class-body path lacks an assignment.

class C:
    if enabled():
        value = 1  # possibly-uninitialized-attribute

The default preset ignores the new error kind, and the strict preset enables it. Users can configure it separately from missing-attribute. Paths that terminate with Never or NoReturn, inherited attributes, conditional method definitions, and stub files are handled. Method assignments do not establish class-body initialization.

Reuse the existing class-body flow results and collect initialization checks when the class body completes.

Extracted from #4326. This PR is based directly on main and can land independently. The instance initialization extension remains in #4326.

  • I am a human, and I fully understand the code I am submitting.

Test Plan

  • 838 related type-checker tests passed.
  • The error configuration test passed.
  • Formatting and lint checks passed. Clippy reports existing warnings in unchanged code.

@meta-codesync

meta-codesync Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions

This comment has been minimized.

@github-actions github-actions Bot added size/xl and removed size/xl labels Oct 23, 2026
@github-actions

This comment has been minimized.

@asukaminato0721
asukaminato0721 force-pushed the split-class-body-initialization branch from 8381ad7 to b425a8e Compare October 24, 2026 03:43
@github-actions github-actions Bot added size/xl and removed size/xl labels Oct 24, 2026
@github-actions

This comment has been minimized.

@kinto0 kinto0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review automatically exported from Phabricator review in Meta.

@asukaminato0721
asukaminato0721 force-pushed the split-class-body-initialization branch from b425a8e to b1d5dc7 Compare October 29, 2026 04:46
@github-actions github-actions Bot added size/xl and removed size/xl labels Oct 29, 2026
@github-actions

Copy link
Copy Markdown

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

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

Projects with changes (7):
  pycryptodome: +1 -0
  urllib3: +3 -0
  spark: +1 -0
  pwndbg: +1 -0
  pandera: +1 -0
  pandas: +2 -0
  dd-trace-py: +2 -0
============================================================

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

pycryptodome (https://github.com/Legrandin/pycryptodome)
+ ERROR lib/Crypto/SelfTest/Signature/test_dss.py:1070:9-15: Attribute `new_tv` may be uninitialized [possibly-uninitialized-attribute]

urllib3 (https://github.com/urllib3/urllib3)
+ ERROR src/urllib3/connection.py:299:9-30: Attribute `_is_legal_header_name` may be uninitialized [possibly-uninitialized-attribute]
+ ERROR src/urllib3/connection.py:300:9-33: Attribute `_is_illegal_header_value` may be uninitialized [possibly-uninitialized-attribute]
+ ERROR src/urllib3/connection.py:303:9-42: Attribute `_contains_disallowed_url_pchar_re` may be uninitialized [possibly-uninitialized-attribute]

spark (https://github.com/apache/spark)
+ ERROR python/pyspark/testing/connectutils.py:68:15-21: Attribute `MockDF` may be uninitialized [possibly-uninitialized-attribute]

pwndbg (https://github.com/pwndbg/pwndbg)
+ ERROR pwndbg/gdblib/tui/context.py:219:9-23: Attribute `___ansi_substr` may be uninitialized [possibly-uninitialized-attribute]

pandera (https://github.com/pandera-dev/pandera)
+ ERROR pandera/typing/fastapi.py:92:41-51: Attribute `ModelField` may be uninitialized [possibly-uninitialized-attribute]

pandas (https://github.com/pandas-dev/pandas)
+ ERROR pandas/io/formats/style.py:4089:17-36: Attribute `template_html_table` may be uninitialized [possibly-uninitialized-attribute]
+ ERROR pandas/io/formats/style.py:4091:17-36: Attribute `template_html_style` may be uninitialized [possibly-uninitialized-attribute]

dd-trace-py (https://github.com/DataDog/dd-trace-py)
+ ERROR ddtrace/testing/internal/pytest/plugin.py:859:9-40: Attribute `pytest_runtest_protocol_wrapper` may be uninitialized [possibly-uninitialized-attribute]
+ ERROR ddtrace/testing/internal/pytest/plugin.py:863:9-32: Attribute `pytest_runtest_protocol` may be uninitialized [possibly-uninitialized-attribute]

meta-codesync Bot pushed a commit that referenced this pull request Oct 29, 2026
Summary:
A class attribute assigned in only one runtime branch is treated as always available. Add `possibly-uninitialized-attribute` at the definition when a completing class-body path lacks an assignment.

```python
class C:
    if enabled():
        value = 1  # possibly-uninitialized-attribute
```

The default preset ignores the new error kind, and the strict preset enables it. Users can configure it separately from `missing-attribute`. Paths that terminate with `Never` or `NoReturn`, inherited attributes, and stub files are handled. Conditionally defined methods are reported here; the next diff in the stack excludes them. Method assignments do not establish class-body initialization.

Reuse the existing class-body flow results and collect initialization checks when the class body completes.

Extracted from #4326. This PR is based directly on `main` and can land independently. The instance initialization extension remains in #4326.
- [x] I am a human, and I fully understand the code I am submitting.

X-link: #5344

Reviewed By: stroxler

Differential Revision: D127200263

Pulled By: connernilsen

fbshipit-source-id: e6854d1a65f49b4161e2becc460abc477bfdecfd
meta-codesync Bot pushed a commit that referenced this pull request Oct 29, 2026
Summary:
Split from #5344 so the exclusion can be reviewed separately from the class-body check in the previous diff.

`possibly-uninitialized-attribute` reports class-body names assigned on only some paths. A method defined under a runtime condition is usually deliberate, such as a version or platform shim, so reporting it adds noise:

```python
class C:
    if runtime_flag():
        def path(self) -> str: ...  # reported by the previous diff
```

Record on each `Definition` whether every definition site of the name is a `def` statement (`function_def_only`), the same way `main_guard_only` is tracked: set at the first site and ANDed in `Definition::merge`. A `del` or any other binding of the name clears it. The flag is copied into the scope's `StaticInfo`, and a class-body name that carries it is treated as initialized. The rule is syntactic: decorated methods (`property`, `staticmethod`, `cached_property`) are exempt, while `lambda` or `staticmethod(f)` assignments are still checked. This is deliberately the simple rule rather than a semantic one: a conditional `f = staticmethod(g)` or `f = lambda self: 1` is just as deliberate as a conditional `def`, but telling such assignments apart from data attributes would need type information that isn't available when bindings are built. Handling them would mean ad hoc syntactic special cases or a type-based check at solve time; the syntactic rule is much simpler, predictable, and the rare conditional lambda method can be written as a `def`.

Conditions on `sys.version_info` and `sys.platform` are evaluated statically, so methods defined under them were never reported.

Differential Revision: D127870321

Pulled By: connernilsen

fbshipit-source-id: 70be9926ca354b0d81d36efee7b80b3da14a0744
@stroxler

stroxler commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

This is confusing, the mirrored diff D127200263 is merged, but the PR didn't auto-close. I suspect the link was somehow lost in the diff metadata :(

Closing now - note that if you had improvements that are still relevant, you could open a new PR against trunk

@stroxler stroxler closed this Oct 29, 2026
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.

3 participants