Skip to content

Commit b435a46

Browse files
authored
Fix SingleMapping.spanFor to use previous line (#2205)
Prior to the fix, the new test failed, showing part of the continuation for the region mapped to A was incorrectly reported as unmapped: ``` 00:00 +28 -4: test/continued_region_test.dart: continued span, later line, later column [E] Expected: '----------\n' '----------\n' '----AAAAAA\n' 'AAAAAAAAAA\n' 'AAAAAAAAAA\n' 'AAAAAABBBB\n' 'BBBBBBBBBB\n' 'BBBBBBBBBB\n' '' Actual: '----------\n' '----------\n' '----AAAAAA\n' 'AAAAAAAAAA\n' 'AAAAAAAAAA\n' '------BBBB\n' 'BBBBBBBBBB\n' 'BBBBBBBBBB\n' '' Which: is different. Expected: ... AAAAAAAA\nAAAAAABBBB ... Actual: ... AAAAAAAA\n------BBBB ... ^ Differ at offset 60 ```
1 parent 4c7dae1 commit b435a46

File tree

4 files changed

+150
-23
lines changed

4 files changed

+150
-23
lines changed

pkgs/source_maps/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
## 0.10.14-wip
22

3+
* Fix `SingleMapping.spanFor` to use the entry from a previous line as specified
4+
by the sourcemap specification
5+
(https://tc39.es/ecma426/#sec-GetOriginalPositions),
6+
37
## 0.10.13
48

59
* Require Dart 3.3

pkgs/source_maps/lib/parser.dart

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -500,31 +500,37 @@ class SingleMapping extends Mapping {
500500
StateError('Invalid entry in sourcemap, expected 1, 4, or 5'
501501
' values, but got $seen.\ntargeturl: $targetUrl, line: $line');
502502

503-
/// Returns [TargetLineEntry] which includes the location in the target [line]
504-
/// number. In particular, the resulting entry is the last entry whose line
505-
/// number is lower or equal to [line].
506-
TargetLineEntry? _findLine(int line) {
507-
var index = binarySearch(lines, (e) => e.line > line);
508-
return (index <= 0) ? null : lines[index - 1];
509-
}
510-
511-
/// Returns [TargetEntry] which includes the location denoted by
512-
/// [line], [column]. If [lineEntry] corresponds to [line], then this will be
513-
/// the last entry whose column is lower or equal than [column]. If
514-
/// [lineEntry] corresponds to a line prior to [line], then the result will be
515-
/// the very last entry on that line.
516-
TargetEntry? _findColumn(int line, int column, TargetLineEntry? lineEntry) {
517-
if (lineEntry == null || lineEntry.entries.isEmpty) return null;
518-
if (lineEntry.line != line) return lineEntry.entries.last;
519-
var entries = lineEntry.entries;
520-
var index = binarySearch(entries, (e) => e.column > column);
521-
return (index <= 0) ? null : entries[index - 1];
503+
/// Returns the last [TargetEntry] which includes the location denoted by
504+
/// [line], [column].
505+
///
506+
/// This corresponds to the computation of _last_ in [GetOriginalPositions][1]
507+
/// in the sourcemap specification.
508+
///
509+
/// [1]: https://tc39.es/ecma426/#sec-GetOriginalPositions
510+
TargetEntry? _findEntry(int line, int column) {
511+
// To find the *last* TargetEntry, we scan backwards, starting from the
512+
// first line after our target line, or the end of [lines].
513+
var lineIndex = binarySearch(lines, (e) => e.line > line);
514+
while (--lineIndex >= 0) {
515+
final lineEntry = lines[lineIndex];
516+
final entries = lineEntry.entries;
517+
if (entries.isEmpty) continue;
518+
// If we scan to a line before the target line, the last entry extends to
519+
// cover our search location.
520+
if (lineEntry.line != line) return entries.last;
521+
final index = binarySearch(entries, (e) => e.column > column);
522+
if (index > 0) return entries[index - 1];
523+
// We get here when the line has entries, but they are all after the
524+
// column. When this happens, the line and column correspond to the
525+
// previous entry, usually the last entry at the previous `lineIndex`.
526+
}
527+
return null;
522528
}
523529

524530
@override
525531
SourceMapSpan? spanFor(int line, int column,
526532
{Map<String, SourceFile>? files, String? uri}) {
527-
var entry = _findColumn(line, column, _findLine(line));
533+
final entry = _findEntry(line, column);
528534
if (entry == null) return null;
529535

530536
var sourceUrlId = entry.sourceUrlId;
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:source_maps/source_maps.dart';
6+
import 'package:source_span/source_span.dart';
7+
import 'package:test/test.dart';
8+
9+
void main() {
10+
/// This is a test for spans of the generated file that continue over several
11+
/// lines.
12+
///
13+
/// In a sourcemap, a span continues from the start encoded position until the
14+
/// next position, regardless of whether the second position in on the same
15+
/// line in the generated file or a subsequent line.
16+
void testSpans(int lineA, int columnA, int lineB, int columnB) {
17+
// Create a sourcemap describing a 'rectangular' generated file with three
18+
// spans, each potentially over several lines: (1) an initial span that is
19+
// unmapped, (2) a span that maps to file 'A', the span continuing until (3)
20+
// a span that maps to file 'B'.
21+
//
22+
// We can describe the mapping by an 'image' of the generated file, where
23+
// the positions marked as 'A' in the 'image' correspond to locations in the
24+
// generated file that map to locations in source file 'A'. Lines and
25+
// columns are zero-based.
26+
//
27+
// 0123456789
28+
// 0: ----------
29+
// 1: ----AAAAAA lineA: 1, columnA: 4, i.e. locationA
30+
// 2: AABBBBBBBB lineB: 2, columnB: 2, i.e. locationB
31+
// 3: BBBBBBBBBB
32+
//
33+
// Once we have the mapping, we probe every position in a 8x10 rectangle to
34+
// validate that it maps to the intended original source file.
35+
36+
expect(isBefore(lineB, columnB, lineA, columnA), isFalse,
37+
reason: 'Test valid only for ordered positions');
38+
39+
SourceLocation location(Uri? uri, int line, int column) {
40+
final offset = line * 10 + column;
41+
return SourceLocation(offset, sourceUrl: uri, line: line, column: column);
42+
}
43+
44+
// Locations in the generated file.
45+
final uriMap = Uri.parse('output.js.map');
46+
final locationA = location(uriMap, lineA, columnA);
47+
final locationB = location(uriMap, lineB, columnB);
48+
49+
// Original source locations.
50+
final sourceA = location(Uri.parse('A'), 0, 0);
51+
final sourceB = location(Uri.parse('B'), 0, 0);
52+
53+
final json = (SourceMapBuilder()
54+
..addLocation(sourceA, locationA, null)
55+
..addLocation(sourceB, locationB, null))
56+
.build(uriMap.toString());
57+
58+
final mapping = parseJson(json);
59+
60+
// Validate by comparing 'images' of the generate file.
61+
final expectedImage = StringBuffer();
62+
final actualImage = StringBuffer();
63+
64+
for (var line = 0; line < 8; line++) {
65+
for (var column = 0; column < 10; column++) {
66+
final span = mapping.spanFor(line, column);
67+
final expected = isBefore(line, column, lineA, columnA)
68+
? '-'
69+
: isBefore(line, column, lineB, columnB)
70+
? 'A'
71+
: 'B';
72+
final actual = span?.start.sourceUrl?.path ?? '-'; // Unmapped -> '-'.
73+
74+
expectedImage.write(expected);
75+
actualImage.write(actual);
76+
}
77+
expectedImage.writeln();
78+
actualImage.writeln();
79+
}
80+
expect(actualImage.toString(), expectedImage.toString());
81+
}
82+
83+
test('continued span, same position', () {
84+
testSpans(2, 4, 2, 4);
85+
});
86+
87+
test('continued span, same line', () {
88+
testSpans(2, 4, 2, 7);
89+
});
90+
91+
test('continued span, next line, earlier column', () {
92+
testSpans(2, 4, 3, 2);
93+
});
94+
95+
test('continued span, next line, later column', () {
96+
testSpans(2, 4, 3, 6);
97+
});
98+
99+
test('continued span, later line, earlier column', () {
100+
testSpans(2, 4, 5, 2);
101+
});
102+
103+
test('continued span, later line, later column', () {
104+
testSpans(2, 4, 5, 6);
105+
});
106+
}
107+
108+
bool isBefore(int line1, int column1, int line2, int column2) {
109+
return line1 < line2 || line1 == line2 && column1 < column2;
110+
}

pkgs/source_maps/test/refactor_test.dart

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,16 @@ void main() {
126126
' | ^\n'
127127
" '");
128128

129-
// Lines added have no mapping (they should inherit the last mapping),
130-
// but the end of the edit region continues were we left off:
131-
expect(_span(4, 1, map, file), isNull);
129+
// Newly added lines had no additional mapping, so they inherit the last
130+
// position on the previously mapped line. The end of the region continues
131+
// where the previous mapping left off.
132+
expect(
133+
_span(4, 1, map, file),
134+
'line 3, column 6: \n'
135+
' ,\n'
136+
'3 | 01*3456789\n'
137+
' | ^\n'
138+
' \'');
132139
expect(
133140
_span(4, 5, map, file),
134141
'line 3, column 8: \n'

0 commit comments

Comments
 (0)