Bugzilla – Full Text Bug Listing
| Summary: | [JUnit 5] Warning The method xxx from the type X is never used locally is shown when using MethodSource | ||
|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Pierre-Yves Bigourdan <pyvesdev> |
| Component: | Core | Assignee: | Pierre-Yves Bigourdan <pyvesdev> |
| Status: | VERIFIED FIXED | QA Contact: | Stephan Herrmann <stephan.herrmann> |
| Severity: | normal | ||
| Priority: | P3 | CC: | daniel_megert, jarthana, noopur_gupta, stephan.herrmann |
| Version: | 4.10 | ||
| Target Milestone: | 4.11 M3 | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: |
https://git.eclipse.org/r/134155 https://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=abf93b41f080158cf2a0f2630bf4a409a995f65b https://bugs.eclipse.org/bugs/show_bug.cgi?id=546084 https://bugs.eclipse.org/bugs/show_bug.cgi?id=558873 |
||
| Whiteboard: | |||
Third-party annotations and reflection easily kill any static analysis ;-/ But JUnit is indeed close to home, so I'm not against this proposal. As we speak about JUnit 5 annotations, are there more that define a reflective access to code? May we assume that the mentioned methods cannot participate in any overloading? As far as I know, there aren't any other JUnit 5 annotations that define a reflective access to code. I'm unclear what you mean by your second question. I've never digged into the source code for any of these static analysers. However, I'm happy to look into this in the coming weeks if no one else picks it up. (In reply to Pierre-Yves B. from comment #2) > I'm unclear what you mean by your second question. If you have @MethodSource("getIntegers") can we assume that there can legally be only one method of that name in the current class? Or do we have to select the no-args variant among same-named candidates? Or does the annotation apply to all methods (overloads) of that name? > I've never digged into the source code for any of these static analysers. > However, I'm happy to look into this in the coming weeks if no one else > picks it up. If you do so, the following may serve as starting points: ProblemReporter.unusedPrivateMethod(AbstractMethodDeclaration) ExtraCompilerModifiers.AccLocallyUsed For an example how we handle well-known annotations, you can confer: - TypeConstants.COM_GOOGLE_INJECT_INJECT - TypeIds.T_ComGoogleInjectInject and search your way from there :) For setting up a JDT workspace I recommend using Oomph. (In reply to Stephan Herrmann from comment #3) > If you have @MethodSource("getIntegers") can we assume that there can > legally be only one method of that name in the current class? Or do we have > to select the no-args variant among same-named candidates? Or does the > annotation apply to all methods (overloads) of that name? Method source can only be used with methods without any parameters (otherwise throws exception "factory method must not declare formal parameters"), so we should keep on warning as we did before for overloaded methods. Two interesting variations to my example above: - when no name parameter is specified for the @MethodSource annotation, the same name as the test method is used as the method source (@MethodSource == @MethodSource("testPositiveIntegers")). This shouldn't require a lot of extra work to cover though. - fully qualified names are supported, for instance @MethodSource("some_package.MethodSourceTest#getIntegers"). If someone decides to put method sources and test methods themselves in different classes, JUnit 5 will still cope happily even if the method sources are private in that different class. This means that ideally we would have to solve a problem such as "for any private static no-argument method in my entire project, is there any parameterized JUnit 5 test in my entire test suite that references it via a fully qualified MethodSource annotation?". I'm inclined to ignore this case and only deal with method sources that are in the same class, as this will probably be by far the most common case. > For setting up a JDT workspace I recommend using Oomph. I've already contributed quite a few patches to JDT-UI, my setup seems to work fine with JDT-Core as well. :) New Gerrit change created: https://git.eclipse.org/r/134155 (In reply to Pierre-Yves B. from comment #4) > I've already contributed quite a few patches to JDT-UI, my setup seems to > work fine with JDT-Core as well. :) Please pardon my ignorance :) Change is ready for merging once we have Jenkin's +1 after rebase. M1 is done. Shall we try to get this merged for next milestone? :) Gerrit change https://git.eclipse.org/r/134155 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=abf93b41f080158cf2a0f2630bf4a409a995f65b (In reply to Eclipse Genie from comment #9) > Gerrit change https://git.eclipse.org/r/134155 was merged to [master]. > Commit: > http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/ > ?id=abf93b41f080158cf2a0f2630bf4a409a995f65b Released for 4.11 M3 Thanks, Pierre-Yves! Awesome! Thanks for your help and the review, Stephan! Verified for 4.11 M3 using build I20190219-1800 See bug 558873 comment #12 which seems to be caused after this change in tests. (In reply to Noopur Gupta from comment #13) > See bug 558873 comment #12 which seems to be caused after this change in > tests. See bug 558873 comment #19 :) |
Consider the following parameterized JUnit 5 test: class MethodSourceTest { @ParameterizedTest @MethodSource("getIntegers") void testPositiveIntegers(Integer integer) { assertTrue(integer >= 0); } private static List<Integer> getIntegers() { return List.of(0, 5, 1); } } Eclipse will display the warning "The method getIntegers() from the type MethodSourceTest is never used locally" and will suggest to delete it as part of its quick fix options, even though getIntegers is actually used as a method source for the test. Given the increasing popularity of the latest version of the JUnit framework, would this be something we would like to deal with?