Skip to content

Fix: support custom <message> output for 9 enforcer rules that ignored getMessage() - #1004

Merged
elharo merged 2 commits into
apache:masterfrom
btpka3:fix/add-message-to-enforcer-rules
Aug 21, 2026
Merged

elharo merged 2 commits into
apache:masterfrom
btpka3:fix/add-message-to-enforcer-rules

Conversation

@btpka3

@btpka3 btpka3 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Problem

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.

For example, with this configuration:

<bannedPlugins>
  <message>Please remove the deprecated plugin before migrating.</message>
  <excludes>
    <exclude>com.example:old-plugin</exclude>
  </excludes>
</bannedPlugins>

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:

StringBuilder buf = new StringBuilder();
if (getMessage() != null) {
    buf.append(getMessage()).append(System.lineSeparator());
}
buf.append("default error details...");
throw new EnforcerRuleException(buf.toString());

Fixed Rules (9 total)

Rule Problem
BannedPlugins Threw result directly, never checked getMessage()
BannedRepositories Threw errMsg directly
BanDuplicatePomDependencyVersions No message prefix
BanDistributionManagement Inner class threw directly, outer never wrapped
RequireMatchingCoordinates No message prefix
RequireExplicitDependencyScope No message prefix
RequireProfileIdsExist No message prefix
RequireSameVersions No message prefix
RequirePrerequisite Used replacement pattern instead of prepend

Tests

All 9 modified rules now have test coverage verifying the custom message is correctly prepended:

Test File Status
TestBannedPlugins New (4 tests)
TestBannedRepositories Added shouldOutputCustomMessageWhenBanned
TestRequireSameVersions Added shouldOutputCustomMessageWhenVersionsDiffer
BanDistributionManagementTest Added shouldOutputCustomMessageWhenBanned
RequirePrerequisiteTest Added shouldOutputCustomMessageWhenPrerequisiteNotSet
TestRequireExplicitDependencyScope New
TestRequireProfileIdsExist New
TestRequireMatchingCoordinates New (2 tests)

All 259 tests pass with mvn -pl enforcer-rules test.

Checklist

  • Your pull request should address just one issue, without pulling in other changes.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Write unit tests that match behavioral changes.
  • Run mvn verify to make sure basic checks pass.
  • You have run the integration tests successfully (mvn -Prun-its verify).

ICLA

ICLA has been submitted to secretary@apache.org and is pending approval.

…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
@elharo
elharo requested a review from kwin August 19, 2026 13:14

@elharo elharo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use \n

if (summary.length() > 0) {
StringBuilder message = new StringBuilder();
if (getMessage() != null) {
message.append(getMessage()).append(System.lineSeparator());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not use System.lineSeparator() here as exception messages must be platform independent. Use \n

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not use System.lineSeparator() here as exception messages must be platform independent. Use \n

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not use System.lineSeparator() here as exception messages must be platform independent. Use \n

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@elharo elharo added the bug Something isn't working label Aug 21, 2026
@elharo
elharo merged commit 0adfeb9 into apache:master Aug 21, 2026
17 checks passed
@github-actions github-actions Bot added this to the 3.6.4 milestone Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants