Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dart_services] Update to analyzer v6 #2726

Merged
merged 7 commits into from
Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkgs/dart_services/lib/src/analysis.dart
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class Analyzer {
Future<void> _checkPackageReferences(List<ImportDirective> imports) async {
final unsupportedImports = project.getUnsupportedImports(
imports,
sourcesFileList: [kMainDart],
sourceFiles: {kMainDart},
);

if (unsupportedImports.isNotEmpty) {
Expand Down
4 changes: 2 additions & 2 deletions pkgs/dart_services/lib/src/compiling.dart
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class Compiler {
}) async {
final imports = getAllImportsFor(source);
final unsupportedImports =
getUnsupportedImports(imports, sourcesFileList: [kMainDart]);
getUnsupportedImports(imports, sourceFiles: {kMainDart});
if (unsupportedImports.isNotEmpty) {
return CompilationResults(problems: [
for (final import in unsupportedImports)
Expand Down Expand Up @@ -121,7 +121,7 @@ class Compiler {
Future<DDCCompilationResults> compileDDC(String source) async {
final imports = getAllImportsFor(source);
final unsupportedImports =
getUnsupportedImports(imports, sourcesFileList: [kMainDart]);
getUnsupportedImports(imports, sourceFiles: {kMainDart});
if (unsupportedImports.isNotEmpty) {
return DDCCompilationResults.failed([
for (final import in unsupportedImports)
Expand Down
2 changes: 1 addition & 1 deletion pkgs/dart_services/lib/src/project_creator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ ${_sdk.experiments.map((experiment) => ' - $experiment').join('\n')}
// `flutter packages get` has been run with a _subset_ of all supported
// Firebase packages, the ones that don't require a Firebase app to be
// configured in JavaScript, before executing Dart. Now add the full set of
// supported Firebase pacakges. This workaround is a very fragile hack.
// supported Firebase packages. This workaround is a very fragile hack.
packages = {
...supportedBasicDartPackages,
...supportedFlutterPackages,
Expand Down
131 changes: 72 additions & 59 deletions pkgs/dart_services/lib/src/project_templates.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import 'dart:io';

import 'package:analyzer/dart/ast/ast.dart';
import 'package:meta/meta.dart';
import 'package:path/path.dart' as path;

/// Sets of project template directory paths.
Expand Down Expand Up @@ -166,27 +167,33 @@ const Set<String> _allowedDartImports = {
};

/// Returns whether [imports] denote use of Flutter Web.
bool usesFlutterWeb(Iterable<ImportDirective> imports) {
return imports.any((import) {
final uriString = import.uri.stringValue;
if (uriString == null) return false;
if (uriString == 'dart:ui') return true;

final packageName = _packageNameFromPackageUri(uriString);
return packageName != null &&
_packagesIndicatingFlutter.contains(packageName);
});
bool usesFlutterWeb(Iterable<ImportDirective> imports) =>
imports.any((import) => isFlutterWebImport(import.uri.stringValue));

/// Whether the [importString] represents an import
/// that denotes use of Flutter Web.
@visibleForTesting
bool isFlutterWebImport(String? importString) {
if (importString == null) return false;
if (importString == 'dart:ui') return true;

final packageName = _packageNameFromPackageUri(importString);
return packageName != null &&
_packagesIndicatingFlutter.contains(packageName);
}

/// Returns whether [imports] denote use of Firebase.
bool usesFirebase(Iterable<ImportDirective> imports) {
return imports.any((import) {
final uriString = import.uri.stringValue;
if (uriString == null) return false;
bool usesFirebase(Iterable<ImportDirective> imports) =>
imports.any((import) => isFirebaseImport(import.uri.stringValue));

final packageName = _packageNameFromPackageUri(uriString);
return packageName != null && firebasePackages.contains(packageName);
});
/// Whether the [importString] represents an import
/// that denotes use of a Firebase package.
@visibleForTesting
bool isFirebaseImport(String? importString) {
if (importString == null) return false;

final packageName = _packageNameFromPackageUri(importString);
return packageName != null && firebasePackages.contains(packageName);
}

/// If [uriString] represents a 'package:' URI, then returns the package name;
Expand All @@ -200,54 +207,60 @@ String? _packageNameFromPackageUri(String uriString) {
}

/// Goes through imports list and returns list of unsupported imports.
///
/// Optional [sourcesFileList] contains a list of the source filenames
/// Optional [sourceFiles] contains a list of the source filenames
/// which are all part of this overall sources file set (these are to
/// be allowed).
///
/// Note: The filenames in [sourcesFileList] were sanitized of any
/// 'package:'/etc syntax as the file set arrives from the endpoint, and before
/// being passed to [getUnsupportedImports]. This is done so the list can't be
/// used to bypass unsupported imports.
/// Note: The filenames in [sourceFiles] were sanitized of any
/// 'package:'/etc syntax as the file set arrives from the endpoint, and
/// before being passed to [getUnsupportedImports].This is done so
/// the list can't be used to bypass unsupported imports.
List<ImportDirective> getUnsupportedImports(
List<ImportDirective> imports, {
List<String>? sourcesFileList,
Set<String>? sourceFiles,
}) {
return imports.where((import) {
final uriString = import.uri.stringValue;
if (uriString == null || uriString.isEmpty) {
return false;
}

// All non-VM 'dart:' imports are ok.
if (uriString.startsWith('dart:')) {
return !_allowedDartImports.contains(uriString);
}

// Filenames from within this compilation files={} sources file set
// are OK. (These filenames have been sanitized to prevent 'package:'
// (and other) prefixes, so the a filename cannot be used to bypass
// import restrictions (see comment above)).
if (sourcesFileList != null && sourcesFileList.contains(uriString)) {
return false;
}

final uri = Uri.tryParse(uriString);
if (uri == null) return false;

// We allow a specific set of package imports.
if (uri.scheme == 'package') {
if (uri.pathSegments.isEmpty) return true;
final package = uri.pathSegments.first;
return !isSupportedPackage(package);
}

// Don't allow file imports.
return true;
}).toList();
return imports
.where((import) => isUnsupportedImport(import.uri.stringValue,
sourceFiles: sourceFiles ?? const {}))
.toList(growable: false);
}

bool isSupportedPackage(String package) {
return _packagesIndicatingFlutter.contains(package) ||
supportedBasicDartPackages.contains(package);
/// Whether the [importString] represents an import
/// that is unsupported.
@visibleForTesting
bool isUnsupportedImport(
String? importString, {
Set<String> sourceFiles = const {},
}) {
if (importString == null || importString.isEmpty) {
return false;
}
// All non-VM 'dart:' imports are ok.
if (importString.startsWith('dart:')) {
return !_allowedDartImports.contains(importString);
}
// Filenames from within this compilation files={} sources file set
// are OK. (These filenames have been sanitized to prevent 'package:'
// (and other) prefixes, so the a filename cannot be used to bypass
// import restrictions (see comment above)).
if (sourceFiles.contains(importString)) {
return false;
}

final uri = Uri.tryParse(importString);
if (uri == null) return false;

// We allow a specific set of package imports.
if (uri.scheme == 'package') {
if (uri.pathSegments.isEmpty) return true;
final package = uri.pathSegments.first;
return !isSupportedPackage(package);
}

// Don't allow file imports.
return true;
}

bool isSupportedPackage(String package) =>
_packagesIndicatingFlutter.contains(package) ||
supportedBasicDartPackages.contains(package);
34 changes: 21 additions & 13 deletions pkgs/dart_services/pubspec.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 5 additions & 4 deletions pkgs/dart_services/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ environment:
sdk: ^3.2.0

dependencies:
analysis_server_lib: ^0.2.4
analyzer: ^5.13.0
analysis_server_lib: ^0.2.5
analyzer: ^6.3.0
parlough marked this conversation as resolved.
Show resolved Hide resolved
args: ^2.4.2
bazel_worker: ^1.0.2
collection: ^1.18.0
Expand All @@ -17,6 +17,7 @@ dependencies:
http: ^1.0.0
json_annotation: any
logging: ^1.2.0
meta: ^1.11.0
path: ^1.8.3
resp_client: ^1.2.0
shelf: ^1.4.1
Expand All @@ -27,10 +28,10 @@ dev_dependencies:
async: ^2.11.0
build_runner: ^2.4.5
dart_flutter_team_lints: ^2.1.1
grinder: ^0.9.4
grinder: ^0.9.5
json_serializable: any
package_config: ^2.1.0
shelf_router_generator: ^1.0.6
synchronized: ^3.1.0
test: ^1.24.3
test: ^1.24.9
test_descriptor: ^2.0.1
Loading
Loading