allow analysis of snippets irregardless of imports (#2744)

pkgs/dart_services/lib/src/analysis.dart

@@ -6,18 +6,16 @@ import 'dart:async';
 import 'dart:io';
 
 import 'package:analysis_server_lib/analysis_server_lib.dart';
-import 'package:analyzer/dart/ast/ast.dart';
 import 'package:logging/logging.dart';
 import 'package:path/path.dart' as path;
 
 import 'common.dart';
-import 'common_server.dart';
-import 'project_templates.dart' as project;
 import 'project_templates.dart';
 import 'pub.dart';
 import 'sdk.dart';
 import 'shared/model.dart' as api;
 import 'utils.dart' as utils;
+import 'utils.dart';
 
 final Logger _logger = Logger('analysis_server');
 
@@ -41,24 +39,14 @@ class Analyzer {
   }
 
   Future<api.AnalysisResponse> analyze(String source) async {
-    final imports = getAllImportsFor(source);
-
-    // TODO(srawlins): Do the work so that each unsupported input is its own
-    // error with a proper SourceSpan.
-    await _checkPackageReferences(imports);
-
-    return analysisServer.analyze(source, imports: imports);
+    return analysisServer.analyze(source);
   }
 
   Future<api.CompleteResponse> complete(String source, int offset) async {
-    await _checkPackageReferences(getAllImportsFor(source));
-
     return analysisServer.complete(source, offset);
   }
 
   Future<api.FixesResponse> fixes(String source, int offset) async {
-    await _checkPackageReferences(getAllImportsFor(source));
-
     return analysisServer.fixes(source, offset);
   }
 
@@ -67,26 +55,9 @@ class Analyzer {
   }
 
   Future<api.DocumentResponse> dartdoc(String source, int offset) async {
-    await _checkPackageReferences(getAllImportsFor(source));
-
     return analysisServer.dartdoc(source, offset);
   }
 
-  /// Check that the set of packages referenced is valid.
-  Future<void> _checkPackageReferences(List<ImportDirective> imports) async {
-    final unsupportedImports = project.getUnsupportedImports(
-      imports,
-      sourceFiles: {kMainDart},
-    );
-
-    if (unsupportedImports.isNotEmpty) {
-      final unsupportedUris =
-          unsupportedImports.map((import) => import.uri.stringValue).toList();
-      final plural = unsupportedUris.length == 1 ? 'import' : 'imports';
-      throw BadRequest('unsupported $plural ${unsupportedUris.join(', ')}');
-    }
-  }
-
   Future<void> shutdown() {
     return analysisServer.shutdown();
   }
@@ -282,10 +253,7 @@ class AnalysisServerWrapper {
     );
   }
 
-  Future<api.AnalysisResponse> analyze(
-    String source, {
-    List<ImportDirective>? imports,
-  }) async {
+  Future<api.AnalysisResponse> analyze(String source) async {
     final sources = _getOverlayMapWithPaths({kMainDart: source});
     await _loadSources(sources);
 
@@ -339,12 +307,57 @@ class AnalysisServerWrapper {
       return a.location.charStart.compareTo(b.location.charStart);
     });
 
-    // Ensure we have imports if they were not passed in.
-    imports ??= getAllImportsFor(source);
+    final imports = getAllImportsFor(source);
+    final importIssues = <api.AnalysisIssue>[];
+
+    for (final import in imports) {
+      final start = import.firstTokenAfterCommentAndMetadata;
+      final end = import.endToken;
+
+      Lines? lines;
+
+      if (import.dartImport) {
+        // ignore dart: imports.
+      } else if (import.packageImport) {
+        if (!isSupportedPackage(import.packageName)) {
+          lines ??= Lines(source);
+
+          importIssues.add(api.AnalysisIssue(
+            kind: 'warning',
+            message: "Unsupported package: 'package:${import.packageName}'.",
+            location: api.Location(
+              charStart: start.charOffset,
+              charLength: end.charEnd - start.charOffset,
+              line: lines.lineForOffset(start.charOffset),
+              column: lines.columnForOffset(start.charOffset),
+            ),
+          ));
+        }
+      } else {
+        lines ??= Lines(source);
+
+        importIssues.add(api.AnalysisIssue(
+          kind: 'error',
+          message: 'Import type not supported.',
+          location: api.Location(
+            charStart: start.charOffset,
+            charLength: end.charEnd - start.charOffset,
+            line: lines.lineForOffset(start.charOffset),
+            column: lines.columnForOffset(start.charOffset),
+          ),
+        ));
+      }
+    }
 
     return api.AnalysisResponse(
-      issues: issues,
-      packageImports: filterSafePackages(imports),
+      issues: [
+        ...importIssues,
+        ...issues,
+      ],
+      packageImports: imports
+          .where((import) => import.packageImport)
+          .map((import) => import.packageName)
+          .toList(),
     );
   }
 

pkgs/dart_services/lib/src/pub.dart

@@ -67,13 +67,10 @@ Map<String, String> packageVersionsFromPubspecLock(String templatePath) {
   return packageVersions;
 }
 
-/// Returns the names of packages that are referenced in this collection.
-/// These package names are sanitized defensively.
-List<String> filterSafePackages(List<ImportDirective> imports) {
-  return imports
-      .where((import) => !import.uri.stringValue!.startsWith('package:../'))
-      .map((import) => Uri.parse(import.uri.stringValue!))
-      .where((uri) => uri.scheme == 'package' && uri.pathSegments.isNotEmpty)
-      .map((uri) => uri.pathSegments.first)
-      .toList();
+extension ImportDirectiveExtension on ImportDirective {
+  bool get dartImport => Uri.parse(uri.stringValue!).scheme == 'dart';
+
+  bool get packageImport => Uri.parse(uri.stringValue!).scheme == 'package';
+
+  String get packageName => Uri.parse(uri.stringValue!).pathSegments.first;
 }

pkgs/dart_services/lib/src/utils.dart

@@ -134,3 +134,37 @@ class ClosureTask<T> extends SchedulerTask<T> {
 /// This pattern is essentially "possibly some letters and colons, followed by a
 /// slash, followed by non-whitespace."
 final _possiblePathPattern = RegExp(r'[a-zA-Z:]*\/\S*');
+
+class Lines {
+  final _starts = <int>[];
+
+  Lines(String source) {
+    final units = source.codeUnits;
+    var nextIsEol = true;
+    for (var i = 0; i < units.length; i++) {
+      if (nextIsEol) {
+        nextIsEol = false;
+        _starts.add(i);
+      }
+      if (units[i] == 10) nextIsEol = true;
+    }
+  }
+
+  /// Return the 1-based line number.
+  int lineForOffset(int offset) {
+    if (_starts.isEmpty) return 1;
+    for (var i = 1; i < _starts.length; i++) {
+      if (offset < _starts[i]) return i;
+    }
+    return _starts.length;
+  }
+
+  /// Return the 1-based column number.
+  int columnForOffset(int offset) {
+    if (_starts.isEmpty) return 1;
+    for (var i = _starts.length - 1; i >= 0; i--) {
+      if (offset >= _starts[i]) return offset - _starts[i] + 1;
+    }
+    return 1;
+  }
+}

pkgs/dart_services/test/pub_test.dart

@@ -70,58 +70,5 @@ void main() { }
             ]));
       });
     });
-
-    group('filterSafePackagesFromImports', () {
-      test('empty', () {
-        const source = '''import 'package:';
-void main() { }
-''';
-        expect(filterSafePackages(getAllImportsFor(source)), isEmpty);
-      });
-
-      test('simple', () {
-        const source = '''
-import 'package:foo/foo.dart';
-import 'package:bar/bar.dart';
-void main() { }
-''';
-        expect(filterSafePackages(getAllImportsFor(source)),
-            unorderedEquals(['foo', 'bar']));
-      });
-
-      test('defensive', () {
-        const source = '''
-library woot;
-import 'dart:math';
-import 'package:../foo/foo.dart';
-void main() { }
-''';
-        final imports = getAllImportsFor(source);
-        expect(imports, hasLength(2));
-        expect(imports[0].uri.stringValue, equals('dart:math'));
-        expect(imports[1].uri.stringValue, equals('package:../foo/foo.dart'));
-        expect(filterSafePackages(imports), isEmpty);
-      });
-
-      test('negative dart import', () {
-        const source = '''
-import 'dart:../bar.dart';
-''';
-        final imports = getAllImportsFor(source);
-        expect(imports, hasLength(1));
-        expect(imports.single.uri.stringValue, equals('dart:../bar.dart'));
-        expect(filterSafePackages(imports), isEmpty);
-      });
-
-      test('negative path import', () {
-        const source = '''
-import '../foo.dart';
-''';
-        final imports = getAllImportsFor(source);
-        expect(imports, hasLength(1));
-        expect(imports.single.uri.stringValue, equals('../foo.dart'));
-        expect(filterSafePackages(imports), isEmpty);
-      });
-    });
   });
 }

pkgs/dart_services/test/server_v3_test.dart → pkgs/dart_services/test/server_test.dart

@@ -12,7 +12,7 @@ import 'package:test/test.dart';
 void main() => defineTests();
 
 void defineTests() {
-  group('server v3', () {
+  group('server', () {
     late final Sdk sdk;
     late final EndpointsServer server;
     late final Client httpClient;
@@ -98,6 +98,22 @@ void main() {
       expect(issue.location.line, 2);
     });
 
+    test('analyze unsupported import', () async {
+      final result = await client.analyze(SourceRequest(source: r'''
+import 'package:foo_bar/foo_bar.dart';
+
+void main() => print('hello world');
+'''));
+
+      expect(result.issues, isNotEmpty);
+
+      final issue = result.issues.first;
+      expect(issue.kind, 'warning');
+      expect(
+          issue.message, contains("Unsupported package: 'package:foo_bar'."));
+      expect(issue.location.line, 1);
+    });
+
     test('complete', () async {
       final result = await client.complete(SourceRequest(source: '''
 void main() {

pkgs/dart_services/test/utils_test.dart

@@ -62,4 +62,42 @@ void defineTests() {
       );
     });
   });
+
+  group('Lines', () {
+    test('empty string', () {
+      final lines = Lines('');
+      expect(lines.lineForOffset(0), 1);
+      expect(lines.lineForOffset(1), 1);
+      expect(lines.columnForOffset(0), 1);
+      expect(lines.columnForOffset(1), 1);
+    });
+
+    test('lineForOffset', () {
+      final lines = Lines('one\ntwo\nthree');
+      expect(lines.lineForOffset(0), 1);
+      expect(lines.lineForOffset(1), 1);
+      expect(lines.lineForOffset(2), 1);
+      expect(lines.lineForOffset(3), 1);
+      expect(lines.lineForOffset(4), 2);
+      expect(lines.lineForOffset(5), 2);
+      expect(lines.lineForOffset(6), 2);
+      expect(lines.lineForOffset(7), 2);
+      expect(lines.lineForOffset(8), 3);
+      expect(lines.lineForOffset(9), 3);
+      expect(lines.lineForOffset(10), 3);
+      expect(lines.lineForOffset(11), 3);
+      expect(lines.lineForOffset(12), 3);
+      expect(lines.lineForOffset(13), 3);
+
+      expect(lines.lineForOffset(14), 3);
+    });
+
+    test('columnForOffset', () {
+      final lines = Lines('one\ntwo\nthree');
+      expect(lines.columnForOffset(0), 1);
+      expect(lines.columnForOffset(1), 2);
+      expect(lines.columnForOffset(2), 3);
+      expect(lines.columnForOffset(3), 4);
+    });
+  });
 }