This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
If I type this in: int val = abs(55); I get an error and an editor hint that offers to create a method called abs(). I would like to also get an editor hint that offers to do a static import of java.lang.Math.abs (and perhaps another hint for a static import of java.lang.Math.*).
This would be great... although in my case I would like to be given the hint to convert a "normal" use of a static method into a statically imported one. That should be a lot more trivial as there is no need to keep all valid method names in memory or anything.
I'm keen to have this as a hint. I have pretty much given up on static imports because they are such a pain in NetBeans. If nobody is currently working on this, I would greatly appreciate a pointer in the right direction so that I could perhaps look at implementing the functionality of turning an explicit use of a static method into a static import. e.g. if I have the code int val = Math.abs(55); I'd like the hinter to offer to turn Math.abs into a static import.
More power to you if you work on this, fommil! The general lack of support for static imports in NetBeans is really grating.
Has this been assigned to anyone?
is anybody considering this for a milestone? It would be really really nice to have! NetBeans support for static imports is really quite poor due to the lack of this feature.
In anybody looking at this? In Eclipse, one can set a bunch of your "favourite static imports", which offers the functionality that the original poster is requesting. That combined with a simple hint to convert a long-hand static method use, to a static one, would be all the static import support I'd need.
After getting acquainted with the java.editor and java.hints module codebases, I believe this RFE is four-fold and should fall under the title "better support for static imports":- 1) The Java Completer should offer static methods for completion as if they were local. I believe this already happens for already imported static methods, but it should also do this for static methods from user-specified classes, potentially with NetBeans shipping a good stock supply as default. This requires a UI similar to the new "Java Code Completer Excluder" that I added as part of bug 125060 (latest UI not yet committed). 2) The Java Hints system should consider the possibility that methods which are undeclared in the local context may be available as static imports from elsewhere (maybe just from the user-defined path), and therefore a hint should be offered to take the appropriate action. This resolves the original poster's example. 3) The Java Editor Auto Import function dovetails with point 2. 4) A Java Hint for converting OldStyle.staticMethodInvocation() -> staticMethodInvocation() with static import, potentially a function that performs this for an entire file. I'm considering taking this RFE on as my second significant patch to the NetBeans codebase. jlahoda are you ok remaining the assignee with me attaching patches (against main-golden) here for feedback? This will span java.editor and java.hints. Initial advice is welcome!
Great, fommil! Some more comments: - The hint (point 4) for converting usages of static methods should only be visible when the cursor is on the respective expression, much like for Assign Return Value To New Variable. Maybe that goes without saying. The point is that static imports are not always desirable, e.g. for EnumSet.of(…), therefore source files shouldn't be displayed sprinkled with corresponding warnings/hints. - As a fifth point, there is Fix Import which should offer static import suggestions for unresolved local method calls. (Maybe that's what you meant with point 3?) - There's also the issue of where static imports are auto-inserted in the list of import directives. Currently auto- insertion of ordinary imports ignores static imports when searching for where to insert the import directive. It would probably be useful to provide different options here (or really good heuristics). Some people prefer static imports to be mixed with ordinary imports, although the comb-like structure due to the "static" keyword makes the imports list less readable. Other people prefer the static imports before the ordinary imports, others prefer after. This is also often project specific. See for example Eclipse's Organize Imports configuration dialog which I believe provides support for all of this.
@matthies thank for your comments Re: visibility, "mouseover" will be the default but users will be able to change this in the hints config menu. A blacklist could potentially be used to avoid hints altogether. Re: fix imports, I never knew NB did this :-) Re: sort support. Hmm, have you seen my bug 125566 report? I believe "sorting members" is part of Eclipse's functionality here... and something I miss since moving over (except sorting of enum ordinals!).
@fommil: For re-ordering imports, extending the functionality of Fix Imports, there's already issue 122109. Although there is some overlap, I believe this should remain a separate issue from better support for static imports. But choosing an ordering is already necessary for the latter. As for your issue 125566, I prefer specific code fixes/refactorings to be kept separate from whole-file source formatting/prettyfication. Of course auto-generation of code (such as import directives) should result in nice source code (hence inserting import directives according to some ordering), but it shouldn't modify other parts of the source. (For non-final variables I'd probably like warnings and a dedicated refactoring command.) See also issue 156460.
thanks for the pointer to issue 122109, I'm now CCd. Not so keen on 156460 although I agree that non-final fields (that can be made final) could do with a warning.
First of all, let me say that this may be a major undertaking. Looking at your proposal, I do not like that the user would be required to set classes for which the static import related features would work. From user's point of view, this does not seem right. An issue here is that although the current index contains symbols (methods and fields) for user classes, it does not contain them for classes indexed from jar files, due to performance concerns. It might be possible to index only static members for classes from jars, but that needs to be investigated (from performance and API point of view). To the individual mentioned parts: 1. (code completion) I think that the unimported methods should be shown only for "all" code completion, similarly to unimported types. 2.+3. (imports) actually, imports handling is mostly done in ComputeImports (java.editor), the hint and fix-all-imports features are only views over the data provided by ComputeImports. Although the features may need some tweaks to present the static imports in a user-friendly way, the hard work will likely be in ComuteImports. 4. (convert to static import hint) I agree with matthies - this should be a caret sensitive hint, at most. Possibly should be disabled by default, so that it would not annoy users that do not want to use static imports. Also note that there are experiments on batch application of hints (see "batch" package in contrib/javahints). Another area that may need changes (and is related to #125566) is import analysis. When a NB plugin generates Java code, it is supposed to use Java infratructure, especially TreeMaker and WorkingCopy.rewrite(). When TreeMaker.QualIdent(Element) is used (or TreeMaker.Type(TypeMirror)), the infrastructure handles the imports for the client. This is performed in ImportAnalysis2 (java.source) and it will likely be necessary to adjust this class for static imports, otherwise code generated through the API will not use static imports (this expects that the client creates QualIdent for the method/field itself, not for its enclosing type, but if it does not, it is a bug in the client). Also, GeneratorUtilities.import{FQNs|Comments} may need to be fixed.
@jlahoda, thanks for your comments I'll take them on board... will certainly need help with this RFE and might put it off for a bit, or at least break it into manageable chunks. Re: user specified classes, this is what Eclipse does and it works surprisingly well. Every coder has a few static utility methods that they tend to use a *lot*. The idea is that the user would add those. In my case, I'd probably add a few methods from Preconditions (part of google-collections). I don't think that functionality is the core of this RFE... the core functionality comes from the ability to convert a Class.staticMethod -> staticMethod with static import. Hints "off" by default is a serious option, but when implemented I'd like to have a poll to see if the majority of users would prefer a mouseover hint by default. I am tempted to install the latest version of Eclipse to see what they are doing with regards to static importing. I recall their support was far superior to Netbeans.
I'm starting with a hint to offer a static import. Expect a patch over the next week or so at the latest.
I've hit a problem and could do with some help. Working out whether or not a METHOD_INVOCATION is actually declared in a qualified way, e.g. Math.abs() vs abs() + static import is actually quite tricky. The only way I seem to be able to do this is to use getSourcePositions and see if there is a "." in the text! That's a horrible solution IMHO, are there any ways to do this using the Compiler Tree API? Other than that... I've got code working that does the detection and currently returns a do-nothing fix.
I agree that user-specified "favorites" for static imports are useful and work well. My co-workers that use Eclipse love them. There needs to be project-specific settings though, or at least the suggested imports must be restricted to the ones available from the libraries and sources of the current project. There's an increasing number of APIs that are designed assuming static imports, in particular ones providing internal DSLs (like this: http://www.martinfowler.com/dslwip/NestedFunction.html), and that are cumbersome to use when static imports have to be added by hand every time. In our team, NetBeans has already become an impediment here, and for some developers it's becoming a serious incentive to switch to Eclipse. So, fommil, your work is really appreciated!
I'm attaching a patch in a moment which implements the hint to convert Klass.staticMethod -> staticMethod. i.e. point 4 in my previous post. Try it out on the Test code at the end of this update. This hint is intended only to convert qualified accesses of static methods to static imports, it is not intended to work if the containing class is not already imported/resolved. Either this hint could offer to do the import of the containing class, or that support should be added to the errors hints. My preference would be with the latter... and that will be the next thing I'll look at. There will obviously be a lot of code duplication, so I'll aim to move a lot of this functionality into somewhere accessible by both. I have several comments and questions:- - how can I discover if the current source level supports static imports? - how can I discover if the MethodInvocationTree or ExpressionTree is actually resolved (i.e. their containing class is imported)... I'd like to exit if not, because that should be a job for the error hints. - is there a better way to discover if a static method is being accessed in a qualified way? The current technique of calling .contains(".") on the ExpressionTree.toString seems really hacky. - is it OK to assume the form of ImportTree.getQualifiedIdentifier().toString()? The form is not documented, but I found other NB code doing similarly. - do java.hints run() methods really have to start with a treePath.getLeaf().getKind() sanity check? Shouldn't we be able to trust the framework to only send us TreePaths that we ask for in getTreeKinds()? - related to previous; is a similar sanity check also required after calling the TreePathHandle.resolve? - JavaFixAllImports.addImports doesn't support static imports. Also, it is directly copied from SourceUtils.addImports! I can't see how to upgrade this code to support static imports... meaning I'll be forced to introduce further code duplication. Anyone have any alternatives? ======== public class Test { public Test(String blah) { int abs = Math.abs(1); Test.doStuff(); Calendar.getInstance(); } public static void doStuff(){ } }
Created attachment 80550 [details] first attempt
I'd like to NetFIX [1] this bug. Is it possible? [1] http://wiki.netbeans.org/NetFIX
Some notes to the patch: > // XXX is this sanity check needed? Yes it is. The resolver is not 100%. > // TODO ignore case where source code is less than Java 1.5 We're preparing a method added to CompilationInfo. > // XXX can't use JavaFixAllImports.addImports because no support for static imports > // XXX smart insertion We have to find a better way to do this. Ideally, make SourceUtils.addImports public and make it work with this. I'll give this a try soon.
I was hoping you'd say that about SourceUtils.addImports ;-)
The following is a good test bed for the "hint" part of this feature ==================== import java.util.logging.Logger; import javax.crypto.KeyAgreement; import static java.util.Calendar.*; public class Test { public Test(String blah) throws Exception { // should only change identifier, no import DONE int abs = Math.abs(1); // should only change identifier, no import DONE Test.getLogger(); // should not offer to do static import (name clash with local) DONE Logger.getLogger(""); // should not offer to do static import (unresolved) XXX EnumSet.of(null); // should not offer to do static import (name clash with existing static import) XXX KeyAgreement.getInstance(""); } public static void getLogger(){ } }
Created attachment 80898 [details] improvements in "pure hint", no handling errors
I considered many more scenarios and the following is the latest test bed. Again, this hint is not intended to work as an "error hint", that will be dealt with separately. In the meantime... anyone know how to exit early if the identifier is not resolved? ========================= import java.util.Calendar; import java.util.Collections; import java.util.logging.Logger; import javax.crypto.KeyAgreement; import static java.util.Calendar.*; public class Test extends Foo { public Test(String blah) throws Exception { // should only change identifier, no import DONE int abs = Math.abs(1); // should only change identifier, no import DONE Test.getLogger(); // should only change identifier, no import DONE Foo.foo(); // should only change identifier, no import DONE Calendar.getAvailableLocales(); // should change identifier and import DONE Collections.emptyList(); // should not offer a static import, name clash with local DONE Logger.getLogger(""); // should not offer a static import, name clash with imported DONE KeyAgreement.getInstance(""); // should not offer a static import, name clash with inherited DONE Bar.foo(); // should not offer a static import, unresolved XXX EnumSet.of(null); } public static void getLogger() { } } class Foo { static protected void foo() { } } class Bar { static protected void foo() { } }
Created attachment 80899 [details] hack to ignore error cases
Created attachment 80922 [details] consider inner classes, addStaticImport in SourceUtils
My last test bed post was a bit of a "brain fart", please ignore... this class is a much better test bed as it tests behaviour for inner/static classes. For me, patch 4 behaves as expected where you see DONE, but I need to do a full recompile to confirm as I'm getting odd compile errors in java.source (not my fault!). ======================== import java.util.Calendar; import java.util.Collections; import java.util.logging.Logger; import javax.crypto.KeyAgreement; import static java.util.Calendar.*; public class Test extends Foo { void doStuff() throws Exception { // should change identifier and import DONE Math.abs(1); Collections.emptySet(); // should only change identifier, no import DONE Test.getLogger(); // should only change identifier, no import DONE Foo.foo(); // should only change identifier, no import DONE Calendar.getAvailableLocales(); // should not offer a static import, name clash with local DONE Logger.getLogger(""); // should not offer a static import, name clash with inherited DONE Bar.foo(); // should not offer a static import, name clash with imported DONE KeyAgreement.getInstance(""); // should not offer a static import, unresolved DONE EnumSet.of(null); } public static void getLogger() { } class FooBar { void doOtherStuff() { // should only change identifier, no import DONE Foo.foo(); // should not offer a static import, name clash with inherited DONE Bar.foo(); } } static class BarFoo { void doMoreStuff() { // should only change identifier, no import DONE Foo.foo(); // should not offer a static import, name clash with inherited DONE Bar.foo(); } } } class Foo { static protected void foo() { } } class Bar { static protected void foo() { } }
Grr... typo in last patch, line 115 of StaticImport.java should have reversed logic, i.e. if (!isSubTypeOrInnerOfSubType(info, klass, enclosingEl)) { Apologies. Since it was only a beta patch, I'll not upload a correction just yet. Also, aesthetic, but variable in 'isStatic' in SourceUtils is better described as being called 'ignore'.
Created attachment 81017 [details] integrated with the error hinter and considered more use cases
Latest patch now integrates with the error hinter, yay! I still need a way to detect the source version >= 1.5. I have an outstanding query on the mailing list which should clear up the METHOD_SELECT issue in ImportClass, see http://www.nabble.com/When-is-a- MEMBER_SELECT-not-a-MemberSelectTree--tt23266103.html Next step is to convert the follow test file into a series of unit tests, then I think I'll create a separate issue for improvements to the completer:- ================== import java.util.Calendar; import java.util.logging.Logger; import javax.crypto.KeyAgreement; import static java.util.Calendar.*; /** * @author Sam Halliday * @see <a href="http://www.netbeans.org/issues/show_bug.cgi?id=89258">issue 89258</a> */ public class Test extends Foo { void doStuff() throws Exception { // should change identifier and import DONE Math.abs(1); // should only change identifier, no import DONE Test.getLogger(); // should only change identifier, no import DONE Foo.foo(); // should only change identifier, no import DONE Calendar.getAvailableLocales(); Calendar.getInstance(); // should not offer a static import, name clash with local DONE Logger.getLogger(""); // should not offer a static import, name clash with inherited DONE Bar.foo(); // should not offer a static import, name clash with imported DONE KeyAgreement.getInstance(""); // hint should not offer a static import, unresolved DONE // error hint should offer a static import DONE Collections.emptySet(); // hint should not offer a static import, unresolved DONE // error hint should not offer a static import, local name clash DONE EnumSet.of(); // hint should not offer a static import, unresolved DONE // error hint should not offer a static import, imported name clash DONE MessageDigest.getInstance("SHA"); // hint should not offer a static import, unresolved DONE // error hint should not offer a static import, target doesn't exist XXX Collections.doesNotExist(); } public static void getLogger() { } public static void of() { } class FooBar { void doOtherStuff() { // should only change identifier, no import DONE Foo.foo(); // should not offer a static import, name clash with inherited DONE Bar.foo(); } } static class BarFoo { void doMoreStuff() { // should only change identifier, no import DONE Foo.foo(); // should not offer a static import, name clash with inherited DONE Bar.foo(); } } } class Foo { static protected void foo() { } } class Bar { static protected void foo() { } }
@jlahoda I'm going to keep working on this despite the feature freeze. Just to let you know I'm not expecting 6.7 inclusion ;-)
Created attachment 81414 [details] started creating unit tests
Created attachment 81437 [details] added test cases, ready for review
I'm very happy with the latest patch upload. It now has extensive test coverage. I realise 6.7 is now closed for new RFEs, so I won't push for inclusion... but it is sitting here ready for integration ;-)
A few quick comments: 1. // XXX is this sanity check needed? (for Tree.Kind in run method of hint) - this is actually not needed at runtime, but the computeErrors method in TreeRuleTestBase can be generally called for any TreePath (esp. for the four generic tests defined in TRTB). If computeErrors verifies the Tree.Kind, the check is not need in the hint. 2. I am not much in favor of making SourceUtils.addImports method(s) public. The main reason is that I do not see any client for these methods aside the java.source, java.editor and java.hints. And, if the method will be there, other clients may be tempted to use these methods instead of proper TreeMaker.Type/QualIdent (making the code worse to maintain, throughout the IDE). IIRC, this was the main concern before 6.0, maybe not so important now (clients learned the advantages for TM.Type/QualIdent), but still should be considered. 3. There is usually no need to keep compatibility inside the implementation packages (JavaFixAllImports.addImports), simply fixing all uses should be enough. I will try to go through the patch more deeply soon. What about the other features, btw?
Created attachment 81548 [details] same as last, but bump up revision number in file name
@jlahoda I think you were looking at an older version of the patch... sorry, I got a little carried away with updates to this page, a lot of questions appear unanswered but I got responses from the mailing list if not here. I also noticed that I didn't change the name of the latest patch to indicate that it was the latest, please look at java.hints-static_import-7.diff only. I created a separate issue 164487 for the Java Completer part of this RFE as it is much more involved. That would provide a UI to allow users to manually specify classes that can be statically imported. Implementing this will involve pulling some helper methods from java.hints into java.{source, editor}. I created a separate issue 89258 for full classpath search for static methods. Admittedly, this is what the OP was asking for, I should really have created a separate RFE for the hint part, but it wasn't clear what the dependency chain looked like until work began. Sorry @gsporar if you've felt spammed, but we are closer to a solution than when you submitted the RFE ;-) Issue 122109 tracks the reordering of static imports. Re: SourceUtils.addImports I have no strong feelings one way or the other, but somebody recommended making it public and the code duplication in java.editor was concerning. Is there a way to turn this into a maintainable API method?
Honzo, can you comment on this? Thanks!
The non-error hint part of this was added to the "contrib" branch in the following commit http://hg.netbeans.org/main/contrib/rev/addfef1b8991 Users should be able to use the build by adding the following UNSTABLE Update Center http://deadlock.netbeans.org/hudson/job/nbms-and-javadoc/lastSuccessfulBuild/artifact/nbbuild/nbms/updates.xml.gz
I'd like to see the support in this patch (and in contrib) extended to handle members, not just methods.
Any update on this Honzo? Thanks!
*** Issue 174782 has been marked as a duplicate of this issue. ***
*** Issue 66669 has been marked as a duplicate of this issue. ***
(arriving here from #174782). Nice duplicate. We are now almost 100.000 bug reports later, and this issue is not yet resolved. Seems to be almost useless to submit P3 bug reports. Make it P2 to give it more visibility.
While we wait for this patch to be accepted in the main NetBeans source, there is an Experimental Hint giving this support. Please feel free to add the following plugin to your Netbeans Tools -> Plugins -> Settings list, it will give you access to the "Java Experimental Hints" plugins, providing some time-saving features http://deadlock.netbeans.org/hudson/job/release67-contrib/lastStableBuild/artifact/updates.xml None of the plugins will be enabled by default. As well as enabling "Static Imports", I also recommend "Serializable".
Sam, if you will agree, I will put the hint from contrib into std. NetBeans distribution. Regarding the error fix (i.e. on unresolvable method/field), I still think this should be done primarily in ComputeImports, so that all import-related features could benefit from that. Also, I still think that the hint should be able to work without any user customization (such customization may be optional, but should not be required, IMO). But maybe bug #164490/#71249 was supposed to cover this?
@jlahoda thanks! Re: error handling, please feel free to look at some of the older patches against main to see how I was intending this to be implemented - should save you some time. I will not be offended if you do what you feel is best. Re: other issues. That was exactly the point of creating them :-)
I have moved the StaticImport hint to std. distro: http://hg.netbeans.org/main-silver/rev/414d6dd8cb2a I have removed it from contrib: http://hg.netbeans.org/main/contrib/rev/2650ebb0414a As there are other bugs (bug #164487, bug #71249) for the other features needed to support static import, I think we can close this one.
Thanks a lot Honzo and more importantly thanks Sam for your patch!
Integrated into 'main-golden', will be available in build *200912100200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main/rev/414d6dd8cb2a User: Jan Lahoda <jlahoda@netbeans.org> Log: #89258: Sam Halliday's (fommil@netbeans.org) hint for conversion of qualified static method reference to import static and unqualified reference moved from contrib to trunk.
Hi, I really like this feature, except that it doesn't import static fields. In fact I'd prefer for the code I'm working on now to have a hint for static fields but not for the methods.
Is this still valid in 7.1?
Set target-milestone of open issue to TBD (was < 7.3, f.e. 6.8), so the issue doesn't get lost.
Set target-milestone of open issue to TBD (was < 7.3, f.e. 6.8), so the issue doesn't get lost. This time the target milestone is really set.
Created attachment 147920 [details] Patch: Introducing support for static imports for fields/enum-fields Please review the patch and commit. The patch introduces support for transforming static (enum) field references to use static imports. The patch also includes unit tests with obvious examples.
Patch applied. Thanks for your contribution. http://hg.netbeans.org/jet-main/rev/080db3c244e0
Thank you Dusan and Benno!
Can this be modified to exclude references to the static "class" variable? I just tried to run this, and i'm using the class objects fairly throughly, and this check suggested import static java.lang.String.class AND import static java.lang.Double.class.
(In reply to ruckc from comment #59) > Can this be modified to exclude references to the static "class" variable? > I just tried to run this, and i'm using the class objects fairly throughly, > and this check suggested import static java.lang.String.class AND import > static java.lang.Double.class. I created a follow-up issue for this RFE https://netbeans.org/bugzilla/show_bug.cgi?id=247045