Skip to content

fix use all usages within class (or within __init__) to infer type, not just initial assignment #1159#2356

Open
asukaminato0721 wants to merge 3 commits intofacebook:mainfrom
asukaminato0721:1159
Open

fix use all usages within class (or within __init__) to infer type, not just initial assignment #1159#2356
asukaminato0721 wants to merge 3 commits intofacebook:mainfrom
asukaminato0721:1159

Conversation

@asukaminato0721
Copy link
Copy Markdown
Contributor

Summary

Fixes #1159

Implemented the issue 1159 request to use assignments within the class (e.g., a = 1 then self.a = "a") when inferring attribute types, so the method assignment widens the inferred type instead of erroring.

Test Plan

add tests

@meta-cla meta-cla bot added the cla signed label Feb 7, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@asukaminato0721 asukaminato0721 marked this pull request as ready for review February 9, 2026 05:52
Copilot AI review requested due to automatic review settings February 9, 2026 05:52
Copy link
Copy Markdown

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 updates Pyrefly’s class attribute inference so that assignments to self.<attr> inside class methods can contribute to the inferred attribute type, avoiding type errors when a class-body attribute is later assigned a different type in a method (Issue #1159).

Changes:

  • Collect and retain all method self-assignments per attribute during class-scope finalization.
  • Plumb collected method assignments through BindingClassField into the solver and widen inferred attribute types by unioning class-body type with method-assigned types (when safe).
  • Add regression tests validating class-body + method assignment widening to a union type.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
pyrefly/lib/test/attributes.rs Adds tests for widening class-body attributes based on method assignments.
pyrefly/lib/binding/scope.rs Collects method-defined attribute assignments and associates them with class fields.
pyrefly/lib/binding/class.rs Stores per-field method assignment metadata into BindingClassField.
pyrefly/lib/binding/binding.rs Extends BindingClassField to include method assignments; adds MethodDefinedAttribute type; updates size asserts.
pyrefly/lib/alt/solve.rs Passes method assignment metadata into class-field solving.
pyrefly/lib/alt/class/class_field.rs Unions class-body inferred types with method-assignment inferred types under certain conditions.
Comments suppressed due to low confidence (1)

pyrefly/lib/binding/scope.rs:2136

  • class_body.kind is moved when matching if let ScopeKind::Class(class_scope) = class_body.kind, but class_body is then used again (e.g., class_body.stat / class_body.flow). This will not compile due to a partial move. Restructure this to avoid moving a single field out of class_body (e.g., destructure class_body into stat/flow/kind first, or match on a reference to kind).
        let class_body = self.pop();
        let class_scope = {
            if let ScopeKind::Class(class_scope) = class_body.kind {
                class_scope
            } else {
                unreachable!("Expected class body scope, got {:?}", class_body.kind)
            }
        };
        self.pop(); // Also pop the annotation scope that wrapped the class body.
        class_body.stat.0.iter_hashed().for_each(

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +84 to +100
testcase!(
test_infers_attribute_union_from_class_and_method_assignments,
r#"
from typing import assert_type

class A:
value = 1

def promote(self, flag: bool) -> None:
if flag:
self.value = "a"

def takes(a: A) -> None:
assert_type(a.value, int | str)
"#,
);

Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

The PR adds coverage for class-body attribute + method reassignment, but there’s still no test exercising the “within init” case (attribute first set in __init__, then assigned a different type in another method) to ensure the new method_assignments plumbing actually widens the inferred attribute type as intended.

Copilot uses AI. Check for mistakes.
Comment on lines 2118 to 2122
/// The resulting map of field definitions:
/// - Includes both fields defined in the class body and implicit definitions
/// coming from self-assignment in methods. If both occur, only the class body
/// definition is tracked.
/// - Panics if the current scope is not a class body.
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

The doc comment says that when both a class-body definition and method self-assignments exist, “only the class body definition is tracked”, but the implementation now also tracks method_assignments and merges them into the returned map. Update the comment to reflect the new behavior to avoid misleading future changes.

Copilot uses AI. Check for mistakes.
class,
name,
direct_annotation.as_ref(),
true,
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

When analyzing each method_assignments value, this passes inferred_from_method = true into analyze_class_field_value. If the attribute exists on an ancestor, analyze_class_field_value will return the inherited type and skip analyzing the assigned value, which can prevent the union from widening to include the method-assigned type. For widening, the method assignment’s actual value type should be analyzed (i.e., avoid the inherited-type short-circuit).

Suggested change
true,
false,

Copilot uses AI. Check for mistakes.
Comment thread pyrefly/lib/alt/class/class_field.rs Outdated
@kinto0
Copy link
Copy Markdown
Contributor

kinto0 commented Feb 13, 2026

interesting... it does end up adding more errors than it removes so the argument of "widen type instead of erroring" might not hold. let's see what other people think

@rchen152
Copy link
Copy Markdown
Contributor

Hmm. I'm not convinced this is a good idea. Looking at the proposed new behavior:

from typing import assert_type
class A:
    a = 1
    def set_a(self, value: str):
        self.a = value
def f(a: A):
    assert_type(a.a, int | str)

this strikes me as potentially surprising behavior (if set_a is buried in a long class definition, a user might be quite confused about where the str type is coming from) and a little out-of-step with the "infer on first use" rule that pyrefly uses elsewhere.

Copy link
Copy Markdown
Contributor

@rchen152 rchen152 left a comment

Choose a reason for hiding this comment

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

(Leaving a comment to clear this out from my review queue.)

@yangdanny97
Copy link
Copy Markdown
Contributor

we should experiment with only allowing this in __init__

@asukaminato0721 asukaminato0721 force-pushed the 1159 branch 2 times, most recently from 6e25e96 to cde1201 Compare February 25, 2026 17:02
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@asukaminato0721
Copy link
Copy Markdown
Contributor Author

@yangdanny97 done

@yangdanny97
Copy link
Copy Markdown
Contributor

yangdanny97 commented Feb 25, 2026

thanks @asukaminato0721

odd, the mypy primer delta is more mixed than i would expect

@migeed-z can you run the classifier on this one?

@github-actions

This comment has been minimized.

@yangdanny97
Copy link
Copy Markdown
Contributor

wow that's a long comment lol, i'll read tomorrow and see if it's legit

@yangdanny97
Copy link
Copy Markdown
Contributor

I think the regressions are mostly caused by something being initialized both on the class & in the constructor.

I wonder if we should only take the union of assignments in __init__ if it's not initialized on the class, and take the class's initialization as the single source of truth if it's present.

The downside is that it wouldn't be able to handle things like this. We currently have a special-case for None on the class that makes it Any | None, but if we restrict this analysis we wouldn't be able to remove that hack.

class C:
  x = None

def __init__(self):
  if something:
    self.x = "foo"

@meta-codesync
Copy link
Copy Markdown
Contributor

meta-codesync bot commented Feb 27, 2026

@yangdanny97 has imported this pull request. If you are a Meta employee, you can view this in D94676249.

@yangdanny97
Copy link
Copy Markdown
Contributor

ok, i tried a few tweaks on top of this, but the delta still does not look super good. the LLM classifier output is somewhat nonsensical, which confused me for a bit.

I'm going to attempt a different approach that synthesizes fake variables for each attribute & infers the type based on their flow info at exit points to __init__, similar to how we do return type inference.

@yangdanny97 yangdanny97 reopened this Mar 13, 2026
@asukaminato0721 asukaminato0721 force-pushed the 1159 branch 2 times, most recently from 63b902b to 015278d Compare March 13, 2026 15:38
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions github-actions bot added the stale label Mar 28, 2026
@yangdanny97 yangdanny97 removed their assignment Mar 31, 2026
@yangdanny97
Copy link
Copy Markdown
Contributor

I wasn't able to figure out a balanced solution here that didn't regress mypy primer

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Copy Markdown

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

mkdocs (https://github.com/mkdocs/mkdocs)
+ ERROR mkdocs/structure/files.py:261:32-47: No matching overload found for function `posixpath.normpath` called with arguments: (((self: Self@File, use_directory_urls: bool | None = None) -> str) | str) [no-matching-overload]
+ ERROR mkdocs/structure/files.py:395:44-49: No matching overload found for function `posixpath.split` called with arguments: (((self: Self@File, use_directory_urls: bool | None = None) -> str) | str) [no-matching-overload]
+ ERROR mkdocs/structure/files.py:400:24-29: No matching overload found for function `urllib.parse.quote` called with arguments: (((self: Self@File, use_directory_urls: bool | None = None) -> str) | str | Unknown) [no-matching-overload]
+ ERROR mkdocs/structure/files.py:419:45-75: No matching overload found for function `posixpath.join` called with arguments: (str, ((self: Self@File, use_directory_urls: bool | None = None) -> str) | str) [no-matching-overload]
+ ERROR mkdocs/structure/files.py:564:49-70: No matching overload found for function `typing.MutableMapping.setdefault` called with arguments: (((self: File, use_directory_urls: bool | None = None) -> str) | str, File) [no-matching-overload]

cloud-init (https://github.com/canonical/cloud-init)
+ ERROR cloudinit/sources/DataSourceMAAS.py:148:9-26: Class member `DataSourceMAAS.check_instance_id` overrides parent class `DataSource` in an inconsistent manner [bad-override]

aiohttp (https://github.com/aio-libs/aiohttp)
+ ERROR aiohttp/connector.py:343:43-65: `StackSummary | Unknown` is not assignable to TypedDict key `source_traceback` with type `list[str] | str | Self@BaseConnector` [bad-typed-dict-key]

bokeh (https://github.com/bokeh/bokeh)
- ERROR src/bokeh/models/annotations/legends.py:319:26-43: `Value[NullStringSpec & str]` is not assignable to attribute `label` with type `NullStringSpec` [bad-assignment]

apprise (https://github.com/caronc/apprise)
- ERROR apprise/asset.py:251:21-253:67: `NotifyFormat | PersistentStoreMode` is not assignable to attribute `__storage_mode` with type `PersistentStoreMode` [bad-assignment]
+ ERROR apprise/asset.py:499:16-35: Returned type `NotifyFormat | PersistentStoreMode` is not assignable to declared return type `PersistentStoreMode` [bad-return]
- ERROR apprise/asset.py:515:16-28: Returned type `Unknown | None` is not assignable to declared return type `tzinfo` [bad-return]
+ ERROR apprise/asset.py:515:16-28: Returned type `tzinfo | Unknown | None` is not assignable to declared return type `tzinfo` [bad-return]
+ ERROR apprise/cli.py:1177:65-78: No matching overload found for function `str.join` called with arguments: (set[object] | set[Unknown]) [no-matching-overload]
- ERROR apprise/config/base.py:146:29-51: `object` is not assignable to attribute `encoding` with type `str` [bad-assignment]
+ ERROR apprise/config/file.py:121:22-57: No matching overload found for function `open` called with arguments: (Unknown, encoding=object | str) [no-matching-overload]
- ERROR apprise/plugins/base.py:375:33-60: `Unknown | None` is not assignable to attribute `interpret_emojis` with type `bool` [bad-assignment]
- ERROR apprise/plugins/kodi.py:321:17-59: `<=` is not supported between `tuple[Literal['kodi'], Literal['xbmc']]` and `int` [unsupported-operation]
+ ERROR apprise/plugins/kodi.py:321:17-59: `<=` is not supported between `tuple[str, str]` and `int` [unsupported-operation]
+ ERROR apprise/plugins/kodi.py:321:17-59: `<=` is not supported between `None` and `int` [unsupported-operation]
- ERROR apprise/plugins/kodi.py:369:17-59: `<=` is not supported between `tuple[Literal['kodi'], Literal['xbmc']]` and `int` [unsupported-operation]
+ ERROR apprise/plugins/kodi.py:369:17-59: `<=` is not supported between `tuple[str, str]` and `int` [unsupported-operation]
+ ERROR apprise/plugins/kodi.py:369:17-59: `<=` is not supported between `None` and `int` [unsupported-operation]

strawberry (https://github.com/strawberry-graphql/strawberry)
+ ERROR strawberry/aiohttp/views.py:88:5-26: Class member `GraphQLView.allow_queries_via_get` overrides parent class `AsyncBaseHTTPView` in an inconsistent manner [bad-override]
+ ERROR strawberry/asgi/__init__.py:100:5-26: Class member `GraphQL.allow_queries_via_get` overrides parent class `AsyncBaseHTTPView` in an inconsistent manner [bad-override]
+ ERROR strawberry/fastapi/router.py:60:5-26: Class member `GraphQLRouter.allow_queries_via_get` overrides parent class `AsyncBaseHTTPView` in an inconsistent manner [bad-override]
+ ERROR strawberry/sanic/views.py:57:5-26: Class member `GraphQLView.allow_queries_via_get` overrides parent class `AsyncBaseHTTPView` in an inconsistent manner [bad-override]

core (https://github.com/home-assistant/core)
+ ERROR homeassistant/components/script/__init__.py:560:5-9: Class member `ScriptEntity.icon` overrides parent class `BaseScriptEntity` in an inconsistent manner [bad-override]
+ ERROR homeassistant/components/script/__init__.py:560:5-9: Class member `ScriptEntity.icon` overrides parent class `RestoreEntity` in an inconsistent manner [bad-override]

@github-actions
Copy link
Copy Markdown

Primer Diff Classification

❌ 2 regression(s) | ✅ 5 improvement(s) | 7 project(s) total | +18, -7 errors

2 regression(s) across mkdocs, aiohttp. error kinds: no-matching-overload, bad-typed-dict-key. caused by finish_class_and_get_field_definitions(). 5 improvement(s) across cloud-init, bokeh, apprise, strawberry, core.

Project Verdict Changes Error Kinds Root Cause
mkdocs ❌ Regression +5 no-matching-overload finish_class_and_get_field_definitions()
cloud-init ✅ Improvement +1 bad-override finish_class_and_get_field_definitions()
aiohttp ❌ Regression +1 bad-typed-dict-key pyrefly/lib/alt/class/class_field.rs
bokeh ✅ Improvement -1 bad-assignment finish_class_and_get_field_definitions()
apprise ✅ Improvement +6, -6 bad-return in asset.py finish_class_and_get_field_definitions()
strawberry ✅ Improvement +4 bad-override finish_class_and_get_field_definitions()
core ✅ Improvement +1 bad-override pyrefly/lib/alt/class/class_field.rs
Detailed analysis

❌ Regression (2)

mkdocs (+5)

The PR's new attribute type widening logic (unioning class-body definitions with __init__ assignments) fails to properly handle cached_property descriptors. The dest_uri attribute is defined at class level as dest_uri = cached_property(_get_dest_path) (line 389), and in __init__ it is conditionally assigned as self.dest_uri = dest_uri (line 345) where dest_uri is a str | None parameter. The widening logic unions the class-body type with the __init__ assignment type. However, pyrefly incorrectly resolves the class-body cached_property(_get_dest_path) as the raw underlying function type (self: Self@File, use_directory_urls: bool | None = None) -> str rather than the descriptor's resolved return type str. This produces a spurious ((self: Self@File, use_directory_urls: bool | None = None) -> str) | str union type for dest_uri. Similarly, url = cached_property(_get_url) is affected the same way. All 5 errors are downstream false positives from this incorrect type inference — functions like posixpath.normpath, posixpath.split, posixpath.join, urlquote, and dict.setdefault don't accept a function type in their string parameter positions. All are pyrefly-only, confirming the regression.
Attribution: The changes in pyrefly/lib/binding/scope.rs (specifically finish_class_and_get_field_definitions()) and pyrefly/lib/alt/class/class_field.rs now collect method assignments (including __init__) and widen attribute types by unioning the class-body type with __init__ assignment types. For dest_uri, the class body defines it as cached_property(_get_dest_path), and the PR's new logic unions this with the str from __init__. But instead of resolving the cached_property descriptor to its return type (str) before unioning, it appears to use the raw function type (self: Self@File, use_directory_urls: bool | None = None) -> str, producing the incorrect union type seen in all 5 errors.

aiohttp (+1)

This is a false positive. The context variable on line 337 is a plain dict literal {'connector': self, 'connections': conns, 'message': 'Unclosed connector'}. Pyrefly appears to be inferring it as a TypedDict with specific value types, then checking the assignment context['source_traceback'] = self._source_traceback against those inferred types. But context is just a dict[str, ...] — there's no TypedDict definition here. The PR's change to widen _source_traceback from None to StackSummary | None (by unioning the class-level None with the __init__ assignment of traceback.extract_stack(...)) exposed this pre-existing issue with TypedDict inference on plain dict literals. Neither mypy nor pyright flag this, confirming it's a pyrefly-specific false positive. The error message showing list[str] | str | Self@BaseConnector as the expected type for the key further confirms incorrect TypedDict inference — those types come from the other values in the dict literal (conns is list[str] from the list comprehension on line 332, 'Unclosed connector' is str, self is Self@BaseConnector).
Attribution: The PR changes how pyrefly infers attribute types by combining class body assignments with __init__ method assignments. In BaseConnector, _source_traceback is defined as None at class level (line 242) and then assigned traceback.extract_stack(...) (which returns StackSummary) in __init__ (line 274). The PR's new logic in pyrefly/lib/alt/class/class_field.rs and pyrefly/lib/binding/scope.rs now unions these types, making _source_traceback have type StackSummary | None (or with Unknown mixed in). Previously it was likely just None. The widened type then fails the TypedDict key check because pyrefly infers context as a TypedDict with specific value types from the literal on lines 337-341, and StackSummary | Unknown doesn't match the expected value type for the source_traceback key. The root issue is that context is a plain dict[str, Any] literal, not a TypedDict — pyrefly is incorrectly treating it as a TypedDict and then the widened _source_traceback type triggers a mismatch.

✅ Improvement (5)

cloud-init (+1)

The PR changes attribute type inference to consider __init__ assignments. The error is a bad-override on DataSourceMAAS.check_instance_id overriding DataSource.check_instance_id in an inconsistent manner. Note that pyright also flags this (as indicated in the error annotation pyright: yes), suggesting this is a real type inconsistency.

Looking at the code: check_instance_id at line 148 takes (self, sys_cfg) and returns either False or the result of self.id_hash == get_id_from_ds_cfg(ncfg), which is always bool. The == operator always returns bool regardless of the types of id_hash.

The bad-override error is likely due to a signature or return type mismatch between DataSourceMAAS.check_instance_id and the parent DataSource.check_instance_id. The parent class method likely has a different parameter type annotation for sys_cfg or a different return type. Since pyright also flags this, it's a genuine override inconsistency that exists in the codebase.

The fact that this is newly detected by pyrefly may be related to the PR's changes in how class members and method signatures are resolved, but the core issue is a method override that doesn't match the parent class's signature - not specifically about the type of id_hash affecting the return type (since == always returns bool). This is a real type inconsistency being correctly caught.

Attribution: The change in pyrefly/lib/binding/scope.rs (specifically finish_class_and_get_field_definitions()) and pyrefly/lib/alt/class/class_field.rs now collects method assignments (including __init__) and unions their types with the class body assignment. For DataSourceMAAS, id_hash = None (class body) is now unioned with self.id_hash = get_id_from_ds_cfg(self.ds_cfg) (in __init__), changing id_hash from None to None | str. This type change propagates to check_instance_id's return type, which now differs from the parent class's expected return type, triggering the bad-override error.

bokeh (-1)

This is an improvement. The removed error was a false positive — the code intentionally assigns a Value wrapper to self.label in __init__ (line 319), and label is declared at the class level as a NullStringSpec property descriptor (line 321). In Bokeh's property system, property descriptors like NullStringSpec accept Value objects as valid inputs through their __set__ descriptor protocol. The value() function from bokeh.core.property.vectorization wraps a string into a Value object, which is a standard way to set spec properties in Bokeh. The type checker was incorrectly flagging this assignment because it wasn't properly accounting for the types accepted by the property descriptor's setter. The PR correctly resolves this false positive.
Attribution: The change in pyrefly/lib/binding/scope.rs (finish_class_and_get_field_definitions()) now collects method-defined attributes and associates them with class fields. The new logic in pyrefly/lib/alt/class/class_field.rs (around line 1862+) filters for __init__ assignments and unions their types with the class body type when there's no explicit annotation. This causes label's inferred type to widen to include the Value[NullStringSpec & str] type from the __init__ assignment, eliminating the bad-assignment error.

apprise (+6, -6)

bad-return in asset.py: Both errors catch real type inconsistencies. The storage_mode property (line 499) can return NotifyFormat due to a bug on line 252 (isinstance check for wrong type). The tzinfo property (line 515) can return None from the class-level default. Both confirmed by pyright.
no-matching-overload in cli.py/config/file.py: The cli.py error on str.join(server.tags) where tags is set[object] is a downstream effect of widened type inference. 1/2 confirmed by pyright. Borderline — the tags attribute type widening may be overly broad.
unsupported-operation in kodi.py: These replace the old literal-typed tuple errors with string-typed tuple errors. Same underlying issue (comparing tuple to int), just different type representation. Both confirmed by pyright.
removed bad-assignment errors: These were reporting type mismatches at assignment sites in init. The PR moves detection to return sites (bad-return), which is arguably better — the bug is in the property return type declaration, not the assignment. Net improvement in error quality.
removed bad-return for tzinfo: The old error used Unknown type (inference failure). The new error uses concrete types (tzinfo | None). This is better error reporting.

Overall: This PR improves type inference by considering init assignments when inferring attribute types. The net effect is 6 added errors (mostly real bugs confirmed by pyright) and 6 removed errors (false positives or errors that moved to better locations). The storage_mode bug in apprise/asset.py line 252 is a genuine code defect — it checks isinstance(storage_mode, NotifyFormat) when it should check isinstance(storage_mode, PersistentStoreMode). The no-matching-overload errors on str.join with set[object] are somewhat noisy but reflect real type imprecision in the codebase.

Per-category reasoning:

  • bad-return in asset.py: Both errors catch real type inconsistencies. The storage_mode property (line 499) can return NotifyFormat due to a bug on line 252 (isinstance check for wrong type). The tzinfo property (line 515) can return None from the class-level default. Both confirmed by pyright.
  • no-matching-overload in cli.py/config/file.py: The cli.py error on str.join(server.tags) where tags is set[object] is a downstream effect of widened type inference. 1/2 confirmed by pyright. Borderline — the tags attribute type widening may be overly broad.
  • unsupported-operation in kodi.py: These replace the old literal-typed tuple errors with string-typed tuple errors. Same underlying issue (comparing tuple to int), just different type representation. Both confirmed by pyright.
  • removed bad-assignment errors: These were reporting type mismatches at assignment sites in init. The PR moves detection to return sites (bad-return), which is arguably better — the bug is in the property return type declaration, not the assignment. Net improvement in error quality.
  • removed bad-return for tzinfo: The old error used Unknown type (inference failure). The new error uses concrete types (tzinfo | None). This is better error reporting.

Attribution: The changes in pyrefly/lib/binding/scope.rs (in finish_class_and_get_field_definitions()) collect method-defined attributes and associate them with class fields. The changes in pyrefly/lib/alt/class/class_field.rs use method_assignments to widen the inferred type by unioning the class-body type with init assignment types. This causes attribute types to be wider (e.g., NotifyFormat | PersistentStoreMode instead of just PersistentStoreMode), which removes assignment-site errors but introduces return-site errors where the widened type doesn't match declared return types.

strawberry (+4)

These bad-override errors are co-reported by pyright (4/4), which is strong evidence they are real override inconsistencies. The child classes define allow_queries_via_get = True at the class level and then reassign it in __init__ from a bool parameter. The parent class AsyncBaseHTTPView likely declares this attribute in a way that the child's (possibly widened) inferred type is inconsistent. Since pyright independently agrees these are bad overrides, pyrefly is correctly detecting a real type-level issue that it previously missed. The PR's new logic to consider __init__ assignments when inferring attribute types is surfacing a legitimate inconsistency.
Attribution: The changes in pyrefly/lib/alt/class/class_field.rs and pyrefly/lib/binding/scope.rs now collect method-defined attributes (including __init__ assignments) and union them with the class body type. This new logic in finish_class_and_get_field_definitions() in pyrefly/lib/binding/scope.rs causes the __init__ assignment self.allow_queries_via_get = allow_queries_via_get to be considered alongside the class body allow_queries_via_get = True, potentially changing the inferred type and triggering the bad-override check against AsyncBaseHTTPView.

core (+1)

This is a borderline case. The PR's type widening logic unions the class-body type (None) and the init assignment type (from cfg.get(CONF_ICON)), producing a widened type for ScriptEntity.icon. The parent class hierarchy (Entity, via ToggleEntity via BaseScriptEntity) defines icon as a property. ScriptEntity overrides this with a plain attribute (icon = None at class level, line 560, and self.icon = cfg.get(CONF_ICON) in init, line 573). The bad-override error flags that the child class attribute is inconsistent with the parent class's property definition. Since pyright also flags this, there is some legitimacy to the override concern (property in parent vs plain attribute in child). However, this is a very common Home Assistant pattern and the override is functionally correct at runtime. The new error is a consequence of improved type inference (widening) interacting with override checking — it's catching a real (if minor) type-level inconsistency that pyright also catches.
Attribution: The change in pyrefly/lib/alt/class/class_field.rs that unions class-body assignments with __init__ assignments (the new block around line 1862) changed the inferred type of ScriptEntity.icon. Previously, icon = None was inferred as None. Now, with the __init__ assignment self.icon = cfg.get(CONF_ICON) (returning Any), the type is widened to None | Any. This widened/changed type triggers the bad-override check against the parent Entity.icon property.

Suggested fixes

Summary: The new attribute type widening logic (unioning class-body definitions with init assignments) fails to account for descriptor types like cached_property, causing 5 false positives in mkdocs and 1 in aiohttp.

1. In the new init_assignments widening block in class_field.rs (around line 1859-1921), the code checks descriptor.is_none() before widening, which correctly skips fields where the annotation resolves to a descriptor. However, when a field is assigned a descriptor value (e.g., dest_uri = cached_property(_get_dest_path)) without an annotation, descriptor is None because descriptor detection happens via annotation, not via the value type. The fix: before entering the widening block, also check whether value_ty (the class-body inferred type) is itself a descriptor type (e.g., cached_property, property, or any type with __get__). If the class-body value is a descriptor, skip the widening entirely — the descriptor's __get__ return type is the correct attribute type, and unioning it with __init__ assignments produces nonsensical types. Specifically, add a check like: && !self.is_descriptor_type(&ty) (or check if value_ty has a __get__ method that would make it a descriptor) to the condition guard around line 1878. This would prevent the union of (self: Self@File, use_directory_urls: bool | None = None) -> str with str | None for dest_uri.

Files: pyrefly/lib/alt/class/class_field.rs
Confidence: high
Affected projects: mkdocs
Fixes: bad-argument
The mkdocs dest_uri field is defined as dest_uri = cached_property(_get_dest_path) at class level. The class-body type resolves to the cached_property instance (which wraps a function). When the widening logic unions this with the init assignment type (str | None), it produces ((self, use_directory_urls) -> str) | str | None instead of just str | None. The descriptor.is_none() guard doesn't catch this because descriptor detection is based on annotations, not value types. Adding a guard that checks whether the class-body value_ty is a descriptor type would prevent widening for cached_property fields. This eliminates all 5 pyrefly-only errors in mkdocs.

2. The aiohttp regression is an indirect consequence of the widening logic changing _source_traceback from None to StackSummary | None, which then triggers a pre-existing pyrefly bug where plain dict literals are inferred as TypedDicts. The primary fix for mkdocs (skipping descriptor types) won't help here since _source_traceback is not a descriptor. A targeted fix: in the widening block around line 1878 in class_field.rs, when the class-body value is a simple None literal assignment (i.e., ClassFieldDefinition::AssignedInBody where the value is None), treat the __init__ assignment type as the authoritative type rather than unioning with None. This way _source_traceback would be inferred as StackSummary (from init) rather than StackSummary | None. Alternatively, this could be addressed by fixing the separate TypedDict-inference-on-plain-dicts bug, which is the deeper root cause.

Files: pyrefly/lib/alt/class/class_field.rs
Confidence: low
Affected projects: aiohttp
Fixes: bad-assignment
The aiohttp error is primarily caused by pyrefly incorrectly inferring a plain dict literal as a TypedDict. The widening from None to StackSummary | None just exposed this pre-existing bug. The real fix should be in TypedDict inference for plain dict literals, not in the widening logic. The widening behavior (None | StackSummary) is actually correct — the attribute genuinely can be None. This suggestion has low confidence because the root cause is elsewhere.


Was this helpful? React with 👍 or 👎

Classification by primer-classifier (7 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.

use all usages within class (or within __init__) to infer type, not just initial assignment

5 participants