Skip to content

Commit

Permalink
(new) formatter language version-aware migrations
Browse files Browse the repository at this point in the history
(Testing to follow.)

See: #56685




Change-Id: I9da0e3868cd3c873c41e8a19a77fad58b9b8dea0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/386901
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Phil Quitslund <[email protected]>
  • Loading branch information
pq authored and Commit Queue committed Sep 26, 2024
1 parent a96c690 commit 13d84fe
Show file tree
Hide file tree
Showing 8 changed files with 134 additions and 12 deletions.
7 changes: 4 additions & 3 deletions pkg/analysis_server/lib/src/g3/utilities.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@ import 'package:analyzer/src/dart/scanner/scanner.dart';
import 'package:analyzer/src/generated/parser.dart' as p;
import 'package:analyzer/src/string_source.dart';
import 'package:dart_style/dart_style.dart';
import 'package:pub_semver/pub_semver.dart';

/// Return a formatted string if successful, throws a [FormatterException] if
/// unable to format. Takes a string as input.
String format(String content) {
/// unable to format. Takes a string as input and an optional [languageVersion].
String format(String content, {Version? languageVersion}) {
var code = SourceCode(content);
var formatter = DartFormatter();
var formatter = DartFormatter(languageVersion: languageVersion);
SourceCode formattedResult;
formattedResult = formatter.formatSource(code);
return formattedResult.text;
Expand Down
7 changes: 6 additions & 1 deletion pkg/analysis_server/lib/src/handler/legacy/edit_format.dart
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,12 @@ class EditFormatHandler extends LegacyHandler {
selectionStart: start,
selectionLength: length,
);
var formatter = DartFormatter(pageWidth: params.lineLength);

var driver = server.getAnalysisDriver(file);
var library = await driver?.getResolvedLibrary(file);
var formatter = DartFormatter(
pageWidth: params.lineLength,
languageVersion: library.effectiveLanguageVersion);
SourceCode formattedResult;
try {
formattedResult = formatter.formatSource(code);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import 'package:analyzer/src/util/file_paths.dart' as file_paths;
import 'package:dart_style/src/dart_formatter.dart';
import 'package:dart_style/src/exceptions.dart';
import 'package:dart_style/src/source_code.dart';
import 'package:pub_semver/pub_semver.dart';

/// The handler for the `edit.formatIfEnabled` request.
class EditFormatIfEnabledHandler extends LegacyHandler {
Expand All @@ -21,17 +22,18 @@ class EditFormatIfEnabledHandler extends LegacyHandler {
EditFormatIfEnabledHandler(
super.server, super.request, super.cancellationToken, super.performance);

/// Format the given [file].
/// Format the given [file] with the given [languageVersion].
///
/// Throws a [FileSystemException] if the file doesn't exist or can't be read.
/// Throws a [FormatterException] if the code could not be formatted.
List<SourceEdit> formatFile(File file) {
List<SourceEdit> formatFile(File file, {Version? languageVersion}) {
// TODO(brianwilkerson): Move this to a superclass when `edit.format` is
// implemented by a handler class so the code can be shared.
var originalContent = file.readAsStringSync();
var code = SourceCode(originalContent);

var formatter = DartFormatter();
var formatter = DartFormatter(languageVersion: languageVersion);

var formatResult = formatter.formatSource(code);
var formattedContent = formatResult.text;

Expand All @@ -56,16 +58,16 @@ class EditFormatIfEnabledHandler extends LegacyHandler {
);
var sourceFileEdits = <SourceFileEdit>[];
for (var context in collection.contexts) {
_formatInContext(context, sourceFileEdits);
await _formatInContext(context, sourceFileEdits);
}
sendResult(EditFormatIfEnabledResult(sourceFileEdits));
}

/// Format all of the Dart files in the given [context] whose associated
/// `codeStyleOptions` enable formatting, adding the edits to the list of
/// [sourceFileEdits].
void _formatInContext(DriverBasedAnalysisContext context,
List<SourceFileEdit> sourceFileEdits) {
Future<void> _formatInContext(DriverBasedAnalysisContext context,
List<SourceFileEdit> sourceFileEdits) async {
var pathContext = context.resourceProvider.pathContext;
for (var filePath in context.contextRoot.analyzedFiles()) {
// Skip anything but .dart files.
Expand All @@ -80,7 +82,10 @@ class EditFormatIfEnabledHandler extends LegacyHandler {
var options = context.getAnalysisOptionsForFile(resource);
if (options.codeStyleOptions.useFormatter) {
try {
var sourceEdits = formatFile(resource);
var library = await context.driver.getResolvedLibrary(filePath);
var languageVersion = library.effectiveLanguageVersion;
var sourceEdits =
formatFile(resource, languageVersion: languageVersion);
if (sourceEdits.isNotEmpty) {
sourceFileEdits
.add(SourceFileEdit(filePath, 0, edits: sourceEdits));
Expand Down
12 changes: 12 additions & 0 deletions pkg/analysis_server/lib/src/handler/legacy/legacy_handler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ import 'package:analysis_server/protocol/protocol.dart';
import 'package:analysis_server/protocol/protocol_generated.dart';
import 'package:analysis_server/src/legacy_analysis_server.dart';
import 'package:analysis_server/src/protocol/protocol_internal.dart';
import 'package:analyzer/dart/analysis/results.dart';
import 'package:analyzer/error/error.dart';
import 'package:analyzer/src/dart/error/syntactic_errors.g.dart';
import 'package:analyzer/src/util/performance/operation_performance.dart';
import 'package:analyzer/src/utilities/cancellation.dart';
import 'package:pub_semver/pub_semver.dart';

/// A request handler for the completion domain.
abstract class CompletionHandler extends LegacyHandler {
Expand Down Expand Up @@ -96,3 +98,13 @@ abstract class LegacyHandler {
params.toNotification(clientUriConverter: server.uriConverter));
}
}

extension SomeResolvedLibraryResultExtension on SomeResolvedLibraryResult? {
Version? get effectiveLanguageVersion {
var self = this;
if (self is ResolvedLibraryResult) {
return self.element.languageVersion.effective;
}
return null;
}
}
6 changes: 5 additions & 1 deletion pkg/analysis_server/lib/src/lsp/source_edits.dart
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,11 @@ ErrorOr<List<TextEdit>?> generateEditsForFormatting(
// Create a new formatter on every request because it may contain state that
// affects repeated formats.
// https://github.com/dart-lang/dart_style/issues/1337
formattedResult = DartFormatter(pageWidth: lineLength).formatSource(code);
var languageVersion =
result.unit.declaredElement?.library.languageVersion.effective;
formattedResult =
DartFormatter(pageWidth: lineLength, languageVersion: languageVersion)
.formatSource(code);
} on FormatterException {
// If the document fails to parse, just return no edits to avoid the
// use seeing edits on every save with invalid code (if LSP gains the
Expand Down
1 change: 1 addition & 0 deletions pkg/analysis_server/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ dependencies:
memory_usage: any
meta: any
path: any
pub_semver: any
stream_channel: any
telemetry: any
test: any
Expand Down
44 changes: 44 additions & 0 deletions pkg/analysis_server/test/integration/edit/format_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import '../support/integration_tests.dart';
void main() {
defineReflectiveSuite(() {
defineReflectiveTests(FormatTest);
defineReflectiveTests(LanguageVersionSpecificFormatTest);
});
}

Expand Down Expand Up @@ -76,3 +77,46 @@ class Class1 {
}
}
}

@reflectiveTest
class LanguageVersionSpecificFormatTest
extends AbstractAnalysisServerIntegrationTest {
Future<String> createTestFile(String text) async {
var pathname = sourcePath('test.dart');
writeFile(pathname, text);
await standardAnalysisSetup();
return pathname;
}

Future<void> test_format_short() async {
var path = await createTestFile('''
// @dart = 3.5
void f({String? argument1, String? argument2}) {}
void g() {
f(argument1: 'An argument', argument2: 'Another argument');
}
''');

var result = await sendEditFormat(path, 0, 0);

// No change in short style.
expect(result.edits, isEmpty);
}

@FailingTest(issue: 'https://github.com/dart-lang/sdk/issues/56685')
Future<void> test_format_tall() async {
var path = await createTestFile('''
void f({String? argument1, String? argument2}) {}
void g() {
f(argument1: 'An argument', argument2: 'Another argument');
}
''');

var result = await sendEditFormat(path, 0, 0);
expect(result.edits, isNotEmpty);
// TODO(pq): update expectations when formatter is complete
}
}
50 changes: 50 additions & 0 deletions pkg/analysis_server/test/lsp/source_edits_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,56 @@ Delete 1:18-2:1
);
}

Future<void> test_minimalEdits_formatting_shortStyle() async {
const startContent = '''
// @dart = 3.5
void f({String argument1, String argument2}) {}
void g() {
f(argument1: 'An argument', argument2: 'Another argument');
}
''';
const endContent = '''
// @dart = 3.5
void f({String argument1, String argument2}) {}
void g() {
f(argument1: 'An argument', argument2: 'Another argument');
}
''';
const expectedEdits = '';

await _assertMinimalEdits(startContent, endContent, expectedEdits);
}

@FailingTest(issue: 'https://github.com/dart-lang/sdk/issues/56685')
Future<void> test_minimalEdits_formatting_tallStyle() async {
const startContent = '''
void f({String? argument1, String? argument2}) {}
void g() {
f(argument1: 'An argument', argument2: 'Another argument');
}
''';
const endContent = '''
void f({String? argument1, String? argument2}) {}
void g() {
f(argument1: 'An argument',
argument2: 'Another argument',
);
}
''';
const expectedEdits = r'''
Insert "\n " at 4:31
Insert ",\n" at 4:60
''';

await _assertMinimalEdits(startContent, endContent, expectedEdits);
}

Future<void> test_minimalEdits_gt_2_combined() async {
const startContent = '''
List<
Expand Down

0 comments on commit 13d84fe

Please sign in to comment.