[kernel/DDC] DDC shouldn't crash when trying to translate line/column to offset for expresion compilation
Follow-up to https://dart-review.googlesource.com/c/sdk/+/446042.
Change-Id: Icfbaa763a6d6089a74f62088602c07c478520741
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/446420
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
diff --git a/pkg/dev_compiler/lib/src/kernel/expression_compiler.dart b/pkg/dev_compiler/lib/src/kernel/expression_compiler.dart
index f02e26f..b17aff6 100644
--- a/pkg/dev_compiler/lib/src/kernel/expression_compiler.dart
+++ b/pkg/dev_compiler/lib/src/kernel/expression_compiler.dart
@@ -353,7 +353,14 @@
// TODO(jensj): Eventually make the scriptUri required.
if (scriptFileUri != null) {
- final offset = _component.getOffset(scriptFileUri, line, column);
+ final offset = _component.getOffsetNoThrow(scriptFileUri, line, column);
+ if (offset < 0) {
+ _log(
+ 'Got offset negative offset ($offset) for '
+ '$scriptFileUri:$line:$column',
+ );
+ return DartScope(library, null, null, null, null, {}, []);
+ }
final scope = DartScopeBuilder2.findScopeFromOffset(
library,
scriptFileUri,
@@ -388,11 +395,19 @@
final foundScopes = <DartScope>[];
for (final fileUri in fileUris) {
- final offset = _component.getOffset(fileUri, line, column);
+ final offset = _component.getOffsetNoThrow(fileUri, line, column);
+ if (offset < 0) continue;
foundScopes.add(
DartScopeBuilder2.findScopeFromOffset(library, fileUri, offset),
);
}
+ if (foundScopes.isEmpty) {
+ _log(
+ 'Got only negative offsets for library/parts ${library.fileUri} '
+ 'at $line:$column.',
+ );
+ return DartScope(library, null, null, null, null, {}, []);
+ }
if (foundScopes.length == 1) {
return foundScopes[0];
}
diff --git a/pkg/dev_compiler/test/expression_compiler/expression_compiler_worker_shared.dart b/pkg/dev_compiler/test/expression_compiler/expression_compiler_worker_shared.dart
index baa3c9b..56d4c53 100644
--- a/pkg/dev_compiler/test/expression_compiler/expression_compiler_worker_shared.dart
+++ b/pkg/dev_compiler/test/expression_compiler/expression_compiler_worker_shared.dart
@@ -217,6 +217,88 @@
);
});
+ test('does not crash on line 0 in a library', () {
+ driver.requestController.add({
+ 'command': 'UpdateDeps',
+ 'inputs': driver.inputs,
+ });
+
+ driver.requestController.add({
+ 'command': 'CompileExpression',
+ 'expression': '1 + 1',
+ 'line': 0,
+ 'column': 0,
+ 'jsModules': {},
+ 'jsScope': {},
+ 'libraryUri': driver.config.getModule('testModule').libraryUris.first,
+ 'moduleName': driver.config.getModule('testModule').moduleName,
+ });
+
+ expect(
+ driver.responseController.stream,
+ emitsInOrder([
+ equals({'succeeded': true}),
+ equals({
+ 'succeeded': true,
+ 'errors': isEmpty,
+ 'warnings': isEmpty,
+ 'infos': isEmpty,
+ 'compiledProcedure': contains('1 + 1;'),
+ }),
+ ]),
+ );
+ });
+
+ test('does not crash on line thats too high in a library', () {
+ driver.requestController.add({
+ 'command': 'UpdateDeps',
+ 'inputs': driver.inputs,
+ });
+
+ driver.requestController.add({
+ 'command': 'CompileExpression',
+ 'expression': '1 + 1',
+ 'line': 10000000,
+ 'column': 1,
+ 'jsModules': {},
+ 'jsScope': {},
+ 'libraryUri': driver.config.getModule('testModule').libraryUris.first,
+ 'moduleName': driver.config.getModule('testModule').moduleName,
+ });
+
+ driver.requestController.add({
+ 'command': 'CompileExpression',
+ 'expression': '2 + 2',
+ 'line': 1,
+ 'column': 10000000,
+ 'jsModules': {},
+ 'jsScope': {},
+ 'libraryUri': driver.config.getModule('testModule').libraryUris.first,
+ 'moduleName': driver.config.getModule('testModule').moduleName,
+ });
+
+ expect(
+ driver.responseController.stream,
+ emitsInOrder([
+ equals({'succeeded': true}),
+ equals({
+ 'succeeded': true,
+ 'errors': isEmpty,
+ 'warnings': isEmpty,
+ 'infos': isEmpty,
+ 'compiledProcedure': contains('1 + 1;'),
+ }),
+ equals({
+ 'succeeded': true,
+ 'errors': isEmpty,
+ 'warnings': isEmpty,
+ 'infos': isEmpty,
+ 'compiledProcedure': contains('2 + 2;'),
+ }),
+ ]),
+ );
+ });
+
test('can compile expressions in a library', () {
driver.requestController.add({
'command': 'UpdateDeps',
diff --git a/pkg/kernel/lib/src/ast/components.dart b/pkg/kernel/lib/src/ast/components.dart
index 17df623..770b677 100644
--- a/pkg/kernel/lib/src/ast/components.dart
+++ b/pkg/kernel/lib/src/ast/components.dart
@@ -159,6 +159,14 @@
return uriToSource[file]?.getOffset(line, column) ?? -1;
}
+ /// Translates line and column numbers to an offset in the given file.
+ ///
+ /// Returns offset of the line and column in the file, or -1 if the
+ /// source is not available, has no lines, or the line is out of range.
+ int getOffsetNoThrow(Uri file, int line, int column) {
+ return uriToSource[file]?.getOffsetNoThrow(line, column) ?? -1;
+ }
+
void addMetadataRepository(MetadataRepository repository) {
metadata[repository.tag] = repository;
}
@@ -291,6 +299,25 @@
RangeError.checkValueInInterval(offset, 0, lineStarts.last, 'offset');
return offset;
}
+
+ /// Translates 1-based line and column numbers to an offset in the given file
+ ///
+ /// Returns offset of the line and column in the file, or -1 if the source
+ /// has no lines or the input is out of range.
+ int getOffsetNoThrow(int line, int column) {
+ List<int>? lineStarts = this.lineStarts;
+ if (lineStarts == null || lineStarts.isEmpty) {
+ return -1;
+ }
+ if (line < 1 || line > lineStarts.length) {
+ return -1;
+ }
+ int offset = lineStarts[line - 1] + column - 1;
+ if (offset < 0 || offset > lineStarts.last) {
+ return -1;
+ }
+ return offset;
+ }
}
abstract class MetadataRepository<T> {
