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.
It would be very useful to be able to exclude classes from appearing in the code completer. Specifically it would be good to be able to add implementation- specific packages (such as sun.* and sunw.*). If possible, I would also like to see method-level exclusion lists. For example, I tend to avoid using Object.{wait*, notify*, clone, finalize} in preference of more appropriate alternatives. It would be good if the code-completer knew to avoid them.
I would like to volunteer to help out with this. Could somebody please help me out by pointing me to the most relevant piece of the NetBeans code for implementing this.
Has this anyone been assigned to anyone? I think it's a fairly major oversight of NetBeans not to have this support... it's in Eclipse and almost everybody I know uses it and would miss it dearly if they came over to NetBeans.
Is anybody watching this RFE?
this was filed under "editor" but I think it should actually be under the hints as it is to do with the classes that are included in the auto completer.
java/hints is about the help indicated be light bulb in the gutter in editor. java specific part of code completion is implemented in java/editor
Is anybody looking at this for the next release? Naively, I should think it relatively simple to add support for this... and the user experience would be tremendous! Firstly, autoimport should be an order of magnitude faster (and smaller footprint), along with a much improved hit-rate for imports. I'd even suggest adding some packages to be filtered by default, including all the sun{w|}.* classes. I'd be willing to help out implement this, although I have no NetBeans coding experience.
I'm looking at implementing this. Could do with assistance! http://www.nabble.com/Help-with-first-steps-into-NB-codebase...-implementing-RFE-125060-td22400452.html
Created attachment 78706 [details] initial implementation attempt
Created attachment 78707 [details] initial implementation attempt
I've uploaded a "work in progress" attempt at implementing this. I believe I've isolated all the relevant parts of the Java completer, Javadoc completer and Fix Imports classes. I also bundled a proposed fix for #160602 because I didn't know how to exclude it from the patch. This is against main-golden from about a week ago. A user interface would be required before this patch is mature. Additionally, it would be good if excluded packages/classes appeared as warnings (to replace the current warnings given exclusively to sun.* classes)... although that might be a follow-up RFE. I have several (seven) questions that I'd like to hear feedback on:- This patch creates a new class named ExcludeCompletion which is generated from Preferences with keys "javaCompletionWhitelist" and "javaCompletionBlacklist", comma-separated lists of fully qualified names. Default values have been provided as a proof of concept. This class is a singleton in order to avoid costly creation every time it is needed. Q1: is the singleton approach the right way to go? Alternatives? Singleton has the disadvantage that it will never be GCd once created. Q2: should it be registered as a "WeakListener"? (This seems irrelevant if it's a singleton) Q3: how thread safe does this class have to be with regards to changing preferences? A read/write lock would fix this, but no need to complicate the class if thread safety is not necessary. This patch will exclude packages/classes from being completed (also in Javadocs) and also from being imported. If an excluded package is typed out explicitly, classes will still be completed... e.g. if "sun." packages are excluded, but the user types out "sun.awt." then classes from "sun.awt." will still be presented. This is a conscious choice as it is clear that the user would want a completion at that point. Q4: should completions be provided if the user has typed out the fqn of a package, even if that package is in the blacklist? I haven't tested this with the completion of annotations at this time, but that's on the TODO list. Q5: besides annotation completion, are there any other contexts that should use the exclusion list? Q6: In this patch, I've applied the filtering to code completion, javadoc completion and fix imports. Are there any other "global" operations (e.g. goto type) that a user would expect this to apply to? I do not believe "goto type" should be excluded as I cannot imagine that as a good user experience. Q7: Should the code completion provide the blacklisted types if they are explicitly imported?
Created attachment 78710 [details] 2nd attempt
There is a bug (or feature) regarding whitelists. If the package "sun" is added to the blacklist, but "sun.security" is added to the whitelist... then completing on the letters "su" do not provide the "sun.security" package. However, if the user completes on "sun." then the package is displayed. Classes from the "sun.security" package will always be completed. Supporting this corner case would eliminate some of the performance improvements provided by this RFE... should it be supported?
ignore last concern, trivially avoided by editing the code to be if (include.size() > 0) for (String entry : include) { if (entry.length() > fqn.length()){ if (entry.startsWith(fqn)) return false; } else if (fqn.startsWith(entry)) return false; }
I've now started work on the UI. It involves making the Java Completion JPanel use a CardLayout and then providing a button named "Exclude" which changes the pane. The new pane has 2 tabs: include and exclude, which show tables of the prefixes to go into the blacklist/whitelist. There are 2 buttons beside this "add" and "remove" to add new prefixes or remove existing ones. All existing entries are editable. Will submit a patch when it's all tied up to work with the Preferences system.
Created attachment 78870 [details] mock up of new completer window (one extra button)
Created attachment 78871 [details] mock up of excluder UI
Created attachment 79003 [details] patch now has UI and working preferences
Confirming that blacklist-5.patch should apply cleanly to main-golden as of now (changeset 123841:49efc443b305). The following code snipped should illustrate the various auto-completions contexts that are subject to this patch (also fix imports). The following code does not compile, but when edited in the NetBeans Java editor, code-complete can be executed at the various obvious points. The packages "sun." and "sunw." are in the default exclude list. If this is accepted, my next action will be to update the "sun.*" warning hinter to use the exclusion list. The include list is provided as a way to override the exclude list on a case-by-case basis. e.g. exclude sun. but keep sun.security. public class Main { /** * {@link } doesn't complete packages OK * {@link Sun } doesn't complete classes OK * {@link sun.security.provider. } completes classes OK * {@link sun. } doesn't complete packages OK * * @param args the command line arguments */ @ // annotations don't complete OK public static void main(String[] args) { new Sun(); // auto-import doesn't import package OK new // doesn't complete packages or classes OK new sun. // doesn't complete packages OK new Sun // doesn't complete classes OK new sun.security.provider. // does complete classes OK } }
Thanks for the contribution. A few comments on the patch: 1. I would recommend to define a simple method (static if possible) somewhere, and hide the implementation. E.g.: o.n.m.java.editor.Utilities.isExcluded(CharSequence s) (note the method takes CharSequence, not String). 2. There is a preferences listener in o.n.m.j.e.Utilities - it would be easier, IMO, to reuse this listener. 3. Might be also good to place the method into the public API (o.n.api.java.source.ElementUtilities.isExcluded?), necessary if the excludes list is to be used by Go to Type, or other features not implemented in java.{editor|hints}. 4. WeakListener allows to unload the class. 5. The completion part relating to types could be moved to o.n.m.j.e.LazyTypeCompletionItem.accept, IMO. The current impl. invokes isExcluded even on types that are not actually visible in the code completion. 6. I think that the impl. should be thread safe (j.u.concurrent.atomic.AtomicReference might help). 7. The UI seems a bit non-standard to me. Would be better, IMO, if the "Exclude" button would open a dialog. Also, the buttons should be aligned (Exclude button is not for me on Linux, Add and Delete should be aligned from both sides, IMO. ), have mnemonics, Remove is typically used instead of Delete, IMO, etc. 8. Bug: by default, the customization dialog shows that sun. and sunw. are excluded, but the code completion does not follow this setting (as the default value is specified in CodeCompletionPanel - might be better to declare a constant for the default value). Might also be a good idea to ship the IDE with empty excludes/includes list. 9. (formal) Please do not use tab characters and exclude unrelated and no-op changes (last hunk of CodeCompletionPanel), do not omit brackets for if/else statements, especially for multi-line ones. Collection.isEmpty() might be simpler than Collection.size() > 0. 10. When modifying the Forbidden Package hint, the possibility to be warn about a package, which is not excluded by the "excluder" should be preserved, IMO(e.g. add a checkout "Automatically warn about excluded packages").
Thanks for the feedback. Your comments on concurrency answer one of my questions... I will fix this. Your comments on Utilities (sort of) answer my question about the singleton, is it still OK to keep a static field reference to the implementation? Optimally, unloading of the class should be possible, but a static ref kills that... meaning there is no point in using a WeakListener, because we are never GCd. A popup UI was my first instinct, but I opted for the CardLayout because it was easier to implement. Will revert to a popup... it all looks aligned on my screen (OS X), so happy for you to tweak it after my next submission. In fact, on OS X, the other JavaCompletion components look unaligned! Didn't realise I was using TAB characters, sorry! Will check my settings. Well spotted on the bug... will fix that and make the default empty. Will also do everything else you've recommended and get back, probably Tuesday - Thursday.
Created attachment 79022 [details] proposed patch
Latest patch uploaded. Responses to your comments below:- 1. Done! 2. Done! I presume the null parameter is only ever received in the first call to lazyInit(). 3. Not sure which project this is in, will leave it alone for now. 4. The implementation is a static field in Utilities, so I don't think unloading can ever happen anyway. Is there a better solution than using a field? I had hoped there would be a non-persistent object cache with fast retrieval. 5. Done. 6. Done, I used a ReadWriteLock to guard against changing the white/blacklists. Note that the thread safety of Utilities is very suspicious to me. None of those fields (besides excluder) are declared volatile (i.e. not guaranteed to be seen across different threads) and the lazyInit static method is subject to a race condition. 7. Now is a popout instead of CardLayout and use "Remove" instead of "Delete". Should now have better alignment in the popup, not sure how to fix the completion pane as it looks fine for me on OS X. 8. Bug should now be fixed. I also took the liberty of creating constants for other preferences as there was code duplication across the Panel and Utilities. I'd urge you to consider shipping "sun.,sunw." as the default blacklist... that way "forbidden" and "excluded" can be thought of as the same from a user perspective. 9. Sorry about that. However I'd rather not change these settings for all my projects... why not define formatting options for the project? This could be something that perhaps all netbeans modules should do. There are probably still some missing brackets for if-continue checks. Note that Collection.isEmpty() is Java 6. 10. I had thought from a user perspective the excluded packages and forbidden packages would be the same. However your comment to have an empty blacklist by default is contrary to that line of thought.
Re 9 and Collection.isEmpty... I'm being stupid, of course this is available from the Collection interface (not Collections). Please amend lines 77 and 89 of ExcludeCompletion accordingly.
Sorry to join with a comment so late in the game, but I just saw Sam's reference to this RFE in the nbusers mailing list. I think it would be great if this inclusion/exclusion list would somehow also be integrated with the Go-to-Type dialog. One of my pet peeves with this dialog is that it shows too many classes! I have a project with 3800 classes. Most of the time, I'm not interested in seeing the java.* classes intermingled with the one I'm looking for. There may already be an RFE to enhance the Go-To-Type dialog in this way, but it seems like this RFE could be extended into that area as well.
Excluding the java.* packages might be a bit tricky! That would mean that they'd not show up in standard code completer. That said, there might be some merit in having a separate exclude list for the go to type. For now, I can certainly investigate using the same exclude list. I'll create a new RFE when this one is accepted and make sure it goes to the right people in go to type.
fommil - which branch/repository are you working from? Curious so I can apply the patch to mine. Thanks.
Created attachment 79180 [details] stylistic improvements
for the sake of completeness, I made the Collection.isEmtpy change in the 7th patch proposal. It doesn't include the fix for bug 160602 as it has been included into the jet tree already. wadechandler, I'm working against main-golden.
Created attachment 79445 [details] add support for excluding methods
The latest patch includes support for excluding methods, in my own exclude list I have:- sun. sunw. java.lang.Object.wait java.lang.Object.notify achieving (almost) what was requested in the original RFE. This means that methods that come from classes in excluded packages, will also be excluded. I looked into the API export, but don't know where this happens. Note that I believe "Go To Type" should indeed use this excluder. I looked into the possibility of adding support for a warning in a hint, but it seemed to be a big job. The Imports hint is not particularly intuitive to me. It would involve a break in backwards compatibility to dump the "Forbidden" list in preference of the "Exclude" list. There are still 3 outstanding corner case bugs that I believe can be lived with:- 1 - consider the case of excluding "org.foo" and "org.bar". If these are the only "org." packages in the classpath, the completer will still include "org" in the list of completion options, even though it offers nothing. 2 - if a package is excluded, it is still possible to access its classes by completing on the fully qualified name. e.g. although "sun." is excluded, the completer will still provide classes if asked to complete on the string "sun.security.provider.". These objects will be difficult to use as their methods will not autocomplete. I'd like to exclude these, but don't know where it is happening. I tried in addTypes and 3 - the method excluder isn't working for methods for the current class. e.g. this.wait() is still appearing in the completer. I will look into this. Also, I noticed that calling Utilities.getElementName(element, true) on a METHOD Element resulted in an empty string! I have worked around this with Utilities.getElementName(e.getEnclosingElement(), true) + "." + sn to recreate the FQN of the method. I didn't want to make any edits to Utilities.getElementName directly.
Created attachment 79447 [details] addresses 3rrd bug from previous comment
Managed to fix the 3rd bug... "method excluder isn't working for methods for the current class". This was all happening in addLocalMembersAndVars, which has a lot of code duplication with addMembers! I believe the original RFE is now fully implemented :-D My personal exclude list looks like sun. sunw. java.lang.Object.wait java.lang.Object.notify java.lang.Object.clone java.lang.Object.finalize
Created attachment 79458 [details] last 2 patches were missing UI changes!
Created attachment 79459 [details] first attempt at integration with hints
Created attachment 79460 [details] stupid 1 character typo in previous patch! grr... a mercurial account would be nice. This still against main-golden.
Created attachment 79471 [details] hint for excluded classes now works
The latest patches are:- - blacklist-11.patch - hints-excluder-2.patch The former implements the original RFE and the latter provides hint warnings if excluded classes are used :-) (editing the Imports hint was trivial) I would strongly recommend that a default exclusion list of "sun.,sunw." be used and I would also argue that this feature now deprecates the "Forbidden Package" hint feature! I cannot think of any situation where a user would want to have a different "exclude" and "forbidden" list. I'd be happy to create a patch that removes that feature... however, user settings would most likely be lost and they'd have to re-enter them. There are 3 outstanding issues, but I don't think either are critical enough to hold off inclusion of this patch (unless msauer has more comments!):- - providing access to this feature in the public API so that it can be used in Go To Type (this has been requested by other users) - does the UI need any more work? My biggest concern is that users expect to be able to enter wildcards or regexes, but such entries are silently ignored upon entry. - where is the documentation? I'd be happy to write a few paragraphs to describe where the exclude list is used and some examples.
Created attachment 79473 [details] really cheeky... this one removes "Forbidden" packages. Would advise revising the default exclude list in this case.
Created attachment 79475 [details] fix stupid typo regarding default WHITELIST and also set default BLACKLIST to "sun.,sunw."
After a few polishes, I guess this can be integrated. - 'Remove' button always removes the first JTable item inside Options Dialog instead of selected one - The 'Exclude' list _has_ to be empty by default, sorry. - As for 'Forbidden' packages removal, I believe this could be easily done for next release - The patch should at least allow entries as sun.*, sun.** (users would expect this). Such entries are currently silently removed upon OD confirmation. - I think we will face some critics from our UI guys for the OD section anyway, lets wait for their opinion. As for the implementation, I agree with J. Lahoda number 1) -- a static Utilities method would be best approach. But we can live with one new class ;) I'll attach a patch using AtomicReference instead of ReentrantReadWriteLock.
Created attachment 79524 [details] patch using AtomicReference
thanks for the feedback, will look at this tonight. Regarding the static method, I followed that approach ;-) there is a separate class but it is hidden and all uses are through Utilities.isExcluded() I believe this is what jlahoda meant, no? How come ReadWriteLock can't be used? It's a Java 1.5 class.
> Regarding the static method, I followed that approach ;-) there is a separate class but it is hidden and all uses are through Utilities.isExcluded() I believe > this is what jlahoda meant, no? I'm not really sure what he meant ;) but your implementation will load a new class. The whole functionality could be covered just by 'isExcluded()' method, not requiring the singleton and a new class. > How come ReadWriteLock can't be used? It's a Java 1.5 class. It can be used. Use of AtomicReference is however more suitable in this case, ReentrantReadWrite lock is an overkill imo. There is also no need for try- finally blocks in your code.
I'm a big fan of doing things in objects instead of monolithic static classes ;-) I see what you are trying to do with the AtomicReference, but not sure it's doing exactly what you intended... will have a look at it tonight. The current patch will expose a race condition where there is a period of having an empty Collection.
-AtomicReference would be better performance-wise than ReadWriteLock. Consistency between exclude and include list (if necessary to keep it at all, which is probably not strictly necessary), could be kept by using one AtomicReference holding a pair of Collection<String>. -Max' use of AtomicReference is of course wrong. It should be something along this lines (+possible Pair): private void update(AtomicReference<Collection<String>> existing, String updated) { Collection<String> nue = new LinkedList<String>(); if (updated == null || updated.length() == 0) { existing.put(nue); //one exit point? return; } String[] entries = updated.split(","); //NOI18N for (String entry : entries) { if (entry != null && entry.length() != 0) { nue.add(entry); } } existing.put(nue); } get() should of be also called only once, of course. Collections.unmodifiableCollection could be used to prevent accidental writes, but that is nitpicking. -if I would write the patch, I would not create a new top-level class, but I do not think this is critical. -re holding data statically - not a problem IMO, as only a very limited amount of data will be retained (the memory overhead of the Exclude class may be bigger than the data stored in exclude/include list). -re class unloading: informally speaking, a class can be unloaded if there is no instance of this class, there are no references to the Class object and its loading classloader is GCable. Classloader is GCable if there are no references to it and all classes that were loaded by this classloader are GCable. So, one leaking instance can prevent unloading of quite a few classes. Currently not critical (as unloading of module classes is rare occurrence), but better prevent possible future problems. -if you want to use these excludes in Go to Type, you will probably need to place the isExcluded method into java.source (ElementUtilities would be the best place IMO), see my original note 3. I have tested -7 patch, and the UI still seemed imperfect on Linux - I will take a look at the newest patch.
Will upload another patch later today to:- - do everything in the static Utilities, not top level class (I'll look into putting this in java.source)... this is not the way I would do it, but having heard feedback from 2 devs on this, I'm more than happy to follow your lead on this. Not entirely convinced about the argument regarding the memory overhead of the Exclude class, though ;-) - more work on UI, particularly semi-regex support - AtomicReference instead of Read/Write, it makes sense for sure
when I open up java.source in Netbeans 6.5.1 I get compile errors... specifically on line 580 of ElementUtilities undef.add(ms); But running "Build" and "Clean/Build" seem to work fine. Hmm. Also, I note that "Jump To" does not depend on "java.source"... is this really the place to put this? I'll leave it in java.editor for now and Go To Type support can go in a follow up.
Created attachment 79570 [details] all static in Utilities, * allowed in UI, "Remove" fixed... I admit the UI could do with input from an NB UI dev. Perhaps each entry should have an "Edit" button which opens up an input dialog, with a line of context (as per the old "Forbidden" panel). D
Another lesson: Attachment comments are not for *long* comments, sorry! :-)
I've integrated the patches. Thank you for your contribution Sam. --- http://hg.netbeans.org/jet-main/rev/343e4ba773e2 http://hg.netbeans.org/jet-main/rev/3af1e26f19ea
Integrated into 'main-golden', will be available in build *200904091401* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main-golden/rev/343e4ba773e2 User: Max Sauer <msauer@netbeans.org> Log: #125060: Exclude classes from code completer
I have a patch almost ready that greatly improves the UI for this feature, and also some additional concurrency protection in Utilities. Will upload today or tomorrow.
Created attachment 79915 [details] update to main-golden 20090410
Latest patch includes:- * much improved UI! User input is now validated and inline help is available when adding new entries. * user option to set/unset excluder for methods. This could easily be updated to "members" in future if requested. * noted that addPackageContent was not calling the excluder (hence queries such as sun.security.provider.COMPLETE) were previously returning excluded results * JavaDoc completer brought up to speed with Java completer (note there is a *lot* of code duplication in this file with Java Completer, in general) * concurrency hardening in Utilities. All fields are now volatile and lazyInit() uses an AtomicBoolean. There was a race condition in the existing code regarding the way 'inited' was being checked, resulting in a potential multi-init. * Utilities given a private constructor and made final (as is good and standard practice for static utility classes)
I'm re-opening this because there is a remaining patch still to be considered/committed (uploaded Apr 10).
I'd like to NetFIX [1] this bug. Is it possible? [1] http://wiki.netbeans.org/NetFIX
Yes. Please fix a bug I've encountered first -- a method/package listed in Excludes cannot be removed vie 'Remove' button from the list. Settings are never updated upon Options Dialog confirmation.
@msauer... is the "remove" bug in main-golden or after application of the update patch "java.editor-excluder-update-2.diff" dated 10 Apr?
@fommil: The version in trunk is OK. Problem occurs with the latest patch (java.editor-excluder-update-2.diff).
Created attachment 80725 [details] update to main-golden 20090422 and fix "remove" bug
I have just assigned #163902 to fommil and making this issue depend on it. Please check the report there ... Thanks a lot, David
@msauer I'm giving the latest patch a makeover, expect an update over the weekend. Taking feedback from issue 163902.
Created attachment 81715 [details] add accessible names, as per issue 163902
OK, was easier to do than I thought... no need to wait till the weekend :-)
Integrated as part of http://hg.netbeans.org/jet-main/rev/1573083284ea
Does this allow me to exclude a directory/package from the project scan ? we have a directory in the source tree that has a lot of jar files in it and thus slows the project scan down.
@nleck: It doesn't. It just affects Code Completion. The only option would be to move them outside src tree. Feel free to file an ENHACEMENT against java/editor, but note that use of non-indexed sources would result in error marks inside editor.
They still get scanned, but won't appear in completion.