Skip to content

Commit e9a99cd

Browse files
jensjohaCommit Queue
jensjoha
authored and
Commit Queue
committed
[DDC] Expression evaluation: Translate script uri if it is a package uri
Turns out we'll sometimes be given the script uri as a package uri, so we have to support that. To further complicate things we're not necessarily given a package config to translate from so we need to use the kernel component as a backup. Change-Id: I06ceb136b0b03164aaf7412d5332ae9c4c3338c4 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/447220 Reviewed-by: Srujan Gaddam <[email protected]> Reviewed-by: Johnni Winther <[email protected]> Commit-Queue: Jens Johansen <[email protected]>
1 parent f3f91ac commit e9a99cd

File tree

3 files changed

+193
-5
lines changed

3 files changed

+193
-5
lines changed

‎pkg/dev_compiler/lib/src/kernel/expression_compiler.dart‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,13 @@ class ExpressionCompiler {
6969
/// Compiles [expression] in library [libraryUri] and file [scriptUri]
7070
/// at [line]:[column] to JavaScript in [moduleName].
7171
///
72-
/// [libraryUri] and [scriptUri] can be the same, but if for instance
72+
/// [libraryUri] and [scriptUriString] can be the same, but if for instance
7373
/// evaluating expressions in a part file the [libraryUri] will be the uri of
74-
/// the "part of" file whereas [scriptUri] will be the uri of the part.
74+
/// the "part of" file whereas [scriptUriString] will be the uri of the part.
75+
/// The [libraryUri] should be an import-uri, i.e. a package uri if it exists.
76+
/// The [scriptUriString] is allowed to be a package uri in which case it will
77+
/// be attempted translated to a file uri to match what is recorded in the
78+
/// kernel file.
7579
///
7680
/// [line] and [column] are 1-based. Library level expressions typically use
7781
/// [line] and [column] 1 as an indicator that there is no relevant location.
@@ -89,7 +93,7 @@ class ExpressionCompiler {
8993
/// { 'x': '1', 'y': 'y', 'o': 'null' }
9094
Future<String?> compileExpressionToJs(
9195
String libraryUri,
92-
String? scriptUri,
96+
String? scriptUriString,
9397
int line,
9498
int column,
9599
Map<String, String> jsScope,
@@ -100,9 +104,26 @@ class ExpressionCompiler {
100104

101105
_log('Compiling expression \n$expression');
102106

107+
var scriptUri = scriptUriString == null
108+
? null
109+
: Uri.parse(scriptUriString);
110+
if (scriptUri != null && scriptUri.isScheme('package')) {
111+
scriptUri = _compiler.translateUri(scriptUri);
112+
if (scriptUri.isScheme('package')) {
113+
// No packages. Try to translate via the component source.
114+
for (final source in _component.uriToSource.values) {
115+
if (source.importUri == scriptUri && source.fileUri != null) {
116+
scriptUri = source.fileUri!;
117+
}
118+
}
119+
// The uri could still be a package-uri, but then failing to do
120+
// expression compilation seems fair.
121+
}
122+
}
123+
103124
var dartScope = _findScopeAt(
104125
Uri.parse(libraryUri),
105-
scriptUri == null ? null : Uri.parse(scriptUri),
126+
scriptUri,
106127
line,
107128
column,
108129
jsScope,

‎pkg/dev_compiler/test/expression_compiler/expression_compiler_worker_shared.dart‎

Lines changed: 154 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,7 @@ void runExpressionCompilationTests(ExpressionCompilerWorkerTestDriver driver) {
528528
);
529529
});
530530

531-
test('can compile expressions in part file', () {
531+
test('can compile expressions in part file without script uri', () {
532532
driver.requestController.add({
533533
'command': 'UpdateDeps',
534534
'inputs': driver.inputs,
@@ -618,6 +618,112 @@ void runExpressionCompilationTests(ExpressionCompilerWorkerTestDriver driver) {
618618
);
619619
});
620620

621+
test(
622+
'can compile expressions in part file with script uri as package uri',
623+
() {
624+
driver.requestController.add({
625+
'command': 'UpdateDeps',
626+
'inputs': driver.inputs,
627+
});
628+
629+
// We're really in the main file - and we're telling as package uri.
630+
driver.requestController.add({
631+
'command': 'CompileExpression',
632+
'expression': 'x.length',
633+
'line': 5,
634+
'column': 3,
635+
'jsModules': {},
636+
'jsScope': {'x': 'x'},
637+
'libraryUri': driver.config.getModule('testModule6').libraryUris.last,
638+
'scriptUri': driver.config.getModule('testModule6').libraryUris.last,
639+
'moduleName': driver.config.getModule('testModule6').moduleName,
640+
});
641+
642+
// We're really in the main file - and we're telling as file uri.
643+
driver.requestController.add({
644+
'command': 'CompileExpression',
645+
'expression': 'x.length',
646+
'line': 5,
647+
'column': 3,
648+
'jsModules': {},
649+
'jsScope': {'x': 'x'},
650+
'libraryUri': driver.config.getModule('testModule6').libraryUris.last,
651+
'scriptUri': driver.config
652+
.getModule('testModule6')
653+
.libraryUrisAsFileUri
654+
.last,
655+
'moduleName': driver.config.getModule('testModule6').moduleName,
656+
});
657+
658+
// We're really in the part file - and we're telling as package uri.
659+
driver.requestController.add({
660+
'command': 'CompileExpression',
661+
'expression': 'x + 1',
662+
'line': 5,
663+
'column': 3,
664+
'jsModules': {},
665+
'jsScope': {'x': 'x'},
666+
'libraryUri': driver.config.getModule('testModule6').libraryUris.last,
667+
'scriptUri': driver.config
668+
.getModule('testModule6')
669+
.partUrisAsPackageUri
670+
.last,
671+
'moduleName': driver.config.getModule('testModule6').moduleName,
672+
});
673+
674+
// We're really in the part file - and we're telling as file uri.
675+
driver.requestController.add({
676+
'command': 'CompileExpression',
677+
'expression': 'x + 1',
678+
'line': 5,
679+
'column': 3,
680+
'jsModules': {},
681+
'jsScope': {'x': 'x'},
682+
'libraryUri': driver.config.getModule('testModule6').libraryUris.last,
683+
'scriptUri': driver.config
684+
.getModule('testModule6')
685+
.partUrisAsFileUri
686+
.last,
687+
'moduleName': driver.config.getModule('testModule6').moduleName,
688+
});
689+
690+
expect(
691+
driver.responseController.stream,
692+
emitsInOrder([
693+
equals({'succeeded': true}),
694+
equals({
695+
'succeeded': true,
696+
'errors': isEmpty,
697+
'warnings': isEmpty,
698+
'infos': isEmpty,
699+
'compiledProcedure': contains('return x.length;'),
700+
}),
701+
equals({
702+
'succeeded': true,
703+
'errors': isEmpty,
704+
'warnings': isEmpty,
705+
'infos': isEmpty,
706+
'compiledProcedure': contains('return x.length;'),
707+
}),
708+
equals({
709+
'succeeded': true,
710+
'errors': isEmpty,
711+
'warnings': isEmpty,
712+
'infos': isEmpty,
713+
'compiledProcedure': contains('return x + 1;'),
714+
}),
715+
equals({
716+
'succeeded': true,
717+
'errors': isEmpty,
718+
'warnings': isEmpty,
719+
'infos': isEmpty,
720+
'compiledProcedure': contains('return x + 1;'),
721+
}),
722+
]),
723+
);
724+
},
725+
);
726+
621727
test('can compile series of expressions in various libraries', () {
622728
driver.requestController.add({
623729
'command': 'UpdateDeps',
@@ -866,6 +972,9 @@ class ModuleConfiguration {
866972
final Uri root;
867973
final String outputDir;
868974
final List<String> libraryUris;
975+
final List<String> libraryUrisAsFileUri;
976+
final List<String> partUrisAsPackageUri;
977+
final List<String> partUrisAsFileUri;
869978
final List<ModuleConfiguration> dependencies;
870979
final String moduleName;
871980
final String jsFileName;
@@ -877,6 +986,9 @@ class ModuleConfiguration {
877986
required this.outputDir,
878987
required this.moduleName,
879988
required this.libraryUris,
989+
this.libraryUrisAsFileUri = const [],
990+
this.partUrisAsPackageUri = const [],
991+
this.partUrisAsFileUri = const [],
880992
required this.dependencies,
881993
required this.jsFileName,
882994
required this.fullDillFileName,
@@ -907,6 +1019,22 @@ class TestProjectConfiguration {
9071019
TestProjectConfiguration(this.rootDirectory, this.moduleFormat);
9081020

9091021
void initialize() {
1022+
final testModule6 = ModuleConfiguration(
1023+
root: root,
1024+
outputDir: outputDir,
1025+
moduleName: 'packages/_testPackage/test_library6',
1026+
libraryUris: ['package:_testPackage/test_library9.dart'],
1027+
libraryUrisAsFileUri: [root.resolve('lib/test_library9.dart').toString()],
1028+
partUrisAsPackageUri: ['package:_testPackage/test_library9_part.dart'],
1029+
partUrisAsFileUri: [
1030+
root.resolve('lib/test_library9_part.dart').toString(),
1031+
],
1032+
dependencies: [],
1033+
jsFileName: 'test_library6.js',
1034+
fullDillFileName: 'test_library6.full.dill',
1035+
summaryDillFileName: 'test_library6.dill',
1036+
);
1037+
9101038
final testModule5 = ModuleConfiguration(
9111039
root: root,
9121040
outputDir: outputDir,
@@ -981,6 +1109,7 @@ class TestProjectConfiguration {
9811109
);
9821110

9831111
modules = {
1112+
'testModule6': testModule6,
9841113
'testModule5': testModule5,
9851114
'testModule4': testModule4,
9861115
'testModule3': testModule3,
@@ -1220,6 +1349,30 @@ void foo() {
12201349
// padding...............
12211350
print(y); // line 6 column 3 offset 101
12221351
}
1352+
''');
1353+
1354+
var testLibrary9 = root.resolve('lib/test_library9.dart');
1355+
var testLibrary9Part = root.resolve('lib/test_library9_part.dart');
1356+
File.fromUri(testLibrary9)
1357+
..createSync()
1358+
..writeAsStringSync('''
1359+
part 'test_library9_part.dart';
1360+
void main() {
1361+
String x = "foo";
1362+
// padding
1363+
foo(); // line 5 column 3 offset 81
1364+
print(x);
1365+
}
1366+
''');
1367+
File.fromUri(testLibrary9Part)
1368+
..createSync()
1369+
..writeAsStringSync('''
1370+
part of 'test_library9.dart';
1371+
void foo() {
1372+
int x = 42;
1373+
// padding.........
1374+
print(x); // line 5 column 3 offset 81
1375+
}
12231376
''');
12241377
}
12251378
}

‎pkg/front_end/lib/src/base/incremental_compiler.dart‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1625,6 +1625,20 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
16251625
_ticker.logMs("Appended libraries");
16261626
}
16271627

1628+
// Coverage-ignore(suite): Not run.
1629+
/// Attempt to translates a package-uri (or dart uri) to a file uri via the
1630+
/// package setup.
1631+
///
1632+
/// If the uri cannot be translated it will be returned unchanged.
1633+
Uri translateUri(Uri uri) {
1634+
IncrementalKernelTarget? lastGoodKernelTarget = this._lastGoodKernelTarget;
1635+
if (lastGoodKernelTarget != null &&
1636+
(uri.isScheme("package") || uri.isScheme("dart"))) {
1637+
return lastGoodKernelTarget.uriTranslator.translate(uri, false) ?? uri;
1638+
}
1639+
return uri;
1640+
}
1641+
16281642
@override
16291643
// Coverage-ignore(suite): Not run.
16301644
Future<Procedure?> compileExpression(

0 commit comments

Comments
 (0)