Skip to content

Add KtClass::ctor_vis and KtFile::file_annotation - #23

Open
milyin wants to merge 2 commits into
mainfrom
ctor-vis-and-file-annotations
Open

Add KtClass::ctor_vis and KtFile::file_annotation#23
milyin wants to merge 2 commits into
mainfrom
ctor-vis-and-file-annotations

Conversation

@milyin

@milyin milyin commented Aug 13, 2026

Copy link
Copy Markdown
Owner

Two constructs the model could not express, both needed by prebindgen-jni for milyin/prebindgen#37.

KtClass::ctor_vis(KtVis)

The class header today is vis + keyword + name + (params) with no slot for a constructor visibility of its own. ctor_vis fills it: anything but KtVis::Default renders the explicit keyword.

public abstract class NativeHandle internal constructor(initialPtr: Long)

Asserts the class kind is Class | Data | Value | Enum — the others have no primary constructor.

KtFile::file_annotation(impl Into<String>)

A Vec<String> on KtFile, rendered as @file:{a} before the package line. merge_files unions and dedupes them, so a package assembled from fragments keeps every fragment's file annotations exactly once.

@file:OptIn(io.example.jni.UnsafeNativeApi::class)
package io.example.jni

Not banner — that is a single string the caller overwrites wholesale.

Verification

Both are exercised by examples/showcase.rs and pinned by tests/golden/showcase.txt. cargo test, cargo clippy --all-targets -D warnings (stable and 1.85.0), and cargo fmt --check are clean.

Version bumped to 0.2.1; prebindgen cannot build against this until it is on crates.io.

Two constructs the model could not express:

* `class Foo internal constructor(…)` — a constructor visibility distinct
  from the class's own. `KtClass::ctor_vis` renders the explicit
  `constructor` keyword for anything but `KtVis::Default`.
* `@file:Xxx` — file-level annotations, rendered above the `package` line.
  `KtFile::file_annotation` appends one; merging a package's fragments
  unions them.

Both are exercised by `examples/showcase.rs` and pinned by its golden file.

Needed by prebindgen-jni to emit `internal` handle constructors and a
blanket `@file:OptIn` over generated sources (milyin/prebindgen#37).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds missing Kotlin constructs to the declaration model so downstream generators (notably prebindgen-jni) can express (1) primary-constructor visibility distinct from class visibility, and (2) Kotlin @file: annotations emitted before the package line.

Changes:

  • Introduce KtClass::ctor_vis(KtVis) and render explicit constructor only when needed for visibility.
  • Add KtFile::file_annotation(...) / file_annotations and merge-time union+dedupe across fragments.
  • Update showcase example + golden output and bump release notes/version.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/model.rs Adds KtClass::ctor_vis and KtFile::file_annotation to the public model.
src/render.rs Renders @file: annotations before package; renders constructor visibility via explicit constructor.
src/file.rs Unions/dedupes file_annotations when merging fragments.
examples/showcase.rs Exercises the new APIs in the example generator.
tests/golden/showcase.txt Updates pinned expected Kotlin output.
Cargo.toml Bumps crate version to 0.2.1.
CHANGELOG.md Documents the added APIs for the 0.2.1 release.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread Cargo.toml Outdated
Comment thread CHANGELOG.md Outdated
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## 0.2.1 - 2026-08-13

@milyin milyin left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found one additional blocking correctness issue beyond the already-raised 0.3.0 SemVer/versioning problem:

ctor_vis accepts visibility combinations that Kotlin rejects. The assertion treats every Class and Enum as unrestricted, so KtClass::enum_(...).ctor_vis(KtVis::Internal) renders an enum constructor that kotlinc rejects with “constructor must be private in enum class”. KtClass::class_with(KtClassModifier::Sealed, ...).ctor_vis(KtVis::Public or Internal) likewise renders code rejected with “constructor must be private or protected in sealed class”.

Please constrain the accepted combinations (enum: Default/Private; sealed class: Default/Private, since KtVis currently has no Protected) and add focused panic/render tests. I reproduced both failures with kotlinc 2.4.10; the Rust suite, Clippy, formatting, and the valid file-annotation/import case all pass.

— Codex (GPT-5)

Review found `ctor_vis` treated every `Class` and `Enum` as unrestricted,
so `KtClass::enum_(…).ctor_vis(KtVis::Internal)` and
`class_with(Sealed, …).ctor_vis(KtVis::Public)` rendered code kotlinc
rejects ("constructor must be private in enum class" / "must be private or
protected in sealed class"). Both now panic; `Default` and `Private` are
the accepted pair, `protected` being unspellable in `KtVis`.

Also 0.3.0 rather than 0.2.1: `KtClass::ctor_vis` and
`KtFile::file_annotations` are new public fields on public structs, which
breaks downstream struct literals and exhaustive destructuring.

The accepted shapes are confirmed against kotlinc 2.4.10. A `data class`
with a restricted constructor is legal but leaks it through `copy()`, so
`ctor_vis` now says to pair it with `@ConsistentCopyVisibility`.
@milyin

milyin commented Aug 13, 2026

Copy link
Copy Markdown
Owner Author

Both addressed in a3d6b26.

ctor_vis accepted illegal combinations — right, and worse than the report: the assertion only checked that the kind has a primary constructor. Now enum class and sealed class accept only KtVis::Default | KtVis::Private; protected, which Kotlin also allows on a sealed class, is unspellable in KtVis. Everything else (plain/open/abstract class, data, value) is unrestricted, and the kinds with no primary constructor still panic as before.

Three #[should_panic] tests, one asserting the accepted pair still renders, one pinning public class H internal constructor(ptr: Long). I also compiled the accepted shapes with kotlinc 2.4.10 to confirm the other direction:

public enum class Kind private constructor(val code: Int) { A(1), B(2) }
public sealed class S private constructor(val x: Int)
public abstract class A internal constructor(val ptr: Long)
public data class D internal constructor(val x: Int)
@JvmInline public value class V private constructor(val b: Int)
public class H internal constructor(ptr: Long)

That surfaced one more thing, left permissive on purpose: a data class with a restricted constructor is legal but leaks it through the generated copy(), which kotlinc warns about and rejects from language version 2.5. Rejecting it here would be over-reach — it compiles today, and the fix is .annotation("ConsistentCopyVisibility") on the caller's side — so ctor_vis's doc says so instead.

SemVer — agreed, 0.3.0. KtClass::ctor_vis and KtFile::file_annotations are new public fields on public structs, so struct literals and exhaustive destructuring downstream break. Changelog header moved with it, and milyin/prebindgen#404 now pins 0.3.0.

cargo test (122 + 3 + 11), clippy -D warnings on stable and 1.85.0, and cargo fmt --check are clean.

@milyin milyin left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed a3d6b26. Both prior findings are resolved: illegal enum/sealed constructor visibilities are rejected with focused coverage, and the public-field additions are correctly released as 0.3.0.

No remaining findings. I independently confirmed the accepted constructor forms compile with kotlinc 2.4.10 and the rejected enum/sealed forms fail as expected. cargo test --all-targets --all-features, Clippy with warnings denied, formatting, rustdoc with warnings denied, cargo package, and CI are all clean.

— Codex (GPT-5)

milyin added a commit to milyin/prebindgen that referenced this pull request Aug 13, 2026
`KtClass::ctor_vis` and `KtFile::file_annotation` are not on crates.io yet
(milyin/kotlin-codegen#23), so every CI job failed to resolve
`kotlin-codegen = "^0.3.0"`. Point the workspace dependency at the branch,
keeping the `version` key beside `git` — that is what a published
prebindgen would carry, and what lets `cargo package` see a version at all.

`cargo package` still fails on it: it drops the git source and resolves the
version against crates.io. That is the same "unpublished dependency" case
the package job already tolerates for the sibling crates, so widen that
grep by one name rather than teach it a second shape.

Revert this commit once 0.3.0 is out. The branch is deleted on merge, so
CI will say so if nobody does.
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.

2 participants