Skip to content

SONARJAVA-5982 S106 Should not be raise on compact source files #5413

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

Merged
tomasz-tylenda-sonarsource merged 1 commit into from
Feb 4, 2026
Merged

SONARJAVA-5982 S106 Should not be raise on compact source files #5413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions ...t-sources/default/src/main/java/checks/SystemOutOrErrUsageCheckCompactOnlyMainSample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
void main() {
String name = IO.readln("Enter your name: ");
IO.println("Hello " + name + "!");
System.out.println("Goodbye " + name + "!");
}
21 changes: 21 additions & 0 deletions ...-sources/default/src/main/java/checks/SystemOutOrErrUsageCheckCompactWithClassSample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
void main() {
String name = IO.readln("Enter your name: ");
IO.println("Hello " + name + "!");
System.out.println("Goodbye " + name + "!");

System.out.println(f(6, 7));

RegularClass rc = new RegularClass();
rc.log("This is a log message.");
}

int f(int x, int y) {
System.err.println("Error: x=" + x + ", y=" + y);
return x + y;
}

class RegularClass {
void log(String message) {
System.out.println(message);
Copy link
Contributor

@asya-vorobeva asya-vorobeva Feb 3, 2026

Choose a reason for hiding this comment

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

Why do we consider this as compliant?

Copy link
Contributor Author

@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource Feb 3, 2026

Choose a reason for hiding this comment

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

My thinking was that using a compact source file indicated that the developer, maybe a student, is writing a small script (single file) or just learning Java. RegularClass is nested under an unnamed implicit class, so it will not be reused and is likely related to the main method, so we should not require a logger there either. WDYT?

Copy link
Contributor

@asya-vorobeva asya-vorobeva Feb 3, 2026

Choose a reason for hiding this comment

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

So, if I understand correctly, we assume that if the developer is beginner, it's not required to use such sophisticated things as loggers, right? As for me, it looks like contradicting to Sonar goals - persuade developers to write better code.
And in the documentation of CSF I see the following:

The motivation for this work is, moreover, not only to help beginning programmers. We aim to help everyone who writes small programs, whether they be students, system administrators writing command-line utilities, or domain experts prototyping core algorithms that will eventually be used in the heart of an enterprise-scale software system.

So how can we be sure who wrote this specific snippet?

Copy link
Contributor Author

@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource Feb 3, 2026

Choose a reason for hiding this comment

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

I personally dislike it when I write a simple program to tests something and Sonar complaint about every single println - it's noisy and not meaningful. I thought of CSF as a good opportunity to fix something that bothers me.

Maybe we can chat about this with the entire squad during the coffee break?

Copy link
Contributor

@asya-vorobeva asya-vorobeva Feb 3, 2026

Choose a reason for hiding this comment

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

Yep, good idea!

Copy link
Contributor

@rombirli rombirli Feb 4, 2026

Choose a reason for hiding this comment

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

In my opinion, classes within a compact source file should be allowed to use System.out or IO directly, since they are only used within that same file.

}
}
24 changes: 20 additions & 4 deletions java-checks/src/main/java/org/sonar/java/checks/SystemOutOrErrUsageCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,36 @@
@Rule(key = "S106")
public class SystemOutOrErrUsageCheck extends IssuableSubscriptionVisitor {

private boolean isCompactSourceFile = false;

@Override
public List<Tree.Kind> nodesToVisit() {
return Collections.singletonList(Tree.Kind.MEMBER_SELECT);
return List.of(
Tree.Kind.COMPILATION_UNIT,
Tree.Kind.IMPLICIT_CLASS,
Tree.Kind.MEMBER_SELECT
);
}

@Override
public void visitNode(Tree tree) {
MemberSelectExpressionTree mset = (MemberSelectExpressionTree) tree;
if (tree.is(Tree.Kind.COMPILATION_UNIT)) {
isCompactSourceFile = false;
} else if (tree.is(Tree.Kind.IMPLICIT_CLASS)) {
// System.out or System.err is allowed in compact source files.
isCompactSourceFile = true;
} else if (!isCompactSourceFile && tree instanceof MemberSelectExpressionTree mset) {
visitMemberSelectExpression(mset);
}
}

private void visitMemberSelectExpression(MemberSelectExpressionTree mset) {
String name = mset.identifier().name();

if ("out".equals(name) && isSystem(mset.expression())) {
reportIssue(tree, "Replace this use of System.out by a logger.");
reportIssue(mset, "Replace this use of System.out by a logger.");
} else if ("err".equals(name) && isSystem(mset.expression())) {
reportIssue(tree, "Replace this use of System.err by a logger.");
reportIssue(mset, "Replace this use of System.err by a logger.");
Comment on lines 56 to +61
Copy link
Contributor

@rombirli rombirli Feb 4, 2026

Choose a reason for hiding this comment

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

Should we also raise on IO. ... as it is just a convenient way to write System.out. ... ?

Copy link
Contributor Author

@tomasz-tylenda-sonarsource tomasz-tylenda-sonarsource Feb 4, 2026

Choose a reason for hiding this comment

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

Yes. SONARJAVA-6090/

}
}

Expand Down
16 changes: 16 additions & 0 deletions java-checks/src/test/java/org/sonar/java/checks/SystemOutOrErrUsageCheckTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,20 @@ void test() {
.withCheck(new SystemOutOrErrUsageCheck())
.verifyIssues();
}

@Test
void test_compact_source_file() {
CheckVerifier.newVerifier()
.onFile(mainCodeSourcesPath("checks/SystemOutOrErrUsageCheckCompactOnlyMainSample.java"))
.withCheck(new SystemOutOrErrUsageCheck())
.verifyNoIssues();
}

@Test
void test_compact_source_file_with_regular_class() {
CheckVerifier.newVerifier()
.onFile(mainCodeSourcesPath("checks/SystemOutOrErrUsageCheckCompactWithClassSample.java"))
.withCheck(new SystemOutOrErrUsageCheck())
.verifyNoIssues();
}
}
Loading