Skip to content

OSS-24153 | Fixed type annotations in assignments not creating symbols - #162

Merged
chrapkowski-sg merged 24 commits into
sourcegraph:scipfrom
ofeki-neosec:bugfix/OSS-24153
Nov 2, 2026
Merged

chrapkowski-sg merged 24 commits into
sourcegraph:scipfrom
ofeki-neosec:bugfix/OSS-24153

Conversation

@ofeki-neosec

@ofeki-neosec ofeki-neosec commented Nov 1, 2024 •

Copy link
Copy Markdown
Contributor

Fix information and identity for name-based annotated variables, including annotations without an initializer.

Use the same canonical declaration symbol for annotation metadata and definition/reference occurrences. Do not treat an annotation as another scope or allocate a disconnected local symbol. Share information deduplication with ordinary assignments so an annotation following an assignment does not add duplicate or orphan records.

Scope

This is a narrow annotation fix, addressing the earlier review request to clarify completeness and add coverage. It covers module/class/function/method variables, including prior assignments. It is not a redesign of member or constructor resolution.

The accumulated constructor heuristics, unrelated evaluator changes, Docker/scripts/generated changes, and already-landed casing/MRO work are excluded. Inherited methods continue to resolve through their declarations rather than being fabricated on the constructor's class.

Verification

  • Added actual indexing cases for initialized and uninitialized annotations across scopes, assignment-before-annotation, unique information records, matching local identities, and absence of orphan local records.
  • An inherited-method control still resolves Child().method() to the defining Base#method()..
  • Production/development builds, all 21 snapshots, all 7 SCIP Jest tests, and formatting passed locally on Linux with Node 24.4.1 and Python 3.10.22.
  • Native Windows/macOS execution was not performed locally.

@varungandhi-src varungandhi-src left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I haven't had the time to look into this fully, so could you elaborate on:

  • Is this meant as a full fix for the underlying bug?
  • Is the 'TODO' meant for yourself or meant to be merged?

Additionally, could you add some tests covering different cases?

@chrapkowski-sg
chrapkowski-sg merged commit e68bd53 into sourcegraph:scip Nov 2, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants