[migrate] Initial implementation of SDK constraint bumping.
Uses the pubspec util `computeVersionBumpEdit` that bumps the pubspec file's `sdk:` one minor version up. Very basic iteration through the given pubspec files to bump all of them. We'll very likely have to refactor this as we get the pre/post migrations involved, but I wanted to get there in small (reviewable) steps. Bug: https://github.com/dart-lang/sdk/issues/63268 Change-Id: I2a7abeacdc76b114dfef7888cd6db6bc8973d01a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/504381 Commit-Queue: Kallen Tu <kallentu@google.com> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
committed by
dart-scoped@luci-project-accounts.iam.gserviceaccount.com
parent
0888111999
commit
efd30a2de0
@@ -6,8 +6,13 @@ import 'package:analysis_server/lsp_protocol/protocol.dart';
|
||||
import 'package:analysis_server/src/lsp/constants.dart';
|
||||
import 'package:analysis_server/src/lsp/error_or.dart';
|
||||
import 'package:analysis_server/src/lsp/handlers/handlers.dart';
|
||||
import 'package:analysis_server/src/lsp/mapping.dart';
|
||||
import 'package:analysis_server/src/utilities/pubspec.dart';
|
||||
import 'package:analysis_server_plugin/src/correction/dart_change_workspace.dart';
|
||||
import 'package:analyzer/file_system/file_system.dart';
|
||||
import 'package:analyzer/source/source_range.dart';
|
||||
import 'package:analyzer/src/util/file_paths.dart' as file_paths;
|
||||
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
|
||||
import 'package:yaml/yaml.dart';
|
||||
|
||||
class MigrateHandler
|
||||
@@ -35,17 +40,85 @@ class MigrateHandler
|
||||
return failure(validationResult);
|
||||
}
|
||||
|
||||
var summaryBuffer = StringBuffer();
|
||||
|
||||
// TODO(kallentu): Add a registry of pre-migration lints to enable based
|
||||
// on pubspec version.
|
||||
|
||||
// TODO(kallentu): Support bumping the pubspec SDK constraint and
|
||||
// reanalyzing with the new version constraint.
|
||||
var targets = validationResult.resultOrNull!;
|
||||
var changeBuilder = await _bumpPubspecConstraints(targets, summaryBuffer);
|
||||
|
||||
// TODO(kallentu): Add a registry of post-migration lints to enable based
|
||||
// on pubspec version.
|
||||
|
||||
// TODO(kallentu): Fix post-migration lints.
|
||||
return success(DartMigrateResult(summary: 'Not implemented yet.'));
|
||||
|
||||
var workspaceEdit = createWorkspaceEdit(
|
||||
server,
|
||||
message.clientCapabilities!,
|
||||
changeBuilder.sourceChange,
|
||||
);
|
||||
return success(
|
||||
DartMigrateResult(
|
||||
summary: summaryBuffer.toString().trim(),
|
||||
edit: workspaceEdit,
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
/// Bumps the SDK constraints in the provided [targets] by 1 minor version.
|
||||
///
|
||||
/// Appends status messages to [summaryBuffer] and returns the computed
|
||||
/// [ChangeBuilder].
|
||||
Future<ChangeBuilder> _bumpPubspecConstraints(
|
||||
List<_PubspecTarget> pubspecTargets,
|
||||
StringBuffer summaryBuffer,
|
||||
) async {
|
||||
var workspace = DartChangeWorkspace(await server.currentSessions);
|
||||
var builder = ChangeBuilder(workspace: workspace);
|
||||
var bumpedLines = <String>[];
|
||||
|
||||
for (var pubspec in pubspecTargets) {
|
||||
var pubspecFile = pubspec.file;
|
||||
var context = server.contextManager.getContextFor(pubspecFile.path);
|
||||
if (context == null) {
|
||||
summaryBuffer.writeln(
|
||||
'- ${pubspec.displayName}: Skipped (not analyzed)',
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
// TODO(kallentu): If any pre-migrations failed, avoid bumping the pubspec
|
||||
// version.
|
||||
|
||||
var versionBumpEdit = computeVersionBumpEdit(pubspecFile);
|
||||
if (versionBumpEdit == null) continue;
|
||||
|
||||
await builder.addYamlFileEdit(pubspecFile.path, (builder) {
|
||||
builder.addSimpleReplacement(
|
||||
SourceRange(versionBumpEdit.offset, versionBumpEdit.length),
|
||||
versionBumpEdit.replacement,
|
||||
);
|
||||
});
|
||||
|
||||
bumpedLines.add(
|
||||
'- ${pubspec.displayName}: ${versionBumpEdit.originalConstraint} -> '
|
||||
'${versionBumpEdit.newConstraint}',
|
||||
);
|
||||
}
|
||||
|
||||
if (bumpedLines.isEmpty) {
|
||||
summaryBuffer.writeln('No SDK constraints were bumped.');
|
||||
} else {
|
||||
summaryBuffer.writeln(
|
||||
'Bumped SDK constraints in ${bumpedLines.length} package(s):',
|
||||
);
|
||||
for (var line in bumpedLines) {
|
||||
summaryBuffer.writeln(line);
|
||||
}
|
||||
}
|
||||
|
||||
return builder;
|
||||
}
|
||||
|
||||
/// Validates that all provided [uris] are directories and each directory
|
||||
@@ -53,7 +126,10 @@ class MigrateHandler
|
||||
///
|
||||
/// Returns an error if any URI points to a file, does not exist, or does
|
||||
/// not contain a `pubspec.yaml` file.
|
||||
ErrorOr<void> _validateMigrationTargets(List<DocumentUri> uris) {
|
||||
ErrorOr<List<_PubspecTarget>> _validateMigrationTargets(
|
||||
List<DocumentUri> uris,
|
||||
) {
|
||||
var targets = <_PubspecTarget>[];
|
||||
for (var uri in uris) {
|
||||
var pathResult = pathOfUri(uri);
|
||||
if (pathResult.isError) {
|
||||
@@ -90,12 +166,15 @@ class MigrateHandler
|
||||
pubspecContent,
|
||||
sourceUrl: pubspecFile.toUri(),
|
||||
);
|
||||
if (pubspec is YamlMap && pubspec['resolution'] == 'workspace') {
|
||||
return error(
|
||||
ErrorCodes.InvalidParams,
|
||||
"The directory '$path' is part of a workspace and can't be migrated"
|
||||
' independently.',
|
||||
);
|
||||
if (pubspec is YamlMap) {
|
||||
if (pubspec['resolution'] == 'workspace') {
|
||||
return error(
|
||||
ErrorCodes.InvalidParams,
|
||||
"The directory '$path' is part of a workspace and can't be"
|
||||
' migrated independently.',
|
||||
);
|
||||
}
|
||||
targets.add(_PubspecTarget(file: pubspecFile, pubspec: pubspec));
|
||||
}
|
||||
} catch (e) {
|
||||
return error(
|
||||
@@ -104,6 +183,21 @@ class MigrateHandler
|
||||
);
|
||||
}
|
||||
}
|
||||
return success(null);
|
||||
return success(targets);
|
||||
}
|
||||
}
|
||||
|
||||
/// A target package's `pubspec.yaml` file and its derived display name.
|
||||
///
|
||||
/// Used to avoid reading and parsing the `pubspec.yaml` file multiple times.
|
||||
class _PubspecTarget {
|
||||
/// The `pubspec.yaml` file for the package.
|
||||
final File file;
|
||||
|
||||
/// The display name of the package, which defaults to the defined package
|
||||
/// name in `pubspec.yaml`, or the parent directory name as a fallback.
|
||||
final String displayName;
|
||||
|
||||
_PubspecTarget({required this.file, required YamlMap pubspec})
|
||||
: displayName = (pubspec['name'] as String?) ?? file.parent.shortName;
|
||||
}
|
||||
|
||||
@@ -18,6 +18,103 @@ void main() {
|
||||
|
||||
@reflectiveTest
|
||||
class MigrateTest extends AbstractLspAnalysisServerTest {
|
||||
Future<void> test_bumpSdkConstraint() async {
|
||||
await _setupProject(
|
||||
pubspecContent: '''
|
||||
name: test_project
|
||||
environment:
|
||||
sdk: '^3.0.0'
|
||||
''',
|
||||
);
|
||||
await _assertMigrationResult(
|
||||
expectedSummary: '''
|
||||
Bumped SDK constraints in 1 package(s):
|
||||
- test_project: ^3.0.0 -> ^3.1.0''',
|
||||
expectedPubspecContent: '''
|
||||
name: test_project
|
||||
environment:
|
||||
sdk: '^3.1.0'
|
||||
''',
|
||||
);
|
||||
}
|
||||
|
||||
Future<void> test_bumpSdkConstraint_emptyPubspec() async {
|
||||
await _setupProject(pubspecContent: '');
|
||||
await _assertMigrationResult(
|
||||
expectedSummary: 'No SDK constraints were bumped.',
|
||||
);
|
||||
}
|
||||
|
||||
Future<void> test_bumpSdkConstraint_multiplePackages() async {
|
||||
await initialize();
|
||||
|
||||
var project1Path = pathContext.join(projectFolderPath, 'project1');
|
||||
var project2Path = pathContext.join(projectFolderPath, 'project2');
|
||||
|
||||
newFile(pathContext.join(project1Path, 'pubspec.yaml'), '''
|
||||
name: project1
|
||||
environment:
|
||||
sdk: '^3.0.0'
|
||||
''');
|
||||
|
||||
newFile(pathContext.join(project2Path, 'pubspec.yaml'), '''
|
||||
name: project2
|
||||
environment:
|
||||
sdk: '^3.2.0'
|
||||
''');
|
||||
|
||||
await _assertMigrationResult(
|
||||
uris: [Uri.file(project1Path), Uri.file(project2Path)],
|
||||
expectedSummary: '''
|
||||
Bumped SDK constraints in 2 package(s):
|
||||
- project1: ^3.0.0 -> ^3.1.0
|
||||
- project2: ^3.2.0 -> ^3.3.0''',
|
||||
);
|
||||
}
|
||||
|
||||
Future<void> test_bumpSdkConstraint_noneBumped() async {
|
||||
await _setupProject(
|
||||
pubspecContent: '''
|
||||
name: test_project
|
||||
''',
|
||||
);
|
||||
await _assertMigrationResult(
|
||||
expectedSummary: 'No SDK constraints were bumped.',
|
||||
);
|
||||
}
|
||||
|
||||
Future<void> test_bumpSdkConstraint_range() async {
|
||||
await _setupProject(
|
||||
pubspecContent: '''
|
||||
name: test_project
|
||||
environment:
|
||||
sdk: '>=3.0.0 <4.0.0'
|
||||
''',
|
||||
);
|
||||
await _assertMigrationResult(
|
||||
expectedSummary: contains('>=3.0.0 <4.0.0 -> >=3.1.0'),
|
||||
expectedPubspecContent: '''
|
||||
name: test_project
|
||||
environment:
|
||||
sdk: '>=3.1.0 <4.0.0'
|
||||
''',
|
||||
);
|
||||
}
|
||||
|
||||
Future<void> test_bumpSdkConstraint_skipped() async {
|
||||
var otherDirPath = convertPath('/other_project');
|
||||
var otherPubspecPath = pathContext.join(otherDirPath, 'pubspec.yaml');
|
||||
|
||||
await _setupProject(
|
||||
pubspecContent: 'name: other_project',
|
||||
customPubspecFilePath: otherPubspecPath,
|
||||
);
|
||||
await _assertMigrationResult(
|
||||
uris: [Uri.file(otherDirPath)],
|
||||
expectedSummary: contains('- other_project: Skipped (not analyzed)'),
|
||||
);
|
||||
}
|
||||
|
||||
Future<void> test_error_directoryWithoutPubspec() async {
|
||||
await initialize();
|
||||
|
||||
@@ -153,16 +250,48 @@ resolution: workspace
|
||||
}
|
||||
|
||||
Future<void> test_validDirectory() async {
|
||||
await initialize();
|
||||
|
||||
newFile(pubspecFilePath, 'name: test_project');
|
||||
await _setupProject(pubspecContent: 'name: test_project');
|
||||
await _assertMigrationResult();
|
||||
}
|
||||
|
||||
Future<void> _assertMigrationResult({
|
||||
List<Uri>? uris,
|
||||
Object? expectedSummary,
|
||||
String? expectedPubspecContent,
|
||||
String expectedPubspecPath = 'pubspec.yaml',
|
||||
}) async {
|
||||
var request = makeRequest(
|
||||
CustomMethods.migrate,
|
||||
DartMigrateParams(uris: [projectFolderUri]),
|
||||
DartMigrateParams(uris: uris ?? [projectFolderUri]),
|
||||
);
|
||||
var response = await sendRequestToServer(request);
|
||||
|
||||
expect(response.error, isNull);
|
||||
|
||||
var result = DartMigrateResult.fromJson(
|
||||
response.result as Map<String, Object?>,
|
||||
);
|
||||
if (expectedSummary != null) {
|
||||
expect(result.summary, expectedSummary);
|
||||
}
|
||||
if (expectedPubspecContent != null) {
|
||||
var workspaceEdit = result.edit!;
|
||||
var expectedContent =
|
||||
'''
|
||||
>>>>>>>>>> $expectedPubspecPath
|
||||
$expectedPubspecContent''';
|
||||
|
||||
verifyEdit(workspaceEdit, expectedContent);
|
||||
}
|
||||
}
|
||||
|
||||
Future<void> _setupProject({
|
||||
required String pubspecContent,
|
||||
String? customPubspecFilePath,
|
||||
}) async {
|
||||
await initialize();
|
||||
|
||||
var pubspecPath = customPubspecFilePath ?? pubspecFilePath;
|
||||
newFile(pubspecPath, pubspecContent);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user