Added resource leak transform and general codemod #339
Conversation
There was a problem hiding this comment.
How do you like this plan:
- Take the new codemod out of
DefaultCodemods - Create tickets for the various improvements discussed here
- Ship this PR
- Work follow-on tickets
- Add back to
DefaultCodemods
That way, we get to keep moving forward, improve incrementally, and don't have to give our users changes they may have formatting objections towards.
| @@ -117,6 +117,7 @@ void it_transforms_webgoat_with_codeql() throws Exception { | |||
| outputFile.getPath(), | |||
| "--sarif", | |||
| "src/test/resources/webgoat_v8.2.2_codeql.sarif", | |||
There was a problem hiding this comment.
This could be solved by making the CodeQL one run with CodemodExecutionPriority.LOW, maybe?
core-codemods/src/main/java/io/codemodder/codemods/ResourceLeakCodemod.java
Show resolved
Hide resolved
core-codemods/src/main/java/io/codemodder/codemods/ResourceLeakFixer.java
Show resolved
Hide resolved
core-codemods/src/main/java/io/codemodder/codemods/ResourceLeakFixer.java
Show resolved
Hide resolved
core-codemods/src/test/resources/prevent-filewriter-leak-with-nio/Test.java.after
Show resolved
Hide resolved
|
Also, this PR and the changes are awesome! |
|
|
||
| @Override | ||
| public CodemodFileScanningResult visit( | ||
| final CodemodInvocationContext context, final CompilationUnit cu) { |
There was a problem hiding this comment.
This will be an extremely expensive codemod to execute as it will force every file to be visited, and every Expression to be evaluated.
I think we need a Semgrep pattern to detect reasonable starting points that can be confirmed more thoroughly by AST-inspecting code.
There was a problem hiding this comment.
This is a guess, but I think it's not that much to test. Again, maybe we should add it to another ticket, and make sure we check that ticket off before we add to DefaultCodemods.
There was a problem hiding this comment.
The only reason it visits Expression is that we cannot express sum types easily. Notice that the first thing it does is test if the given expression is ObjectCreationExpression or MethodCallExpression. After that the first check is if it is a AutoCloseable type. I think those two check are fast and restrictive enough to nip most visits without spending much.
core-codemods/src/main/java/io/codemodder/codemods/ResourceLeakFixer.java
Show resolved
Hide resolved
- Small changes to method names in test. - Changed the way resource generation methods in Files are detected.
|
Added a new resource leak transform and general codemod
You may notice the changes to the PreventFileWriterLeakWithFilesCodemod test files. This is due to a newly discovered issue. Refer to #338.