diff --git a/pkg/analysis_server/lib/src/services/correction/fix/pubspec/fix_generator.dart b/pkg/analysis_server/lib/src/services/correction/fix/pubspec/fix_generator.dart index 2183af8ecfab..e58ed31f66bd 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix/pubspec/fix_generator.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix/pubspec/fix_generator.dart @@ -14,6 +14,7 @@ import 'package:analyzer/file_system/file_system.dart'; import 'package:analyzer/source/line_info.dart'; import 'package:analyzer/source/source_range.dart'; import 'package:analyzer/src/diagnostic/diagnostic.dart' as diag; +import 'package:linter/src/diagnostic.dart' as linter_diag; import 'package:analyzer/src/generated/java_core.dart'; import 'package:analyzer/src/pubspec/validators/missing_dependency_validator.dart'; import 'package:analyzer/src/util/yaml.dart'; @@ -27,6 +28,7 @@ class PubspecFixGenerator { static const List codesWithFixes = [ diag.missingDependency, diag.missingName, + linter_diag.sortPubDependencies, ]; /// The resource provider used to access the file system. @@ -124,6 +126,8 @@ class PubspecFixGenerator { // Consider removing the dependency. } else if (diagnosticCode == diag.missingDependency) { await _addMissingDependency(diagnosticCode); + } else if (diagnosticCode == linter_diag.sortPubDependencies) { + await _sortPubDependencies(); } return fixes; } @@ -303,6 +307,136 @@ class PubspecFixGenerator { _addFixFromBuilder(builder, PubspecFixKind.addName); } + /// Sorts dependencies alphabetically in the section containing the + /// diagnostic. + Future _sortPubDependencies() async { + var rootNode = node; + if (rootNode is! YamlMap) { + return; + } + + // Find the dependency section containing the diagnostic offset. + YamlMap? dependencySection; + for (var sectionName in [ + 'dependencies', + 'dev_dependencies', + 'dependency_overrides', + ]) { + var section = rootNode[sectionName]; + if (section is YamlMap) { + var sectionStart = section.span.start.offset; + var sectionEnd = section.span.end.offset; + if (diagnosticOffset >= sectionStart && diagnosticOffset <= sectionEnd) { + dependencySection = section; + break; + } + } + } + + if (dependencySection == null) { + return; + } + + // Collect all dependency entries with their text representations. + var entries = <_DependencyEntry>[]; + for (var entry in dependencySection.nodes.entries) { + var keyNode = entry.key as YamlNode; + var valueNode = entry.value; + var keyName = keyNode.value as String; + + // Calculate the start of this entry (including the key). + var entryStart = keyNode.span.start.offset; + // Calculate the end of this entry (end of value). + var entryEnd = valueNode.span.end.offset; + + // Get the full text for this entry from the content. + var entryText = content.substring(entryStart, entryEnd); + + entries.add(_DependencyEntry(keyName, entryStart, entryEnd, entryText)); + } + + if (entries.length < 2) { + // Nothing to sort if there's only one or no entries. + return; + } + + // Sort entries alphabetically by package name. + var sortedEntries = List<_DependencyEntry>.from(entries) + ..sort((a, b) => a.name.compareTo(b.name)); + + // Check if already sorted. + var alreadySorted = true; + for (var i = 0; i < entries.length; i++) { + if (entries[i].name != sortedEntries[i].name) { + alreadySorted = false; + break; + } + } + if (alreadySorted) { + return; + } + + // Build the sorted content. + // First, find the indentation used for dependencies. + var firstEntryLine = lineInfo.getLocation(entries.first.startOffset).lineNumber - 1; + var lineStart = lineInfo.lineStarts[firstEntryLine]; + var indentation = ''; + for (var i = lineStart; i < entries.first.startOffset; i++) { + var char = content[i]; + if (char == ' ' || char == '\t') { + indentation += char; + } else { + break; + } + } + + // Build the new sorted section content. + var sortedContent = StringBuffer(); + for (var i = 0; i < sortedEntries.length; i++) { + var entry = sortedEntries[i]; + // Normalize the entry text to use consistent indentation. + var normalizedText = _normalizeEntryIndentation(entry.text, indentation); + sortedContent.write(normalizedText); + if (i < sortedEntries.length - 1) { + sortedContent.write(endOfLine); + } + } + + // Calculate the range to replace (from first entry start to last entry end). + var replaceStart = lineStart; + var lastEntry = entries.last; + var replaceEnd = lastEntry.endOffset; + + var builder = ChangeBuilder( + workspace: _NonDartChangeWorkspace(resourceProvider), + defaultEol: endOfLine, + ); + await builder.addYamlFileEdit(file, (builder) { + builder.addSimpleReplacement( + SourceRange(replaceStart, replaceEnd - replaceStart), + sortedContent.toString(), + ); + }); + _addFixFromBuilder(builder, PubspecFixKind.sortDependencies); + } + + /// Normalizes the indentation of a dependency entry. + String _normalizeEntryIndentation(String text, String indentation) { + var lines = text.split(RegExp(r'\r?\n')); + var result = StringBuffer(); + for (var i = 0; i < lines.length; i++) { + var line = lines[i]; + if (i == 0) { + // First line gets the base indentation. + result.write('$indentation${line.trimLeft()}'); + } else { + // Subsequent lines keep their relative indentation. + result.write('$endOfLine$indentation ${line.trimLeft()}'); + } + } + return result.toString(); + } + (String, int) _getTextAndOffset( YamlMap node, String sectionName, @@ -394,3 +528,20 @@ class _Range { _Range(this.startOffset, this.endOffset); } + +/// Represents a dependency entry in the pubspec.yaml file. +class _DependencyEntry { + /// The package name (the key in the YAML map). + final String name; + + /// The start offset of this entry in the file content. + final int startOffset; + + /// The end offset of this entry in the file content. + final int endOffset; + + /// The full text of this entry (key: value). + final String text; + + _DependencyEntry(this.name, this.startOffset, this.endOffset, this.text); +} diff --git a/pkg/analysis_server/lib/src/services/correction/fix/pubspec/fix_kind.dart b/pkg/analysis_server/lib/src/services/correction/fix/pubspec/fix_kind.dart index 989045d508fc..afaa41b26352 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix/pubspec/fix_kind.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix/pubspec/fix_kind.dart @@ -16,6 +16,12 @@ abstract final class PubspecFixKind { PubspecFixKindPriority._default, 'Update pubspec with the missing dependencies', ); + // Sorts dependencies alphabetically in the pubspec.yaml file. + static const sortDependencies = FixKind( + 'pubspec.fix.sort.dependencies', + PubspecFixKindPriority._default, + 'Sort dependencies alphabetically', + ); } abstract final class PubspecFixKindPriority { diff --git a/pkg/analysis_server/test/src/services/correction/fix/pubspec/sort_pub_dependencies_test.dart b/pkg/analysis_server/test/src/services/correction/fix/pubspec/sort_pub_dependencies_test.dart new file mode 100644 index 000000000000..800e2abb392d --- /dev/null +++ b/pkg/analysis_server/test/src/services/correction/fix/pubspec/sort_pub_dependencies_test.dart @@ -0,0 +1,219 @@ +// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:analysis_server/src/services/correction/fix/pubspec/fix_generator.dart'; +import 'package:analysis_server/src/services/correction/fix/pubspec/fix_kind.dart'; +import 'package:analysis_server_plugin/edit/fix/fix.dart'; +import 'package:analyzer/dart/analysis/analysis_options.dart'; +import 'package:analyzer/diagnostic/diagnostic.dart'; +import 'package:analyzer/source/file_source.dart'; +import 'package:analyzer/src/pubspec/pubspec_validator.dart'; +import 'package:analyzer/src/test_utilities/platform.dart'; +import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; +import 'package:analyzer_testing/resource_provider_mixin.dart'; +import 'package:linter/src/rules.dart'; +import 'package:linter/src/rules/pub/sort_pub_dependencies.dart'; +import 'package:test/test.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; +import 'package:yaml/yaml.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(SortPubDependenciesTest); + }); +} + +@reflectiveTest +class SortPubDependenciesTest with ResourceProviderMixin { + /// The content of the pubspec file that is being tested. + late String content; + + /// The result of parsing the [content]. + late YamlNode node; + + /// The diagnostic to be fixed. + late Diagnostic diagnostic; + + FixKind get kind => PubspecFixKind.sortDependencies; + + Future assertHasFix(String initialContent, String expected) async { + _validate(initialContent); + expected = normalizeNewlinesForPlatform(expected); + + var fixes = await _getFixes(); + expect(fixes, hasLength(1)); + var fix = fixes[0]; + expect(fix.kind, kind); + var edits = fix.change.edits; + expect(edits, hasLength(1)); + var actual = _applyEdits(content, edits[0].edits); + expect(actual, expected); + } + + Future assertNoFix(String initialContent) async { + _validate(initialContent); + var fixes = await _getFixes(); + expect(fixes, isEmpty); + } + + Future test_sortDependencies_alreadySorted() async { + // When dependencies are already sorted, the lint should not trigger. + var initialContent = ''' +name: test +dependencies: + aaa: ^1.0.0 + bbb: ^2.0.0 + ccc: ^3.0.0 +'''; + initialContent = normalizeNewlinesForPlatform(initialContent); + content = initialContent; + var pubspecFile = newFile('/home/test/pubspec.yaml', content); + node = loadYamlNode(content); + + var errors = validatePubspec( + source: FileSource(pubspecFile), + contents: node, + provider: resourceProvider, + analysisOptions: _createAnalysisOptions(), + ); + // No lint error should be reported for sorted dependencies. + expect(errors.where((e) => e.diagnosticCode.name == 'sort_pub_dependencies'), isEmpty); + } + + Future test_sortDependencies_simple() async { + await assertHasFix( + ''' +name: test +dependencies: + ccc: ^3.0.0 + aaa: ^1.0.0 + bbb: ^2.0.0 +''', + ''' +name: test +dependencies: + aaa: ^1.0.0 + bbb: ^2.0.0 + ccc: ^3.0.0 +''', + ); + } + + Future test_sortDependencies_twoUnsorted() async { + await assertHasFix( + ''' +name: test +dependencies: + bbb: ^2.0.0 + aaa: ^1.0.0 +''', + ''' +name: test +dependencies: + aaa: ^1.0.0 + bbb: ^2.0.0 +''', + ); + } + + Future test_sortDevDependencies() async { + await assertHasFix( + ''' +name: test +dev_dependencies: + zzz: ^1.0.0 + aaa: ^2.0.0 +''', + ''' +name: test +dev_dependencies: + aaa: ^2.0.0 + zzz: ^1.0.0 +''', + ); + } + + Future test_sortDependencyOverrides() async { + await assertHasFix( + ''' +name: test +dependency_overrides: + xyz: ^1.0.0 + abc: ^2.0.0 +''', + ''' +name: test +dependency_overrides: + abc: ^2.0.0 + xyz: ^1.0.0 +''', + ); + } + + String _applyEdits(String content, List edits) { + // Apply edits in reverse order to preserve offsets. + var sortedEdits = List.from(edits) + ..sort((a, b) => b.offset.compareTo(a.offset)); + var result = content; + for (var edit in sortedEdits) { + result = result.substring(0, edit.offset) + + edit.replacement + + result.substring(edit.offset + edit.length); + } + return result; + } + + AnalysisOptions _createAnalysisOptions() { + // Register linter rules if not already registered. + registerLintRules(); + return _TestAnalysisOptions( + lintRules: [SortPubDependencies()], + ); + } + + Future> _getFixes() async { + var generator = PubspecFixGenerator( + resourceProvider, + diagnostic, + content, + node, + defaultEol: testEol, + ); + return await generator.computeFixes(); + } + + void _validate(String initialContent) { + content = normalizeNewlinesForPlatform(initialContent); + var pubspecFile = newFile('/home/test/pubspec.yaml', content); + node = loadYamlNode(content); + + var errors = validatePubspec( + source: FileSource(pubspecFile), + contents: node, + provider: resourceProvider, + analysisOptions: _createAnalysisOptions(), + ); + + // Find the sort_pub_dependencies error. + var sortErrors = errors + .where((e) => e.diagnosticCode.name == 'sort_pub_dependencies') + .toList(); + expect(sortErrors, hasLength(1), reason: 'Expected exactly one sort_pub_dependencies error'); + diagnostic = sortErrors.first; + } +} + +/// A minimal test implementation of AnalysisOptions for testing lint rules. +class _TestAnalysisOptions implements AnalysisOptions { + @override + final List lintRules; + + _TestAnalysisOptions({required this.lintRules}); + + @override + bool get lint => true; + + @override + dynamic noSuchMethod(Invocation invocation) => null; +}