Fixes source map bug: getLocationMessage was incorrect on file segments
R=jmesserly@google.com
Review URL: https://codereview.chromium.org//18083030
git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart/pkg/source_maps@24759 260f80e4-7a28-3924-810f-c04153c831b5
diff --git a/lib/span.dart b/lib/span.dart
index fd302bc..e7bf5bf 100644
--- a/lib/span.dart
+++ b/lib/span.dart
@@ -6,7 +6,7 @@
library source_maps.span;
import 'dart:utf' show stringToCodepoints;
-import 'dart:math' show min;
+import 'dart:math' show min, max;
import 'src/utils.dart';
@@ -216,24 +216,18 @@
Location location(int offset) => new FileLocation(this, offset);
/// Gets the 0-based line corresponding to an offset.
- int getLine(int offset) {
- return binarySearch(_lineStarts, (o) => o > offset) - 1;
- }
+ int getLine(int offset) => binarySearch(_lineStarts, (o) => o > offset) - 1;
/// Gets the 0-based column corresponding to an offset.
- int getColumn(int line, int offset) {
- return offset - _lineStarts[line];
- }
+ int getColumn(int line, int offset) => offset - _lineStarts[line];
/// Get the offset for a given line and column
- int getOffset(int line, int column) {
- return _lineStarts[min(line, _lineStarts.length - 1)] + column;
- }
+ int getOffset(int line, int column) =>
+ _lineStarts[max(min(line, _lineStarts.length - 1), 0)] + column;
/// Gets the text at the given offsets.
- String getText(int start, [int end]) {
- return new String.fromCharCodes(_decodedChars.sublist(start, end));
- }
+ String getText(int start, [int end]) =>
+ new String.fromCharCodes(_decodedChars.sublist(max(start, 0), end));
/// Create a pretty string representation from a span.
String getLocationMessage(String message, int start, int end,
@@ -253,16 +247,12 @@
var buf = new StringBuffer(msg);
buf.write('\n');
- var textLine;
// +1 for 0-indexing, +1 again to avoid the last line
- if ((line + 2) < _lineStarts.length) {
- textLine = getText(_lineStarts[line], _lineStarts[line + 1]);
- } else {
- textLine = getText(_lineStarts[line]);
- textLine = '$textLine\n';
- }
+ var textLine = getText(getOffset(line, 0), getOffset(line + 1, 0));
+
+ column = min(column, textLine.length - 1);
int toColumn = min(column + end - start, textLine.length);
if (useColors) {
if (color == null) {
@@ -301,30 +291,52 @@
final int _baseLine;
final int _baseColumn;
+ // TODO(sigmund): consider providing an end-offset to correctly truncate
+ // values passed the end of the segment.
SourceFileSegment(String url, String textSegment, Location startOffset)
: _baseOffset = startOffset.offset,
_baseLine = startOffset.line,
_baseColumn = startOffset.column,
super.text(url, textSegment);
+ /// Craete a span, where [start] is relative to this segment's base offset.
+ /// The returned span stores the real offset on the file, so that error
+ /// messages are reported at the real location.
Span span(int start, [int end, bool isIdentifier = false]) =>
super.span(start + _baseOffset,
end == null ? null : end + _baseOffset, isIdentifier);
+ /// Create a location, where [offset] relative to this segment's base offset.
+ /// The returned span stores the real offset on the file, so that error
+ /// messages are reported at the real location.
Location location(int offset) => super.location(offset + _baseOffset);
+ /// Return the line on the underlying file associated with the [offset] of the
+ /// underlying file. This method operates on the real offsets from the
+ /// original file, so that error messages can be reported accurately.
int getLine(int offset) =>
- super.getLine(offset - _baseOffset) + _baseLine;
+ super.getLine(max(offset - _baseOffset, 0)) + _baseLine;
+ /// Return the column on the underlying file associated with [line] and
+ /// [offset], where [line] is absolute from the beginning of the underlying
+ /// file. This method operates on the real offsets from the original file, so
+ /// that error messages can be reported accurately.
int getColumn(int line, int offset) {
- var col = super.getColumn(line - _baseLine, offset - _baseOffset);
+ var col = super.getColumn(line - _baseLine, max(offset - _baseOffset, 0));
return line == _baseLine ? col + _baseColumn : col;
}
+ /// Return the offset associated with a line and column. This method operates
+ /// on the real offsets from the original file, so that error messages can be
+ /// reported accurately.
int getOffset(int line, int column) =>
super.getOffset(line - _baseLine,
line == _baseLine ? column - _baseColumn : column) + _baseOffset;
+ /// Retrieve the text associated with the specified range. This method
+ /// operates on the real offsets from the original file, so that error
+ /// messages can be reported accurately.
String getText(int start, [int end]) =>
- super.getText(start - _baseOffset, end - _baseOffset);
+ super.getText(start - _baseOffset,
+ end == null ? null : end - _baseOffset);
}
diff --git a/test/span_test.dart b/test/span_test.dart
index 8f61b58..66bd8a8 100644
--- a/test/span_test.dart
+++ b/test/span_test.dart
@@ -165,32 +165,96 @@
}
});
- test('file segment', () {
- var segment = new SourceFileSegment('file',
- TEST_FILE.substring(12), loc(12));
+ test('range check for large offsets', () {
+ var start = TEST_FILE.length;
+ expect(file.getLocationMessage('the message', start, start + 9),
+ 'file:13:1: the message\n');
+ });
+
+ group('file segment', () {
+ var baseOffset = 123;
+ var segmentText = TEST_FILE.substring(baseOffset, TEST_FILE.length - 100);
+ var segment = new SourceFileSegment('file', segmentText, loc(baseOffset));
sline(int n) => segment.getLine(n);
scol(int n) => segment.getColumn(segment.getLine(n), n);
-
line(int n) => file.getLine(n);
col(int n) => file.getColumn(file.getLine(n), n);
- int j = 0;
- int lineOffset = 0;
- for (int i = 12; i < TEST_FILE.length; i++) {
- if (i > lineOffset + newLines[j]) {
- lineOffset += newLines[j] + 1;
- j++;
- }
- expect(segment.location(i - 12).offset, i);
- expect(segment.location(i - 12).line, line(i));
- expect(segment.location(i - 12).column, col(i));
- expect(segment.span(i - 12).start.offset, i);
- expect(segment.span(i - 12).start.line, line(i));
- expect(segment.span(i - 12).start.column, col(i));
+ test('get line and column', () {
+ int j = 0;
+ int lineOffset = 0;
+ for (int i = baseOffset; i < segmentText.length; i++) {
+ if (i > lineOffset + newLines[j]) {
+ lineOffset += newLines[j] + 1;
+ j++;
+ }
+ expect(segment.location(i - baseOffset).offset, i);
+ expect(segment.location(i - baseOffset).line, line(i));
+ expect(segment.location(i - baseOffset).column, col(i));
+ expect(segment.span(i - baseOffset).start.offset, i);
+ expect(segment.span(i - baseOffset).start.line, line(i));
+ expect(segment.span(i - baseOffset).start.column, col(i));
- expect(sline(i), line(i));
- expect(scol(i), col(i));
- }
+ expect(sline(i), line(i));
+ expect(scol(i), col(i));
+ }
+ });
+
+ test('get text', () {
+ var start = 10 + 80 + 31 + 27 + 4 + 2;
+ expect(segment.getText(start, start + 9), file.getText(start, start + 9));
+ });
+
+ group('location message', () {
+ test('first line', () {
+ var start = baseOffset + 7;
+ expect(segment.getLocationMessage('the message', start, start + 2),
+ file.getLocationMessage('the message', start, start + 2));
+ });
+
+ test('in a middle line', () {
+ // Example from another test above:
+ var start = 10 + 80 + 31 + 27 + 4 + 2;
+ expect(segment.getLocationMessage('the message', start, start + 9),
+ file.getLocationMessage('the message', start, start + 9));
+ });
+
+ test('last segment line', () {
+ var start = segmentText.length - 4;
+ expect(segment.getLocationMessage('the message', start, start + 2),
+ file.getLocationMessage('the message', start, start + 2));
+ });
+
+ test('past segment, same as last segment line', () {
+ var start = segmentText.length;
+ expect(segment.getLocationMessage('the message', start, start + 2),
+ file.getLocationMessage('the message', start, start + 2));
+
+ start = segmentText.length + 20;
+ expect(segment.getLocationMessage('the message', start, start + 2),
+ file.getLocationMessage('the message', start, start + 2));
+ });
+
+ test('past segment, past its line', () {
+ var start = TEST_FILE.length - 2;
+ expect(file.getLocationMessage('the message', start, start + 1),
+ 'file:12:29: the message\n'
+ '123456789_1+3456789_123456789\n'
+ ' ^');
+
+ // TODO(sigmund): consider also fixing this and report file:10:4
+ // The answer below is different because the segment parsing only knows
+ // about the 10 lines it has (and nothing about the possible extra lines
+ // afterwards)
+ expect(segment.getLocationMessage('the message', start, start + 1),
+ 'file:10:112: the message\n');
+
+ // The number 112 includes the count of extra characters past the
+ // segment, which we verify here:
+ var lastSegmentLineStart = segmentText.lastIndexOf('\n');
+ expect(start - baseOffset - lastSegmentLineStart, 112);
+ });
+ });
});
test('span isIdentifier defaults to false', () {
