Add a method to BulkFixProcessor to compute changes to pubspec

Change-Id: I1c743df3780fea3f8cf930dad5085b4d2388fca8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/330520
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Keerti Parthasarathy <keertip@google.com>

pkg/analysis_server/lib/protocol/protocol_constants.dart

@@ -193,6 +193,7 @@ const String EDIT_REQUEST_BULK_FIXES = 'edit.bulkFixes';
 const String EDIT_REQUEST_BULK_FIXES_CODES = 'codes';
 const String EDIT_REQUEST_BULK_FIXES_INCLUDED = 'included';
 const String EDIT_REQUEST_BULK_FIXES_IN_TEST_MODE = 'inTestMode';
+const String EDIT_REQUEST_BULK_FIXES_UPDATE_PUBSPEC = 'updatePubspec';
 const String EDIT_REQUEST_FORMAT = 'edit.format';
 const String EDIT_REQUEST_FORMAT_FILE = 'file';
 const String EDIT_REQUEST_FORMAT_IF_ENABLED = 'edit.formatIfEnabled';

pkg/analysis_server/lib/protocol/protocol_generated.dart

@@ -5962,6 +5962,7 @@ class DiagnosticGetServerPortResult implements ResponseResult {
 /// {
 ///   "included": List<FilePath>
 ///   "inTestMode": optional bool
+///   "updatePubspec": optional bool
 ///   "codes": optional List<String>
 /// }
 ///
@@ -5985,10 +5986,20 @@ class EditBulkFixesParams implements RequestParams {
   /// If this field is omitted the flag defaults to false.
   bool? inTestMode;
 
+  /// A flag indicating whether to validate that the dependencies used by the
+  /// included files are listed in the pubspec file. If specified, the fix
+  /// processor will compute the set of packages imported in the source and
+  /// check to see if they are listed in the corresponding pubspec file, and
+  /// compute the fixes, if any.
+  ///
+  /// If this field is omitted the flag defaults to false.
+  bool? updatePubspec;
+
   /// A list of diagnostic codes to be fixed.
   List<String>? codes;
 
-  EditBulkFixesParams(this.included, {this.inTestMode, this.codes});
+  EditBulkFixesParams(this.included,
+      {this.inTestMode, this.updatePubspec, this.codes});
 
   factory EditBulkFixesParams.fromJson(
       JsonDecoder jsonDecoder, String jsonPath, Object? json) {
@@ -6006,13 +6017,18 @@ class EditBulkFixesParams implements RequestParams {
         inTestMode =
             jsonDecoder.decodeBool('$jsonPath.inTestMode', json['inTestMode']);
       }
+      bool? updatePubspec;
+      if (json.containsKey('updatePubspec')) {
+        updatePubspec = jsonDecoder.decodeBool(
+            '$jsonPath.updatePubspec', json['updatePubspec']);
+      }
       List<String>? codes;
       if (json.containsKey('codes')) {
         codes = jsonDecoder.decodeList(
             '$jsonPath.codes', json['codes'], jsonDecoder.decodeString);
       }
       return EditBulkFixesParams(included,
-          inTestMode: inTestMode, codes: codes);
+          inTestMode: inTestMode, updatePubspec: updatePubspec, codes: codes);
     } else {
       throw jsonDecoder.mismatch(jsonPath, 'edit.bulkFixes params', json);
     }
@@ -6031,6 +6047,10 @@ class EditBulkFixesParams implements RequestParams {
     if (inTestMode != null) {
       result['inTestMode'] = inTestMode;
     }
+    var updatePubspec = this.updatePubspec;
+    if (updatePubspec != null) {
+      result['updatePubspec'] = updatePubspec;
+    }
     var codes = this.codes;
     if (codes != null) {
       result['codes'] = codes;
@@ -6052,6 +6072,7 @@ class EditBulkFixesParams implements RequestParams {
       return listEqual(
               included, other.included, (String a, String b) => a == b) &&
           inTestMode == other.inTestMode &&
+          updatePubspec == other.updatePubspec &&
           listEqual(codes, other.codes, (String a, String b) => a == b);
     }
     return false;
@@ -6061,6 +6082,7 @@ class EditBulkFixesParams implements RequestParams {
   int get hashCode => Object.hash(
         Object.hashAll(included),
         inTestMode,
+        updatePubspec,
         Object.hashAll(codes ?? []),
       );
 }

pkg/analysis_server/lib/src/handler/legacy/edit_bulk_fixes.dart

@@ -32,6 +32,7 @@ class EditBulkFixes extends LegacyHandler {
       }
 
       var codes = params.codes?.map((e) => e.toLowerCase()).toList();
+      var updatePubspec = params.updatePubspec ?? false;
       var collection = AnalysisContextCollectionImpl(
         includedPaths: params.included,
         resourceProvider: server.resourceProvider,
@@ -42,13 +43,18 @@ class EditBulkFixes extends LegacyHandler {
           collection.contexts.map((c) => c.currentSession).toList());
       var processor = BulkFixProcessor(server.instrumentationService, workspace,
           useConfigFiles: params.inTestMode ?? false, codes: codes);
-      var result = await processor.fixErrors(collection.contexts);
-      var message = result.errorMessage;
-      if (message != null) {
-        sendResult(EditBulkFixesResult(message, [], []));
+      if (!updatePubspec) {
+        var result = await processor.fixErrors(collection.contexts);
+        var message = result.errorMessage;
+        if (message != null) {
+          sendResult(EditBulkFixesResult(message, [], []));
+        } else {
+          sendResult(EditBulkFixesResult(
+              '', result.builder!.sourceChange.edits, processor.fixDetails));
+        }
       } else {
-        sendResult(EditBulkFixesResult(
-            '', result.builder!.sourceChange.edits, processor.fixDetails));
+        var result = await processor.fixPubspec(collection.contexts);
+        sendResult(EditBulkFixesResult('', result.edits, result.details));
       }
     } catch (exception, stackTrace) {
       // TODO(brianwilkerson) Move exception handling outside [handle].

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

@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:_fe_analyzer_shared/src/scanner/errors.dart';
+import 'package:analysis_server/plugin/edit/fix/fix_core.dart';
 import 'package:analysis_server/plugin/edit/fix/fix_dart.dart';
 import 'package:analysis_server/protocol/protocol_generated.dart'
     hide AnalysisOptions;
@@ -18,8 +19,8 @@ import 'package:analysis_server/src/services/correction/fix/data_driven/transfor
 import 'package:analysis_server/src/services/correction/fix_internal.dart';
 import 'package:analysis_server/src/services/linter/lint_names.dart';
 import 'package:analyzer/dart/analysis/analysis_context.dart';
-import 'package:analyzer/dart/analysis/analysis_options.dart';
 import 'package:analyzer/dart/analysis/results.dart';
+import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/error/error.dart';
 import 'package:analyzer/error/listener.dart';
 import 'package:analyzer/exception/exception.dart';
@@ -34,18 +35,25 @@ import 'package:analyzer/src/error/codes.dart';
 import 'package:analyzer/src/lint/linter.dart';
 import 'package:analyzer/src/lint/linter_visitor.dart';
 import 'package:analyzer/src/lint/registry.dart';
+import 'package:analyzer/src/pubspec/pubspec_warning_code.dart';
+import 'package:analyzer/src/pubspec/validators/missing_dependency_validator.dart';
+import 'package:analyzer/src/source/source_resource.dart';
 import 'package:analyzer/src/string_source.dart';
 import 'package:analyzer/src/util/file_paths.dart' as file_paths;
 import 'package:analyzer/src/util/performance/operation_performance.dart';
 import 'package:analyzer/src/utilities/cancellation.dart';
 import 'package:analyzer/src/utilities/extensions/string.dart';
+import 'package:analyzer/src/workspace/blaze.dart';
+import 'package:analyzer/src/workspace/gn.dart';
 import 'package:analyzer_plugin/protocol/protocol_common.dart'
     show SourceFileEdit;
 import 'package:analyzer_plugin/src/utilities/change_builder/change_builder_core.dart';
 import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
 import 'package:analyzer_plugin/utilities/change_builder/conflicting_edit_exception.dart';
 import 'package:collection/collection.dart';
+import 'package:yaml/yaml.dart';
 
+import 'fix/pubspec/fix_generator.dart';
 import 'organize_imports.dart';
 
 /// A fix producer that produces changes that will fix multiple diagnostics in
@@ -267,6 +275,11 @@ class BulkFixProcessor {
           List<AnalysisContext> contexts) =>
       _computeFixesUsingParsedResult(contexts);
 
+  /// Return a [PubspecFixRequestResult] that includes edits to the pubspec
+  /// files in the given [contexts].
+  Future<PubspecFixRequestResult> fixPubspec(List<AnalysisContext> contexts) =>
+      _computeChangesToPubspec(contexts);
+
   /// Return a [BulkFixRequestResult] that includes a change builder that has
   /// been used to format the dart files in the given [contexts].
   Future<BulkFixRequestResult> formatCode(List<AnalysisContext> contexts) =>
@@ -318,6 +331,77 @@ class BulkFixProcessor {
     }
   }
 
+  Future<PubspecFixRequestResult> _computeChangesToPubspec(
+      List<AnalysisContext> contexts) async {
+    var fixes = <SourceFileEdit>[];
+    var details = <BulkFix>[];
+    for (var context in contexts) {
+      var workspace = context.contextRoot.workspace;
+      if (workspace is GnWorkspace || workspace is BlazeWorkspace) {
+        continue;
+      }
+      // Find the pubspec file
+      var rootFolder = context.contextRoot.root;
+      var pubspecFile = rootFolder.getChild('pubspec.yaml') as File;
+      if (!pubspecFile.exists) {
+        continue;
+      }
+      var packages = <String>{};
+      var devPackages = <String>{};
+
+      var pathContext = context.contextRoot.resourceProvider.pathContext;
+      final libPath = rootFolder.getChild('lib').path;
+      final binPath = rootFolder.getChild('bin').path;
+
+      bool isPublic(String path) {
+        if (path.startsWith(libPath) || path.startsWith(binPath)) {
+          return true;
+        }
+        return false;
+      }
+
+      for (var path in context.contextRoot.analyzedFiles()) {
+        if (!file_paths.isDart(pathContext, path) ||
+            file_paths.isGenerated(path)) {
+          continue;
+        }
+        // Get the list of imports used in the files.
+        var result = context.currentSession.getParsedLibrary(path)
+            as ParsedLibraryResult;
+        for (var unit in result.units) {
+          var directives = unit.unit.directives;
+          for (var directive in directives) {
+            var uri =
+                (directive is ImportDirective) ? directive.uri.stringValue : '';
+            if (uri!.startsWith('package:')) {
+              final name = Uri.parse(uri).pathSegments.first;
+              if (isPublic(path)) {
+                packages.add(name);
+              } else {
+                devPackages.add(name);
+              }
+            }
+          }
+        }
+      }
+
+      // Compute changes to pubspec.
+      var result = await _runPubspecValidatorAndFixGenerator(
+          FileSource(pubspecFile),
+          packages,
+          devPackages,
+          context.contextRoot.resourceProvider);
+      if (result.isNotEmpty) {
+        for (var fix in result) {
+          fixes.addAll(fix.change.edits);
+        }
+        details.add(BulkFix(pubspecFile.path,
+            [BulkFixDetail(PubspecWarningCode.MISSING_DEPENDENCY.name, 1)]));
+      }
+    }
+    return PubspecFixRequestResult(fixes, details);
+  }
+
   /// Implementation for [fixErrors] and [hasFixes].
   ///
   /// Return a [BulkFixRequestResult] that includes a change builder that has
@@ -373,7 +457,6 @@ class BulkFixProcessor {
         }
       }
     }
-
     return BulkFixRequestResult(builder);
   }
 
@@ -828,6 +911,28 @@ class BulkFixProcessor {
     }
     return null;
   }
+
+  Future<List<Fix>> _runPubspecValidatorAndFixGenerator(
+      Source pubspec,
+      Set<String> usedDeps,
+      Set<String> usedDevDeps,
+      ResourceProvider resourceProvider) async {
+    String contents = pubspec.contents.data;
+    YamlNode node = loadYamlNode(contents);
+    if (node is! YamlMap) {
+      // The file is empty.
+      node = YamlMap();
+    }
+
+    var errors = MissingDependencyValidator(node, pubspec, resourceProvider)
+        .validate(usedDeps, usedDevDeps);
+    if (errors.isNotEmpty) {
+      var generator =
+          PubspecFixGenerator(resourceProvider, errors[0], contents, node);
+      return await generator.computeFixes();
+    }
+    return [];
+  }
 }
 
 class BulkFixRequestResult {
@@ -946,6 +1051,13 @@ class IterativeBulkFixProcessor {
   }
 }
 
+class PubspecFixRequestResult {
+  final List<SourceFileEdit> edits;
+  final List<BulkFix> details;
+
+  PubspecFixRequestResult(this.edits, this.details);
+}
+
 extension on String {
   String pluralized(int count) => count == 1 ? toString() : '${toString()}s';
 }

pkg/analysis_server/test/integration/support/integration_test_methods.dart

@@ -1859,6 +1859,16 @@ abstract class IntegrationTest {
   ///
   ///   If this field is omitted the flag defaults to false.
   ///
+  /// updatePubspec: bool (optional)
+  ///
+  ///   A flag indicating whether to validate that the dependencies used by the
+  ///   included files are listed in the pubspec file. If specified, the fix
+  ///   processor will compute the set of packages imported in the source and
+  ///   check to see if they are listed in the corresponding pubspec file, and
+  ///   compute the fixes, if any.
+  ///
+  ///   If this field is omitted the flag defaults to false.
+  ///
   /// codes: List<String> (optional)
   ///
   ///   A list of diagnostic codes to be fixed.
@@ -1878,10 +1888,10 @@ abstract class IntegrationTest {
   ///   Details that summarize the fixes associated with the recommended
   ///   changes.
   Future<EditBulkFixesResult> sendEditBulkFixes(List<String> included,
-      {bool? inTestMode, List<String>? codes}) async {
-    var params =
-        EditBulkFixesParams(included, inTestMode: inTestMode, codes: codes)
-            .toJson();
+      {bool? inTestMode, bool? updatePubspec, List<String>? codes}) async {
+    var params = EditBulkFixesParams(included,
+            inTestMode: inTestMode, updatePubspec: updatePubspec, codes: codes)
+        .toJson();
     var result = await server.send('edit.bulkFixes', params);
     var decoder = ResponseDecoder(null);
     return EditBulkFixesResult.fromJson(decoder, 'result', result);

pkg/analysis_server/test/integration/support/protocol_matchers.dart

@@ -2293,11 +2293,17 @@ final Matcher isDiagnosticGetServerPortResult = LazyMatcher(() =>
 /// {
 ///   "included": List<FilePath>
 ///   "inTestMode": optional bool
+///   "updatePubspec": optional bool
 ///   "codes": optional List<String>
 /// }
 final Matcher isEditBulkFixesParams = LazyMatcher(() => MatchesJsonObject(
-    'edit.bulkFixes params', {'included': isListOf(isFilePath)},
-    optionalFields: {'inTestMode': isBool, 'codes': isListOf(isString)}));
+        'edit.bulkFixes params', {
+      'included': isListOf(isFilePath)
+    }, optionalFields: {
+      'inTestMode': isBool,
+      'updatePubspec': isBool,
+      'codes': isListOf(isString)
+    }));
 
 /// edit.bulkFixes result
 ///

pkg/analysis_server/test/src/services/correction/fix/bulk_fix_processor_test.dart

@@ -16,6 +16,7 @@ void main() {
     defineReflectiveTests(HasFixesTest);
     defineReflectiveTests(ChangeMapTest);
     defineReflectiveTests(NoFixTest);
+    defineReflectiveTests(PubspecFixTest);
   });
 }
 
@@ -145,3 +146,87 @@ void bad() {
     expect(processor.fixDetails, isEmpty);
   }
 }
+
+@reflectiveTest
+class PubspecFixTest extends BulkFixProcessorTest {
+  Future<void> test_fix() async {
+    var content = '''
+name: test
+''';
+    var expected = '''
+name: test
+dependencies:
+  a: any
+''';
+    updateTestPubspecFile(content);
+
+    await resolveTestCode('''
+import 'package:a/a.dart';
+
+void bad() {
+  try {
+  } on Error catch (e) {
+    print(e);
+  }
+}
+''');
+    await assertFixPubspec(content, expected);
+  }
+
+  Future<void> test_multiple_changes() async {
+    var content = '''
+name: test
+dependencies:
+  a: any
+''';
+    var expected = '''
+name: test
+dependencies:
+  a: any
+  b: any
+  c: any
+''';
+    updateTestPubspecFile(content);
+
+    await resolveTestCode('''
+import 'package:b/b.dart';
+import 'package:c/c.dart';
+
+void bad() {
+  try {
+  } on Error catch (e) {
+    print(e);
+  }
+}
+''');
+
+    await assertFixPubspec(content, expected);
+  }
+
+  Future<void> test_no_fix() async {
+    var content = '''
+name: test
+dependencies:
+  a: any
+''';
+    var expected = '''
+name: test
+dependencies:
+  a: any
+''';
+
+    updateTestPubspecFile(content);
+    await resolveTestCode('''
+import 'package:a/a.dart';
+
+void bad() {
+  try {
+  } on Error catch (e) {
+    print(e);
+  }
+}
+''');
+
+    await assertFixPubspec(content, expected);
+  }
+}

pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart

@@ -100,6 +100,23 @@ abstract class BulkFixProcessorTest extends AbstractSingleUnitTest {
     return DartChangeWorkspace([await session]);
   }
 
+  /// Computes fixes for the pubspecs in the given contexts.
+  Future<void> assertFixPubspec(String original, String expected) async {
+    var tracker = DeclarationsTracker(MemoryByteStore(), resourceProvider);
+    var analysisContext = contextFor(testFile);
+    tracker.addContext(analysisContext);
+    var processor = BulkFixProcessor(
+        TestInstrumentationService(), await workspace,
+        useConfigFiles: useConfigFiles);
+    var fixes = (await processor.fixPubspec([analysisContext])).edits;
+    var edits = <SourceEdit>[];
+    for (var fix in fixes) {
+      edits.addAll(fix.edits);
+    }
+    var result = SourceEdit.applySequence(original, edits);
+    expect(result, expected);
+  }
+
   Future<void> assertFormat(String expectedCode) async {
     var tracker = DeclarationsTracker(MemoryByteStore(), resourceProvider);
     var analysisContext = contextFor(testFile);

pkg/analysis_server/tool/spec/generated/java/AnalysisServer.java

@@ -517,9 +517,14 @@ public interface AnalysisServer {
    *        difference is that in test mode the fix processor will look for a configuration file that
    *        can modify the content of the data file used to compute the fixes when data-driven fixes
    *        are being considered. If this field is omitted the flag defaults to false.
+   * @param updatePubspec A flag indicating whether to validate that the dependencies used by the
+   *        included files are listed in the pubspec file. If specified, the fix processor will
+   *        compute the set of packages imported in the source and check to see if they are listed in
+   *        the corresponding pubspec file, and compute the fixes, if any. If this field is omitted
+   *        the flag defaults to false.
    * @param codes A list of diagnostic codes to be fixed.
    */
-  public void edit_bulkFixes(List<String> included, boolean inTestMode, List<String> codes, BulkFixesConsumer consumer);
+  public void edit_bulkFixes(List<String> included, boolean inTestMode, boolean updatePubspec, List<String> codes, BulkFixesConsumer consumer);
 
   /**
    * {@code edit.format}

pkg/analysis_server/tool/spec/spec_input.html

@@ -2564,6 +2564,18 @@
           If this field is omitted the flag defaults to <tt>false</tt>.
         </p>
       </field>
+      <field name="updatePubspec" optional="true">
+        <ref>bool</ref>
+        <p>
+          A flag indicating whether to validate that the dependencies used by the included files are
+          listed in the pubspec file. If specified, the fix processor will compute the set of
+          packages imported in the source and check to see if they are listed in the corresponding
+          pubspec file, and compute the fixes, if any.
+        </p>
+        <p>
+          If this field is omitted the flag defaults to <tt>false</tt>.
+        </p>
+      </field>
       <field name="codes" optional="true">
         <list>
           <ref>String</ref>

pkg/analysis_server_client/lib/src/protocol/protocol_constants.dart

@@ -193,6 +193,7 @@ const String EDIT_REQUEST_BULK_FIXES = 'edit.bulkFixes';
 const String EDIT_REQUEST_BULK_FIXES_CODES = 'codes';
 const String EDIT_REQUEST_BULK_FIXES_INCLUDED = 'included';
 const String EDIT_REQUEST_BULK_FIXES_IN_TEST_MODE = 'inTestMode';
+const String EDIT_REQUEST_BULK_FIXES_UPDATE_PUBSPEC = 'updatePubspec';
 const String EDIT_REQUEST_FORMAT = 'edit.format';
 const String EDIT_REQUEST_FORMAT_FILE = 'file';
 const String EDIT_REQUEST_FORMAT_IF_ENABLED = 'edit.formatIfEnabled';

pkg/analysis_server_client/lib/src/protocol/protocol_generated.dart

@@ -5962,6 +5962,7 @@ class DiagnosticGetServerPortResult implements ResponseResult {
 /// {
 ///   "included": List<FilePath>
 ///   "inTestMode": optional bool
+///   "updatePubspec": optional bool
 ///   "codes": optional List<String>
 /// }
 ///
@@ -5985,10 +5986,20 @@ class EditBulkFixesParams implements RequestParams {
   /// If this field is omitted the flag defaults to false.
   bool? inTestMode;
 
+  /// A flag indicating whether to validate that the dependencies used by the
+  /// included files are listed in the pubspec file. If specified, the fix
+  /// processor will compute the set of packages imported in the source and
+  /// check to see if they are listed in the corresponding pubspec file, and
+  /// compute the fixes, if any.
+  ///
+  /// If this field is omitted the flag defaults to false.
+  bool? updatePubspec;
+
   /// A list of diagnostic codes to be fixed.
   List<String>? codes;
 
-  EditBulkFixesParams(this.included, {this.inTestMode, this.codes});
+  EditBulkFixesParams(this.included,
+      {this.inTestMode, this.updatePubspec, this.codes});
 
   factory EditBulkFixesParams.fromJson(
       JsonDecoder jsonDecoder, String jsonPath, Object? json) {
@@ -6006,13 +6017,18 @@ class EditBulkFixesParams implements RequestParams {
         inTestMode =
             jsonDecoder.decodeBool('$jsonPath.inTestMode', json['inTestMode']);
       }
+      bool? updatePubspec;
+      if (json.containsKey('updatePubspec')) {
+        updatePubspec = jsonDecoder.decodeBool(
+            '$jsonPath.updatePubspec', json['updatePubspec']);
+      }
       List<String>? codes;
       if (json.containsKey('codes')) {
         codes = jsonDecoder.decodeList(
             '$jsonPath.codes', json['codes'], jsonDecoder.decodeString);
       }
       return EditBulkFixesParams(included,
-          inTestMode: inTestMode, codes: codes);
+          inTestMode: inTestMode, updatePubspec: updatePubspec, codes: codes);
     } else {
       throw jsonDecoder.mismatch(jsonPath, 'edit.bulkFixes params', json);
     }
@@ -6031,6 +6047,10 @@ class EditBulkFixesParams implements RequestParams {
     if (inTestMode != null) {
       result['inTestMode'] = inTestMode;
     }
+    var updatePubspec = this.updatePubspec;
+    if (updatePubspec != null) {
+      result['updatePubspec'] = updatePubspec;
+    }
     var codes = this.codes;
     if (codes != null) {
       result['codes'] = codes;
@@ -6052,6 +6072,7 @@ class EditBulkFixesParams implements RequestParams {
       return listEqual(
               included, other.included, (String a, String b) => a == b) &&
           inTestMode == other.inTestMode &&
+          updatePubspec == other.updatePubspec &&
           listEqual(codes, other.codes, (String a, String b) => a == b);
     }
     return false;
@@ -6061,6 +6082,7 @@ class EditBulkFixesParams implements RequestParams {
   int get hashCode => Object.hash(
         Object.hashAll(included),
         inTestMode,
+        updatePubspec,
         Object.hashAll(codes ?? []),
       );
 }