Skip to content

SONARJAVA-6197 S1451 should not fail at analysis time when an empty headerFormat rule property marked as a regular expression is provided #5577

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
NoemieBenard wants to merge 5 commits into
base: master
Choose a base branch
from
Open

SONARJAVA-6197 S1451 should not fail at analysis time when an empty headerFormat rule property marked as a regular expression is provided #5577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5f47ccc
modify S1451 to accept empty headerFormat
NoemieBenard Apr 16, 2026
5c9bbeb
fix incorrect test
NoemieBenard Apr 17, 2026
c0f6858
add tests for regex
NoemieBenard Apr 17, 2026
0874843
modify check so that it accepts an empty header in both cases (regex …
NoemieBenard Apr 17, 2026
ce69613
remove unused import
NoemieBenard Apr 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions java-checks-test-sources/default/src/main/java/checks/FileHeaderCheck/Class4.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@

package checks.FileHeaderCheck;

public class Class4 {
}
// Compliant

5 changes: 5 additions & 0 deletions java-checks-test-sources/default/src/main/java/checks/FileHeaderCheck/Class5.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package checks.FileHeaderCheck;

public class Class5 {
}
// Compliant
6 changes: 6 additions & 0 deletions java-checks-test-sources/default/src/main/java/checks/FileHeaderCheck/Regex5.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@

package checks.FileHeaderCheck;

public class Regex5 {
}
// Compliant
5 changes: 5 additions & 0 deletions java-checks-test-sources/default/src/main/java/checks/FileHeaderCheck/Regex6.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package checks.FileHeaderCheck;

public class Regex6 {
}
// Compliant
22 changes: 13 additions & 9 deletions java-checks/src/main/java/org/sonar/java/checks/FileHeaderCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,21 @@ public List<Tree.Kind> nodesToVisit() {
@Override
public void setContext(JavaFileScannerContext context) {
super.context = context;
if (isRegularExpression) {
if (searchPattern == null) {
try {
searchPattern = Pattern.compile(getHeaderFormat(), Pattern.DOTALL);
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException("[" + getClass().getSimpleName() + "] Unable to compile the regular expression: " + headerFormat, e);
if (headerFormat.isEmpty()) {
expectedLines = new String[]{};
isRegularExpression = false;
Comment on lines +61 to +63
Copy link
Copy Markdown
Contributor

@sonar-review-alpha sonar-review-alpha bot Apr 17, 2026

Choose a reason for hiding this comment

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

isRegularExpression is a @RuleProperty-annotated public field set by the SonarQube framework to represent user configuration. Mutating it inside setContext() means the field no longer reflects the user's intent after the first file is scanned.

In practice this is harmless — the isEmpty() guard is re-evaluated on every call so the mutation is always re-applied for empty headers. But if the framework ever inspects or logs the field value after scanning (for diagnostics or rule-parameter serialization), it will see false even when the user explicitly set true.

A cleaner approach: replace the mutation with a local boolean that shadows the field for the duration of the call:

boolean effectiveRegex = isRegularExpression;
if (headerFormat.isEmpty()) {
  expectedLines = new String[]{};
  effectiveRegex = false;
} else if (isRegularExpression) {
  ...
}

This keeps rule-property state immutable and makes the intent explicit.

Suggested change
if (headerFormat.isEmpty()) {
expectedLines = new String[]{};
isRegularExpression = false;
if (headerFormat.isEmpty()) {
expectedLines = new String[]{};
} else {
if (isRegularExpression) {
if (searchPattern == null) {
try {
searchPattern = Pattern.compile(getHeaderFormat(), Pattern.DOTALL);
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException("[" + getClass().getSimpleName() + "] Unable to compile the regular expression: " + headerFormat, e);
}
}
} else {
expectedLines = headerFormat.split("(?:\r)?\n|\r");
}
}
  • Mark as noise

} else {
if (isRegularExpression) {
if (searchPattern == null) {
try {
searchPattern = Pattern.compile(getHeaderFormat(), Pattern.DOTALL);
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException("[" + getClass().getSimpleName() + "] Unable to compile the regular expression: " + headerFormat, e);
}
}
} else {
expectedLines = headerFormat.split("(?:\r)?\n|\r");
}
} else {
expectedLines = headerFormat.split("(?:\r)?\n|\r");
}
visitFile();
}
Expand Down Expand Up @@ -114,7 +119,6 @@ private static boolean matches(String[] expectedLines, List<String> lines) {
} else {
result = false;
}

return result;
}

Expand Down
26 changes: 26 additions & 0 deletions java-checks/src/test/java/org/sonar/java/checks/FileHeaderCheckTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,18 @@ void test() {
.onFile(mainCodeSourcesPath("checks/FileHeaderCheck/Class3.java"))
.withCheck(check)
.verifyNoIssues();

check = new FileHeaderCheck();
CheckVerifier.newVerifier()
.onFile(mainCodeSourcesPath("checks/FileHeaderCheck/Class4.java"))
.withCheck(check)
.verifyNoIssues();

check = new FileHeaderCheck();
CheckVerifier.newVerifier()
.onFile(mainCodeSourcesPath("checks/FileHeaderCheck/Class5.java"))
.withCheck(check)
.verifyNoIssues();
}

@Test
Expand Down Expand Up @@ -139,6 +151,20 @@ void regex() {
.onFile(mainCodeSourcesPath("checks/FileHeaderCheck/Regex4.java"))
.withCheck(check)
.verifyIssues();

check = new FileHeaderCheck();
check.isRegularExpression = true;
CheckVerifier.newVerifier()
.onFile(mainCodeSourcesPath("checks/FileHeaderCheck/Regex5.java"))
.withCheck(check)
.verifyNoIssues();

check = new FileHeaderCheck();
check.isRegularExpression = true;
CheckVerifier.newVerifier()
.onFile(mainCodeSourcesPath("checks/FileHeaderCheck/Regex6.java"))
.withCheck(check)
.verifyNoIssues();
}

@Test
Expand Down
Loading