Improve console output, reject (required) Empty#311
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens validation option semantics and improves developer feedback by introducing compile-time rejection for invalid (required) usage on google.protobuf.Empty, expanding (set_once) test coverage to external message types, and enhancing compiler warning readability. It also adds an api-discovery helper (scripts + skill docs) aimed at avoiding repeated Gradle-cache JAR unzips, along with routine version/docs updates.
Changes:
- Reject
(required)ongoogle.protobuf.Empty(includingrepeatedandmap<_, Empty>) at compile time, with dedicated compilation-error fixtures/tests. - Add integration coverage for
(set_once)on an external dependency type (google.protobuf.Timestamp) via new proto fixture + tests. - Improve console output for unsigned-integer warnings and introduce
api-discoveryscripts/skill documentation; bump snapshot version and regenerate dependency reports.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| version.gradle.kts | Bumps published library snapshot version. |
| docs/dependencies/pom.xml | Aligns docs artifact version with the new snapshot. |
| docs/dependencies/dependencies.md | Regenerates dependency/license report for the new version. |
| context/src/main/kotlin/io/spine/tools/validation/option/required/RequiredOption.kt | Adds compile-time rejection for (required) on google.protobuf.Empty (incl. list/map value cases). |
| context-tests/src/testFixtures/proto/spine/validation/required_option_spec.proto | Adds compilation-failure fixtures for (required) applied to Empty in different shapes. |
| context-tests/src/test/kotlin/io/spine/tools/validation/RequiredOptionSpec.kt | Adds compilation-error assertions for the new Empty rejection cases. |
| tests/validating/src/testFixtures/proto/spine/test/tools/validate/required.proto | Removes runtime fixture that previously demonstrated (required) + Empty behavior. |
| tests/validating/src/test/kotlin/io/spine/test/options/required/RequiredMessageITest.kt | Removes runtime test now superseded by compile-time rejection. |
| tests/validating/src/testFixtures/proto/spine/test/tools/validate/set_once_fields.proto | Adds Timestamp import and Measurement message for (set_once) external-type coverage. |
| tests/validating/src/testFixtures/kotlin/io/spine/test/options/setonce/SetOnceTestEnv.kt | Adds reusable Timestamp test constants. |
| tests/validating/src/test/kotlin/io/spine/test/options/setonce/SetOnceFieldsITest.kt | Adds integration tests ensuring (set_once) works for external message types. |
| java/src/main/kotlin/io/spine/tools/validation/java/generate/option/bound/BoundedFieldGenerator.kt | Refactors unsigned-integer warning message to multi-line for readability. |
| CLAUDE.md | Adds workflow guidance to use api-discovery when inspecting cached dependencies. |
| .agents/skills/api-discovery/SKILL.md | Documents the api-discovery workflow, bootstrap, and stale-sibling refresh procedure. |
| .agents/scripts/api-discovery/discover | Adds resolver script to map Maven coords to local sources (sibling-first, cache fallback). |
| .agents/scripts/api-discovery/extract-sources | Adds cached extraction of -sources.jar into a workstation cache. |
| .agents/scripts/api-discovery/update-sibling | Adds guarded git pull --ff-only helper for stale sibling repos (opt-in). |
| .agents/scripts/api-discovery/clean-cache | Adds manual cache pruning utility. |
| .agents/scripts/api-discovery/lib/common.sh | Shared helpers for parsing, resolution, and cache path logic. |
| .agents/scripts/api-discovery/README.md | Implementation/reference docs for the api-discovery scripts. |
| .agents/scripts/api-discovery/.gitignore | Ignores per-developer workspace-root override file. |
| .agents/tasks/reject-required-on-empty.md | Task log/planning doc for the compile-time (required) + Empty work. |
| .agents/tasks/issue-175-set-once-timestamp.md | Task log/planning doc for the (set_once) + Timestamp integration test. |
| .agents/tasks/api-discovery.md | Task log/planning doc for the api-discovery feature. |
| .agents/tasks/prohibit-automatic-commits.md | Task doc describing commit-safety policy work (process-only). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f775de0620
ℹ️ 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".
This PR:
Emptymessage is claimed as(required), including inrepeatedandmapvalues (Produce compile-time error on(required)set on anEmptyfield #146).(set_once)constraint (Implement negative tests for(set_once)and(goes)#175).