From 23a6d3399de9b0eb551d2b069d750ded03b5ac01 Mon Sep 17 00:00:00 2001 From: Stephen Adams Date: Thu, 9 Oct 2025 12:53:04 -0700 Subject: [PATCH 1/6] Fix `SingleMapping.spanFor` to use previous line --- pkgs/source_maps/CHANGELOG.md | 4 +++ pkgs/source_maps/lib/parser.dart | 43 +++++++++++++----------- pkgs/source_maps/test/refactor_test.dart | 8 ++++- 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/pkgs/source_maps/CHANGELOG.md b/pkgs/source_maps/CHANGELOG.md index b06ac72ea..ad79d389f 100644 --- a/pkgs/source_maps/CHANGELOG.md +++ b/pkgs/source_maps/CHANGELOG.md @@ -1,5 +1,9 @@ ## 0.10.14-wip +* Fix `SingleMapping.spanFor` to use the entry from a previous line as specified + by the sourcemap specification + (https://tc39.es/ecma426/#sec-GetOriginalPositions), + ## 0.10.13 * Require Dart 3.3 diff --git a/pkgs/source_maps/lib/parser.dart b/pkgs/source_maps/lib/parser.dart index 590dfc682..961e0ef01 100644 --- a/pkgs/source_maps/lib/parser.dart +++ b/pkgs/source_maps/lib/parser.dart @@ -500,31 +500,34 @@ class SingleMapping extends Mapping { StateError('Invalid entry in sourcemap, expected 1, 4, or 5' ' values, but got $seen.\ntargeturl: $targetUrl, line: $line'); - /// Returns [TargetLineEntry] which includes the location in the target [line] - /// number. In particular, the resulting entry is the last entry whose line - /// number is lower or equal to [line]. - TargetLineEntry? _findLine(int line) { - var index = binarySearch(lines, (e) => e.line > line); - return (index <= 0) ? null : lines[index - 1]; - } - - /// Returns [TargetEntry] which includes the location denoted by - /// [line], [column]. If [lineEntry] corresponds to [line], then this will be - /// the last entry whose column is lower or equal than [column]. If - /// [lineEntry] corresponds to a line prior to [line], then the result will be - /// the very last entry on that line. - TargetEntry? _findColumn(int line, int column, TargetLineEntry? lineEntry) { - if (lineEntry == null || lineEntry.entries.isEmpty) return null; - if (lineEntry.line != line) return lineEntry.entries.last; - var entries = lineEntry.entries; - var index = binarySearch(entries, (e) => e.column > column); - return (index <= 0) ? null : entries[index - 1]; + /// Returns the last [TargetEntry] which includes the location denoted by + /// [line], [column]. + /// + /// This corresponds to the computation of _last_ in [GetOriginalPositions][1] + /// in the sourcemap specification. + /// + /// [1]: https://tc39.es/ecma426/#sec-GetOriginalPositions + TargetEntry? _findEntry(int line, int column) { + // To find the *last* TargetEntry, we scan backwards, starting from the first + // line after our target line, or the end of [lines]. + int lineIndex = binarySearch(lines, (e) => e.line > line); + while (--lineIndex >= 0) { + final lineEntry = lines[lineIndex]; + final entries = lineEntry.entries; + if (entries.isEmpty) continue; + // If we scan to a line before the target line, the last entry extends to + // cover our search location. + if (lineEntry.line != line) return entries.last; + final index = binarySearch(entries, (e) => e.column > column); + if (index > 0) return entries[index - 1]; + } + return null; } @override SourceMapSpan? spanFor(int line, int column, {Map? files, String? uri}) { - var entry = _findColumn(line, column, _findLine(line)); + final entry = _findEntry(line, column); if (entry == null) return null; var sourceUrlId = entry.sourceUrlId; diff --git a/pkgs/source_maps/test/refactor_test.dart b/pkgs/source_maps/test/refactor_test.dart index 5bc3818e5..76d31e202 100644 --- a/pkgs/source_maps/test/refactor_test.dart +++ b/pkgs/source_maps/test/refactor_test.dart @@ -128,7 +128,13 @@ void main() { // Lines added have no mapping (they should inherit the last mapping), // but the end of the edit region continues were we left off: - expect(_span(4, 1, map, file), isNull); + expect( + _span(4, 1, map, file), + 'line 3, column 6: \n' + ' ,\n' + '3 | 01*3456789\n' + ' | ^\n' + ' \''); expect( _span(4, 5, map, file), 'line 3, column 8: \n' From 7bd8d0ecfec4c7146fcfdec9181b495f9ba52122 Mon Sep 17 00:00:00 2001 From: Stephen Adams Date: Thu, 9 Oct 2025 14:28:37 -0700 Subject: [PATCH 2/6] fix lints --- pkgs/source_maps/lib/parser.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkgs/source_maps/lib/parser.dart b/pkgs/source_maps/lib/parser.dart index 961e0ef01..933af1dad 100644 --- a/pkgs/source_maps/lib/parser.dart +++ b/pkgs/source_maps/lib/parser.dart @@ -508,9 +508,9 @@ class SingleMapping extends Mapping { /// /// [1]: https://tc39.es/ecma426/#sec-GetOriginalPositions TargetEntry? _findEntry(int line, int column) { - // To find the *last* TargetEntry, we scan backwards, starting from the first - // line after our target line, or the end of [lines]. - int lineIndex = binarySearch(lines, (e) => e.line > line); + // To find the *last* TargetEntry, we scan backwards, starting from the + // first line after our target line, or the end of [lines]. + var lineIndex = binarySearch(lines, (e) => e.line > line); while (--lineIndex >= 0) { final lineEntry = lines[lineIndex]; final entries = lineEntry.entries; From f33078fafdca6df9fce5d7cfcae45013de8a1972 Mon Sep 17 00:00:00 2001 From: Stephen Adams Date: Thu, 9 Oct 2025 20:21:36 -0700 Subject: [PATCH 3/6] xxx --- pkgs/source_maps/lib/parser.dart | 33 ++++++++++++++---------- pkgs/source_maps/test/end2end_test.dart | 33 ++++++++++++++++++++++++ pkgs/source_maps/test/refactor_test.dart | 6 +++-- 3 files changed, 57 insertions(+), 15 deletions(-) diff --git a/pkgs/source_maps/lib/parser.dart b/pkgs/source_maps/lib/parser.dart index 933af1dad..16c642c24 100644 --- a/pkgs/source_maps/lib/parser.dart +++ b/pkgs/source_maps/lib/parser.dart @@ -324,20 +324,24 @@ class SingleMapping extends Mapping { } var sourceUrl = sourceEntry.source.sourceUrl; - var urlId = urls.putIfAbsent( - sourceUrl == null ? '' : sourceUrl.toString(), () => urls.length); - - if (sourceEntry.source is FileLocation) { - files.putIfAbsent( - urlId, () => (sourceEntry.source as FileLocation).file); - } - var sourceEntryIdentifierName = sourceEntry.identifierName; - var srcNameId = sourceEntryIdentifierName == null - ? null - : names.putIfAbsent(sourceEntryIdentifierName, () => names.length); - targetEntries.add(TargetEntry(sourceEntry.target.column, urlId, - sourceEntry.source.line, sourceEntry.source.column, srcNameId)); + //if (sourceUrl == null && sourceEntryIdentifierName == null) { + // targetEntries.add(TargetEntry(sourceEntry.target.column)); + //} else { + var urlId = urls.putIfAbsent( + sourceUrl == null ? '' : sourceUrl.toString(), () => urls.length); + + if (sourceEntry.source is FileLocation) { + files.putIfAbsent( + urlId, () => (sourceEntry.source as FileLocation).file); + } + + var srcNameId = sourceEntryIdentifierName == null + ? null + : names.putIfAbsent(sourceEntryIdentifierName, () => names.length); + targetEntries.add(TargetEntry(sourceEntry.target.column, urlId, + sourceEntry.source.line, sourceEntry.source.column, srcNameId)); + //} } return SingleMapping._(fileUrl, urls.values.map((i) => files[i]).toList(), urls.keys.toList(), names.keys.toList(), lines); @@ -520,6 +524,9 @@ class SingleMapping extends Mapping { if (lineEntry.line != line) return entries.last; final index = binarySearch(entries, (e) => e.column > column); if (index > 0) return entries[index - 1]; + // We get here when the line has entries, but they are all after the + // column. When this happens, the line and column correspond to the + // previous entry, usually the last entry at the previous `lineIndex`. } return null; } diff --git a/pkgs/source_maps/test/end2end_test.dart b/pkgs/source_maps/test/end2end_test.dart index 84dd5badc..20e5749f4 100644 --- a/pkgs/source_maps/test/end2end_test.dart +++ b/pkgs/source_maps/test/end2end_test.dart @@ -104,6 +104,39 @@ void main() { check(outputExpr, mapping, inputExpr, true); }); + test('build + parse - continued entries', () { + final uriA = Uri.parse('A'); + final uriB = Uri.parse('B'); + final uriM = Uri.parse('output.map'); + + SourceLocation location(Uri? uri, int line, int column) { + final offset = line * 10 + column; + return SourceLocation(offset, sourceUrl: uri, line: line, column: column); + } + + final location1 = location(uriM, 3, 3); + final location2 = location(uriM, 6, 5); + final location3 = location(uriM, 8, 7); + + final json = (SourceMapBuilder() + ..addLocation(SourceLocation(0, sourceUrl: uriA), location1, null) + ..addLocation(SourceLocation(0, sourceUrl: uriB), location2, null) + ..addLocation(SourceLocation(0), location3, null)) + .build(uriM.toString()); + + final mapping = parseJson(json); + + for (var line = 0; line < 10; line++) { + for (var column = 0; column < 10; column++) { + final span = mapping.spanFor(line, column); + if (span == null) + + print('$line $column\t$span'); + } + } + print(json); + }); + test('printer projecting marks + parse', () { var out = inputContent.replaceAll('long', '_s'); var file = SourceFile.fromString(out, url: 'output2.dart'); diff --git a/pkgs/source_maps/test/refactor_test.dart b/pkgs/source_maps/test/refactor_test.dart index 76d31e202..a3de5ceee 100644 --- a/pkgs/source_maps/test/refactor_test.dart +++ b/pkgs/source_maps/test/refactor_test.dart @@ -59,6 +59,7 @@ void main() { var printer = (txn.commit()..build('')); var output = printer.text; var map = parse(printer.map!); + print(printer.map); expect(output, '0123456789\n0*23456789\n01*34__\n 789\na___cdefghij\nabcd*fghij\n'); @@ -126,8 +127,9 @@ void main() { ' | ^\n' " '"); - // Lines added have no mapping (they should inherit the last mapping), - // but the end of the edit region continues were we left off: + // Newly added lines had no additional mapping, so they inherit the last + // position on the previously mapped line. The end of the region continues + // where the previous mapping left off. expect( _span(4, 1, map, file), 'line 3, column 6: \n' From 6b3d38d1bd47b9ad692a51c04001729b2c4e3a1c Mon Sep 17 00:00:00 2001 From: Stephen Adams Date: Fri, 10 Oct 2025 11:15:13 -0700 Subject: [PATCH 4/6] xxx --- pkgs/source_maps/lib/parser.dart | 30 ++++++++++----------- pkgs/source_maps/test/end2end_test.dart | 33 ------------------------ pkgs/source_maps/test/refactor_test.dart | 1 - 3 files changed, 13 insertions(+), 51 deletions(-) diff --git a/pkgs/source_maps/lib/parser.dart b/pkgs/source_maps/lib/parser.dart index 16c642c24..2ce4d6d38 100644 --- a/pkgs/source_maps/lib/parser.dart +++ b/pkgs/source_maps/lib/parser.dart @@ -324,24 +324,20 @@ class SingleMapping extends Mapping { } var sourceUrl = sourceEntry.source.sourceUrl; - var sourceEntryIdentifierName = sourceEntry.identifierName; - //if (sourceUrl == null && sourceEntryIdentifierName == null) { - // targetEntries.add(TargetEntry(sourceEntry.target.column)); - //} else { - var urlId = urls.putIfAbsent( - sourceUrl == null ? '' : sourceUrl.toString(), () => urls.length); - - if (sourceEntry.source is FileLocation) { - files.putIfAbsent( - urlId, () => (sourceEntry.source as FileLocation).file); - } + var urlId = urls.putIfAbsent( + sourceUrl == null ? '' : sourceUrl.toString(), () => urls.length); - var srcNameId = sourceEntryIdentifierName == null - ? null - : names.putIfAbsent(sourceEntryIdentifierName, () => names.length); - targetEntries.add(TargetEntry(sourceEntry.target.column, urlId, - sourceEntry.source.line, sourceEntry.source.column, srcNameId)); - //} + if (sourceEntry.source is FileLocation) { + files.putIfAbsent( + urlId, () => (sourceEntry.source as FileLocation).file); + } + + var sourceEntryIdentifierName = sourceEntry.identifierName; + var srcNameId = sourceEntryIdentifierName == null + ? null + : names.putIfAbsent(sourceEntryIdentifierName, () => names.length); + targetEntries.add(TargetEntry(sourceEntry.target.column, urlId, + sourceEntry.source.line, sourceEntry.source.column, srcNameId)); } return SingleMapping._(fileUrl, urls.values.map((i) => files[i]).toList(), urls.keys.toList(), names.keys.toList(), lines); diff --git a/pkgs/source_maps/test/end2end_test.dart b/pkgs/source_maps/test/end2end_test.dart index 20e5749f4..84dd5badc 100644 --- a/pkgs/source_maps/test/end2end_test.dart +++ b/pkgs/source_maps/test/end2end_test.dart @@ -104,39 +104,6 @@ void main() { check(outputExpr, mapping, inputExpr, true); }); - test('build + parse - continued entries', () { - final uriA = Uri.parse('A'); - final uriB = Uri.parse('B'); - final uriM = Uri.parse('output.map'); - - SourceLocation location(Uri? uri, int line, int column) { - final offset = line * 10 + column; - return SourceLocation(offset, sourceUrl: uri, line: line, column: column); - } - - final location1 = location(uriM, 3, 3); - final location2 = location(uriM, 6, 5); - final location3 = location(uriM, 8, 7); - - final json = (SourceMapBuilder() - ..addLocation(SourceLocation(0, sourceUrl: uriA), location1, null) - ..addLocation(SourceLocation(0, sourceUrl: uriB), location2, null) - ..addLocation(SourceLocation(0), location3, null)) - .build(uriM.toString()); - - final mapping = parseJson(json); - - for (var line = 0; line < 10; line++) { - for (var column = 0; column < 10; column++) { - final span = mapping.spanFor(line, column); - if (span == null) - - print('$line $column\t$span'); - } - } - print(json); - }); - test('printer projecting marks + parse', () { var out = inputContent.replaceAll('long', '_s'); var file = SourceFile.fromString(out, url: 'output2.dart'); diff --git a/pkgs/source_maps/test/refactor_test.dart b/pkgs/source_maps/test/refactor_test.dart index a3de5ceee..5ac239c54 100644 --- a/pkgs/source_maps/test/refactor_test.dart +++ b/pkgs/source_maps/test/refactor_test.dart @@ -59,7 +59,6 @@ void main() { var printer = (txn.commit()..build('')); var output = printer.text; var map = parse(printer.map!); - print(printer.map); expect(output, '0123456789\n0*23456789\n01*34__\n 789\na___cdefghij\nabcd*fghij\n'); From 614df72b482e30f27229c324831632cf11d7635b Mon Sep 17 00:00:00 2001 From: Stephen Adams Date: Fri, 10 Oct 2025 13:58:50 -0700 Subject: [PATCH 5/6] add test --- .../test/continued_region_test.dart | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 pkgs/source_maps/test/continued_region_test.dart diff --git a/pkgs/source_maps/test/continued_region_test.dart b/pkgs/source_maps/test/continued_region_test.dart new file mode 100644 index 000000000..7bb8f5ff4 --- /dev/null +++ b/pkgs/source_maps/test/continued_region_test.dart @@ -0,0 +1,110 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:source_maps/source_maps.dart'; +import 'package:source_span/source_span.dart'; +import 'package:test/test.dart'; + +void main() { + /// This is a test for spans of the generated file that continue over several + /// lines. + /// + /// In a sourcemap, a span continues from the start encoded position until the + /// next position, regardless of whether the second position in on the same + /// line in the generated file or a subsequent line. + void testSpans(int lineA, int columnA, int lineB, int columnB) { + // Create a sourcemap describing a 'rectangular' generated file with three + // spans, each potentially over several lines: (1) an initial span that is + // unmapped, (2) a span that maps to file 'A', the span continuing until (3) + // a span that maps to file 'B'. + // + // We can describe the mapping by an 'image' of the generated file, where + // the positions marked as 'A' in the 'image' correspond to locations in the + // generated file that map to locations in source file file 'A'. Lines and + // columns are zero-based. + // + // 0123456789 + // 0: ---------- + // 1: ----AAAAAA lineA: 1, columnA: 4, i.e. locationA + // 2: AABBBBBBBB lineB: 2, columnB: 2, i.e. locationB + // 3: BBBBBBBBBB + // + // Once we have the mapping, we probe every position in a 8x10 rectangle to + // validate that it maps to the intended original source file. + + expect(isBefore(lineB, columnB, lineA, columnA), isFalse, + reason: 'Test valid only for ordered positions'); + + SourceLocation location(Uri? uri, int line, int column) { + final offset = line * 10 + column; + return SourceLocation(offset, sourceUrl: uri, line: line, column: column); + } + + // Locations in the generated file. + final uriMap = Uri.parse('output.js.map'); + final locationA = location(uriMap, lineA, columnA); + final locationB = location(uriMap, lineB, columnB); + + // Original source locations. + final sourceA = location(Uri.parse('A'), 0, 0); + final sourceB = location(Uri.parse('B'), 0, 0); + + final json = (SourceMapBuilder() + ..addLocation(sourceA, locationA, null) + ..addLocation(sourceB, locationB, null)) + .build(uriMap.toString()); + + final mapping = parseJson(json); + + // Validate by comparing 'images' of the generate file. + final expectedImage = StringBuffer(); + final actualImage = StringBuffer(); + + for (var line = 0; line < 8; line++) { + for (var column = 0; column < 10; column++) { + final span = mapping.spanFor(line, column); + final expected = isBefore(line, column, lineA, columnA) + ? '-' + : isBefore(line, column, lineB, columnB) + ? 'A' + : 'B'; + final actual = span?.start.sourceUrl?.path ?? '-'; // Unmapped -> '-'. + + expectedImage.write(expected); + actualImage.write(actual); + } + expectedImage.writeln(); + actualImage.writeln(); + } + expect(actualImage.toString(), expectedImage.toString()); + } + + test('continued span, same position', () { + testSpans(2, 4, 2, 4); + }); + + test('continued span, same line', () { + testSpans(2, 4, 2, 7); + }); + + test('continued span, next line, earlier column', () { + testSpans(2, 4, 3, 2); + }); + + test('continued span, next line, later column', () { + testSpans(2, 4, 3, 6); + }); + + test('continued span, later line, earlier column', () { + testSpans(2, 4, 5, 2); + }); + + test('continued span, later line, later column', () { + testSpans(2, 4, 5, 6); + }); +} + +bool isBefore(int line1, int column1, int line2, int column2) { + return line1 < line2 || line1 == line2 && column1 < column2; +} From a344b6f198722281c25d88df92531830933e3f89 Mon Sep 17 00:00:00 2001 From: Stephen Adams Date: Fri, 10 Oct 2025 15:56:34 -0700 Subject: [PATCH 6/6] fix typo --- pkgs/source_maps/test/continued_region_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/source_maps/test/continued_region_test.dart b/pkgs/source_maps/test/continued_region_test.dart index 7bb8f5ff4..eb12e3012 100644 --- a/pkgs/source_maps/test/continued_region_test.dart +++ b/pkgs/source_maps/test/continued_region_test.dart @@ -21,7 +21,7 @@ void main() { // // We can describe the mapping by an 'image' of the generated file, where // the positions marked as 'A' in the 'image' correspond to locations in the - // generated file that map to locations in source file file 'A'. Lines and + // generated file that map to locations in source file 'A'. Lines and // columns are zero-based. // // 0123456789