Skip to content

Commit 13d84fe

Browse files
pqCommit Queue
authored and
Commit Queue
committedSep 26, 2024
(new) formatter language version-aware migrations
(Testing to follow.) See: #56685 Change-Id: I9da0e3868cd3c873c41e8a19a77fad58b9b8dea0 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/386901 Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Commit-Queue: Phil Quitslund <pquitslund@google.com>
1 parent a96c690 commit 13d84fe

File tree

8 files changed

+134
-12
lines changed

8 files changed

+134
-12
lines changed
 

‎pkg/analysis_server/lib/src/g3/utilities.dart

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,13 @@ import 'package:analyzer/src/dart/scanner/scanner.dart';
1616
import 'package:analyzer/src/generated/parser.dart' as p;
1717
import 'package:analyzer/src/string_source.dart';
1818
import 'package:dart_style/dart_style.dart';
19+
import 'package:pub_semver/pub_semver.dart';
1920

2021
/// Return a formatted string if successful, throws a [FormatterException] if
21-
/// unable to format. Takes a string as input.
22-
String format(String content) {
22+
/// unable to format. Takes a string as input and an optional [languageVersion].
23+
String format(String content, {Version? languageVersion}) {
2324
var code = SourceCode(content);
24-
var formatter = DartFormatter();
25+
var formatter = DartFormatter(languageVersion: languageVersion);
2526
SourceCode formattedResult;
2627
formattedResult = formatter.formatSource(code);
2728
return formattedResult.text;

‎pkg/analysis_server/lib/src/handler/legacy/edit_format.dart

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,12 @@ class EditFormatHandler extends LegacyHandler {
4848
selectionStart: start,
4949
selectionLength: length,
5050
);
51-
var formatter = DartFormatter(pageWidth: params.lineLength);
51+
52+
var driver = server.getAnalysisDriver(file);
53+
var library = await driver?.getResolvedLibrary(file);
54+
var formatter = DartFormatter(
55+
pageWidth: params.lineLength,
56+
languageVersion: library.effectiveLanguageVersion);
5257
SourceCode formattedResult;
5358
try {
5459
formattedResult = formatter.formatSource(code);

‎pkg/analysis_server/lib/src/handler/legacy/edit_format_if_enabled.dart

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import 'package:analyzer/src/util/file_paths.dart' as file_paths;
1313
import 'package:dart_style/src/dart_formatter.dart';
1414
import 'package:dart_style/src/exceptions.dart';
1515
import 'package:dart_style/src/source_code.dart';
16+
import 'package:pub_semver/pub_semver.dart';
1617

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

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

34-
var formatter = DartFormatter();
35+
var formatter = DartFormatter(languageVersion: languageVersion);
36+
3537
var formatResult = formatter.formatSource(code);
3638
var formattedContent = formatResult.text;
3739

@@ -56,16 +58,16 @@ class EditFormatIfEnabledHandler extends LegacyHandler {
5658
);
5759
var sourceFileEdits = <SourceFileEdit>[];
5860
for (var context in collection.contexts) {
59-
_formatInContext(context, sourceFileEdits);
61+
await _formatInContext(context, sourceFileEdits);
6062
}
6163
sendResult(EditFormatIfEnabledResult(sourceFileEdits));
6264
}
6365

6466
/// Format all of the Dart files in the given [context] whose associated
6567
/// `codeStyleOptions` enable formatting, adding the edits to the list of
6668
/// [sourceFileEdits].
67-
void _formatInContext(DriverBasedAnalysisContext context,
68-
List<SourceFileEdit> sourceFileEdits) {
69+
Future<void> _formatInContext(DriverBasedAnalysisContext context,
70+
List<SourceFileEdit> sourceFileEdits) async {
6971
var pathContext = context.resourceProvider.pathContext;
7072
for (var filePath in context.contextRoot.analyzedFiles()) {
7173
// Skip anything but .dart files.
@@ -80,7 +82,10 @@ class EditFormatIfEnabledHandler extends LegacyHandler {
8082
var options = context.getAnalysisOptionsForFile(resource);
8183
if (options.codeStyleOptions.useFormatter) {
8284
try {
83-
var sourceEdits = formatFile(resource);
85+
var library = await context.driver.getResolvedLibrary(filePath);
86+
var languageVersion = library.effectiveLanguageVersion;
87+
var sourceEdits =
88+
formatFile(resource, languageVersion: languageVersion);
8489
if (sourceEdits.isNotEmpty) {
8590
sourceFileEdits
8691
.add(SourceFileEdit(filePath, 0, edits: sourceEdits));

‎pkg/analysis_server/lib/src/handler/legacy/legacy_handler.dart

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,12 @@ import 'package:analysis_server/protocol/protocol.dart';
99
import 'package:analysis_server/protocol/protocol_generated.dart';
1010
import 'package:analysis_server/src/legacy_analysis_server.dart';
1111
import 'package:analysis_server/src/protocol/protocol_internal.dart';
12+
import 'package:analyzer/dart/analysis/results.dart';
1213
import 'package:analyzer/error/error.dart';
1314
import 'package:analyzer/src/dart/error/syntactic_errors.g.dart';
1415
import 'package:analyzer/src/util/performance/operation_performance.dart';
1516
import 'package:analyzer/src/utilities/cancellation.dart';
17+
import 'package:pub_semver/pub_semver.dart';
1618

1719
/// A request handler for the completion domain.
1820
abstract class CompletionHandler extends LegacyHandler {
@@ -96,3 +98,13 @@ abstract class LegacyHandler {
9698
params.toNotification(clientUriConverter: server.uriConverter));
9799
}
98100
}
101+
102+
extension SomeResolvedLibraryResultExtension on SomeResolvedLibraryResult? {
103+
Version? get effectiveLanguageVersion {
104+
var self = this;
105+
if (self is ResolvedLibraryResult) {
106+
return self.element.languageVersion.effective;
107+
}
108+
return null;
109+
}
110+
}

‎pkg/analysis_server/lib/src/lsp/source_edits.dart

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,11 @@ ErrorOr<List<TextEdit>?> generateEditsForFormatting(
9191
// Create a new formatter on every request because it may contain state that
9292
// affects repeated formats.
9393
// https://github.com/dart-lang/dart_style/issues/1337
94-
formattedResult = DartFormatter(pageWidth: lineLength).formatSource(code);
94+
var languageVersion =
95+
result.unit.declaredElement?.library.languageVersion.effective;
96+
formattedResult =
97+
DartFormatter(pageWidth: lineLength, languageVersion: languageVersion)
98+
.formatSource(code);
9599
} on FormatterException {
96100
// If the document fails to parse, just return no edits to avoid the
97101
// use seeing edits on every save with invalid code (if LSP gains the

‎pkg/analysis_server/pubspec.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ dependencies:
2424
memory_usage: any
2525
meta: any
2626
path: any
27+
pub_semver: any
2728
stream_channel: any
2829
telemetry: any
2930
test: any

‎pkg/analysis_server/test/integration/edit/format_test.dart

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import '../support/integration_tests.dart';
1010
void main() {
1111
defineReflectiveSuite(() {
1212
defineReflectiveTests(FormatTest);
13+
defineReflectiveTests(LanguageVersionSpecificFormatTest);
1314
});
1415
}
1516

@@ -76,3 +77,46 @@ class Class1 {
7677
}
7778
}
7879
}
80+
81+
@reflectiveTest
82+
class LanguageVersionSpecificFormatTest
83+
extends AbstractAnalysisServerIntegrationTest {
84+
Future<String> createTestFile(String text) async {
85+
var pathname = sourcePath('test.dart');
86+
writeFile(pathname, text);
87+
await standardAnalysisSetup();
88+
return pathname;
89+
}
90+
91+
Future<void> test_format_short() async {
92+
var path = await createTestFile('''
93+
// @dart = 3.5
94+
95+
void f({String? argument1, String? argument2}) {}
96+
97+
void g() {
98+
f(argument1: 'An argument', argument2: 'Another argument');
99+
}
100+
''');
101+
102+
var result = await sendEditFormat(path, 0, 0);
103+
104+
// No change in short style.
105+
expect(result.edits, isEmpty);
106+
}
107+
108+
@FailingTest(issue: 'https://github.com/dart-lang/sdk/issues/56685')
109+
Future<void> test_format_tall() async {
110+
var path = await createTestFile('''
111+
void f({String? argument1, String? argument2}) {}
112+
113+
void g() {
114+
f(argument1: 'An argument', argument2: 'Another argument');
115+
}
116+
''');
117+
118+
var result = await sendEditFormat(path, 0, 0);
119+
expect(result.edits, isNotEmpty);
120+
// TODO(pq): update expectations when formatter is complete
121+
}
122+
}

‎pkg/analysis_server/test/lsp/source_edits_test.dart

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,56 @@ Delete 1:18-2:1
356356
);
357357
}
358358

359+
Future<void> test_minimalEdits_formatting_shortStyle() async {
360+
const startContent = '''
361+
// @dart = 3.5
362+
363+
void f({String argument1, String argument2}) {}
364+
365+
void g() {
366+
f(argument1: 'An argument', argument2: 'Another argument');
367+
}
368+
''';
369+
const endContent = '''
370+
// @dart = 3.5
371+
372+
void f({String argument1, String argument2}) {}
373+
374+
void g() {
375+
f(argument1: 'An argument', argument2: 'Another argument');
376+
}
377+
''';
378+
const expectedEdits = '';
379+
380+
await _assertMinimalEdits(startContent, endContent, expectedEdits);
381+
}
382+
383+
@FailingTest(issue: 'https://github.com/dart-lang/sdk/issues/56685')
384+
Future<void> test_minimalEdits_formatting_tallStyle() async {
385+
const startContent = '''
386+
void f({String? argument1, String? argument2}) {}
387+
388+
void g() {
389+
f(argument1: 'An argument', argument2: 'Another argument');
390+
}
391+
''';
392+
const endContent = '''
393+
void f({String? argument1, String? argument2}) {}
394+
395+
void g() {
396+
f(argument1: 'An argument',
397+
argument2: 'Another argument',
398+
);
399+
}
400+
''';
401+
const expectedEdits = r'''
402+
Insert "\n " at 4:31
403+
Insert ",\n" at 4:60
404+
''';
405+
406+
await _assertMinimalEdits(startContent, endContent, expectedEdits);
407+
}
408+
359409
Future<void> test_minimalEdits_gt_2_combined() async {
360410
const startContent = '''
361411
List<

0 commit comments

Comments
 (0)