Fix bug where an interface definition had a conflicting symbol with a const in the same file (#206)

* Reproduce issue with conflicting const/interface

* Fix the bug

Verified on https://sourcegraph.com/github.com/microsoft/vscode@9984da1/-/blob/src/vs/workbench/services/workspaces/common/workspaceEditing.ts?L13:18#tab=implementations_typescript

* Document why we have commented out code

* Add comment explaining why we avoid ambiguous results at definitions

Author: olafurpg

Development.md

@@ -20,6 +20,12 @@ npm run update-snapshots
 
 Generate snapshots and update.
 
+## Skipping files/test for local development
+
+Search for the query `"Uncomment below if you want to skip` to find places where
+you can uncomment code to skip tests/files for a faster edit/test/debug feedback
+loop during local development.
+
 ## Snapshotting arbitrary projects
 
 ```sh

snapshots/input/syntax/src/conflicting-const-interface.ts

@@ -0,0 +1,3 @@
+export const ConflictingConst = 42
+export interface ConflictingConst {}
+export class ImplementsConflictingConst implements ConflictingConst {}

snapshots/output/pure-js/src/main.js

@@ -106,7 +106,7 @@
     if (forever()) {
 //      ^^^^^^^ reference pure-js 1.0.0 src/`main.js`/forever().
       var k = 1
-//        ^ definition local 14
+//        ^ definition local 17
 //        documentation ```ts\nvar k: number\n```
     }
     print_fib(k)

snapshots/output/syntax/src/accessors.ts

@@ -13,7 +13,7 @@
 //                ^^^^^^^ reference syntax 1.0.0 src/`accessors.ts`/C#_length.
     }
     set length(value: number) {
-//      ^^^^^^ definition syntax 1.0.0 src/`accessors.ts`/C#`<get>length`().
+//      ^^^^^^ definition syntax 1.0.0 src/`accessors.ts`/C#`<set>length`().
 //      documentation ```ts\nget length: number\n```
 //             ^^^^^ definition syntax 1.0.0 src/`accessors.ts`/C#`<set>length`().(value)
 //             documentation ```ts\n(parameter) value: number\n```
@@ -46,7 +46,7 @@
 //                ^^^^^^^ reference syntax 1.0.0 src/`accessors.ts`/D#_length.
     }
     public set length(value: number) {
-//             ^^^^^^ definition syntax 1.0.0 src/`accessors.ts`/D#`<get>length`().
+//             ^^^^^^ definition syntax 1.0.0 src/`accessors.ts`/D#`<set>length`().
 //             documentation ```ts\nget length: number\n```
 //                    ^^^^^ definition syntax 1.0.0 src/`accessors.ts`/D#`<set>length`().(value)
 //                    documentation ```ts\n(parameter) value: number\n```
@@ -65,7 +65,7 @@
 //                ^^^^^^^^^ reference syntax 1.0.0 src/`accessors.ts`/D#_capacity.
     }
     private set capacity(value: number) {
-//              ^^^^^^^^ definition syntax 1.0.0 src/`accessors.ts`/D#`<get>capacity`().
+//              ^^^^^^^^ definition syntax 1.0.0 src/`accessors.ts`/D#`<set>capacity`().
 //              documentation ```ts\nget capacity: number\n```
 //                       ^^^^^ definition syntax 1.0.0 src/`accessors.ts`/D#`<set>capacity`().(value)
 //                       documentation ```ts\n(parameter) value: number\n```

snapshots/output/syntax/src/conflicting-const-interface.ts

@@ -0,0 +1,15 @@
+  export const ConflictingConst = 42
+// definition syntax 1.0.0 src/`conflicting-const-interface.ts`/
+//documentation ```ts\nmodule "conflicting-const-interface.ts"\n```
+//             ^^^^^^^^^^^^^^^^ definition syntax 1.0.0 src/`conflicting-const-interface.ts`/ConflictingConst.
+//             documentation ```ts\ninterface ConflictingConst\n```
+  export interface ConflictingConst {}
+//                 ^^^^^^^^^^^^^^^^ definition syntax 1.0.0 src/`conflicting-const-interface.ts`/ConflictingConst#
+//                 documentation ```ts\ninterface ConflictingConst\n```
+  export class ImplementsConflictingConst implements ConflictingConst {}
+//             ^^^^^^^^^^^^^^^^^^^^^^^^^^ definition syntax 1.0.0 src/`conflicting-const-interface.ts`/ImplementsConflictingConst#
+//             documentation ```ts\nclass ImplementsConflictingConst\n```
+//             relationship implementation scip-typescript npm syntax 1.0.0 src/`conflicting-const-interface.ts`/ConflictingConst#
+//                                                   ^^^^^^^^^^^^^^^^ reference syntax 1.0.0 src/`conflicting-const-interface.ts`/ConflictingConst.
+//                                                   ^^^^^^^^^^^^^^^^ reference syntax 1.0.0 src/`conflicting-const-interface.ts`/ConflictingConst#
+  

src/FileIndexer.ts

@@ -37,6 +37,10 @@ export class FileIndexer {
     this.workingDirectoryRegExp = new RegExp(options.cwd, 'g')
   }
   public index(): void {
+    // Uncomment below if you want to skip certain files for local development.
+    // if (!this.sourceFile.fileName.includes('conflicting-const')) {
+    //   return
+    // }
     this.emitSourceFileOccurrence()
     this.visit(this.sourceFile)
   }
@@ -104,7 +108,17 @@ export class FileIndexer {
     if (isDefinitionNode) {
       role |= scip.scip.SymbolRole.Definition
     }
-    for (const declaration of sym?.declarations || []) {
+    const declarations = isDefinitionNode
+      ? // Don't emit ambiguous definition at definition-site. You can reproduce
+        // ambiguous results by triggering "Go to definition" in VS Code on `Conflict`
+        // in the example below:
+        // export const Conflict = 42
+        // export interface Conflict {}
+        //                  ^^^^^^^^ "Go to definition" shows two results: const and interface.
+        // See https://github.com/sourcegraph/scip-typescript/pull/206 for more details.
+        [node.parent]
+      : sym?.declarations || []
+    for (const declaration of declarations) {
       const scipSymbol = this.scipSymbol(declaration)
 
       if (scipSymbol.isEmpty()) {

src/main.test.ts

@@ -33,6 +33,10 @@ interface PackageJson {
   workspaces: string[]
 }
 for (const snapshotDirectory of snapshotDirectories) {
+  // Uncomment below if you want to skip certain tests for local development.
+  // if (!snapshotDirectory.includes('syntax')) {
+  //   continue
+  // }
   const inputRoot = join(inputDirectory, snapshotDirectory)
   const outputRoot = join(outputDirectory, snapshotDirectory)
   if (!fs.statSync(inputRoot).isDirectory()) {