Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discussion: Manipulation with node attributes by CtRole #1581

Closed
pvojtechovsky opened this issue Oct 8, 2017 · 4 comments
Closed

Discussion: Manipulation with node attributes by CtRole #1581

pvojtechovsky opened this issue Oct 8, 2017 · 4 comments

Comments

@pvojtechovsky
Copy link
Collaborator

I think it would be helpful to have a methods, which can get/set value of an AST node by CtRole.
Some internal spoon algorithms would be simpler then. For example ReplacementVisitor. And I would like to use that in new more powerful SubsitutionVisitor and TemplateMatcher.

For example:

CtType someElement = ...
//this is new feature
Object value = ...getValue(someElement, CtRole.INTERFACE);
//this is just standard call which shows what line above have to do internally.
assertEquals(someElement.getInterfaces(), value)

I would like to have methods:

  • Object getValue(CtElement element, CtRole role)
  • void setValue(CtElement element, CtRole role, Object newValue)
  • CtTypeReference getValueType(Class<? extends CtElement> elementClass, CtRole role)
  • ...?

I would like to implement a generator, which creates code which would provide implementation of these methods. But first I would like to discuss with You all, this concept.

Concept 1

To have a new class/interface CtMetaModel, which would provide these functions

  • Object CtMetaModel#getValue(CtElement element, CtRole role)
  • void CtMetaModel#setValue(CtElement element, CtRole role, Object newValue)
  • CtTypeReference CtMetaModel#getValueType(Class<? extends CtElement> elementClass, CtRole role)

Concept 2

To have an interface CtRoleProvider with methods

  • Object CtRoleProvider#getValue(CtElement element)
  • void CtRoleProvider#setValue(CtElement element, Object newValue)
  • CtTypeReference CtRoleProvider#getValueType(Class<? extends CtElement> elementClass)
  • ...
    and to have one implementation of CtRoleProvider for each CtRole enum value.
public enum CtRole {
	NAME(new RoleProvider4NAME()),
	TYPE(new RoleProvider4TYPE()),
	BODY(new RoleProvider4BODY()),
 ...
}

Concept 3

To have it implemented directly in spoon model. It means

interface CtElement {
Object getDynamicValue(CtRole role);
void setDynamicValue(CtRole role, Object value);
CtTypeReference getDynamicValueType(CtRole role);
}

WDYT?

@pvojtechovsky
Copy link
Collaborator Author

pvojtechovsky commented Oct 8, 2017

Another related idea. The API above would be simpler if each CtRole would have exactly one type of the returned value. But it is actually not like this in these cases:

CtRole.BODY

  • type CtBlock in CtCatchImpl, CtExecutableImpl, CtLambdaImpl, CtSynchronizedImpl, CtTryImpl has type
  • type CtStatement in CtLoopImpl

CtRole.CASE

  • type List<spoon.reflect.code.CtCase> in CtSwitchImpl
  • type CtExpression in CtCaseImpl

CtRole.ELSE

  • type CtExpression in CtConditionalImpl
  • type CtStatement in CtIfImpl

CtRole.EXPRESSION

  • type String in CtCodeSnippetExpressionImpl, CtCodeSnippetStatementImpl
  • List<spoon.reflect.code.CtExpression<?>> in CtNewArrayImpl
Value type Node type
spoon.reflect.code.CtExpression<?> CtForEachImpl
spoon.reflect.code.CtExpression<?> CtSynchronizedImpl
spoon.reflect.code.CtExpression<java.lang.Boolean> CtDoImpl
spoon.reflect.code.CtExpression<java.lang.Boolean> CtForImpl
spoon.reflect.code.CtExpression<java.lang.Boolean> CtWhileImpl
spoon.reflect.code.CtExpression<java.lang.Integer> CtArrayAccessImpl
spoon.reflect.code.CtExpression CtReturnImpl
spoon.reflect.code.CtExpression CtSwitchImpl
spoon.reflect.code.CtExpression CtAssertImpl
spoon.reflect.code.CtExpression CtLambdaImpl
spoon.reflect.code.CtExpression CtUnaryOperatorImpl
spoon.reflect.declaration.CtClass<?> CtNewClassImpl

CtRole.LABEL

  • type String in CtConstructorCallImpl, CtInvocationImpl, CtStatementImpl, CtUnaryOperatorImpl
  • type CtStatement in CtContinueImpl

CtRole.OPERATOR_KIND

  • type BinaryOperatorKind in CtBinaryOperatorImpl, CtOperatorAssignmentImpl
  • type UnaryOperatorKind in CtUnaryOperatorImpl

... and more ... now I see that there is more such case. I thought at beginning that it is only rare case...

@monperrus
Copy link
Collaborator

This is long standing need, thanks for bringing this up. In terms of baby PR, I would propose first only two additional methods (no new interface and class)

  • <T> T CtElement#element#getValue(CtRole role)
  • CtElement#setValue(CtRole role, Object newValue)

@pvojtechovsky
Copy link
Collaborator Author

Thanks for suggestion about CtElement API. Are the names getValue and setValue good? I afraid they will be confusing e.g. getValue/setValue is on CtAnnotation.

I would prefer a longer unique name of methods like

  • getDynamicValue
  • getValueByRole
  • getRoleBasedValue

I would propose first only two additional methods (no new interface and class)

no interface, is the problem, because first step in each such method has to be looking for a code, which handles CtRole on appropriate CtElement instance. I think we should implement this resolving code only once. Therefore I have designed interface CtRoleHandler. I love it! So it will be hard to convince me to delete it ;-)

@monperrus
Copy link
Collaborator

Closing this issue in order to avoid a discussion split in two threads.
So the main discussion is in #1582 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants