SONARGO-113 Rewrite tests IdenticalBinaryOperandCheckTest - OctalValuesCheckTest #86
Conversation
|
|
Coverage is 100%, no follow-up ticket to rework the checks needed |
| SwitchCaseTooBigCheck.class, | ||
| SwitchWithoutDefaultCheck.class, | ||
| NestedSwitchCheck.class, |
There was a problem hiding this comment.
Renamed to be more Go-specific
| @@ -29,17 +29,16 @@ | |||
| @Rule(key = "S1764") | |||
| public class IdenticalBinaryOperandCheck implements SlangCheck { | |||
|
|
|||
| public static final String MESSAGE = "Correct one of the identical sub-expressions on both sides this operator"; | |||
There was a problem hiding this comment.
To trigger SQS coverage
| @@ -57,19 +56,19 @@ private static List<Tree> collectConditions(MatchTree matchTree) { | |||
|
|
|||
| private static List<Tree> collectConditions(IfTree ifTree, List<Tree> list) { | |||
| list.add(skipParentheses(ifTree.condition())); | |||
| Tree elseBranch = ifTree.elseBranch(); | |||
| var elseBranch = ifTree.elseBranch(); | |||
There was a problem hiding this comment.
To trigger SQS coverage
| @@ -44,16 +44,16 @@ public class IfConditionalAlwaysTrueOrFalseCheck implements SlangCheck { | |||
| @Override | |||
| public void initialize(InitContext init) { | |||
| init.register(IfTree.class, (ctx, ifTree) -> { | |||
| Tree condition = ifTree.condition(); | |||
| var condition = ifTree.condition(); | |||
There was a problem hiding this comment.
To trigger SQS coverage
| .ifPresent(parentMatch -> reportIssue(ctx, matchTree))); | ||
| } | ||
|
|
||
| private static void reportIssue(CheckContext ctx, MatchTree matchTree) { |
There was a problem hiding this comment.
To trigger SQS coverage
| @@ -42,7 +42,7 @@ public void initialize(InitContext init) { | |||
|
|
|||
| private static boolean isException(IntegerLiteralTree literalTree) { | |||
| // octal literal < 8 are authorized, as well as octal literals with 3 digits, as they are often used for file permissions | |||
| BigInteger value = literalTree.getIntegerValue(); | |||
| var value = literalTree.getIntegerValue(); | |||
There was a problem hiding this comment.
To trigger SQS coverage
| @@ -40,7 +40,7 @@ public class MatchCaseTooBigCheck implements SlangCheck { | |||
| @Override | |||
| public void initialize(InitContext init) { | |||
| init.register(MatchCaseTree.class, (ctx, matchCaseTree) -> { | |||
| int linesOfCode = matchCaseTree.metaData().linesOfCode().size(); | |||
| var linesOfCode = matchCaseTree.metaData().linesOfCode().size(); | |||
There was a problem hiding this comment.
To trigger SQS coverage
|
|
||
| @Override | ||
| public void initialize(InitContext init) { | ||
| init.register(MatchTree.class, (ctx, tree) -> { | ||
| if (tree.cases().stream().noneMatch(matchCase -> matchCase.expression() == null)) { | ||
| Token keyword = tree.keyword(); | ||
| String message = String.format("Add a default clause to this \"%s\" statement.", keyword.text()); | ||
| var keyword = tree.keyword(); |
There was a problem hiding this comment.
To trigger SQS coverage
| return 1 | ||
| } | ||
|
|
||
| if n := 3; true { // False negative |
There was a problem hiding this comment.
Created SONARGO-119
| return 1 | ||
| } else if cond && false { // Noncompliant | ||
| return 1 | ||
| } else if isFoo() && false { // Compliant, isFoo can have a side-effect |
There was a problem hiding this comment.
Not sure if it's a correct exception or if we should raise an issue (and create a follow-up ticket)
In the original Slang tests, this case was not documented. It's not a properly defined exception in the check's code either 🤔
There was a problem hiding this comment.
For me this would definitely be a problematic code. However, the existing error message Remove this useless if is not very well applicable here. Maybe we have another rule that can flag this?
Edit: however, this rule also checks boolean expressions with variables, so maybe we can just customize error messages to smth like Remove this useless if statement: condition will always have the same value. If call to isFoo is required, extract it.
There was a problem hiding this comment.
Maybe we have another rule that can flag this?
Indeed, it should be covered by S1125, actually, as it is done in Java and other flagship languages 😄
|
This PR is stale because it has been open 7 days with no activity. If there is no activity in the next 7 days it will be closed automatically |
SONARGO-113