ErrorReporter should know isNonNullableByDefault.

So, when we call DartType.getDisplayString() we know if the type presentation should include nullability suffixes. This fixes many, but not all places where we need to present types. There are places where we don't go through ErrorReporter, but call ErrorListener directly. I plan to replace all these with ErrorReporter. R=brianwilkerson@google.com, paulberry@google.com Bug: #39651 Change-Id: Ic77a556e7834d8f757c8b13eed37ed1d34f47348 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/127744 Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
dart-lang · Dec 10, 2019 · 9456316 · 9456316
1 parent 02a8b01
commit 9456316
Show file tree

Hide file tree

Showing 19 changed files with 190 additions and 50 deletions.
diff --git a/pkg/analysis_server/test/services/linter/linter_test.dart b/pkg/analysis_server/test/services/linter/linter_test.dart
@@ -31,7 +31,11 @@ class LinterRuleOptionsValidatorTest {
   setUp() {
     registerLintRules();
     recorder = new RecordingErrorListener();
-    reporter = new ErrorReporter(recorder, new _TestSource());
+    reporter = new ErrorReporter(
+      recorder,
+      new _TestSource(),
+      isNonNullableByDefault: false,
+    );
   }
 
   test_linter_defined_rules() {

diff --git a/pkg/analyzer/lib/error/listener.dart b/pkg/analyzer/lib/error/listener.dart
@@ -67,6 +67,11 @@ class ErrorReporter {
    */
   final Source _defaultSource;
 
+  /**
+   * Is `true` if the library being analyzed is non-nullable by default.
+   */
+  final bool isNonNullableByDefault;
+
   /**
    * The source to be used when reporting errors.
    */
@@ -77,7 +82,8 @@ class ErrorReporter {
    * given [_errorListener]. Errors will be reported against the
    * [_defaultSource] unless another source is provided later.
    */
-  ErrorReporter(this._errorListener, this._defaultSource) {
+  ErrorReporter(this._errorListener, this._defaultSource,
+      {this.isNonNullableByDefault = false}) {
     if (_errorListener == null) {
       throw ArgumentError("An error listener must be provided");
     } else if (_defaultSource == null) {
@@ -204,13 +210,13 @@ class ErrorReporter {
           buffer.write(name);
           (type as TypeImpl).appendTo(
             buffer,
-            withNullability: false,
+            withNullability: isNonNullableByDefault,
             skipAllDynamicArguments: false,
           );
           return buffer.toString();
         }
       }
-      return type.getDisplayString(withNullability: false);
+      return type.getDisplayString(withNullability: isNonNullableByDefault);
     }
 
     Map<String, List<_TypeToConvert>> typeGroups = {};

diff --git a/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart b/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart
@@ -138,16 +138,16 @@ class LibraryAnalyzer {
     }
     timerLibraryAnalyzerFreshUnit.stop();
 
+    _libraryElement = _elementFactory.libraryOfUri(_library.uriStr);
+    _libraryScope = LibraryScope(_libraryElement);
+
     // Resolve URIs in directives to corresponding sources.
     FeatureSet featureSet = units[_library].featureSet;
     units.forEach((file, unit) {
       _validateFeatureSet(unit, featureSet);
       _resolveUriBasedDirectives(file, unit);
     });
 
-    _libraryElement = _elementFactory.libraryOfUri(_library.uriStr);
-    _libraryScope = LibraryScope(_libraryElement);
-
     timerLibraryAnalyzerResolve.start();
     _resolveDirectives(units);
 
@@ -465,7 +465,11 @@ class LibraryAnalyzer {
   ErrorReporter _getErrorReporter(FileState file) {
     return _errorReporters.putIfAbsent(file, () {
       RecordingErrorListener listener = _getErrorListener(file);
-      return ErrorReporter(listener, file.source);
+      return ErrorReporter(
+        listener,
+        file.source,
+        isNonNullableByDefault: _libraryElement.isNonNullableByDefault,
+      );
     });
   }
 

diff --git a/pkg/analyzer/lib/src/dart/constant/constant_verifier.dart b/pkg/analyzer/lib/src/dart/constant/constant_verifier.dart
@@ -458,8 +458,11 @@ class ConstantVerifier extends RecursiveAstVisitor<void> {
   /// @return the value of the compile time constant
   DartObjectImpl _validate(Expression expression, ErrorCode errorCode) {
     RecordingErrorListener errorListener = RecordingErrorListener();
-    ErrorReporter subErrorReporter =
-        ErrorReporter(errorListener, _errorReporter.source);
+    ErrorReporter subErrorReporter = ErrorReporter(
+      errorListener,
+      _errorReporter.source,
+      isNonNullableByDefault: _currentLibrary.isNonNullableByDefault,
+    );
     DartObjectImpl result =
         expression.accept(ConstantVisitor(_evaluationEngine, subErrorReporter));
     _reportErrors(errorListener.errors, errorCode);
@@ -550,8 +553,11 @@ class ConstantVerifier extends RecursiveAstVisitor<void> {
             // can't be evaluated we'll just report a single error.
             AnalysisErrorListener errorListener =
                 AnalysisErrorListener.NULL_LISTENER;
-            ErrorReporter subErrorReporter =
-                ErrorReporter(errorListener, _errorReporter.source);
+            ErrorReporter subErrorReporter = ErrorReporter(
+              errorListener,
+              _errorReporter.source,
+              isNonNullableByDefault: _currentLibrary.isNonNullableByDefault,
+            );
             DartObjectImpl result = initializer
                 .accept(ConstantVisitor(_evaluationEngine, subErrorReporter));
             if (result == null) {

diff --git a/pkg/analyzer/lib/src/dart/constant/evaluation.dart b/pkg/analyzer/lib/src/dart/constant/evaluation.dart
@@ -98,6 +98,10 @@ class ConstantEvaluationEngine {
             ),
         experimentStatus = experimentStatus ?? ExperimentStatus();
 
+  bool get _isNonNullableByDefault {
+    return (typeSystem as TypeSystemImpl).isNonNullableByDefault;
+  }
+
   /// Check that the arguments to a call to fromEnvironment() are correct. The
   /// [arguments] are the AST nodes of the arguments. The [argumentValues] are
   /// the values of the unnamed arguments. The [namedArgumentValues] are the
@@ -169,8 +173,11 @@ class ConstantEvaluationEngine {
         Expression defaultValue = constant.constantInitializer;
         if (defaultValue != null) {
           RecordingErrorListener errorListener = RecordingErrorListener();
-          ErrorReporter errorReporter =
-              ErrorReporter(errorListener, constant.source);
+          ErrorReporter errorReporter = ErrorReporter(
+            errorListener,
+            constant.source,
+            isNonNullableByDefault: _isNonNullableByDefault,
+          );
           DartObjectImpl dartObject =
               defaultValue.accept(ConstantVisitor(this, errorReporter));
           constant.evaluationResult =
@@ -184,8 +191,11 @@ class ConstantEvaluationEngine {
       Expression constantInitializer = constant.constantInitializer;
       if (constantInitializer != null) {
         RecordingErrorListener errorListener = RecordingErrorListener();
-        ErrorReporter errorReporter =
-            ErrorReporter(errorListener, constant.source);
+        ErrorReporter errorReporter = ErrorReporter(
+          errorListener,
+          constant.source,
+          isNonNullableByDefault: _isNonNullableByDefault,
+        );
         DartObjectImpl dartObject =
             constantInitializer.accept(ConstantVisitor(this, errorReporter));
         // Only check the type for truly const declarations (don't check final
@@ -235,8 +245,11 @@ class ConstantEvaluationEngine {
           element.isConst &&
           constNode.arguments != null) {
         RecordingErrorListener errorListener = RecordingErrorListener();
-        ErrorReporter errorReporter =
-            ErrorReporter(errorListener, constant.source);
+        ErrorReporter errorReporter = ErrorReporter(
+          errorListener,
+          constant.source,
+          isNonNullableByDefault: _isNonNullableByDefault,
+        );
         ConstantVisitor constantVisitor = ConstantVisitor(this, errorReporter);
         DartObjectImpl result = evaluateConstructorCall(
             constNode,
@@ -551,8 +564,11 @@ class ConstantEvaluationEngine {
     // different source. But they still should cause a constant evaluation
     // error for the current node.
     var externalErrorListener = BooleanErrorListener();
-    var externalErrorReporter =
-        ErrorReporter(externalErrorListener, constructor.source);
+    var externalErrorReporter = ErrorReporter(
+      externalErrorListener,
+      constructor.source,
+      isNonNullableByDefault: _isNonNullableByDefault,
+    );
 
     // Start with final fields that are initialized at their declaration site.
     List<FieldElement> fields = constructor.enclosingElement.fields;
@@ -811,8 +827,11 @@ class ConstantEvaluationEngine {
       ConstantEvaluationTarget constant) {
     if (constant is VariableElement) {
       RecordingErrorListener errorListener = RecordingErrorListener();
-      ErrorReporter errorReporter =
-          ErrorReporter(errorListener, constant.source);
+      ErrorReporter errorReporter = ErrorReporter(
+        errorListener,
+        constant.source,
+        isNonNullableByDefault: _isNonNullableByDefault,
+      );
       // TODO(paulberry): It would be really nice if we could extract enough
       // information from the 'cycle' argument to provide the user with a
       // description of the cycle.

diff --git a/pkg/analyzer/lib/src/dart/resolver/resolution_visitor.dart b/pkg/analyzer/lib/src/dart/resolver/resolution_visitor.dart
@@ -97,7 +97,11 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
     var typeProvider = libraryElement.typeProvider;
     var unitSource = unitElement.source;
     var isNonNullableByDefault = featureSet.isEnabled(Feature.non_nullable);
-    var errorReporter = ErrorReporter(errorListener, unitSource);
+    var errorReporter = ErrorReporter(
+      errorListener,
+      unitSource,
+      isNonNullableByDefault: isNonNullableByDefault,
+    );
 
     var typeNameResolver = TypeNameResolver(
       libraryElement.typeSystem,

diff --git a/pkg/analyzer/lib/src/generated/constant.dart b/pkg/analyzer/lib/src/generated/constant.dart
@@ -129,7 +129,11 @@ class ConstantEvaluator {
 
   EvaluationResult evaluate(Expression expression) {
     RecordingErrorListener errorListener = RecordingErrorListener();
-    ErrorReporter errorReporter = ErrorReporter(errorListener, _source);
+    ErrorReporter errorReporter = ErrorReporter(
+      errorListener,
+      _source,
+      isNonNullableByDefault: _typeSystem.isNonNullableByDefault,
+    );
     DartObjectImpl result = expression.accept(ConstantVisitor(
         ConstantEvaluationEngine(_typeProvider, DeclaredVariables(),
             typeSystem: _typeSystem),

diff --git a/pkg/analyzer/lib/src/generated/parser_fasta.dart b/pkg/analyzer/lib/src/generated/parser_fasta.dart
@@ -380,7 +380,11 @@ class _Parser2 extends ParserAdapter {
   factory _Parser2(
       Source source, AnalysisErrorListener errorListener, FeatureSet featureSet,
       {bool allowNativeClause = false}) {
-    var errorReporter = ErrorReporter(errorListener, source);
+    var errorReporter = ErrorReporter(
+      errorListener,
+      source,
+      isNonNullableByDefault: featureSet.isEnabled(Feature.non_nullable),
+    );
     return _Parser2._(source, errorReporter, source.uri, featureSet,
         allowNativeClause: allowNativeClause);
   }

diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart
@@ -2729,7 +2729,11 @@ abstract class ScopedVisitor extends UnifyingAstVisitor<void> {
       AnalysisErrorListener errorListener,
       {Scope nameScope})
       : source = source,
-        errorReporter = ErrorReporter(errorListener, source) {
+        errorReporter = ErrorReporter(
+          errorListener,
+          source,
+          isNonNullableByDefault: definingLibrary.isNonNullableByDefault,
+        ) {
     if (nameScope == null) {
       this.nameScope = LibraryScope(definingLibrary);
     } else {

diff --git a/pkg/analyzer/lib/src/lint/linter.dart b/pkg/analyzer/lib/src/lint/linter.dart
@@ -114,7 +114,11 @@ class DartLinter implements AnalysisErrorListener {
           // processing gets pushed down, this hack can go away.)
           if (rule.reporter == null && sourceUrl != null) {
             var source = createSource(sourceUrl);
-            rule.reporter = ErrorReporter(this, source);
+            rule.reporter = ErrorReporter(
+              this,
+              source,
+              isNonNullableByDefault: false,
+            );
           }
           try {
             spec.accept(visitor);
@@ -343,15 +347,24 @@ class LinterContextImpl implements LinterContext {
 
   @override
   LinterConstantEvaluationResult evaluateConstant(Expression node) {
-    var source = currentUnit.unit.declaredElement.source;
+    var unitElement = currentUnit.unit.declaredElement;
+    var source = unitElement.source;
+    var libraryElement = unitElement.library;
+
     var errorListener = RecordingErrorListener();
+    var errorReporter = ErrorReporter(
+      errorListener,
+      source,
+      isNonNullableByDefault: libraryElement.isNonNullableByDefault,
+    );
+
     var visitor = ConstantVisitor(
       ConstantEvaluationEngine(
         typeProvider,
         declaredVariables,
         typeSystem: typeSystem,
       ),
-      ErrorReporter(errorListener, source),
+      errorReporter,
     );
 
     var value = node.accept(visitor);
@@ -364,7 +377,11 @@ class LinterContextImpl implements LinterContext {
     var libraryElement = unitElement.library;
 
     var listener = ConstantAnalysisErrorListener();
-    var errorReporter = ErrorReporter(listener, unitElement.source);
+    var errorReporter = ErrorReporter(
+      listener,
+      unitElement.source,
+      isNonNullableByDefault: libraryElement.isNonNullableByDefault,
+    );
 
     node.accept(
       ConstantVerifier(
@@ -628,7 +645,11 @@ class SourceLinter implements DartLinter, AnalysisErrorListener {
           // processing gets pushed down, this hack can go away.)
           if (rule.reporter == null && sourceUrl != null) {
             var source = createSource(sourceUrl);
-            rule.reporter = ErrorReporter(this, source);
+            rule.reporter = ErrorReporter(
+              this,
+              source,
+              isNonNullableByDefault: false,
+            );
           }
           try {
             spec.accept(visitor);

diff --git a/pkg/analyzer/lib/src/manifest/manifest_validator.dart b/pkg/analyzer/lib/src/manifest/manifest_validator.dart
@@ -29,7 +29,11 @@ class ManifestValidator {
    */
   List<AnalysisError> validate(String contents, bool checkManifest) {
     RecordingErrorListener recorder = RecordingErrorListener();
-    ErrorReporter reporter = ErrorReporter(recorder, source);
+    ErrorReporter reporter = ErrorReporter(
+      recorder,
+      source,
+      isNonNullableByDefault: false,
+    );
 
     if (checkManifest) {
       var document =

diff --git a/pkg/analyzer/lib/src/pubspec/pubspec_validator.dart b/pkg/analyzer/lib/src/pubspec/pubspec_validator.dart
@@ -60,7 +60,11 @@ class PubspecValidator {
    */
   List<AnalysisError> validate(Map<dynamic, YamlNode> contents) {
     RecordingErrorListener recorder = RecordingErrorListener();
-    ErrorReporter reporter = ErrorReporter(recorder, source);
+    ErrorReporter reporter = ErrorReporter(
+      recorder,
+      source,
+      isNonNullableByDefault: false,
+    );
 
     _validateDependencies(reporter, contents);
     _validateFlutter(reporter, contents);

diff --git a/pkg/analyzer/lib/src/task/options.dart b/pkg/analyzer/lib/src/task/options.dart
@@ -425,7 +425,11 @@ class OptionsFileValidator {
 
   List<AnalysisError> validate(YamlMap options) {
     RecordingErrorListener recorder = RecordingErrorListener();
-    ErrorReporter reporter = ErrorReporter(recorder, source);
+    ErrorReporter reporter = ErrorReporter(
+      recorder,
+      source,
+      isNonNullableByDefault: false,
+    );
     if (AnalysisEngine.ANALYSIS_OPTIONS_FILE == source.shortName) {
       reporter.reportError(AnalysisError(
           source,