Improve code generation for repeated and map fields with (required)#307
Conversation
There was a problem hiding this comment.
Pull request overview
This PR expands (required) code generation to validate repeated and map fields not only for “presence” (non-empty collection) but also for “non-default elements” (e.g., empty strings, default message instances, zero enum values). It also updates test coverage accordingly and introduces/updates several repo “agent” workflow hooks and conventions.
Changes:
- Extend Java validation code generation for
(required)sorepeated/mapfields fail when they contain “missing” elements (empty strings, default messages, zero enums). - Add/enable integration tests for required
repeatedenums and requiredmapfields (string/message/enum values). - Update agent workflow/config files (new hooks, memory/task-plan structure) and bump snapshot version references.
Reviewed changes
Copilot reviewed 30 out of 37 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| version.gradle.kts | Bumps validationVersion snapshot. |
| tests/validating/src/testFixtures/proto/spine/test/tools/validate/required.proto | Adds protos for required maps with message/enum values; updates imports/header. |
| tests/validating/src/test/kotlin/io/spine/test/options/required/RequiredRepeatedEnumITest.kt | Enables tests for required repeated enums (default/zero enum values). |
| tests/validating/src/test/kotlin/io/spine/test/options/required/RequiredMapWithStringsITest.kt | Adds stricter coverage for required map entries with empty string values. |
| tests/validating/src/test/kotlin/io/spine/test/options/required/RequiredMapWithMessagesITest.kt | New tests for required maps with message values (default instance detection). |
| tests/validating/src/test/kotlin/io/spine/test/options/required/RequiredMapEnumITest.kt | New tests for required maps with enum values (zero-index detection). |
| tests/runtime/src/test/kotlin/io/spine/validation/option/RequiredSpec.kt | Updates runtime spec to reject default message entries in required repeated. |
| tests/consumer/src/test/kotlin/io/spine/validation/test/RequiredRuleITest.kt | Updates expected violation counts for required repeated messages containing default instances. |
| java/src/main/kotlin/io/spine/tools/validation/java/generate/option/RequiredGenerator.kt | Implements “missing element” detection for repeated/map fields in generated Java validators. |
| docs/dependencies/pom.xml | Updates docs dependency version to the new snapshot. |
| docs/dependencies/dependencies.md | Regenerates dependency report for the new snapshot version. |
| CLAUDE.md | Rewrites agent-facing repo conventions and workflow guidance. |
| AGENTS.md | Adds guidance to use git mv for tracked file moves. |
| .claude/settings.json | Updates hook script paths; adds publish-version gate; switches sanitize hook to .sh. |
| .claude/scripts/sanitize-source-code.kt | Removes misnamed sanitize script (was bash content in .kt). |
| .agents/tasks/README.md | New durable task-plan format/spec. |
| .agents/skills/version-bumped/SKILL.md | New “version-bumped” skill documentation. |
| .agents/skills/version-bumped/scripts/version-bumped.sh | New gate script to ensure branch version bump vs base ref. |
| .agents/skills/pre-pr/SKILL.md | Updates referenced hook path for gh pr create gating. |
| .agents/skills/move-files/SKILL.md | Strengthens guidance: always use git mv; adds version-bumped step. |
| .agents/skills/java-to-kotlin/SKILL.md | Adds final “ensure version bumped” step. |
| .agents/skills/dependency-update/SKILL.md | Adds /version-bumped to recommended workflow; updates commit-message guidance. |
| .agents/skills/bump-gradle/SKILL.md | Adds final “ensure version bumped” step. |
| .agents/scripts/sanitize-source-code.sh | New sanitize hook script supporting both file-path and patch-text inputs. |
| .agents/scripts/publish-version-gate.sh | New hook blocking risky ./gradlew invocations without a version bump. |
| .agents/scripts/protect-version-file.sh | Enhances version-file edit protection to detect patch-based edits too. |
| .agents/scripts/pre-pr-gate.sh | New hook blocking gh pr create unless /pre-pr has passed for current HEAD. |
| .agents/memory/reference/.gitkeep | Initializes memory reference directory. |
| .agents/memory/README.md | Documents team memory structure and write protocol. |
| .agents/memory/MEMORY.md | Adds memory index scaffold. |
| .agents/memory/project/.gitkeep | Initializes memory project directory. |
| .agents/memory/feedback/.gitkeep | Initializes memory feedback directory. |
| .agents/_TOC.md | Updates agent docs TOC to include new memory/tasks docs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e562ccdd1a
ℹ️ 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".
| private fun elementMissingCheck(type: Type): String? = when { | ||
| type.isPrimitive && type.primitive == TYPE_STRING -> "String::isEmpty" | ||
| type.isMessage -> "v -> v.equals(v.getDefaultInstanceForType())" | ||
| type.isEnum -> "e -> e.getNumber() == 0" |
There was a problem hiding this comment.
Handle UNRECOGNIZED enums in required collections
The generated predicate for enum elements uses e.getNumber() == 0, but protobuf Java enums expose unknown wire values as UNRECOGNIZED, and calling getNumber() on that constant throws IllegalArgumentException. That means validating a (required) repeated/map enum field can now crash instead of reporting violations when messages contain forward-compatible enum values from a newer producer. This regression is introduced by the new per-element check and affects runtime validation behavior.
Useful? React with 👍 / 👎.
This PR adds code generation for handling
repeatedandmapfields when the(required)option is set. We validate values in all the cases. Now we handle empty strings, default messages, and zero enum items.Other notable changes
configwas updated.