Skip to content

fix(java): java codegen versioned namespace - #204

Closed
ospreyboi wants to merge 3 commits into
accordproject:mainfrom
ospreyboi:fix/java-codegen-versioned-namespace
Closed

fix(java): java codegen versioned namespace#204
ospreyboi wants to merge 3 commits into
accordproject:mainfrom
ospreyboi:fix/java-codegen-versioned-namespace

Conversation

@ospreyboi

@ospreyboi ospreyboi commented Mar 8, 2026

Copy link
Copy Markdown

Closes #159

Hello!!
I was exploring the Accord Project codebase
and stumbled across this bug. I tried generating Java code
from a versioned Concerto model and noticed the version was just
silently dropped from the package name
which makes the output
incorrect and breaks downstream consumers.

Turns out parseNamespace() already returns the version, it just
was not being used! The fixing was surprisingly small — just two lines
changed to destructure version alongside name and include it
in the namespace string when present.

I also added a dedicated test case for versioned namespaces and
updated the snapshots so everything is consistent.

Changes

  • lib/codegen/fromcto/java/javavisitor.js — fixed startClassFile
    and import generation to include version in namespace when present
  • test/codegen/fromcto/java/javavisitor.js — added test for versioned namespace
  • Updated snapshots to reflect corrected output

Signed-off-by: ospreyboi <akshaosprey460@gmail.com>
Signed-off-by: ospreyboi <akshaosprey460@gmail.com>
@mttrbrts

Copy link
Copy Markdown
Member

Thanks for fixing this! Before we merge, can you confirm the generated output compiles successfully with Javac?

This comment was generated by AI on behalf of @mttrbrts.

@mttrbrts

Copy link
Copy Markdown
Member

@copilot resolve the merge conflicts in this pull request

Merges upstream main into fix/java-codegen-versioned-namespace.
Resolves conflicts in javavisitor.js by combining main's
shouldImportType guard with the PR's versioned-namespace logic.
Updates writeImport to preserve version in the import path.
Snapshot file regenerated against main's test naming convention.

Signed-off-by: mttrbrts <code@rbrts.uk>
@mttrbrts mttrbrts changed the title Fix/java codegen versioned namespace fix(java): java codegen versioned namespace Aug 4, 2026
@mttrbrts

mttrbrts commented Aug 4, 2026

Copy link
Copy Markdown
Member

Thank you for taking the time to dig into this, @ospreyboi — the issue is a real one and the instinct to use parseNamespace() was correct.

Unfortunately the approach hits a hard constraint in the Java language spec: package names and import statements may only contain valid Java identifiers separated by dots. The @ character is not permitted, so output like:

package concerto.decorator@1.0.0;
import concerto@1.0.0.Concept;

causes javac to error with ';' expected and '.' expected on every file, and the verify-java CI job confirms this.

There is no simple escaping mechanism that would work cleanly here — the most obvious alternatives (e.g. v1_0_0 suffix, a nested v1 sub-package) all result in names that don't round-trip back to the original Concerto namespace in a useful way and would be confusing to Java consumers.

The version is intentionally dropped in the current codegen to produce idiomatic, compilable Java. Closing this PR and the linked issue #159 as won't fix — this is a language-level limitation rather than a bug.

@mttrbrts mttrbrts closed this Aug 4, 2026
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.

Missing version numbers in Java codegen

2 participants