Support multiple occurrences of text in context R=nweiz@google.com Review URL: https://codereview.chromium.org//1041163005
diff --git a/pkgs/source_span/CHANGELOG.md b/pkgs/source_span/CHANGELOG.md index e749513..51b0fea 100644 --- a/pkgs/source_span/CHANGELOG.md +++ b/pkgs/source_span/CHANGELOG.md
@@ -1,3 +1,8 @@ +# 1.1.2 + +* Fixed validation in `SourceSpanWithContext` to allow multiple occurrences of + `text` within `context`. + # 1.1.1 * Fixed `FileSpan`'s context to include the full span text, not just the first
diff --git a/pkgs/source_span/lib/src/span_mixin.dart b/pkgs/source_span/lib/src/span_mixin.dart index a93723f..b4503fa 100644 --- a/pkgs/source_span/lib/src/span_mixin.dart +++ b/pkgs/source_span/lib/src/span_mixin.dart
@@ -65,11 +65,10 @@ var textLine; if (this is SourceSpanWithContext) { var context = (this as SourceSpanWithContext).context; - var textIndex = context.indexOf(text.split('\n').first); - var lineStart = context.lastIndexOf('\n', textIndex); - if (lineStart != -1) { - buffer.write(context.substring(0, lineStart + 1)); - context = context.substring(lineStart + 1); + var lineStart = findLineStart(context, text, column); + if (lineStart != null && lineStart > 0) { + buffer.write(context.substring(0, lineStart)); + context = context.substring(lineStart); } var endIndex = context.indexOf('\n'); textLine = endIndex == -1 ? context : context.substring(0, endIndex + 1);
diff --git a/pkgs/source_span/lib/src/span_with_context.dart b/pkgs/source_span/lib/src/span_with_context.dart index edbb8b6..0012e3f 100644 --- a/pkgs/source_span/lib/src/span_with_context.dart +++ b/pkgs/source_span/lib/src/span_with_context.dart
@@ -6,6 +6,7 @@ import 'location.dart'; import 'span.dart'; +import 'utils.dart'; /// A class that describes a segment of source text with additional context. class SourceSpanWithContext extends SourceSpanBase { @@ -23,14 +24,12 @@ SourceSpanWithContext( SourceLocation start, SourceLocation end, String text, this.context) : super(start, end, text) { - var index = context.indexOf(text); - if (index == -1) { + if (!context.contains(text)) { throw new ArgumentError( 'The context line "$context" must contain "$text".'); } - var beginningOfLine = context.lastIndexOf('\n', index) + 1; - if (start.column != index - beginningOfLine) { + if (findLineStart(context, text, start.column) == null) { throw new ArgumentError('The span text "$text" must start at ' 'column ${start.column + 1} in a line within "$context".'); }
diff --git a/pkgs/source_span/lib/src/utils.dart b/pkgs/source_span/lib/src/utils.dart index 4a8eb55..2d33865 100644 --- a/pkgs/source_span/lib/src/utils.dart +++ b/pkgs/source_span/lib/src/utils.dart
@@ -37,3 +37,20 @@ return max; } +/// Finds a line in [context] containing [text] at the specified [column]. +/// +/// Returns the index in [context] where that line begins, or null if none +/// exists. +int findLineStart(String context, String text, int column) { + var isEmpty = text == ''; + var index = context.indexOf(text); + while (index != -1) { + var lineStart = context.lastIndexOf('\n', index) + 1; + var textColumn = index - lineStart; + if (column == textColumn || (isEmpty && column == textColumn + 1)) { + return lineStart; + } + index = context.indexOf(text, index + 1); + } + return null; +}
diff --git a/pkgs/source_span/pubspec.yaml b/pkgs/source_span/pubspec.yaml index 73c2667..16eee7c 100644 --- a/pkgs/source_span/pubspec.yaml +++ b/pkgs/source_span/pubspec.yaml
@@ -1,5 +1,5 @@ name: source_span -version: 1.1.1 +version: 1.1.2 author: Dart Team <misc@dartlang.org> description: A library for identifying source spans and locations. homepage: http://github.com/dart-lang/source_span
diff --git a/pkgs/source_span/test/span_test.dart b/pkgs/source_span/test/span_test.dart index 39b0b94..2657f5f 100644 --- a/pkgs/source_span/test/span_test.dart +++ b/pkgs/source_span/test/span_test.dart
@@ -65,6 +65,21 @@ new SourceSpanWithContext(start, end, "abc", "\n---abc--"); new SourceSpanWithContext(start, end, "abc", "\n\n---abc--"); }); + + test('text can occur multiple times in context', () { + var start1 = new SourceLocation(4, line: 55, column: 2); + var end1 = new SourceLocation(7, line: 55, column: 5); + var start2 = new SourceLocation(4, line: 55, column: 8); + var end2 = new SourceLocation(7, line: 55, column: 11); + new SourceSpanWithContext(start1, end1, "abc", "--abc---abc--\n"); + new SourceSpanWithContext(start1, end1, "abc", "--abc--abc--\n"); + new SourceSpanWithContext(start2, end2, "abc", "--abc---abc--\n"); + new SourceSpanWithContext(start2, end2, "abc", "---abc--abc--\n"); + expect(() => new SourceSpanWithContext( + start1, end1, "abc", "---abc--abc--\n"), throwsArgumentError); + expect(() => new SourceSpanWithContext( + start2, end2, "abc", "--abc--abc--\n"), throwsArgumentError); + }); }); group('for union()', () { @@ -238,6 +253,18 @@ ${colors.YELLOW}^^^^^^^${colors.NONE}""")); }); + test("underlines correctly when text appears twice", () { + var span = new SourceSpanWithContext( + new SourceLocation(9, column: 9, sourceUrl: "foo.dart"), + new SourceLocation(12, column: 12, sourceUrl: "foo.dart"), + "foo", + "-----foo foo-----"); + expect(span.message("oh no", color: colors.YELLOW), equals(""" +line 1, column 10 of foo.dart: oh no +-----foo ${colors.YELLOW}foo${colors.NONE}----- + ${colors.YELLOW}^^^${colors.NONE}""")); + }); + test("supports lines of preceeding context", () { var span = new SourceSpanWithContext( new SourceLocation(5, line: 3, column: 5, sourceUrl: "foo.dart"),
diff --git a/pkgs/source_span/test/utils_test.dart b/pkgs/source_span/test/utils_test.dart index a998847..74975c3 100644 --- a/pkgs/source_span/test/utils_test.dart +++ b/pkgs/source_span/test/utils_test.dart
@@ -39,6 +39,45 @@ } }); }); + + group('find line start', () { + test('skip entries in wrong column', () { + var context = '0_bb\n1_bbb\n2b____\n3bbb\n'; + var index = findLineStart(context, 'b', 1); + expect(index, 11); + expect(context.substring(index - 1, index + 3), '\n2b_'); + }); + + test('end of line column for empty text', () { + var context = '0123\n56789\nabcdefgh\n'; + var index = findLineStart(context, '', 5); + expect(index, 5); + expect(context[index], '5'); + }); + + test('column at the end of the file for empty text', () { + var context = '0\n2\n45\n'; + var index = findLineStart(context, '', 2); + expect(index, 4); + expect(context[index], '4'); + + context = '0\n2\n45'; + index = findLineStart(context, '', 2); + expect(index, 4); + }); + + test('found on the first line', () { + var context = '0\n2\n45\n'; + var index = findLineStart(context, '0', 0); + expect(index, 0); + }); + + test('not found', () { + var context = '0\n2\n45\n'; + var index = findLineStart(context, '0', 1); + expect(index, isNull); + }); + }); } _linearSearch(list, predicate) {