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.

Bug 89605 - Language independent refactoring API
Summary: Language independent refactoring API
Status: RESOLVED FIXED
Alias: None
Product: editor
Classification: Unclassified
Component: Refactoring (show other bugs)
Version: 6.x
Hardware: All All
: P2 blocker (vote)
Assignee: Jan Becicka
URL: http://wiki.netbeans.org/wiki/view/Re...
Keywords: API, API_REVIEW, UMBRELLA
: 66935 (view as bug list)
Depends on: 51337 95977 95978 95979 95980 95981 96014 96015 96016 96017 96019
Blocks: 90532 90533 95490
  Show dependency tree
 
Reported: 2006-11-19 13:00 UTC by Petr Hrebejk
Modified: 2007-04-03 18:02 UTC (History)
9 users (show)

See Also:
Issue Type: ENHANCEMENT
Exception Reporter:


Attachments
Arch description. (70.63 KB, text/html)
2007-02-05 15:48 UTC, Jan Becicka
Details
Javadoc (295.26 KB, application/octet-stream)
2007-02-05 15:59 UTC, Jan Becicka
Details
Test coverage. Thanks to Jirka Prox. (108.54 KB, application/x-compressed)
2007-02-09 16:56 UTC, Jan Becicka
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Petr Hrebejk 2006-11-19 13:00:54 UTC
Make the refactoring API language agnostic.
Comment 1 Petr Pisl 2006-11-22 13:05:14 UTC
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.
Comment 2 Jan Becicka 2006-11-22 13:56:15 UTC
*** Issue 66935 has been marked as a duplicate of this issue. ***
Comment 3 Jan Becicka 2006-11-29 09:02:37 UTC
Prototype will be ready for M6, final for M7 I hope.
Comment 4 Jan Becicka 2007-02-05 13:48:37 UTC
Dependency on issue 48427 and issue 76722 is not blocker for this task.
Comment 5 Jan Becicka 2007-02-05 15:46:21 UTC
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.
Comment 6 Jan Becicka 2007-02-05 15:48:39 UTC
Created attachment 38056 [details]
Arch description.
Comment 7 Jan Becicka 2007-02-05 15:59:20 UTC
Created attachment 38057 [details]
Javadoc
Comment 8 Jan Becicka 2007-02-06 08:54:46 UTC
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.
Comment 9 Jesse Glick 2007-02-09 03:31:34 UTC
[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.
Comment 10 Jan Becicka 2007-02-09 09:43:28 UTC
Jesse, thanks for review. I added questions to wiki page at
http://wiki.netbeans.org/wiki/view/RefactoringAPIReview
Comment 11 Jaroslav Tulach 2007-02-09 10:15:33 UTC
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.
Comment 12 Jan Becicka 2007-02-09 11:40:39 UTC
Thanks for review. Questions moved to wiki page. Please add additional comment
to wiki page. It is easier to work with comments through wiki.
Comment 13 Jan Becicka 2007-02-09 16:56:48 UTC
Created attachment 38305 [details]
Test coverage. Thanks to Jirka Prox.
Comment 14 Sonali Kochar 2007-02-09 18:44:25 UTC
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

Comment 15 _ sandipchitale 2007-02-13 06:13:39 UTC
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.
Comment 16 _ sandipchitale 2007-02-14 17:46:12 UTC
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.
Comment 17 Jan Becicka 2007-02-15 17:54:56 UTC
API review result is accepted with TCRs, which must be implemented. Final
version will be ready after M7.
Comment 18 Jan Becicka 2007-02-16 14:10:46 UTC
There were several request for better documentation.
I created FAQ page. Feel free to add more questions.
http://wiki.netbeans.org/wiki/view/RefactoringFAQ
Comment 19 Jan Becicka 2007-02-21 11:04:48 UTC
All TCRs and TCAs implemented. If there are no objections, I will close this
issue as fixed.
Comment 20 Jan Pokorsky 2007-02-23 13:47:34 UTC
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.
Comment 21 Jan Becicka 2007-03-14 17:47:34 UTC
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?

Comment 22 Jan Pokorsky 2007-03-14 21:32:31 UTC
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.
Comment 23 Miloslav Metelka 2007-03-15 09:14:27 UTC
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.
Comment 24 Jan Becicka 2007-03-17 02:08:00 UTC
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.