Skip to content

Scala: relative imports (import Owner.member in the same package, import subpkg.Name) are never resolved; fail closed since #2360 #2428

Description

@htarnacki

Version

Built from source, main @ 2278498 (v0.11.0-186) and the #2360 head (bb3f1cdd)

Platform / Install channel / Binary variant

Linux (x64) / Built from source / standard

What happened, and what did you expect?

Follow-up to #2153 / #2360, as agreed in the review there. Scala resolves import paths relative to the packages visible at the import site before trying them as absolute paths: inside package a.b.c, import d.E means a.b.c.d.E if that exists, and import Owner.member in the same package means a.b.c.Owner.member. With chained clauses (package a.b / package c) the members of a.b are visible too, so import x.Y may also mean a.b.x.Y. A single package a.b.c clause does not make a.b visible.

The import resolver only knows the absolute form. Every relative import of an in-repo symbol is therefore lost:

Expected: an IMPORTS edge to the member the relative path names (Addr.State, util.Fmt.Fmt, shared.Log.Log below), and the calls through it at import_map confidence.

Measured on twitter/finagle (ca472de): this is the single edge #2360 loses versus main — StabilizingAddrTest.scala (package com.twitter.finagle.addr) import StabilizingAddr.State._, which main bound to the StabilizingAddr owner and #2360 drops. finagle almost always writes absolute imports, so the corpus under-represents the pattern; codebases that lean on same-package and sub-package imports lose the corresponding share of their Scala IMPORTS edges.

Reproduction

  1. Dummy snippet, six files under t/:
// t/Addresses.scala
package net.addr
object Addr {
  object State { val Healthy: Int = 1; val Unhealthy: Int = 2 }
  def parse(s: String): Addr = new Addr(s)
}
class Addr(val host: String)
// t/Watcher.scala  — same-package relative member import
package net.addr
import Addr.State
object Watcher { def check(): Int = State.Healthy }
// t/util/Fmt.scala
package net.addr.util
object Fmt { def render(a: Addr): String = a.host }
// t/Printer.scala  — sub-package relative import
package net.addr
import util.Fmt
object Printer { def show(a: Addr): String = Fmt.render(a) }
// t/shared/Log.scala
package net.shared
object Log { def info(msg: String): Unit = println(msg) }
// t/Report.scala  — relative to the outer clause of a chained package
package net
package addr
import shared.Log
object Report { def emit(a: Addr): Unit = Log.info(a.host) }
  1. Command: codebase-memory-mcp cli index_repository --repo-path /tmp/repro, then read the IMPORTS / CALLS edges of the project (search_graph for Fmt, or the SQLite file directly).

  2. Result — all four definition nodes exist (t.Addresses.Addr.State, t.util.Fmt.Fmt, t.shared.Log.Log, …):

    import main 2278498 feat(scala): parse import selectors and resolve package-aware imports (#2153) #2360 bb3f1cdd expected
    import Addr.State IMPORTS → t.Addresses.Addr (owner, not the member) no edge IMPORTS → t.Addresses.Addr.State
    import util.Fmt no edge no edge IMPORTS → t.util.Fmt.Fmt
    import shared.Log no edge no edge IMPORTS → t.shared.Log.Log
    Fmt.render(a) / Log.info(...) CALLS, unique_name CALLS, unique_name CALLS, import_map

    Note the file names are deliberately different from the object names (Addresses.scala for object Addr). When they coincide and the two files sit in the same directory, Strategy 1 (path-based module resolution) happens to bind the import to the sibling file's node rather than the member, which masks the defect in the simplest layouts.

Where it is

  • pass_pkgmap.c, resolve_scala_namespace_import: takes imp->module_path as an absolute dotted path, walks its prefixes through the namespace map and the (package, top-level name) index, and returns NULL when no prefix is a known package. The importing file's own package is never consulted, although it is already available: the namespace-map walk records file QN → package and (since feat(scala): parse import selectors and resolve package-aware imports (#2153) #2360) writes it as the package property on the File node.
  • Chained clauses are joined into one namespace string (net.addr for package net / package addr), so the resolver cannot tell that net is visible as well.

Proposed fix

In resolve_scala_namespace_import, before the absolute attempt, prefix the import path with each package visible at the import site, longest first, and run the same fail-closed lookup on each candidate:

  1. the file's full package (net.addr → net.addr.Addr.State, net.addr.util.Fmt);
  2. for chained clauses, each shorter clause prefix (net → net.shared.Log); a single clause package a.b.c contributes only a.b.c, not a.b;
  3. the absolute path as today.

First candidate that resolves wins; if none does, NULL as now. Step 2 needs the clause boundaries, which means recording the chain (or the offsets of the joins) next to the joined namespace instead of only the joined string — one extra small field per Scala file. The lookup per candidate is the existing O(1) index probe, so the cost is bounded by the number of clauses. Java and Kotlin are unaffected (they do not enter this resolver).

Tests: edge_imports cases for the three imports above (positive), plus a negative one showing that package a.b.c + import d.E does not bind a.b.d.E, and the incremental-reindex parity check extended to a relative import.

I can send this as a small PR stacked on #2360 if wanted.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    parsing/qualityGraph extraction bugs, false positives, missing edgesux/behaviorDisplay bugs, docs, adoption UX

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions