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.

Sign up for GitHub

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

refactor: push down the equality checking code in EqualsVisitor #1853

Merged
merged 6 commits into from
Feb 18, 2018
Merged

refactor: push down the equality checking code in EqualsVisitor #1853

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b72b901
refactor: push down the equality checking code in EqualsVisitor
monperrus Feb 16, 2018
63e9565
Commit by Martin Monperrus on 17 February 2018
monperrus Feb 17, 2018
e1b7c3e
Commit by Martin Monperrus on 17 February 2018
monperrus Feb 17, 2018
4aad049
Commit by Martin Monperrus on 17 February 2018
monperrus Feb 17, 2018
7df65fc
Commit by Martin Monperrus on 17 February 2018
monperrus Feb 17, 2018
73a04bc
Commit by Martin Monperrus on 17 February 2018
monperrus Feb 17, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 5 additions & 71 deletions src/main/java/spoon/reflect/visitor/CtAbstractBiScanner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,81 +19,15 @@
import spoon.reflect.declaration.CtElement;
import spoon.reflect.path.CtRole;

import java.util.ArrayDeque;
import java.util.Collection;
import java.util.Deque;
import java.util.Iterator;

/**
* This abstract bi scanner class declares all scan methods useful for CtBiScannerDefault
* Defines the core bi-scan responsibility.
*/
public abstract class CtAbstractBiScanner extends CtAbstractVisitor {
protected Deque<CtElement> stack = new ArrayDeque<>();

protected void enter(CtElement e) {
}

protected void exit(CtElement e) {
}

protected boolean isNotEqual = false;

public boolean biScan(Collection<? extends CtElement> elements, Collection<? extends CtElement> others) {
return biScan(null, elements, others);
}
public boolean biScan(CtRole role, Collection<? extends CtElement> elements, Collection<? extends CtElement> others) {
if (isNotEqual) {
return isNotEqual;
}
if (elements == null) {
if (others != null) {
return fail();
}
return isNotEqual;
} else if (others == null) {
return fail();
}
if ((elements.size()) != (others.size())) {
return fail();
}
for (Iterator<? extends CtElement> firstIt = elements.iterator(), secondIt = others.iterator(); (firstIt.hasNext()) && (secondIt.hasNext());) {
biScan(role, firstIt.next(), secondIt.next());
}
return isNotEqual;
}

public boolean biScan(CtRole role, CtElement element, CtElement other) {
return biScan(element, other);
}
public boolean biScan(CtElement element, CtElement other) {
if (isNotEqual) {
return isNotEqual;
}
if (element == null) {
if (other != null) {
return fail();
}
return isNotEqual;
} else if (other == null) {
return fail();
}
if (element == other) {
return isNotEqual;
}
/** This method is called to compare `element` and `other` when traversing two trees in parallel.*/
public abstract void biScan(CtElement element, CtElement other);

stack.push(other);
try {
element.accept(this);
} catch (java.lang.ClassCastException e) {
return fail();
} finally {
stack.pop();
}
return isNotEqual;
}
/** This method is called to compare `element` and `other` according to the role when traversing two trees in parallel. */
public abstract void biScan(CtRole role, CtElement element, CtElement other);

public boolean fail() {
isNotEqual = true;
return true;
}
}
38 changes: 37 additions & 1 deletion src/main/java/spoon/reflect/visitor/CtBiScannerDefault.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@
package spoon.reflect.visitor;


import spoon.reflect.declaration.CtElement;
import spoon.reflect.path.CtRole;

import java.util.ArrayDeque;
import java.util.Collection;
import java.util.Deque;
import java.util.Iterator;

/**
* This visitor implements a deep-search scan on the model for 2 elements.
*
Expand All @@ -28,7 +36,35 @@
* Is used by EqualsVisitor.
*/
// autogenerated by CtBiScannerGenerator
public abstract class CtBiScannerDefault extends spoon.reflect.visitor.CtAbstractBiScanner {
public class CtBiScannerDefault extends spoon.reflect.visitor.CtAbstractBiScanner {
protected Deque<CtElement> stack = new ArrayDeque<>();

protected void enter(CtElement e) {
}

protected void exit(CtElement e) {
}

public void biScan(CtElement element, CtElement other) {
stack.push(other);
try {
element.accept(this);
} finally {
stack.pop();
}
}

public void biScan(CtRole role, CtElement element, CtElement other) {
biScan(element, other);
}

protected boolean biScan(CtRole role, Collection<? extends CtElement> elements, Collection<? extends CtElement> others) {
for (Iterator<? extends CtElement> firstIt = elements.iterator(), secondIt = others.iterator(); (firstIt.hasNext()) && (secondIt.hasNext());) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess this code does not work for Set since beginning, because you cannot assure that elements in 2 Sets are iterated in consistent pairs. I wonder why it works ... WDYT?

biScan(role, firstIt.next(), secondIt.next());
}
return true;
}

// autogenerated by CtBiScannerGenerator
public <A extends java.lang.annotation.Annotation> void visitCtAnnotation(final spoon.reflect.declaration.CtAnnotation<A> annotation) {
spoon.reflect.declaration.CtAnnotation other = ((spoon.reflect.declaration.CtAnnotation) (this.stack.peek()));
Expand Down
64 changes: 63 additions & 1 deletion src/main/java/spoon/support/visitor/equals/EqualsVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,22 @@


import spoon.reflect.declaration.CtElement;
import spoon.reflect.path.CtRole;
import spoon.reflect.visitor.CtBiScannerDefault;

import java.util.Collection;

/**
* Used to check equality between an element and another one.
*
*/
public class EqualsVisitor extends CtBiScannerDefault {
public static boolean equals(CtElement element, CtElement other) {
return !new EqualsVisitor().biScan(element, other);
EqualsVisitor equalsVisitor = new EqualsVisitor();
equalsVisitor.biScan(element, other);

// double negation is always hard to understand, but this is legacy :-)
return !equalsVisitor.isNotEqual;
}

private final EqualsChecker checker = new EqualsChecker();
Expand All @@ -41,5 +48,60 @@ protected void enter(CtElement e) {
fail();
}
}
protected boolean isNotEqual = false;

@Override
protected boolean biScan(CtRole role, Collection<? extends CtElement> elements, Collection<? extends CtElement> others) {
if (isNotEqual) {
return isNotEqual;
}
if (elements == null) {
if (others != null) {
return fail();
}
return isNotEqual;
} else if (others == null) {
return fail();
}
if ((elements.size()) != (others.size())) {
return fail();
}
super.biScan(role, elements, others);
return isNotEqual;
}

@Override
public void biScan(CtElement element, CtElement other) {
if (isNotEqual) {
return;
}
if (element == null) {
if (other != null) {
fail();
return;
}
return;
} else if (other == null) {
fail();
return;
}
if (element == other) {
return;
}

try {
super.biScan(element, other);
} catch (java.lang.ClassCastException e) {
fail();
}

return;
}

private boolean fail() {
isNotEqual = true;
return true;
}

}

37 changes: 36 additions & 1 deletion src/test/java/spoon/generating/scanner/CtBiScannerTemplate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,15 @@
*/
package spoon.generating.scanner;

import spoon.reflect.declaration.CtElement;
import spoon.reflect.path.CtRole;
import spoon.reflect.visitor.CtAbstractBiScanner;

import java.util.ArrayDeque;
import java.util.Collection;
import java.util.Deque;
import java.util.Iterator;

/**
* This visitor implements a deep-search scan on the model for 2 elements.
*
Expand All @@ -28,5 +35,33 @@
*
* Is used by EqualsVisitor.
*/
abstract class CtBiScannerTemplate extends CtAbstractBiScanner {
class CtBiScannerTemplate extends CtAbstractBiScanner {
protected Deque<CtElement> stack = new ArrayDeque<>();

protected void enter(CtElement e) {
}

protected void exit(CtElement e) {
}

public void biScan(CtElement element, CtElement other) {
stack.push(other);
try {
element.accept(this);
} finally {
stack.pop();
}
}

public void biScan(CtRole role, CtElement element, CtElement other) {
biScan(element, other);
}

protected boolean biScan(CtRole role, Collection<? extends CtElement> elements, Collection<? extends CtElement> others) {
for (Iterator<? extends CtElement> firstIt = elements.iterator(), secondIt = others.iterator(); (firstIt.hasNext()) && (secondIt.hasNext());) {
biScan(role, firstIt.next(), secondIt.next());
}
return true;
}

}
4 changes: 2 additions & 2 deletions src/test/java/spoon/test/main/MainTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ public void visitCtTypeParameterReference(CtTypeParameterReference ref) {
private void checkEqualityBetweenOriginalAndClone(CtPackage pack) {
class ActualCounterScanner extends CtBiScannerDefault {
@Override
public boolean biScan(CtElement element, CtElement other) {
public void biScan(CtElement element, CtElement other) {
if (element == null) {
if (other != null) {
Assert.fail("element can't be null if other isn't null.");
Expand All @@ -149,7 +149,7 @@ public boolean biScan(CtElement element, CtElement other) {
assertEquals(element, other);
assertFalse(element == other);
}
return super.biScan(element, other);
super.biScan(element, other);
}
}
final ActualCounterScanner actual = new ActualCounterScanner();
Expand Down