Skip a SourceFileSet that belongs to another language#77
Conversation
`Renderer.renderSources()` filtered the given source set to the files written in the renderer's language, but still called `render()` when the filtered subset was empty because the set belonged to another language. Source and target roots are language-specific directories (e.g. `.../main/java`, `.../main/kotlin`), and every renderer is invoked on every set. So, with Kotlin enabled, a Java renderer received the Kotlin-only set rooted under `.../main/kotlin` and crashed while trying to look up a `.java` file there. Skip rendering when the set has files but none are in the renderer's language. An empty set is still passed through, so renderers that create files from scratch keep working. Remove the now-unnecessary per-file workaround in `UuidJavaRenderer`. Fixes #17. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Document that an incremental `integrationTest` can launch the previous compiler version after a version bump, because the Gradle plugin's embedded version resource is not regenerated. Verify integration with a clean build. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Regenerated by the build after bumping the version to 2.0.0-SNAPSHOT.058. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a crash that occurred when a renderer was invoked with a SourceFileSet rooted in another language’s directory (e.g., Java renderer receiving a Kotlin-only set), by skipping rendering for non-empty sets that contain no files matching the renderer’s language, while still allowing truly empty sets to be rendered for “create-from-scratch” renderers.
Changes:
- Updated
Renderer.renderSources()to skip non-empty source sets that contain no files in the renderer’s language (but still render truly empty sets). - Added regression tests in
RendererSpeccovering: matching-language rendering, other-language skipping, and empty-set rendering. - Removed the temporary per-file workaround from
UuidJavaRenderernow thatRenderer.renderSources()properly guards the call.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
api/src/main/kotlin/io/spine/tools/compiler/render/Renderer.kt |
Adds a language-aware guard to skip rendering for non-empty sets that belong to another language; updates KDoc accordingly. |
api/src/test/kotlin/io/spine/tools/compiler/render/RendererSpec.kt |
Adds regression tests for rendering/skip behavior across matching language, other language, and empty sets. |
tests/compiler-extension/src/main/java/io/spine/tools/compiler/test/uuid/UuidJavaRenderer.java |
Removes the workaround that previously avoided failing on mismatched-language source roots. |
version.gradle.kts |
Bumps snapshot version from .057 to .058 for publishing / version gating. |
docs/dependencies/pom.xml |
Updates the dependency-report POM version to match the bumped snapshot. |
docs/dependencies/dependencies.md |
Regenerates dependency report content for the bumped snapshot version. |
.agents/memory/project/integration-tests-stale-compiler-after-version-bump.md |
Skipped (config-managed .agents/** file per repo review rules). |
.agents/memory/MEMORY.md |
Skipped (config-managed .agents/** file per repo review rules). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 263139da85
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Restore the `findFile` guard in `UuidJavaRenderer` (net change vs master is an improved comment). `select(UuidType.class)` returns UUID types from every source root, but a `SourceFileSet` covers a single root; when a Java-producing root other than `java` exists (e.g. `grpc`), `Renderer.renderSources()` still invokes this renderer because the set has `.java` files, so it must skip the types whose file is absent from the current set. Flagged in PR review. The framework guard in `Renderer.renderSources()` still fixes issue #17 (a Kotlin-only set is never passed to a Java renderer) and is covered by `RendererSpec`; the per-file guard handles the orthogonal multi-Java-root case. Review nits: - `Renderer.renderSources` KDoc: singular `sources contains`. - `RendererSpec`: name the `input`/`output` arguments at the `sourceSetWith` call sites to avoid silent swaps of the same-typed `Path` parameters. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
In-place re-runs of "Build on Ubuntu" kept hitting the known feature-branch build-cache flake (green on master, reproduces on unrelated docs-only branches). This empty commit forces fresh CI runs on new runners. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #77 +/- ##
============================================
+ Coverage 75.60% 75.62% +0.01%
Complexity 677 677
============================================
Files 203 203
Lines 3947 3950 +3
Branches 390 392 +2
============================================
+ Hits 2984 2987 +3
Misses 845 845
Partials 118 118 🚀 New features to boost your workflow:
|
Fixes #17.
Problem
Renderer.renderSources()filtered the givenSourceFileSetto the files written in the renderer'slanguage, but still invoked the abstractrender()when that filtered subset was empty because the set belonged to another language.Source and target roots are language-specific directories (
.../main/java,.../main/kotlin, …), and every renderer is invoked on every source set. So with Kotlin enabled, a Java renderer received the Kotlin-only set (rooted under.../main/kotlin) and crashed while looking up a.javafile there:Fix
Skip rendering when the set has files but none match the renderer's language:
The
!sources.isEmptyhalf is the key distinction from a naiverelevantFiles.isEmpty()guard (which brokePipelineSpec): a truly empty set is still passed torender(), so renderers that create files from scratch (e.g.InternalAccessRenderer) keep working.UuidJavaRendererkeeps its per-filefindFile(...)guard:select(UuidType.class)returns UUID types from every source root, so when a Java-producing root other thanjavais present (e.g.grpc), the renderer must still skip types whose.javafile is absent from the current set. (An earlier revision of this PR removed that guard; review correctly flagged it is still needed, so it is retained — only its comment is clarified.)Changes
Renderer.renderSources()— the language guard + doc.RendererSpec— 3 regression tests: skip on another language, render on matching language, render on an empty set.UuidJavaRenderer— retains the per-filefindFile(...)guard for multi-root safety (net change vsmasteris a clearer comment).version.gradle.kts—2.0.0-SNAPSHOT.057→.058(version gate), as its own commit.Verification
RendererSpec6/6,PipelineSpec17/17, pluscli/jvm/api-tests.integrationTest --no-build-cache: BUILD SUCCESSFUL — the Kotlin-enabledconsumer/in-place-consumernow resolve the fixedcompiler-cli-all:…058and generate code without the crash and without the workaround.Notes for the reviewer
Bump version -> `2.0.0-SNAPSHOT.058`), and the regenerateddocs/dependencies/reports are committed on their own (Update dependency reports)..agents/memory/about a build gotcha encountered here: after a version bump, an incrementalintegrationTestcan launch the previous compiler because the plugin's embedded version resource isn't regenerated — verify with a clean build. It is intentionally a standalone commit so it can be reviewed (or dropped) independently of the fix.🤖 Generated with Claude Code