Skip to content
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.

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

[69] NullPointer exception in eco code java Sonar plugin #70

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- [#49](https://github.com/green-code-initiative/ecoCode-java/pull/49) Add test to ensure all Rules are registered
- [#336](https://github.com/green-code-initiative/ecoCode/issues/336) [Adds Maven Wrapper](https://github.com/green-code-initiative/ecoCode-java/pull/67)
- [#69](https://github.com/green-code-initiative/ecoCode-java/pull/69) correction of NullPointer in EC79 rule

### Deleted

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ public void visitNode(Tree tree) {
@Override
public void leaveNode(Tree tree) {
if (tree.is(Tree.Kind.TRY_STATEMENT)) {
if(!withinTry.isEmpty()) {
withinTry.pop();
}
List<Tree> secondaryTrees = toReport.pop();
if (!secondaryTrees.isEmpty()) {
reportIssue(tree, MESSAGE_RULE);
Expand Down
45 changes: 45 additions & 0 deletions src/test/files/FreeResourcesOfAutoCloseableInterface2.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package files;
dedece35 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

please rename this test class with an explicit name corresponding to use case instead of "2"

Copy link
Member

Choose a reason for hiding this comment

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

I will done it if you want after the merge of this PR (if you don't have time to do it)


import java.io.FileWriter;
import java.io.IOException;

/*
* ecoCode - Java language - Provides rules to reduce the environmental footprint of your Java programs
* Copyright © 2023 Green Code Initiative (https://www.ecocode.io)
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
public class FreeResourcesOfAutoCloseableInterface2 {

/**
* The first methods adds a "try" in the stack used to follow if the code is in a try
*/
public void callingMethodWithTheTry() throws IOException {
try {
calledMethodWithoutTry();
} finally {
// Empty block of code
}
}

/**
* The "try" should have been poped from the stack before entering here
*/
private void calledMethodWithoutTry() throws IOException {
FileWriter myWriter = new FileWriter("somefilepath");
Copy link
Member

Choose a reason for hiding this comment

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

sorry, but for me, this use case is an "non compliant" case.
This is a good example explaining that if the FileWriter is instanciated inside a private method, it must be protected with a try-with-resource. In this case, it must be done.
Sorry I can't accept the correction ... for me, the global algorithm must be refactored to manage this use case !

myWriter.write("something");
myWriter.flush();
myWriter.close();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,11 @@ void test_no_java_version() {
.withCheck(new FreeResourcesOfAutoCloseableInterface())
.verifyIssues();
}
@Test
void test_when_try_before_auto_closeable_but_different_hierarchy_of_code() {
CheckVerifier.newVerifier()
.onFile("src/test/files/FreeResourcesOfAutoCloseableInterface2.java")
.withCheck(new FreeResourcesOfAutoCloseableInterface())
.verifyNoIssues();
}
}
Loading