From 468008597e371ed4abac73ea2a14a08bbd16c7d1 Mon Sep 17 00:00:00 2001 From: Brendan Murphy Date: Tue, 8 Sep 2026 11:51:54 +0100 Subject: [PATCH] fix: emit missing symbol information --- .../snapshots/input/issue_223/consumer.py | 7 + .../snapshots/input/issue_223/contracts.py | 22 +++ .../snapshots/input/issue_223/pyproject.toml | 3 + .../snapshots/output/issue_223/consumer.py | 21 +++ .../snapshots/output/issue_223/contracts.py | 173 ++++++++++++++++++ packages/pyright-scip/src/test-runner.ts | 3 +- packages/pyright-scip/src/treeVisitor.ts | 19 +- packages/pyright-scip/test/test-main.ts | 36 ++++ 8 files changed, 274 insertions(+), 10 deletions(-) create mode 100644 packages/pyright-scip/snapshots/input/issue_223/consumer.py create mode 100644 packages/pyright-scip/snapshots/input/issue_223/contracts.py create mode 100644 packages/pyright-scip/snapshots/input/issue_223/pyproject.toml create mode 100644 packages/pyright-scip/snapshots/output/issue_223/consumer.py create mode 100644 packages/pyright-scip/snapshots/output/issue_223/contracts.py diff --git a/packages/pyright-scip/snapshots/input/issue_223/consumer.py b/packages/pyright-scip/snapshots/input/issue_223/consumer.py new file mode 100644 index 000000000..ca2e1d265 --- /dev/null +++ b/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) diff --git a/packages/pyright-scip/snapshots/input/issue_223/contracts.py b/packages/pyright-scip/snapshots/input/issue_223/contracts.py new file mode 100644 index 000000000..b8e41bb89 --- /dev/null +++ b/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) diff --git a/packages/pyright-scip/snapshots/input/issue_223/pyproject.toml b/packages/pyright-scip/snapshots/input/issue_223/pyproject.toml new file mode 100644 index 000000000..8c7a150ed --- /dev/null +++ b/packages/pyright-scip/snapshots/input/issue_223/pyproject.toml @@ -0,0 +1,3 @@ +[project] +name = "scip-final-repro" +version = "0.1.0" diff --git a/packages/pyright-scip/snapshots/output/issue_223/consumer.py b/packages/pyright-scip/snapshots/output/issue_223/consumer.py new file mode 100644 index 000000000..2fc675abf --- /dev/null +++ b/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) + diff --git a/packages/pyright-scip/snapshots/output/issue_223/contracts.py b/packages/pyright-scip/snapshots/output/issue_223/contracts.py new file mode 100644 index 000000000..5e161d964 --- /dev/null +++ b/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: +# > +# >     FeatureName = "\... +# >       &nbs... +# > +# > where, normally, OptionalRelease < Ma... +# > of the same form as sys.version\_info: +# > +# >     (PY\_MAJOR\_VERS... +# >      PY\_MINOR\... +# > PY\_MICRO\_VERSION, # the 0; an int +# > PY\_RELEASE\_LEVEL, # "alpha", "beta", "... +# > PY\_RELEASE\_SERIAL # the 3; an int +# >     ) +# > +# > OptionalRelease records the first releas... +# > +# >     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... +# > +# >     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# + diff --git a/packages/pyright-scip/src/test-runner.ts b/packages/pyright-scip/src/test-runner.ts index 2f8ffe27b..9e7c29c30 100644 --- a/packages/pyright-scip/src/test-runner.ts +++ b/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; } diff --git a/packages/pyright-scip/src/treeVisitor.ts b/packages/pyright-scip/src/treeVisitor.ts index 4ccbe804d..aabde4c06 100644 --- a/packages/pyright-scip/src/treeVisitor.ts +++ b/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,17 @@ export class TreeVisitor extends ParseTreeWalker { return this.config.pythonEnvironment.getPackageForModule(moduleName); } - private emitExternalSymbolInformation(node: ParseNode, symbol: ScipSymbol, documentation: string[]) { + private emitExternalSymbolInformation( + node: ParseNode, + symbol: ScipSymbol, + documentation: string[], + lookupDocumentation = true + ) { if (this.externalSymbols.has(symbol.value)) { return; } - if (documentation.length === 0) { + if (documentation.length === 0 && lookupDocumentation) { const nodeFileInfo = getFileInfo(node)!; const hoverResult = this.program.getHoverForPosition( nodeFileInfo.filePath, @@ -1450,10 +1455,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. diff --git a/packages/pyright-scip/test/test-main.ts b/packages/pyright-scip/test/test-main.ts index 5dbc97021..e093eb0d5 100644 --- a/packages/pyright-scip/test/test-main.ts +++ b/packages/pyright-scip/test/test-main.ts @@ -47,6 +47,36 @@ function validateOutputExists(outputDirectory: string, testName: string) { return null; } +function validateSymbolInformation(scipIndex: scip.Index, testName: string) { + const externalSymbols = new Set(scipIndex.external_symbols.map((symbol) => symbol.symbol)); + const missing = new Set(); + + for (const document of scipIndex.documents) { + const documentSymbols = new Set(document.symbols.map((symbol) => symbol.symbol)); + for (const occurrence of document.occurrences) { + const isEnclosingDefinition = + (occurrence.symbol_roles & scip.SymbolRole.Definition) > 0 && occurrence.enclosing_range.length > 0; + const isStdlib = occurrence.symbol.startsWith('scip-python python python-stdlib '); + if ( + (isEnclosingDefinition && !documentSymbols.has(occurrence.symbol)) || + (isStdlib && !externalSymbols.has(occurrence.symbol)) + ) { + missing.add(occurrence.symbol); + } + } + } + + if (missing.size > 0) { + return { + testName, + type: 'missing-symbol-information' as const, + message: `Missing SymbolInformation for: ${Array.from(missing).join(', ')}`, + }; + } + + return null; +} + function processSingleTest( testName: string, inputDirectory: string, @@ -116,6 +146,12 @@ function processSingleTest( return results; } + const symbolInformationFailure = validateSymbolInformation(scipIndex, testName); + if (symbolInformationFailure) { + results.failed.push(symbolInformationFailure); + return results; + } + if (options.mode === 'check') { const testOutputPath = path.join(outputDirectory, testName); if (!fs.existsSync(testOutputPath)) {