[ty] Ignore rejected member annotations for synthesized bindings by charliermarsh · Pull Request #25427 · astral-sh/ruff

charliermarsh · 2026-05-28T11:08:33Z

Summary

We now emit invalid-type-form for an invalid annotation on a non-name target (#25324)... But it turns out that we still allow that annotation to participate in downstream operations, since we create an annotated-assignment definition for it during the indexing pass.

For example:

def accepts_inner(inner: dict[str, int]): ...

def _(x: dict[str, object]):
    x["inner"]: dict[str, float | str] = {"a": 1, "b": "a"}  # error: [invalid-type-form]
    x["inner"]["c"] = 1.0  # error: [invalid-assignment] because we infer from the RHS, not the annotation
    x["inner"] = {"inner": {"a": 1}}
    accepts_inner(**x["inner"])  # ok

The rejected annotation should not widen the dictionary assigned to x["inner"], nor should it constrain the later replacement. (The same issue applies to rejected attribute annotations, like obj.inner: T.)

We now represent each syntactically indexed declaration outcome as either Declared(...) or Rejected. Declaration consumers only use Declared(...).

astral-sh-bot · 2026-05-28T11:10:12Z

Typing conformance results

No changes detected ✅

Current numbers

The percentage of diagnostics emitted that were expected errors held steady at 91.94%. The percentage of expected errors that received a diagnostic held steady at 87.09%. The number of fully passing files held steady at 92/134.

astral-sh-bot · 2026-05-28T11:10:59Z

Memory usage report

Memory usage unchanged ✅

astral-sh-bot · 2026-05-28T11:13:10Z

`ecosystem-analyzer` results

No diagnostic changes detected ✅

Full report with detailed diff (timing results)

charliermarsh · 2026-05-30T20:38:33Z

For context, this is a bit like #25340.

carljm · 2026-06-02T00:25:44Z

Haven't looked at this PR closely, but I was curious about the approach and had Codex look at it; it pointed out that this panics in a stub file:

# repro.pyi
x: dict[str, object]
x["a"]: int
reveal_type(x["a"])

I think because we consider declarations in stub files to also be bindings.

charliermarsh · 2026-06-02T00:29:57Z

fml

carljm · 2026-06-02T00:30:08Z

It's really too bad that we can't just ignore these rejected declarations entirely in semantic indexing and avoid a lot of this complexity. It seems like maybe we could for subscripts, since those are never valid? So for x[0]: int we could just not record a definition at all, and for x[0]: int = 1 we could record something that functions only as binding, not as declaration.

But for attributes we can't, since self.x: int is valid, and it's not until typecheck time that we can tell for sure whether self is a valid "self" target.

Although I'd be really tempted to just make this decision in semantic indexing based on "are we in a method and self is its first argument", so we can discard these declarations all up front and eliminate the new enum etc. If you end up in the venn diagram of "doing weird things with shadowing self" and "making invalid attribute annotations on non-self", then you just get some odd results and that's fine.

charliermarsh · 2026-06-02T00:41:40Z

I agree that it's a pain, though I'm partial to getting it right as long as we can hide the complexity from callers...

oconnor663

[Edit: Just refreshed and saw @carljm's comment above after posting this. Looks like we had similar thoughts.]

Mostly nits about comment wording, so approving with comments. However, one high level thing I'd like to double check about the approach here: Are we sure we can't omit these bad-annotation definitions from the SemanticIndex entirely?

My guess is a lot of cases we can (e.g. x[0]: int is clearly never valid), but not all cases. IIUC the edge cases look like this:

class C:
    @classmethod
    def ok_method(x) -> None:
        x.attr: int = 1  # valid (`x` is "self" here)

    @staticmethod
    def bad_method(x) -> None:
        x.attr: int = 1  # invalid (staticmethods don't get a "self")

We could almost analyze those cases entirely statically, except that @classmethod and @staticmethod aren't keywords, and we have to resolve them during type inference (after semantic indexing).

I think that's the rub? But I'm curious if you agree. It feels like it would be nicer to discard bad definitions early if there was a way.

oconnor663 · 2026-06-02T00:18:33Z

+/// The inferred declaration produced by a syntactically indexed declaration.
+#[derive(Debug, Clone, Copy, Eq, PartialEq, salsa::Update, get_size2::GetSize)]
+pub(crate) enum InferredDeclaration<'db> {
+    /// A semantic declaration with an inferred declared type.


Unless "semantic declaration" is a term of art that's new to me(?), I think it's clearer to just call this a "valid declaration".

oconnor663 · 2026-06-02T00:23:19Z

+pub(crate) enum InferredDeclaration<'db> {
+    /// A semantic declaration with an inferred declared type.
+    Declared(TypeAndQualifiers<'db>),
+    /// A syntactic declaration candidate that is not a valid semantic declaration.


IIUC "syntactic declaration" means "an annotation or annotated assignment that the SemanticIndex has a Definition for". Is that right? If so, using the word "syntactic" to refer to the SemanticIndex is pretty confusing, even though I see where you're coming from.

Also I think it's important to give a couple examples of invalid declarations here, so that the reader knows what we're talking about.

oconnor663 · 2026-06-02T00:30:23Z

+
+            if let Some(value) = value {
+                // The value remains an ordinary assignment, but normal binding resolution
+                // would consult this rejected annotation as a declaration.


The grammar/logic of this comment seems like it almost makes sense but then not quite?

oconnor663 · 2026-06-02T00:34:09Z


-/// Infer the type of a declaration.
-pub(crate) fn declaration_type<'db>(
+/// Infer whether a syntactically indexed declaration is a semantic declaration.


The important thing this function is doing, in the overwhelming majority of valid cases, is still inferring the type of the declaration, correct? Changing the comment like this seems to obscure that.

oconnor663 · 2026-06-02T00:38:17Z

    /// The other 50% have less than 10 declarations. Because of that, use a
    /// slice with linear search over a hash map.
-    declarations: Box<[(Definition<'db>, TypeAndQualifiers<'db>)]>,
+    declarations: Box<[(Definition<'db>, InferredDeclaration<'db>)]>,


Possibly misguided q: Do we need to retain the invalid ones here? Or could we filter out just the valid ones and keep the tuple type unchanged?

charliermarsh · 2026-06-03T02:48:23Z

👍 Yeah, I don't think it's possible. I also wanted to do that, but ultimately we do need semantic information to correctly determine where/when to discard because we can't robustly identify the implicit receiver rules.

astral-sh-bot Bot added the ty Multi-file analysis & type inference label May 28, 2026

charliermarsh force-pushed the charlie/ignore-invalid-subscript-annotations branch from 76cf309 to 9a8a555 Compare May 28, 2026 11:33

charliermarsh changed the base branch from charlie/non-never to charlie/reject-assignment May 28, 2026 11:34

charliermarsh changed the title ~~[ty] Ignore invalid subscript annotations for synthesized bindings~~ [ty] Ignore rejected member annotations for synthesized bindings May 28, 2026

charliermarsh force-pushed the charlie/ignore-invalid-subscript-annotations branch 2 times, most recently from 8e66497 to 80432a6 Compare May 28, 2026 11:43

charliermarsh added the bug Something isn't working label May 28, 2026

charliermarsh changed the base branch from charlie/reject-assignment to main May 28, 2026 13:39

charliermarsh force-pushed the charlie/ignore-invalid-subscript-annotations branch 7 times, most recently from a410e49 to e73c5f3 Compare May 30, 2026 14:27

charliermarsh marked this pull request as ready for review May 30, 2026 14:34

charliermarsh requested review from AlexWaygood, carljm, dcreager, ibraheemdev and sharkdp as code owners May 30, 2026 14:34

astral-sh-bot Bot assigned oconnor663 May 30, 2026

astral-sh-bot Bot requested a review from oconnor663 May 30, 2026 14:34

charliermarsh force-pushed the charlie/ignore-invalid-subscript-annotations branch 3 times, most recently from c411148 to 82bf02e Compare June 2, 2026 01:09

oconnor663 reviewed Jun 2, 2026

View reviewed changes

charliermarsh added 2 commits June 2, 2026 22:49

[ty] Ignore rejected member annotations for synthesized bindings

f722773

Fix stub panic

89d5aa1

charliermarsh force-pushed the charlie/ignore-invalid-subscript-annotations branch from 82bf02e to b433318 Compare June 3, 2026 02:55

Review feedback

73c2d3f

charliermarsh force-pushed the charlie/ignore-invalid-subscript-annotations branch from b433318 to 73c2d3f Compare June 3, 2026 02:57

Move further up

a7a348d

charliermarsh enabled auto-merge (squash) June 3, 2026 11:30

charliermarsh merged commit bc802a8 into main Jun 3, 2026
58 checks passed

charliermarsh deleted the charlie/ignore-invalid-subscript-annotations branch June 3, 2026 11:32

Conversation

charliermarsh commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

astral-sh-bot Bot commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Typing conformance results

No changes detected ✅

Uh oh!

astral-sh-bot Bot commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Memory usage report

Uh oh!

astral-sh-bot Bot commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ecosystem-analyzer results

Uh oh!

charliermarsh commented May 30, 2026

Uh oh!

carljm commented Jun 2, 2026

Uh oh!

charliermarsh commented Jun 2, 2026

Uh oh!

carljm commented Jun 2, 2026

Uh oh!

charliermarsh commented Jun 2, 2026

Uh oh!

oconnor663 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oconnor663 Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

oconnor663 Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

oconnor663 Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

oconnor663 Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

oconnor663 Jun 2, 2026

Choose a reason for hiding this comment

Uh oh!

charliermarsh commented Jun 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

charliermarsh commented May 28, 2026 •

edited

Loading

astral-sh-bot Bot commented May 28, 2026 •

edited

Loading

astral-sh-bot Bot commented May 28, 2026 •

edited

Loading

astral-sh-bot Bot commented May 28, 2026 •

edited

Loading

`ecosystem-analyzer` results

oconnor663 left a comment •

edited

Loading