Skip to content

Dart: capitalized ordinary function used as local-var initializer can still misroute/drop a receiver call (residual risk after #2474) #2568

Description

@carlos-alm

Context

PR #2567 (fixing #2474) added typeMap seeding for var svc = UserService(repo); in Dart's extractor (handleDartLocalVarTypeMap / handle_dart_local_var_type_map), gated on the callee being capitalized to avoid misclassifying an ordinary lowercase factory function (var svc = makeService();) as a constructor call.

A Greptile review on that PR correctly pointed out the capitalization gate is not sufficient: Dart does not require a class name and an ordinary function name to differ in capitalization — a legally-named uppercase top-level function (OrderService MakeOrderService() { ... }) is syntactically indistinguishable from a constructor call at this position, and gets wrongly seeded as if the local's type were the literal function name.

What I verified empirically (not just reasoned about)

Added a dual-engine integration test reproducing this exact shape:

class OrderService {
  void placeOrder() {}
}

OrderService MakeOrderService() {
  return OrderService();
}

void placeAnOrder() {
  var order = MakeOrderService();
  order.placeOrder();
}

Result on BOTH engines: the placeAnOrder -> OrderService.placeOrder call edge is dropped entirely (not misrouted to a wrong node in this single-candidate case, but genuinely missing).

Root cause, traced through both resolvers:

  • WASM/TS: resolveByReceiver (src/domain/graph/resolver/strategy.ts) branches on if (typeName) { ...only tries the (possibly wrong) type's qualified method / prototype alias... } else { ...tries resolveViaDirectQualifiedMethod using the receiver's OWN bare name... }. Once any typeMap entry exists for the receiver — right or wrong — the else branch (the receiver-name-based fallback) is never attempted at all.
  • Native/Rust: resolve_call_targets_core (crates/codegraph-core/src/domain/graph/builder/stages/build_edges.rs) has the identical structural property — step "3.5. Direct qualified method lookup ... when the receiver is a class name with no typeMap entry" is guarded by if type_lookup.is_none() && inline_new_type.is_none(), so it's skipped whenever any type_lookup entry exists, right or wrong.

This is a pre-existing, language-agnostic property of the shared resolver, not something #2474 introduced — any receiver-type guess (from any language's extractor) that turns out to have zero real methods forecloses the untyped/direct-qualified fallback tier for that call, for every language that funnels through this shared cascade. #2474's Dart fix merely creates one more situation (an uppercase ordinary function) where a wrong guess can occur.

I did NOT attempt to fix the shared resolver cascade in #2567 — merging the type-aware and no-type-fallback branches (so a failed type-aware lookup falls through to try the receiver's-own-name qualified lookup too) is a resolver-architecture change with blast radius across every language using this cascade, not a Dart-specific fix, and deserves its own dedicated investigation rather than being folded into a single-language typing bug fix.

Two independent, worth-separating problems

  1. Shared resolver: a receiver-type guess (right OR wrong) unconditionally forecloses the untyped/direct-qualified fallback tier, in both resolveByReceiver (TS) and resolve_call_targets_core (Rust). Fixing this (e.g., falling through to the receiver-name-based lookup when the type-aware lookup finds literally nothing) would help many languages, not just Dart, and reduce (though not eliminate) the practical impact of any heuristic-based type guess anywhere in the codebase.
  2. Dart-specific: src/extractors/dart.ts's walkDartNode is a single combined tree-walk (unlike the Rust extractor's two-pass match_dart_node + match_dart_type_map design, and unlike javascript.ts's equivalent two-pass convention). This means a same-file negative check ("is this capitalized callee a known ordinary function already extracted into ctx.definitions, not a class?") would be declaration-order-dependent in the TS/WASM engine, while reliable in the Rust/native engine's two-pass design — an engine-parity hazard if implemented only on one side. Refactoring walkDartNode into a two-pass design to make this check reliable in both engines is a bigger, dedicated architectural change.

Suggested follow-up work (either or both)

  • Investigate whether resolveByReceiver/resolve_call_targets_core can safely fall through to the receiver-name-based/global fallback tier when the type-aware tier finds zero real matches, across all languages that use this cascade (careful: needs its own precision/recall validation against the resolution-benchmark fixture suite, since loosening this could introduce new false-positive edges for other languages).
  • Refactor src/extractors/dart.ts's walkDartNode into the same two-pass (match_dart_node-then-match_dart_type_map-equivalent) design already used by javascript.ts and mirrored in dart.rs, enabling a reliable same-file "is this name already a known ordinary function?" negative check for the local-var-constructor-call heuristic (and any future Dart heuristics that need the same kind of same-file cross-reference).

Current mitigation shipped in #2567

The capitalization gate (only seed when the callee is capitalized) meaningfully narrows — but does not eliminate — the false-positive rate, matching this codebase's own existing precedent for the identical class of ambiguity in javascript.ts's Foo.create() factory-method heuristic (confidence 0.7, also gated on capitalization alone, also carrying the same theoretical coincidental-collision risk). #2567 lowers this Dart heuristic's confidence to the same 0.7 tier for consistency.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions