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

Target type of a static field must be a CtTypeAccess #346

Closed
monperrus opened this issue Sep 28, 2015 · 5 comments
Closed

Target type of a static field must be a CtTypeAccess #346

monperrus opened this issue Sep 28, 2015 · 5 comments

Comments

@monperrus
Copy link
Collaborator

see test TargetedExpressionTest::testStaticTargets
in https://github.com/monperrus/spoon/tree/target

@GerardPaligot
Copy link
Contributor

Here, the problem is: What should returned getTarget() on a CtFieldAccess?

According to the documentation of this method declared in the CtTargetedExpression interface, we should returned null but there is one particular case: when we specify this. in front of the field access. It isn't a good practice but it is valid.

We can remove this information when we are in a static context but there is a consequence with this modification: we can't reprint "this" in DefaultJavaPrettyPrinter. For me, this case is the only case where getTarget() shouldn't returned null but ThisAccess and must be specified in the javadoc.

@monperrus your opinion?

@monperrus
Copy link
Collaborator Author

the only thing is that elements.get(0/1/2) should return the same value for sake of consistency.

I'd say null is a good option

@GerardPaligot
Copy link
Contributor

I don't agree with you. Like you got a different target for a field not static (this.field and field). Static fields shouldn't be different.

@monperrus
Copy link
Collaborator Author

The point is that they should all return the same object because they
refer to the same thing.

It's related to the way we refer to classes, so it's also related to #349.

I've looked at the code and this point is a mess, to refer to a class we
have either:
CtFieldAccess on .class
and
CtLiteral containing a TypeReference (rather a hack, used for instanceof
and annotation values)

One option is to introduce a clean CtClassReferenceExpression (similar
to CtExecutableReferenceExpression) and to unify this.

@GerardPaligot
Copy link
Contributor

Here, the target type must be a CtTypeAccess when we use a static field.

@GerardPaligot GerardPaligot changed the title bug for accesses on static fields Target type of a static field must be a CtTypeReference Dec 17, 2015
@GerardPaligot GerardPaligot changed the title Target type of a static field must be a CtTypeReference Target type of a static field must be a CtTypeAccess Jan 4, 2016
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