A2: a companion object becomes its own type - #8
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
KtCompanion::named currently permits empty names, which can render invalid Kotlin (companion object ) and should be rejected or normalized.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR is the second step of the validation umbrella work, restructuring the declaration model so a Kotlin companion object cannot be constructed in an invalid top-level declaration position by making it a dedicated KtCompanion type.
Changes:
- Introduces
KtCompanionas its own model type (noInto<KtDecl>), and updatesKtClassto holdOption<KtCompanion>instead of a boxedKtClass. - Updates the renderer to render companions via a dedicated
render_companionpath and shares superclass/interface rendering viarender_supertypes. - Updates and extends tests to cover named vs anonymous companion rendering.
File summaries
| File | Description |
|---|---|
| src/model.rs | Removes KtClassKind::Companion, adds KtCompanion, and updates KtClass companion storage/API accordingly. |
| src/render.rs | Adds companion-specific import collection/rendering and factors supertype rendering into a shared helper. |
| src/tests.rs | Migrates companion construction to KtCompanion and adds coverage for named/anonymous rendering. |
| src/lib.rs | Exports KtCompanion from the crate prelude. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Comment on lines
+392
to
+398
| /// A named `companion object Factory { … }`. | ||
| pub fn named(name: impl Into<String>) -> Self { | ||
| Self { | ||
| name: Some(name.into()), | ||
| ..Self::default() | ||
| } | ||
| } |
This was referenced Aug 6, 2026
milyin
changed the base branch from
step/a1-class-kind-payload
to
docs/validation-umbrella
August 6, 2026 11:38
`KtClassKind::Companion` was documented as "only valid as
`KtClass::companion`" and nothing enforced it, so a companion object
could be built as a top-level declaration and rendered into a file where
`companion object { … }` is meaningless.
Give it its own type. `KtCompanion` has no `impl Into<KtDecl>`, so it
cannot reach a declaration position at all — proved by a `compile_fail`
doctest rather than by a runtime check. Being a distinct type it also
has no primary constructor to misuse, and no `KtClassKind` to be given
the wrong one.
Two incidental cleanups fall out:
* `name: Option<String>` replaces the empty-string-means-anonymous
sentinel the renderer used to special-case.
* `KtClass::companion` drops its `Box`: the indirection now comes from
the `Vec<KtDecl>` inside `KtCompanion`.
Supertype rendering is shared between class and companion rather than
duplicated. Rendering is otherwise unchanged: all 41 pre-existing tests
pass untouched.
milyin
force-pushed
the
step/a2-companion-type
branch
from
August 6, 2026 11:40
191a2c1 to
2aee77c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second step of #6. Stacked on #7.
KtClassKind::Companionwas documented as "only valid asKtClass::companion"and nothing enforced it. A companion object could be built as a top-level
declaration, and the renderer would emit
companion object { … }at file level,where it means nothing.
KtCompanionis now its own type with noimpl Into<KtDecl>, so it cannotreach a declaration position at all. That is proved by a
compile_faildoctestrather than by a runtime check:
Being a distinct type, it also has no
KtClassKindto be handed the wrongvariant and no primary constructor to misuse — a companion object cannot have
one, and now there is nowhere to put one.
Incidental cleanups
name: Option<String>replaces the empty-string-means-anonymous sentinelthe renderer used to special-case. Naming a companion (
companion object Factory) is now explicit instead of implied by a non-empty string.KtClass::companiondrops itsBox— the indirection now comes from theVec<KtDecl>insideKtCompanion.duplicated.
Verification
All 41 pre-existing tests pass untouched — output is byte-identical. Two
tests added for the named and anonymous forms, plus the
compile_faildoctest.cargo clippy --all-targetsclean.