Skip to content

SONARJAVA-6196 S1451 should provide a default template for headers#5578

Open
NoemieBenard wants to merge 7 commits intomasterfrom
nb/sonarjava-6196
Open

SONARJAVA-6196 S1451 should provide a default template for headers#5578
NoemieBenard wants to merge 7 commits intomasterfrom
nb/sonarjava-6196

Conversation

@NoemieBenard
Copy link
Copy Markdown
Contributor

Add default template for headerFormat in S1451.

@hashicorp-vault-sonar-prod
Copy link
Copy Markdown
Contributor

hashicorp-vault-sonar-prod bot commented Apr 17, 2026

SONARJAVA-6196

@sonar-review-alpha
Copy link
Copy Markdown
Contributor

sonar-review-alpha bot commented Apr 17, 2026

Summary

Changed S1451 (file header check) to provide a default header template instead of an empty default.

The check previously required users to manually configure a headerFormat property or the rule would be inactive. This PR adds a sensible default—a copyright/license header template with placeholders (<Your-Product-Name>, <Year-From>-<Year-To>, <Your-Company-Name>) that users can customize in their quality profile.

The implementation also distinguishes between "no header required" (explicit empty string) and "default header" (the new template), ensuring that when headerFormat is explicitly set to empty, no header issues are flagged. Tests confirm both cases work correctly.

What reviewers should know

Key changes to review:

  • DEFAULT_HEADER_FORMAT constant (line 34): Changed from "" to a multi-line copyright/license template with instruction text
  • setContext() logic (line 68-71): Added explicit handling for empty headerFormat to skip header checking without errors
  • Test additions: New test cases verify that empty headerFormat produces no issues, and that files with the default header template pass

Why this matters:

  • Before: Users had to supply a header format or the rule was pointless; no out-of-box experience
  • After: The rule has a sensible default that users will see immediately, with clear placeholders to customize

Watch for:

  • The empty-string check happens at configuration time (setContext), not during file scanning—correct and efficient
  • Minor formatting fix (spacing on line 90) is unrelated cleanup

  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

@sonarqube-next
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@sonar-review-alpha sonar-review-alpha bot left a comment

Choose a reason for hiding this comment

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

One real test coverage gap needs fixing before merge — the rest of the implementation is correct.

🗣️ Give feedback

Comment on lines +116 to +120
check = new FileHeaderCheck();
CheckVerifier.newVerifier()
.onFile(mainCodeSourcesPath("checks/FileHeaderCheck/Class6.java"))
.withCheck(check)
.verifyNoIssues();
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.

The test verifies that Class6.java (which contains the default header) is compliant — but there is no negative test asserting that a file without the default header gets flagged.

This gap matters concretely: if DEFAULT_HEADER_FORMAT ever became empty (e.g., due to a text-block indentation accident), the headerFormat.isEmpty() branch would fire, set expectedLines = {}, and matches() would return true for any file. The verifyNoIssues() call would still pass, giving a false sense of correctness.

Add a complementary case that runs the default FileHeaderCheck() against a file that lacks the header and asserts an issue is raised — something like:

check = new FileHeaderCheck();
CheckVerifier.newVerifier()
  .onFile(mainCodeSourcesPath("checks/FileHeaderCheck/Class1.java"))
  .withCheck(check)
  .verifyIssueOnFile("Add or update the header of this file.");
  • Mark as noise

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant