Skip to content

Allow extendWith for the new MoreKnownTypes package - #959

Merged
alexander-yevsyukov merged 8 commits into
masterfrom
update-more-known-types-path
Aug 19, 2026
Merged

Allow extendWith for the new MoreKnownTypes package#959
alexander-yevsyukov merged 8 commits into
masterfrom
update-more-known-types-path

Conversation

@alexander-yevsyukov

@alexander-yevsyukov alexander-yevsyukov commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Closes #958.

What changed

KnownTypes.Holder.extendWith names its sole permitted caller as a string
matched against the call stack by InvocationGuard. In tool-base,
MoreKnownTypes moves from io.spine.tools.type to
io.spine.tools.proto.type — the package the new proto-code module owns
after the split in SpineEventEngine/tool-base#189 — so the old value rejected
the very class the guard exists to admit.

  • The allowed name and the @Internal comment above it now read
    io.spine.tools.proto.type.MoreKnownTypes.
  • A MoreKnownTypes test double under
    base/src/test/kotlin/io/spine/tools/proto/type/ bears that name, and
    KnownTypesSpec gained a positive case calling through it.
  • Unrelated to the guard name, but on the same code path: the
    SecurityException message interpolated the Class object and then
    appended a literal .name, reporting the caller as
    class io.spine.tools.proto.type.MoreKnownTypes.name — visible in the
    stack trace quoted in Update the extendWith invocation guard for the new MoreKnownTypes package #958. Fixed, along with nonAllowedCaller now
    returning the exception it builds rather than throwing it, matching its
    declared return type. Both call sites already wrote throw nonAllowedCaller(...), so behaviour is unchanged apart from the message.

Why the test matters

The guard is checked at runtime, not compile time. Everything compiles
regardless, and the failure surfaces only when extendWith is actually
called — in tool-base that happens through protobuf-setup-plugins, so a
name mismatch breaks a consumer's Gradle build rather than a test suite
here. KnownTypesSpec previously covered only the negative case (client code
is rejected); nothing pinned the allowed value. The new double closes that
gap: reverting the guard to the old name fails
allow calling 'extendWith' from 'MoreKnownTypes' with exactly the
SecurityException reported in the issue.

Two invariants make the double work, both documented in its KDoc:

  • It must bear the permitted FQN exactly — hence a Kotlin object, whose
    methods compile to instance methods on the class named by the declaration,
    rather than a top-level function landing in MoreKnownTypesKt.
  • It must call extendWith directly. CallerProvider.previousCallerClass
    skips exactly three StackWalker frames, so an extra frame from a
    delegating helper would break the match quietly, leaving the test passing
    for the wrong reason.

The double extends the known types with an empty TypeSet; TypeSet.union
returns this for an empty argument, so the singleton is swapped for a new
wrapper over the identical set. Content is unchanged, and KnownTypesSpec
re-reads KnownTypes.instance() per test while the serialization spec
compares toString(), so no ordering dependency is introduced.

For the reviewer

  • Version carrying the fix: 2.0.0-SNAPSHOT.441. tool-base cannot
    publish the renamed package until this ships.

  • Single name vs. a migration window. The issue notes that one updated
    name suffices given the release ordering — base-libraries first, then
    tool-base — and that is what this PR does. Worth knowing: the mismatch
    also runs the other way, so a new base with an older tool-base fails
    too. Contrary to the issue's assumption, InvocationGuard.allowOnly
    already has a vararg overload, so keeping both names during the migration
    is a one-line change if you would rather have the window:

    InvocationGuard.allowOnly("io.spine.tools.proto.type.MoreKnownTypes",
                              "io.spine.tools.type.MoreKnownTypes");
  • The docs/dependencies/ commit is regeneration only; it was stale after
    the .441 bump and the config update already on this branch. The added
    kotlin-stdlib-jdk7/8:1.8.20 and kotlinx-coroutines:1.7.3 entries come
    from that config update, not from this change.

Also on this branch: the config update

Two commits predate the #958 work and are unrelated to the invocation guard.
They are listed here for review scope; they are not part of the fix.

  • 7d717f41ef — version bump .440 -> .441.
  • 78553fe153config submodule update. config distributes
    buildSrc, so advancing the submodule pointer brings its content with it:
    the new ResolvedVersions type, the PomGeneratorIgTest TestKit
    integration test, the DependencyWriter/PomGenerator/PomXmlWriter
    rework behind them, and the local dependency bumps below.

Per AGENTS.md § Code review, config-distributed paths belong to a review of
the config repo rather than this one, so the pre-PR reviewers were scoped to
the base/ sources and version.gradle.kts. The dependency changes were
checked for rollbacks; all five move forward:

Artifact From To
Base (incl. versionForBuildScript) .426 .440
Compiler .522 .523
CoreJvm .080 .082
CoreJvmCompiler .404 .410
ToolBase / Validation .460 .462

Splitting these into a separate PR would require rewriting already-pushed
history, so they stay here; the guard fix is confined to 874ea52cdc,
3cf35e9f6d, and 54a465d43f.

Verification

./gradlew build dokkaGenerate green on JDK 17. Pre-PR reviewers
(spine-code-review, kotlin-engineer, review-docs) all approved; their
findings are folded into the final commit.

alexander-yevsyukov and others added 7 commits August 19, 2026 00:43
`MoreKnownTypes` moves from `io.spine.tools.type` to
`io.spine.tools.proto.type` in `tool-base` as part of splitting the
monolithic `tool-base` module. `KnownTypes.Holder.extendWith` names its
sole permitted caller as a string checked against the call stack, so the
old value rejected the very class the guard exists to admit.

The check has no compile-time footprint: everything compiles, and the
`SecurityException` appears only when `protobuf-setup-plugins` calls
`extendWith` during a consumer's Gradle build. To keep that failure in
this repository, `MoreKnownTypes` test double bears the permitted name
and exercises the guard from `KnownTypesSpec`, which so far covered only
the rejection of client code.

The double extends the known types with an empty `TypeSet`, so the
singleton keeps its contents and the remaining specs are unaffected.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The message interpolated the `Class` object and then appended a literal
`.name`, so a rejected call reported the class as
`class io.spine.tools.proto.type.MoreKnownTypes.name`.

`nonAllowedCaller` now returns the exception it builds instead of
throwing it, matching its return type; the callers already throw.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The reports were stale after the `2.0.0-SNAPSHOT.441` bump and
the `config` update earlier on this branch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Document the invariant that `extendWithNothing` must call the guarded
method directly: the guard resolves the immediate caller frame, so an
extra frame would break the match quietly rather than loudly.

Also align the task plan on "test double" and wrap an over-long line.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 19, 2026 00:04
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

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

Updates KnownTypes.Holder.extendWith to permit the renamed MoreKnownTypes caller (io.spine.tools.proto.type.MoreKnownTypes) and adds a regression test to pin the allowed FQN, preventing runtime build failures in downstream consumers. The PR also includes build tooling updates (POM/dependency report generation) and routine version/report regenerations.

Changes:

  • Update KnownTypes invocation guard and its @Internal note to allow io.spine.tools.proto.type.MoreKnownTypes.
  • Fix InvocationGuard’s SecurityException message formatting and align nonAllowedCaller behavior with its return type.
  • Add tests (incl. a MoreKnownTypes test double) to verify the guard admits the intended caller and regenerate dependency reports / bump versions.

Reviewed changes

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

Show a summary per file
File Description
version.gradle.kts Bumps versionToPublish to 2.0.0-SNAPSHOT.441.
docs/dependencies/pom.xml Updates generated POM version to .441.
docs/dependencies/dependencies.md Regenerates dependency/license report for .441.
buildSrc/src/test/kotlin/io/spine/gradle/report/pom/PomGeneratorIgTest.kt Adds Gradle TestKit integration test coverage for generatePom.
buildSrc/src/test/kotlin/io/spine/gradle/report/pom/DependencyWriterSpec.kt Adjusts dependency writer tests to use resolved-version inputs and adds a failure-to-resolve case.
buildSrc/src/main/kotlin/io/spine/gradle/report/pom/ResolvedVersions.kt Introduces per-project resolved-version collection and serialization utilities.
buildSrc/src/main/kotlin/io/spine/gradle/report/pom/PomXmlWriter.kt Threads resolved-version lookup into POM XML generation.
buildSrc/src/main/kotlin/io/spine/gradle/report/pom/PomGenerator.kt Registers per-project collectors and makes generatePom depend on them.
buildSrc/src/main/kotlin/io/spine/gradle/report/pom/DependencyWriter.kt Refactors dependency collection to consume resolved versions from a provider function.
buildSrc/src/main/kotlin/io/spine/dependency/local/Validation.kt Bumps local Validation dependency version.
buildSrc/src/main/kotlin/io/spine/dependency/local/ToolBase.kt Bumps local ToolBase dependency + dogfooding version.
buildSrc/src/main/kotlin/io/spine/dependency/local/CoreJvmCompiler.kt Bumps Core JVM Compiler versions.
buildSrc/src/main/kotlin/io/spine/dependency/local/CoreJvm.kt Bumps Core JVM dependency version.
buildSrc/src/main/kotlin/io/spine/dependency/local/Compiler.kt Bumps Compiler fallback versions.
buildSrc/src/main/kotlin/io/spine/dependency/local/Base.kt Bumps local Base versions referenced from buildSrc.
buildSrc/src/main/kotlin/io/spine/dependency/lib/JacksonV2.kt Improves Jackson 2.x docs/comments with link to JSTEP-1.
buildSrc/src/main/kotlin/io/spine/dependency/lib/Jackson.kt Improves docs/comments and refactors Moneta artifact coordinate usage.
buildSrc/build.gradle.kts Adds Gradle TestKit and passes buildSrc.classpath into tests via JVM args.
base/src/test/kotlin/io/spine/type/KnownTypesSpec.kt Adds a positive test asserting extendWith is allowed via MoreKnownTypes.
base/src/test/kotlin/io/spine/tools/proto/type/MoreKnownTypes.kt Adds test double with the exact permitted FQN to call extendWith directly.
base/src/main/kotlin/io/spine/security/InvocationGuard.kt Fixes exception message formatting and returns exception instead of throwing inside helper.
base/src/main/java/io/spine/type/KnownTypes.java Updates allowed caller FQN and @Internal comment to the new package.
.agents/tasks/958-more-known-types-guard.md Adds internal task tracking note for issue #958.

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

Comment thread version.gradle.kts
@alexander-yevsyukov
alexander-yevsyukov enabled auto-merge (squash) August 19, 2026 00:07
@alexander-yevsyukov alexander-yevsyukov moved this from 🏗 In progress to In Review in v2.0 Aug 19, 2026
@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.30%. Comparing base (51cb428) to head (6b406da).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #959      +/-   ##
==========================================
+ Coverage   94.01%   94.30%   +0.28%     
==========================================
  Files         194      194              
  Lines        4177     4177              
  Branches      345      345              
==========================================
+ Hits         3927     3939      +12     
+ Misses        149      137      -12     
  Partials      101      101              
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@alexander-yevsyukov
alexander-yevsyukov merged commit fa7d185 into master Aug 19, 2026
10 checks passed
@alexander-yevsyukov
alexander-yevsyukov deleted the update-more-known-types-path branch August 19, 2026 11:14
@github-project-automation github-project-automation Bot moved this from In Review to ✅ Done in v2.0 Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

Update the extendWith invocation guard for the new MoreKnownTypes package

3 participants