dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Revert "Support overlapping deletions in quick fixes"

Browse Source 
This reverts commit ff34b81174.

Reason for revert: This causes a failing test when trying to roll the engine into flutter/flutter. See https://github.com/flutter/flutter/issues/92181 for more context.

Original change's description:
> Support overlapping deletions in quick fixes
>
> I believe that the tests cover the added behavior (and some existing
> behavior), but they aren't complete. I'll add more tests over time, but
> for now the test coverage is strictly better than it used to be.
>
> Change-Id: I8dd228cb2b3c477c28e6d20c6da4a549a30d1afb
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/217280
> Reviewed-by: Phil Quitslund <pquitslund@google.com>
> Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
> Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: Ib50418544a2fd30c255ffc118b42c45aded3ae1c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/217760
Reviewed-by: Devon Carew <devoncarew@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Devon Carew <devoncarew@google.com>
This commit is contained in:
Devon Carew
2021-10-22 00:12:46 +00:00
committed by commit-bot@chromium.org
parent cc1cc4fa6e
commit e20ffbb3ce
2 changed files with 65 additions and 223 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/analyzer_plugin/lib/src/protocol/protocol_internal.dart
+2 -32
View File
@@ -4,7 +4,6 @@
import 'dart:collection';
import 'dart:convert' hide JsonDecoder;
import 'dart:math' as math;
import 'package:analyzer_plugin/protocol/protocol.dart';
import 'package:analyzer_plugin/protocol/protocol_common.dart';
@@ -29,17 +28,6 @@ void addAllEditsForSource(
/// If the invariants can't be preserved, then a [ConflictingEditException] is
/// thrown.
void addEditForSource(SourceFileEdit sourceFileEdit, SourceEdit sourceEdit) {
/// If the [leftEdit] and the [rightEdit] can be merged, then merge them.
SourceEdit? _merge(SourceEdit leftEdit, SourceEdit rightEdit) {
assert(leftEdit.offset <= rightEdit.offset);
if (leftEdit.isDeletion && rightEdit.isDeletion) {
var offset = leftEdit.offset;
var end = math.max(leftEdit.end, rightEdit.end);
return SourceEdit(offset, end - offset, '');
}
return null;
}
var edits = sourceFileEdit.edits;
var length = edits.length;
var index = 0;
@@ -51,12 +39,7 @@ void addEditForSource(SourceFileEdit sourceFileEdit, SourceEdit sourceEdit) {
// The [previousEdit] has an offset that is strictly greater than the offset
// of the [sourceEdit] so we only need to look at the end of the
// [sourceEdit] to know whether they overlap.
if (sourceEdit.end > previousEdit.offset) {
var mergedEdit = _merge(sourceEdit, previousEdit);
if (mergedEdit != null) {
edits[index - 1] = mergedEdit;
return;
}
if (sourceEdit.offset + sourceEdit.length > previousEdit.offset) {
throw ConflictingEditException(
newEdit: sourceEdit, existingEdit: previousEdit);
}
@@ -71,12 +54,7 @@ void addEditForSource(SourceFileEdit sourceFileEdit, SourceEdit sourceEdit) {
if ((sourceEdit.offset == nextEdit.offset &&
sourceEdit.length > 0 &&
nextEdit.length > 0) ||
nextEdit.end > sourceEdit.offset) {
var mergedEdit = _merge(nextEdit, sourceEdit);
if (mergedEdit != null) {
edits[index] = mergedEdit;
return;
}
nextEdit.offset + nextEdit.length > sourceEdit.offset) {
throw ConflictingEditException(
newEdit: sourceEdit, existingEdit: nextEdit);
}
@@ -490,11 +468,3 @@ abstract class ResponseResult implements HasToJson {
/// the given [id], where the request was received at the given [requestTime].
Response toResponse(String id, int requestTime);
}
extension SourceEditExtensions on SourceEdit {
/// Return `true` if this source edit represents a deletion.
bool get isDeletion => replacement.isEmpty;
/// Return `true` if this source edit represents an insertion.
bool get isInsertion => length == 0;
}
pkg/analyzer_plugin/test/src/utilities/change_builder/change_builder_core_test.dart
+63 -191
View File
@@ -17,7 +17,6 @@ void main() {
defineReflectiveSuite(() {
defineReflectiveTests(ChangeBuilderImplTest);
defineReflectiveTests(EditBuilderImplTest);
defineReflectiveTests(FileEditBuilderImpl_ConflictingTest);
defineReflectiveTests(FileEditBuilderImplTest);
defineReflectiveTests(LinkedEditBuilderImplTest);
});
@@ -298,33 +297,27 @@ class EditBuilderImplTest extends AbstractChangeBuilderTest {
}
}
/// Tests that are specifically targeted at the handling of conflicting edits.
@reflectiveTest
class FileEditBuilderImpl_ConflictingTest extends AbstractChangeBuilderTest {
class FileEditBuilderImplTest extends AbstractChangeBuilderTest {
String path = '/test.dart';
Matcher get hasConflict => throwsA(isA<ConflictingEditException>());
Future<void> test_deletion_deletion_adjacent_left() async {
var firstOffset = 30;
var firstLength = 5;
var secondOffset = 23;
var secondLength = 7;
Future<void> test_addDeletion() async {
var offset = 23;
var length = 7;
await builder.addGenericFileEdit(path, (builder) {
builder.addDeletion(SourceRange(firstOffset, firstLength));
builder.addDeletion(SourceRange(secondOffset, secondLength));
builder.addDeletion(SourceRange(offset, length));
});
var edits = builder.sourceChange.edits[0].edits;
expect(edits, hasLength(2));
expect(edits[0].offset, firstOffset);
expect(edits[0].length, firstLength);
expect(edits, hasLength(1));
expect(edits[0].offset, offset);
expect(edits[0].length, length);
expect(edits[0].replacement, isEmpty);
expect(edits[1].offset, secondOffset);
expect(edits[1].length, secondLength);
expect(edits[1].replacement, isEmpty);
}
Future<void> test_deletion_deletion_adjacent_right() async {
Future<void> test_addDeletion_adjacent_lowerOffsetFirst() async {
// TODO(brianwilkerson) This should also merge the deletions, but is written
// to ensure that existing uses of FileEditBuilder continue to work even
// without that change.
var firstOffset = 23;
var firstLength = 7;
var secondOffset = 30;
@@ -343,23 +336,31 @@ class FileEditBuilderImpl_ConflictingTest extends AbstractChangeBuilderTest {
expect(edits[1].replacement, isEmpty);
}
Future<void> test_deletion_deletion_overlap_left() async {
var firstOffset = 27;
var firstLength = 8;
var secondOffset = 23;
var secondLength = 7;
Future<void> test_addDeletion_adjacent_lowerOffsetSecond() async {
// TODO(brianwilkerson) This should also merge the deletions, but is written
// to ensure that existing uses of FileEditBuilder continue to work even
// without that change.
var firstOffset = 23;
var firstLength = 7;
var secondOffset = 30;
var secondLength = 5;
await builder.addGenericFileEdit(path, (builder) {
builder.addDeletion(SourceRange(firstOffset, firstLength));
builder.addDeletion(SourceRange(secondOffset, secondLength));
builder.addDeletion(SourceRange(firstOffset, firstLength));
});
var edits = builder.sourceChange.edits[0].edits;
expect(edits, hasLength(1));
expect(edits, hasLength(2));
expect(edits[0].offset, secondOffset);
expect(edits[0].length, firstOffset + firstLength - secondOffset);
expect(edits[0].length, secondLength);
expect(edits[0].replacement, isEmpty);
expect(edits[1].offset, firstOffset);
expect(edits[1].length, firstLength);
expect(edits[1].replacement, isEmpty);
}
Future<void> test_deletion_deletion_overlap_right() async {
@failingTest
Future<void> test_addDeletion_overlapping() async {
// This support is not yet implemented.
var firstOffset = 23;
var firstLength = 7;
var secondOffset = 27;
@@ -375,169 +376,6 @@ class FileEditBuilderImpl_ConflictingTest extends AbstractChangeBuilderTest {
expect(edits[0].replacement, isEmpty);
}
Future<void> test_deletion_insertion_adjacent_left() async {
var deletionOffset = 23;
var deletionLength = 7;
var insertionOffset = 23;
var insertionText = 'x';
await builder.addGenericFileEdit(path, (builder) {
builder.addDeletion(SourceRange(deletionOffset, deletionLength));
expect(() {
builder.addSimpleInsertion(insertionOffset, insertionText);
}, hasConflict);
});
var edits = builder.sourceChange.edits[0].edits;
expect(edits, hasLength(1));
expect(edits[0].offset, deletionOffset);
expect(edits[0].length, deletionLength);
expect(edits[0].replacement, '');
}
Future<void> test_deletion_insertion_adjacent_right() async {
var deletionOffset = 23;
var deletionLength = 7;
var insertionOffset = 30;
var insertionText = 'x';
await builder.addGenericFileEdit(path, (builder) {
builder.addDeletion(SourceRange(deletionOffset, deletionLength));
builder.addSimpleInsertion(insertionOffset, insertionText);
});
var edits = builder.sourceChange.edits[0].edits;
expect(edits, hasLength(2));
expect(edits[0].offset, insertionOffset);
expect(edits[0].length, 0);
expect(edits[0].replacement, insertionText);
expect(edits[1].offset, deletionOffset);
expect(edits[1].length, deletionLength);
expect(edits[1].replacement, isEmpty);
}
Future<void> test_deletion_insertion_overlap() async {
var deletionOffset = 23;
var deletionLength = 7;
var insertionOffset = 26;
var insertionText = 'x';
await builder.addGenericFileEdit(path, (builder) {
builder.addDeletion(SourceRange(deletionOffset, deletionLength));
expect(() {
builder.addSimpleInsertion(insertionOffset, insertionText);
}, hasConflict);
});
var edits = builder.sourceChange.edits[0].edits;
expect(edits, hasLength(1));
expect(edits[0].offset, deletionOffset);
expect(edits[0].length, deletionLength);
expect(edits[0].replacement, '');
}
Future<void> test_insertion_deletion_adjacent_left() async {
var deletionOffset = 23;
var deletionLength = 7;
var insertionOffset = 23;
var insertionText = 'x';
await builder.addGenericFileEdit(path, (builder) {
builder.addSimpleInsertion(insertionOffset, insertionText);
builder.addDeletion(SourceRange(deletionOffset, deletionLength));
});
var edits = builder.sourceChange.edits[0].edits;
expect(edits, hasLength(2));
expect(edits[0].offset, deletionOffset);
expect(edits[0].length, deletionLength);
expect(edits[0].replacement, isEmpty);
expect(edits[1].offset, insertionOffset);
expect(edits[1].length, 0);
expect(edits[1].replacement, insertionText);
}
Future<void> test_insertion_deletion_adjacent_right() async {
var deletionOffset = 23;
var deletionLength = 7;
var insertionOffset = 30;
var insertionText = 'x';
await builder.addGenericFileEdit(path, (builder) {
builder.addSimpleInsertion(insertionOffset, insertionText);
builder.addDeletion(SourceRange(deletionOffset, deletionLength));
});
var edits = builder.sourceChange.edits[0].edits;
expect(edits, hasLength(2));
expect(edits[0].offset, insertionOffset);
expect(edits[0].length, 0);
expect(edits[0].replacement, insertionText);
expect(edits[1].offset, deletionOffset);
expect(edits[1].length, deletionLength);
expect(edits[1].replacement, isEmpty);
}
Future<void> test_insertion_deletion_overlap() async {
var deletionOffset = 23;
var deletionLength = 7;
var insertionOffset = 26;
var insertionText = 'x';
await builder.addGenericFileEdit(path, (builder) {
builder.addSimpleInsertion(insertionOffset, insertionText);
expect(() {
builder.addDeletion(SourceRange(deletionOffset, deletionLength));
}, hasConflict);
});
var edits = builder.sourceChange.edits[0].edits;
expect(edits, hasLength(1));
expect(edits[0].offset, insertionOffset);
expect(edits[0].length, 0);
expect(edits[0].replacement, insertionText);
}
Future<void> test_replacement_replacement_overlap_left() async {
var offset = 23;
var length = 7;
var text = 'x';
await builder.addGenericFileEdit(path, (builder) {
builder.addSimpleReplacement(SourceRange(offset, length), text);
expect(() {
builder.addSimpleReplacement(SourceRange(offset - 2, length), text);
}, hasConflict);
});
var edits = builder.sourceChange.edits[0].edits;
expect(edits, hasLength(1));
expect(edits[0].offset, offset);
expect(edits[0].length, length);
expect(edits[0].replacement, text);
}
Future<void> test_replacement_replacement_overlap_right() async {
var offset = 23;
var length = 7;
var text = 'x';
await builder.addGenericFileEdit(path, (builder) {
builder.addSimpleReplacement(SourceRange(offset, length), text);
expect(() {
builder.addSimpleReplacement(SourceRange(offset + 2, length), text);
}, hasConflict);
});
var edits = builder.sourceChange.edits[0].edits;
expect(edits, hasLength(1));
expect(edits[0].offset, offset);
expect(edits[0].length, length);
expect(edits[0].replacement, text);
}
}
@reflectiveTest
class FileEditBuilderImplTest extends AbstractChangeBuilderTest {
String path = '/test.dart';
Future<void> test_addDeletion() async {
var offset = 23;
var length = 7;
await builder.addGenericFileEdit(path, (builder) {
builder.addDeletion(SourceRange(offset, length));
});
var edits = builder.sourceChange.edits[0].edits;
expect(edits, hasLength(1));
expect(edits[0].offset, offset);
expect(edits[0].length, length);
expect(edits[0].replacement, isEmpty);
}
Future<void> test_addInsertion() async {
await builder.addGenericFileEdit(path, (builder) {
builder.addInsertion(10, (builder) {
@@ -634,6 +472,40 @@ class FileEditBuilderImplTest extends AbstractChangeBuilderTest {
expect(edits[1].replacement, text);
}
Future<void> test_addSimpleReplacement_overlapsHead() async {
var offset = 23;
var length = 7;
var text = 'xyz';
await builder.addGenericFileEdit(path, (builder) {
builder.addSimpleReplacement(SourceRange(offset, length), text);
expect(() {
builder.addSimpleReplacement(SourceRange(offset - 2, length), text);
}, throwsA(isA<ConflictingEditException>()));
});
var edits = builder.sourceChange.edits[0].edits;
expect(edits, hasLength(1));
expect(edits[0].offset, offset);
expect(edits[0].length, length);
expect(edits[0].replacement, text);
}
Future<void> test_addSimpleReplacement_overlapsTail() async {
var offset = 23;
var length = 7;
var text = 'xyz';
await builder.addGenericFileEdit(path, (builder) {
builder.addSimpleReplacement(SourceRange(offset, length), text);
expect(() {
builder.addSimpleReplacement(SourceRange(offset + 2, length), text);
}, throwsA(isA<ConflictingEditException>()));
});
var edits = builder.sourceChange.edits[0].edits;
expect(edits, hasLength(1));
expect(edits[0].offset, offset);
expect(edits[0].length, length);
expect(edits[0].replacement, text);
}
Future<void> test_createEditBuilder() async {
await builder.addGenericFileEdit(path, (builder) {
var offset = 4;