SONARGO-111 Rewrite tests AllBranchesIdenticalCheckTest - EmptyBlockCheckTest #90
Conversation
| init.register(IfTree.class, (ctx, ifTree) -> { | ||
| if (isIfWithMaxTwoBranches(ctx.parent(), ifTree) && !hasBlockBranch(ifTree)) { | ||
| getBooleanLiteral(ifTree.thenBranch(), ifTree.elseBranch()) | ||
| .ifPresent(booleanLiteral -> ctx.reportIssue(booleanLiteral, MESSAGE)); | ||
| } | ||
| }); | ||
|
|
There was a problem hiding this comment.
IF cannot be inlined in GO, so removing this part of the code.
| @@ -87,7 +88,7 @@ private static boolean hasMinimumSize(FunctionDeclarationTree function) { | |||
| if (functionBody == null) { | |||
| return false; | |||
| } | |||
| return functionBody.statementOrExpressions().size() >= MINIMUM_STATEMENTS_COUNT; | |||
| return functionBody.statementOrExpressions().stream().filter(TreeUtils.IS_NOT_SEMICOLON).count() >= MINIMUM_STATEMENTS_COUNT; | |||
There was a problem hiding this comment.
Semicolon are threated as statements so ignoring them.
| private static boolean isPanicCall(Tree tree) { | ||
| return tree instanceof NativeTree exptStmtNativeTree | ||
| && exptStmtNativeTree.nativeKind().toString().equals("[0](ExprStmt)") | ||
| && exptStmtNativeTree.children().size() == 1 | ||
| && exptStmtNativeTree.children().get(0) instanceof NativeTree callExprNativeTree | ||
| && callExprNativeTree.nativeKind().toString().equals("X(CallExpr)") | ||
| && !callExprNativeTree.children().isEmpty() | ||
| && callExprNativeTree.children().get(0) instanceof IdentifierTree identifierTree | ||
| && identifierTree.name().equals("panic"); | ||
| } |
There was a problem hiding this comment.
There is now throw in Go, but Panic is relatively equivalent in that use case (left the normal execution flow).
| @@ -44,11 +49,11 @@ public void initialize(InitContext init) { | |||
| } | |||
|
|
|||
| IfTree prevTree = ifTree; | |||
| boolean endsWithReturn = endsWithReturnBreakOrThrow(ifTree); | |||
| boolean endsWithReturn = endsWithReturnBreakOrPanic(ifTree); | |||
There was a problem hiding this comment.
I'm not sure if we should change the behaviour of this rule 🤔 Before it raised when panic() was called in last if else statement, what we can see in its/ruling/src/integrationTest/resources/expected/go-S126.json change.
IMO this rule should raise issues even if one of the branch contains throw/return/break/panic().
The rule is not in Sonar_Way so I didn't found any issue on "Next" and the impact is low so it can be as it is now.
There was a problem hiding this comment.
This change is just about replacing throw by panic, which could be considered the equivalent in Go (at least in this context of execution flow). Before, it simply ignored any panic call.
To me, the new issue raised in its/ruling/src/integrationTest/resources/expected/go-S126.json looks like a TP according to the rule behavior.
I agree that I don't see the why of the logic around throw/return/break/panic(), but it would remove most logic implemented in the check, which would be a significant impact, probably outside of the scope of simply migrating this rule to go 😅
sonar-go-checks/src/main/java/org/sonar/go/checks/ElseIfWithoutElseCheck.java
Show resolved
Hide resolved
sonar-go-checks/src/test/resources/checks/BadFunctionNameCheck/bad_function_name.go
Outdated
Show resolved
Hide resolved
sonar-go-checks/src/test/resources/checks/BadFunctionNameCheck/bad_function_name.go
Show resolved
Hide resolved
sonar-go-checks/src/test/resources/checks/BooleanInversionCheck/boolean_inversion.go
Outdated
Show resolved
Hide resolved
...resources/checks/DuplicatedFunctionImplementationCheck/duplicated_function_implementation.go
Outdated
Show resolved
Hide resolved
...resources/checks/DuplicatedFunctionImplementationCheck/duplicated_function_implementation.go
Outdated
Show resolved
Hide resolved
...resources/checks/DuplicatedFunctionImplementationCheck/duplicated_function_implementation.go
Outdated
Show resolved
Hide resolved
...resources/checks/DuplicatedFunctionImplementationCheck/duplicated_function_implementation.go
Outdated
Show resolved
Hide resolved
...resources/checks/DuplicatedFunctionImplementationCheck/duplicated_function_implementation.go
Outdated
Show resolved
Hide resolved
|
SONARGO-111