Menu

#1313 [patch] False negative for DMI_HARDCODED_ABSOLUTE_FILENAME

3.0.1
closed-fixed
patch (11)
5
2014-10-19
2014-10-18
No

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

Discussion

  • William Pugh

    William Pugh - 2014-10-19

    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.

     
  • William Pugh

    William Pugh - 2014-10-19
    • status: open --> closed-fixed
    • assigned_to: William Pugh
    • Group: 3.x --> 3.0.1
     
  • Tagir Valeev

    Tagir Valeev - 2014-10-19

    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: #1308
    Bugs: #1312
    Feature Requests: #311
    Feature Requests: #312


Log in to post a comment.