Bug 209375 - Support for consecutive changes to a tree
Support for consecutive changes to a tree
Status: RESOLVED FIXED
Product: java
Classification: Unclassified
Component: Source
7.2
PC Linux
: P2 (vote)
: 7.2
Assigned To: Jan Lahoda
issues@java
: API, API_REVIEW_FAST
Depends on: 211087
Blocks: 207577
  Show dependency treegraph
 
Reported: 2012-03-09 12:11 UTC by Jan Lahoda
Modified: 2012-04-14 09:44 UTC (History)
2 users (show)

See Also:
Issue Type: DEFECT
:


Attachments
Proposed API change. (5.60 KB, patch)
2012-04-02 13:01 UTC, Jan Lahoda
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Lahoda 2012-03-09 12:11:26 UTC
See bug #207577#c16 (JG18) for context.

Sometimes, it is necessary to incrementally add some features to a tree (e.g. annotations to modifiers, class members, etc). Currently, the code that performs this task needs keep the intermediate results itself - there is no way to read the "target" of WorkingCopy.rewrite. Being able to get the current "target" tree of a rewrite would make writing some of java.hints much easier. It is currently unclear whether there is a strong usecase outside java.hints, so this needs to be investigated.

A possible solution would be to add WorkingCopy.getRewriteTarget(Tree) which would go through the changes map until it would find the tree that is the current "target" rewrite of the given tree.

Another alternative would be to make similar API in spi.java.hints only (in TransformationContext).
Comment 1 Ralph Ruijs 2012-03-09 12:45:45 UTC
Would like to have this available also in java refactoring. I think this would help in solving #208495.
Comment 2 Jan Lahoda 2012-04-02 13:01:50 UTC
Created attachment 117655 [details]
Proposed API change.
Comment 3 Jan Lahoda 2012-04-02 13:02:31 UTC
Please review. Thanks.
Comment 4 Jesse Glick 2012-04-10 23:28:07 UTC
Missing space in "method<code>resolveRewriteTarget</code>".


What would be the expected usage pattern in UseNbBundleMessages as an example? Currently this has something like:

ModifiersTree modifiers = ((MethodTree) enclosing).getModifiers();
ModifiersTree nueModifiers = generatorUtilities.appendToAnnotationValue(modifiers, ...);
wc.rewrite(modifiers, nueModifiers);

I guess this would be rewritten to:

ModifiersTree modifiers = (ModifiersTree) wc.resolveRewriteTarget(((MethodTree) enclosing).getModifiers());
ModifiersTree nueModifiers = generatorUtilities.appendToAnnotationValue(modifiers, ...);
wc.rewrite(modifiers, nueModifiers);

right?
Comment 5 Jesse Glick 2012-04-11 00:06:04 UTC
By the way in core-main #dbb0f6059f57 I have tried to prepare for the new method by playing with a test for the corrected behavior in UseNbBundleMessages. Pending the API change, I expected assertOutput to fail after the second fix by showing just @Messages("k1=v1") rather than @Messages({"k1=v1", "k2=v2"}) as desired. But I also found two other failures:

1. assertVerbatimOutput shows Bundle.properties as containing "k2=v2\n" rather than "" as expected.

2. assertOutput shows org.openide.util.NbBundle.getMessage(Test.class, "k2") rather than k2() as expected.

Perhaps this is just a limitation in HintTest not being prepared for iteratively applied fixes? But then [JG01] HintTest ought to be fixed so that uses of resolveRewriteTarget can be tested like anything else in a hint.
Comment 6 Jan Lahoda 2012-04-11 13:47:09 UTC
The problem with 'org.openide.util.NbBundle.getMessage(Test.class, "k2")' is bug #201871. The problem with the bundle is caused by the fact that the resource content is not read and committed into the Document if available, which I have fixed in changeset jet-main#3c76cc153e62 (will become public soon). After fixing these two problems, the test passes, as the two "applyFix" invocations are independent (i.e. the test does not actually test the consecutive changes to one tree). So far, I do not have exact proposal how to extend the test harness to allow writing such tests (more precisely, the changes I have in mind so far - e.g. adding applyFixes() on HintOutput) might prove to be too limiting in the future). Might be overkill to defer the API addition until the test harness is enhanced.

The usage pattern is as you described.
Comment 7 Jesse Glick 2012-04-11 13:51:23 UTC
(In reply to comment #6)
> I do not have exact proposal how to
> extend the test harness to allow writing such tests [...].
> Might be overkill to defer the API
> addition until the test harness is enhanced.

Agreed; just wanted to make sure this was at least under consideration. Ideally I would want to be able to write a test which fails until resolveRewriteTarget is used.
Comment 8 Quality Engineering 2012-04-12 09:58:08 UTC
Integrated into 'main-golden', will be available in build *201204120400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/dbb0f6059f57
User: Jesse Glick <jglick@netbeans.org>
Log: Commented-out test in preparation for #209375 (sequentially applied fixes).
Comment 9 Jan Lahoda 2012-04-12 11:24:32 UTC
Integrated:
http://hg.netbeans.org/jet-main/rev/4fe09b8ec828

Created #211087 for JG01.
Comment 10 Quality Engineering 2012-04-13 09:50:30 UTC
Integrated into 'main-golden', will be available in build *201204130400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/3c76cc153e62
User: Jan Lahoda <jlahoda@netbeans.org>
Log: #209375: non-Java resource changes must respect content of any Document opened for the given resource
Comment 11 Quality Engineering 2012-04-14 09:44:00 UTC
Integrated into 'main-golden', will be available in build *201204140400* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/3375b3dc27eb
User: Jesse Glick <jglick@netbeans.org>
Log: Taking advantage of #209375.


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo