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

review: feat: support to parse CtType, CtClass and CtField from JDK element CtPath.toString() #4989

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
9 changes: 9 additions & 0 deletions doc/path.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@ path = new CtPathStringBuilder().fromString(".spoon.test.path.testclasses.Foo.fo
List<CtElement> l = path.evaluateOn(root)
```

If the root parameter is omitted, spoon will try to find shadow elements (e.g. jdk classes).

> Querying shadow elements currently only supports types, methods and fields.

```java
path = new CtPathStringBuilder().fromString("#subPackage[name=java]#subPackage[name=util]#containedType[name=HashSet]");
List<CtElement> l = path.evaluateOn()
```

### Creating AST paths

#### From an existing element
Expand Down
12 changes: 12 additions & 0 deletions src/main/java/spoon/reflect/declaration/CtClass.java
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,18 @@ public interface CtClass<T> extends CtType<T>, CtStatement, CtSealable {
@PropertyGetter(role = CONSTRUCTOR)
CtConstructor<T> getConstructor(CtTypeReference<?>... parameterTypes);

/**
* Returns the constructor of the class that takes the given signature.
* e.g. java.util.HashSet has constructor with no parameters `()`
* e.g. java.util.HashSet has constructor with a int and a float parameters `(int,float)`
* e.g. java.util.HashSet has constructor with a java.util.Collection parameter `(java.util.Collection)`
*
* Derived from {@link #getTypeMembers()}
*/
@DerivedProperty
@PropertyGetter(role = CONSTRUCTOR)
CtConstructor<T> getConstructorBySignature(String signature);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite see how this relates to the title and scope of this PR. This should be done in a separate PR IMHO.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a CtConstructor has a CtPath ends with #constructor[signature=(int)], I need this function to get the CtElement.


/**
* Returns the constructors of this class. This includes the default
* constructor if this class has no constructors explicitly declared.
Expand Down
8 changes: 8 additions & 0 deletions src/main/java/spoon/reflect/declaration/CtType.java
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,14 @@ public interface CtType<T> extends CtNamedElement, CtTypeInformation, CtTypeMemb
@PropertyGetter(role = METHOD)
<R> CtMethod<R> getMethod(String name, CtTypeReference<?>... parameterTypes);

/**
* Gets a method from its signature.
*
* @return null if does not exit
*/
@PropertyGetter(role = METHOD)
<R> CtMethod<R> getMethodBySignature(String signature);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite see how this relates to the title and scope of this PR. This should be done in a separate PR IMHO.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same to constructor, a CtMethod has a CtPath ends with #method[signature=(...)], I need this function to get the CtElement.


/**
* Returns the methods that are directly declared by this class or
* interface.
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/spoon/reflect/path/CtPath.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ public interface CtPath {
*/
<T extends CtElement> List<T> evaluateOn(CtElement... startNode);

/**
* Search for elements matching this CtPatch from shadow model.
*/
CtElement evaluateOnShadowModel();

/**
*
* Returns the path that is relative to the given element (subpath from it to the end of the path).
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/spoon/reflect/path/CtPathStringBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
import spoon.reflect.path.impl.CtNamedPathElement;
import spoon.reflect.path.impl.CtPathElement;
import spoon.reflect.path.impl.CtPathImpl;
import spoon.reflect.path.impl.CtTypedNameElement;
import spoon.reflect.path.impl.CtRolePathElement;
import spoon.reflect.path.impl.CtTypedNameElement;

import java.util.ArrayDeque;
import java.util.Deque;
Expand Down Expand Up @@ -145,7 +145,8 @@ private String parseArgumentValue(Tokenizer tokenizer, String argName, CtPathEle
}
} else if ("]".equals(token) || ";".equals(token)) {
//finished reading of argument value
pathElement.addArgument(argName, argValue.toString());
//[fix bug]:AbstractPathElement.getArguments([constructor with no parameters])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what this is doing here, I will need to have a look at it later. This also feels like it should be a different PR and have a test case :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it's a small bug in method AbstractPathElement.getArguments(), when a default constructor in, it produces redundant ).

pathElement.addArgument(argName, argValue.toString().replace("())", "()"));
return token;
}
argValue.append(token);
Expand Down
68 changes: 68 additions & 0 deletions src/main/java/spoon/reflect/path/impl/CtPathImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,12 @@
*/
package spoon.reflect.path.impl;

import spoon.reflect.declaration.CtClass;
import spoon.reflect.declaration.CtElement;
import spoon.reflect.declaration.CtType;
import spoon.reflect.factory.TypeFactory;
import spoon.reflect.path.CtPath;
import spoon.reflect.path.CtRole;

import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -36,6 +40,70 @@ public <T extends CtElement> List<T> evaluateOn(CtElement... startNode) {
return (List<T>) filtered;
}

@Override
public CtElement evaluateOnShadowModel() {
List<String> classRoleNameList = new LinkedList<>();
CtType<?> ctType = null;
for (CtPathElement element : elements) {
if (element instanceof CtRolePathElement) { // search by CtRolePathElement
Collection<String> values = ((CtRolePathElement) element).getArguments().values();
String val = null;
if (values.iterator().hasNext()) {
val = values.iterator().next();
}
if (val != null) {
if (CtRole.SUB_PACKAGE.equals(((CtRolePathElement) element).getRole())
|| CtRole.CONTAINED_TYPE.equals(((CtRolePathElement) element).getRole())) {
classRoleNameList.add(val);
}
Class<?> cls = getJdkClass(String.join(".", classRoleNameList));
if (cls != null) {
if (ctType == null) {
ctType = new TypeFactory().get(cls);
} else {
if (CtRole.METHOD.equals(((CtRolePathElement) element).getRole())) {
return ctType.getMethodBySignature(val);
}
if (CtRole.CONSTRUCTOR.equals(((CtRolePathElement) element).getRole())) {
return ((CtClass) ctType).getConstructorBySignature(val);
}
if (CtRole.FIELD.equals(((CtRolePathElement) element).getRole())) {
return ctType.getField(val);
}
}
}
}
}
}
return ctType;
}

private Class<?> getJdkClass(String name) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: This is duplicated in a few places in spoon

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I found similar one: TypeFactory.get(final String qualifiedName).
However, this method doesn't support shadow elements while another reload method TypeFactory.get(Class<?> cl) does.

So the question is that we need a function that transform String to Class<?>, as the method above did,
or add extra logic in TypeFactory.get(final String qualifiedName) to process shadow elements intendly.

name = name.replaceAll("[\\[\\]]", "");
switch (name) {
case "byte":
return byte.class;
case "int":
return int.class;
case "long":
return long.class;
case "float":
return float.class;
case "double":
return double.class;
case "char":
return char.class;
case "boolean":
return boolean.class;
default:
try {
return Class.forName(name);
} catch (ClassNotFoundException e) {
}
}
return null;
}

@Override
public CtPath relativePath(CtElement parent) {
List<CtElement> roots = new ArrayList<>();
Expand Down
14 changes: 14 additions & 0 deletions src/main/java/spoon/support/reflect/declaration/CtClassImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,20 @@ public CtConstructor<T> getConstructor(CtTypeReference<?>... parameterTypes) {
return null;
}

@Override
public CtConstructor<T> getConstructorBySignature(String signature) {
for (CtTypeMember typeMember : getTypeMembers()) {
if (!(typeMember instanceof CtConstructor)) {
continue;
}
CtConstructor<T> c = (CtConstructor<T>) typeMember;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unchecked: unchecked cast


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

if (c.getSignature().replaceAll(c.getDeclaringType().getQualifiedName(), "").equals(signature)) {
return c;
}
}
return null;
}

@Override
public Set<CtConstructor<T>> getConstructors() {
Set<CtConstructor<T>> constructors = new SignatureBasedSortedSet<>();
Expand Down
15 changes: 15 additions & 0 deletions src/main/java/spoon/support/reflect/declaration/CtTypeImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,21 @@ public <R> CtMethod<R> getMethod(String name, CtTypeReference<?>... parameterTyp
return null;
}

@Override
@SuppressWarnings("unchecked")
public <R> CtMethod<R> getMethodBySignature(String signature) {
if (signature == null) {
return null;
}
String name = signature.substring(0, signature.indexOf('('));
for (CtMethod<?> candidate : getMethodsByName(name)) {
if (candidate.getSignature().equals(signature)) {
return (CtMethod<R>) candidate;
}
}
return null;
}

protected boolean hasSameParameters(CtExecutable<?> candidate, CtTypeReference<?>... parameterTypes) {
if (candidate.getParameters().size() != parameterTypes.length) {
return false;
Expand Down
29 changes: 29 additions & 0 deletions src/test/java/spoon/test/path/PathTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import spoon.reflect.declaration.CtType;
import spoon.reflect.declaration.CtTypeMember;
import spoon.reflect.factory.Factory;
import spoon.reflect.factory.TypeFactory;
import spoon.reflect.path.CtElementPathBuilder;
import spoon.reflect.path.CtPath;
import spoon.reflect.path.CtPathBuilder;
Expand All @@ -55,6 +56,7 @@
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
kaml8 marked this conversation as resolved.
Show resolved Hide resolved
import static org.junit.jupiter.api.Assertions.assertNotNull;

/**
* Created by nicolas on 10/06/2015.
Expand Down Expand Up @@ -409,4 +411,31 @@ public void testFieldOfArrayType() {
assertEquals(1, result.size());
assertSame(argType, result.get(0));
}

@Test
public void testGetJdkElementByCtPathString() {
CtPath path;
// test class
path = new CtPathStringBuilder().fromString("#subPackage[name=java]#subPackage[name=util]#containedType[name=HashSet]");
assertNotNull(path.evaluateOnShadowModel());
CtType class_HashSet = new TypeFactory().get(java.util.HashSet.class);
assertEquals(path.evaluateOnShadowModel(), class_HashSet);
// if unable to get the method or the field, it will try to return the class.
// test method
path = new CtPathStringBuilder().fromString("#subPackage[name=java]#subPackage[name=util]#containedType[name=HashSet]#method[signature=contains(java.lang.Object)]");
assertNotNull(path.evaluateOnShadowModel());
assertEquals(path.evaluateOnShadowModel(), class_HashSet.getMethodBySignature("contains(java.lang.Object)"));
// test constructor
path = new CtPathStringBuilder().fromString("#subPackage[name=java]#subPackage[name=util]#containedType[name=HashSet]#constructor[signature=()]");
assertNotNull(path.evaluateOnShadowModel());
assertEquals(path.evaluateOnShadowModel(), ((CtClass) class_HashSet).getConstructorBySignature("()"));

path = new CtPathStringBuilder().fromString("#subPackage[name=java]#subPackage[name=util]#containedType[name=HashSet]#constructor[signature=(int)]");
assertNotNull(path.evaluateOnShadowModel());
assertEquals(path.evaluateOnShadowModel(), ((CtClass) class_HashSet).getConstructorBySignature("(int)"));
// test field
path = new CtPathStringBuilder().fromString("#subPackage[name=java]#subPackage[name=util]#containedType[name=HashSet]#field[name=map]");
assertNotNull(path.evaluateOnShadowModel());
assertEquals(path.evaluateOnShadowModel(), class_HashSet.getField("map"));
}
}