Make AnalysisSession.getErrors() non-nullable.

Change-Id: Ib5971e4c7327d96bf7a08b576b487833615d4ae6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/189622
Commit-Queue: Konstantin Shcheglov <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>

Author: scheglov

pkg/analyzer/CHANGELOG.md

@@ -1,5 +1,6 @@
-## 1.2.0
+## 1.2.0-dev
 * Deprecated all setters in API of AST. Use `parseString()` instead.
+* `AnalysisSession.getErrors()` does not return `null`, check its `state`.
 
 ## 1.1.0
 * Deprecated `TypeProvider.futureType2()`, `iterableType2()`, etc.

pkg/analyzer/example/analyze.dart

@@ -31,7 +31,7 @@ void main(List<String> args) async {
         continue;
       }
 
-      final errorsResult = (await context.currentSession.getErrors(filePath))!;
+      final errorsResult = await context.currentSession.getErrors(filePath);
       for (final error in errorsResult.errors) {
         if (error.errorCode.type != ErrorType.TODO) {
           print(

pkg/analyzer/lib/dart/analysis/results.dart

@@ -16,17 +16,20 @@ import 'package:analyzer/src/generated/source.dart';
 /// Clients may not extend, implement or mix-in this class.
 abstract class AnalysisResult {
   /// The absolute and normalized path of the file that was analyzed.
+  /// If [state] is not [ResultState.VALID], throws [StateError].
   ///
   /// TODO(migration): should not be nullable
   String? get path;
 
   /// Return the session used to compute this result.
+  /// If [state] is not [ResultState.VALID], throws [StateError].
   AnalysisSession get session;
 
   /// The state of the results.
   ResultState get state;
 
   /// The absolute URI of the file that was analyzed.
+  /// If [state] is not [ResultState.VALID], throws [StateError].
   Uri get uri;
 }
 
@@ -35,6 +38,7 @@ abstract class AnalysisResult {
 /// Clients may not extend, implement or mix-in this class.
 abstract class AnalysisResultWithErrors implements FileResult {
   /// The analysis errors that were computed during analysis.
+  /// If [state] is not [ResultState.VALID], throws [StateError].
   List<AnalysisError> get errors;
 }
 
@@ -69,9 +73,11 @@ abstract class ErrorsResult implements AnalysisResultWithErrors {}
 /// Clients may not extend, implement or mix-in this class.
 abstract class FileResult implements AnalysisResult {
   /// Whether the file is a part.
+  /// If [state] is not [ResultState.VALID], throws [StateError].
   bool get isPart;
 
   /// Information about lines in the content.
+  /// If [state] is not [ResultState.VALID], throws [StateError].
   LineInfo get lineInfo;
 }
 
@@ -174,6 +180,14 @@ enum ResultState {
   /// directory, or it might not represent anything.
   NOT_A_FILE,
 
+  /// An indication that analysis could not be performed because the path does
+  /// not represent the corresponding URI.
+  ///
+  /// This usually happens in Bazel workspaces, when a URI is resolved to
+  /// a generated file, but there is also a writable file to which this URI
+  /// would be resolved, if there were no generated file.
+  NOT_FILE_OF_URI,
+
   /// An indication that analysis completed normally and the results are valid.
   VALID
 }

pkg/analyzer/lib/dart/analysis/session.dart

@@ -36,9 +36,7 @@ abstract class AnalysisSession {
   ///
   /// If the file cannot be analyzed by this session, then the result will have
   /// a result state indicating the nature of the problem.
-  ///
-  /// TODO(migration): should not be nullable
-  Future<ErrorsResult?> getErrors(String path);
+  Future<ErrorsResult> getErrors(String path);
 
   /// Return information about the file at the given absolute, normalized
   /// [path].

pkg/analyzer/lib/src/dart/analysis/driver.dart

@@ -556,10 +556,10 @@ class AnalysisDriver implements AnalysisDriverGeneric {
   ///
   /// This method does not use analysis priorities, and must not be used in
   /// interactive analysis, such as Analysis Server or its plugins.
-  Future<ErrorsResult?> getErrors(String path) async {
+  Future<ErrorsResult> getErrors(String path) async {
     _throwIfNotAbsolutePath(path);
     if (!_fsState.hasUri(path)) {
-      return null;
+      return NotValidErrorsResultImpl(ResultState.NOT_FILE_OF_URI);
     }
 
     var completer = Completer<ErrorsResult>();

pkg/analyzer/lib/src/dart/analysis/results.dart

@@ -66,6 +66,56 @@ class FileResultImpl extends AnalysisResultImpl implements FileResult {
   ResultState get state => ResultState.VALID;
 }
 
+/// The implementation of [AnalysisResult] when not [ResultState.VALID].
+class NotValidAnalysisResultImpl implements AnalysisResult {
+  @override
+  final ResultState state;
+
+  NotValidAnalysisResultImpl(this.state);
+
+  @override
+  String? get path {
+    throw StateError('This result is not valid');
+  }
+
+  @override
+  AnalysisSession get session {
+    throw StateError('This result is not valid');
+  }
+
+  @override
+  Uri get uri {
+    throw StateError('This result is not valid');
+  }
+}
+
+/// The implementation of [ErrorsResult] when not [ResultState.VALID].
+class NotValidErrorsResultImpl extends NotValidFileResultImpl
+    implements ErrorsResult {
+  NotValidErrorsResultImpl(ResultState state) : super(state);
+
+  @override
+  List<AnalysisError> get errors {
+    throw StateError('This result is not valid');
+  }
+}
+
+/// The implementation of [FileResult] when not [ResultState.VALID].
+class NotValidFileResultImpl extends NotValidAnalysisResultImpl
+    implements FileResult {
+  NotValidFileResultImpl(ResultState state) : super(state);
+
+  @override
+  bool get isPart {
+    throw StateError('This result is not valid');
+  }
+
+  @override
+  LineInfo get lineInfo {
+    throw StateError('This result is not valid');
+  }
+}
+
 class ParsedLibraryResultImpl extends AnalysisResultImpl
     implements ParsedLibraryResult {
   @override

pkg/analyzer/lib/src/dart/analysis/session.dart

@@ -59,7 +59,7 @@ class AnalysisSessionImpl implements AnalysisSession {
   driver.AnalysisDriver getDriver() => _driver;
 
   @override
-  Future<ErrorsResult?> getErrors(String path) {
+  Future<ErrorsResult> getErrors(String path) {
     _checkConsistency();
     return _driver.getErrors(path);
   }

pkg/analyzer/lib/src/lint/analysis.dart

@@ -201,7 +201,7 @@ class LintDriver {
 
     List<AnalysisErrorInfo> errors = [];
     for (Source source in sources) {
-      var errorsResult = (await analysisDriver.getErrors(source.fullName))!;
+      var errorsResult = await analysisDriver.getErrors(source.fullName);
       errors.add(
           AnalysisErrorInfoImpl(errorsResult.errors, errorsResult.lineInfo));
       _sourcesAnalyzed.add(source);

pkg/analyzer/test/src/dart/analysis/driver_test.dart

@@ -1156,7 +1156,7 @@ bbb() {}
     String content = 'int f() => 42 + bar();';
     addTestFile(content, priority: true);
 
-    var result = (await driver.getErrors(testFile))!;
+    var result = await driver.getErrors(testFile);
     expect(result.path, testFile);
     expect(result.uri.toString(), 'package:test/test.dart');
     expect(result.errors, hasLength(1));
@@ -2212,7 +2212,7 @@ import 'b.dart';
     getFile(asyncPath).delete();
     addTestFile('class C {}');
 
-    ErrorsResult result = (await driver.getErrors(testFile))!;
+    ErrorsResult result = await driver.getErrors(testFile);
     expect(result.errors, hasLength(1));
 
     AnalysisError error = result.errors[0];
@@ -2224,7 +2224,7 @@ import 'b.dart';
     getFile(corePath).delete();
     addTestFile('class C {}');
 
-    ErrorsResult result = (await driver.getErrors(testFile))!;
+    ErrorsResult result = await driver.getErrors(testFile);
     expect(result.errors, hasLength(1));
 
     AnalysisError error = result.errors[0];
@@ -2452,13 +2452,13 @@ var b = new B();
 
     // Process a.dart so that we know that it's a library for c.dart later.
     {
-      ErrorsResult result = (await driver.getErrors(a))!;
+      ErrorsResult result = await driver.getErrors(a);
       expect(result.errors, isEmpty);
     }
 
     // c.dart does not have errors in the context of a.dart
     {
-      ErrorsResult result = (await driver.getErrors(c))!;
+      ErrorsResult result = await driver.getErrors(c);
       expect(result.errors, isEmpty);
     }
   }
@@ -2488,7 +2488,7 @@ var b = new B();
 
     // c.dart is resolve in the context of a.dart, so have no errors
     {
-      ErrorsResult result = (await driver.getErrors(c))!;
+      ErrorsResult result = await driver.getErrors(c);
       expect(result.errors, isEmpty);
     }
   }

pkg/analyzer/test/src/dart/analysis/session_test.dart

@@ -4,6 +4,7 @@
 
 import 'package:analyzer/dart/analysis/analysis_context.dart';
 import 'package:analyzer/dart/analysis/analysis_context_collection.dart';
+import 'package:analyzer/dart/analysis/results.dart';
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/src/dart/analysis/analysis_context_collection.dart';
 import 'package:analyzer/src/dart/analysis/session.dart';
@@ -13,12 +14,44 @@ import 'package:analyzer/src/test_utilities/resource_provider_mixin.dart';
 import 'package:test/test.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
+import '../resolution/context_collection_resolution.dart';
+
 main() {
   defineReflectiveSuite(() {
     defineReflectiveTests(AnalysisSessionImplTest);
+    defineReflectiveTests(AnalysisSessionImpl_BazelWorkspaceTest);
   });
 }
 
+@reflectiveTest
+class AnalysisSessionImpl_BazelWorkspaceTest
+    extends BazelWorkspaceResolutionTest {
+  void test_getErrors_notFileOfUri() async {
+    var relPath = 'dart/my/lib/a.dart';
+    newFile('$workspaceRootPath/bazel-bin/$relPath');
+
+    var path = '$workspaceRootPath/$relPath';
+    var session = contextFor(path).currentSession;
+    var result = await session.getErrors(path);
+    expect(result.state, ResultState.NOT_FILE_OF_URI);
+    expect(() => result.errors, throwsStateError);
+  }
+
+  void test_getErrors_valid() async {
+    var file = newFile(
+      '$workspaceRootPath/dart/my/lib/a.dart',
+      content: 'var x = 0',
+    );
+
+    var session = contextFor(file.path).currentSession;
+    var result = await session.getErrors(file.path);
+    expect(result.state, ResultState.VALID);
+    expect(result.path, file.path);
+    expect(result.errors, hasLength(1));
+    expect(result.uri.toString(), 'package:dart.my/a.dart');
+  }
+}
+
 @reflectiveTest
 class AnalysisSessionImplTest with ResourceProviderMixin {
   late final AnalysisContextCollection contextCollection;
@@ -55,7 +88,7 @@ test:lib/
 
   test_getErrors() async {
     newFile(testPath, content: 'class C {');
-    var errorsResult = (await session.getErrors(testPath))!;
+    var errorsResult = await session.getErrors(testPath);
     expect(errorsResult.session, session);
     expect(errorsResult.path, testPath);
     expect(errorsResult.errors, isNotEmpty);