Template language PSI tree sometimes incorrect wrt outer tokens

OK I'll answer my own question...

I implemented TreePatcher and took care of it.  But I think this is maybe fixing a  bug with IJ's SimpleTreePatcher? 

If you have a patch to SimpleTreePatcher, we can try to run our tests with it and maybe apply it. But what's the problem? Outer elements should be able to occur anywhere and shouldn't lead to troubles. If they do, maybe the place where they cause troubles should be fixed instead?

Yeah, I agree outer elements should be able to exist anywhere, but some internal code doesn't handle it well.  For instance, the following exception happens because the highlighting code assumes the first child of a psi literal is the literal token; it doesn't handle the outer content case:

PsiElement(ManTemplateTokenType.CONTENT_TOKEN)
java.lang.AssertionError: PsiElement(ManTemplateTokenType.CONTENT_TOKEN)
 at com.intellij.codeInsight.daemon.impl.analysis.HighlightUtil.checkLiteralExpressionParsingError(HighlightUtil.java:922)
 at com.intellij.codeInsight.daemon.impl.analysis.HighlightVisitorImpl.visitLiteralExpression(HighlightVisitorImpl.java:805)
 at com.intellij.psi.impl.source.tree.java.PsiLiteralExpressionImpl.accept(PsiLiteralExpressionImpl.java:182)
 at com.intellij.codeInsight.daemon.impl.analysis.HighlightVisitorImpl.visit(HighlightVisitorImpl.java:134)
 at com.intellij.codeInsight.daemon.impl.GeneralHighlightingPass.runVisitors(GeneralHighlightingPass.java:371)
 at com.intellij.codeInsight.daemon.impl.GeneralHighlightingPass.lambda$collectHighlights$5(GeneralHighlightingPass.java:303)
 at com.intellij.codeInsight.daemon.impl.GeneralHighlightingPass.analyzeByVisitors(GeneralHighlightingPass.java:330)
 at com.intellij.codeInsight.daemon.impl.GeneralHighlightingPass.lambda$analyzeByVisitors$6(GeneralHighlightingPass.java:333)
 at com.intellij.codeInsight.daemon.impl.analysis.HighlightVisitorImpl.lambda$analyze$2(HighlightVisitorImpl.java:163)
 at com.intellij.codeInsight.daemon.impl.analysis.RefCountHolder.analyze(RefCountHolder.java:336)
 at com.intellij.codeInsight.daemon.impl.analysis.HighlightVisitorImpl.analyze(HighlightVisitorImpl.java:162)

So, yes, I think it would be better if these places were fixed... if they can all be identified.  I've seen a few, but this is the only one I have handy.

After a bit of testing this is the only stack trace I've run across, so I think the bug is isolated to:

HighlightUtil#checkLiteralExpressionParsingError()

The first two lines are the problem:

PsiElement literal = expression.getFirstChild();
assert literal instanceof PsiJavaToken : literal;

Probably change this to instead find the literal token instead of assuming it is the first child.

Hi Peter.  I found another place where the internal intellij code does not handle embedded outer elements:

PsiBinaryExpressionImpl:

@Override
public ASTNode findChildByRole(int role) {
...
    case ChildRole.LOPERAND:
      return getFirstChildNode();
...
}

This code assumes the first child is the left operand.  When instead this is an outer element, it blows up down the road.

I haven't looked, but I suspect there are other places similar to this in internal intellij code.

Yes, there probably are... I wonder how many, and if it might be indeed better to rewrite the tree patcher.  How do you handle that in yours?

Here is my class:

/**
 * todo: this class exists to work around a bug with IntelliJ's internal code where it does not expect
 * a template content element as a child of Java language elements, maybe delete this class if this bug
 * is ever fixed.
 *
 * @See https://intellij-support.jetbrains.com/hc/en-us/community/posts/360000444624-Template-language-PSI-tree-sometimes-incorrect-wrt-outer-tokens
 */
public class ManTreePatcher extends SimpleTreePatcher
{
  @Override
  public void insert( CompositeElement parent, TreeElement anchorBefore, OuterLanguageElement toInsert )
  {
    if( anchorBefore != null )
    {
      anchorBefore = getElemToInsertBefore( anchorBefore );
    }
    super.insert( parent, anchorBefore, toInsert );
  }

  /**
   * Ensure content element is not inserted as the first child of an expression language
   * element where internal IJ code does not expect it.
   */
  private TreeElement getElemToInsertBefore( TreeElement anchorBefore )
  {
    CompositeElement parent = anchorBefore.getTreeParent();
    
    while( parent != null &&
           parent.rawFirstChild() == anchorBefore &&
           (parent.getElementType() instanceof JavaLiteralExpressionElementType ||
            parent instanceof PsiExpression) )
    {
      anchorBefore = parent;
      parent = parent.getTreeParent();
    }

    return anchorBefore;
  }
}

Had to refine it a bit more.  Grab it online.

Thanks, I'll try to do a similar thing in IDE core. I've filed https://youtrack.jetbrains.com/issue/IDEA-191988 for this.