fix: emit missing symbol information (#226)

* fix: emit missing symbol information

* test: resolve temporary fixture paths on macOS

Author: brendan-m-murphy

.github/workflows/scip-snapshot.yml

@@ -42,5 +42,6 @@ jobs:
           fi
       - run: npm install
       - run: cd ./packages/pyright-scip/ && npm install && npm run build
+      - run: cd ./packages/pyright-scip/ && npm test -- --runInBand
       - run: python --version
       - run: cd ./packages/pyright-scip/ && npm run check-snapshots

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

@@ -0,0 +1,7 @@
+from __future__ import annotations
+
+from contracts import Foo
+
+
+def make_foo(x: float) -> Foo:
+    return Foo(x=x)

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

@@ -0,0 +1,22 @@
+# format-options: showDocs,showRanges
+
+from __future__ import annotations
+
+from dataclasses import dataclass
+from numbers import Real
+import math
+
+
+def _validate(value: object) -> None:
+    if not isinstance(value, Foo):
+        raise ValueError("must be Foo")
+    if not isinstance(value.x, Real) or not math.isfinite(value.x):
+        raise ValueError("x must be finite real")
+
+
+@dataclass(frozen=True)
+class Foo:
+    x: float
+
+    def __post_init__(self) -> None:
+        _validate(self)

packages/pyright-scip/snapshots/input/issue_223/pyproject.toml

@@ -0,0 +1,3 @@
+[project]
+name = "scip-final-repro"
+version = "0.1.0"

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

@@ -0,0 +1,21 @@
+# < definition scip-python python scip-final-repro 0.1.0 consumer/__init__:
+
+from __future__ import annotations
+#    ^^^^^^^^^^ reference  python-stdlib 3.11 __future__/__init__:
+#                      ^^^^^^^^^^^ reference  python-stdlib 3.11 __future__/annotations.annotations.
+
+from contracts import Foo
+#    ^^^^^^^^^ reference  scip-final-repro 0.1.0 contracts/__init__:
+#                     ^^^ reference  scip-final-repro 0.1.0 contracts/Foo#
+
+
+def make_foo(x: float) -> Foo:
+#   ^^^^^^^^ definition  scip-final-repro 0.1.0 consumer/make_foo().
+#            ^ definition  scip-final-repro 0.1.0 consumer/make_foo().(x)
+#               ^^^^^ reference  python-stdlib 3.11 builtins/float#
+#                         ^^^ reference  scip-final-repro 0.1.0 contracts/Foo#
+    return Foo(x=x)
+#          ^^^ reference  scip-final-repro 0.1.0 contracts/Foo#
+#              ^ reference  scip-final-repro 0.1.0 contracts/Foo#x.
+#                ^ reference  scip-final-repro 0.1.0 consumer/make_foo().(x)
+

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

@@ -0,0 +1,173 @@
+# < definition scip-python python scip-final-repro 0.1.0 contracts/__init__:
+#documentation (module) contracts
+
+# format-options: showDocs,showRanges
+
+from __future__ import annotations
+#    ^^^^^^^^^^ reference  python-stdlib 3.11 __future__/__init__:
+#    external documentation ```python
+#                > (module) __future__
+#                > ```
+#    external documentation ---
+#                > 
+#    external documentation Record of phased-in incompatible languag...
+#                > 
+#                > Each line is of the form:
+#                > 
+#                > &nbsp;&nbsp;&nbsp;&nbsp;FeatureName = "\...
+#                > &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbs...
+#                > 
+#                > where, normally, OptionalRelease &lt; Ma...
+#                > of the same form as sys.version\_info:
+#                > 
+#                > &nbsp;&nbsp;&nbsp;&nbsp;(PY\_MAJOR\_VERS...
+#                > &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;PY\_MINOR\...
+#                > PY\_MICRO\_VERSION, # the 0; an int
+#                > PY\_RELEASE\_LEVEL, # "alpha", "beta", "...
+#                > PY\_RELEASE\_SERIAL # the 3; an int  
+#                > &nbsp;&nbsp;&nbsp;&nbsp;)
+#                > 
+#                > OptionalRelease records the first releas...
+#                > 
+#                > &nbsp;&nbsp;&nbsp;&nbsp;from \_\_future\...
+#                > 
+#                > was accepted.
+#                > 
+#                > In the case of MandatoryReleases that ha...
+#                > MandatoryRelease predicts the release in...
+#                > of the language.
+#                > 
+#                > Else MandatoryRelease records when the f...
+#                > in releases at or after that, modules no...
+#                > 
+#                > &nbsp;&nbsp;&nbsp;&nbsp;from \_\_future\...
+#                > 
+#                > to use the feature in question, but may ...
+#                > 
+#                > MandatoryRelease may also be None, meani...
+#                > dropped.
+#                > 
+#                > Instances of class \_Feature have two co...
+#                > .getOptionalRelease() and .getMandatoryR...
+#                > 
+#                > CompilerFlag is the (bitfield) flag that...
+#                > argument to the builtin function compile...
+#                > dynamically compiled code.  This flag is...
+#                > attribute on \_Future instances.  These ...
+#                > #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.
+#                      external documentation ```python
+#                                  > (variable) annotations: _Feature
+#                                  > ```
+
+from dataclasses import dataclass
+#    ^^^^^^^^^^^ reference  python-stdlib 3.11 dataclasses/__init__:
+#    external documentation ```python
+#                > (module) dataclasses
+#                > ```
+#                       ^^^^^^^^^ reference  python-stdlib 3.11 dataclasses/dataclass().
+from numbers import Real
+#    ^^^^^^^ reference  python-stdlib 3.11 numbers/__init__:
+#    external documentation ```python
+#                > (module) numbers
+#                > ```
+#    external documentation ---
+#                > 
+#    external documentation Abstract Base Classes (ABCs) for numbers...
+#                > 
+#                > TODO: Fill out more detailed documentati...
+#                   ^^^^ reference  python-stdlib 3.11 numbers/Real#
+import math
+#      ^^^^ reference  python-stdlib 3.11 math/__init__:
+
+
+# < start enclosing_range scip-python python scip-final-repro 0.1.0 contracts/_validate().
+def _validate(value: object) -> None:
+#   ^^^^^^^^^ definition  scip-final-repro 0.1.0 contracts/_validate().
+#   documentation ```python
+#               > def _validate(
+#               >   value: object
+#               > ) -> None:
+#               > ```
+#             ^^^^^ definition  scip-final-repro 0.1.0 contracts/_validate().(value)
+#                    ^^^^^^ reference  python-stdlib 3.11 builtins/object#
+#                    external documentation ```python
+#                                > (class) object
+#                                > ```
+    if not isinstance(value, Foo):
+#          ^^^^^^^^^^ reference local 0
+#          external documentation ```python
+#                      > (function) def isinstance(
+#                      >     __obj: object,
+#                      >     __class_or_tuple: _ClassInfo,
+#                      >     /
+#                      > ) -> bool
+#                      > ```
+#                     ^^^^^ reference  scip-final-repro 0.1.0 contracts/_validate().(value)
+#                            ^^^ reference  scip-final-repro 0.1.0 contracts/Foo#
+        raise ValueError("must be Foo")
+#             ^^^^^^^^^^ reference  python-stdlib 3.11 builtins/ValueError#
+#             external documentation ```python
+#                         > class ValueError(*args: object)
+#                         > ```
+    if not isinstance(value.x, Real) or not math.isfinite(value.x):
+#          ^^^^^^^^^^ reference local 1
+#          external documentation ```python
+#                      > (function) def isinstance(
+#                      >     __obj: object,
+#                      >     __class_or_tuple: _ClassInfo,
+#                      >     /
+#                      > ) -> bool
+#                      > ```
+#                     ^^^^^ reference  scip-final-repro 0.1.0 contracts/_validate().(value)
+#                           ^ reference  scip-final-repro 0.1.0 contracts/Foo#x.
+#                              ^^^^ reference  python-stdlib 3.11 numbers/Real#
+#                                           ^^^^ reference  python-stdlib 3.11 math/__init__:
+#                                                ^^^^^^^^ reference  python-stdlib 3.11 math/isfinite().
+#                                                         ^^^^^ reference  scip-final-repro 0.1.0 contracts/_validate().(value)
+#                                                               ^ reference  scip-final-repro 0.1.0 contracts/Foo#x.
+        raise ValueError("x must be finite real")
+#             ^^^^^^^^^^ reference  python-stdlib 3.11 builtins/ValueError#
+# < end enclosing_range scip-python python scip-final-repro 0.1.0 contracts/_validate().
+
+
+# < start enclosing_range scip-python python scip-final-repro 0.1.0 contracts/Foo#
+@dataclass(frozen=True)
+#^^^^^^^^^ reference  python-stdlib 3.11 dataclasses/dataclass().
+#          ^^^^^^ reference  python-stdlib 3.11 dataclasses/dataclass().(frozen)
+#          external documentation ```python
+#                      > (parameter) frozen: bool
+#                      > ```
+class Foo:
+#     ^^^ definition  scip-final-repro 0.1.0 contracts/Foo#
+#     documentation ```python
+#                 > @dataclass(frozen=True)
+#                 > class Foo:
+#                 > ```
+    x: float
+#   ^ definition  scip-final-repro 0.1.0 contracts/Foo#x.
+#   documentation ```python
+#               > (variable) x: float
+#               > ```
+#      ^^^^^ reference  python-stdlib 3.11 builtins/float#
+#      external documentation ```python
+#                  > (class) float
+#                  > ```
+
+#   ⌄ start enclosing_range scip-python python scip-final-repro 0.1.0 contracts/Foo#__post_init__().
+    def __post_init__(self) -> None:
+#       ^^^^^^^^^^^^^ definition  scip-final-repro 0.1.0 contracts/Foo#__post_init__().
+#       documentation ```python
+#                   > def __post_init__(
+#                   >   self
+#                   > ) -> None:
+#                   > ```
+#                     ^^^^ definition  scip-final-repro 0.1.0 contracts/Foo#__post_init__().(self)
+        _validate(self)
+#       ^^^^^^^^^ reference  scip-final-repro 0.1.0 contracts/_validate().
+#                 ^^^^ reference  scip-final-repro 0.1.0 contracts/Foo#__post_init__().(self)
+#   ^ end enclosing_range scip-python python scip-final-repro 0.1.0 contracts/Foo#__post_init__().
+# < end enclosing_range scip-python python scip-final-repro 0.1.0 contracts/Foo#
+

packages/pyright-scip/src/test-runner.ts

@@ -11,7 +11,8 @@ export interface TestFailure {
         | 'content-mismatch'
         | 'orphaned-output'
         | 'caught-exception'
-        | 'sometimes-assertion';
+        | 'sometimes-assertion'
+        | 'missing-symbol-information';
     message: string;
 }
 

packages/pyright-scip/src/treeVisitor.ts

@@ -508,6 +508,7 @@ export class TreeVisitor extends ParseTreeWalker {
             if (pythonPackage) {
                 const symbol = Symbols.makeModuleInit(pythonPackage, moduleName);
                 this.pushNewOccurrence(node.module, symbol);
+                this.emitExternalSymbolInformation(node.module, symbol, [], false);
             } else {
                 // For python packages & modules that we cannot resolve,
                 // we'll just make a local for the file and note that we could not resolve this module.
@@ -566,8 +567,9 @@ export class TreeVisitor extends ParseTreeWalker {
             return this.emitDeclarationWithoutNode(node, decl);
         }
 
+        const isDefinition = decl.node.id === parent.id;
         const existingSymbol = this.rawGetLsifSymbol(decl.node);
-        if (existingSymbol) {
+        if (existingSymbol && !(isDefinition && decl.node.nodeType === ParseNodeType.Class)) {
             if (decl.node.id === parent.id || decl.node.id === node.id) {
                 switch (decl.node.nodeType) {
                     case ParseNodeType.Function:
@@ -583,8 +585,6 @@ export class TreeVisitor extends ParseTreeWalker {
             return true;
         }
 
-        const isDefinition = decl.node.id === parent.id;
-
         const builtinType = this.evaluator.getBuiltInType(node, node.value);
         if (this.isStdlib(decl, builtinType)) {
             this.emitBuiltinScipSymbol(node, builtinType, decl);
@@ -1431,12 +1431,18 @@ export class TreeVisitor extends ParseTreeWalker {
         return this.config.pythonEnvironment.getPackageForModule(moduleName);
     }
 
-    private emitExternalSymbolInformation(node: ParseNode, symbol: ScipSymbol, documentation: string[]) {
-        if (this.externalSymbols.has(symbol.value)) {
+    private emitExternalSymbolInformation(
+        node: ParseNode,
+        symbol: ScipSymbol,
+        documentation: string[],
+        lookupDocumentation = true
+    ) {
+        const existing = this.externalSymbols.get(symbol.value);
+        if (existing && existing.documentation.length > 0) {
             return;
         }
 
-        if (documentation.length === 0) {
+        if (documentation.length === 0 && lookupDocumentation) {
             const nodeFileInfo = getFileInfo(node)!;
             const hoverResult = this.program.getHoverForPosition(
                 nodeFileInfo.filePath,
@@ -1450,10 +1456,6 @@ export class TreeVisitor extends ParseTreeWalker {
             }
         }
 
-        if (documentation.length === 0) {
-            return;
-        }
-
         // TODO: Could consider adding the documentation finder stuff
         // from emitSymbolInformationOnce, but at this point we don't
         // need that.