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

packages/pyright-scip/snapshots/output/issue_223/consumer.py

@@ -2,7 +2,7 @@
 
 from __future__ import annotations
 #    ^^^^^^^^^^ reference  python-stdlib 3.11 __future__/__init__:
-#                      ^^^^^^^^^^^ reference  python-stdlib 3.11 __future__/annotations.annotations.
+#                      ^^^^^^^^^^^ reference  python-stdlib 3.11 __future__/annotations.
 
 from contracts import Foo
 #    ^^^^^^^^^ reference  scip-final-repro 0.1.0 contracts/__init__:

packages/pyright-scip/snapshots/output/issue_223/contracts.py

@@ -57,7 +57,7 @@
 #                > #defines of CO\_xxx flags in Include/cpy...
 #                > 
 #                > No feature line is ever to be deleted fr...
-#                      ^^^^^^^^^^^ reference  python-stdlib 3.11 __future__/annotations.annotations.
+#                      ^^^^^^^^^^^ reference  python-stdlib 3.11 __future__/annotations.
 #                      external documentation ```python
 #                                  > (variable) annotations: _Feature
 #                                  > ```

packages/pyright-scip/src/treeVisitor.ts

@@ -266,28 +266,12 @@ export class TreeVisitor extends ParseTreeWalker {
     }
 
     override visitTypeAnnotation(node: TypeAnnotationNode): boolean {
-        // We are close to being able to look up a symbol, which could give us additional information here.
-        //  Perhaps we should be using this for additional information for any given name?
-        //  We can revisit this in visitName or perhaps when looking up the lsif symbol
-
-        // If we see a type annotation and we are currently inside of a class,
-        // that means that we are describing fields of a class (as far as I can tell),
-        // so we need to push a new symbol
-        const enclosingClass = ParseTreeUtils.getEnclosingClass(node, true);
-        if (enclosingClass) {
-            const hoverResult = this.program.getHoverForPosition(
-                this.fileInfo!.filePath,
-                convertOffsetToPosition(node.start, this.fileInfo!.lines),
-                'markdown',
-                _cancellationToken
-            );
-
-            this.document.symbols.push(
-                new scip.SymbolInformation({
-                    symbol: this.getScipSymbol(node).value,
-                    documentation: _formatHover(hoverResult!),
-                })
-            );
+        if (node.valueExpression.nodeType === ParseNodeType.Name) {
+            const name = node.valueExpression;
+            const declaration = this.evaluator.getDeclarationsForNameNode(name)?.[0];
+            if (declaration?.node) {
+                this.emitSymbolInformationOnce(name, this.getScipSymbol(declaration.node));
+            }
         }
 
         return true;
@@ -311,13 +295,7 @@ export class TreeVisitor extends ParseTreeWalker {
                         documentation.push('```python\n' + assignmentDoc.join('\n') + '\n```');
                     }
 
-                    // node.typeAnnotationComment
-                    this.document.symbols.push(
-                        new scip.SymbolInformation({
-                            symbol: this.getScipSymbol(dec.node).value,
-                            documentation,
-                        })
-                    );
+                    this.emitSymbolInformationOnce(dec.node, this.getScipSymbol(dec.node), documentation);
                 }
             }
         }
@@ -1161,15 +1139,15 @@ export class TreeVisitor extends ParseTreeWalker {
                     }
                 }
 
-                return Symbols.makeTerm(this.getScipSymbol(enclosingSuite || parent), (node as NameNode).value);
+                // An annotation describes the name; it does not introduce another scope.
+                const scope =
+                    enclosingSuite || (parent.nodeType === ParseNodeType.TypeAnnotation ? parent.parent! : parent);
+                return Symbols.makeTerm(this.getScipSymbol(scope), (node as NameNode).value);
             }
             case ParseNodeType.TypeAnnotation: {
                 switch (node.valueExpression.nodeType) {
                     case ParseNodeType.Name: {
-                        return Symbols.makeTerm(
-                            this.getScipSymbol(ParseTreeUtils.getEnclosingSuite(node) || node.parent!),
-                            node.valueExpression.value
-                        );
+                        return this.getScipSymbol(node.valueExpression);
                     }
                     default: {
                         softAssert(false, 'Unhandled type annotation');

packages/pyright-scip/test/test-main.ts

@@ -310,6 +310,103 @@ version = "16.05"`,
     }
 }
 
+function testAnnotatedVariables(): void {
+    // macOS temporary directories can be symlinked; match the analyzer's resolved file paths.
+    const projectRoot = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'scip-annotations-')));
+    const source = `initialized: int = 1
+uninitialized: int
+print(initialized, uninitialized)
+class Base:
+    field: int = 2
+    pending: int
+    def method(self):
+        method_local: int = 3
+        return method_local
+class Child(Base):
+    pass
+def function():
+    function_local: int = 4
+    return function_local
+obj = Child()
+obj.method()
+module_assigned = 5
+module_assigned: int
+def assigned_first():
+    local_assigned = 6
+    local_assigned: int
+    return local_assigned
+class AssignedFirst:
+    class_assigned = 7
+    class_assigned: int
+`;
+    try {
+        fs.writeFileSync(path.join(projectRoot, 'main.py'), source);
+        indexAction({
+            projectName: 'annotations',
+            projectVersion: '1.0',
+            cwd: projectRoot,
+            output: 'index.scip',
+            environment: path.resolve('snapshots/testEnv.json'),
+            infer: { projectVersionFromCommit: false },
+            quiet: true,
+            dev: false,
+            showProgressRateLimit: undefined,
+        });
+        const index = scip.Index.deserializeBinary(fs.readFileSync(path.join(projectRoot, 'index.scip')));
+        const document = index.documents.find((doc) => doc.relative_path === 'main.py')!;
+        for (const [line, expected] of [
+            [0, 'main/initialized.'],
+            [1, 'main/uninitialized.'],
+            [4, 'main/Base#field.'],
+            [5, 'main/Base#pending.'],
+            [7, 'local '],
+            [12, 'local '],
+            [16, 'main/module_assigned.'],
+            [19, 'local '],
+            [23, 'main/AssignedFirst#class_assigned.'],
+        ] as const) {
+            const definition = document.occurrences.find(
+                (occurrence) =>
+                    occurrence.range[0] === line &&
+                    occurrence.range.length === 3 &&
+                    occurrence.range[2] > occurrence.range[1] &&
+                    (occurrence.symbol_roles & scip.SymbolRole.Definition) !== 0
+            )!;
+            assert.ok(definition, `Missing definition on line ${line}`);
+            assert.ok(
+                expected === 'local ' ? definition.symbol.startsWith(expected) : definition.symbol.endsWith(expected),
+                definition.symbol
+            );
+            const information = document.symbols.filter((symbol) => symbol.symbol === definition.symbol);
+            assert.strictEqual(information.length, 1, `Expected one information record for ${definition.symbol}`);
+            assert.ok(information[0].documentation.length > 0);
+            if ([0, 1, 7, 12].includes(line)) {
+                assert.ok(
+                    document.occurrences.some(
+                        (occurrence) =>
+                            occurrence.symbol === definition.symbol &&
+                            occurrence.symbol_roles === scip.SymbolRole.ReadAccess
+                    )
+                );
+            }
+        }
+        assert.ok(
+            document.occurrences.some(
+                (occurrence) => occurrence.range[0] === 15 && occurrence.symbol.endsWith('main/Base#method().')
+            )
+        );
+        assert.ok(!document.occurrences.some((occurrence) => occurrence.symbol.endsWith('main/Child#method().')));
+        for (const symbol of document.symbols.filter((symbol) => symbol.symbol.startsWith('local '))) {
+            assert.ok(
+                document.occurrences.some((occurrence) => occurrence.symbol === symbol.symbol),
+                `Orphan local information: ${symbol.symbol}`
+            );
+        }
+    } finally {
+        fs.rmSync(projectRoot, { recursive: true, force: true });
+    }
+}
+
 function testImportRoles(): void {
     // macOS temporary directories can be symlinked; match the analyzer's resolved file paths.
     const projectRoot = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'scip-import-roles-')));
@@ -542,6 +639,7 @@ function testMissingProjectVersion(): void {
 
 function unitTests(): void {
     testPyprojectParsing();
+    testAnnotatedVariables();
     testImportRoles();
     testExternalDocumentation();
     testPyprojectLineEndings();