Incorrect Go to Def for React element (TSX)

[varungandhi-src]

https://sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/client/web/src/enterprise/batches/detail/changesets/BatchChangeChangesets.tsx?L263

Go to Definition for ChangesetFilterRow takes you to interface Step.

[olafurpg]

I'm able to reproduce. I just snapshotted the LSIF to see what symbols are getting generated.

cd sourcegraph
lsif-typescript index --yarn-workspaces
go build github.com/sourcegraph/sourcegraph/lib/codeintel/tools/lsif-typed
lsif-typed dump.lsif-typed > dump.lsif
lsif-java snapshot-lsif

At call-site it references React/FunctionComponent#. I recall seeing similar behavior with tsserver via VS Code

                <ChangesetFilterRow
//               ^^^^^^^^^^^^^^^^^^ reference lsif-typescript npm @types/react 17.0.0 `index.d.ts`/React/FunctionComponent#

At definition-site, it weirdly enough defines that same symbol

interface Step {
//        ^^^^ definition lsif-typescript npm @types/react 17.0.0 `index.d.ts`/React/FunctionComponent# Step
//        ^^^^ definition lsif-typescript npm @sourcegraph/web 1.10.1 src/auth/`PostSignUpPage.tsx`/Step# Step

The type of ChangeFilterRow is FunctionComponent<ChangeFilterRowProps>

export const ChangesetFilterRow: React.FunctionComponent<ChangesetFilterRowProps> = ({
//           ^^^^^^^^^^^^^^^^^^ definition lsif-typescript npm @sourcegraph/web 1.10.1 src/enterprise/batches/detail/changesets/`ChangesetFilterRow.tsx`/ChangesetFilterRow. FunctionComponent<ChangesetFilterRowProps>

Based on that observation, we're using the symbol of the type instead of the symbol itself. This might be happening around how we de-alias import specifiers, which are local symbols by default but lsif-typescript takes the type of the import node.

Fixing the definition site is probably the best first-step towards improving the situation, I have no clue why that definition is getting emitted.

[varungandhi-src]

Minimal example:

import React from 'react'

import { StepN } from './Steps'

interface Step {
  content: React.ReactElement
}

This gives 2 identifiers as you'd expect:

this.lsifSymbol(decl) {
  role: 'UnspecifiedSymbolRole',
  'ident.range': [ 2, 9, 14 ],
  'decl.kind': 'ImportSpecifier',
  'decl.range': [ 2, 9, 14 ],
  lsifSymbol: LsifSymbol {
    value: 'lsif-typescript npm @types/react 17.0.0 `index.d.ts`/React/FunctionComponent#'
  }
}
this.lsifSymbol(decl) {
  role: 'Definition',
  'ident.range': [ 4, 10, 14 ],
  'decl.kind': 'InterfaceDeclaration',
  'decl.range': [ 4, 0, 6, 1 ],
  lsifSymbol: LsifSymbol {
    value: 'lsif-typescript npm react-example 1.0.0 src/`Shadow.tsx`/Step#'
  }
}

For some reason, when you change the imported identifier to Step (so the local definition is shadowing the import), you get 3 identifiers instead of 2:

this.lsifSymbol(decl) { // 1 -- sensible
  role: 'UnspecifiedSymbolRole',
  'ident.range': [ 2, 9, 13 ],
  'ident.parent.kind': 'ImportSpecifier',
  'ident.parent.range': [ 2, 9, 13 ],
  'decl.kind': 'ImportSpecifier',
  'decl.range': [ 2, 9, 13 ],
  lsifSymbol: LsifSymbol {
    value: 'lsif-typescript npm @types/react 17.0.0 `index.d.ts`/React/FunctionComponent#'
  }
}
this.lsifSymbol(decl) { // 2
  role: 'UnspecifiedSymbolRole',
  'ident.range': [ 2, 9, 13 ],
  'ident.parent.kind': 'ImportSpecifier',
  'ident.parent.range': [ 2, 9, 13 ],
  'decl.kind': 'InterfaceDeclaration', // wrong
  'decl.range': [ 4, 0, 6, 1 ],
  lsifSymbol: LsifSymbol {
    value: 'lsif-typescript npm react-example 1.0.0 src/`Shadow.tsx`/Step#'
  }
}
this.lsifSymbol(decl) { // 3
  role: 'Definition',
  'ident.range': [ 4, 10, 14 ],
  'ident.parent.kind': 'InterfaceDeclaration',
  'ident.parent.range': [ 4, 0, 6, 1 ],
  'decl.kind': 'ImportSpecifier', // wrong
  'decl.range': [ 2, 9, 13 ],
  lsifSymbol: LsifSymbol {
    value: 'lsif-typescript npm @types/react 17.0.0 `index.d.ts`/React/FunctionComponent#'
  }
}

[varungandhi-src]

Twitter discussion: it looks like both IntelliJ and VS Code have incorrect behavior for Go to Def when the same name is used for an interface and an imported const. It seems like the VS Code bug is a known issue.

[varungandhi-src]

Just to clarify/rephrase what was stated earlier, I think there are two related but independent bugs here:

1. The wrong symbol is being emitted for the definition. I've filed a separate issue for this: Incorrect definition emitted when an interface name collides with an imported const #68.
2. The Go to Def on the element is wrong (and seems to match what is in tsserver). "At call-site it references React/FunctionComponent#. I recall seeing similar behavior with tsserver via VS Code" -- This seems to be a known tsserver bug.