Skip to content

A5: external is a body kind, not a modifier - #11

Merged
milyin merged 1 commit into
docs/validation-umbrellafrom
step/a5-external-body
Aug 6, 2026
Merged

A5: external is a body kind, not a modifier#11
milyin merged 1 commit into
docs/validation-umbrellafrom
step/a5-external-body

Conversation

@milyin

@milyin milyin commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Fifth and last structural step of #6. Stacked on #10.

external was a free-form string in KtFun::modifiers while the body lived in
a separate field, so this was representable and rendered:

external fun f(): Int { return 1 }   // natively implemented *and* has a body

Making it a KtBody variant puts both in the same field, so setting either
clears the other and the combination has nowhere to exist:

KtFun::new("f").external().body(code)   // -> Block
KtFun::new("g").body(code).external()   // -> External

KtFun::external() replaces .modifier("external"), and passing the string
panics with a message pointing at it — otherwise the old spelling would quietly
reintroduce the hole this PR closes.

What stays for the validator

KtBody::None remains. Whether a bodiless function is legal is positional
an interface member needs no keyword, an abstract class member does — and
position is not something a type can capture here. It is listed as B3 in the
umbrella.

Verification

All 49 pre-existing tests pass; external renders in exactly the same position
as before, so no golden string moved. Three added: rendering, the
mutual-exclusion property, and the modifier-string panic.
cargo clippy --all-targets clean.


Part A is complete with this PR. Five shapes that used to render as
non-compiling Kotlin are now unbuildable: constructor parameters on kinds that
have no constructor, empty data class, wrong-arity value class, top-level
companion object, two constructed superclasses, a bodied fun interface
method, and external with a body.

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

Moves Kotlin external from a free-form function modifier string into the typed function body representation (KtBody::External) so external fun … { … } becomes unrepresentable in the model, while keeping rendering order consistent and adding regression tests.

Changes:

  • Add KtBody::External and a dedicated KtFun::external() builder; reject "external" passed via KtFun::modifier(...).
  • Update rendering/import-collection logic to treat External like a bodiless function while still rendering the external keyword ahead of other modifiers.
  • Update and add tests covering rendering, mutual exclusion of external vs body, and the modifier-string panic.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/model.rs Introduces KtBody::External, adds KtFun::external(), and guards against "external" being added via modifier().
src/render.rs Renders external from the body-kind and treats External like None for body output and body-import collection.
src/tests.rs Migrates existing tests to .external() and adds new tests for rendering, exclusivity, and the panic guard.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/model.rs
Comment on lines +784 to +790
let m = m.into();
assert!(
m != "external",
"`external` is not a modifier here — use `KtFun::external()`, which \
also rules out giving the function a body"
);
self.modifiers.push(m);
`external` was a free-form string in `KtFun::modifiers` while the body
lived in a separate field, so `external fun f(): Int { … }` — a function
that is both natively implemented and has a body — was representable and
rendered.

Make it a `KtBody` variant. Both live in the one `body` field now, so
setting either clears the other and the combination has nowhere to
exist. `KtFun::external()` replaces `.modifier("external")`, and passing
the string panics with a message pointing at it, so the old spelling
cannot quietly reintroduce the hole.

`KtBody::None` stays: whether a bodiless function is legal depends on
where it sits — an interface member needs no keyword — and position is
not something a type can capture here. That one remains for the
validator.

All 49 pre-existing tests pass; `external` renders in the same place as
before. Three added.
@milyin
milyin force-pushed the step/a4-fun-signature branch from b3c8bd1 to ce48226 Compare August 6, 2026 11:40
@milyin
milyin force-pushed the step/a5-external-body branch from 75908e0 to d1c977f Compare August 6, 2026 11:40
@milyin
milyin changed the base branch from step/a4-fun-signature to docs/validation-umbrella August 6, 2026 11:40
@milyin
milyin merged commit ee29ce4 into docs/validation-umbrella Aug 6, 2026
0 of 4 checks passed
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