deleteDeclaration: don't crash on the top node.

A misbehaved client can sometimes cause the server to reach
`deleteDeclaration` with the SourceFile, and it will crash due to no
`node.parent`.  I couldn't find a good way to create a test for it, but
I could trigger it manually by having a file with just a `,`, and
sending an explicit `getCodeFixes` command to the server with
`errorCodes: [6133]`.

Do three things to improve this:

1. `textChanges.ts`: if we get here with the root node, delete it
   instead of failing.

2. `fixUnusedIdentifier.ts`: check that we don't `delete` a node that is
   the whole source file, so the error is more focused (should have more
   similar failure stacks).

3. `session.ts`: when there was any failure in `getCodeFixes`, check if
   the input had a diag code that does not appear in the requested text
   range, and throw an error saying that the failure is probably a
   result of a bad request.

Closes #33726 (probably not fixing it, but making it easier to find the
cause)
This commit is contained in:
Eli Barzilay
2021-02-25 16:57:48 -05:00
parent fb60c9f46e
commit 6ce82ab02e
3 changed files with 31 additions and 7 deletions

View File

@@ -237,8 +237,10 @@ namespace ts.codefix {
if (isParameter(parent)) {
tryDeleteParameter(changes, sourceFile, parent, checker, sourceFiles, program, cancellationToken, isFixAll);
}
else if (!isFixAll || !(isIdentifier(token) && FindAllReferences.Core.isSymbolReferencedInFile(token, checker, sourceFile))) {
changes.delete(sourceFile, isImportClause(parent) ? token : isComputedPropertyName(parent) ? parent.parent : parent);
else if (!(isFixAll && isIdentifier(token) && FindAllReferences.Core.isSymbolReferencedInFile(token, checker, sourceFile))) {
const node = isImportClause(parent) ? token : isComputedPropertyName(parent) ? parent.parent : parent;
Debug.assert(node !== sourceFile, "should not delete whole source file");
changes.delete(sourceFile, node);
}
}