INRIA / spoon

Spoon is a metaprogramming library to analyze and transform Java source code. :spoon: is made with :heart:, :beers: and :sparkles:. It parses source files to build a well-designed AST with powerful analysis and transformation API.
http://spoon.gforge.inria.fr/
Other
1.74k stars 347 forks source link

CtExecutableReference in cloned method should refer to the clone, not to the method being cloned. #1869

Closed leventov closed 6 years ago

leventov commented 6 years ago

Diff of the test which fails:

diff --git a/src/test/java/spoon/test/method/MethodTest.java b/src/test/java/spoon/test/method/MethodTest.java
index 0b585aa0..d689d4c0 100644
--- a/src/test/java/spoon/test/method/MethodTest.java
+++ b/src/test/java/spoon/test/method/MethodTest.java
@@ -17,6 +17,7 @@

 package spoon.test.method;

+import org.junit.Assert;
 import org.junit.Test;
 import spoon.Launcher;
 import spoon.reflect.declaration.CtClass;
@@ -24,6 +25,8 @@ import spoon.reflect.declaration.CtMethod;
 import spoon.reflect.declaration.CtType;
 import spoon.reflect.declaration.ModifierKind;
 import spoon.reflect.factory.Factory;
+import spoon.reflect.reference.CtExecutableReference;
+import spoon.reflect.visitor.CtScanner;
 import spoon.reflect.visitor.filter.NamedElementFilter;
 import spoon.test.delete.testclasses.Adobada;
 import spoon.test.method.testclasses.Tacos;
@@ -110,4 +113,22 @@ public class MethodTest {
        assertTrue(compareFound);
    }

+   @Test
+   public void testCloneMethod() throws Exception {
+       Launcher l = new Launcher();
+       l.getEnvironment().setNoClasspath(true);
+       l.addInputResource("src/test/resources/noclasspath/A2.java");
+       l.buildModel();
+       CtMethod<?> methodB = l.getFactory().Class().get("A2").getMethodsByName("b").get(0);
+       CtMethod<?> methodClone = methodB.clone();
+       methodClone.accept(new CtScanner() {
+           @Override
+           public <T> void visitCtExecutableReference(CtExecutableReference<T> reference) {
+               if (reference.getDeclaration() == methodB) {
+                   Assert.fail("Cloned method shouldn't have references to the former method");
+               }
+               super.visitCtExecutableReference(reference);
+           }
+       });
+   }
 }
monperrus commented 6 years ago

I have two concerns with this.

First, there is no clearly better semantics. One can object that the cloned should call the same methods as before. Both semantics can be confusing depending on one's assumption.

Second, it is impossible, since reference resolving is name-based and the method name after cloning is the same, so it would resolve to the same declaration. That's normal.

One solution would be that in the getDeclaration, one detects that there are two possible resolutions, and one throws an exception "ambiguous resolution".

monperrus commented 6 years ago

the issue title says CtParameterReference but your test case is for CtExecutableReference.

For CtParameterReference, I agree with you, it should point to the clone. Does it work already?

leventov commented 6 years ago

It just produces wrong AST, later transformations with it are going to break. It's not about calling (or otherwise referencing, in Spoon's AST model) other methods, it's about referencing itself.

It's not only the problem with CtParameterReference. If a method is recursive, i. e. calls itself, the cloned method should also become "recursive", not calling the "other" method.

leventov commented 6 years ago

Also I think if CtClass is cloned, all methods in the clone should refer to each other, rather than refer to the methods in the class being cloned.

This issue might be solved generically in CloneVisitor. Along with producing a clone, it creates an IdentitityHashMap of all declarations being cloned. Then, the clone is traversed once again, replacing all references to the cloned declarations.

A-la identity tracking in the Java's built-in serialization mechanism.

monperrus commented 6 years ago

It's not only the problem with CtParameterReference.

It seems that there is no problem with parameter references, they refer well to the clone, as verified from your example per

        methodClone.accept(new CtScanner() {
            @Override
            public <T> void visitCtParameterReference(CtParameterReference<T> reference) {
                assertSame(methodClone, reference.getDeclaration().getParent(CtMethod.class));
                super.visitCtParameterReference(reference);
            }
        });

Changed the title of the issue to refer to CtExecutableReference instead.

monperrus commented 6 years ago

So what you are saying that that cloning should comply with the following contract:

for all references ref in x, 
   if ref.getDeclation = x before clone
      refclone.getDeclaration = xCLone after clone  

I agree, this generic contract could probably be enforced in CloneVisitor.

Is that what you mean?

leventov commented 6 years ago

@monperrus yes, if I understand it right. And it's not about just one cloned object x, as in the given example of CtClass and cross-referencing methods.

monperrus commented 6 years ago

The problem if we tailor the cloning code like this is that we break the contract

y = x.clone <=> y.equal(y)

See failing tests in https://travis-ci.org/monperrus/spoon/jobs/345684253 of branch https://github.com/INRIA/spoon/compare/master...monperrus:refactor-clone-relative-3

leventov commented 6 years ago

Equality of arbitrary ASTs is a tricky question. I understand equality of constructs (expressions, primarily) within an execution block, that could be used for implementing optimizations such as CSE. The change in the clone() behaviour won't affect this, because we never do CSE optimizations across methods (once they are cloned).

On the level of methods and above, I don't think equiality makes much sense. Each declaration is inherently unique, so inheriting Object's equality seems more reasonable to me.

monperrus commented 6 years ago

In Spoon, we are interested in syntactic equality, not behavioral equality (as for CSE).

Currently, the beauty is that we generate the code of clone and equal from a unique specification, and hence by construction, the contract is satisfied.

leventov commented 6 years ago

What's practical application of syntactic equiality of methods and classes? For example, in my code, I always use IdentitiyHashMap when operating with CtMethods.

monperrus commented 6 years ago

Here is a wrap-up. We study whether cloned recursive methods can point to themselves. For this, we can change the model, the cloning code or both.

Overall, I'm considering either Option 2 or WONTFIX.

leventov commented 6 years ago

For me, it's definitely 4. Could you give a specific example when people do compare for equality large execution blocks and declarations such as methods and classes?

monperrus commented 6 years ago

Well, I don't want to go over, for instance, the 10,226 occurrences of "equals" in astor to show you exactly the problem.

monperrus commented 6 years ago

Here is a 5th solution: https://github.com/INRIA/spoon/compare/master...monperrus:refactor-clone-relative-4 The idea is to define special cloning methods which 1) fit this interesting use case we discuss here and 2) keep full backward compatibility and all contracts.

Note that it goes beyond methods and CtExecutableReferences, because it has support for cloning and rewriring whole classes, it rewires type references, exec references, and field references (see test testCloneRewireLargeClass).

WDYT?

leventov commented 6 years ago

Current clone produces just broken AST, with unpredictable behaviour if you try to apply later transformations, refactorings, or code analysis. I discovered this issue when I doing method dependency analysis, and have seen dependencies that "shouldn't be there", while analysing methods, that were cloned several processing stages before that. I spend several hours chasing for this bug. Current clone() behaviour ought to be fixed.

leventov commented 6 years ago

Well, a separate method like duplicate() will work for me, but think about other users..

monperrus commented 6 years ago

https://github.com/INRIA/spoon/compare/master...monperrus:refactor-clone-relative-4 contains this special method, not called "duplicate" but "cloneAndRewire"

monperrus commented 6 years ago

I would say that your debug session was because current cloning went against your implicit assumptions, not because it is broken.

To me, it's not broken, because cloning is currently very regular and systematic.

The general question we are discussing here is whether clones should include:

leventov commented 6 years ago

It's not about "pretty printing" it's about producing independent entities.

Let's discuss for what cloning is used? I use cloning to produce several slight modifications of one method, to later have them all in one class, instead of writing repetitive code by hand.

Could you give an example when the current semantics of clone are needed?

monperrus commented 6 years ago

yes, this is what I mean, "minimal" means not linked to anything else, hence independent.

what you need/expect seems to be such a minimal / light clone.

Could you give an example when the current semantics of clone are needed?

If you have a class A that uses methods of a class B, and you want that all calls in C=clone(A) to still resolve to B. With a minimal clone, you wouldn't resolve to them (but you would get a pretty-printable and compilable C).

pvojtechovsky commented 6 years ago

I use cloning to produce several slight modifications of one method, to later have them all in one class, instead of writing repetitive code by hand.

Spoon has Templates for that purpose. The templates works like this 1) clone a template element 2) substitute references to template owner by references to receiver of the generated AST

I suggest to keep current Spoon cloning light/simple/stupid because it is predicatable and if you need more clever cloning, then implement a layer above the current clonning, which does what you need. PR is highly welcome.

Note: Current Spoon templating engine is quite powerfull, but it has some limitations. So I am already improving that.

monperrus commented 6 years ago

It's an interesting idea to use the substitution engine for this use case. We may even implement methods cloneAndRewire of https://github.com/INRIA/spoon/compare/master...monperrus:refactor-clone-relative-4 like this. I guess we would need a way to create a template object on the fly from a CtClass.

How would you write the equivalent of ctClass.clone().substituteReferences() with the substitution visitor?

pvojtechovsky commented 6 years ago

I guess we would need a way to create a template object on the fly from a CtClass.

yes, it works like this in #1686

Pattern pattern = PatternBuilder.create(factory, SomeClass.class, 
    templateBuilder -> templateBuilder.setTypeMember("someMethodName"))
  .build();
CtMethod method = pattern.applyToType(targetType, CtMethod.class, Collections.emptyMap());

The pattern above created method which is a copy of SomeClass#someMethodName, with correctly resolved references to targetType and method added as the last member of targetType

monperrus commented 6 years ago

https://github.com/INRIA/spoon/compare/master...monperrus:refactor-clone-relative-4 now clearly covers your use case (see test cases).

It goes even beyond since is support copying types as well.

I think those new methods can be very useful for many transformations, starting with those done by @nharrand

WDYT?

pvojtechovsky commented 6 years ago

These copy methods should not generate name on it´s own, but it should be input parametr - so the copy has directly required name. Otherwise client has to process same algorithm again to change the names. WDYT?

leventov commented 6 years ago

Well, I don't want to go over, for instance, the 10,226 occurrences of "equals" in astor to show you exactly the problem.

I actually "went over them" by adding Thread.dumpStack() to CtExecutableImpl.equals() and run astor's tests. Although one-third of the tests have failed early, so I don't know what I could have found there, in the rest two thirds of the tests all calls to CtExecutableImpl.equals() have one of the following three stack tails:

1)

java.lang.Exception: Stack trace
  at java.lang.Thread.dumpStack(Thread.java:1336)
  at spoon.support.reflect.declaration.CtExecutableImpl.equals(CtExecutableImpl.java:202)
  at java.util.HashMap.putVal(HashMap.java:648)
  at java.util.HashMap.put(HashMap.java:612)
  at java.util.HashSet.add(HashSet.java:220)
  at spoon.support.reflect.declaration.CtTypeImpl$2.accept(CtTypeImpl.java:996)
  at spoon.support.reflect.declaration.CtTypeImpl$2.accept(CtTypeImpl.java:987)
  at spoon.reflect.visitor.chain.CtQueryImpl$OutputFunctionWrapper._accept(CtQueryImpl.java:471)
  at spoon.reflect.visitor.chain.CtQueryImpl$AbstractStep.accept(CtQueryImpl.java:319)
  at spoon.reflect.visitor.filter.AllTypeMembersFunction$1.accept(AllTypeMembersFunction.java:78)
  at spoon.reflect.visitor.filter.AllTypeMembersFunction$1.accept(AllTypeMembersFunction.java:73)
  at spoon.reflect.visitor.chain.CtQueryImpl$OutputFunctionWrapper._accept(CtQueryImpl.java:471)
  at spoon.reflect.visitor.chain.CtQueryImpl$AbstractStep.accept(CtQueryImpl.java:319)
  at spoon.reflect.visitor.filter.SuperInheritanceHierarchyFunction.sendResult(SuperInheritanceHierarchyFunction.java:360)
  at spoon.reflect.visitor.filter.SuperInheritanceHierarchyFunction.visitSuperClasses(SuperInheritanceHierarchyFunction.java:281)
  at spoon.reflect.visitor.filter.SuperInheritanceHierarchyFunction.apply(SuperInheritanceHierarchyFunction.java:248)
  at spoon.reflect.visitor.filter.SuperInheritanceHierarchyFunction.apply(SuperInheritanceHierarchyFunction.java:51)
  at spoon.reflect.visitor.chain.CtQueryImpl$LazyFunctionWrapper._accept(CtQueryImpl.java:505)
  at spoon.reflect.visitor.chain.CtQueryImpl$AbstractStep.accept(CtQueryImpl.java:319)
  at spoon.reflect.visitor.chain.CtQueryImpl.forEach(CtQueryImpl.java:105)
  at spoon.reflect.visitor.filter.AllTypeMembersFunction.apply(AllTypeMembersFunction.java:73)
  at spoon.reflect.visitor.filter.AllTypeMembersFunction.apply(AllTypeMembersFunction.java:36)
  at spoon.reflect.visitor.chain.CtQueryImpl$LazyFunctionWrapper._accept(CtQueryImpl.java:505)
  at spoon.reflect.visitor.chain.CtQueryImpl$AbstractStep.accept(CtQueryImpl.java:319)
  at spoon.reflect.visitor.chain.CtQueryImpl.forEach(CtQueryImpl.java:105)
  at spoon.support.reflect.declaration.CtTypeImpl.getAllMethods(CtTypeImpl.java:987)

There is actually a performance bug in CtTypeImpl.getAllMethods(), if you look at it. It also rolls "signature equality" by hand, that is also a small evidence of uselessness of the "default" method equality.

To fix this performance bug, you should add methods to a hash set with custom equality, either by adding a wrapper of CtMethod (https://stackoverflow.com/a/5453268/648955), or using a custom library (see http://fastutil.di.unimi.it/docs/it/unimi/dsi/fastutil/objects/Object2ObjectOpenCustomHashMap.html)

2)

java.lang.Exception: Stack trace
  at java.lang.Thread.dumpStack(Thread.java:1336)
  at spoon.support.reflect.declaration.CtExecutableImpl.equals(CtExecutableImpl.java:202)
  at spoon.support.reflect.declaration.CtTypeImpl.addMethod(CtTypeImpl.java:626)
  at spoon.support.visitor.java.internal.TypeRuntimeBuilderContext.addMethod(TypeRuntimeBuilderContext.java:54)
  at spoon.support.visitor.java.JavaReflectionTreeBuilder.visitMethod(JavaReflectionTreeBuilder.java:271)
  at spoon.support.visitor.java.JavaReflectionTreeBuilder.visitMethod(JavaReflectionTreeBuilder.java:256)
  at spoon.support.visitor.java.JavaReflectionVisitorImpl.visitClass(JavaReflectionVisitorImpl.java:58)
  at spoon.support.visitor.java.JavaReflectionTreeBuilder.visitClass(JavaReflectionTreeBuilder.java:150)
  at spoon.support.visitor.java.JavaReflectionTreeBuilder.scan(JavaReflectionTreeBuilder.java:111)
  at spoon.reflect.factory.TypeFactory.get(TypeFactory.java:527)

3)

java.lang.Exception: Stack trace
  at java.lang.Thread.dumpStack(Thread.java:1336)
  at spoon.support.reflect.declaration.CtExecutableImpl.equals(CtExecutableImpl.java:202)
  at spoon.support.reflect.declaration.CtTypeImpl.addMethod(CtTypeImpl.java:626)
  at spoon.support.compiler.jdt.ParentExiter.scanCtType(ParentExiter.java:242)
  at spoon.reflect.visitor.CtInheritanceScanner.visitCtClass(CtInheritanceScanner.java:498)
  at spoon.support.compiler.jdt.ParentExiter.visitCtClass(ParentExiter.java:481)
  at spoon.support.reflect.declaration.CtClassImpl.accept(CtClassImpl.java:68)
  at spoon.reflect.visitor.CtInheritanceScanner.scan(CtInheritanceScanner.java:181)
  at spoon.support.compiler.jdt.ContextBuilder.exit(ContextBuilder.java:130)
  at spoon.support.compiler.jdt.JDTTreeBuilder.endVisit(JDTTreeBuilder.java:482)
  at org.eclipse.jdt.internal.compiler.ast.MethodDeclaration.traverse(MethodDeclaration.java:358)
  at org.eclipse.jdt.internal.compiler.ast.TypeDeclaration.traverse(TypeDeclaration.java:1367)
  at org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration.traverse(CompilationUnitDeclaration.java:778)
  at org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration.traverse(CompilationUnitDeclaration.java:739)
  at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.buildModel(JDTBasedSpoonCompiler.java:414)
  at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.buildUnitsAndModel(JDTBasedSpoonCompiler.java:362)
  at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.buildSources(JDTBasedSpoonCompiler.java:335)
  at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.build(JDTBasedSpoonCompiler.java:116)
  at spoon.support.compiler.jdt.JDTBasedSpoonCompiler.build(JDTBasedSpoonCompiler.java:99)

Those two cases boil down to construction of AST and adding newly constructed methods to a CtTypeImpl. CtTypeImpl.addMethod() also compares methods only by signature on the normal path, and compares methods "fully" only to check that there are no bugs in the Spoon itself (throws AssertionError otherwise). So, it's use of current equals() is internal to Spoon (and very specific - checking the library's own sanity), and shouldn't necessarily be exposed to the library users.

Could you provide another example of valid use of CtExecutableImpl.equals() or CtTypeImpl.equals() by the Spoon library users? I cannot imagine one.

leventov commented 6 years ago

If you have a class A that uses methods of a class B, and you want that all calls in C=clone(A) to still resolve to B. With a minimal clone, you wouldn't resolve to them (but you would get a pretty-printable and compilable C).

In this case, the proposed change to clone() behaviour will keep references to B. It will replace A's own references to itself, in the clone instead of references to A references to C are going to appear. But the references to B are unaffected.

So with regard to clone(), I also don't yet see an evidence that the current behavior is ever more reasonable than the proposed behaviour.

Also interesting to note, that my use of Spoon, by which I discovered this issue (I mean the beginning of this topic, #1869), is not new. It used to use Spoon 5.2.0 and worked fine. Recently I tried to update to Spoon 6.1.0 and this issue arose. And I don't see any notes about the change in clone() behaviour that could be relevant in release notes of the versions of Spoon in between. Probably it's just because of #1875 bug that was introduced at some point, but it's also possible that clone() used to work as I propose here, but then silently switched to the current logic, that is now considered "classic".

monperrus commented 6 years ago

after #1875, #1884 is a second concrete outcome of this very interesting discussion.

leventov commented 6 years ago

Could you please answer to my last two messages?

monperrus commented 6 years ago

Could you provide another example of valid use of CtExecutableImpl.equals() or CtTypeImpl.equals() by the Spoon library users?

What do you mean by "valid use"?

leventov commented 6 years ago

When the users actually need those methods as they are implemented today. Rather than actually needing only signature equality, as in the example above.

monperrus commented 6 years ago

It's an interesting point. There are points to consider when you provide a library.

First, for any non-trivial library providing complex behaviors, it is impossible to envision all possible usages. Beyond that, I think that it is success criterion when a library starts to be used in an opportunistic manner, outside the planned and designed usage space.

Second, it is well known that there is always somebody relying on a specific behavior (sometimes called @hwright Hyrum's law, well represented in the XKCD below).

Consequently, humbly, I don't pretend mastering all possible ways of using the clone API, and the Spoon API in general.

leventov commented 6 years ago

Consider the whole picture:

  1. There is a definition of clone(), that hardly makes any sense, because it produces AST broken in some unspecified ways. Also you didn't yet demonstrate the case when this definition would be useful even in theory. But let's assume that somebody needs this definition of clone().

  2. There is a definition of CtMethod/CtClass.equals(), that is demonstrated to be useful only within the Spoon library itself, internally, to check the library's own sanity. It wasn't demonstrated that this definition of equals() on CtMethod/CtClass exposed to users is anything more than a performance trap, when people occasionally use CtMethod/CtClass as HashMap keys, or making a set from a list of ctMethods (applying "distinct"), but instead of reference equality that they really need they try to scan the whole AST trees. And it's really easy to fall into this trap: Spoon core code has fallen itself, see https://github.com/INRIA/spoon/issues/1869#issuecomment-368324519. Users are forced to really carefully use IdentitityHashMap and Collections.newSetFromMap(new IdentityHashMap()).

    With regard to this, I also want to call a citation from "Effective Java": "Don't override equals() unless you really have to: in many cases, the implementation inherited from Object is exactly what you want". Especially considering controversy of this definition: does it compare Java AST objects, or text representations as they are pretty-printed (what about optional spaces and braces in this case?), or code semantics?

  3. It was suggested that there is a requirement that decl2 = decl1.clone() => decl1.equals(decl2), for CtMethods and CtClasses, with the definitions of clone() and equals(), given above. Ok, only note that this is not a requirement of Object.clone() contract. In fact, for every class that inherits equals() from Object (but doesn't implement clone() as return this;) it doesn't hold.

To me, this is a disservice of your users: in order not to break the code for imaginary users, you serve all users with a performance trap, and force all users to carefully write some boilerplate code.

You are free to do that, but could you please provide users a utility that replaces clone() and behaves as explained here: https://github.com/INRIA/spoon/issues/1869#issuecomment-366551014?

pvojtechovsky commented 6 years ago

1 ... There is a definition of clone(), that hardly makes any sense, because it produces AST broken in some unspecified ways.

Current definition of clone is simple: clone each element and attribute recursivelly. No extra mapping.

produces AST broken ...

I read this issue and I do not understand what is broken. I understand that you would like that type references to type A points to clone of A after clonning and to origin A. Spoon has template engine for such operations. It lets you clone any method and redirect type references from origin declaring type to any type which will receive such cloned method ... and many other mappings which are usually needed too - improved clonning algorithm cannot do it well too. So if it is the problem of clonning you see, then I would not call it broken AST, because such AST is valid - no consistency problem. The consistency problems occurs when you add such cloned AST into existing AST ... that is the problem. So for me it looks like you are using clonning in bad way. Use templates instead.

There is a definition of CtMethod/CtClass.equals()

I also do not like current CtMethod/CtClass.equals(). I always had a feeling that it is the cause of performance problems in Spoon. And the stacktrace of CtTypeImpl#getAllMethods you attached above is really a performance problem.... and probably not the only one ;-) Solution? ... I do not know. But I think it should be discussed ... and it might bring some good results.

clonning x equals decl2 = decl1.clone() => decl1.equals(decl2)

if you use templates instead clonning, then this problem is no more relevant. And from my point of view this contract is not interesting.

could you please provide users a utility that replaces clone() and behaves as explained here

I think spoon template is that utility. The legacy templates are already quite usable and we work on even more powerful implementation.

leventov commented 6 years ago

@pvojtechovsky could you please provide the piece of code, using Spoon templates, that does what I want to do? Given a CtMethod object, which I want to "clone". Would it be able to clone references to local class, defined inside that method?

void foo() {
  class Bar {
    void bar() {
      bar();
      foo();
    }
  }
  return new Bar();
}
pvojtechovsky commented 6 years ago

Nice example, I will try that.

@leventov One question: is your real method which you need to clone A) an arbitrary method of the source code B) a special hand written template method, which was made for the purpose "To be cloned" ?

leventov commented 6 years ago

@pvojtechovsky my method is a real Java method of a fully working real Java class, that is not written with the idea that it's going to be cloned in the future. It's not a complete "black box" method, though, I write them myself. They couldn't probably contain local classes, which are exotic, but easily anonymous classes and lambdas.

pvojtechovsky commented 6 years ago

that is not written with the idea that it's going to be cloned in the future.

So then current spoon Templates are not a solution for you. But #1686 is the tool, which will do what you need. I have just added the tests . See test cases in #1686 in src/test/java/spoon/test/template/PatternTest.java

@leventov, please check if these tests correctly checks that all references and executables are mapped well - as you would expect. Thank You

The code for clonning of any method with correct mapping of references looks like this:

//create a spoon template for the method foo of `sourceType`
Pattern pattern = PatternBuilder.create(
        (CtMethod) sourceType.getMethodsByName("foo").get(0))
        .build();
//generate a new method (you call it clone, but it is not a clone) using that spoon template
//the method is added the targetType automatically in this case too
pattern.applyToType(targetType, CtMethod.class, Collections.emptyMap());
leventov commented 6 years ago

@pvojtechovsky could it be boiled down to a single method, that returns another method? Like CtMethod.clone(), just with a different name?

pvojtechovsky commented 6 years ago

CtMethod.clone()

such method would not work, because you at least need to specify reference to target type. But usually the target type reference is not the only parameter and one needs to change the generated method name, and other things...

... or I still do not understand your use case... why do you need to copy whole methods/types ? Do you change anything after copying? If yes, then spoon template can again support you and to generate directly the adapted/modified copy... so you need more parameters anyway.

And if you think that PatternBuilder API and Pattern#apply API is too complicated, then I invite you into discussion in #1686. Now is the best time to improve that API - there are no clients yet, so no problems with backward compatibility ;-)

monperrus commented 6 years ago

[Issue Cleaning Marathon] No more activity here, closing the issue.