ANN: fix infinite loop when invoking a quick fix on an element with a syntax error

[Kobzol]

This issue produced an interesting situation. In the following code (which has a syntax error in the reference lifetime):

fn foo(a: &u32) {}

fn main() {
    let a = 1;
    foo(&'static a);
}

the foo call is parsed as having two arguments. Therefore the RemoveRedundantFunctionArgumentsFix shows up, which tries to remove the argument a, along with its surrounding whitespace and comments. The code that handles this is RsElement.deleteWithSurroundingCommaAndWhitespace in RsElement.kt:

fun RsElement.deleteWithSurroundingCommaAndWhitespace() {
    while (nextSibling?.isWhitespaceOrComment == true) {
        nextSibling?.delete()
    }
    while (prevSibling?.isWhitespaceOrComment == true) {
        prevSibling?.delete()
    }
    deleteWithSurroundingComma()
}

The problem is that when the whitespace before a in &'static a gets removed, the plugin somehow automatically adds it back again (maybe because it's preceded by an error element?). This caused the above function to end in an infinite loop. To resolve this pathetic case, I introduced a simple infinite loop guard, which will run the removal at most five times (this should be enough to remove successive whitespace/comments in most situations).

The bug was pretty nasty, as it immediately deadlocked the whole IDE.

Fixes: #7830

changelog: Avoid IDE freeze in certain situations when invoking quick fixes or intentions on elements with syntax errors.

[vlad20012]

LGTM as a hotfix, but I think it's better to do either

1. Check for PsiErrorElement there
2. Split collect and removal steps. Collect using PsiElement.rightSiblings and PsiElement.leftSiblings sequences

bors r+

[Kobzol]

Hmm, but if we collect the siblings first, isn't it possible that one of the siblings will be invalidated if we remove another sibling? E.g. I first collect [S1, S2, S3], then remove S3, isn't it possible that it will invalidate S2/S1?

[bors]

Build succeeded:

• check

[vlad20012]

Hmm, but if we collect the siblings first, isn't it possible that one of the siblings will be invalidated if we remove another sibling? E.g. I first collect [S1, S2, S3], then remove S3, isn't it possible that it will invalidate S2/S1?

HmmMMMmm, as far as I know, no, it's impossible. You can even construct an invalid PSI like remove a very essential keyword. The PSI will be invalidated only after a reparse that will be performed after the end of an intention action invocation (or in doPostponedOperationsAndUnblockDocument/forcePsiPostprocessAndRestoreElement)