[analyzer] Stop using Diagnostic.data and deprecate it.

The only two uses of `Diagnostic.data` have been replaced with
expandos keyed off the `Diagnostic` object. To make this possible, the
`at...` methods of `DiagnosticReporter` have been changed to return
the `Diagnostic` object they create. This paves the way for a future
CL series, in which I plan to change the API for reporting errors to a
more literate API in which the caller will have access to the
`Diagnostic` object.

Change-Id: I40820387abcd53daf934974c93106803d706fe07
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/444461
Commit-Queue: Paul Berry <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>
Reviewed-by: Samuel Rawlins <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>

Author: stereotype441

pkg/analysis_server/lib/src/services/correction/dart/add_missing_switch_cases.dart

@@ -32,8 +32,8 @@ class AddMissingSwitchCases extends ResolvedCorrectionProducer {
       return;
     }
 
-    var patternPartsList = diagnostic.data;
-    if (patternPartsList is! List<List<MissingPatternPart>>) {
+    var patternPartsList = MissingPatternPart.byDiagnostic[diagnostic];
+    if (patternPartsList == null) {
       return;
     }
 

pkg/analysis_server/lib/src/services/correction/fix/pubspec/fix_generator.dart

@@ -154,7 +154,7 @@ class PubspecFixGenerator {
       defaultEol: endOfLine,
     );
 
-    var data = diagnostic.data as MissingDependencyData;
+    var data = MissingDependencyData.byDiagnostic[diagnostic]!;
     var addDeps = data.addDeps;
     var addDevDeps = data.addDevDeps;
     var removeDevDeps = data.removeDevDeps;

pkg/analyzer/CHANGELOG.md

@@ -22,6 +22,8 @@
 * Deprecate `ClassElement.isImplementableIn`; use `isImplementableOutside` instead.
 * Deprecate `ClassElement.isMixableIn`; use `isMixableOutside` instead.
 * Deprecate `MixinElement.isImplementableIn`; use `isImplementableOutside` instead.
+* Deprecate `Diagnostic.data`. Instead, please use an expando to attach data to
+  the `Diagnostic` object.
 
 ## 8.0.0
 * Remove deprecated element model V1.

pkg/analyzer/api.txt

@@ -4763,8 +4763,8 @@ package:analyzer/dart/sdk/build_sdk_summary.dart:
   buildSdkSummary (function: Future<Uint8List> Function({String? embedderYamlPath, required ResourceProvider resourceProvider, required String sdkPath}))
 package:analyzer/diagnostic/diagnostic.dart:
   Diagnostic (class extends Object):
-    forValues (constructor: Diagnostic Function({List<DiagnosticMessage> contextMessages, String? correctionMessage, Object? data, DiagnosticCode? diagnosticCode, deprecated DiagnosticCode? errorCode, required int length, required String message, required int offset, required Source source}))
-    tmp (constructor: Diagnostic Function({List<Object?> arguments, List<DiagnosticMessage> contextMessages, Object? data, DiagnosticCode? diagnosticCode, deprecated DiagnosticCode? errorCode, required int length, required int offset, required Source source}))
+    forValues (constructor: Diagnostic Function({List<DiagnosticMessage> contextMessages, String? correctionMessage, deprecated Object? data, DiagnosticCode? diagnosticCode, deprecated DiagnosticCode? errorCode, required int length, required String message, required int offset, required Source source}))
+    tmp (constructor: Diagnostic Function({List<Object?> arguments, List<DiagnosticMessage> contextMessages, deprecated Object? data, DiagnosticCode? diagnosticCode, deprecated DiagnosticCode? errorCode, required int length, required int offset, required Source source}))
     contextMessages (getter: List<DiagnosticMessage>)
     correction (getter: String?, deprecated)
     correctionMessage (getter: String?)
@@ -4874,13 +4874,13 @@ package:analyzer/error/listener.dart:
     lockLevel (getter: int)
     lockLevel= (setter: int)
     source (getter: Source)
-    atConstructorDeclaration (method: void Function(ConstructorDeclaration, DiagnosticCode, {List<Object>? arguments, List<DiagnosticMessage>? contextMessages, Object? data}))
-    atElement2 (method: void Function(Element, DiagnosticCode, {List<Object>? arguments, List<DiagnosticMessage>? contextMessages, Object? data}), experimental)
-    atEntity (method: void Function(SyntacticEntity, DiagnosticCode, {List<Object>? arguments, List<DiagnosticMessage>? contextMessages, Object? data}))
-    atNode (method: void Function(AstNode, DiagnosticCode, {List<Object>? arguments, List<DiagnosticMessage>? contextMessages, Object? data}))
-    atOffset (method: void Function({List<Object>? arguments, List<DiagnosticMessage>? contextMessages, Object? data, DiagnosticCode? diagnosticCode, deprecated DiagnosticCode? errorCode, required int length, required int offset}))
-    atSourceSpan (method: void Function(SourceSpan, DiagnosticCode, {List<Object>? arguments, List<DiagnosticMessage>? contextMessages, Object? data}))
-    atToken (method: void Function(Token, DiagnosticCode, {List<Object>? arguments, List<DiagnosticMessage>? contextMessages, Object? data}))
+    atConstructorDeclaration (method: Diagnostic Function(ConstructorDeclaration, DiagnosticCode, {List<Object>? arguments, List<DiagnosticMessage>? contextMessages, deprecated Object? data}))
+    atElement2 (method: Diagnostic Function(Element, DiagnosticCode, {List<Object>? arguments, List<DiagnosticMessage>? contextMessages, deprecated Object? data}), experimental)
+    atEntity (method: Diagnostic Function(SyntacticEntity, DiagnosticCode, {List<Object>? arguments, List<DiagnosticMessage>? contextMessages, deprecated Object? data}))
+    atNode (method: Diagnostic Function(AstNode, DiagnosticCode, {List<Object>? arguments, List<DiagnosticMessage>? contextMessages, deprecated Object? data}))
+    atOffset (method: Diagnostic Function({List<Object>? arguments, List<DiagnosticMessage>? contextMessages, deprecated Object? data, DiagnosticCode? diagnosticCode, deprecated DiagnosticCode? errorCode, required int length, required int offset}))
+    atSourceSpan (method: Diagnostic Function(SourceSpan, DiagnosticCode, {List<Object>? arguments, List<DiagnosticMessage>? contextMessages, deprecated Object? data}))
+    atToken (method: Diagnostic Function(Token, DiagnosticCode, {List<Object>? arguments, List<DiagnosticMessage>? contextMessages, deprecated Object? data}))
     reportError (method: void Function(Diagnostic))
   RecordingDiagnosticListener (class extends Object implements AnalysisErrorListener, DiagnosticListener):
     new (constructor: RecordingDiagnosticListener Function())

pkg/analyzer/lib/diagnostic/diagnostic.dart

@@ -24,6 +24,7 @@ class Diagnostic {
   final List<DiagnosticMessage> contextMessages;
 
   /// Data associated with this diagnostic, specific for [diagnosticCode].
+  @Deprecated('Use an expando instead')
   final Object? data;
 
   /// A description of how to fix the problem, or `null` if there is no such
@@ -46,7 +47,7 @@ class Diagnostic {
     required String message,
     this.correctionMessage,
     this.contextMessages = const [],
-    this.data,
+    @Deprecated('Use an expando instead') this.data,
   }) : diagnosticCode = _useNonNullCodeBetween(diagnosticCode, errorCode),
        problemMessage = DiagnosticMessageImpl(
          filePath: source.fullName,
@@ -72,7 +73,7 @@ class Diagnostic {
     DiagnosticCode? errorCode,
     List<Object?> arguments = const [],
     List<DiagnosticMessage> contextMessages = const [],
-    Object? data,
+    @Deprecated('Use an expando instead') Object? data,
   }) {
     var code = _useNonNullCodeBetween(diagnosticCode, errorCode);
     assert(
@@ -98,6 +99,7 @@ class Diagnostic {
       message: message,
       correctionMessage: correctionMessage,
       contextMessages: contextMessages,
+      // ignore: deprecated_member_use_from_same_package
       data: data,
     );
   }

pkg/analyzer/lib/error/listener.dart

@@ -96,87 +96,102 @@ class DiagnosticReporter {
   /// Reports a diagnostic with the given [diagnosticCode] and [arguments].
   ///
   /// The location of the diagnostic will be the name of the [node].
-  void atConstructorDeclaration(
+  ///
+  /// The reported [Diagnostic] is returned so that the caller may attach
+  /// additional information to it (for example, using an expando).
+  Diagnostic atConstructorDeclaration(
     ConstructorDeclaration node,
     DiagnosticCode diagnosticCode, {
     List<Object>? arguments,
     List<DiagnosticMessage>? contextMessages,
-    Object? data,
+    @Deprecated('Use an expando instead') Object? data,
   }) {
     // TODO(brianwilkerson): Consider extending this method to take any
     //  declaration and compute the correct range for the name of that
     //  declaration. This might make it easier to be consistent.
     if (node.name case var nameToken?) {
       var offset = node.returnType.offset;
-      atOffset(
+      return atOffset(
         offset: offset,
         length: nameToken.end - offset,
         diagnosticCode: diagnosticCode,
         arguments: arguments,
       );
     } else {
-      atNode(node.returnType, diagnosticCode, arguments: arguments);
+      return atNode(node.returnType, diagnosticCode, arguments: arguments);
     }
   }
 
   /// Reports a diagnostic with the given [diagnosticCode] and [arguments].
   ///
   /// The [element] is used to compute the location of the diagnostic.
+  ///
+  /// The reported [Diagnostic] is returned so that the caller may attach
+  /// additional information to it (for example, using an expando).
   @experimental
-  void atElement2(
+  Diagnostic atElement2(
     Element element,
     DiagnosticCode diagnosticCode, {
     List<Object>? arguments,
     List<DiagnosticMessage>? contextMessages,
-    Object? data,
+    @Deprecated('Use an expando instead') Object? data,
   }) {
     var nonSynthetic = element.nonSynthetic;
-    atOffset(
+    return atOffset(
       diagnosticCode: diagnosticCode,
       offset: nonSynthetic.firstFragment.nameOffset ?? -1,
       length: nonSynthetic.name?.length ?? 0,
       arguments: arguments,
       contextMessages: contextMessages,
+      // ignore: deprecated_member_use_from_same_package
       data: data,
     );
   }
 
   /// Reports a diagnostic with the given [diagnosticCode] and [arguments].
   ///
   /// The [entity] is used to compute the location of the diagnostic.
-  void atEntity(
+  ///
+  /// The reported [Diagnostic] is returned so that the caller may attach
+  /// additional information to it (for example, using an expando).
+  Diagnostic atEntity(
     SyntacticEntity entity,
     DiagnosticCode diagnosticCode, {
     List<Object>? arguments,
     List<DiagnosticMessage>? contextMessages,
-    Object? data,
+    @Deprecated('Use an expando instead') Object? data,
   }) {
-    atOffset(
+    return atOffset(
       diagnosticCode: diagnosticCode,
       offset: entity.offset,
       length: entity.length,
       arguments: arguments,
       contextMessages: contextMessages,
+      // ignore: deprecated_member_use_from_same_package
       data: data,
     );
   }
 
   /// Reports a diagnostic with the given [diagnosticCode] and [arguments].
   ///
   /// The [node] is used to compute the location of the diagnostic.
-  void atNode(
+  ///
+  /// The reported [Diagnostic] is returned so that the caller may attach
+  /// additional information to it (for example, using an expando).
+  Diagnostic atNode(
     AstNode node,
     DiagnosticCode diagnosticCode, {
     List<Object>? arguments,
     List<DiagnosticMessage>? contextMessages,
-    Object? data,
+    @Deprecated('Use an expando instead') Object? data,
   }) {
-    atOffset(
+    return atOffset(
       diagnosticCode: diagnosticCode,
       offset: node.offset,
       length: node.length,
       arguments: arguments,
       contextMessages: contextMessages,
+      // ignore: deprecated_member_use_from_same_package
       data: data,
     );
   }
@@ -186,18 +201,18 @@ class DiagnosticReporter {
   ///
   /// The location of the diagnostic is specified by the given [offset] and
   /// [length].
-  void atOffset({
+  ///
+  /// The reported [Diagnostic] is returned so that the caller may attach
+  /// additional information to it (for example, using an expando).
+  Diagnostic atOffset({
     required int offset,
     required int length,
     @Deprecated("Use 'diagnosticCode' instead") DiagnosticCode? errorCode,
     DiagnosticCode? diagnosticCode,
     List<Object>? arguments,
     List<DiagnosticMessage>? contextMessages,
-    Object? data,
+    @Deprecated('Use an expando instead') Object? data,
   }) {
-    if (lockLevel != 0) {
-      return;
-    }
     if ((errorCode == null && diagnosticCode == null) ||
         (errorCode != null && diagnosticCode != null)) {
       throw ArgumentError(
@@ -225,61 +240,73 @@ class DiagnosticReporter {
 
     contextMessages ??= [];
     contextMessages.addAll(convertTypeNames(arguments));
-    _diagnosticListener.onDiagnostic(
-      Diagnostic.tmp(
-        source: _source,
-        offset: offset,
-        length: length,
-        diagnosticCode: diagnosticCode,
-        arguments: arguments ?? const [],
-        contextMessages: contextMessages,
-        data: data,
-      ),
+    var diagnostic = Diagnostic.tmp(
+      source: _source,
+      offset: offset,
+      length: length,
+      diagnosticCode: diagnosticCode,
+      arguments: arguments ?? const [],
+      contextMessages: contextMessages,
+      // ignore: deprecated_member_use_from_same_package
+      data: data,
     );
+    reportError(diagnostic);
+    return diagnostic;
   }
 
   /// Reports a diagnostic with the given [diagnosticCode] and [arguments].
   ///
   /// The [span] is used to compute the location of the diagnostic.
-  void atSourceSpan(
+  ///
+  /// The reported [Diagnostic] is returned so that the caller may attach
+  /// additional information to it (for example, using an expando).
+  Diagnostic atSourceSpan(
     SourceSpan span,
     DiagnosticCode diagnosticCode, {
     List<Object>? arguments,
     List<DiagnosticMessage>? contextMessages,
-    Object? data,
+    @Deprecated('Use an expando instead') Object? data,
   }) {
-    atOffset(
+    return atOffset(
       diagnosticCode: diagnosticCode,
       offset: span.start.offset,
       length: span.length,
       arguments: arguments,
       contextMessages: contextMessages,
+      // ignore: deprecated_member_use_from_same_package
       data: data,
     );
   }
 
   /// Reports a diagnostic with the given [diagnosticCode] and [arguments].
   ///
   /// The [token] is used to compute the location of the diagnostic.
-  void atToken(
+  ///
+  /// The reported [Diagnostic] is returned so that the caller may attach
+  /// additional information to it (for example, using an expando).
+  Diagnostic atToken(
     Token token,
     DiagnosticCode diagnosticCode, {
     List<Object>? arguments,
     List<DiagnosticMessage>? contextMessages,
-    Object? data,
+    @Deprecated('Use an expando instead') Object? data,
   }) {
-    atOffset(
+    return atOffset(
       diagnosticCode: diagnosticCode,
       offset: token.offset,
       length: token.length,
       arguments: arguments,
       contextMessages: contextMessages,
+      // ignore: deprecated_member_use_from_same_package
       data: data,
     );
   }
 
   /// Report the given [diagnostic].
   void reportError(Diagnostic diagnostic) {
+    if (lockLevel != 0) {
+      return;
+    }
     _diagnosticListener.onDiagnostic(diagnostic);
   }
 }

pkg/analyzer/lib/src/dart/constant/constant_verifier.dart

@@ -1115,7 +1115,7 @@ class ConstantVerifier extends RecursiveAstVisitor<void> {
               correctionData.add(correctionDataBuffer.parts);
             }
           }
-          _diagnosticReporter.atToken(
+          var diagnostic = _diagnosticReporter.atToken(
             switchKeyword,
             isSwitchExpression
                 ? CompileTimeErrorCode.NON_EXHAUSTIVE_SWITCH_EXPRESSION
@@ -1125,8 +1125,10 @@ class ConstantVerifier extends RecursiveAstVisitor<void> {
               errorBuffer.toString(),
               correctionTextBuffer.toString(),
             ],
-            data: correctionData.isNotEmpty ? correctionData : null,
           );
+          if (correctionData.isNotEmpty) {
+            MissingPatternPart.byDiagnostic[diagnostic] = correctionData;
+          }
         }
       } else {
         if (defaultNode != null && mustBeExhaustive) {

pkg/analyzer/lib/src/error/literal_element_verifier.dart

@@ -257,10 +257,11 @@ class LiteralElementVerifier {
     );
 
     if (iterableType == null) {
-      return _diagnosticReporter.atNode(
+      _diagnosticReporter.atNode(
         expression,
         CompileTimeErrorCode.NOT_ITERABLE_SPREAD,
       );
+      return;
     }
 
     var iterableElementType = iterableType.typeArguments[0];
@@ -357,10 +358,11 @@ class LiteralElementVerifier {
     var mapType = expressionType.asInstanceOf(typeProvider.mapElement);
 
     if (mapType == null) {
-      return _diagnosticReporter.atNode(
+      _diagnosticReporter.atNode(
         expression,
         CompileTimeErrorCode.NOT_MAP_SPREAD,
       );
+      return;
     }
 
     var keyType = mapType.typeArguments[0];

pkg/analyzer/lib/src/generated/exhaustiveness.dart

@@ -476,7 +476,14 @@ class MissingPatternEnumValuePart extends MissingPatternPart {
   String toString() => value2.name!;
 }
 
-abstract class MissingPatternPart {}
+abstract class MissingPatternPart {
+  /// Expando associating each
+  /// [CompileTimeErrorCode.NON_EXHAUSTIVE_SWITCH_EXPRESSION] or
+  /// [CompileTimeErrorCode.NON_EXHAUSTIVE_SWITCH_STATEMENT] diagnostic with a
+  /// list of missing patterns; this data is used by the analysis server to add
+  /// missing switch cases.
+  static final byDiagnostic = Expando<List<List<MissingPatternPart>>>();
+}
 
 class MissingPatternTextPart extends MissingPatternPart {
   final String text;