Skip to content

fix: Resolve the package of class/module symbols from the declaring file - #229

Open
anguslees wants to merge 1 commit into
sourcegraph:scipfrom
anguslees:fix-external-class-package
Open

anguslees wants to merge 1 commit into
sourcegraph:scipfrom
anguslees:fix-external-class-package

Conversation

@anguslees

Copy link
Copy Markdown

Problem

References to a class or module reached through an alias declaration (pkg.Thing,
or Thing re-exported from a submodule) are emitted under the project package
instead of the package that declares them. Functions are unaffected.

With requests 2.32.3 listed in --environment:

requests.get(...)                     requests 2.32.3 `requests.api`/get().             ok
except requests.exceptions.HTTPError  requests 2.32.3 `requests.exceptions`/HTTPError#  ok
requests.HTTPError                    <project> 0 `requests.exceptions`/HTTPError#      wrong
requests.Response                     <project> 0 `requests.models`/Response#           wrong
requests.Session                      <project> 0 `requests.sessions`/Session#          wrong

Over a random sample of 300 documents of a large project, 8% of occurrences whose
module belonged to a wheel listed in --environment carried the project package.
After this change it is 0.0%.

This breaks cross-repository navigation to the dependency's own index, folds every
dependency's classes into the project namespace, and members built from a cached
class symbol (Session#get().(url)) inherit the wrong package.

Cause

typeToSymbol resolves the package of class and module types by the file
containing the reference:

} else if (Types.isClass(typeObj)) {
    const pythonPackage = this.getPackageInfo(node, typeObj.details.moduleName)!;
    return Symbols.makeClass(pythonPackage, typeObj.details.moduleName, node.value);
...
} else if (Types.isModule(typeObj)) {
    const pythonPackage = this.getPackageInfo(node, typeObj.moduleName)!;
    return Symbols.makeModuleInit(pythonPackage, typeObj.moduleName);

getPackageInfo returns this.projectPackage whenever node's file is under the
project root, which is always the case while visiting a project file. The function
path already uses the declaring file and is correct:

private getFunctionSymbol(decl: FunctionDeclaration): ScipSymbol {
    let pythonPackage = this.guessPackage(decl.moduleName, decl.path);
    if (!pythonPackage) {
        return ScipSymbol.local(this.counter.next());
    }

The TypeVar-bound branch of getScipSymbol has the same pattern.

Fix

Use guessPackage with the type's own moduleName and filePath for classes,
modules and TypeVar bounds. When no package is found, fall back to a local symbol,
as getFunctionSymbol does.

Test

New snapshot external_class_import. A fake wheel widgets 1.2.3 lives in
snapshots/site-packages/ and is listed in testEnv.json; widgets/__init__.py
re-exports Widget and WidgetError from widgets/models.py. The test project
reaches it through extraPaths in its pyrightconfig.json, so its files stay
outside the project root and are classified as external. pythonVersion is pinned
to 3.11 because a config file otherwise makes pyright take the version from the
local interpreter, which would differ from the other snapshots.

import widgets
from widgets import Widget

def render(name: str, width: int) -> str:
    widget: widgets.Widget = widgets.Widget(name, width=width)
    try:
        return widgets.Widget.render(widget, prefix="> ")
    except widgets.WidgetError:
        return ""

Snapshot before and after the fix:

     widget: widgets.Widget = widgets.Widget(name, width=width)
-#                                    ^^^^^^ reference  snapshot-util 0.1 `widgets.models`/Widget#
+#                                    ^^^^^^ reference  widgets 1.2.3 `widgets.models`/Widget#
         return widgets.Widget.render(widget, prefix="> ")
-#                      ^^^^^^ reference  snapshot-util 0.1 `widgets.models`/Widget#
+#                      ^^^^^^ reference  widgets 1.2.3 `widgets.models`/Widget#
     except widgets.WidgetError:
-#                  ^^^^^^^^^^^ reference  snapshot-util 0.1 `widgets.models`/WidgetError#
+#                  ^^^^^^^^^^^ reference  widgets 1.2.3 `widgets.models`/WidgetError#

No existing snapshot changes. npm run build-agent, npm run check-snapshots,
npm test and npm run check:prettier all pass.

typeToSymbol resolved the package of a class or module type with
getPackageInfo(node, ...), which classifies by the file containing the
reference. Every reference emitted while visiting a project file is in
the project, so any class reached through an alias declaration
(`pkg.Thing`, or `Thing` re-exported from a submodule) was emitted under
the project package instead of the package that declares it. Functions
already used guessPackage(decl.moduleName, decl.path) and were correct.

Use guessPackage with the type's own moduleName and filePath for classes,
modules and TypeVar bounds, falling back to a local symbol when the
package is unknown, as getFunctionSymbol does.

Adds a snapshot test with a fake `widgets` package exposed through
extraPaths and listed in testEnv.json, so references to classes
re-exported from a submodule are checked against the dependency's
package.
@anguslees
anguslees force-pushed the fix-external-class-package branch from 1c06795 to d6175bb Compare October 14, 2026 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant