Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

fix: Don't mark references in import statements as definitions#63284

Merged
varungandhi-src merged 2 commits into
mainfrom
vg/scip-ctags/ignore-imports
Jun 17, 2024
Merged

fix: Don't mark references in import statements as definitions#63284
varungandhi-src merged 2 commits into
mainfrom
vg/scip-ctags/ignore-imports

Conversation

@varungandhi-src

@varungandhi-src varungandhi-src commented Jun 17, 2024

Copy link
Copy Markdown
Contributor

scip-ctags directly affects type:symbol output, and type:symbol search is
currently only expected to return definitions, as it is used in the symbols sidebar
(where we want to see the symbol outline for the file), as well as type:symbol
search directly in the search UI.

Returning references in import statements is not useful.

For more context, see https://linear.app/sourcegraph/issue/SRCH-238

This patch is limited to JS/TS only. I've filed a follow-up issue for looking at
other languages here: https://linear.app/sourcegraph/issue/GRAPH-687

Test plan

Added new snapshot test

Changelog

  • Fixes a bug where import statements in TypeScript code showed up in type:symbol search.

@cla-bot cla-bot Bot added the cla-signed label Jun 17, 2024
@github-actions github-actions Bot added team/graph Graph Team (previously Code Intel/Language Tools/Language Platform) team/product-platform labels Jun 17, 2024
@varungandhi-src varungandhi-src changed the title fix: Stop marking import statements as variables fix: Don't mark references in import statements as definitions Jun 17, 2024

@kritzcreek kritzcreek 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.

Change makes sense to me, though the sparse test coverage is a bit worrying.

@varungandhi-src varungandhi-src force-pushed the vg/scip-ctags/ignore-imports branch from 468f280 to 1a6d3d6 Compare June 17, 2024 10:16
@varungandhi-src

Copy link
Copy Markdown
Contributor Author

@kritzcreek I added tests for the three different grammar constructs being targeted by the queries earlier, instead of just the one test.

@varungandhi-src varungandhi-src enabled auto-merge (squash) June 17, 2024 10:18
@varungandhi-src varungandhi-src merged commit a8e25c3 into main Jun 17, 2024
@varungandhi-src varungandhi-src deleted the vg/scip-ctags/ignore-imports branch June 17, 2024 10:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed team/graph Graph Team (previously Code Intel/Language Tools/Language Platform) team/product-platform

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants