test: add ArchUnit rule to prevent primitive types on @Option fields + fix object versions#2517
test: add ArchUnit rule to prevent primitive types on @Option fields + fix object versions#2517maxandersen wants to merge 2 commits into
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Add CliArchitectureTest that ensures all Aesh CLI annotation fields (@option, @argument, @arguments, @OptionList, @OptionGroup) use boxed types instead of primitives. Primitive types have implicit defaults (false, 0, etc.) making it impossible for JBangDefaultValueProvider to distinguish 'not set' from 'explicitly set to the default'. This breaks config-file overriding. Currently detects 36 violations: 35 boolean fields and 1 int field that should be Boolean/Integer respectively. Refs #2504
maxandersen
force-pushed
the
test/archunit-no-primitive-options
branch
from
9c76e69 to
8239c27
Compare
…ValueProvider Change all boolean/int @Option-annotated fields to use boxed types (Boolean/Integer) so JBangDefaultValueProvider can distinguish between 'not set' (null) and 'explicitly set to default' (false/0). Files changed: - BaseCommand.java: boolean insecure → Boolean - JBang.java: boolean versionRequested → Boolean - Edit.java: boolean live, noOpen, sandbox → Boolean - Init.java: boolean force, edit → Boolean - Info.java: boolean dependenciesOnly, open → Boolean - Version.java: boolean checkForUpdate, update → Boolean - Alias.java: boolean force (AliasAdd), showOrigin (AliasList) → Boolean - Catalog.java: boolean force (CatalogAdd), showOrigin (CatalogList) → Boolean - Config.java: boolean global (BaseConfigCommand), showOrigin, showAvailable → Boolean - Template.java: boolean force, showOrigin, showFiles, showProperties → Boolean - App.java: boolean force, noBuild (AppInstall), force (AppSetup) → Boolean - Cache.java: boolean all → Boolean - Jdk.java: boolean force, available, details, forVersion → Boolean - Export.java: boolean force → Boolean - Wrapper.java: boolean force → Boolean - DependencyInfoMixin.java: boolean ignoreTransitiveRepositories → Boolean - CatalogFileOptionsMixin.java: boolean global → Boolean - Deps.java: int max → Integer All usages updated to use Boolean.TRUE.equals() pattern for null-safe evaluation. Also fixed an unboxing NPE in Jdk.JdkList where 'available' was used in a ternary without null-check.
maxandersen
force-pushed
the
test/archunit-no-primitive-options
branch
from
dc91dd6 to
210e870
Compare
maxandersen
changed the title
|
Never heard of ArchUnit before, pretty neat 👍 |
|
I have mentioned it before :) but hadn't had a good usecase to find problems and now realize it's output is great for driving LLMs to fix stuff. So perfect storm! Note: @stalep working on fix in aesh so we hopefully don't need to keep this. |
As you can see I don't always listen very well 😅 |
|
It's more me sucking at speaking well :) |
2 participants
Summary
Add an ArchUnit test (
CliArchitectureTest) that ensures all Aesh CLI annotation fields (@Option,@Argument,@Arguments,@OptionList,@OptionGroup) use boxed types instead of Java primitives.Problem
Primitive types have implicit defaults (
false,0, etc.) which make it impossible forJBangDefaultValueProviderto distinguish "user didn't pass the flag" from "user explicitly set the default value". This breaks config-file overriding — a config value can never be negated from the CLI because the primitive default shadows it.For example, with
boolean insecure, settinginsecure=trueinjbang.propertiesworks, but passing--no-insecureon the CLI can't override it becausefalse(the primitive default) is indistinguishable from "not set".What this PR adds
archunit-junit5:1.4.0) inbuild.gradleCliArchitectureTestwith 5 rules (one per Aesh annotation), checking all 8 Java primitive typesCurrent violations detected
The test currently fails intentionally to document the 36 existing violations:
booleanfields that should beBooleanintfield (Deps.max) that should beIntegerExample violation message:
Next steps
The actual migration of primitive → boxed types should be done in a follow-up PR, with null checks added where each field is read.
Refs #2504