Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@

package checks.FileHeaderCheck;

public class ClassBlankLine {
}
// Compliant

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.

ClassDefaultHeader.java?

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* <Your-Product-Name>
* Copyright (c) <Year-From>-<Year-To> <Your-Company-Name>
*
* Please configure this header in your SonarCloud/SonarQube quality profile.
*/
package checks.FileHeaderCheck;

public class ClassDefaultHeader {
}
// Compliant (default header)
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package checks.FileHeaderCheck;

public class ClassNoBlankLine {
}
// Compliant
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ public class FileHeaderCheck extends IssuableSubscriptionVisitor {
defaultValue = "false")
public boolean isRegularExpression = false;

private String[] expectedLines;
private Pattern searchPattern = null;

@Override
Expand All @@ -58,18 +57,26 @@ 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()) {
checkExpectedLines(new String[] {});
return;
}

if (!isRegularExpression) {
String[] expectedLines = headerFormat.split("(?:\r)?\n|\r");
checkExpectedLines(expectedLines);
return;
}

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");
}
visitFile();
checkRegularExpression(context.getFileContent());
}

private String getHeaderFormat() {
Expand All @@ -80,13 +87,9 @@ private String getHeaderFormat() {
return format;
}

private void visitFile() {
if (isRegularExpression) {
checkRegularExpression(context.getFileContent());
} else {
if (!matches(expectedLines, context.getFileLines())) {
addIssueOnFile(MESSAGE);
}
private void checkExpectedLines(String[] expectedLines) {
if (!matches(expectedLines, context.getFileLines())) {
addIssueOnFile(MESSAGE);
}
}

Expand All @@ -104,9 +107,9 @@ private static boolean matches(String[] expectedLines, List<String> lines) {
result = true;

Iterator<String> it = lines.iterator();
for (int i = 0; i < expectedLines.length; i++) {
for (String expectedLine : expectedLines) {
String line = it.next();
if (!line.equals(expectedLines[i])) {
if (!line.equals(expectedLine)) {
result = false;
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,26 @@ void test() {
.onFile(mainCodeSourcesPath("checks/FileHeaderCheck/Class3.java"))
.withCheck(check)
.verifyNoIssues();

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

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

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.

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


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

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

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

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

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

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -429,8 +429,8 @@ void do_not_filter_checks_when_no_autoscan() throws IOException {
"CustomRepository:CustomMainCheck",
"CustomRepository:CustomJspCheck",
"CustomRepository:CustomTestCheck",
// not in SonarWay (FileHeaderCheck)
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.

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.

"java:S1451",
// not in SonarWay (CatchUsesExceptionWithContextCheck)
"java:S1166",
// main check in SonarWay (DefaultPackageCheck)
"java:S1220",
// main check in SonarWay, not supported by autoscan (CombineCatchCheck)
Expand Down
Loading