Community
Participate
Working Groups
Excerpt from https://frostillic.us/blog/posts/A3EDC8BB9DB6AFF38525834D005A2E45 "It's also pretty good at recognizing legal-but-inadvisable idioms and offering in-place conversions. For example, if you write this: String bundles = osgiBundleList.stream().collect(Collectors.joining(",")); ...it will have a quick action to convert it to this instead: String bundles = String.join(",", osgiBundleList); This sort of thing isn't unique to IntelliJ (Eclipse has some of this and there's no shortage of code-linting tools), but it does a particularly good job integrating it in a useful and non-obtrusive way." I've written a basic detector for something like 'X.stream().collect(Collectors.joining(Y))' at MessageSend#analyseCode() that reports the issue and a quick fix in LocalCorrectionsSubProcessor (jdt.ui) to produce 'String.join(Y, X)'. What I'm noticing is that there's no singular place in jdt.core to place such detectors. They seem to be placed in the analyseCode method of various ASTNode inheriting types. Maybe that's because for the sake of efficiency they should be placed at the narrowest scope needed for evaluation, so I guess that makes sense. The other thing I've noticed is that there don't seem to be that many detectors+quick fixes contributions to jdt.core/jdt.ui for suggestions like the ones mentioned above. Would such things be wanted, or is SpotBugs [1] (formerly FindBugs) meant to satisfy this need ? The problem I see here is that SpotBugs can't ship as part of Platform or even an EPP due to legal issues [2] so people using plain Eclipse (and unaware of update sites / plugins) are left seeing a gap in functionality when compared to other IDEs that ship such detector+quick fixes. [1] https://spotbugs.github.io/ [2] https://bugs.eclipse.org/bugs/show_bug.cgi?id=512880#c3
Commenting on one technical aspect: (In reply to Roland Grunberg from comment #0) > What I'm noticing is that there's no singular place in jdt.core to place > such detectors. They seem to be placed in the analyseCode method of various > ASTNode inheriting types. Maybe that's because for the sake of efficiency > they should be placed at the narrowest scope needed for evaluation, so I > guess that makes sense. When you look at classes like MessageSend, these are internal classes of the compiler, which where never intended for plugging in arbitrary analyses. Contributions should use the public AST, which has all the structural information, but still the compiler structures have more information. Now, over the years we did add several new analyses into the compiler, where the requirements included: - minimal impact on performance - be sufficiently general as to serve all Java developers As concerns performance this indeed implied to analyse "as we go", i.e., avoid introducing an additional analysis phase. Originally, analyseCode() was specifically focused on flow analysis. I'm aware that this picture is somewhat blurred meanwhile. Also note, that well-known types are detected much earlier during compilation, so that analysis downstream can be implemented more efficiently.
@Roland: would you prefer such extra checks to happen in compiler or in IDE? If not in compiler, then a documentListener or ResourceListener moght work, retrieving the AST and placing markers. That would work for JDT-LS as well. For quickfixes, the story is a bit more annoying as markerResolutions are IIRC an IDE concept that depends more or less directly on UI (not good for JDT-LS). I think we miss in the markers API a way to define markerResolutions without UI (a label + a document change) so it would be reusable fully in JDT-LS et al.
(In reply to Mickael Istria from comment #2) > @Roland: would you prefer such extra checks to happen in compiler or in IDE? > If not in compiler, then a documentListener or ResourceListener moght work, > retrieving the AST and placing markers. That would work for JDT-LS as well. > For quickfixes, the story is a bit more annoying as markerResolutions are > IIRC an IDE concept that depends more or less directly on UI (not good for > JDT-LS). I think we miss in the markers API a way to define > markerResolutions without UI (a label + a document change) so it would be > reusable fully in JDT-LS et al. Yes, it's the entry point I was looking into as I couldn't really see any good examples of existing problems being reported through the the public AST (I presume org.eclipse.jdt.core/dom/org/eclipse/jdt/core/dom/ ). After you mentioned IDocumentListener / ResourceListener, I found CompilationUnitEditor which implements IJavaReconcilingListener#reconciled(CompilationUnit, boolean, ProgressMonitor) so at least it saves having to call the ASTParser on the sources every single time. From there I'm guessing configuring the returned IMarker from IResource#createMarker(..) is enough to get things working. The QuickFixProcessor (iirc) is the layer of abstraction above the whole mark resolution generation API, and yes, it's in JDT UI though a good place to start might be to figure out how to split out the non-UI parts of ICompletionProposal. If such changes are wanted I think a good starting point would be to find the most common ones that are the least likely to produce false positives from SpotBugs and to bring them into JDT.
As a POC, I have https://github.com/rgrunber/eclipse.jdt.ui/commit/d4794d55675bbdfc894076ade9f92e4651cfc192 . Some things have been temporarily hard-coded so we avoid touching jdt.core for now . Does it seem reasonable though ? I'd probably want to get a few more problems with corresponding fixes done (from SpotBugs) to confirm this approach would work.
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
