SONARJAVA-5982 S106 Should not be raise on compact source files#5413
Conversation
tomasz-tylenda-sonarsource
force-pushed
the
tt/SONARJAVA-5982_println_on_compact_cources
branch
from
8e23e5f to
ce5b55b
Compare
tomasz-tylenda-sonarsource
force-pushed
the
tt/SONARJAVA-5982_println_on_compact_cources
branch
from
3cbfb49 to
4809d73
Compare
|
|
|
||
| class RegularClass { | ||
| void log(String message) { | ||
| System.out.println(message); |
There was a problem hiding this comment.
Why do we consider this as compliant?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
rombirli
left a comment
rombirli
left a comment
There was a problem hiding this comment.
LGTM with small comment, IMO we should raise on IO.println as it is equivalent to System.out.println
| 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."); |
There was a problem hiding this comment.
Should we also raise on IO. ... as it is just a convenient way to write System.out. ... ?
tomasz-tylenda-sonarsource
deleted the
tt/SONARJAVA-5982_println_on_compact_cources
branch
3 participants
No description provided.