Excludes for createTempFile forbidden method #14465

relentless-pursuit · 2024-02-11T20:38:00Z

This is in continuation to #14220.

We need to refactor below classes by removing createTempFile, createTempDir (if any) occurences as mentioned in the linked issue.

[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.filters.SuppressionFilterTest (SuppressionFilterTest.java:171)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.filters.SuppressionFilterTest (SuppressionFilterTest.java:174)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.filters.SuppressionFilterTest (SuppressionFilterTest.java:203)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.filters.SuppressionFilterTest (SuppressionFilterTest.java:207)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.utils.XpathUtilTest (XpathUtilTest.java:103)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.utils.XpathUtilTest (XpathUtilTest.java:123)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.utils.XpathUtilTest (XpathUtilTest.java:140)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.utils.XpathUtilTest (XpathUtilTest.java:168)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.TreeWalkerTest (TreeWalkerTest.java:223)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.TreeWalkerTest (TreeWalkerTest.java:237)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.TreeWalkerTest (TreeWalkerTest.java:289)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.TreeWalkerTest (TreeWalkerTest.java:548)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.TreeWalkerTest (TreeWalkerTest.java:560)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.TreeWalkerTest (TreeWalkerTest.java:572)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.TreeWalkerTest (TreeWalkerTest.java:587)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.TreeWalkerTest (TreeWalkerTest.java:590)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.TreeWalkerTest (TreeWalkerTest.java:621)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.TreeWalkerTest (TreeWalkerTest.java:623)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.PropertyCacheFileTest (PropertyCacheFileTest.java:95)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.PropertyCacheFileTest (PropertyCacheFileTest.java:152)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.PropertyCacheFileTest (PropertyCacheFileTest.java:172)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.PropertyCacheFileTest (PropertyCacheFileTest.java:201)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.PropertyCacheFileTest (PropertyCacheFileTest.java:202)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.PropertyCacheFileTest (PropertyCacheFileTest.java:229)
[ERROR] Forbidden method invocation: java.nio.file.Files#createTempFile(java.lang.String,java.lang.String,java.nio.file.attribute.FileAttribute[]) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.PropertyCacheFileTest (PropertyCacheFileTest.java:330)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.PropertyCacheFileTest (PropertyCacheFileTest.java:379)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.PropertyCacheFileTest (PropertyCacheFileTest.java:427)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.PropertyCacheFileTest (PropertyCacheFileTest.java:458)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.PropertyCacheFileTest (PropertyCacheFileTest.java:494)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.utils.FilterUtilTest (FilterUtilTest.java:44)
[ERROR] Forbidden method invocation: java.nio.file.Files#createTempFile(java.nio.file.Path,java.lang.String,java.lang.String,java.nio.file.attribute.FileAttribute[]) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in org.checkstyle.suppressionxpathfilter.AbstractXpathTestSupport (AbstractXpathTestSupport.java:124)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.checks.javadoc.AbstractJavadocCheckTest (AbstractJavadocCheckTest.java:365)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.checks.imports.ImportControlCheckTest (ImportControlCheckTest.java:344)
[ERROR] Forbidden method invocation: java.io.File#createTempFile(java.lang.String,java.lang.String,java.io.File) [Use of this method is forbidden, please use @TempDir for better temporary directory control]
[ERROR]   in com.puppycrawl.tools.checkstyle.checks.imports.ImportControlCheckTest (ImportControlCheckTest.java:347)
[ERROR] Scanned 3799 class file(s) for forbidden API invocations (in 1.89s), 34 error(s).

Once, you have refactored the code, you can remove the name of the corresponding class from excludes section in pom.xml

<!-- Excludes for createTempFile forbidden method -->
                <exclude>**/SuppressionFilterTest.class</exclude>
                <exclude>**/XpathUtilTest.class</exclude>
                <exclude>**/TreeWalkerTest.class</exclude>
                <exclude>**/PropertyCacheFileTest.class</exclude>
                <exclude>**/FilterUtilTest.class</exclude>
                <exclude>**/AbstractXpathTestSupport.class</exclude>
                <exclude>**/AbstractJavadocCheckTest.class</exclude>
                <exclude>**/ImportControlCheckTest.class</exclude>

The text was updated successfully, but these errors were encountered:

relentless-pursuit · 2024-02-12T06:34:42Z

I have some confusion on this issue. I have highlighted the same here. #14414 (comment). Once, I have some clarity, we can start working.

MANISH-K-07 · 2024-02-12T09:54:27Z

@rnveach
cc @romani , @nrmancuso
Firstly, sorry to get involved.
Please correct me if my understanding is wrong....

Response to #14414 (comment) :

But, i am afraid if there is any similar annotation like @tempdir as such for the createTempFile method.

@relentless-pursuit , you are actually right. I don't think there is any alternative as such.
However, can't we solve the issue at hand keeping the createTempFile as it is ?

From #14220 (comment) :

Files.createTempDirectory does not obey the system property java.io.tmpdir and does not attempt to clean up the temporary directory on completion of the run.

The major issue that I see with createTempFie is the lack of clean-up property.

Since the files are already a part of temporaryFolder , could we not simply add an explicit deletion ?..
Won't a simple closing call to deleteOnExit() suffice ?? This can't be hard as only 2-3 instances of said usage are found per test

relentless-pursuit · 2024-02-12T13:07:12Z

@rnveach cc @romani , @nrmancuso Firstly, sorry to get involved. Please correct me if my understanding is wrong....

Response to #14414 (comment) :

But, i am afraid if there is any similar annotation like @tempdir as such for the createTempFile method.

@relentless-pursuit , you are actually right. I don't think there is any alternative as such. However, can't we solve the issue at hand keeping the createTempFile as it is ?

From #14220 (comment) :

Files.createTempDirectory does not obey the system property java.io.tmpdir and does not attempt to clean up the temporary directory on completion of the run.

The major issue that I see with createTempFie is the lack of clean-up property.

Since the files are already a part of temporaryFolder , could we not simply add an explicit deletion ?.. Won't a simple closing call to deleteOnExit() suffice ?? This can't be hard as only 2-3 instances of said usage are found per test

No worries. Feel free to contribute to enrich the PR. As i had limited understanding in the beginning, and assumed we mandatorily need to create files, but I don't think that is the case. However, some test cases need actual files. And, i have used File.createFile instead of Files.createTempFile.

MANISH-K-07 · 2024-02-12T13:46:06Z

No worries. Feel free to contribute to enrich the PR. As i had limited understanding in the beginning, and assumed we mandatorily need to create files, but I don't think that is the case. However, some test cases need actual files. And, i have used File.createFile instead of Files.createTempFile.

Seems you have found a workaround ;)

As mentioned at #14414 (comment) ,
I'm still not sure if using the TempDir ensures that the temp files in the folder are always cleaned up though !!
Is this not of concern ?

rnveach · 2024-02-12T17:42:27Z

Please look at API for cleanup.

https://junit.org/junit5/docs/5.4.1/api/org/junit/jupiter/api/io/TempDir.html

Temporary Directory Deletion

When the end of the scope of a temporary directory is reached, i.e. when the test method or class has finished execution, JUnit will attempt to recursively delete all files and directories in the temporary directory and, finally, the temporary directory itself. In case deletion of a file or directory fails, an IOException will be thrown that will cause the test or test class to fail.

You can also see the same code if you browse the TempDir and its javadocs pulled in from maven.

Clean Up

By default, when the end of the scope of a temporary directory is reached,

— when the test method or class has finished execution — JUnit will

attempt to clean up the temporary directory by recursively deleting all files

and directories in the temporary directory and, finally, the temporary directory

itself. In case deletion of a file or directory fails, an {@link IOException}

will be thrown that will cause the test or test class to fail.

It is better we use things already built for us, instead of trying to do our own workaround.

But, i am afraid if there is any similar annotation like https://github.com/tempdir as such for the createTempFile method.

It seems to me, all you need to do is create files in the temp directory you created from the annotation. As long as you specify the files go in such temp directory then things should work out. As mentioned above, the TempDir annotation will attempt to clean up any files in the directory recursively.

You can always verify this on your local. As the other issue mentioned, just change java.io.tmpdir to some other, empty directory to see if it remains empty after a test run. Just note as the docs specify, it is not guarantee things will get cleaned up. It depends on how the JVM exits, and if the OS allows the deletion (See normal file deletes in Java).

rnveach · 2024-02-12T17:52:43Z

#14414 (comment)

forbidding Files.createTempFile, is it needed?

If there is someway to bypass java.io.tmpdir and use the system's temp directory, then it needs to be banned IMO. This is what the other issue was about and it was approved and implemented. I haven't looked up Files.createTempFile but I assume one of the method signatures is to not provide a directory location and it will use something else instead.

Edit: In my implementation, createTempFile(String prefix, String suffix) calls createTempFile(prefix, suffix, null), null calls TempDirectory.location(), and the code around it uses "java.io.tmpdir". So it should be safe.

Based on this information, we may want to rethink banning Files.createTempFile as it seems safe.

relentless-pursuit · 2024-02-12T18:00:51Z

#14414 (comment)

forbidding Files.createTempFile, is it needed?

If there is someway to bypass java.io.tmpdir and use the system's temp directory, then it needs to be banned IMO. This is what the other issue was about and it was approved and implemented. I haven't looked up Files.createTempFile but I assume one of the method signatures is to not provide a directory location and it will use something else instead.

Edit: In my implementation, createTempFile(String prefix, String suffix) calls createTempFile(prefix, suffix, null), null calls TempDirectory.location(), and the code around it uses "java.io.tmpdir". So it should be safe.

I have removed createTempFile. And, the test cases are wokring fine, except in some where i had to have a file. In my PR, i am just creating a path or, path to the file, and not the file itself, and eliminating the need of any tempFile. For most test cases, it worked fine. Meanwhile, i will analyse your review comments and understand better. Please let me know if the changes in the PR look good to you.

relentless-pursuit · 2024-02-14T21:15:50Z

You can always verify this on your local. As the other issue mentioned, just change java.io.tmpdir to some other, empty directory to see if it remains empty after a test run. Just note as the docs specify, it is not guarantee things will get cleaned up. It depends on how the JVM exits, and if the OS allows the deletion (See normal file deletes in Java).

I did. So far, it seems to be cleaning up.

relentless-pursuit · 2024-02-14T21:19:07Z

Won't a simple closing call to deleteOnExit() suffice ??

Well, it can be a solution. But, then it becomes a manual cleanup. And, it comes with it's own trade-off. As highlighted already, @TempDir, though does automate the process, but doesn't guarantee. So, maybe a good option rather doing it manually.

nrmancuso · 2024-03-09T23:42:50Z

We missed removal of exclusions at https://github.com/Lmh-java/checkstyle/blob/16259a82bc779d4adbf329721cfa0eef33d79cd4/pom.xml#L1868

Edit: nevermind, contributor needed to rebase

relentless-pursuit mentioned this issue Feb 11, 2024

Issue #14220: Forbid Files.createTempDirectory in Checkstyle and replace with @TempDir #14414

Merged

romani added approved miscellaneous labels Feb 11, 2024

relentless-pursuit pushed a commit to relentless-pursuit/checkstyle that referenced this issue Feb 12, 2024

Issue checkstyle#14465: Refactor to exclude createTempFile

02a8ee3

relentless-pursuit mentioned this issue Feb 12, 2024

Issue #14465: Refactor to exclude createTempFile methods #14467

Merged

relentless-pursuit pushed a commit to relentless-pursuit/checkstyle that referenced this issue Feb 15, 2024

Issue checkstyle#14465: Refactor to exclude createTempFile

3ebd155

relentless-pursuit pushed a commit to relentless-pursuit/checkstyle that referenced this issue Feb 22, 2024

Issue checkstyle#14465: Refactor to exclude createTempFile

1bf6b5f

relentless-pursuit pushed a commit to relentless-pursuit/checkstyle that referenced this issue Feb 22, 2024

Issue checkstyle#14465: Refactor to exclude createTempFile

f9d2fbf

relentless-pursuit pushed a commit to relentless-pursuit/checkstyle that referenced this issue Mar 1, 2024

Issue checkstyle#14465: Refactor to exclude createTempFile

e2565ab

relentless-pursuit pushed a commit to relentless-pursuit/checkstyle that referenced this issue Mar 1, 2024

Issue checkstyle#14465: Refactor to exclude createTempFile

425d8cd

relentless-pursuit pushed a commit to relentless-pursuit/checkstyle that referenced this issue Mar 6, 2024

Issue checkstyle#14465: Refactor to exclude createTempFile

31491c9

relentless-pursuit pushed a commit to relentless-pursuit/checkstyle that referenced this issue Mar 6, 2024

Issue checkstyle#14465: Refactor to exclude createTempFile

005ccdf

relentless-pursuit pushed a commit to relentless-pursuit/checkstyle that referenced this issue Mar 6, 2024

Issue checkstyle#14465: Refactor to exclude createTempFile

a272d34

relentless-pursuit pushed a commit to relentless-pursuit/checkstyle that referenced this issue Mar 7, 2024

Issue checkstyle#14465: Refactor to exclude createTempFile

4093df5

relentless-pursuit pushed a commit to relentless-pursuit/checkstyle that referenced this issue Mar 8, 2024

Issue checkstyle#14465: Refactor to exclude createTempFile

131b1e4

rnveach pushed a commit that referenced this issue Mar 9, 2024

Issue #14465: Refactor to exclude createTempFile

19f4b5f

rnveach closed this as completed in #14467 Mar 9, 2024

github-actions bot added this to the 10.14.1 milestone Mar 9, 2024

nrmancuso reopened this Mar 9, 2024

nrmancuso closed this as completed Mar 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Excludes for createTempFile forbidden method #14465

Excludes for createTempFile forbidden method #14465

relentless-pursuit commented Feb 11, 2024

relentless-pursuit commented Feb 12, 2024

MANISH-K-07 commented Feb 12, 2024

relentless-pursuit commented Feb 12, 2024

MANISH-K-07 commented Feb 12, 2024

rnveach commented Feb 12, 2024 •

edited

Clean Up

rnveach commented Feb 12, 2024 •

edited

relentless-pursuit commented Feb 12, 2024 •

edited

relentless-pursuit commented Feb 14, 2024

relentless-pursuit commented Feb 14, 2024

nrmancuso commented Mar 9, 2024 •

edited

Excludes for createTempFile forbidden method #14465

Excludes for createTempFile forbidden method #14465

Comments

relentless-pursuit commented Feb 11, 2024

relentless-pursuit commented Feb 12, 2024

MANISH-K-07 commented Feb 12, 2024

relentless-pursuit commented Feb 12, 2024

MANISH-K-07 commented Feb 12, 2024

rnveach commented Feb 12, 2024 • edited

Clean Up

rnveach commented Feb 12, 2024 • edited

relentless-pursuit commented Feb 12, 2024 • edited

relentless-pursuit commented Feb 14, 2024

relentless-pursuit commented Feb 14, 2024

nrmancuso commented Mar 9, 2024 • edited

rnveach commented Feb 12, 2024 •

edited

rnveach commented Feb 12, 2024 •

edited

relentless-pursuit commented Feb 12, 2024 •

edited

nrmancuso commented Mar 9, 2024 •

edited