Skip to content

Fix nested subscript assignment inference - #11255

Open
AG0708 wants to merge 3 commits into
pylint-dev:mainfrom
AG0708:codex/10050-nested-subscript-assignment
Open

Fix nested subscript assignment inference#11255
AG0708 wants to merge 3 commits into
pylint-dev:mainfrom
AG0708:codex/10050-nested-subscript-assignment

Conversation

@AG0708

@AG0708 AG0708 commented Aug 12, 2026

Copy link
Copy Markdown

Type of Changes

Type
🐛 Bug fix

Description

Pylint could infer the original None value from a nested dictionary literal even when the same key was assigned a dictionary immediately before the nested assignment. That produced unsupported-assignment-operation for valid code.

This adds a narrow fallback for adjacent assignments to the exact same literal-key chain when the root is inferred as a built-in dictionary or list. Custom mappings and non-subscriptable replacement values still emit E1137; the regression suite covers both cases.

Validation:

  • functional suite: 892 passed, 12 skipped
  • checker suite: 406 passed, 44 skipped, 4 xfailed
  • full pre-commit run --all-files: passed
  • towncrier draft: passed

Closes #10050

Implementation and tests were developed with OpenAI Codex assistance and reviewed against custom-mapping and aliasing edge cases before submission.

When a nested literal-key subscript is assigned immediately after the
same slot receives a value, prefer that adjacent assignment over stale
inference from the original container literal.

Restrict the fallback to built-in list or dictionary roots and exact
name-rooted literal-key chains. Preserve the diagnostic for custom setters
and when the adjacent value is not subscriptable.

Fixes pylint-dev#10050.

Signed-off-by: Abhinav Gorrepati <gorrepatiabhinav1@gmail.com>
Co-authored-by: OpenAI Codex <noreply@openai.com>
@Pierre-Sassoulas Pierre-Sassoulas added False Positive 🦟 A message is emitted but nothing is wrong with the code Skip news 🔇 This change does not require a changelog entry and removed Skip news 🔇 This change does not require a changelog entry labels Aug 12, 2026

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for contributing to pylint. Not looking too much into the details, it seems it adds a lot of complexity. Could it be 'hidden' by a better astroid inference ?

@AG0708

AG0708 commented Aug 12, 2026

Copy link
Copy Markdown
Author

Thanks—that makes sense. I confirmed the underlying behavior is Astroid inference: at the final assignment, test_dict["key"] still infers to the original None and does not see the immediately preceding subscript mutation. I’m investigating whether this can be expressed as a smaller Astroid-level inference improvement (or a reusable inference helper) so the checker does not carry the mutation logic itself. I’ll update the patch after checking the soundness boundaries for custom mappings and properties.

@AG0708

AG0708 commented Aug 12, 2026

Copy link
Copy Markdown
Author

Follow-up: I opened pylint-dev/astroid#3220 with the narrow inference fix, including downstream Pylint regression coverage and adversarial custom-mapping/property cases. If that lands, I’ll simplify this PR to remove the checker-side inference fallback and retain only the Pylint regression/news plus the required Astroid dependency update.

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for working on pylint, I'm going to add some test cases, it's going to be easier than describing them :)

The adjacent-assignment fallback only recovers the exact shape reported
in pylint-dev#10050: a Store subscript whose replacement was assigned by the
immediately preceding sibling statement. Add functional tests for the
neighbouring shapes that still emit, so the remaining false positives
are visible and a broader fix can flip them in one place:

* an unrelated statement between the two assignments
* the item read (unsubscriptable-object) or deleted
  (unsupported-delete-operation) instead of written
* a three level chain, where only the outermost subscript is covered
* the replacement stored through an alias of the same dictionary
* an ambiguous replacement value, where safe_infer returns None and the
  stale container literal is used instead of bailing out

Also guard the shapes that already work: a function scope, a list root
and an augmented assignment.
@codecov

codecov Bot commented Aug 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.29630% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 96.40%. Comparing base (5d7d0cb) to head (dc9e001).
⚠️ Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
pylint/checkers/typecheck.py 96.29% 1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (96.29%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #11255      +/-   ##
==========================================
+ Coverage   96.36%   96.40%   +0.04%     
==========================================
  Files         178      178              
  Lines       19952    20069     +117     
==========================================
+ Hits        19227    19348     +121     
+ Misses        725      721       -4     
Files with missing lines Coverage Δ
pylint/checkers/typecheck.py 96.57% <96.29%> (-0.01%) ⬇️

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

This comment was generated for commit dc9e001

@AG0708

AG0708 commented Aug 16, 2026

Copy link
Copy Markdown
Author

Thanks—the added cases made the limitation clear. I generalized pylint-dev/astroid#3220 in 6546320 so the inference layer now handles the six new shapes here: intervening statements, reads, deletes, deeper chains, aliases, and ambiguous writes.

Running this branch against that Astroid commit removes all six newly documented false positives. Astroid's 483 inference tests and all changed-file hooks pass; its remaining full-suite failures reproduce unchanged on the parent commit.

Once the Astroid change is available, I will remove the checker-side fallback from this PR and keep the Pylint functional coverage and release note.

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could add some coverage or remove the code that can't be reached, please ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

False Positive 🦟 A message is emitted but nothing is wrong with the code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

unsupported-assignment-operation false positive

2 participants