Skip to content

Commit 364b366

Browse files
bkonyiCommit Queue
authored andcommitted
Reland "[ DDC / CFE ] Add support for allowing imports of unsupported libraries"
This reverts commit c616db3. Reason for revert: Fix landed downstream in Flutter engine: flutter/flutter#180127 Original change's description: > Revert "[ DDC / CFE ] Add support for allowing imports of unsupported libraries" > > This reverts commit b5e60be. > > Reason for revert: broke Flutter web engine tests > > Original change's description: > > [ DDC / CFE ] Add support for allowing imports of unsupported libraries > > > > This change adds support for allowing for imports of unsupported > > platform-specific libraries when the > > `--include-unsupported-platform-library-stubs` flag is provided to the > > CFE. > > > > This flag sets the `includeUnsupportedPlatformLibraryStubs` property in > > `TargetFlags`, which `Target`s can use to conditionally return different > > `DartLibrarySupport` objects with different supported/unsupported > > library sets. > > > > A `checkForUnsupportedDartColonImports` function has been added to > > `Target` that uses the value of `dartLibrarySupport` to determine if > > there's any unsupported library imports. This function is called after > > the various transformation operations provided by the `Target` > > implementation, meaning the import of an unsupported library specified > > in `dartLibrarySupport` will now result in a compilation error (this > > includes `dart:mirrors` imports for VM targets when mirrors are > > disabled, which was previously handled by the VM itself). > > > > Related to #62125 > > > > TEST=Tests added / modified > > > > Change-Id: Ife819b2e1a6d28f67d80aab6701cd23a1724aa4d > > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/465760 > > Reviewed-by: Nicholas Shahan <[email protected]> > > Reviewed-by: Johnni Winther <[email protected]> > > Commit-Queue: Ben Konyi <[email protected]> > > Change-Id: I0b59f00e55a2424f783351abd977eb38409ce01f > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/469100 > Reviewed-by: Nate Biggs <[email protected]> > Commit-Queue: Alexander Markov <[email protected]> > Bot-Commit: Rubber Stamper <[email protected]> > Reviewed-by: Ben Konyi <[email protected]> > Reviewed-by: Sigmund Cherem <[email protected]> Change-Id: I1ae2eac675432286aebabea3c1f58caf35a27fbb Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/469240 Commit-Queue: Ben Konyi <[email protected]> Reviewed-by: Sigmund Cherem <[email protected]> Bot-Commit: Rubber Stamper <[email protected]>
1 parent f380cba commit 364b366

File tree

55 files changed

+833
-147
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+833
-147
lines changed

pkg/_fe_analyzer_shared/lib/src/util/libraries_specification.dart

Lines changed: 109 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
/// }
3333
/// "mirrors": {
3434
/// "uri": "mirrors/mirrors.dart",
35-
/// "supported": false
35+
/// "support_conditional_import": false
3636
/// }
3737
/// }
3838
/// }
@@ -61,24 +61,46 @@
6161
/// which will be resolved relative to the location of the library
6262
/// specification file.
6363
///
64-
/// - The "supported" entry on the library information is optional. The value
65-
/// is a boolean indicating whether the library is supported in the
66-
/// underlying target. However, since the libraries are assumed to be
67-
/// supported by default, we only expect users to use `false`.
64+
/// - The "support_conditional_import" entry on the library information is
65+
/// optional. The value is a boolean indicating whether the library is
66+
/// supported in the underlying target. However, since the libraries are
67+
/// assumed to be supported by default, we only expect users to use
68+
/// `false`.
6869
///
6970
/// The purpose of this value is to configure conditional imports and
7071
/// environment constants. By default every platform library that is
7172
/// available in the "libraries" section implicitly defines an environment
7273
/// variable `dart.library.name` as `"true"`, to indicate that the library
7374
/// is supported. Some backends allow imports to an unsupported platform
74-
/// library (turning a static error into a runtime error when the library is
75-
/// eventually accessed). These backends can use `supported: false` to
76-
/// report that such library is still not supported in conditional imports
77-
/// and const `fromEnvironment` expressions.
75+
/// library (turning a static error into a runtime error when the library
76+
/// is eventually accessed). These backends can use
77+
/// `support_conditional_import: false` to report that such library is
78+
/// still not supported in conditional imports and const `fromEnvironment`
79+
/// expressions.
7880
///
7981
/// Internal libraries are never supported through conditional imports and
8082
/// const `fromEnvironment` expressions.
8183
///
84+
/// - The "support_direct_import" entry on the library information is
85+
/// optional. The value is an enumerated type indicating whether the library
86+
/// is allowed to be imported by the underlying target. This property can
87+
/// take one of the following values:
88+
///
89+
/// - "always": the library is supported by the underlying target and
90+
/// can always be imported. This is the default.
91+
/// - "never": the library is not supported by the underlying target and
92+
/// imports of the library will be treated as compile time errors.
93+
/// - "with_flag": the library is only supported when the
94+
/// `--include-unsupported-platform-library-stubs` flag is set. If the
95+
/// flag is not set, imports of the library will be treated as
96+
/// compile time errors. Otherwise, the import will be allowed.
97+
///
98+
/// Some backends allow imports to an unsupported platform library (turning
99+
/// a static error into a runtime error when the library is eventually
100+
/// accessed). These backends can use `support_direct_import: "with_flag"`
101+
/// to control access to these libraries, allowing for developer tooling to
102+
/// handle code which imports unsupported libraries for a given platform.
103+
///
82104
/// - The "include" entry is a list of maps, each containing either a "path"
83105
/// and a "target" entry, or only a "target" entry.
84106
///
@@ -314,17 +336,36 @@ class LibrariesSpecification {
314336
_reportError(messagePatchesMustBeListOrString(libraryName));
315337
}
316338

317-
dynamic supported = data['supported'] ?? true;
318-
if (supported is! bool) {
319-
_reportError(messageSupportedIsNotABool(supported));
339+
final Object supportConditionalImport =
340+
data['support_conditional_import'] ?? true;
341+
if (supportConditionalImport is! bool) {
342+
_reportError(
343+
messagePropertyIsNotABool(
344+
'support_conditional_import',
345+
supportConditionalImport,
346+
),
347+
);
320348
}
349+
350+
final Object? supportDirectImportRaw = data['support_direct_import'];
351+
final Importability? importability = supportDirectImportRaw == null
352+
? Importability.always
353+
: Importability.fromJson(supportDirectImportRaw);
354+
if (importability == null) {
355+
_reportError(
356+
messageSupportDirectImportIsNotValidValue(supportDirectImportRaw!),
357+
);
358+
}
359+
321360
libraries[libraryName] = new LibraryInfo(
322361
libraryName,
323362
uri,
324363
patches,
325364
// Internal libraries are never supported through conditional
326365
// imports and const `fromEnvironment` expressions.
327-
isSupported: supported && !libraryName.startsWith('_'),
366+
supportConditionalImport:
367+
supportConditionalImport && !libraryName.startsWith('_'),
368+
importability: importability,
328369
);
329370
});
330371
currentTargets.remove(targetName);
@@ -362,8 +403,11 @@ class LibrariesSpecification {
362403
'uri': pathFor(lib.uri),
363404
'patches': lib.patches.map(pathFor).toList(),
364405
};
365-
if (!lib.isSupported) {
366-
libraries[name]['supported'] = false;
406+
if (!lib.supportConditionalImport) {
407+
libraries[name]['support_conditional_import'] = false;
408+
}
409+
if (lib.importability != Importability.always) {
410+
libraries[name]['support_direct_import'] = lib.importability.value;
367411
}
368412
});
369413
result[targetName] = {'libraries': libraries};
@@ -372,6 +416,32 @@ class LibrariesSpecification {
372416
}
373417
}
374418

419+
/// Determines whether or not a `dart:*` library is importable for a given
420+
/// platform.
421+
enum Importability {
422+
/// This `dart:*` library is always importable on the target platform.
423+
always(value: 'always'),
424+
425+
/// This `dart:*` library is only importable on the target platform when
426+
/// `--include-unsupported-platform-library-stubs` is provided to the CFE.
427+
withFlag(value: 'with_flag'),
428+
429+
/// This `dart:*` library is never importable on the target platform.
430+
never(value: 'never');
431+
432+
const Importability({required this.value});
433+
434+
final String value;
435+
436+
static Importability? fromJson(Object? value) {
437+
if (value is! String) return null;
438+
if (value == always.value) return always;
439+
if (value == withFlag.value) return withFlag;
440+
if (value == never.value) return never;
441+
return null;
442+
}
443+
}
444+
375445
/// Specifies information about all libraries supported by a given target.
376446
class TargetLibrariesSpecification {
377447
/// Name of the target platform.
@@ -404,13 +474,19 @@ class LibraryInfo {
404474

405475
/// Whether the library is supported and thus `dart.library.name` is "true"
406476
/// for conditional imports and fromEnvironment constants.
407-
final bool isSupported;
477+
final bool supportConditionalImport;
478+
479+
/// Whether the library is importable for a given target platform.
480+
///
481+
/// If not explicitly provided, this field defaults to [Importability.always].
482+
final Importability importability;
408483

409484
const LibraryInfo(
410485
this.name,
411486
this.uri,
412487
this.patches, {
413-
this.isSupported = true,
488+
this.supportConditionalImport = true,
489+
this.importability = Importability.always,
414490
});
415491

416492
/// The import uri for the defined library.
@@ -495,6 +571,19 @@ String messageUnsupportedUriScheme(String uriValue, Uri specUri) =>
495571
String messagePatchesMustBeListOrString(String libraryName) =>
496572
'"patches" entry for "$libraryName" is not a list or a string.';
497573

498-
String messageSupportedIsNotABool(Object supportedValue) =>
499-
'"supported" entry: expected a `bool` but '
500-
'got a `${supportedValue.runtimeType}` ("$supportedValue").';
574+
String messagePropertyIsNotABool(String key, Object value) =>
575+
'"$key" entry: expected a `bool` but '
576+
'got a `${value.runtimeType}` ("$value").';
577+
578+
String messageOnlyOnePropertyCanBeDefined(List<String> properties) {
579+
return 'Only one of the following properties can be defined: '
580+
'${properties.map((e) => '`$e`').join(',')}.';
581+
}
582+
583+
String messageSupportDirectImportIsNotValidValue(Object value) {
584+
final String values = Importability.values
585+
.map((e) => '`${e.value}`')
586+
.join(',');
587+
return '"support_direct_import" entry: expected one of $values but got a '
588+
'`${value.runtimeType}` ("$value").';
589+
}

0 commit comments

Comments
 (0)