Fine. Improvements for ManifestFunctionType and formal parameters.

Capture more of a function type’s surface by recording formal-parameter
metadata and the `covariant` flag in manifests, and by matching them
during reuse checks. Encoding now uses each parameter’s
`InternalFormalParameterElement`, so we can serialize metadata,
covariance, and requiredness directly.

Normalize named-parameter handling to reduce churn:
- Named parameters are compared in a stable, sorted order; reordering them
  no longer changes the manifest.
- Positional parameters remain order-sensitive.
- Type-parameter reordering is invariant when unbounded; when bounds differ,
  order remains significant.

Fix a parameter-iteration bug in `ManifestFunctionType.match` by
advancing a single index across positional and named parameters,
avoiding off-by-one mismatches when both kinds are present.

Remove `operator ==` overrides from manifest types and rely solely on
`match()` for structural comparison. This prevents accidental equality
misuse and clarifies the intended comparison semantics.

Plumb and serialize metadata consistently:
- Write/read parameter metadata and covariance in manifests.
- Expose `metadata` on `InternalVariableElement`.
- Simplify bundle writing by removing an unnecessary cast when writing
  parameter metadata.

Bump the on-disk data version to 528 due to the serialization changes.

Overall, this makes manifest matching more precise (catching changes
like `@deprecated` or `covariant`) and more stable (ignoring harmless
named parameter reorders), improving reuse correctness and reducing
manifest churn.

Change-Id: Iea2a04339eaa96f63f10ac9852833dd34028845b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/447860
Commit-Queue: Konstantin Shcheglov <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>

Author: scheglov

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

@@ -106,7 +106,7 @@ testFineAfterLibraryAnalyzerHook;
 // TODO(scheglov): Clean up the list of implicitly analyzed files.
 class AnalysisDriver {
   /// The version of data format, should be incremented on every format change.
-  static const int DATA_VERSION = 527;
+  static const int DATA_VERSION = 528;
 
   /// The number of exception contexts allowed to write. Once this field is
   /// zero, we stop writing any new exception contexts in this process.

pkg/analyzer/lib/src/dart/element/element.dart

@@ -5283,6 +5283,9 @@ mixin InternalSetterElement on InternalPropertyAccessorElement
 }
 
 mixin InternalVariableElement implements VariableElement {
+  @override
+  MetadataImpl get metadata;
+
   @override
   TypeImpl get type;
 }

pkg/analyzer/lib/src/fine/manifest_type.dart

@@ -8,10 +8,10 @@ import 'package:analyzer/dart/element/type.dart';
 import 'package:analyzer/src/dart/element/element.dart';
 import 'package:analyzer/src/dart/element/type.dart';
 import 'package:analyzer/src/fine/manifest_context.dart';
+import 'package:analyzer/src/fine/manifest_item.dart';
 import 'package:analyzer/src/summary2/data_reader.dart';
 import 'package:analyzer/src/summary2/data_writer.dart';
 import 'package:analyzer/src/utilities/extensions/collection.dart';
-import 'package:collection/collection.dart';
 
 final class ManifestDynamicType extends ManifestType {
   static final instance = ManifestDynamicType._();
@@ -30,11 +30,15 @@ final class ManifestDynamicType extends ManifestType {
 }
 
 sealed class ManifestFunctionFormalParameter {
+  final ManifestMetadata metadata;
   final bool isRequired;
+  final bool isCovariant;
   final ManifestType type;
 
   ManifestFunctionFormalParameter({
+    required this.metadata,
     required this.isRequired,
+    required this.isCovariant,
     required this.type,
   });
 }
@@ -44,49 +48,49 @@ class ManifestFunctionNamedFormalParameter
   final String name;
 
   factory ManifestFunctionNamedFormalParameter.encode(
-    EncodeContext context, {
-    required bool isRequired,
-    required DartType type,
-    required String name,
-  }) {
+    EncodeContext context,
+    InternalFormalParameterElement element,
+  ) {
     return ManifestFunctionNamedFormalParameter._(
-      isRequired: isRequired,
-      type: type.encode(context),
-      name: name,
+      metadata: ManifestMetadata.encode(context, element.metadata),
+      isRequired: element.isRequired,
+      isCovariant: element.isCovariant,
+      type: element.type.encode(context),
+      name: element.name ?? '',
     );
   }
 
   factory ManifestFunctionNamedFormalParameter.read(SummaryDataReader reader) {
     return ManifestFunctionNamedFormalParameter._(
+      metadata: ManifestMetadata.read(reader),
       isRequired: reader.readBool(),
+      isCovariant: reader.readBool(),
       type: ManifestType.read(reader),
       name: reader.readStringUtf8(),
     );
   }
 
   ManifestFunctionNamedFormalParameter._({
+    required super.metadata,
     required super.isRequired,
+    required super.isCovariant,
     required super.type,
     required this.name,
   });
 
-  @override
-  bool operator ==(Object other) {
-    if (identical(this, other)) return true;
-    return other is ManifestFunctionNamedFormalParameter &&
-        other.name == name &&
-        other.type == type;
-  }
-
   bool match(MatchContext context, InternalFormalParameterElement element) {
     return element.isNamed &&
+        metadata.match(context, element.metadata) &&
         element.isRequired == isRequired &&
+        element.isCovariant == isCovariant &&
         type.match(context, element.type) &&
         element.name == name;
   }
 
   void write(BufferedSink sink) {
+    metadata.write(sink);
     sink.writeBool(isRequired);
+    sink.writeBool(isCovariant);
     type.write(sink);
     sink.writeStringUtf8(name);
   }
@@ -95,46 +99,47 @@ class ManifestFunctionNamedFormalParameter
 class ManifestFunctionPositionalFormalParameter
     extends ManifestFunctionFormalParameter {
   factory ManifestFunctionPositionalFormalParameter.encode(
-    EncodeContext context, {
-    required bool isRequired,
-    required DartType type,
-  }) {
+    EncodeContext context,
+    InternalFormalParameterElement element,
+  ) {
     return ManifestFunctionPositionalFormalParameter._(
-      isRequired: isRequired,
-      type: type.encode(context),
+      metadata: ManifestMetadata.encode(context, element.metadata),
+      isRequired: element.isRequiredPositional,
+      isCovariant: element.isCovariant,
+      type: element.type.encode(context),
     );
   }
 
   factory ManifestFunctionPositionalFormalParameter.read(
     SummaryDataReader reader,
   ) {
     return ManifestFunctionPositionalFormalParameter._(
+      metadata: ManifestMetadata.read(reader),
       isRequired: reader.readBool(),
+      isCovariant: reader.readBool(),
       type: ManifestType.read(reader),
     );
   }
 
   ManifestFunctionPositionalFormalParameter._({
+    required super.metadata,
     required super.isRequired,
+    required super.isCovariant,
     required super.type,
   });
 
-  @override
-  bool operator ==(Object other) {
-    if (identical(this, other)) return true;
-    return other is ManifestFunctionPositionalFormalParameter &&
-        other.isRequired == isRequired &&
-        other.type == type;
-  }
-
   bool match(MatchContext context, InternalFormalParameterElement element) {
     return element.isPositional &&
+        metadata.match(context, element.metadata) &&
         element.isRequired == isRequired &&
+        element.isCovariant == isCovariant &&
         type.match(context, element.type);
   }
 
   void write(BufferedSink sink) {
+    metadata.write(sink);
     sink.writeBool(isRequired);
+    sink.writeBool(isCovariant);
     type.write(sink);
   }
 }
@@ -153,18 +158,17 @@ final class ManifestFunctionType extends ManifestType {
       return ManifestFunctionType._(
         typeParameters: typeParameters,
         returnType: type.returnType.encode(context),
-        positional: type.positionalParameterTypes.indexed.map((pair) {
-          return ManifestFunctionPositionalFormalParameter._(
-            isRequired: pair.$1 < type.requiredPositionalParameterCount,
-            type: pair.$2.encode(context),
-          );
-        }).toFixedList(),
+        positional: type.formalParameters
+            .where((element) => element.isPositional)
+            .map((element) {
+              return ManifestFunctionPositionalFormalParameter.encode(
+                context,
+                element,
+              );
+            })
+            .toFixedList(),
         named: type.sortedNamedParametersShared.map((element) {
-          return ManifestFunctionNamedFormalParameter._(
-            isRequired: element.isRequired,
-            type: element.type.encode(context),
-            name: element.name!,
-          );
+          return ManifestFunctionNamedFormalParameter.encode(context, element);
         }).toFixedList(),
         nullabilitySuffix: type.nullabilitySuffix,
       );
@@ -195,21 +199,6 @@ final class ManifestFunctionType extends ManifestType {
     required super.nullabilitySuffix,
   });
 
-  @override
-  bool operator ==(Object other) {
-    if (identical(this, other)) return true;
-    return other is ManifestFunctionType &&
-        const ListEquality<ManifestFunctionPositionalFormalParameter>().equals(
-          other.positional,
-          positional,
-        ) &&
-        const ListEquality<ManifestFunctionNamedFormalParameter>().equals(
-          other.named,
-          named,
-        ) &&
-        other.nullabilitySuffix == nullabilitySuffix;
-  }
-
   @override
   bool match(MatchContext context, DartType type) {
     if (type is! FunctionTypeImpl) {
@@ -229,7 +218,7 @@ final class ManifestFunctionType extends ManifestType {
       var index = 0;
 
       for (var i = 0; i < positional.length; i++) {
-        if (i >= formalParameters.length) {
+        if (index >= formalParameters.length) {
           return false;
         }
         var manifest = positional[i];
@@ -240,7 +229,7 @@ final class ManifestFunctionType extends ManifestType {
       }
 
       for (var i = 0; i < named.length; i++) {
-        if (i >= formalParameters.length) {
+        if (index >= formalParameters.length) {
           return false;
         }
         var manifest = named[i];
@@ -309,15 +298,6 @@ final class ManifestInterfaceType extends ManifestType {
     required super.nullabilitySuffix,
   });
 
-  @override
-  bool operator ==(Object other) {
-    if (identical(this, other)) return true;
-    return other is ManifestInterfaceType &&
-        other.element == element &&
-        const ListEquality<ManifestType>().equals(other.arguments, arguments) &&
-        other.nullabilitySuffix == nullabilitySuffix;
-  }
-
   @override
   bool match(MatchContext context, DartType type) {
     if (type is! InterfaceType) {
@@ -384,13 +364,6 @@ final class ManifestNeverType extends ManifestType {
 
   ManifestNeverType._({required super.nullabilitySuffix});
 
-  @override
-  bool operator ==(Object other) {
-    if (identical(this, other)) return true;
-    return other is ManifestNeverType &&
-        other.nullabilitySuffix == nullabilitySuffix;
-  }
-
   @override
   bool match(MatchContext context, DartType type) {
     if (type is! NeverTypeImpl) {
@@ -448,21 +421,6 @@ final class ManifestRecordType extends ManifestType {
     required super.nullabilitySuffix,
   });
 
-  @override
-  bool operator ==(Object other) {
-    if (identical(this, other)) return true;
-    return other is ManifestRecordType &&
-        const ListEquality<ManifestType>().equals(
-          other.positionalFields,
-          positionalFields,
-        ) &&
-        const ListEquality<ManifestRecordTypeNamedField>().equals(
-          other.namedFields,
-          namedFields,
-        ) &&
-        other.nullabilitySuffix == nullabilitySuffix;
-  }
-
   @override
   bool match(MatchContext context, DartType type) {
     if (type is! RecordType) {
@@ -530,14 +488,6 @@ class ManifestRecordTypeNamedField {
 
   ManifestRecordTypeNamedField._({required this.name, required this.type});
 
-  @override
-  bool operator ==(Object other) {
-    if (identical(this, other)) return true;
-    return other is ManifestRecordTypeNamedField &&
-        other.name == name &&
-        other.type == type;
-  }
-
   bool match(MatchContext context, RecordTypeNamedField field) {
     return field.name == name && type.match(context, field.type);
   }
@@ -604,12 +554,6 @@ class ManifestTypeParameter {
 
   ManifestTypeParameter._({required this.bound});
 
-  @override
-  bool operator ==(Object other) {
-    if (identical(this, other)) return true;
-    return other is ManifestTypeParameter && other.bound == bound;
-  }
-
   bool match(MatchContext context, TypeParameterElement element) {
     return bound.match(context, element.bound);
   }
@@ -648,14 +592,6 @@ final class ManifestTypeParameterType extends ManifestType {
     required super.nullabilitySuffix,
   });
 
-  @override
-  bool operator ==(Object other) {
-    if (identical(this, other)) return true;
-    return other is ManifestTypeParameterType &&
-        other.index == index &&
-        other.nullabilitySuffix == nullabilitySuffix;
-  }
-
   @override
   bool match(MatchContext context, DartType type) {
     if (type is! TypeParameterTypeImpl) {

pkg/analyzer/lib/src/summary2/bundle_writer.dart

@@ -1045,7 +1045,7 @@ class ResolutionSink extends _SummaryDataWriter {
         );
       }, withAnnotations: withAnnotations);
       if (withAnnotations) {
-        _writeMetadata(parameter.metadata as MetadataImpl);
+        _writeMetadata(parameter.metadata);
       }
     });
   }