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

Refactors FileHeaderCheck to explicitly handle empty headerFormat configuration. Previously, empty header formats were not handled consistently; now they're treated as "no header requirement" (no issues reported). The refactoring also improves code clarity by:

• Restructuring setContext with early returns for different code paths (empty format, non-regex, regex)
• Replacing instance variable expectedLines with local variable scoped to non-regex path
• Renaming visitFile() to checkExpectedLines() for clarity
• Modernizing the matches() method to use enhanced for-loops

Includes test coverage for empty headerFormat in both regex and non-regex modes, plus the default (unconfigured) header behavior.

What reviewers should know

Where to start: Review the refactored setContext method (lines 58-80) — the control flow is now clearer with explicit returns for each configuration scenario.

Key behavior change: When headerFormat is empty, the check now reports no issues. This is intentional (fixing SONARJAVA-6269) and tested in FileHeaderCheckTest.

Implementation details:

• Empty format check happens first with early return (lines 61-64)
• Non-regex mode extracts and checks expected lines locally (lines 66-70)
• Regex mode compiles and checks pattern (lines 72-79)
• The matches() method still has the same logic but uses an enhanced for-loop (line 110) instead of index-based iteration

Tests to focus on:

• FileHeaderCheckTest: Added test cases for empty format (both modes) and default behavior (lines 101-121, 162-176)
• New test source files demonstrate compliant cases: ClassBlankLine.java, ClassNoBlankLine.java, ClassDefaultHeader.java
• JavaSensorTest: Unrelated update replacing S1451 reference with S1166 (line 432)

---

• 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

[tomasz-tylenda-sonarsource]

Does this test still need a fix? I could not reproduce it locally. If it does, could we find a different rule, not in SonarWay, which fails? Maybe S1166.

We can discuss it offline.

[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! ✅

The new commit is a clean follow-up: S1451 no longer fires on the test files after the empty-header fix, so it's replaced with S1166 (CatchUsesExceptionWithContextCheck) in the sensor test to maintain coverage of the "not in SonarWay" assertion. No issues with this change. The two previously flagged concerns about ClassBlankLine.java test coverage in FileHeaderCheckTest remain unresolved.

🗣️ Give feedback