diff --git a/pkg/analysis_server/lib/src/services/correction/bulk_fix_processor.dart b/pkg/analysis_server/lib/src/services/correction/bulk_fix_processor.dart index d01d7c6402a1..9872c0b61f71 100644 --- a/pkg/analysis_server/lib/src/services/correction/bulk_fix_processor.dart +++ b/pkg/analysis_server/lib/src/services/correction/bulk_fix_processor.dart @@ -42,8 +42,7 @@ import 'package:analyzer/src/util/performance/operation_performance.dart'; import 'package:analyzer/src/utilities/cancellation.dart'; import 'package:analyzer/src/utilities/extensions/analysis_session.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/src/workspace/pub.dart'; import 'package:analyzer_plugin/protocol/protocol_common.dart' show SourceFileEdit; import 'package:analyzer_plugin/src/utilities/change_builder/change_builder_core.dart'; @@ -355,28 +354,12 @@ class BulkFixProcessor { var details = []; for (var context in contexts) { var workspace = context.contextRoot.workspace; - if (workspace is GnWorkspace || workspace is BlazeWorkspace) { + if (workspace is! PackageConfigWorkspace) { continue; } - // Find the pubspec file - var rootFolder = context.contextRoot.root; - var pubspecFile = rootFolder.getChild('pubspec.yaml') as File; - if (!pubspecFile.exists) { - continue; - } - var packages = {}; - var devPackages = {}; - 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; - } + var resourceProvider = workspace.provider; + var packageToDeps = {}; for (var path in context.contextRoot.analyzedFiles()) { if (!file_paths.isDart(pathContext, path) || @@ -384,8 +367,27 @@ class BulkFixProcessor { file_paths.isMacroGenerated(path)) { continue; } - // Get the list of imports used in the files. + var package = workspace.findPackageFor(path); + if (package is! PubPackage) { + continue; + } + final libPath = + resourceProvider.getFolder(package.root).getChild('lib').path; + final binPath = + resourceProvider.getFolder(package.root).getChild('bin').path; + + bool isPublic(String path, PubPackage package) { + if (path.startsWith(libPath) || path.startsWith(binPath)) { + return true; + } + return false; + } + + var pubspecDeps = + packageToDeps.putIfAbsent(package, () => _PubspecDeps()); + + // Get the list of imports used in the files. var result = context.currentSession.getParsedLibrary(path); if (result is! ParsedLibraryResult) { return PubspecFixRequestResult(fixes, details); @@ -398,28 +400,32 @@ class BulkFixProcessor { (directive is ImportDirective) ? directive.uri.stringValue : ''; if (uri!.startsWith('package:')) { final name = Uri.parse(uri).pathSegments.first; - if (isPublic(path)) { - packages.add(name); + if (isPublic(path, package)) { + pubspecDeps.packages.add(name); } else { - devPackages.add(name); + pubspecDeps.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); + // Iterate over packages in the workspace, compute changes to pubspec. + for (var package in packageToDeps.keys) { + var pubspecDeps = packageToDeps[package]!; + var pubspecFile = package.pubspecFile; + var result = await _runPubspecValidatorAndFixGenerator( + FileSource(pubspecFile), + pubspecDeps.packages, + pubspecDeps.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)])); } - details.add(BulkFix(pubspecFile.path, - [BulkFixDetail(PubspecWarningCode.MISSING_DEPENDENCY.name, 1)])); } } return PubspecFixRequestResult(fixes, details); @@ -1064,6 +1070,11 @@ class PubspecFixRequestResult { PubspecFixRequestResult(this.edits, this.details); } +class _PubspecDeps { + final Set packages = {}; + final Set devPackages = {}; +} + extension on int { String get isAre => this == 1 ? 'is' : 'are'; } diff --git a/pkg/analysis_server/test/src/services/correction/fix/bulk_fix_processor_test.dart b/pkg/analysis_server/test/src/services/correction/fix/bulk_fix_processor_test.dart index 28b815840d17..6c67b53a84a5 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/bulk_fix_processor_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/bulk_fix_processor_test.dart @@ -149,6 +149,53 @@ void bad() { @reflectiveTest class PubspecFixTest extends BulkFixProcessorTest { + @FailingTest( + reason: 'TODO(keertip: Fix issue with deletes where the entry to' + 'be removed is in between entries.') + Future test_delete_change() async { + var content = ''' +name: test +dependencies: + a: any +dev_dependencies: + b: any + c: any + d: any +'''; + var expected = ''' +name: test +dependencies: + a: any + c: any +dev_dependencies: + b: any + d: any +'''; + updateTestPubspecFile(content); + + newFile('$testPackageLibPath/lib.dart', ''' +import 'package:c/c.dart'; + +void bad() { + try { + } on Error catch (e) { + print(e); + } +} +'''); + var testFile = newFile('$testPackageTestPath/test.dart', ''' +import 'package:b/b.dart'; +import 'package:c/c.dart'; +import 'package:d/d.dart'; +import 'package:test/lib.dart'; +void f() { + print(C()); +} +'''); + await getResolvedUnit(testFile); + await assertFixPubspec(content, expected); + } + Future test_fix() async { var content = ''' name: test @@ -203,6 +250,69 @@ void bad() { await assertFixPubspec(content, expected); } + Future test_multiple_pubspec_change() async { + var content = ''' +name: test +dependencies: + a: any +dev_dependencies: + b: any + d: any + c: any +'''; + var expected = ''' +name: test +dependencies: + a: any + c: any + test2: any +dev_dependencies: + b: any + d: any +'''; + updateTestPubspecFile(content); + + newFile('$testPackageLibPath/lib.dart', ''' +import 'package:c/c.dart'; +import 'package:test2/lib.dart'; +import 'package:flutter_gen/gen.dart'; + +void bad() { + try { + } on Error catch (e) { + print(e); + } +} +'''); + var testFile = newFile('$testPackageTestPath/test.dart', ''' +import 'package:b/b.dart'; +import 'package:c/c.dart'; +import 'package:d/d.dart'; +import 'package:test/lib.dart'; +void f() { + print(C()); +} +'''); + + newFile('$workspaceRootPath/test2/lib.dart', ''' +import 'package:d/d.dart'; +import 'package:flutter_gen/gen.dart'; + +class A{} +'''); + var test2PubspecContent = ''' +name: test2 +deps: + d: any +'''; + var test2Pubspec = + newFile('$workspaceRootPath/test2/pubspec.yaml', test2PubspecContent); + await getResolvedUnit(testFile); + await assertFixPubspec(content, expected); + await assertFixPubspec(test2PubspecContent, test2PubspecContent, + file: test2Pubspec); + } + Future test_no_fix() async { var content = ''' name: test diff --git a/pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart b/pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart index 024c271e1dd7..541db97d6260 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart @@ -7,6 +7,7 @@ import 'package:analysis_server/src/services/correction/change_workspace.dart'; import 'package:analysis_server/src/services/correction/fix.dart'; import 'package:analysis_server/src/services/correction/fix_internal.dart'; import 'package:analyzer/error/error.dart'; +import 'package:analyzer/file_system/file_system.dart'; import 'package:analyzer/src/dart/analysis/byte_store.dart'; import 'package:analyzer/src/dart/error/lint_codes.dart'; import 'package:analyzer/src/services/available_declarations.dart'; @@ -99,9 +100,10 @@ abstract class BulkFixProcessorTest extends AbstractSingleUnitTest { } /// Computes fixes for the pubspecs in the given contexts. - Future assertFixPubspec(String original, String expected) async { + Future assertFixPubspec(String original, String expected, + {File? file}) async { var tracker = DeclarationsTracker(MemoryByteStore(), resourceProvider); - var analysisContext = contextFor(testFile); + var analysisContext = contextFor(file ?? testFile); tracker.addContext(analysisContext); var processor = BulkFixProcessor(TestInstrumentationService(), await workspace); diff --git a/pkg/analyzer/lib/src/pubspec/validators/missing_dependency_validator.dart b/pkg/analyzer/lib/src/pubspec/validators/missing_dependency_validator.dart index 8b30299aa9f8..36ea124d252d 100644 --- a/pkg/analyzer/lib/src/pubspec/validators/missing_dependency_validator.dart +++ b/pkg/analyzer/lib/src/pubspec/validators/missing_dependency_validator.dart @@ -38,6 +38,12 @@ class MissingDependencyValidator { /// The listener to record the errors. final RecordingErrorListener recorder; + /// A set of names of special packages that should not be added as + /// dependencies in the `pubspec.yaml` file. For example, the flutter_gen + /// codegen package is specified in a special `flutter` section of the + /// `pubspec.yaml` file and not as part of the `dependencies` section. + final Set noDepsPackages = {'flutter_gen'}; + MissingDependencyValidator(this.contents, this.source, this.provider) : recorder = RecordingErrorListener() { reporter = ErrorReporter(recorder, source); @@ -79,6 +85,10 @@ class MissingDependencyValidator { // Ensure that the package itself is not listed as a dependency. usedDeps.remove(packageName); usedDevDeps.remove(packageName); + for (var package in noDepsPackages) { + usedDeps.remove(package); + usedDevDeps.remove(package); + } var availableDeps = [ if (dependencies.isNotEmpty) diff --git a/pkg/analyzer/lib/src/workspace/pub.dart b/pkg/analyzer/lib/src/workspace/pub.dart index 49f8d30d481b..b7315d9973b8 100644 --- a/pkg/analyzer/lib/src/workspace/pub.dart +++ b/pkg/analyzer/lib/src/workspace/pub.dart @@ -393,9 +393,11 @@ class PubPackage extends WorkspacePackage { final String? _name; - final String? _pubspecContent; + final String? pubspecContent; - final Pubspec? _pubspec; + final Pubspec? pubspec; + + final File pubspecFile; VersionConstraint? _sdkVersionConstraint; @@ -410,15 +412,12 @@ class PubPackage extends WorkspacePackage { var pubspecContent = pubspecFile.readAsStringSync(); var pubspec = Pubspec.parse(pubspecContent); var packageName = pubspec.name?.value.text; - return PubPackage._(root, workspace, pubspecContent, pubspec, packageName); + return PubPackage._( + root, workspace, pubspecContent, pubspecFile, pubspec, packageName); } - PubPackage._(this.root, this.workspace, this._pubspecContent, this._pubspec, - this._name); - - Pubspec? get pubspec => _pubspec; - - String? get pubspecContent => _pubspecContent; + PubPackage._(this.root, this.workspace, this.pubspecContent, this.pubspecFile, + this.pubspec, this._name); /// The version range for the SDK specified for this package , or `null` if /// it is ill-formatted or not set. @@ -426,7 +425,7 @@ class PubPackage extends WorkspacePackage { if (!_parsedSdkConstraint) { _parsedSdkConstraint = true; - var sdkValue = _pubspec?.environment?.sdk?.value.text; + var sdkValue = pubspec?.environment?.sdk?.value.text; if (sdkValue != null) { try { _sdkVersionConstraint = VersionConstraint.parse(sdkValue);