Skip to content

[ty] Disallow read-only fields in TypedDict updates#24128

Merged
charliermarsh merged 1 commit intomainfrom
charlie/ty-up-readonly
Mar 24, 2026
Merged

[ty] Disallow read-only fields in TypedDict updates#24128
charliermarsh merged 1 commit intomainfrom
charlie/ty-up-readonly

Conversation

@charliermarsh
Copy link
Copy Markdown
Member

@charliermarsh charliermarsh commented Mar 23, 2026

Summary

Closes astral-sh/ty#3098.

@charliermarsh charliermarsh changed the title Charlie/ty up readonly [ty] Disallow read-only fields in TypedDict updates Mar 23, 2026
@charliermarsh charliermarsh force-pushed the charlie/ty-up-readonly branch from 60d3bcc to ede055d Compare March 23, 2026 01:01
@charliermarsh charliermarsh added the ty Multi-file analysis & type inference label Mar 23, 2026
@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot bot commented Mar 23, 2026 •
edited
Loading

Typing conformance results improved 🎉

The percentage of diagnostics emitted that were expected errors increased from 85.38% to 85.39%. The percentage of expected errors that received a diagnostic increased from 78.70% to 78.79%. The number of fully passing files improved from 64/132 to 65/132.

Summary

How are test cases classified?

Each test case represents one expected error annotation or a group of annotations sharing a tag. Counts are per test case, not per diagnostic — multiple diagnostics on the same line count as one. Required annotations (E) are true positives when ty flags the expected location and false negatives when it does not. Optional annotations (E?) are true positives when flagged but true negatives (not false negatives) when not. Tagged annotations (E[tag]) require ty to flag exactly one of the tagged lines; tagged multi-annotations (E[tag+]) allow any number up to the tag count. Flagging unexpected locations counts as a false positive.

Metric Old New Diff Outcome
True Positives 835 836 +1 ⏫ (✅)
False Positives 143 143 +0
False Negatives 226 225 -1 ⏬ (✅)
Total Diagnostics 1055 1056 +1 ⏫
Precision 85.38% 85.39% +0.01% ⏫ (✅)
Recall 78.70% 78.79% +0.09% ⏫ (✅)
Passing Files 64/132 65/132 +1 ⏫ (✅)

Test file breakdown

1 file altered
File True Positives False Positives False Negatives Status
typeddicts_readonly_update.py 1 (+1) ✅ 0 0 (-1) ✅ ✅ Newly Passing 🎉
Total (all files) 836 (+1) ✅ 143 225 (-1) ✅ 65/132

True positives added (1)

1 diagnostic
Test case Diff

typeddicts_readonly_update.py:23

 
+error[invalid-argument-type] Argument is incorrect: Expected `<TypedDict with items 'x', 'y'> | Iterable[tuple[str, object]]`, found `A`

@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot bot commented Mar 23, 2026 •
edited
Loading

Memory usage report

Memory usage unchanged ✅

@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot bot commented Mar 23, 2026 •
edited
Loading

ecosystem-analyzer results

Lint rule Added Removed Changed
invalid-await 40 0 0
invalid-return-type 1 0 0
Total 41 0 0

Changes in flaky projects detected. Raw diff output excludes flaky projects; see the HTML report for details.

Full report with detailed diff (timing results)

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 23, 2026 •
edited
Loading

Merging this PR will not alter performance

✅ 27 untouched benchmarks
⏩ 30 skipped benchmarks1

Comparing charlie/ty-up-readonly (70b4f79) with main (cd0705d)

Open in CodSpeed

Footnotes

  1. 30 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@charliermarsh charliermarsh marked this pull request as ready for review March 23, 2026 01:33
@charliermarsh charliermarsh force-pushed the charlie/ty-up branch 2 times, most recently from 8952548 to 9d0c63b Compare March 23, 2026 18:42
Base automatically changed from charlie/ty-up to main March 23, 2026 19:20
@charliermarsh charliermarsh force-pushed the charlie/ty-up-readonly branch from ede055d to 70b4f79 Compare March 23, 2026 19:27
@carljm carljm removed their request for review March 23, 2026 19:38
ibraheemdev
ibraheemdev approved these changes Mar 24, 2026
View reviewed changes
crates/ty_python_semantic/src/types/typed_dict.rs
let mut field = field.clone().with_required(false);
if field.is_read_only() {
field.declared_ty = Type::Never;
}
Copy link
Copy Markdown
Member

@ibraheemdev ibraheemdev Mar 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I'm a little confused wny we need to keep the field present with Never here, it seems like TypedDict subtyping should handle the Never case correctly, and if not we should fix it there instead of here (i.e., that a TypedDict A { id: NotRequired[Never] } is a subtype of B {}. Is there a reason you special-cased it here, is this a pattern that shows up in the ecosystem?

Copy link
Copy Markdown
Member Author

@charliermarsh charliermarsh Mar 24, 2026

Choose a reason for hiding this comment

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

This is the argument type for update. If we omit this, it'll accept these fields. Maybe I'm misunderstanding?

Copy link
Copy Markdown
Member

@ibraheemdev ibraheemdev Mar 24, 2026

Choose a reason for hiding this comment

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

Ah sorry, I misunderstood.

@charliermarsh charliermarsh merged commit 6c8101a into main Mar 24, 2026
49 checks passed
@charliermarsh charliermarsh deleted the charlie/ty-up-readonly branch March 24, 2026 20:31
carljm added a commit that referenced this pull request Mar 25, 2026
@carljm
Merge branch 'main' into charlie/metaclass
9114b71 
* main:
  [ty] make `test-case` a dev-dependency (#24187)
  [ty] implement cycle normalization for more types to prevent too-many-cycle panics (#24061)
  [ty] Silence all diagnostics in unreachable code (#24179)
  [ty] Intern `InferableTypeVars` (#24161)
  Implement unnecessary-if (RUF050) (#24114)
  Recognize `Self` annotation and `self` assignment in SLF001 (#24144)
  Bump the npm version before publish (#24178)
  [ty] Disallow Self in metaclass and static methods (#23231)
  Use trusted publishing for NPM packages (#24171)
  [ty] Respect non-explicitly defined dataclass params (#24170)
  Add RUF072: warn when using  operator on an f-string (#24162)
  [ty] Check return type of generator functions (#24026)
  Implement useless-finally (RUF-072) (#24165)
  [ty] Add test for a dataclass with a default field converter (#24169)
  [ty] Dataclass field converters (#23088)
  [flake8-bandit] Treat sys.executable as trusted input in S603 (#24106)
  [ty] Add support for `typing.Concatenate` (#23689)
  `ASYNC115`: autofix to use full qualified `anyio.lowlevel` import (#24166)
  [ty] Disallow read-only fields in TypedDict updates (#24128)
  Speed up diagnostic rendering (#24146)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ibraheemdev ibraheemdev ibraheemdev approved these changes

@AlexWaygood AlexWaygood Awaiting requested review from AlexWaygood AlexWaygood is a code owner

@sharkdp sharkdp Awaiting requested review from sharkdp sharkdp is a code owner

@dcreager dcreager Awaiting requested review from dcreager dcreager is a code owner

Assignees

@ibraheemdev ibraheemdev

Labels

ty Multi-file analysis & type inference

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Updating a read-only TypedDict item via .update(…) should be forbidden

2 participants

@charliermarsh @ibraheemdev