Open miklossy opened 4 years ago
https://github.com/eclipse/xtext/issues/2345 Is also in that area
Steps to reproduce:
Given the following A.xtend
file where the cursor is placed between the o
and the .
:
package foo
class A {
def a() {
switch(foo.bar) {
}
}
}
When pressing backspace to delete the last o
from the foo
Then the exception above occurs.
@cdietrich Can you reproduce based on the description above?
Thanks @miklossy, this allows to reproduce the exception.
move to 2.24
oldNode:
CompositeNode XExpression
CompositeNodeWithSemanticElement XSwitchExpressionImpl
CompositeNode XMemberFeatureCall
CompositeNodeWithSemanticElement XSwitchExpressionImpl
CompositeNode XFeatureCall
CompositeNode
LeafNode fo
LeafNode .
CompositeNode
LeafNode a
newNode:
CompositeNode XExpression
CompositeNode XConditionalExpression
CompositeNodeWithSemanticElement XMemberFeatureCallImplCustom
CompositeNode XMemberFeatureCall
CompositeNodeWithSemanticElement XFeatureCallImplCustom
CompositeNode XFeatureCall
CompositeNode
LeafNode foo
LeafNode .
CompositeNode
LeafNode a
It seems the partial parser does choose the correct AST element to replace and the correct rule to parse. But for some reason (that I don't understand) a CompositeNode is created for the XConditionalExpression in case of a partial parse. Because of this the node trees can not be synchronized. Not yet an idea about how to fix.
It happens also on some of my DSLs (based on Jbase); from what I understand is here:
public class LookAheadPreservingNodeModelBuilder extends NodeModelBuilder {
@Override
public void replaceAndTransferLookAhead(INode oldNode, INode newRootNode) {
Iterator<AbstractNode> oldNodes = ((AbstractNode) oldNode).basicIterator();
Iterator<AbstractNode> newNodes = ((AbstractNode) newRootNode).basicIterator();
newNodes.next(); // basicGetFirstChild to skip that one
while(oldNodes.hasNext()) {
AbstractNode nextOld = oldNodes.next();
AbstractNode nextNew = newNodes.next();
if (nextOld instanceof CompositeNode) {
/* BANG */ setLookAhead((CompositeNode) nextNew, ((CompositeNode) nextOld).getLookAhead());
}
}
if (newNodes.hasNext()) {
throw new RuntimeException();
}
super.replaceAndTransferLookAhead(oldNode, newRootNode);
}
}
because nextOld
is a CompositeNode, but nextNew
is NOT.
Indeed, in In super class NodeModelBuilder we have
public void replaceAndTransferLookAhead(INode oldNode, INode newRootNode) {
AbstractNode newNode = ((CompositeNode) newRootNode).basicGetFirstChild();
replaceWithoutChildren((AbstractNode) oldNode, newNode);
if (oldNode instanceof ICompositeNode && newNode instanceof CompositeNode) {
CompositeNode newCompositeNode = (CompositeNode) newNode;
newCompositeNode.basicSetLookAhead(((ICompositeNode) oldNode).getLookAhead());
}
ICompositeNode root = newNode.getRootNode();
BidiTreeIterator<AbstractNode> iterator = ((AbstractNode) root).basicIterator();
int offset = 0;
while(iterator.hasNext()) {
AbstractNode node = iterator.next();
if (node instanceof LeafNode) {
((LeafNode) node).basicSetTotalOffset(offset);
offset += node.getTotalLength();
}
}
}
so both nodes are tested with istanceof
. @szarnekow is there any reason why this is not done in the redefined method? Is that a bug?
The documentation of the LookAheadPreservingNodeModelBuilder states that it should be used together with the TokenSequencePreservingPartialParsingHelper (see https://github.com/eclipse/xtext-extras/blob/master/org.eclipse.xtext.xbase/src/org/eclipse/xtext/xbase/parser/LookAheadPreservingNodeModelBuilder.java#L19) I assume this is the case for JBase?
@szarnekow yes, because I inherit from DefaultXbaseRuntimeModule which does:
@Override
public Class<? extends IPartialParsingHelper> bindIPartialParserHelper() {
return TokenSequencePreservingPartialParsingHelper.class;
}
I did a quick experiment and inject this (hopefully) "patched" version in my Jbase DSLs:
public class PatchedLookAheadPreservingNodeModelBuilder extends NodeModelBuilder {
@Override
public void replaceAndTransferLookAhead(INode oldNode, INode newRootNode) {
Iterator<AbstractNode> oldNodes = ((AbstractNode) oldNode).basicIterator();
Iterator<AbstractNode> newNodes = ((AbstractNode) newRootNode).basicIterator();
newNodes.next(); // basicGetFirstChild to skip that one
while(oldNodes.hasNext()) {
AbstractNode nextOld = oldNodes.next();
AbstractNode nextNew = newNodes.next();
if (nextOld instanceof CompositeNode && nextNew instanceof CompositeNode) {
setLookAhead((CompositeNode) nextNew, ((CompositeNode) nextOld).getLookAhead());
}
}
super.replaceAndTransferLookAhead(oldNode, newRootNode);
}
}
where I added the additional instanceof check and removed the throw of RuntimeException and it looks like it works perfectly...
It would be interesting to analyse why isSameTokenSequence
returns true but the produced node models do have a different shape.
@szarnekow do you foresee any problem in my possible solution? (to be honest, I don't understand all the details ;)
if it can help, a way to reproduce the problem in my Jbase languages is starting from this valid code in the editor (these languages also accept programs with just expressions, similarly to PureXbase):
System.out.println("");
Modifying the program as follows
System.out.println("h");
shows a dialog with an CCE in XtextReconcilerJob.
@szarnekow here are some analysis using the above example:
in reparse
int originalLength = rightNode.getTotalEndOffset() - leftNode.getTotalOffset();
int expectedLength = originalLength - changedRegion.getLength() + changedRegion.getText().length();
if (!isSameTokenSequence(originalText.substring(0, originalLength), newText, expectedLength)) {
// different token sequence, cannot perform a partial parse run
return fullyReparse(parser, previousParseResult, changedRegion);
}
originalText: ""); newText: "h");
so the two lengths are 3 and 4
getting here:
protected boolean isSameTokenSequence(TokenSource originalSource, TokenSource newSource, int expectedLength) {
Token token = originalSource.nextToken();
int newLength = 0;
while(Token.EOF != token.getType()) {
Token newToken = newSource.nextToken();
if (token.getType() != newToken.getType()) {
return false;
}
newLength += TokenTool.getLength(newToken);
token = originalSource.nextToken();
}
return newLength == expectedLength;
}
the loop exits with the equality of the two lengths... I don't understand the details... what I note that might sound suspicious to me is the the type of the tokens is always the same, but some of them have text
that is null
.
PING! Especially if you plan to release 2.26 soon
cc @szarnekow
Would it be OK to file a PR that mitigates this issue? We could skip all non-composite Nodes as the lookahead can only be set on CompositeNode instances. I encountered this error, but in my case it was a hidden node that caused the CCE.
@cdietrich @szarnekow
am not sure what the correct mitgation would be. lets see what sebastian says
Just hit the following exception while editing an
*.xtend
file:Versions:
Xtext SDK: 2.23.0.v20200608-0122
Xtend IDE: 2.23.0.v20200608-0122