-
-
Notifications
You must be signed in to change notification settings - Fork 351
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
feat: Adds unique serializable path for each CtElement from Root Element. #1874
Changes from 25 commits
24e6e0b
5163384
d3bd277
b1b806f
64a2758
eda3e7c
afc30dd
e3e66c4
3c69d39
be56d34
1e604a7
14e0c2c
aa826b8
7865217
45d6820
8bc9df3
e2b309c
a7290f4
b2266ae
1aaef21
d846e6f
047f914
b1f2276
786d411
8f82a94
8cfc1ad
67ec78b
7ee9921
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
package spoon.reflect.path; | ||
|
||
import spoon.reflect.declaration.CtElement; | ||
import spoon.reflect.declaration.CtNamedElement; | ||
import spoon.reflect.meta.RoleHandler; | ||
import spoon.reflect.meta.impl.RoleHandlerHelper; | ||
import spoon.reflect.path.impl.CtPathElement; | ||
import spoon.reflect.path.impl.CtPathImpl; | ||
import spoon.reflect.path.impl.CtRolePathElement; | ||
import spoon.reflect.reference.CtReference; | ||
|
||
import java.util.List; | ||
import java.util.Map; | ||
|
||
/** | ||
* This builder allow to create some CtPath from CtElements | ||
* | ||
* Created by nharrand on 21/02/2018. | ||
*/ | ||
public class CtElementPathBuilder { | ||
/** | ||
* Build path to a CtElement el, from one of its parent. | ||
* | ||
* @throws CtPathException is thrown when root is not a parent of el. | ||
* | ||
* @param el : the element to which the CtPath leads to | ||
* @param root : Starting point of the CtPath | ||
* @return CtPath from root to el | ||
*/ | ||
public CtPath fromElement(CtElement el, CtElement root) throws CtPathException { | ||
CtPathImpl path = new CtPathImpl(); | ||
CtElement cur = el; | ||
while (cur != root) { | ||
CtElement parent = cur.getParent(); | ||
CtRole role = cur.getRoleInParent(); | ||
RoleHandler roleHandler = RoleHandlerHelper.getOptionalRoleHandler(parent.getClass(), role); | ||
if (role == null || roleHandler == null) { | ||
throw new CtPathException(); | ||
} | ||
CtPathElement pathElement = new CtRolePathElement(role); | ||
switch (roleHandler.getContainerKind()) { | ||
case SINGLE: | ||
break; | ||
|
||
case LIST: | ||
//Element needs to be differentiated from its brothers | ||
List list = roleHandler.asList(parent); | ||
//Assumes that List's order is deterministic. | ||
//Can't be replaced by list.indexOf(cur) | ||
//Because objects must be the same (and not just equals) | ||
int index = 0; | ||
for (Object o : list) { | ||
if (o == cur) { | ||
break; | ||
} | ||
index++; | ||
} | ||
pathElement.addArgument("index", index + ""); | ||
break; | ||
|
||
case SET: | ||
String name; | ||
if (cur instanceof CtNamedElement) { | ||
name = ((CtNamedElement) cur).getSimpleName(); | ||
} else if (cur instanceof CtReference) { | ||
name = ((CtReference) cur).getSimpleName(); | ||
} else { | ||
throw new CtPathException(); | ||
} | ||
pathElement.addArgument("name", name); | ||
break; | ||
|
||
case MAP: | ||
Map map = roleHandler.asMap(parent); | ||
String key = null; | ||
for (Object o : map.keySet()) { | ||
if (map.get(o) == cur) { | ||
key = (String) o; | ||
break; | ||
} | ||
} | ||
if (key == null) { | ||
throw new CtPathException(); | ||
} else { | ||
pathElement.addArgument("key", key); | ||
} | ||
break; | ||
} | ||
cur = parent; | ||
path.addFirst(pathElement); | ||
} | ||
return path; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,15 +16,17 @@ | |
*/ | ||
package spoon.reflect.path.impl; | ||
|
||
import spoon.reflect.code.CtIf; | ||
import spoon.reflect.declaration.CtElement; | ||
import spoon.reflect.declaration.CtExecutable; | ||
import spoon.reflect.declaration.CtField; | ||
import spoon.reflect.declaration.CtNamedElement; | ||
import spoon.reflect.meta.RoleHandler; | ||
import spoon.reflect.meta.impl.RoleHandlerHelper; | ||
import spoon.reflect.path.CtPathException; | ||
import spoon.reflect.path.CtRole; | ||
import spoon.reflect.visitor.CtInheritanceScanner; | ||
import spoon.reflect.reference.CtReference; | ||
|
||
import java.util.Collection; | ||
import java.util.LinkedList; | ||
import java.util.Set; | ||
|
||
/** | ||
* A CtPathElement that define some roles for matching. | ||
|
@@ -40,60 +42,6 @@ public class CtRolePathElement extends AbstractPathElement<CtElement, CtElement> | |
|
||
public static final String STRING = "#"; | ||
|
||
private class RoleVisitor extends CtInheritanceScanner { | ||
private Collection<CtElement> matchs = new LinkedList<>(); | ||
|
||
private RoleVisitor() { | ||
} | ||
|
||
@Override | ||
public <R> void scanCtExecutable(CtExecutable<R> e) { | ||
super.scanCtExecutable(e); | ||
|
||
switch (role) { | ||
case BODY: | ||
if (e.getBody() != null) { | ||
if (getArguments().containsKey("index") | ||
&& e.getBody() | ||
.getStatements() | ||
.size() > Integer.parseInt( | ||
getArguments().get("index"))) { | ||
matchs.add(e.getBody().getStatements().get(Integer | ||
.parseInt(getArguments().get("index")))); | ||
} else { | ||
matchs.addAll(e.getBody().getStatements()); | ||
} | ||
} | ||
} | ||
|
||
} | ||
|
||
@Override | ||
public <T> void visitCtField(CtField<T> e) { | ||
super.visitCtField(e); | ||
|
||
if (role == CtRole.DEFAULT_EXPRESSION && e.getDefaultExpression() != null) { | ||
matchs.add(e.getDefaultExpression()); | ||
} | ||
} | ||
|
||
@Override | ||
public void visitCtIf(CtIf e) { | ||
super.visitCtIf(e); | ||
|
||
switch (role) { | ||
case THEN: | ||
if (e.getThenStatement() != null) { | ||
matchs.add(e.getThenStatement()); | ||
} | ||
case ELSE: | ||
if (e.getElseStatement() != null) { | ||
matchs.add(e.getElseStatement()); | ||
} | ||
} | ||
} | ||
} | ||
|
||
private final CtRole role; | ||
|
||
public CtRolePathElement(CtRole role) { | ||
|
@@ -106,14 +54,65 @@ public CtRole getRole() { | |
|
||
@Override | ||
public String toString() { | ||
return STRING + role.toString() + getParamString(); | ||
return STRING + getRole().toString() + getParamString(); | ||
} | ||
|
||
public CtElement getFromSet(Set set, String name) throws CtPathException { | ||
for (Object o: set) { | ||
if (o instanceof CtNamedElement) { | ||
if (((CtNamedElement) o).getSimpleName().equals(name)) { | ||
return (CtElement) o; | ||
} | ||
} else if (o instanceof CtReference) { | ||
if (((CtReference) o).getSimpleName().equals(name)) { | ||
return (CtElement) o; | ||
} | ||
} else { | ||
throw new CtPathException(); | ||
} | ||
} | ||
throw new CtPathException(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. throwing of exception as part of normal program flow is not a good idea. It has a performance impact. So it is much faster (main line debug mode), when you return null and check it in caller. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 7ee9921 |
||
} | ||
|
||
@Override | ||
public Collection<CtElement> getElements(Collection<CtElement> roots) { | ||
RoleVisitor visitor = new RoleVisitor(); | ||
visitor.scan(roots); | ||
return visitor.matchs; | ||
Collection<CtElement> matchs = new LinkedList<>(); | ||
for (CtElement root : roots) { | ||
RoleHandler roleHandler = RoleHandlerHelper.getOptionalRoleHandler(root.getClass(), getRole()); | ||
if (roleHandler != null) { | ||
switch (roleHandler.getContainerKind()) { | ||
case SINGLE: | ||
matchs.add(roleHandler.getValue(root)); | ||
break; | ||
|
||
case LIST: | ||
if (getArguments().containsKey("index")) { | ||
int index = Integer.parseInt(getArguments().get("index")); | ||
matchs.add((CtElement) roleHandler.asList(root).get(index)); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. else it should add all items of the into There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and what about case when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, you are right. I only had in mind my Unique path case in mind here, but yes. I'll fix this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, that also means that I should add negative test cases... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We should extends that behavior to sets and maps, don't you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I agree. For Set it should add all items of the Set. And for Map it should add all values of the map. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implemented in 67ec78b |
||
break; | ||
|
||
case SET: | ||
if (getArguments().containsKey("name")) { | ||
String name = getArguments().get("name"); | ||
try { | ||
matchs.add(getFromSet(roleHandler.asSet(root), name)); | ||
} catch (CtPathException e) { | ||
//System.err.println("[ERROR] Element not found for name: " + name); | ||
//No element found for name. | ||
} | ||
} | ||
break; | ||
|
||
case MAP: | ||
if (getArguments().containsKey("key")) { | ||
String name = getArguments().get("key"); | ||
matchs.add((CtElement) roleHandler.asMap(root).get(name)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess it is not correct to add null into matchs, when map doesn't contain a key There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed. I'll fix this too. |
||
} | ||
break; | ||
} | ||
} | ||
} | ||
return matchs; | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need it public? Note: In Spoon we try to make public API as small as possible. It is easier to maintain and refactor then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we don't. I'll change it.