SONARJAVA-6269 S1451 should correctly handle empty headerFormat

[NoemieBenard]

Refactor FileHeaderCheck to handle empty headers correctly.

[hashicorp-vault-sonar-prod]

SONARJAVA-6269

[sonar-review-alpha]

Summary

This PR fixes SONARJAVA-6269 where FileHeaderCheck was incorrectly raising issues when headerFormat was empty. The fix refactors the check to:

1. Handle empty headers explicitly — Added early return when headerFormat is empty, so files are compliant regardless of what they contain
2. Clarify the control flow — Separated logic paths in setContext() for empty headers, non-regex patterns, and regex patterns, making each case obvious
3. Remove class-level state — expectedLines is no longer stored as a field; it's now computed locally in setContext() when needed

The result is that S1451 no longer produces false positives when users leave headerFormat unconfigured or set it empty, while maintaining the existing behavior for non-empty patterns (both regex and literal text).

What reviewers should know

Start with the main logic: Review the refactored setContext() method in FileHeaderCheck.java (lines 59–77). The three early returns make it clear how the check branches based on configuration.

Key changes to understand:

• The old visitFile() method is renamed to checkExpectedLines() and now takes the expected lines as a parameter (no longer relies on instance state)
• Empty headerFormat is now handled as a special case, not as a potential bug
• The for-loop in matches() was modernized to an enhanced for-loop (minor cleanup)

Test coverage: New test files and test cases cover all three scenarios now:

• Empty headerFormat with/without leading blank lines → should pass
• Default header (unchanged configuration) → should pass
• Both regex and non-regex modes with empty headerFormat → should pass

Note: The removal of S1451 from JavaSensorTest (line 432) is intentional — it documents that this check is no longer expected to produce issues in the default configuration after this fix.

---

• Generate Walkthrough
• Generate Diagram

🗣️ Give feedback

[tomasz-tylenda-sonarsource]

ClassDefaultHeader.java?

[tomasz-tylenda-sonarsource]

Do you really need this file and RegexNoBlankLine.java? Why not reuse ClassBlankLine.java‎?

[sonar-review-alpha]

The logic fix is correct and the refactoring is clean. One test case is not actually exercising the bug, which slightly weakens the test suite's diagnostic value.

🗣️ Give feedback

[sonar-review-alpha]

ClassBlankLine.java starts with a blank line. Under the old code, "".split(…) produced [""], and matches([""], lines) compared the first line to "" — a blank-first-line file already passed. This test case would have been green before the fix too, so it doesn't verify anything new.

The real non-regex regression test is ClassNoBlankLine.java (first line is package …, which the old code incorrectly flagged). Consider replacing ClassBlankLine.java with a file that would have been mistakenly flagged under the old behavior — or just remove this case, since ClassNoBlankLine.java already covers the fix.

• Mark as noise

[sonar-review-alpha]

The logic is correct and the refactoring is clean. The previously raised concern about ClassBlankLine.java as a non-regex regression test remains open.

🗣️ Give feedback

[sonar-review-alpha]

ClassBlankLine.java starts with a blank line (line 1 is ""). Under the old code, "".split("(?:\r)?\n|\r") returned [""], and matches([""], fileLines) compared the first line "" to the expected "" — a match, so no issue was reported. This test was already green before the fix and doesn't verify anything new for the non-regex path.

The real regression test for the non-regex fix is ClassNoBlankLine.java immediately below (first line is "package...", which the old code incorrectly flagged). Either remove the ClassBlankLine.java case here, or replace it with a comment explaining that it's not a regression test but a boundary-condition sanity check.

• Mark as noise

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

[sonar-review-alpha]

LGTM! ✅

Clean fix for two related bugs triggered by an empty headerFormat. The refactoring is correct and the logic is easy to follow.

Behavior verified:

• Empty array passed to matches() always returns true (0 <= lines.size() holds even for empty files; the for-each never executes), so no issue is ever raised — exactly the desired "skip check" semantics.
• The regex path is fully guarded: the empty check fires before getHeaderFormat() is reached, preventing the pre-existing StringIndexOutOfBoundsException on "".charAt(0).
• The searchPattern lazy-init guard is unchanged and still correct for non-empty regex headers.

🗣️ Give feedback