fix: support three-state logic for arguments#1587
Merged
Merged
Conversation
…verridden by defaults Boolean parameters in AbstractLockfileMojo were declared as primitive `boolean` with a `defaultValue`, meaning Maven always bound a value (true or false) even when the user never set the parameter. This made it impossible for mergeConfigWithCliArgs to distinguish "explicitly set by user" from "just the default" — the default would silently override the stored lockfile config. Fix: change the five bool parameters used by mergeConfigWithCliArgs (includeMavenPlugins, allowValidationFailure, allowPomValidationFailure, allowEnvironmentalValidationFailure, includeEnvironment) from `boolean` to `Boolean` and remove their `defaultValue`. When null (not set), mergeConfigWithCliArgs now preserves the lockfile's stored value; when non-null (explicitly set via CLI or pom.xml), it overrides. getConfig() applies system defaults for the null case: includeMavenPlugins → true (plugins included by default) includeEnvironment → true Also updates Config's default no-arg constructor to reflect the new includeMavenPlugins default of true. https://claude.ai/code/session_01PeZS3ZNzvZQDFPUXAer6ur
The default change belongs in PR #1576 (feat: include Maven plugins by default). This PR's scope is the nullable Boolean refactor for mergeConfigWithCliArgs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
algomaster99
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Convert five boolean configuration parameters (
includeMavenPlugins,allowValidationFailure,allowPomValidationFailure,allowEnvironmentalValidationFailure,includeEnvironment) from primitivebooleanto nullableBooleanto enable three-state logic: explicitly set via CLI/pom.xml, or null when not set. This allows proper distinction between "not configured" and "configured to false", enabling better merging of CLI arguments with stored lockfile configs.Key Changes
Parameter type conversion: Changed all five boolean parameters from
booleantoBooleaninAbstractLockfileMojo, removingdefaultValueattributes from@Parameterannotations to allow null values.Updated
getConfig()logic: Modified to use null-safe comparisons (Boolean.FALSE.equals(),Boolean.TRUE.equals()) with explicit comments documenting default behavior:includeMavenPluginsdefaults totruewhen not explicitly setincludeEnvironmentdefaults totruewhen not explicitly setfalse(Error mode) when not explicitly setEnhanced
mergeConfigWithCliArgs()semantics: Changed from "only override if true" to "only override if non-null", preserving stored lockfile config values when CLI arguments are not explicitly provided. This prevents silent tightening of existing lockfile configs by plugin defaults.Test update: Updated
ValidateMojoTest.storedConfigUsedWhenCliArgNotSet()to assertisNull()instead ofisFalse(), reflecting the new nullable parameter semantics.Implementation Details
The change enables proper CLI override behavior: when a user does not specify a parameter, the stored lockfile configuration is preserved; when explicitly set (even to false), the CLI value takes precedence. This is critical for the
mergeConfigWithCliArgs()method used during validation to avoid inadvertently changing validation policies from a stored lockfile.