-
-
Notifications
You must be signed in to change notification settings - Fork 355
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
Merged
Merged
Changes from 6 commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
24e6e0b
add unique path to element getter
nharrand 5163384
refactor test unique path
nharrand d3bd277
refactor: remove unecessary exception for CtElement.getPath()
nharrand b1b806f
refactor: remove unecessary exception for CtElement.getPath()
nharrand 64a2758
fix(CtPathStringBuilder): Now supports CtUniqueRolePath
nharrand eda3e7c
doc(MainTest.testElementToPathToElementEquivalency): adds contract info
nharrand afc30dd
refactor(MainTest): cosmetic
nharrand e3e66c4
merge
nharrand 3c69d39
doc(CtElementPathBuilder): add missing javadoc
nharrand be56d34
refactor: fix coding style
nharrand 1e604a7
fix: MainTest merging error
nharrand 14e0c2c
Update CtPathStringBuilder.java
monperrus aa826b8
refactor(CtRolePathElement): Replace old behavior by new one from CtU…
nharrand 7865217
refactor(CtRolePathElement): fix coding style
nharrand 45d6820
doc(CtPathStringBuilder): fix javadoc
nharrand 8bc9df3
fix(CtElementImpl.getPath()): unsilence exception.
nharrand e2b309c
Merge branch 'feature-unique-path' of github.com:nharrand/spoon into …
nharrand a7290f4
refactor(CtRolePathElement,CtElementPathBuilder): uses RoleHandlerHelper
nharrand b2266ae
refactor(CtRolePathElement): remove old commented implem
nharrand 1aaef21
Update path.md
monperrus d846e6f
Update CtElement.java
monperrus 047f914
adds super.scan because the diff coverage is strange
monperrus b1f2276
fix(CtElementPathBuilder): List.indexOf rely on equality not identity
nharrand 786d411
refactor(CtElementPathBuilder): fix coding style
nharrand 8f82a94
simplify api of evaluateOn
monperrus 8cfc1ad
fix(CtRolePathElement): the evaluation of a path that leads nowhere r…
nharrand 67ec78b
fix(CtRolePathElement): extends unspecified index behavior to sets an…
nharrand 7ee9921
fix(CtRolePathElement): replace exception throwing with return null w…
nharrand File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
67 changes: 67 additions & 0 deletions
67
src/main/java/spoon/reflect/path/CtElementPathBuilder.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
package spoon.reflect.path; | ||
|
||
import spoon.reflect.declaration.CtElement; | ||
import spoon.reflect.declaration.CtNamedElement; | ||
import spoon.reflect.path.impl.CtPathElement; | ||
import spoon.reflect.path.impl.CtPathImpl; | ||
import spoon.reflect.path.impl.CtUniqueRolePathElement; | ||
import spoon.reflect.reference.CtReference; | ||
|
||
import java.util.Collection; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Set; | ||
|
||
public class CtElementPathBuilder { | ||
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(); | ||
if(parent == null || role == null) throw new CtPathException(); | ||
CtPathElement pathElement = new CtUniqueRolePathElement(role); | ||
if(parent.getValueByRole(role) instanceof List) { | ||
//Element needs to be differentiated from its brothers | ||
List list = parent.getValueByRole(role); | ||
//Assumes that List's order is deterministic. | ||
int index = 0; | ||
for(Object o : list) { | ||
if(o == cur) break; | ||
index++; | ||
} | ||
pathElement.addArgument("index", index + ""); | ||
} else if (parent.getValueByRole(role) instanceof Set){ | ||
if(!(cur instanceof CtNamedElement) && !(cur instanceof CtReference)) throw new CtPathException(); | ||
//Element needs to be differentiated from its brothers | ||
Set set = parent.getValueByRole(role); | ||
String name = null; | ||
for(Object o : set) { | ||
if(o == cur) { | ||
if(cur instanceof CtNamedElement) | ||
name = ((CtNamedElement) cur).getSimpleName(); | ||
else name = ((CtReference) cur).getSimpleName(); | ||
break; | ||
} | ||
} | ||
if(name == null) throw new CtPathException(); | ||
else pathElement.addArgument("name", name); | ||
|
||
} else if (parent.getValueByRole(role) instanceof Map){ | ||
Map map = parent.getValueByRole(role); | ||
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); | ||
} | ||
cur = parent; | ||
path.addFirst(pathElement); | ||
} | ||
return path; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
66 changes: 66 additions & 0 deletions
66
src/main/java/spoon/reflect/path/impl/CtUniqueRolePathElement.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
package spoon.reflect.path.impl; | ||
|
||
import spoon.reflect.declaration.CtElement; | ||
import spoon.reflect.declaration.CtNamedElement; | ||
import spoon.reflect.path.CtPathException; | ||
import spoon.reflect.path.CtRole; | ||
import spoon.reflect.reference.CtReference; | ||
|
||
import java.util.*; | ||
|
||
public class CtUniqueRolePathElement extends CtRolePathElement { | ||
public static final String STRING = "@"; | ||
|
||
public CtUniqueRolePathElement(CtRole role) { | ||
super(role); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
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(); | ||
} | ||
|
||
@Override | ||
public Collection<CtElement> getElements(Collection<CtElement> roots) { | ||
Collection<CtElement> matchs = new LinkedList<>(); | ||
for (CtElement root : roots) { | ||
if(root.getValueByRole(getRole()) instanceof List) { | ||
if(getArguments().containsKey("index")) { | ||
int index = Integer.parseInt(getArguments().get("index")); | ||
matchs.add((CtElement) ((List) root.getValueByRole(getRole())).get(index)); | ||
} | ||
} else if(root.getValueByRole(getRole()) instanceof Set) { | ||
if(getArguments().containsKey("name")) { | ||
String name = getArguments().get("name"); | ||
try { | ||
matchs.add(getFromSet(root.getValueByRole(getRole()), name)); | ||
} catch (Exception e) { | ||
System.err.println("[ERROR] Element not found for name: " + name); | ||
} | ||
} | ||
} else if(root.getValueByRole(getRole()) instanceof Map) { | ||
if(getArguments().containsKey("key")) { | ||
String name = getArguments().get("key"); | ||
matchs.add((CtElement) ((Map) root.getValueByRole(getRole())).get(name)); | ||
} | ||
} else { | ||
CtElement el = root.getValueByRole(getRole()); | ||
matchs.add(el); | ||
} | ||
} | ||
return matchs; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,7 @@ | |
import spoon.reflect.declaration.CtType; | ||
import spoon.reflect.declaration.CtTypeParameter; | ||
import spoon.reflect.declaration.ParentNotInitializedException; | ||
import spoon.reflect.path.CtRole; | ||
import spoon.reflect.path.*; | ||
import spoon.reflect.reference.CtArrayTypeReference; | ||
import spoon.reflect.reference.CtExecutableReference; | ||
import spoon.reflect.reference.CtFieldReference; | ||
|
@@ -40,13 +40,9 @@ | |
import java.io.PrintStream; | ||
import java.io.PrintWriter; | ||
import java.io.StringWriter; | ||
import java.util.ArrayDeque; | ||
import java.util.Deque; | ||
import java.util.HashSet; | ||
import java.util.IdentityHashMap; | ||
import java.util.Map; | ||
import java.util.Set; | ||
import java.util.*; | ||
|
||
import static junit.framework.TestCase.fail; | ||
import static org.junit.Assert.assertEquals; | ||
import static org.junit.Assert.assertFalse; | ||
import static org.junit.Assert.assertNotNull; | ||
|
@@ -439,6 +435,35 @@ public void scan(CtRole role, CtElement element) { | |
}); | ||
} | ||
|
||
@Test | ||
public void testElementToPathToElementEquivalency() { | ||
|
||
List<CtElement> list = new LinkedList<>(); | ||
list.add(rootPackage); | ||
|
||
rootPackage.accept(new CtScanner() { | ||
@Override | ||
public void scan(CtRole role, CtElement element) { | ||
if (element != null) { | ||
CtPath path = element.getPath(); | ||
String pathStr = path.toString(); | ||
try { | ||
CtPath pathRead = new CtPathStringBuilder().fromString(pathStr); | ||
Collection<CtElement> returnedElements = pathRead.evaluateOn(list); | ||
//contract: CtUniqueRolePathElement.evaluateOn() returns a unique elements if provided only a list of one inputs | ||
assertEquals(returnedElements.size(), 1); | ||
CtElement actualElement = (CtElement) returnedElements.toArray()[0]; | ||
//contract: Element -> Path -> String -> Path -> Element leads to the original element | ||
assertEquals(element, actualElement); | ||
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. assertSame? 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, that sounds better. I'll fix it ASAP. |
||
} catch (CtPathException e) { | ||
fail(); | ||
} | ||
} | ||
super.scan(role, element); | ||
} | ||
}); | ||
} | ||
|
||
@Test | ||
public void testTest() throws Exception { | ||
// the tests should be spoonable | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
why not removing the try/catch and let the exception go?
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.
The exception is supposed to be risen from CtElementPathBuilder().fromElement(CtElement el, CtElement from) when from is not a parent of el. So in this case, I think it can't happen and I didn't want to annoy the user with the handling of an exception which is not supposed to happen.
But still, we can let it go.
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.
"let it go" 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.
I have a counter proposal: Why not catch
CtPathException
(which should not happen in that context) and throw aSpoonException
to show that it's an spoon implementation error. Further more it would avoid that the user has to handle it.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.
OK for me.