Netflix-Skunkworks / rewrite

Distributed code search and refactoring for Java
Apache License 2.0
290 stars 30 forks source link

changeMethodTarget() has unexpected effect on static method invocation #17

Open gunnarmorling opened 7 years ago

gunnarmorling commented 7 years ago

I'm having these classes:

package com.a;

public class A {

    public void foo() {}
}
package com.a;

public class AFactory {

    public static A make() {
        return new A();
    }
}
package com.b;

import com.a.A;
import com.a.AFactory;

public class B {

    A a1 = new A();
    A a2 = new A();

    public void test() {
        a1.foo();
        AFactory.make().foo();
    }
}

And I'm applying the following refactoring:

Path sourceDir = ...;

List<Path> paths = Arrays.asList(
        sourceDir.resolve( "com/a/A.java" ),
        sourceDir.resolve( "com/a/AFactory.java" ),
        sourceDir.resolve( "com/b/B.java" )
);

CompilationUnit cu = parser.parse( paths ).get( 2 );

Class aType = Type.Class.build( "com.a.A", Collections.emptyList(), null, Collections.emptyList() );

CompilationUnit fix = cu.refactor()
    .changeMethodTarget(
            cu.findMethodCalls( "com.a.A *(..)" ), "a2",
            aType
    )
    .fix();

System.out.println( fix.print() );

Then both method invocations in B#test() are modified:

public void test() {
    a2.foo();
    a2.foo();
}

The change of AFactory.make() is surprising. I see how that expression is the target of in invocation of foo(), so it makes sense in a way. Am I just missing the right way to exclude this invocation?

I think in the end a way would be needed to tie a selected invocation not to the type declaring the method but to the element (field, local variable etc.) it is invoked on.

gunnarmorling commented 7 years ago

So I solved this by filtering out all method calls whose select is of type MethodInvocation. Perhaps one could have some way to control the result set of findMethodCalls() more specifically for such purposes?