[analyzer] Add support for "formatter" in analysis_options + use page_width in LSP

This adds support for validation + completion for `formatter/page_width` in analysis_options, and uses this value in preference to the client-supplied value in the LSP server.

It does not yet add support for the legacy protocol, and there are a few questions in #56864 (comment).

See #56864

Change-Id: I7844e3abd1191beb9cc24197bbc8aa7c9e9ad4fa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/388822
Commit-Queue: Brian Wilkerson <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>

Author: DanTup

pkg/analysis_server/lib/src/lsp/client_configuration.dart

@@ -286,7 +286,8 @@ class LspResourceClientConfiguration {
         true;
   }
 
-  /// The line length used when formatting documents.
+  /// The line length used when formatting documents if not specified in
+  /// `analysis_options.yaml`.
   ///
   /// If null, the formatters default will be used.
   int? get lineLength =>

pkg/analysis_server/lib/src/lsp/handlers/handler_format_on_type.dart

@@ -39,7 +39,7 @@ class FormatOnTypeHandler extends SharedMessageHandler<
     }
 
     var lineLength = server.lspClientConfiguration.forResource(path).lineLength;
-    return generateEditsForFormatting(result, lineLength);
+    return generateEditsForFormatting(result, defaultPageWidth: lineLength);
   }
 
   @override

pkg/analysis_server/lib/src/lsp/handlers/handler_format_range.dart

@@ -39,7 +39,8 @@ class FormatRangeHandler extends SharedMessageHandler<
     }
 
     var lineLength = server.lspClientConfiguration.forResource(path).lineLength;
-    return generateEditsForFormatting(result, lineLength, range: range);
+    return generateEditsForFormatting(result,
+        defaultPageWidth: lineLength, range: range);
   }
 
   @override

pkg/analysis_server/lib/src/lsp/handlers/handler_formatting.dart

@@ -39,7 +39,7 @@ class FormattingHandler
     }
 
     var lineLength = server.lspClientConfiguration.forResource(path).lineLength;
-    return generateEditsForFormatting(result, lineLength);
+    return generateEditsForFormatting(result, defaultPageWidth: lineLength);
   }
 
   @override

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

@@ -78,13 +78,25 @@ ErrorOr<({String content, List<plugin.SourceEdit> edits})>
   return ErrorOr.success((content: newContent, edits: serverEdits));
 }
 
+/// Generates a list of [TextEdit]s to format the code for [result].
+///
+/// [defaultPageWidth] will be used as the default page width if [result] does
+/// not have an analysis_options file that defines a page width.
+///
+/// If [range] is provided, only edits that intersect with this range will be
+/// returned.
 ErrorOr<List<TextEdit>?> generateEditsForFormatting(
-  ParsedUnitResult result,
-  int? lineLength, {
+  ParsedUnitResult result, {
+  int? defaultPageWidth,
   Range? range,
 }) {
   var unformattedSource = result.content;
 
+  // The analysis options page width always takes priority over the default from
+  // the LSP configuration.
+  var effectivePageWidth =
+      result.analysisOptions.formatterOptions.pageWidth ?? defaultPageWidth;
+
   var code = SourceCode(unformattedSource);
   SourceCode formattedResult;
   try {
@@ -94,9 +106,9 @@ ErrorOr<List<TextEdit>?> generateEditsForFormatting(
     var languageVersion =
         result.unit.declaredElement?.library.languageVersion.effective ??
             DartFormatter.latestLanguageVersion;
-    formattedResult =
-        DartFormatter(pageWidth: lineLength, languageVersion: languageVersion)
-            .formatSource(code);
+    var formatter = DartFormatter(
+        pageWidth: effectivePageWidth, languageVersion: languageVersion);
+    formattedResult = formatter.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

pkg/analysis_server/lib/src/services/completion/yaml/analysis_options_generator.dart

@@ -39,6 +39,9 @@ class AnalysisOptionsGenerator extends YamlCompletionGenerator {
     AnalyzerOptions.codeStyle: MapProducer({
       AnalyzerOptions.format: BooleanProducer(),
     }),
+    AnalyzerOptions.formatter: MapProducer({
+      AnalyzerOptions.pageWidth: EmptyProducer(),
+    }),
     // TODO(brianwilkerson): Create a producer to produce `package:` URIs.
     AnalyzerOptions.include: EmptyProducer(),
     // TODO(brianwilkerson): Create constants for 'linter' and 'rules'.

pkg/analysis_server/lib/src/services/correction/bulk_fix_processor.dart

@@ -829,7 +829,7 @@ class BulkFixProcessor {
           continue;
         }
 
-        var formatResult = generateEditsForFormatting(result, null);
+        var formatResult = generateEditsForFormatting(result);
         await formatResult.mapResult((formatResult) async {
           var edits = formatResult ?? [];
           if (edits.isNotEmpty) {

pkg/analysis_server/test/domain_completion_test.dart

@@ -2035,6 +2035,8 @@ suggestions
     kind: identifier
   |code-style: |
     kind: identifier
+  |formatter: |
+    kind: identifier
   |include: |
     kind: identifier
   |linter: |

pkg/analysis_server/test/lsp/format_test.dart

@@ -20,6 +20,11 @@ void main() {
 
 @reflectiveTest
 class FormatTest extends AbstractLspAnalysisServerTest {
+  /// Some sample code that is over 50 characters and will be wrapped if the
+  /// page width is set to 40.
+  static const codeThatWrapsAt40 =
+      "var a = '        10        20        30        40';";
+
   Future<List<TextEdit>> expectFormattedContents(
       Uri uri, String original, String expected) async {
     var formatEdits = (await formatDocument(uri))!;
@@ -36,6 +41,11 @@ class FormatTest extends AbstractLspAnalysisServerTest {
     return formatEdits;
   }
 
+  Future<String> formatContents(Uri uri, String original) async {
+    var formatEdits = (await formatDocument(uri))!;
+    return applyTextEdits(original, formatEdits);
+  }
+
   Future<void> test_alreadyFormatted() async {
     const contents = '''
 void f() {
@@ -383,6 +393,83 @@ void f() => print('123456789 123456789 123456789 123456789 123456789 123456789 1
     await expectFormattedContents(mainFileUri, contents, expectedLongLines);
   }
 
+  Future<void> test_lineLength_analysisOptions() async {
+    const codeContent = codeThatWrapsAt40;
+    const optionsContent = '''
+formatter:
+  page_width: 40
+''';
+
+    newFile(analysisOptionsPath, optionsContent);
+    newFile(mainFilePath, codeContent);
+    await initialize();
+
+    // Ignore trailing newlines when checking for wrapping.
+    var formatted = await formatContents(mainFileUri, codeContent);
+    expect(formatted.trim(), contains('\n'));
+  }
+
+  Future<void> test_lineLength_analysisOptions_nestedEmpty() async {
+    const codeContent = codeThatWrapsAt40;
+    const optionsContent = '''
+formatter:
+  page_width: 40
+''';
+    var nestedAnalysisOptionsPath =
+        join(projectFolderPath, 'lib', 'analysis_options.yaml');
+
+    newFile(analysisOptionsPath, optionsContent);
+    newFile(
+        nestedAnalysisOptionsPath, '# empty'); // suppress the parent options.
+    newFile(mainFilePath, codeContent);
+    await initialize();
+
+    // Ignore trailing newlines when checking for wrapping.
+    var formatted = await formatContents(mainFileUri, codeContent);
+    expect(formatted.trim(), isNot(contains('\n')));
+  }
+
+  Future<void> test_lineLength_analysisOptions_nestedIncludes() async {
+    const codeContent = codeThatWrapsAt40;
+    const optionsContent = '''
+formatter:
+  page_width: 40
+''';
+    var nestedAnalysisOptionsPath =
+        join(projectFolderPath, 'lib', 'analysis_options.yaml');
+
+    newFile(analysisOptionsPath, optionsContent);
+    newFile(nestedAnalysisOptionsPath, 'include: ../analysis_options.yaml');
+    newFile(mainFilePath, codeContent);
+    await initialize();
+
+    // Ignore trailing newlines when checking for wrapping.
+    var formatted = await formatContents(mainFileUri, codeContent);
+    expect(formatted.trim(), contains('\n'));
+  }
+
+  Future<void> test_lineLength_analysisOptions_overridesConfig() async {
+    const codeContent = codeThatWrapsAt40;
+    const optionsContent = '''
+formatter:
+  page_width: 40
+''';
+
+    newFile(analysisOptionsPath, optionsContent);
+    newFile(mainFilePath, codeContent);
+    await provideConfig(
+      initialize,
+      {
+        // This won't apply because analysis_options wins.
+        'lineLength': 100,
+      },
+    );
+
+    // Ignore trailing newlines when checking for wrapping.
+    var formatted = await formatContents(mainFileUri, codeContent);
+    expect(formatted.trim(), contains('\n'));
+  }
+
   Future<void> test_lineLength_outsideWorkspaceFolders() async {
     const contents = '''
 void f() {

pkg/analysis_server/test/src/services/completion/yaml/analysis_options_generator_test.dart

@@ -128,11 +128,20 @@ code-style:
     getCompletions('^');
     assertSuggestion('${AnalyzerOptions.analyzer}: ');
     assertSuggestion('${AnalyzerOptions.codeStyle}: ');
+    assertSuggestion('${AnalyzerOptions.formatter}: ');
     assertSuggestion('${AnalyzerOptions.include}: ');
     // TODO(brianwilkerson): Replace this with a constant.
     assertSuggestion('linter: ');
   }
 
+  void test_formatter() {
+    getCompletions('''
+formatter:
+  ^
+''');
+    assertSuggestion('${AnalyzerOptions.pageWidth}: ');
+  }
+
   void test_linter() {
     getCompletions('''
 linter:

pkg/analysis_server/tool/lsp_spec/README.md

@@ -36,7 +36,7 @@ Client workspace settings are requested with `workspace/configuration` during in
 
 - `dart.analysisExcludedFolders` (`List<String>?`): An array of paths (absolute or relative to each workspace folder) that should be excluded from analysis.
 - `dart.enableSdkFormatter` (`bool?`): When set to `false`, prevents registration (or unregisters) the SDK formatter. When set to `true` or not supplied, will register/reregister the SDK formatter.
-- `dart.lineLength` (`int?`): The number of characters the formatter should wrap code at. If unspecified, code will be wrapped at `80` characters.
+- `dart.lineLength` (`int?`): Sets a default value for the formatter to wrap code at if no value is specified in `formatter.page_width` in `analysis_options.yaml`. If unspecified by both, code will be wrapped at `80` characters.
 - `dart.completeFunctionCalls` (`bool?`): When set to true, completes functions/methods with their required parameters.
 - `dart.showTodos` (`bool?`): Whether to generate diagnostics for TODO comments. If unspecified, diagnostics will not be generated.
 - `dart.renameFilesWithClasses` (`String`): When set to `"always"`, will include edits to rename files when classes are renamed if the filename matches the class name (but in snake_form). When set to `"prompt"`, a prompt will be shown on each class rename asking to confirm the file rename. Otherwise, files will not be renamed. Renames are performed using LSP's ResourceOperation edits - that means the rename is simply included in the resulting `WorkspaceEdit` and must be handled by the client.

pkg/analyzer/lib/dart/analysis/analysis_options.dart

@@ -4,6 +4,7 @@
 
 import 'package:analyzer/dart/analysis/code_style_options.dart';
 import 'package:analyzer/dart/analysis/features.dart';
+import 'package:analyzer/dart/analysis/formatter_options.dart';
 import 'package:analyzer/source/error_processor.dart';
 import 'package:analyzer/src/lint/linter.dart';
 import 'package:pub_semver/src/version_constraint.dart';
@@ -38,6 +39,9 @@ abstract class AnalysisOptions {
   /// analysis.
   List<String> get excludePatterns;
 
+  /// Return the options used to control the formatter.
+  FormatterOptions get formatterOptions;
+
   /// Return `true` if analysis is to generate hint results (e.g. best practices
   /// and analysis based on certain annotations).
   @Deprecated("Use 'warning' instead")

pkg/analyzer/lib/dart/analysis/formatter_options.dart

@@ -0,0 +1,12 @@
+// 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.
+
+/// A set of options related to the formatter that apply to the code within a
+/// single analysis context.
+///
+/// Clients may not extend, implement or mix-in this class.
+abstract class FormatterOptions {
+  /// The width configured for where the formatter should wrap code.
+  int? get pageWidth;
+}

pkg/analyzer/lib/src/analysis_options/apply_options.dart

@@ -4,9 +4,11 @@
 
 import 'package:analyzer/dart/analysis/code_style_options.dart';
 import 'package:analyzer/dart/analysis/features.dart';
+import 'package:analyzer/dart/analysis/formatter_options.dart';
 import 'package:analyzer/error/error.dart';
 import 'package:analyzer/source/error_processor.dart';
 import 'package:analyzer/src/analysis_options/code_style_options.dart';
+import 'package:analyzer/src/analysis_options/formatter_options.dart';
 import 'package:analyzer/src/dart/analysis/experiments.dart';
 import 'package:analyzer/src/generated/engine.dart';
 import 'package:analyzer/src/generated/utilities_general.dart';
@@ -148,6 +150,18 @@ extension on AnalysisOptionsImpl {
     return CodeStyleOptionsImpl(this, useFormatter: useFormatter);
   }
 
+  FormatterOptions buildFormatterOptions(YamlNode? formatter) {
+    int? pageWidth;
+    if (formatter is YamlMap) {
+      var formatNode = formatter.valueAt(AnalyzerOptions.pageWidth);
+      var formatValue = formatNode?.value;
+      if (formatValue is int && formatValue > 0) {
+        pageWidth = formatValue;
+      }
+    }
+    return FormatterOptionsImpl(this, pageWidth: pageWidth);
+  }
+
   void _applyLegacyPlugins(YamlNode? plugins) {
     var pluginName = plugins.stringValue;
     if (pluginName != null) {
@@ -226,6 +240,10 @@ extension AnalysisOptionsImplExtensions on AnalysisOptionsImpl {
     var codeStyle = optionMap.valueAt(AnalyzerOptions.codeStyle);
     codeStyleOptions = buildCodeStyleOptions(codeStyle);
 
+    // Process the 'formatter' option.
+    var formatter = optionMap.valueAt(AnalyzerOptions.formatter);
+    formatterOptions = buildFormatterOptions(formatter);
+
     var config = parseConfig(optionMap);
     if (config != null) {
       var enabledRules = Registry.ruleRegistry.enabled(config);

pkg/analyzer/lib/src/analysis_options/formatter_options.dart

@@ -0,0 +1,18 @@
+// 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:analyzer/dart/analysis/analysis_options.dart';
+import 'package:analyzer/dart/analysis/formatter_options.dart';
+
+/// The concrete implementation of [FormatterOptions].
+class FormatterOptionsImpl implements FormatterOptions {
+  /// The analysis options that owns this instance.
+  final AnalysisOptions options;
+
+  /// The width configured for where the formatter should wrap code.
+  @override
+  final int? pageWidth;
+
+  FormatterOptionsImpl(this.options, {this.pageWidth});
+}

pkg/analyzer/lib/src/generated/engine.dart

@@ -9,13 +9,15 @@ import 'package:analyzer/dart/analysis/analysis_options.dart';
 import 'package:analyzer/dart/analysis/code_style_options.dart';
 import 'package:analyzer/dart/analysis/declared_variables.dart';
 import 'package:analyzer/dart/analysis/features.dart';
+import 'package:analyzer/dart/analysis/formatter_options.dart';
 import 'package:analyzer/error/error.dart';
 import 'package:analyzer/file_system/file_system.dart';
 import 'package:analyzer/instrumentation/instrumentation.dart';
 import 'package:analyzer/source/error_processor.dart';
 import 'package:analyzer/source/line_info.dart';
 import 'package:analyzer/source/source.dart';
 import 'package:analyzer/src/analysis_options/code_style_options.dart';
+import 'package:analyzer/src/analysis_options/formatter_options.dart';
 import 'package:analyzer/src/dart/analysis/experiments.dart';
 import 'package:analyzer/src/generated/source.dart' show SourceFactory;
 import 'package:analyzer/src/lint/linter.dart';
@@ -239,6 +241,9 @@ class AnalysisOptionsImpl implements AnalysisOptions {
   @override
   late CodeStyleOptions codeStyleOptions;
 
+  @override
+  late FormatterOptions formatterOptions;
+
   /// The set of "un-ignorable" error names, as parsed in [AnalyzerOptions] from
   /// an analysis options file.
   Set<String> unignorableNames = {};
@@ -247,12 +252,14 @@ class AnalysisOptionsImpl implements AnalysisOptions {
   /// values.
   AnalysisOptionsImpl({this.file}) {
     codeStyleOptions = CodeStyleOptionsImpl(this, useFormatter: false);
+    formatterOptions = FormatterOptionsImpl(this);
   }
 
   /// Initialize a newly created set of analysis options to have the same values
   /// as those in the given set of analysis [options].
   AnalysisOptionsImpl.from(AnalysisOptions options) {
     codeStyleOptions = options.codeStyleOptions;
+    formatterOptions = options.formatterOptions;
     contextFeatures = options.contextFeatures;
     enabledLegacyPluginNames = options.enabledLegacyPluginNames;
     errorProcessors = options.errorProcessors;