diff --git a/graph/python/engine/expression-resolution/expr-type.dl b/graph/python/engine/expression-resolution/expr-type.dl index 8385f7830..30df9f4de 100644 --- a/graph/python/engine/expression-resolution/expr-type.dl +++ b/graph/python/engine/expression-resolution/expr-type.dl @@ -430,6 +430,19 @@ binding_value_type(p, b, t) :- assign_pair(p, tgt, val), expr_type(p, val, t). +// ── AN IMPORTED MODULE-LEVEL VALUE IS THE BINDING IT NAMES (#1140) ─────────── +// `from order_service import order_service` resolves to the exporting module's +// own binding row (import_resolved_target kind VARIABLE), and that binding's +// value type is derived above from its assignment. Without this clause the +// type stopped at the module boundary: the same `order_service.cancel()` was a +// known edge next to the assignment and a name match one import away. The +// write-count trade-offs are inherited, not re-decided -- a multi-write export +// arrives as the union and the tier follows from the target count as usual. +binding_value_type(p, b, t) :- + import_binding(p, b, i), + import_resolved_target(p, "VARIABLE", b2, i), + binding_value_type(p, b2, t). + // ── A CONDITIONAL EXPRESSION IS EITHER BRANCH ──────────────────────────────── // `w = HtmlWriter() if flag else PlainWriter()`. CONDITIONAL_EXPRESSION was present in // the IR with BODY / CONDITION / ORELSE children and NO rule read it, so a ternary had diff --git a/graph/test/python/cases/43-module-singleton-import/src/api.py b/graph/test/python/cases/43-module-singleton-import/src/api.py new file mode 100644 index 000000000..a169f808f --- /dev/null +++ b/graph/test/python/cases/43-module-singleton-import/src/api.py @@ -0,0 +1,7 @@ +"""Imports the singleton whose name collides with its module's last segment.""" + +from order_service import order_service + + +def cancel_endpoint(oid): + return order_service.cancel(oid) diff --git a/graph/test/python/cases/43-module-singleton-import/src/order_service.py b/graph/test/python/cases/43-module-singleton-import/src/order_service.py new file mode 100644 index 000000000..a3ba14336 --- /dev/null +++ b/graph/test/python/cases/43-module-singleton-import/src/order_service.py @@ -0,0 +1,20 @@ +"""A module-level singleton named after its own module — the ordinary idiom. + +`from order_service import order_service` must bind the VALUE, not suffix-match +back to this module and report a module import (#1140). +""" + + +class OrderService: + def cancel(self, oid): + return oid + + def refund(self, oid): + return oid + + +order_service = OrderService() + + +def same_module_caller(oid): + return order_service.cancel(oid) diff --git a/graph/test/python/cases/43-module-singleton-import/src/pkg/__init__.py b/graph/test/python/cases/43-module-singleton-import/src/pkg/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/graph/test/python/cases/43-module-singleton-import/src/pkg/tools.py b/graph/test/python/cases/43-module-singleton-import/src/pkg/tools.py new file mode 100644 index 000000000..a02ce3d5e --- /dev/null +++ b/graph/test/python/cases/43-module-singleton-import/src/pkg/tools.py @@ -0,0 +1,5 @@ +"""Control: a GENUINE submodule import must keep resolving as a module.""" + + +def helper(): + return 1 diff --git a/graph/test/python/cases/43-module-singleton-import/src/publisher.py b/graph/test/python/cases/43-module-singleton-import/src/publisher.py new file mode 100644 index 000000000..4466e43e1 --- /dev/null +++ b/graph/test/python/cases/43-module-singleton-import/src/publisher.py @@ -0,0 +1,5 @@ +from signals import order_placed + + +def publish(sender): + return order_placed.send(sender) diff --git a/graph/test/python/cases/43-module-singleton-import/src/signals.py b/graph/test/python/cases/43-module-singleton-import/src/signals.py new file mode 100644 index 000000000..b5bdb0e8b --- /dev/null +++ b/graph/test/python/cases/43-module-singleton-import/src/signals.py @@ -0,0 +1,9 @@ +"""A module-level value whose name does NOT collide with the module name.""" + + +class Signal: + def send(self, sender): + return sender + + +order_placed = Signal() diff --git a/graph/test/python/cases/43-module-singleton-import/src/use_pkg.py b/graph/test/python/cases/43-module-singleton-import/src/use_pkg.py new file mode 100644 index 000000000..027d3e320 --- /dev/null +++ b/graph/test/python/cases/43-module-singleton-import/src/use_pkg.py @@ -0,0 +1,5 @@ +from pkg import tools + + +def run(): + return tools.helper() diff --git a/graph/test/python/expected/21-url-and-signal-dispatch.edges b/graph/test/python/expected/21-url-and-signal-dispatch.edges index 63aeda156..9d07c714d 100644 --- a/graph/test/python/expected/21-url-and-signal-dispatch.edges +++ b/graph/test/python/expected/21-url-and-signal-dispatch.edges @@ -1,13 +1,13 @@ ambiguous_unknown DECORATOR_APPLICATION handlers. -> - ambiguous_unknown DECORATOR_APPLICATION main. -> - ambiguous_unknown METHOD_CALL main.not_a_signal_send -> - -ambiguous_unknown METHOD_CALL ship.ship_order -> - boundary_lib SIMPLE_CALL main. -> builtin:object.__init__ boundary_lib SIMPLE_CALL signals. -> builtin:object.__init__ known_edge DECORATOR_CALL handlers. -> handlers.receiver known_edge DECORATOR_CALL main. -> main.receiver known_edge METHOD_CALL main. -> main.Signal.connect known_edge METHOD_CALL main.place_order -> main.Signal.send +known_edge METHOD_CALL ship.ship_order -> signals.Signal.send known_edge SIMPLE_CALL handlers.on_never_shipped -> handlers._record known_edge SIMPLE_CALL handlers.on_shipped -> handlers._record known_edge SIMPLE_CALL main. -> main.path diff --git a/graph/test/python/expected/21-url-and-signal-dispatch.tiers b/graph/test/python/expected/21-url-and-signal-dispatch.tiers index dff4f56f8..56e700521 100644 --- a/graph/test/python/expected/21-url-and-signal-dispatch.tiers +++ b/graph/test/python/expected/21-url-and-signal-dispatch.tiers @@ -1,9 +1,9 @@ distinct call sites emitted: 32 --- by tier: edge ROWS, and the distinct SITES they cover --- - 6 rows 6 sites ambiguous_unknown + 5 rows 5 sites ambiguous_unknown 4 rows 4 sites boundary_lib - 22 rows 22 sites known_edge + 23 rows 23 sites known_edge --- edge rows by call kind --- 4 DECORATOR_APPLICATION @@ -13,14 +13,13 @@ distinct call sites emitted: 32 --- unresolved reasons --- 4 decorator_factory_result_untyped - 1 untyped_receiver:local_untyped 1 untyped_receiver:parameter --- the engine's own conservation ledger --- 32 _total_sites - 6 ambiguous_unknown + 5 ambiguous_unknown 4 boundary_lib - 22 known_edge + 23 known_edge --- reconciling rows against the conserved site count --- edge rows 32 diff --git a/graph/test/python/expected/42-signal-forms.edges b/graph/test/python/expected/42-signal-forms.edges index c42f783d5..456f0118c 100644 --- a/graph/test/python/expected/42-signal-forms.edges +++ b/graph/test/python/expected/42-signal-forms.edges @@ -1,11 +1,11 @@ ambiguous_unknown DECORATOR_APPLICATION handlers. -> - ambiguous_unknown METHOD_CALL handlers. -> - ambiguous_unknown METHOD_CALL orders.close -> - -ambiguous_unknown METHOD_CALL orders.pay -> - -ambiguous_unknown METHOD_CALL orders.place -> - ambiguous_unknown METHOD_CALL orders.ship -> - -ambiguous_unknown METHOD_CALL orders.void -> - boundary_lib DECORATOR_CALL handlers. -> external:receiver boundary_lib SIMPLE_CALL signals. -> builtin:object.__init__ known_edge DECORATOR_CALL handlers. -> handlers.remember known_edge METHOD_CALL orders.not_a_signal -> orders.Outbox.asend +known_edge METHOD_CALL orders.pay -> signals.Signal.asend +known_edge METHOD_CALL orders.place -> signals.Signal.send +known_edge METHOD_CALL orders.void -> signals.Signal.send diff --git a/graph/test/python/expected/42-signal-forms.tiers b/graph/test/python/expected/42-signal-forms.tiers index e28d1d9d7..e780b18a8 100644 --- a/graph/test/python/expected/42-signal-forms.tiers +++ b/graph/test/python/expected/42-signal-forms.tiers @@ -1,9 +1,9 @@ distinct call sites emitted: 22 --- by tier: edge ROWS, and the distinct SITES they cover --- - 11 rows 11 sites ambiguous_unknown + 8 rows 8 sites ambiguous_unknown 9 rows 9 sites boundary_lib - 2 rows 2 sites known_edge + 5 rows 5 sites known_edge --- edge rows by call kind --- 5 DECORATOR_APPLICATION @@ -14,13 +14,12 @@ distinct call sites emitted: 22 --- unresolved reasons --- 5 decorator_factory_result_untyped 3 untyped_receiver:attribute_object_untyped - 3 untyped_receiver:local_untyped --- the engine's own conservation ledger --- 22 _total_sites - 11 ambiguous_unknown + 8 ambiguous_unknown 9 boundary_lib - 2 known_edge + 5 known_edge --- reconciling rows against the conserved site count --- edge rows 22 diff --git a/graph/test/python/expected/43-module-singleton-import.edges b/graph/test/python/expected/43-module-singleton-import.edges new file mode 100644 index 000000000..c35a14c76 --- /dev/null +++ b/graph/test/python/expected/43-module-singleton-import.edges @@ -0,0 +1,6 @@ +boundary_lib SIMPLE_CALL order_service. -> builtin:object.__init__ +boundary_lib SIMPLE_CALL signals. -> builtin:object.__init__ +known_edge METHOD_CALL api.cancel_endpoint -> order_service.OrderService.cancel +known_edge METHOD_CALL order_service.same_module_caller -> order_service.OrderService.cancel +known_edge METHOD_CALL publisher.publish -> signals.Signal.send +known_edge METHOD_CALL use_pkg.run -> pkg.tools.helper diff --git a/graph/test/python/expected/43-module-singleton-import.entries b/graph/test/python/expected/43-module-singleton-import.entries new file mode 100644 index 000000000..276e39fa5 --- /dev/null +++ b/graph/test/python/expected/43-module-singleton-import.entries @@ -0,0 +1 @@ +── entry_point (0) ── diff --git a/graph/test/python/expected/43-module-singleton-import.framework b/graph/test/python/expected/43-module-singleton-import.framework new file mode 100644 index 000000000..29577b757 --- /dev/null +++ b/graph/test/python/expected/43-module-singleton-import.framework @@ -0,0 +1,6 @@ +── framework_edge (0) ── +── framework_unjoined (1) ── + 1 signal_dispatch no_receiver +── remote_edge (0) ── +── remote_unserved (0) ── +── remote_unsent (0) ── diff --git a/graph/test/python/expected/43-module-singleton-import.tiers b/graph/test/python/expected/43-module-singleton-import.tiers new file mode 100644 index 000000000..3f658c74f --- /dev/null +++ b/graph/test/python/expected/43-module-singleton-import.tiers @@ -0,0 +1,23 @@ +distinct call sites emitted: 6 + +--- by tier: edge ROWS, and the distinct SITES they cover --- + 2 rows 2 sites boundary_lib + 4 rows 4 sites known_edge + +--- edge rows by call kind --- + 4 METHOD_CALL + 2 SIMPLE_CALL + +--- unresolved reasons --- + (none — every site resolved) + +--- the engine's own conservation ledger --- + 6 _total_sites + 2 boundary_lib + 4 known_edge + +--- reconciling rows against the conserved site count --- + edge rows 6 + minus extra rows from multi-target sites 0 + = tier/site pairs 6 + engine's conserved site total 6 diff --git a/parser/src/parsers/python/extractors/python-resolution-linker.ts b/parser/src/parsers/python/extractors/python-resolution-linker.ts index 798ac85b1..0d5c54857 100644 --- a/parser/src/parsers/python/extractors/python-resolution-linker.ts +++ b/parser/src/parsers/python/extractors/python-resolution-linker.ts @@ -283,12 +283,27 @@ export class PythonResolutionLinker { ? undefined : exportsByModule.get(targetModule.qualifiedName)?.get(member) ?? this.followReExport(member, targetModule, exportsByModule, moduleByQualifiedName); - if (declared === undefined || declared === null) { + // A module-level VARIABLE is a member too, and the interpreter's + // member-first order applies to it the same as to a def or a class. + // Without this check, `from svc.order_service import order_service` — + // the ordinary singleton idiom, where the value is named after its + // module — fell into the submodule fallback below, whose findModule + // matches by SUFFIX and so handed back svc.order_service ITSELF: the + // import resolved to MODULE with an empty hash and the VARIABLE + // branch further down never ran (#1140). + const variableMember = + targetModule === undefined + ? undefined + : moduleVariablesByModule.get(targetModule.qualifiedName)?.get(member); + if ((declared === undefined || declared === null) && variableMember === undefined) { const asModule = targetName === null || targetName === '' ? member : `${targetName}.${member}`; const memberModule = this.findModule(asModule, moduleByQualifiedName); - if (memberModule) { + // The module found by suffix must not be the target module itself: + // `from X import Y` never binds X, so a "submodule" that IS X is a + // suffix collision, not an answer. + if (memberModule && memberModule !== targetModule) { record.setResolution(memberModule.moduleHash, PythonImportTargetKind.MODULE, ''); stats.importsResolved += 1; continue; @@ -688,6 +703,20 @@ export class PythonResolutionLinker { } } + // Per-module resolution context, built for EVERY module before ANY module + // resolves its call sites. The split matters for one reason: an imported + // module-level value's type lives in the EXPORTING module's local type + // index, and module order is arbitrary, so typing and resolution cannot + // share one sweep. + const resolutionCtxByModuleHash = new Map; + bindingByScopeAndName: Map; + parentScopeOf: Map; + boundNames: Set; + importedModuleNames: Set; + typesByName: Map; + localTypeByBinding: Map; + }>(); for (const module of modules) { const entityByBinding = new Map(); for (const method of module.methods) { @@ -779,6 +808,66 @@ export class PythonResolutionLinker { mroCache, }); + resolutionCtxByModuleHash.set(module.moduleHash, { + entityByBinding, + bindingByScopeAndName, + parentScopeOf, + boundNames, + importedModuleNames, + typesByName, + localTypeByBinding, + }); + } + + // A from-import of a module-level VALUE carries the binding it names + // (#1140, PythonImportTargetKind.VARIABLE) — but the IMPORTING module's + // local type index knew nothing about that binding, so + // `order_service.cancel()` still fell to a name match whenever + // `order_service = OrderService()` lives in another module, while the same + // call in the exporting module resolved. The exporter's own index has + // already typed that binding on the same three grounds any local uses; + // copy the answer onto the import's binding so the ordinary NAME-receiver + // lookup finds it. One hop only, by construction: a re-exported value + // resolves to an import binding, which is never an assigned module-scope + // binding, so it was not given VARIABLE kind in the first place. + for (const module of modules) { + const own = resolutionCtxByModuleHash.get(module.moduleHash); + if (!own) { + continue; + } + for (const record of module.imports) { + if (record.getResolvedTargetKind() !== PythonImportTargetKind.VARIABLE) { + continue; + } + const importBinding = record.getBindingLinkHash(); + const exportedBinding = record.getResolvedTargetHash(); + // An entry that already exists wins: the name is also assigned in this + // module, and that assignment (or its refusal, null) is the local truth. + if (importBinding === '' || exportedBinding === '' || own.localTypeByBinding.has(importBinding)) { + continue; + } + const exporter = resolutionCtxByModuleHash.get(record.getResolvedModuleLinkHash()); + const type = exporter?.localTypeByBinding.get(exportedBinding); + if (type) { + own.localTypeByBinding.set(importBinding, type); + } + } + } + + for (const module of modules) { + const ctx = resolutionCtxByModuleHash.get(module.moduleHash); + if (!ctx) { + continue; + } + const { + entityByBinding, + bindingByScopeAndName, + parentScopeOf, + boundNames, + importedModuleNames, + typesByName, + localTypeByBinding, + } = ctx; for (const callSite of module.callSites) { // Retry anything WITHOUT A HASH, not merely anything UNRESOLVED. The // single-file pass has no module graph, so it can only say IMPORTED for