Skip to content

Duplicate FQNs make a Java application unloadable, not just its ambiguous queries unanswerable #420

Description

@rahlk

JCodeanalyzer._add_type raises on the second declaration spelling one qualified name:

def _add_type(self, t: JType, path: str) -> None:
    name = t.qualified_name
    if name in self._types:
        raise CodeanalyzerExecutionException(duplicate_type_name(name))
    self._types[name] = t
    self._file_of[name] = path

Surfacing the ambiguity is right — duplicate_type_name's docstring makes the case, and silently shadowing would be worse. The defect is when and how much it refuses.

1. It fires at construction

self._index() is the last line of JCodeanalyzer.__init__ (cldk/analysis/java/codeanalyzer/codeanalyzer.py:221). So an application containing duplicate FQNs cannot be loaded, and every query becomes unavailable — including the overwhelming majority that touch none of the duplicated types. The ambiguity is localised to a handful of names; the failure is total.

The realistic trigger is a monorepo of services that vendor a shared internal library, so several copies of one package reach one analysis. That input is exactly what codellm-devkit/codeanalyzer-java#269 is about, and it is not exotic.

2. The two backends disagree on when

JNeo4jBackend._idx is a @cached_property (cldk/analysis/java/neo4j/neo4j_backend.py:647), so it raises lazily on first use, not at construction — despite its docstring saying it "Mirrors JCodeanalyzer._index". Same input, same eventual error, different point of failure depending on which backend you chose.

3. The message blames the wrong component

type qualified name 'X' is declared twice: codeanalyzer-java emitted two declarations that spell one name

The analyzer did the right thing: two source files genuinely declare that name, and it emitted both with distinct can:// ids. The duplication is in the source tree. Attributing it to the emitter sends the reader to the wrong repository.

Proposed fix: id-keyed storage, unchanged signatures

  • Index by can:// id, which is distinct per copy, so nothing is dropped and construction always succeeds.
  • Keep a name → entries mapping that may hold several entries for one name.
  • Public signatures stay exactly as they are — get_class(name) -> JType | None and friends. A name with one entry answers as it does today; a contested name raises at that query, carrying the competing files.
  • Both backends then fail at the same point, which also settles (2).

Rejected: widening the return types to "all candidates". Those methods are on the shared cross-language ABC (cldk/analysis/commons/backend.py:119-136), so it would touch 44 definitions across 10 files — the ABC plus an in-memory backend, a Neo4j backend and a facade for each of Java, Python and TypeScript — and would charge Python and TypeScript for a Java monorepo's vendored copies. A well-formed single-service analysis has exactly one class per FQN; the plural shape belongs in an additive get_classes(name) if a caller ever needs it, not in the contract everything depends on.

Sequencing

Should land with or behind codellm-devkit/codeanalyzer-java#269. The index-time raise is currently the only thing telling a caller the input is unsafe; making bundled analyses load without #269's detection in place means they start quietly answering with the wrong-copy edges the analyzer emits.

Not a TypeScript concern: TS signatures carry the module path (src/models.Entity), so the cross-file collision cannot occur there. See #419 for that correction.

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions