Migration: Fix a corner case crash when removing all metadata nodes.

Previously, when removing all metadata nodes from code such as this:

    f({
      @removeMe
      int? i}) {}

If the node(s) being removed occupied entire lines (as they do in this
example), the logic for trying to decide whether to collapse together
empty `{}`, `[]`, or `()` would get confused by the fact that the text
being removed began at the start of the line, even though the parent
AST node (`@removeMe int? i`) didn't begin until partway through the
line; this would cause an assertion failure (or, if assertions were
disabled, an out-of-range error).

As of yet, this has never been a problem in practice, because the only
circumstance in which the migration engine removes metadata is when
adding other metadata (and thus the logic for trying to collapse
together empty `{}`, `[]` or `()` doesn't trigger).  But in a future
CL I'll be adding logic that *can* remove the only metadata and
trigger this problem.  So to prepare for that, this CL fixes the
out-of-range problem.

Change-Id: Ie2d050d656512a7c725752427eaaec1c087f3c82
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206941
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This commit is contained in:
Paul Berry 2021-07-15 11:57:49 +00:00 committed by commit-bot@chromium.org
parent 82f2906764
commit e60a3c5cd8
2 changed files with 18 additions and 2 deletions

View file

@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
import 'dart:convert';
import 'dart:math';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/precedence.dart';
@ -1424,8 +1425,8 @@ class _PassThroughBuilderImpl implements PassThroughBuilder {
if (firstPlanIndex == 0 && lastPlanIndex == sequenceNodes!.length - 1) {
// We're removing everything. Try to remove additional whitespace so
// that we're left with just `()`, `{}`, or `[]`.
var candidateFirstRemovalOffset =
planner._backAcrossWhitespace(firstRemovalOffset, node!.offset);
var candidateFirstRemovalOffset = planner._backAcrossWhitespace(
firstRemovalOffset, min(firstRemovalOffset, node!.offset));
if (planner
._isJustAfter(candidateFirstRemovalOffset, const ['(', '[', '{'])) {
var candidateLastRemovalEnd =

View file

@ -888,6 +888,21 @@ class C {
expect(changes.keys, [entry.offset]);
}
Future<void>
test_remove_metadata_from_defaultFormalParameter_all_full_line() async {
await analyze('''
f({
@deprecated
int? x}) {}''');
var deprecated = findNode.annotation('@deprecated');
checkPlan(
planner!.passThrough(deprecated.parent,
innerPlans: [planner!.removeNode(deprecated)]),
'''
f({
int? x}) {}''');
}
Future<void> test_remove_parameter() async {
await analyze('f(int x, int y, int z) => null;');
var parameter = findNode.simple('y').parent!;