Depends on #1238. feature: TypeFactory#createReference(type,includingFormalTypeParams) #1237

pvojtechovsky · 2017-03-23T20:53:19Z

A baby step. The TypeFactory method which creates a CtTypeReference, which has initialized actual type arguments following formal type parameters of the input CtType.

I need it to simplify architecture of CtTypeParameter resolving algorithm. This algorithm can be theoretically applied to
A) CtTypeReference - which has actual type arguments
B) CtType - which has formal type parameters

The new method of this PR let's me simplify it to A) only, because B) can be easily covered to A) by:

CtTypeReference typeRef = fatory.Type().createReference(type, true);

monperrus · 2017-03-24T09:53:27Z

it is clearly backward-compatible.

By definition, the actual type arguments would never be bound, they would only by "E" or "T" because they come from the formal definition? Correct?

could you add a test?

pvojtechovsky · 2017-03-25T08:00:10Z

"Houston, we have a problem"...

The test fails... because there is a problem in a spoon model. I made issue #1238 for that.

monperrus · 2017-03-27T10:24:40Z

OK, let's fix #1238 first then.

monperrus · 2017-03-30T15:00:36Z

here is a fix. WDYT?

spoon-bot · 2017-03-30T15:04:37Z

Revapi Analysis results

Old API: fr.inria.gforge.spoon:spoon-core:jar:5.6.0

New API: fr.inria.gforge.spoon:spoon-core:jar:5.7.0-SNAPSHOT

Detected changes: 9.

Change 1

Name	Element
Old	class spoon.reflect.visitor.filter.CatchVariableReferenceFunction
New	class spoon.reflect.visitor.filter.CatchVariableReferenceFunction
Code	java.class.nonFinalClassInheritsFromNewClass
Description	Non-final class now inherits from 'spoon.reflect.visitor.filter.LocalVariableReferenceFunction'.
Breaking	binary: potentially_breaking, source: potentially_breaking

Change 1

Name	Element
Old	class spoon.reflect.visitor.filter.CatchVariableReferenceFunction
New	class spoon.reflect.visitor.filter.CatchVariableReferenceFunction
Code	java.class.noLongerImplementsInterface
Description	Class no longer implements interface 'spoon.reflect.visitor.chain.CtConsumableFunction<spoon.reflect.code.CtCatchVariable<?>>'.
Breaking	binary: breaking, source: breaking

Change 2

Name	Element
Old	class spoon.reflect.visitor.filter.FieldReferenceFunction
New	class spoon.reflect.visitor.filter.FieldReferenceFunction
Code	java.class.noLongerImplementsInterface
Description	Class no longer implements interface 'spoon.reflect.visitor.chain.CtConsumableFunction<spoon.reflect.declaration.CtField<?>>'.
Breaking	binary: breaking, source: breaking

Change 2

Name	Element
Old	class spoon.reflect.visitor.filter.FieldReferenceFunction
New	class spoon.reflect.visitor.filter.FieldReferenceFunction
Code	java.class.nowImplementsInterface
Description	Class now implements interface 'spoon.reflect.visitor.chain.CtConsumableFunction<spoon.reflect.declaration.CtElement>'.
Breaking	binary: non_breaking, source: non_breaking

Change 2

Name	Element
Old	class spoon.reflect.visitor.filter.FieldReferenceFunction
New	class spoon.reflect.visitor.filter.FieldReferenceFunction
Code	java.class.superTypeTypeParametersChanged
Description	Super type's type parameters changed from 'spoon.reflect.visitor.chain.CtConsumableFunction<spoon.reflect.declaration.CtField<?>>' to 'spoon.reflect.visitor.chain.CtConsumableFunction<spoon.reflect.declaration.CtElement>'.
Breaking	binary: potentially_breaking, source: potentially_breaking

Change 3

Name	Element
Old	class spoon.reflect.visitor.filter.LocalVariableReferenceFunction
New	class spoon.reflect.visitor.filter.LocalVariableReferenceFunction
Code	java.class.noLongerImplementsInterface
Description	Class no longer implements interface 'spoon.reflect.visitor.chain.CtConsumableFunction<spoon.reflect.code.CtLocalVariable<?>>'.
Breaking	binary: breaking, source: breaking

Change 3

Name	Element
Old	class spoon.reflect.visitor.filter.LocalVariableReferenceFunction
New	class spoon.reflect.visitor.filter.LocalVariableReferenceFunction
Code	java.class.nowImplementsInterface
Description	Class now implements interface 'spoon.reflect.visitor.chain.CtConsumableFunction<spoon.reflect.declaration.CtElement>'.
Breaking	binary: non_breaking, source: non_breaking

Change 3

Name	Element
Old	class spoon.reflect.visitor.filter.LocalVariableReferenceFunction
New	class spoon.reflect.visitor.filter.LocalVariableReferenceFunction
Code	java.class.superTypeTypeParametersChanged
Description	Super type's type parameters changed from 'spoon.reflect.visitor.chain.CtConsumableFunction<spoon.reflect.code.CtLocalVariable<?>>' to 'spoon.reflect.visitor.chain.CtConsumableFunction<spoon.reflect.declaration.CtElement>'.
Breaking	binary: potentially_breaking, source: potentially_breaking

Change 4

Name	Element
Old	class spoon.reflect.visitor.filter.ParameterReferenceFunction
New	class spoon.reflect.visitor.filter.ParameterReferenceFunction
Code	java.class.nonFinalClassInheritsFromNewClass
Description	Non-final class now inherits from 'spoon.reflect.visitor.filter.LocalVariableReferenceFunction'.
Breaking	binary: potentially_breaking, source: potentially_breaking

Change 4

Name	Element
Old	class spoon.reflect.visitor.filter.ParameterReferenceFunction
New	class spoon.reflect.visitor.filter.ParameterReferenceFunction
Code	java.class.noLongerImplementsInterface
Description	Class no longer implements interface 'spoon.reflect.visitor.chain.CtConsumableFunction<spoon.reflect.declaration.CtParameter<?>>'.
Breaking	binary: breaking, source: breaking

Change 5

Name	Element
Old	class spoon.reflect.visitor.filter.VariableReferenceFunction
New	class spoon.reflect.visitor.filter.VariableReferenceFunction
Code	java.class.noLongerImplementsInterface
Description	Class no longer implements interface 'spoon.reflect.visitor.chain.CtConsumableFunction<spoon.reflect.declaration.CtVariable<?>>'.
Breaking	binary: breaking, source: breaking

Change 5

Name	Element
Old	class spoon.reflect.visitor.filter.VariableReferenceFunction
New	class spoon.reflect.visitor.filter.VariableReferenceFunction
Code	java.class.nowImplementsInterface
Description	Class now implements interface 'spoon.reflect.visitor.chain.CtConsumableFunction<spoon.reflect.declaration.CtElement>'.
Breaking	binary: non_breaking, source: non_breaking

Change 5

Name	Element
Old	class spoon.reflect.visitor.filter.VariableReferenceFunction
New	class spoon.reflect.visitor.filter.VariableReferenceFunction
Code	java.class.superTypeTypeParametersChanged
Description	Super type's type parameters changed from 'spoon.reflect.visitor.chain.CtConsumableFunction<spoon.reflect.declaration.CtVariable<?>>' to 'spoon.reflect.visitor.chain.CtConsumableFunction<spoon.reflect.declaration.CtElement>'.
Breaking	binary: potentially_breaking, source: potentially_breaking

Change 6

Name	Element
Old	none
New	method java.util.List<java.io.File> spoon.reflect.cu.CompilationUnit::getBinaryFiles()
Code	java.method.addedToInterface
Description	Method was added to an interface.
Breaking	binary: non_breaking, source: breaking, semantic: potentially_breaking

Change 7

Name	Element
Old	parameter void spoon.reflect.visitor.filter.FieldReferenceFunction::apply(===spoon.reflect.declaration.CtField<?>===, spoon.reflect.visitor.chain.CtConsumer<java.lang.Object>)
New	parameter void spoon.reflect.visitor.filter.FieldReferenceFunction::apply(===spoon.reflect.declaration.CtElement===, spoon.reflect.visitor.chain.CtConsumer<java.lang.Object>)
Code	java.method.parameterTypeChanged
Description	The type of the parameter changed from 'spoon.reflect.declaration.CtField<?>' to 'spoon.reflect.declaration.CtElement'.
Breaking	binary: breaking, source: potentially_breaking

Change 8

Name	Element
Old	parameter void spoon.reflect.visitor.filter.LocalVariableReferenceFunction::apply(===spoon.reflect.code.CtLocalVariable<?>===, spoon.reflect.visitor.chain.CtConsumer<java.lang.Object>)
New	parameter void spoon.reflect.visitor.filter.LocalVariableReferenceFunction::apply(===spoon.reflect.declaration.CtElement===, spoon.reflect.visitor.chain.CtConsumer<java.lang.Object>)
Code	java.method.parameterTypeChanged
Description	The type of the parameter changed from 'spoon.reflect.code.CtLocalVariable<?>' to 'spoon.reflect.declaration.CtElement'.
Breaking	binary: breaking, source: potentially_breaking

Change 9

Name	Element
Old	parameter void spoon.reflect.visitor.filter.VariableReferenceFunction::apply(===spoon.reflect.declaration.CtVariable<?>===, spoon.reflect.visitor.chain.CtConsumer<java.lang.Object>)
New	parameter void spoon.reflect.visitor.filter.VariableReferenceFunction::apply(===spoon.reflect.declaration.CtElement===, spoon.reflect.visitor.chain.CtConsumer<java.lang.Object>)
Code	java.method.parameterTypeChanged
Description	The type of the parameter changed from 'spoon.reflect.declaration.CtVariable<?>' to 'spoon.reflect.declaration.CtElement'.
Breaking	binary: breaking, source: potentially_breaking

pvojtechovsky · 2017-03-30T18:54:57Z

I am not sure with these things:

CtType type = ...
CtTypeReference ref = type.getReference();
assertSame(type, ref.getDeclaration());

This contract (code above) is often used for types. And all the existing code, which depends on this contract will fail if the processed class is a generic class and instead of CtTypeReference there is instance of CtTypeParameterRefrence internally.
Let's answer first the question: Q1 "Why anybody needs to put reference to formal type parameter as actual type argument out of the scope of it's visibility?"
A) to build a printable/compilable java model
B) to build a special model, which is efficient for future analyze of the origin spoon model

B is correct. A makes no sense. Because every formal type parameter is valid only in scope of it's declarer and if it is out of that scope then it is invalid.

class Cat<T> {}
Cat<T> cat; //makes no sense because <T> is out of it's scope.

So that operation is not the way how to build a compilable/printable spoon model. It is the way how to create special structures, which are useful for analyzing of spoon model.
If you agree with this answer, then let's answer another question: Q2) Do we want
A) dynamic lookup following java rules (which we already broke, by removing of parameter out of it's scope) ?
B) or we want to be able to put reference to parameter at any arbitrary place we need and to be always able to found the origin declaration of that parameter?

For me B is correct.

How to continue? I do not need to have this issue solved to be able to continue with other PRs. May be it would be helpful, but I can live without that too. I though it is clear and small BS, so I suggested to do it. But I see that we see that concept different so I suggest to postpone this discussion after somebody will really need it for some real use case.

Your current implementation, which breaks first contract above, would be even problem for my code. So I prefer to not have it.

pvojtechovsky · 2017-03-30T19:06:56Z

Your code is kind of compromise, which tries to be working for clients whose answer for Q1 and Q2 is both A and B too. But that compromise stops working on code like this:

class Dog<T>{}
class Cat<T> {
  Dog<T> dog;
}

If we understand that code as printable/compilable spoon model, then it is clear that T of Dog<T> dog points to T of Cat.
But in my use case (an analytic structure needed to adapt parameters to be able to resolve whether method overrides another method) I need to be able to create reference to 'T' which points to Dog

monperrus · 2017-03-31T18:32:45Z

Very interesting discussion. Basically, for type parameters, we have two kinds of declaration. So we need an additional method in CtTypeParameter. Two questions. What should be the name of the new method? I'd suggest getTypeParameterDeclaration (following the convention of getTypeDeclaration and getExecutableDeclaration) what should be the behavior of both? class Dog<X>{} class Cat<T> { Dog<T> dog; } for Dog<T>, I propose that getDeclaration points to T and that getTypeParameterDeclaration points to X. OK for you?

pvojtechovsky · 2017-03-31T19:07:55Z

I did not got it.

So we need an additional method in CtTypeParameter.
do you mean CtTypeParameterReference ?

What will this new method return in this context?

class Cat<T>{
T field;
}

Concerning dynamic lookup... I agree with you that in normal cases the dynamic lookup is correct. But there is question whether we need to switch off dynamic lookup in some situations and be able to directly point to declaration.

monperrus · 2017-03-31T20:45:33Z

I did not got it.

See recent commit

What will this new method return in this context?
class Cat<T>{
T field;
}

T.getDeclaration would return the T of Cat
t.getParameterDeclaration... either null or the same as T.getDeclaration (currently null)

spoon-bot · 2017-03-31T20:47:43Z

Revapi Analysis results

Old API: fr.inria.gforge.spoon:spoon-core:jar:5.7.0-20170329.224511-23

New API: fr.inria.gforge.spoon:spoon-core:jar:5.7.0-SNAPSHOT

Detected changes: 1.

Change 1

Name	Element
Old	none
New	method spoon.reflect.declaration.CtTypeParameter spoon.reflect.reference.CtTypeParameterReference::getTypeParameterDeclaration()
Code	java.method.addedToInterface
Description	Method was added to an interface.
Breaking	binary: non_breaking, source: breaking, semantic: potentially_breaking

pvojtechovsky · 2017-04-01T06:39:08Z

Now I noticed your commit. I understand it now. It is an interesting solution!
But you are solving different problem then me :-) ... there are so many ways how to use and understand spoon.
I will explain in #1243 how I understood adapting of type parameters and then it will be easier to continue discussion here.
Thank you for your time!

monperrus · 2017-04-01T09:37:16Z

I'm just trying to merge this PR which is about "TypeFactory#createReference(type,includingFormalTypeParams)", with an appropriate test.

To me it's OK like this. It's probably not the perfect solution but it's a working one and we can improve it in the future in the context of another PR.

WDYT?

pvojtechovsky · 2017-04-01T11:46:46Z

src/main/java/spoon/reflect/factory/TypeFactory.java

@@ -342,8 +356,7 @@ public CtTypeParameterReference createReference(CtTypeParameter type) {
 			ref.addAnnotation(ctAnnotation.clone());
 		}
 		ref.setSimpleName(type.getSimpleName());
-		//TypeParameter reference without parent is unusable. It lost information about it's declarer
-		ref.setParent(type);
+


This change is problem for my code. Please rollback it.

pvojtechovsky · 2017-04-01T11:49:04Z

Yes this solution is ok, just do not create reference without parent. It breaks basic contract of usage uf type and typeReference.

Others expects that CtTypeParameter#getReference() is directly usable too. See this test added by Simon Urli.

pvojtechovsky · 2017-04-02T16:36:29Z

Thank You. It is OK for me now.

monperrus · 2017-04-02T19:31:37Z

Good, I'll merge tomorrow and close #1238 and #1233.

pvojtechovsky · 2017-04-02T19:44:47Z

OK, thanks!

feature: TypeFactory#createReference(type,includingFormalTypeParams)

49b9c4c

pvojtechovsky added 2 commits March 25, 2017 08:27

test usage of CtTypeParameterReference as actual type argument

fac870c

test testTypeParameterReferenceAsActualTypeArgument

a215b2e

pvojtechovsky mentioned this pull request Mar 25, 2017

CtTypeParameterReference cannot be used as actual type argument #1238

Closed

monperrus changed the title ~~feature: TypeFactory#createReference(type,includingFormalTypeParams)~~ Depends on #1238. feature: TypeFactory#createReference(type,includingFormalTypeParams) Mar 27, 2017

fix

96c68bf

pvojtechovsky commented Apr 1, 2017

View reviewed changes

monperrus force-pushed the createTypeRefWithTypeParams branch 2 times, most recently from 6883a11 to acd4f24 Compare April 2, 2017 09:22

add getTypeParameterDeclaration

93d0aa3

monperrus force-pushed the createTypeRefWithTypeParams branch from acd4f24 to 93d0aa3 Compare April 2, 2017 19:30

monperrus merged commit 5e7ee76 into INRIA:master Apr 3, 2017

pvojtechovsky mentioned this pull request Apr 4, 2017

review1: feature: CtTypeParameter#isSubtypeOf #1218

Merged

pvojtechovsky deleted the createTypeRefWithTypeParams branch May 5, 2017 16:55

surli mentioned this pull request Jun 1, 2017

Release: Spoon-5.7.0 incoming! #1328

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Depends on #1238. feature: TypeFactory#createReference(type,includingFormalTypeParams) #1237

Depends on #1238. feature: TypeFactory#createReference(type,includingFormalTypeParams) #1237

pvojtechovsky commented Mar 23, 2017

monperrus commented Mar 24, 2017

pvojtechovsky commented Mar 25, 2017

monperrus commented Mar 27, 2017

monperrus commented Mar 30, 2017 •

edited

Loading

spoon-bot commented Mar 30, 2017

pvojtechovsky commented Mar 30, 2017

pvojtechovsky commented Mar 30, 2017

monperrus commented Mar 31, 2017 via email

pvojtechovsky commented Mar 31, 2017

monperrus commented Mar 31, 2017

spoon-bot commented Mar 31, 2017

pvojtechovsky commented Apr 1, 2017

monperrus commented Apr 1, 2017

pvojtechovsky Apr 1, 2017

pvojtechovsky commented Apr 1, 2017 •

edited

Loading

pvojtechovsky commented Apr 2, 2017

monperrus commented Apr 2, 2017

pvojtechovsky commented Apr 2, 2017

Depends on #1238. feature: TypeFactory#createReference(type,includingFormalTypeParams) #1237

Depends on #1238. feature: TypeFactory#createReference(type,includingFormalTypeParams) #1237

Conversation

pvojtechovsky commented Mar 23, 2017

monperrus commented Mar 24, 2017

pvojtechovsky commented Mar 25, 2017

monperrus commented Mar 27, 2017

monperrus commented Mar 30, 2017 • edited Loading

spoon-bot commented Mar 30, 2017

Revapi Analysis results

Change 1

Change 1

Change 2

Change 2

Change 2

Change 3

Change 3

Change 3

Change 4

Change 4

Change 5

Change 5

Change 5

Change 6

Change 7

Change 8

Change 9

pvojtechovsky commented Mar 30, 2017

pvojtechovsky commented Mar 30, 2017

monperrus commented Mar 31, 2017 via email

pvojtechovsky commented Mar 31, 2017

monperrus commented Mar 31, 2017

spoon-bot commented Mar 31, 2017

Revapi Analysis results

Change 1

pvojtechovsky commented Apr 1, 2017

monperrus commented Apr 1, 2017

pvojtechovsky Apr 1, 2017

Choose a reason for hiding this comment

pvojtechovsky commented Apr 1, 2017 • edited Loading

pvojtechovsky commented Apr 2, 2017

monperrus commented Apr 2, 2017

pvojtechovsky commented Apr 2, 2017

monperrus commented Mar 30, 2017 •

edited

Loading

pvojtechovsky commented Apr 1, 2017 •

edited

Loading