diff --git a/pkg/analyzer_plugin/lib/src/protocol/protocol_internal.dart b/pkg/analyzer_plugin/lib/src/protocol/protocol_internal.dart index 459974c2087..777ccda5a2a 100644 --- a/pkg/analyzer_plugin/lib/src/protocol/protocol_internal.dart +++ b/pkg/analyzer_plugin/lib/src/protocol/protocol_internal.dart @@ -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; -} diff --git a/pkg/analyzer_plugin/test/src/utilities/change_builder/change_builder_core_test.dart b/pkg/analyzer_plugin/test/src/utilities/change_builder/change_builder_core_test.dart index 6ed22f919a9..a571f0460bb 100644 --- a/pkg/analyzer_plugin/test/src/utilities/change_builder/change_builder_core_test.dart +++ b/pkg/analyzer_plugin/test/src/utilities/change_builder/change_builder_core_test.dart @@ -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()); - - Future test_deletion_deletion_adjacent_left() async { - var firstOffset = 30; - var firstLength = 5; - var secondOffset = 23; - var secondLength = 7; + Future 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 test_deletion_deletion_adjacent_right() async { + Future 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 test_deletion_deletion_overlap_left() async { - var firstOffset = 27; - var firstLength = 8; - var secondOffset = 23; - var secondLength = 7; + Future 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 test_deletion_deletion_overlap_right() async { + @failingTest + Future 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 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 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 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 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 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 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 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 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 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 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 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())); + }); + 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 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())); + }); + 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 test_createEditBuilder() async { await builder.addGenericFileEdit(path, (builder) { var offset = 4;