Bug 68687 - "Convert SwingUtilities Methods" transformer tries to reformat runnables
"Convert SwingUtilities Methods" transformer tries to reformat runnables
Status: RESOLVED FIXED
Product: contrib
Classification: Unclassified
Component: Jackpot
5.x
All All
: P2 (vote)
: TBD
Assigned To: _ tball
issues@contrib
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-11-14 20:33 UTC by Jesse Glick
Modified: 2005-11-16 05:50 UTC (History)
0 users

See Also:
Issue Type: DEFECT
:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2005-11-14 20:33:28 UTC
If I have some code like

SwingUtilities.invokeLater(new Runnable() {
    public void run() {
        boolean b = m();
        use(b);
        //abuse(b);
    }
});

then the Jackpot transformation will try to rewrite to

java.awt.EventQueue.invokeLater(new Runnable() {
    public void run() {
        boolean b = m();

        use(b);
    }
});

1. Should at least offer to import EventQueue.

2. Should not add blank line after declaration. Regardless of what my formatting
preferences are set to for *new* code, the Runnable anonymous class impl is not
what is being modified, so it should never be reformatted.

3. Should not delete comment at end of block. It may have been meaningful.

Marking P2 because when I actually try to use this transformer on my code, I
just have to reject all the diffs; safer to do it by hand or with a Perl script
or something.
Comment 1 _ tball 2005-11-14 21:30:29 UTC
Perl?  Anything but that!  I'll get right on it!!!
Comment 2 _ tball 2005-11-16 04:59:35 UTC
Fixed issues 2 and 3 by using a more narrow rule match.  I am splitting off
issue 1 to a separate issue, 68779.
Comment 3 Jesse Glick 2005-11-16 05:21:25 UTC
Re. "fixed issues 2 and 3 by using a more narrow rule match" - seems to me that
the bug was in the patching engine, not the rule;

$su.invokeLater($r) => java.awt.EventQueue.invokeLater($r)

uses the same term $r on both sides, so IMHO any text characters within the
physical bounds of what the expression $r represents - from 'n' to '}' - should
be left untouched, with the exception that it is permissible to add or remove a
fixed amount of indentation whitespace on each contained line (though even that
should be a configurable option, since for really long or heavily edited blocks
it is often better to leave the indentation broken rather than touch interior
lines).
Comment 4 _ tball 2005-11-16 05:50:22 UTC
You are right, it's a hack.  The underlying problem is that the patching engine
works on trees, not lines, and a transform involves rewriting one tree with
another.  By removing the "($r$)" from both method invocations, only their
select children are matched and not their argument lists -- okay for this
transformation, but not a general solution.  I've special-cased class and method
trees where you obviously don't want all of their children rewritten but need to
enhance the tree-diff'er to do a better job comparing children of other tree
types.  

I'll file a separate bug for this, but don't want to rewrite the tree-diff'er
until there is a better regression suite in place.  The block simplifier needs
work, too, and can be done as part of this rewrite.


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