DMI_HARDCODED_ABSOLUTE_FILENAME looks if absolute path is passed to the java.io.File.File(String) constructor. That's nice, but there are many other constructors in JDK which take the file path as an argument like FileWriter constructor and so on. Also there's a Paths.get in Java 7 which also needs to be checked. I patched the DumbMethodInvocations detector to detect most of such cases. In our project I found 6 new instances of this bug (in loosely written unit tests). A couple of new tests also added.
Bug fix is here:
https://code.google.com/r/amaembo-findbugs/source/list?name=hardcoded-path-improvements
All my patches are here:
https://code.google.com/r/amaembo-findbugs/source/list?name=master
I've merged in your changes, however I would really appreciate you adding test cases to validate these changes. Your comment above says you added test cases, but I didn't see any.
What I'd like to see is the addition of a file findbugsTestCases/src/java/sfBugsNew/Bug1313.java, which includes examples of the additional calls detected by the changes, with an annotation of @ExpectWarning("DMI_HARDCODED_ABSOLUTE_FILENAME")
By merging in your changes, I picked up all of your changes, thank you. But I'd appreciate test files as mentioned for all changes.
Thank you for merging all my changes! I have some more improvement ideas, so it became a little bit hard to maintain the separate branch for each bug report. I guess, the following reports can be closed now:
[bugs:#1312] [bugs:#1308] [feature-requests:#312] [feature-requests:#311]
Tests I created are located in findbugsTestCases/src/java directly. Usually I didn't create new test file if there's one already. I just add more test cases. I always make proper annotation and even add it for older tests. The following test files were changed/created by me:
findbugsTestCases/src/java/DumbMethodInvocations.java (for this particular bug)
findbugsTestCases/src/java/WMITest.java (for [bugs:#1312] [bugs:#1308])
findbugsTestCases/src/java/RepeatedConditionals.java (for [feature-requests:#312])
findbugsTestCases/src/java/MutableStatic.java (for [feature-requests:#311])
findbugsTestCases/src/java/namedPackage/MutableStaticInPackage.java (for [feature-requests:#311])
Sorry I didn't know that test cases should be named as bug number. I thought it's better to modify existing tests if any. Usually I commit the code before submitting a bug to this tracker, so I don't even have a bug number during commit. But this not a big problem: I can first open a bug without a patch, get the number, commit the patch with a test case, then comment the bug with a patch link.
So the question is: should I move my tests now to separate files inside sfBugsNew? I can do this if it really matters.
Related
Bugs:
#1308Bugs:
#1312Feature Requests:
#311Feature Requests:
#312