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 68687 - "Convert SwingUtilities Methods" transformer tries to reformat runnables
Summary: "Convert SwingUtilities Methods" transformer tries to reformat runnables
Status: RESOLVED FIXED
Alias: None
Product: contrib
Classification: Unclassified
Component: Jackpot (show other bugs)
Version: 5.x
Hardware: All All
: P2 blocker (vote)
Assignee: _ tball
URL:
Keywords:
Depends on:
Blocks:
 
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
Exception Reporter:


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.