Quickfix NullPointerException in PartialSerializer

[sergio-otero]

https://www.eclipse.org/forums/index.php/m/1779172

I'm having problems in Xtext 2.13 with quickfixes that i didn't have in 2.12, i suppose related to the changes applied to support multiple quickfix resolutions.

The problem appears when a node A is added to the AST tree and then another node is added as a child of A

java.lang.NullPointerException
    at org.eclipse.xtext.ide.serializer.impl.PartialSerializer$SerializeRecursiveStrategy.serialize(PartialSerializer.java:155)
    at org.eclipse.xtext.ide.serializer.impl.PartialSerializer.serializeChanges(PartialSerializer.java:280)
    at org.eclipse.xtext.ide.serializer.impl.RecordingXtextResourceUpdater.applyChange(RecordingXtextResourceUpdater.java:80)
    at org.eclipse.xtext.ide.serializer.impl.ChangeSerializer.endRecordChanges(ChangeSerializer.java:162)
    at org.eclipse.xtext.ide.serializer.impl.ChangeSerializer.applyModifications(ChangeSerializer.java:93)
    at org.eclipse.xtext.ui.editor.model.edit.BatchModification.applyInWorkspace(BatchModification.java:142)

The problem happens with a single quickfix, but it can be avoided somehow waiting to add the node A until it's fully set. The problem is much more difficult to solve if several quickfixes are applied together and the business rules are complex

As an example, i've made a simple test case

grammar org.xtext.example.mydsl.MyDsl with org.eclipse.xtext.common.Terminals

generate myDsl "http://www.xtext.org/example/mydsl/MyDsl"

Model:
    classes+=Class* statements+=Statement+;

Class:
    'class' name=ID '{' vars+=Variable+ '}';

Variable: name=ID;

Statement: var=[Variable|VariableQN] '=' INT;   

VariableQN hidden(): ID '.' ID;

@Fix(Diagnostic.LINKING_DIAGNOSTIC)
def void fixMissingEntity(Issue issue, IssueResolutionAcceptor acceptor) {
    if (issue.message.contains("Variable")) {
        fixMissingVariable(issue, acceptor);
    }
}

private def fixMissingVariable(Issue issue, IssueResolutionAcceptor acceptor) {
    acceptor.acceptMulti(
        issue,
        "Create missing variable definition",
        "Create missing variable definition",
        "Entity.gif",
        [ Statement element, ICompositeModificationContext<Statement> context |
            val fullReferenceName = getIssueMissingReferenceName(context.issue)
            val nameArray = fullReferenceName.split("\\.")
            val className = nameArray.get(0)
            val fieldName = nameArray.get(1)

            val root = element.eContainer as Model

            context.addModification(root, [ctx |
                var cl = root.classes.findFirst[it.name == className]

                if (cl === null) {
                    // The class didn't exist
                    cl =  MyDslFactory::eINSTANCE.createClass() => [
                        name = className
                    ];

                    // before adding the class, we can add the field
                    addVariable(cl, fieldName)

                    ctx.classes.add(cl)

                    // after adding the class, adding the field will cause NullPointerException in Serialization
                    // addVariable(cl, fieldName)
                } else {
                    // The class existed (maybe created in a previous multiquickfix)
                    // NullPointerException if 2 fixs are applied together and the parent class was added in the first fix
                    addVariable(cl, fieldName)
                }
            ])

        ]
    )
}

protected def boolean addVariable(Class cl, String fieldName) {
    val atr = MyDslFactory::eINSTANCE.createVariable() => [
        name = fieldName
    ];
    cl.vars.add(atr)
}

private def String getIssueMissingReferenceName(Issue issue) {
    // Not the best way ...

    val issueMessage = issue.message
    val j = issueMessage.lastIndexOf("'")
    val i = issueMessage.lastIndexOf("'", j - 1)

    issueMessage.substring(i+1, j)
}

Example code:

// aplying the fix to this 2 problems together works because they create their own classes
Class1.test1 = 1 
Class2.test2 = 1

// aplying the fix to this 2 problems together don't work because the first adds the class plus field and the second adds another field to the new class
Class3.test3 = 1 
Class3.test4 = 1

[cdietrich]

cc @meysholdt @dhuebner

[cdietrich]

code does

if (originalEObjectRegion == null) {
                    strategies.add(new SerializeRecursiveStrategy(originalEObjectRegion, obj, modified));
                    continue;
                }

so that SerializeRecursiveStrategy is initialized with null

[cdietrich]

to reproduce with our tests

    "#22" ChildWithSubChilds
;

ChildWithSubChilds:
    {ChildWithSubChilds}
    children+=ChildWithSubChild*
;
ChildWithSubChild:
    {ChildWithSubChild}"subs"
    subChilds+=SubChild*
;

SubChild: name=ID;

    @Test
    def void testOptionalChildListInsertIntoEmpty2b() {
        recordDiff(ChildWithSubChilds, "#22") [
            children += createChildWithSubChild => [subChilds += createSubChild =>[name="A"]]
            children.head => [subChilds += createSubChild =>[name="A2"]]
        ] === '''
            NOT ADAPED YET
        '''
    }

@meysholdt do you have any hints`?

[cdietrich]

ping @meysholdt

[kthoms]

@meysholdt Moving to 2.15. If you see a chance to fix it for 2.14 then retarget it again. Otherwise please work on it for 2.15.