fix str not assignable to LiteralString attribute #3057

[asukaminato0721]

Summary

Fixes #3057

stops freezing a method-defined instance attribute at its first self.attr write and instead tracks later writes, including self.attr += ..., then widens the inferred field type across those writes.

Test Plan

add test

[Copilot]

Pull request overview

This PR fixes the false-positive from #3057 by tracking multiple method-defined instance-attribute writes (including self.attr += ...) and widening the inferred field type across those writes, rather than “freezing” on the first assignment.

Changes:

• Record and retain all writes to method-defined self.<attr> fields, including augmented assignments, and use them to widen the inferred field type.
• Refactor augmented-assignment type computation into a reusable helper for both statement checking and method-field inference.
• Add a regression test covering LiteralString-ish initialization followed by += widening to str.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pyrefly/lib/test/literal.rs	Adds a regression test for self.attr += ... widening a previously-literal-string-like attribute to str.
pyrefly/lib/query.rs	Updates class-field type extraction to handle DefinedInMethod now carrying multiple writes.
pyrefly/lib/binding/stmt.rs	Records known self.<attr> augmented-assignment writes so later inference can widen across them.
pyrefly/lib/binding/scope.rs	Changes per-method instance-attribute tracking to store a list of writes; builds DefinedInMethod from those writes.
pyrefly/lib/binding/function.rs	Updates unchecked-body self-attr discovery to fit the new InstanceAttribute shape.
pyrefly/lib/binding/binding.rs	Updates ClassFieldDefinition::DefinedInMethod to hold multiple writes (MethodFieldWrites).
pyrefly/lib/alt/operators.rs	Extracts augmented-assignment result-type computation into augassign_result_type.
pyrefly/lib/alt/class/class_field.rs	Uses the collected method writes to widen the inferred field type across assignments/augassign.

---

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

[Copilot]

SelfAttrNames::find populates InstanceAttribute.receiver_kind with MethodSelfKind::Instance unconditionally. In the unchecked-body path this can misclassify cls.attr = ... writes in @classmethods (or other non-instance receivers) as instance attributes, which can affect downstream field initialization/visibility logic. Consider passing method_self_kind into SelfAttrNames::find (and storing it on the visitor) so receiver_kind matches the actual method receiver kind.

[rchen152]

This seems to be going in the same direction as #2356, which Danny wasn't able to merge due to mypy_primer regressions. That suggests to me that collecting all writes to infer a type is likely the wrong approach. What happens if you just try promoting LiteralString to str when inferring attribute types? We might need special-casing for ALL_CAPS constants, which we already have elsewhere to avoid promoting constant module attributes.

[github-actions]

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

urllib3 (https://github.com/urllib3/urllib3)
+ ERROR src/urllib3/response.py:644:16-42: Expected a type form, got instance of `tuple[*tuple[type[Exception], ...], Unknown, Unknown]` [not-a-type]

scikit-learn (https://github.com/scikit-learn/scikit-learn)
- ERROR sklearn/preprocessing/_target_encoder.py:488:33-49: `str | Unknown` is not assignable to attribute `target_type_` with type `LiteralString` [bad-assignment]

apprise (https://github.com/caronc/apprise)
- ERROR tests/helpers/rest.py:88:21-93:10: `str` is not assignable to attribute `body` with type `LiteralString` [bad-assignment]

starlette (https://github.com/encode/starlette)
- ERROR starlette/middleware/sessions.py:37:13-32: `str` is not assignable to attribute `security_flags` with type `LiteralString` [bad-assignment]

scrapy (https://github.com/scrapy/scrapy)
+ ERROR scrapy/utils/signal.py:61:16-24: Expected a type form, got instance of `tuple[*tuple[Any, ...], type[StopDownload]]` [not-a-type]

[github-actions]

Primer Diff Classification

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

2 regression(s) across urllib3, scrapy. error kinds: not-a-type. caused by augassign_result_type(). 3 improvement(s) across scikit-learn, apprise, starlette.

Project	Verdict	Changes	Error Kinds	Root Cause
urllib3	❌ Regression	+1	not-a-type	augassign_result_type()
scikit-learn	✅ Improvement	-1	bad-assignment	augassign_result_type()
apprise	✅ Improvement	-1	bad-assignment	pyrefly/lib/alt/class/class_field.rs
starlette	✅ Improvement	-1	bad-assignment	augassign_result_type()
scrapy	❌ Regression	+1	not-a-type	augassign_result_type()

Detailed analysis

❌ Regression (2)

urllib3 (+1)

This is a false positive regression. The PR's refactoring of augmented assignment handling broke the type inference for tuple += on class attributes. The resulting type tuple[*tuple[type[Exception], ...], Unknown, Unknown] is an inference artifact — the actual type should remain tuple[type[Exception], ...]. Pyrefly then incorrectly rejects this inferred type as not being a valid type form in the except clause. The code is valid Python and correctly typed.
Attribution: The refactoring of augassign_result_type() in pyrefly/lib/alt/operators.rs changed how tuple += operations are typed. The class-level DECODER_ERROR_CLASSES += (brotli.error,) and DECODER_ERROR_CLASSES += (zstd.ZstdError,) augmented assignments now produce tuple[*tuple[type[Exception], ...], Unknown, Unknown] instead of maintaining tuple[type[Exception], ...]. The Unknown elements come from the conditionally-imported brotli.error and zstd.ZstdError. This malformed tuple type then fails the not-a-type check when used in the except clause at line 644.

scrapy (+1)

This is a false positive. Using a dynamically constructed tuple of exception types in an except clause is valid Python. The variable dont_log contains exception classes (via Any and StopDownload), and Python's except accepts tuples of exception types. Pyrefly incorrectly flags this as 'not a type form' when it should recognize that a tuple potentially containing exception types is valid in this context. Neither mypy nor pyright flag this.
Attribution: The refactoring of augmented assignment handling in pyrefly/lib/alt/operators.rs — specifically extracting augassign_result_type() as a separate method — likely changed how the type of dont_log is inferred after dont_log += (StopDownload,). The old inline code and the new extracted method should produce the same result, but the refactoring may have subtly changed the type that flows into the except clause check, causing pyrefly to now see tuple[*tuple[Any, ...], type[StopDownload]] where before it may have seen something more permissive (like Any or a simpler tuple type).

✅ Improvement (3)

scikit-learn (-1)

The removed error was a false positive. The attribute target_type_ in TargetEncoder is assigned in two branches: on line 486 it gets inferred_type_of_target (return value of type_of_target(), which returns str), and on line 488 it gets self.target_type (a constructor parameter with default "auto"). Pyrefly was incorrectly inferring the attribute type as LiteralString based on the first write (line 486) and then rejecting the second write on line 488 where self.target_type has type str | Unknown. The error str | Unknown is not assignable to attribute target_type_ with type LiteralString indicates pyrefly locked in a too-narrow type for the attribute. The PR correctly widens LiteralString to str for method-defined instance attributes without annotations, which matches the documented type (target_type_: str per the docstring/Attributes section). This is an improvement — pyrefly is now less noisy and more correct.
Attribution: The change in pyrefly/lib/alt/class/class_field.rs at lines 2369-2378 adds a transformation that promotes LiteralString to str for method-defined instance attributes without explicit annotations (and that aren't constants). This directly fixes the false positive where target_type_ was inferred as LiteralString instead of str. Additionally, the refactoring of augassign_result_type() in pyrefly/lib/alt/operators.rs and the query changes in pyrefly/lib/query.rs (separating DefinedInMethod handling) support tracking later writes to instance attributes, allowing the type to be properly widened.

apprise (-1)

This is a clear false positive removal. The attribute self.body is assigned on line 85 using "".join() with a generator containing choice(str_alpha + str_num + " ") calls. The choice() function returns str, so "".join() over non-literal string elements returns str. The second assignment on line 88 uses "\r\n".join() with list comprehension slicing self.body, which also produces str. Pyrefly was incorrectly inferring the attribute type as LiteralString (likely from the "" or "\r\n" literal separator in the .join() call), then flagging the second write as incompatible. The PR fixes this by widening LiteralString to str for method-defined instance attributes without annotations, which is the correct behavior since the attribute's type should reflect all writes to it and str.join() returns str regardless of the separator being a literal.
Attribution: The fix is in pyrefly/lib/alt/class/class_field.rs where a new transform was added (lines 2369-2377) that promotes LiteralString to str for method-defined instance attributes without explicit annotations (when the name isn't a constant). Additionally, the augassign_result_type method was extracted in pyrefly/lib/alt/operators.rs to be reusable, and pyrefly/lib/query.rs was changed to handle DefinedInMethod class fields differently — using the class field's computed type rather than just the first value's type. Together these changes ensure that self.body is correctly inferred as str rather than being frozen at LiteralString.

starlette (-1)

This is a clear improvement. The old behavior was a false positive: pyrefly inferred self.security_flags as LiteralString based on its first assignment on line 33 ("httponly; samesite=" + same_site, where same_site is Literal["lax", "strict", "none"], a subtype of LiteralString, so LiteralString + LiteralString yields LiteralString). Line 35's += "; secure" also preserves LiteralString. However, on line 37, the += with an f-string f"; domain={domain}" (where domain: str) produces str, since an f-string containing a str interpolation yields str, not LiteralString. Per the typing spec, LiteralString + str produces str, so the attribute should be widened to str. The old checker rejected this as a bad-assignment because it kept the attribute frozen at LiteralString. The PR correctly handles this by widening LiteralString to str for unannotated method-defined instance attributes.
Attribution: The fix comes from two changes: (1) In pyrefly/lib/alt/class/class_field.rs, a new block transforms LiteralString to str for method-inferred instance attributes without annotations (the is_constant_name check ensures class-level constants are not affected). (2) In pyrefly/lib/alt/operators.rs, the augmented assignment logic is refactored into augassign_result_type() to properly compute the result type. (3) In pyrefly/lib/query.rs, the hover/query logic for DefinedInMethod fields is separated to use the class field type rather than the initial value type, ensuring the widened type is reported.

Suggested fixes

Summary: The refactoring of augmented assignment into augassign_result_type() changed tuple concatenation behavior: the old code checked the full (non-distributed) base type for being a tuple, while the new code checks each union member, causing incorrect tuple concat results when the base is a union containing a tuple type.

1. In augassign_result_type() in pyrefly/lib/alt/operators.rs, the tuple concatenation branch now matches on distributed union members (lhs/rhs) instead of the original full types (base/rhs). When base is something like tuple[type[Exception], ...] | Unknown, distributing over the union causes the tuple member to be concatenated with individual non-tuple union members, producing malformed types like tuple[*tuple[type[Exception], ...], Unknown]. Fix: change the tuple concat branch to only fire when BOTH lhs and rhs are Type::Tuple, which is already the case syntactically, BUT the issue is that when rhs was Unknown (from a conditional import), the outer distribute_over_union on rhs passes Unknown through, and Unknown doesn't match Type::Tuple, so it falls to binop_call. The real issue is that distribute_over_union on base splits tuple[...] | Unknown into two calls: one with lhs=tuple[...] and one with lhs=Unknown. For lhs=tuple[...], the inner distribution over rhs (which might be tuple[type[brotli.error]]) does tuple_concat, producing a concrete concatenated tuple. Then the lhs=Unknown branch propagates Unknown. These get unioned back. The old code used base directly which was the full union and didn't match Type::Tuple. The fix should ensure that when lhs is a variadic/unbounded tuple (like tuple[X, ...]), concatenating with a concrete tuple should preserve the variadic nature. Specifically, in tuple_concat() or in the branch itself, when one side is an unbounded tuple tuple[T, ...], the result of concatenation should remain tuple[T, ...] (since adding finite elements to an unbounded tuple keeps it unbounded with the same element type, assuming the added elements are subtypes). Alternatively, fall through to binop_call for unbounded tuples, which would use __iadd__ and return the proper type.

Files: pyrefly/lib/alt/operators.rs
Confidence: high
Affected projects: urllib3, scrapy
Fixes: not-a-type
The urllib3 case: DECODER_ERROR_CLASSES: tuple[type[Exception], ...] = (DecodeError,) then DECODER_ERROR_CLASSES += (brotli.error,). The base is tuple[type[Exception], ...] (unbounded), rhs is tuple[type[brotli.error]] (concrete 1-element). tuple_concat of these produces tuple[*tuple[type[Exception], ...], type[brotli.error]] instead of keeping it as tuple[type[Exception], ...]. The scrapy case is similar with dont_log += (StopDownload,) where dont_log starts as tuple[Any, ...]. In both cases, concatenating an unbounded/variadic tuple with a concrete tuple via tuple_concat produces a malformed unpacked type. The fix: in the tuple concat branch of augassign_result_type(), when lhs is an unbounded tuple (homogeneous tuple[T, ...]), don't use tuple_concat — instead fall through to binop_call which will use __iadd__ and return the correct homogeneous tuple type. This eliminates 2 not-a-type errors across urllib3 and scrapy.

---

Was this helpful? React with 👍 or 👎

_{Classification by primer-classifier (5 LLM)}