[EC63] UnnecessarilyAssignValuesToVariables rule on caught exception

[natixis-caen]

Describe the bug
Caught exceptions but not used are analyzed as code smells.

To Reproduce
Steps to reproduce the behavior:

1. Add this code snippet in file UnnecessarilyAssignValuesToVariablesTestCheck.java

public void testCompliantCatchException() throws Exception {
    try {
        int i = 1;
        System.out.println(i);
    } catch (Exception e) { // Noncompliant {{The variable is declared but not really used}}
        System.out.println("Something went wrong.");
    }
}

2. Run unit test
3. The code is non compliant

Expected behavior
The caught exception should be allowed to be unused.

Software Versions

• SonarQube Version: Version 9.9.1 (build 69595)
• Plugin Version: 1.4.0

[dedece35]

Hi @natixis-caen,
thank you for issue.
I agree with you for Java language.

Do you think you can make this correction in a new PR ? if yes, I can be the reviewer if you want.

thank you.

[natixis-caen]

Hi @dedece35 ,
I can try to fix it, I guess I have to find a way to exclude exception variables in this method:

    private class GetVariableVisitor extends BaseTreeVisitor {
        @Override
        public void visitVariable(VariableTree tree) {
            if (!tree.parent().is(Kind.METHOD)) {
                variableList.put(tree.simpleName().name(), tree);
            }
            super.visitVariable(tree);
        }
    }

Any help would be appreciated!

[dedece35]

@natixis-caen,
I think it's quite good idea, but you have to explore AST object to exclude detected variables if we are on "catch" block and variable type is "Exception" ... something like that.

[natixis-caen]

Here is the PR:
#230

[dedece35]

Thank you ... do you check de DoD list when a developer contribute ?
please check it and then tell me when ok for review : https://github.com/green-code-initiative/ecoCode-common/blob/main/doc/starter-pack.md#definition-of-done-of-a-pr

[natixis-caen]

Yes I checked it out and it seems like there's no need.

[dedece35]

Hi @natixis-caen,
sorry but regarding DoD list , you didn't create a PR in real test-project to update it with the new use case
(here is real test-project for java language : https://github.com/green-code-initiative/ecoCode-java-test-project/blob/main/src/main/java/fr/greencodeinitiative/java/checks/UnnecessarilyAssignValuesToVariablesTestCheck.java)
Other development is ok for me.

[natixis-caen]

Hello,
I did add a compliant test actually:
https://github.com/green-code-initiative/ecoCode/pull/230/files#diff-0f7d43ab11da0fdb865fdbd6c9f5631f4974452d5541afcd2ba34ea3de97cbe5

Did you missed it or am I totally wrong?

[dedece35]

Hi @natixis-caen, i'm ok with this test but I'm talking about this repository : https://github.com/green-code-initiative/ecoCode-java-test-project
it's real test project that purpose is to test all use cases once local SonarQube is deployed

[natixis-caen]

Here it is: https://github.com/green-code-initiative/ecoCode-java-test-project/pull/11

[dedece35]

Please, wait for draft #258

[dedece35]

Hi @natixis-caen,
like said in related PR #230, the EC63 rule has been deprecated ... check details in PR #230