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.
Make the refactoring API language agnostic.
Yes we need it in the J2EE team. For as it's importanat to have this ability to create refactoring for web frameworks, html pages etc.
*** Issue 66935 has been marked as a duplicate of this issue. ***
Prototype will be ready for M6, final for M7 I hope.
Dependency on issue 48427 and issue 76722 is not blocker for this task.
History of Refactoring API goes back to NetBeans 4.0. Predecessor of API was not meant to be public in very first version (NB4.0). Demand for some kind of API was higher and higher after 4.0 -> ...and friend API come into the world (NB4.1 - issue 52016). NetBeans 4.1 Refactoring API was tailor-made for Java refactorings and was based on JMI for Java meta-model. All the JMI stuff was unfortunately removed in favor of Retouche project in 6.0. Moreover demand for language independent Refactoring API started to come from several groups (java, jsp, xml). Refactoring API badly needed improvements and promotion to be a public API. Shortly "Refactoring API" module propose to solve these issues and provide unified infrastructure for all Refactoring features for tools based on NetBeans IDE. The source of this module is available in NetBeans cvs under refactoring/api module including javadoc and architectural description. Proposed stability level for 6.0 is Under Development.
Created attachment 38056 [details] Arch description.
Created attachment 38057 [details] Javadoc
I'd like to ask for review of Refactoring API. Required doc attached. Please ask questions at jupiter wiki (see url). If you have any procedural questions (like missing some doc etc..) please add comments here into this issue or send me an email.
[JG01] RefactoringActionsFactory Javadoc example InstanceContent ic = new InstanceContent(); ic.add(node); Lookup l = new AbstractLookup(ic); could be simplified to Lookup l = Lookups.singleton(node); [JG02] Javadoc of BackupFacility is badly broken. [JG03] Strange that BackupFacility is a singleton. Also strange that it uses long's as handles rather than some opaque Handle class. [JG04] TreeElementFactory.cleanUp should not be public if it is an "implementation detail". [JG05] Not clear to me why ActionsImplementationProvider.*Impl methods return a Runnable rather than simply doing the body. [JG06] Various classes and methods are missing Javadoc comments. Everything should be carefully documented.
Jesse, thanks for review. I added questions to wiki page at http://wiki.netbeans.org/wiki/view/RefactoringAPIReview
This is a huge submission. I am pretty sure that the listed usecases do not justify all the methods and classes in the API - for example TreeElement's purpose is not explained much, imho. Y01 What is test coverage for the API, can you generate it and attach it? Can I see the tests that check the API behaviour, where they are? Y02 What getComposite is for? Y03 You should use <api group="property"/> for the properties that either this API itself, or its Java implementation understands. The reason is that 95% of users of the API is likely to work with Java and there is no special API for java (or is it?). That is why this API will serve as a documentation of all the additional properties and behaviours of Java. That is why I think this API shall more document the actual behaviour of Java implementation. Y04 Rewrite Context. It should not subclass AbstractLookup - that is an implementation detail. It should not have add method that is public. If you want it to be public, than justify that with usecases, but I am pretty sure that you want to give the right to modify context just to someone (probably refactoring plugins) but not everyone. Y05 The API is full of Object..., Object[], etc. It deserves a change - maybe for Lookup which is more typed than array of Objects. Whatever happens it shall be documented what are the possible values of these Object... parameters, what Java implementation accepts, etc.
Thanks for review. Questions moved to wiki page. Please add additional comment to wiki page. It is easier to work with comments through wiki.
Created attachment 38305 [details] Test coverage. Thanks to Jirka Prox.
SK0 :: Need openInEditor() and showPreview() methods on the RefactoringElement class. The class implementing RefactoringCustomUI gets a collection of RefactoringElements, which are then used to get the corresponding TreeElements. The XML module code then uses the TreeElements to draw a preview Graph. The Graph is nothing but a different visual representation of the Jtree and we would like similiar (single click/dbl click) mouse behavior on the usage nodes as in Jtree. SK1:: Need a way to filter out duplicate tree nodes (TreeElements) from the Preview Tree
In VW we have a couple of use cases that may not have been covered: - When one renames a Jsp file which is a JsfJspDataObject, we want to do the correct refactoring. However, along with this we want to perform additional refactorings to the backing bean Java file which is a JsfJavaDataObject. We want to delegate most of the Java refactoring to the refactoring provided by refactoring/java module. However we want to also perform additional Java refactorings. - How the inplace renaming of nodes in explorer translates into refactoring actions/operations could also be handled by the refactoring framework in a standard way (i.e. based on canRename() in ActionProviderImpl). If not please at least provide guidance through a sample/javadoc or some other documentation. - Similarly the translation of drag & drop as well as cut/copy/paste into refactoring actions/operations could also be handled by the refactoring framework in a standard way. If not please at least provide guidance through a sample/javadoc or some other documentation.
I did not get a chance to or forgot to mention in time in Feb 13, 2007 API review one more issue about Refactoring Undo and Redo. It is not clear top me as to how the clients are supposed to plug into the Undo and Redo Refactoring actions. Are there standard guidelines for this? This is important because how will several spi implementors coordinate how the Undo/Redo Refactoring actions are supported. Please point to me any location if it is already documented.
API review result is accepted with TCRs, which must be implemented. Final version will be ready after M7.
There were several request for better documentation. I created FAQ page. Feel free to add more questions. http://wiki.netbeans.org/wiki/view/RefactoringFAQ
All TCRs and TCAs implemented. If there are no objections, I will close this issue as fixed.
I am a bit surprised the API contains org.openide.text.PositionBounds without objections from Jarda and Mila as reviewers. Within another review you guys stated it is going to be deprecated.
From my point of view PositionBounds is non deprecated API class and I don't have any single reason to remove it. So can I close this issue?
Sorry for stepping to review so late but my nowadays attempts to implement the plug-in hit to following issues. JP1 - Jardo could you comment on PositionBound? Y02 - this has not been resolved IMO. getComposite should return some descriptor like TreeElement. There could be language specific factories something like JavaTreeElementFactory.createTreeElement(TreePathHande|ElementHandle|FileObject|SourceGroup|...) Now it is absolutely unclear what should the implementer return. JP2 - RefactoringElementsBag - this seems to me a bit over-designed. Is there any reason not to use RefactoringElementImplementation(performChange|undoChange) instead of Transaction interface? RefactoringElementsBag would offer addPlain(), addFileChange(), addTransaction(). REB.add, REB.registerXXX methods and Transaction interface could be removed. JP3 - TreeElementFactoryImplementation.cleanUp - should not it be removed as well as TreeElementFactory.cleanUp (JG04)? JP4 - Rename SimpleRefactoringElementImpl to SimpleRefactoringElementImplementation to keep naming consistent. JP5 - RefactoringElementImplementation.getPosition - javadoc should permit null as a valid option since positions do not make always sense. JP6 - It would be great if you could describe life-cycle of RefactoringElementImplementation, Transaction, ... objects in SPI or FAQs. This would help to prevent potential memory leaks since providers might cache some stuff that can be hold by e.g. UndoRedo manager then.
PositionBounds versus Position: the difference is mainly better performance. With PB you create extra objects and work with PositionRef instances that are further organized in a queue of PositionRef.Manager so it consumes some extra memory and time. The benefit of PB is that if the document over which PB gets created would become closed then the PB would get switched into an "offline" line:column representation automatically. With regular Position you would have to do this manually. Not sure whether this may happen in your case.
Y02: getComposite() method removed in favor of getLookup() method. "Migration guide": Replace Object getComposite() { return value; } with Lookup getLookup() { return Lookups.singleton(value); } and eventually replace MyValue v = (MyValue) el.getComposite(); with MyValue v = el.getLookup().lookup(MyValue.class); JP3: addFileChange method added as suggested. registerTransaction() kept as is, since it is impossible to implement it as suggested. Jan Pokorsky agreed. JP4: Done JP5, JP6: will do later. does not require api change. Just javadoc improvement. Closing as fixed. All other API changes will go through API_REVIEW_FAST. Thanks to all.