Repository navigation
Fix: support custom <message> output for 9 enforcer rules that ignored getMessage() - #1004
Conversation
β¦tMessage() Several enforcer rules did not output the user-configured <message> when the rule failed. This meant users could not provide custom context or explanations for rule violations. Fixed the following rules to prepend getMessage() to the error output when set: - BannedPlugins - BannedRepositories - BanDuplicatePomDependencyVersions - BanDistributionManagement - RequireMatchingCoordinates - RequireExplicitDependencyScope - RequireProfileIdsExist - RequireSameVersions - RequirePrerequisite Also updated toString() methods to include getMessage() for better debugging support, and added setter methods where missing. Added/updated tests: - New TestBannedPlugins test class - Added custom message test to TestBannedRepositories - Added custom message test to TestRequireSameVersions
| if (summary.length() > 0) { | ||
| StringBuilder message = new StringBuilder(); | ||
| if (getMessage() != null) { | ||
| message.append(getMessage()).append(System.lineSeparator()); |
There was a problem hiding this comment.
do not use System.lineSeparator() here as exception messages must be platform independent. Use \n
There was a problem hiding this comment.
Fixed. Replaced System.lineSeparator() with "\n" in all affected files (see commit d4f64b5).
| throw new EnforcerRuleException(result); | ||
| StringBuilder buf = new StringBuilder(); | ||
| if (getMessage() != null) { | ||
| buf.append(getMessage()).append(System.lineSeparator()); |
There was a problem hiding this comment.
do not use System.lineSeparator() here as exception messages must be platform independent. Use \n
There was a problem hiding this comment.
Fixed. Replaced System.lineSeparator() with "\n" in all affected files (see commit d4f64b5).
| throw new EnforcerRuleException(errMsg); | ||
| StringBuilder buf = new StringBuilder(); | ||
| if (getMessage() != null) { | ||
| buf.append(getMessage()).append(System.lineSeparator()); |
There was a problem hiding this comment.
do not use System.lineSeparator() here as exception messages must be platform independent. Use \n
There was a problem hiding this comment.
Fixed. Replaced System.lineSeparator() with "\n" in all affected files (see commit d4f64b5).
Exception messages must be platform-independent. Replace all System.lineSeparator() calls with literal "\n" in enforcer rule error messages to avoid \r\n on Windows.
| } | ||
| } | ||
|
|
||
| private void executeCheck(DistributionManagementCheck check) throws EnforcerRuleException { |
There was a problem hiding this comment.
Not for this PR but as I look at this now I realize we've designed this completely wrong. There shouldn't be exceptions here at all. Bleah.
Problem
Several enforcer rules did not output the user-configured
<message>whenthe rule failed. This meant users could not provide custom context or
explanations for rule violations.
For example, with this configuration:
The custom message was silently ignored and only the default error details were shown.
Solution
Follow the existing pattern (already used by
AlwaysFail,BannedDependenciesBase, etc.)to prepend
getMessage()to the error output when set:Fixed Rules (9 total)
BannedPluginsgetMessage()BannedRepositoriesBanDuplicatePomDependencyVersionsBanDistributionManagementRequireMatchingCoordinatesRequireExplicitDependencyScopeRequireProfileIdsExistRequireSameVersionsRequirePrerequisiteTests
All 9 modified rules now have test coverage verifying the custom message is correctly prepended:
TestBannedPluginsTestBannedRepositoriesshouldOutputCustomMessageWhenBannedTestRequireSameVersionsshouldOutputCustomMessageWhenVersionsDifferBanDistributionManagementTestshouldOutputCustomMessageWhenBannedRequirePrerequisiteTestshouldOutputCustomMessageWhenPrerequisiteNotSetTestRequireExplicitDependencyScopeTestRequireProfileIdsExistTestRequireMatchingCoordinatesAll 259 tests pass with
mvn -pl enforcer-rules test.Checklist
mvn verifyto make sure basic checks pass.mvn -Prun-its verify).ICLA
ICLA has been submitted to secretary@apache.org and is pending approval.