Skip to content

Commit

Permalink
fix: fix position of asserts (and refactor sniper) (#3215)
Browse files Browse the repository at this point in the history
  • Loading branch information
monperrus authored and nharrand committed Jan 27, 2020
1 parent 7a58221 commit 7ff1cdc
Show file tree
Hide file tree
Showing 15 changed files with 231 additions and 78 deletions.
5 changes: 1 addition & 4 deletions src/main/java/spoon/support/compiler/jdt/ContextBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
import org.eclipse.jdt.internal.compiler.ast.TypeReference;
import org.eclipse.jdt.internal.compiler.lookup.FieldBinding;
import org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding;

import spoon.Launcher;
import spoon.compiler.Environment;
import spoon.reflect.code.CtCatchVariable;
import spoon.reflect.code.CtConstructorCall;
Expand Down Expand Up @@ -47,8 +45,8 @@
import java.util.EnumSet;
import java.util.List;

import static spoon.reflect.ModelElementContainerDefaultCapacities.CASTS_CONTAINER_DEFAULT_CAPACITY;
import static java.lang.String.format;
import static spoon.reflect.ModelElementContainerDefaultCapacities.CASTS_CONTAINER_DEFAULT_CAPACITY;

@Internal
public class ContextBuilder {
Expand Down Expand Up @@ -92,7 +90,6 @@ void enter(CtElement e, ASTNode node) {
e.setPosition(this.jdtTreeBuilder.getPositionBuilder().buildPositionCtElement(e, node));
} catch (Exception ex) {
e.setPosition(SourcePosition.NOPOSITION);
Launcher.LOGGER.warn("PositionBuilder failed for element " + e.toString(), ex);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.eclipse.jdt.internal.compiler.ast.AnnotationMethodDeclaration;
import org.eclipse.jdt.internal.compiler.ast.Argument;
import org.eclipse.jdt.internal.compiler.ast.ArrayTypeReference;
import org.eclipse.jdt.internal.compiler.ast.AssertStatement;
import org.eclipse.jdt.internal.compiler.ast.CaseStatement;
import org.eclipse.jdt.internal.compiler.ast.Expression;
import org.eclipse.jdt.internal.compiler.ast.FieldDeclaration;
Expand Down Expand Up @@ -68,9 +69,10 @@ SourcePosition buildPosition(int sourceStart, int sourceEnd) {
return this.jdtTreeBuilder.getFactory().Core().createSourcePosition(cu, sourceStart, sourceEnd, lineSeparatorPositions);
}

/** creates a position for a given element with the information of ASTNode */
SourcePosition buildPositionCtElement(CtElement e, ASTNode node) {
if (e instanceof CtCatch) {
//we cannot compute position of CtCatch, because we do not know position of it's body yet
//we cannot compute position of CtCatch, because we do not know position of its body yet
//it is computed later by #buildPosition(CtCatch)
return SourcePosition.NOPOSITION;
}
Expand Down Expand Up @@ -405,6 +407,9 @@ SourcePosition buildPositionCtElement(CtElement e, ASTNode node) {
if (contents[sourceEnd] != ':') {
return handlePositionProblem("Unexpected character " + contents[sourceEnd] + " instead of \':\' in CtCase on: " + sourceEnd);
}
} else if ((node instanceof AssertStatement)) {
AssertStatement assert_ = (AssertStatement) node;
sourceEnd = findNextChar(contents, contents.length, sourceEnd, ';');
}

if (e instanceof CtModifiable) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import spoon.Launcher;
import spoon.reflect.ModelElementContainerDefaultCapacities;
import spoon.reflect.annotations.MetamodelPropertyField;
import spoon.reflect.code.CtComment;
Expand Down Expand Up @@ -165,9 +164,6 @@ public <A extends Annotation> CtAnnotation<A> getAnnotation(CtTypeReference<A> a
public List<CtAnnotation<? extends Annotation>> getAnnotations() {
if (this instanceof CtShadowable) {
CtShadowable shadowable = (CtShadowable) this;
if (shadowable.isShadow()) {
Launcher.LOGGER.debug("Some annotations might be unreachable from the shadow element: " + this.getShortRepresentation());
}
}
return unmodifiableList(annotations);
}
Expand Down
163 changes: 100 additions & 63 deletions src/main/java/spoon/support/sniper/SniperJavaPrettyPrinter.java
Original file line number Diff line number Diff line change
Expand Up @@ -149,37 +149,47 @@ private String detectLineSeparator(String text) {
* @param printAction the executor of the action, we are listening for. Call it send token to output
*/
void onTokenWriterWrite(TokenType tokenType, String token, CtComment comment, Runnable printAction) {
onPrintEvent(new TokenPrinterEvent(tokenType, token, comment) {
executePrintEvent(new TokenPrinterEvent(tokenType, token, comment) {
@Override
public void print(Boolean muted) {
runInMutedState(muted, printAction);
public void print(boolean muted) {
boolean originMuted = mutableTokenWriter.isMuted();
try {
if (originMuted != muted) {
mutableTokenWriter.setMuted(muted);
}
printAction.run();
} finally {
if (originMuted != muted) {
mutableTokenWriter.setMuted(originMuted);
}
}
}
@Override
public void printSourceFragment(SourceFragment fragment, Boolean isModified) {
boolean isCollectionStarted = false;
if (fragment instanceof CollectionSourceFragment) {
CollectionSourceFragment csf = (CollectionSourceFragment) fragment;
//we started scanning of collection of elements
SourceFragmentContext listContext = csf.isOrdered()
? new SourceFragmentContextList(mutableTokenWriter, null, csf.getItems(), getChangeResolver())
: new SourceFragmentContextSet(mutableTokenWriter, null, csf.getItems(), getChangeResolver());
SourceFragmentContext listContext = getCollectionContext(null, (CollectionSourceFragment) fragment, isModified);
//push the context of this collection
sourceFragmentContextStack.push(listContext);
pushContext(listContext);
isCollectionStarted = true;
}

if (isModified == null || isModified) {
//print origin token
printAction.run();
return;
}
mutableTokenWriter.getPrinterHelper().directPrint(fragment.getSourceCode());
if (isCollectionStarted) {
mutableTokenWriter.setMuted(true);
} else {
mutableTokenWriter.getPrinterHelper().directPrint(fragment.getSourceCode());
}
}
});
}

private void pushContext(SourceFragmentContext listContext) {
listContext.onPush();
sourceFragmentContextStack.push(listContext);
}


private static boolean hasImplicitAncestor(CtElement el) {
Expand Down Expand Up @@ -217,9 +227,9 @@ public String printElement(CtElement element) {
element,
Collections.singletonList(esf),
new ChangeResolver(getChangeCollector(), element)),
() -> onPrintEvent(new ElementPrinterEvent(role, element) {
() -> executePrintEvent(new ElementPrinterEvent(role, element) {
@Override
public void print(Boolean muted) {
public void print(boolean muted) {
superScanInContext(element, SourceFragmentContextPrettyPrint.INSTANCE, muted);
}

Expand All @@ -244,9 +254,9 @@ public void printSourceFragment(SourceFragment fragment, Boolean isModified) {
public SniperJavaPrettyPrinter scan(CtElement element) {
if (element != null) {
CtRole role = getRoleInCompilationUnit(element);
onPrintEvent(new ElementPrinterEvent(role, element) {
executePrintEvent(new ElementPrinterEvent(role, element) {
@Override
public void print(Boolean muted) {
public void print(boolean muted) {
superScanInContext(element, SourceFragmentContextPrettyPrint.INSTANCE, muted);
}
@Override
Expand All @@ -269,15 +279,15 @@ private CtRole getRoleInCompilationUnit(CtElement element) {
/**
* Called whenever {@link DefaultJavaPrettyPrinter} scans/prints an element or writes a token
*/
private void onPrintEvent(PrinterEvent event) {
private void executePrintEvent(PrinterEvent event) {
SourceFragmentContext sfc = detectCurrentContext(event);
if (sfc == null) {
throw new SpoonException("Missing SourceFragmentContext");
}
//there is an context let it handle scanning
if (mutableTokenWriter.isMuted()) {
//it is already muted by an parent. Simply scan and ignore all tokens,
event.print(null);
event.print(true);
return;
}
//let context handle the event
Expand All @@ -295,8 +305,7 @@ private SourceFragmentContext detectCurrentContext(PrinterEvent event) {
while ((sfc = sourceFragmentContextStack.peek()) != null && sfc.matchesPrinterEvent(event) == false) {
//this context handles only subset of roles, which just finished
//leave it and return back to parent context
sourceFragmentContextStack.pop();
sfc.onFinished();
sfc = popSourceFragmentContext();
}
return sfc;
}
Expand All @@ -312,28 +321,26 @@ private void scanInternal(CtRole role, CtElement element, SourceFragment fragmen
if (mutableTokenWriter.isMuted()) {
throw new SpoonException("Unexpected state of sniper pretty printer. TokenWriter is muted.");
}


//it is not muted yet, so this element or any sibling is modified
if (fragment == null) {
throw new SpoonException("Missing source fragment. Call PrintEvent#print instead.");
}
//we have sources of fragment
if (fragment instanceof CollectionSourceFragment) {
CollectionSourceFragment csf = (CollectionSourceFragment) fragment;
//we started scanning of collection of elements
SourceFragmentContext listContext = csf.isOrdered()
? new SourceFragmentContextList(mutableTokenWriter, element, csf.getItems(), getChangeResolver())
: new SourceFragmentContextSet(mutableTokenWriter, element, csf.getItems(), getChangeResolver());
SourceFragmentContext listContext = getCollectionContext(element, (CollectionSourceFragment) fragment, isFragmentModified);
//push the context of this collection
sourceFragmentContextStack.push(listContext);
pushContext(listContext);


//and scan first element of that collection again in new context of that collection
if (Boolean.FALSE.equals(isFragmentModified)) {
// we print the original source code
mutableTokenWriter.getPrinterHelper().directPrint(fragment.getSourceCode());
//and mute the token writer and let DJPP scan it and ignore everything
mutableTokenWriter.setMuted(true);
//TODO check if DJPP needs this call somewhere (because of some state)... may be we can skip this scan completely??
scan(element);
//and keep it muted until SourceFragmentContextList is finished
} else {
// we print it normally
scan(element);
}
} else if (fragment instanceof ElementSourceFragment) {
Expand Down Expand Up @@ -364,6 +371,45 @@ private void scanInternal(CtRole role, CtElement element, SourceFragment fragmen
}
}

private SourceFragmentContext getCollectionContext(CtElement element, CollectionSourceFragment csf, boolean isModified) {
return csf.isOrdered()
? new SourceFragmentContextList(mutableTokenWriter, element, csf.getItems(), getChangeResolver()) {
@Override
public void onPush() {
super.onPush();
if (!isModified) {
mutableTokenWriter.setMuted(true);
}
}

@Override
public void onFinished() {
super.onFinished();
if (!isModified) {
mutableTokenWriter.setMuted(false);
}
}

}
: new SourceFragmentContextSet(mutableTokenWriter, element, csf.getItems(), getChangeResolver()) {
@Override
public void onPush() {
super.onPush();
if (!isModified) {
mutableTokenWriter.setMuted(true);
}
}

@Override
public void onFinished() {
super.onFinished();
if (!isModified) {
mutableTokenWriter.setMuted(false);
}
}
};
}

/**
* Call normal java printing in defined `context`
* @param element to be printed element
Expand All @@ -373,10 +419,19 @@ private void scanInternal(CtRole role, CtElement element, SourceFragment fragmen
* false - not muted
* null - same like before
*/
private void superScanInContext(CtElement element, SourceFragmentContext context, Boolean muted) {
runInContext(context,
() -> runInMutedState(muted,
() -> super.scan(element)));
private void superScanInContext(CtElement element, SourceFragmentContext context, boolean muted) {
boolean originMuted = mutableTokenWriter.isMuted();
try {
if (originMuted != muted) {
mutableTokenWriter.setMuted(muted);
}
runInContext(context,
() -> super.scan(element));
} finally {
if (originMuted != muted) {
mutableTokenWriter.setMuted(originMuted);
}
}
}

/**
Expand All @@ -385,47 +440,29 @@ private void superScanInContext(CtElement element, SourceFragmentContext context
* @param code a to be processed {@link Runnable}
*/
private void runInContext(SourceFragmentContext context, Runnable code) {
sourceFragmentContextStack.push(context);
pushContext(context);
try {
code.run();
} finally {
//remove `context` and all it's child contexts
// we make sure to remve all contexts that have been pushed so far
// and we also remove parameter `context`
// so that we can leave the sourceFragmentContextStack clean
while (true) {
if (sourceFragmentContextStack.isEmpty()) {
throw new SpoonException("Inconsistent sourceFragmentContextStack"); //NOSONAR
}
SourceFragmentContext c = sourceFragmentContextStack.pop();
c.onFinished();
SourceFragmentContext c = popSourceFragmentContext();
if (c == context) {
break;
}
}
}
}
/**
* Run code using {@link MutableTokenWriter} in defined state.
* After this function leaves, the muted status is restored.
* @param muted required muted status
* @param code to be processed {@link Runnable}
*/
private void runInMutedState(Boolean muted, Runnable code) {
boolean originMuted = mutableTokenWriter.isMuted();
if (muted == null) {
muted = originMuted;
}
try {
mutableTokenWriter.setMuted(muted);
code.run();
} finally {
//assure that muted status did not changed in between
if (mutableTokenWriter.isMuted() != muted) {
if (mutableTokenWriter.isMuted()) {
throw new SpoonException("Unexpected state: Token writer is muted after scanning"); //NOSONAR
} else {
throw new SpoonException("Unexpected state: Token writer is not muted after scanning"); //NOSONAR
}
}
mutableTokenWriter.setMuted(originMuted);
}

/** makes the two atomic operations together pop+finish to maintain core contracts */
private SourceFragmentContext popSourceFragmentContext() {
SourceFragmentContext c = sourceFragmentContextStack.pop();
c.onFinished();
return c;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public void onPrintEvent(PrinterEvent event) {
//but may be it is not good idea

//send all inc/dec tab to printer helper to have configured expected indentation
event.print(null);
event.print(false);
return;
}
if (tpe.getType().isWhiteSpace()) {
Expand Down Expand Up @@ -285,4 +285,8 @@ protected void printStandardSpaces() {
}
separatorActions.clear();
}

@Override
public void onPush() {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,16 +77,14 @@ protected String getSuffixSpace() {

@Override
public void onFinished() {
//we are at the end of the list of elements. Printer just tries to print something out of this context.
// we are at the end of the list of elements. Printer just tries to print something out of this context.
if (mutableTokenWriter.isMuted() == false) {
//print list suffix
String suffix = getSuffixSpace();
if (suffix != null) {
//we have origin source code for that list suffix
mutableTokenWriter.getPrinterHelper().directPrint(suffix);
separatorActions.clear();
}
}
mutableTokenWriter.setMuted(false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public interface PrinterEvent {
* false if {@link DefaultJavaPrettyPrinter} will really print into output.
* null if `muted` status should be kept as it is
*/
void print(Boolean muted);
void print(boolean muted);

/**
* We have a source fragment of to be printed element.
Expand Down
Loading

0 comments on commit 7ff1cdc

Please sign in to comment.