Description Frederic Fusier 2008-03-13 04:25:02 EDT

Using build I20080305-1100.

I had the following code:
    if (this.inlineTagStarted) {
        if (previousTag.fragments().size() == 0) {
            // no existing fragment => just add the element
            TagElement inlineTag = this.ast.newTagElement();
            previousTag.fragments().add(inlineTag);
            previousTag = inlineTag;
    } else {
            ASTNode lastFragment = (ASTNode) previousTag.fragments().get(previousTag.fragments().size()-1);
            if (lastFragment.getNodeType() == ASTNode.TAG_ELEMENT) {
                previousTag = (TagElement) lastFragment;
                previousStart = previousTag.getStartPosition();
            }
        }
    }
    previousTag.fragments().add(text);

Then, I want to extract previousTag.fragments() in a local variable to avoid multiple method calls...

The code looks as follow after the extraction:
    List fragments = previousTag.fragments();
    if (this.inlineTagStarted) {
        if (fragments.size() == 0) {
            // no existing fragment => just add the element
            TagElement inlineTag = this.ast.newTagElement();
            fragments.add(inlineTag);
            previousTag = inlineTag;
        } else {
            ASTNode lastFragment = (ASTNode) fragments.get(size-1);
            if (lastFragment.getNodeType() == ASTNode.TAG_ELEMENT) {
                previousTag = (TagElement) lastFragment;
                previousStart = previousTag.getStartPosition();
            }
        }
    }
    fragments.add(text);

Unfortunately, this is not correct. The last line behaves differently after the extraction than before. The local variable will not be changed if 'previousTag' is modified inside the if TAG_ELEMENT block...

Perhaps it's hard to detect this case, but the last line should remain as:
    previousTag.fragments().add(text);

Set as 'major' as if I hadn't carefully look at the new code, I could have missed this point and silently introduced a bug in my code...