Bug 328481

Comment 1 Stephan Herrmann 2010-10-23 06:37:16 EDT

Aside from extending the quickfixes' action, test 
LocalCorrectionsQuickFixTest.testUnusedVariables7() obviously needs
adjusting, since this comment:
  buf.append("        e+=\"\";\n");//makes e an used variable
is no longer true. For an easy fix I suggest changing to s.t. like
  buf.append("        e= e.toUpperCase();\n");//makes e an used variable
(in all three contents strings).

Comment 2 Dani Megert 2010-10-23 06:39:15 EDT

(In reply to comment #1)
> Aside from extending the quickfixes' action, test 
> LocalCorrectionsQuickFixTest.testUnusedVariables7() obviously needs
> adjusting, since this comment:
>   buf.append("        e+=\"\";\n");//makes e an used variable
> is no longer true. For an easy fix I suggest changing to s.t. like
>   buf.append("        e= e.toUpperCase();\n");//makes e an used variable
> (in all three contents strings).

Yep, I'm about to fix the JUnit failures in the latest build.

Comment 3 Deepak Azad 2010-10-27 09:15:18 EDT

Created attachment 181829 [details]
fix + tests

(In reply to comment #0)
> This means we not only remove the declaration but also the usages of the
> variables.
Patch fixes these.

Comment 4 Markus Keller 2010-10-27 10:10:07 EDT

The fix goes into the right direction, but there are some cases that are not handled correctly:

    void foo(boolean b) {
        for (int i= 0; ; ) {
            if (b)
                i++;
            System.out.println("hi");
            i-= 18;
        }
    }
    
    private int fField;
    void bar() {
        new Snippet().fField++;
    }

See RemoveUnusedMemberOperation#removeVariableWithInitializer(..) for existing code that solves such problems when processing assignments.

Comment 5 Markus Keller 2010-10-27 10:19:45 EDT

>         new Snippet().fField++;

Sorry, that's also not handled correctly for assignments, e.g.:
        new Snippet().fField= 12;

Please fix if easily possible, otherwise make a new bug for such problems.

Comment 6 Deepak Azad 2010-10-27 10:55:26 EDT

Created attachment 181841 [details]
fix + tests

Fixes this case as well
>         for (int i= 0; ; ) {
>             if (b)
>                 i++;
>             System.out.println("hi");
>             i-= 18;
>         }

Will file a new bug for 
>     private int fField;
>     void bar() {
>         new Snippet().fField++;
>     }

Comment 7 Markus Keller 2010-10-27 10:59:23 EDT

(In reply to comment #6)
+1 for M3.

Comment 8 Deepak Azad 2010-10-27 11:05:45 EDT

Fixed in HEAD.

> Will file a new bug for 
> >     private int fField;
> >     void bar() {
> >         new Snippet().fField++;
> >     }
Filed Bug 328839.

Comment 9 Deepak Azad 2010-10-28 02:07:33 EDT

Verified in I20101027-1800