Use "@Dart=" comment when determining which style rules to apply. (#1766)

Use "@Dart=" comment when determining which style rules to apply.

The formatter correctly used those comments to switch between the old
short and new tall style, but ignored them for language versioned style
rule changes after 3.7. Now the language version of the file is
consistently respected for all style rules.

In the process of fixing this, I made two other clean-ups:

* Introduce a FormattingStyle class separate from DartFormatter. The
  latter had dual use as both the public API used to configure
  formatting and the internal object passed around by the formatter to
  access that configuration. That was annoying because it meant I
  couldn't hang handy utility methods that the implementation wants off
  that class because it's public. And it meant that I couldn't change
  what state it stored (like the language version from the "@Dart"
  comment) without it being a breaking API change.

  Now there is a separate internal class that wraps the DartFormatter
  that can contain that logic. By consistently passing that around and
  not the DartFormatter, it ensures that code can't accidentally forget
  to use the language version in the "@Dart" comment.

* Pick a consistent naming convention for version numbers in
  identifiers. When we shipped Dart 3.8, a bunch of style rules changed.
  I needed a way to refer to the older style and couldn't come up with
  anything better than "3.7". But you can't have a dot in identifier,
  so I did "37". With Dart 3.10 on the horizon, that doesn't make sense:
  "310" is confusing and ambiguous. I considered "3_7", but that
  violates the lint for type names which shouldn't contain "_". Went
  with "3Dot7" instead which I don't love but seems to work.

Fix #1762.

Author: munificent

CHANGELOG.md

@@ -12,6 +12,14 @@
 * Don't force a space between `?` and `.` if a null-aware element contains a
   dot shorthand.
 
+### Bug fixes
+
+* Respect `@dart=` version comments when determining which >3.7 style to apply.
+  The formatter correctly used those comments to switch between the old short
+  and new tall style, but ignored them for language versioned style rule changes
+  after 3.7. Now the language version of the file is consistently respected for
+  all style rules (#1762).
+
 ## 3.1.2
 
 * Support dot shorthand syntax.

benchmark/run.dart

@@ -12,6 +12,7 @@ import 'package:args/args.dart';
 import 'package:dart_style/dart_style.dart';
 import 'package:dart_style/src/debug.dart' as debug;
 import 'package:dart_style/src/front_end/ast_node_visitor.dart';
+import 'package:dart_style/src/front_end/formatting_style.dart';
 import 'package:dart_style/src/profile.dart';
 import 'package:dart_style/src/short/source_visitor.dart';
 import 'package:dart_style/src/testing/benchmark.dart';
@@ -172,7 +173,11 @@ double _runTrial(
       var visitor = SourceVisitor(formatter, parseResult.lineInfo, source);
       result = visitor.run(parseResult.unit).text;
     } else {
-      var visitor = AstNodeVisitor(formatter, parseResult.lineInfo, source);
+      var visitor = AstNodeVisitor(
+        FormattingStyle(formatter),
+        parseResult.lineInfo,
+        source,
+      );
       result = visitor.run(source, parseResult.unit).text;
     }
   }

lib/src/ast_extensions.dart

@@ -458,7 +458,7 @@ extension AdjacentStringsExtensions on AdjacentStrings {
   ///
   /// To balance these, we omit the indentation when an adjacent string
   /// expression is in a context where it's unlikely to be confusing.
-  bool get indentStringsV37 {
+  bool get indentStrings3Dot7 {
     return switch (parent) {
       ArgumentList(:var arguments) => _hasOtherStringArgument(arguments),
 

lib/src/back_end/code_writer.dart

@@ -154,8 +154,8 @@ final class CodeWriter {
   /// Increases the indentation by [indent] relative to the current amount of
   /// indentation.
   void pushIndent(Indent indent) {
-    if (_cache.isVersion37) {
-      _pushIndentV37(indent);
+    if (_cache.is3Dot7) {
+      _pushIndent3Dot7(indent);
     } else {
       var parent = _indentStack.last;
 
@@ -191,8 +191,8 @@ final class CodeWriter {
   /// This is only used in a couple of corners of if-case and for-in headers
   /// where the indentation is unusual.
   void pushCollapsibleIndent() {
-    if (_cache.isVersion37) {
-      _pushIndentV37(Indent.expression, canCollapse: true);
+    if (_cache.is3Dot7) {
+      _pushIndent3Dot7(Indent.expression, canCollapse: true);
     } else {
       pushIndent(Indent.controlFlowClause);
     }
@@ -228,26 +228,26 @@ final class CodeWriter {
   /// To deal with this, 3.7 had a notion of "collapsible" indentation. In 3.8
   /// and later, there is a different mechanism for merging indentation kinds.
   /// This function implements the former.
-  void _pushIndentV37(Indent indent, {bool canCollapse = false}) {
+  void _pushIndent3Dot7(Indent indent, {bool canCollapse = false}) {
     var parentIndent = _indentStack.last.spaces;
     var parentCollapse = _indentStack.last.collapsible;
 
     if (parentCollapse == indent.spaces) {
       // We're indenting by the same existing collapsible amount, so collapse
       // this new indentation with that existing one.
-      _indentStack.add(_IndentLevel.v37(parentIndent, 0));
+      _indentStack.add(_IndentLevel.v3Dot7(parentIndent, 0));
     } else if (canCollapse) {
       // We should never get multiple levels of nested collapsible indentation.
       assert(parentCollapse == 0);
 
       // Increase the indentation and note that it can be collapsed with
       // further indentation.
       _indentStack.add(
-        _IndentLevel.v37(parentIndent + indent.spaces, indent.spaces),
+        _IndentLevel.v3Dot7(parentIndent + indent.spaces, indent.spaces),
       );
     } else {
       // Regular indentation, so just increase the indent.
-      _indentStack.add(_IndentLevel.v37(parentIndent + indent.spaces, 0));
+      _indentStack.add(_IndentLevel.v3Dot7(parentIndent + indent.spaces, 0));
     }
   }
 
@@ -399,7 +399,7 @@ final class CodeWriter {
       );
 
       bool invalid;
-      if (_cache.isVersion37) {
+      if (_cache.is3Dot7) {
         // If the child must be inline, then invalidate because we know it
         // contains some kind of newline.
         // TODO(rnystrom): It would be better if this logic wasn't different for
@@ -640,7 +640,7 @@ final class _IndentLevel {
   /// Only used for 3.7 style.
   final int collapsible;
 
-  _IndentLevel.v37(this.spaces, this.collapsible) : type = Indent.none;
+  _IndentLevel.v3Dot7(this.spaces, this.collapsible) : type = Indent.none;
 
   _IndentLevel(this.type, this.spaces) : collapsible = 0;
 

lib/src/back_end/solution_cache.dart

@@ -24,11 +24,11 @@ import 'solver.dart';
 /// cached subtree Solution.
 final class SolutionCache {
   /// Whether this cache and all solutions in it use the 3.7 style solver.
-  final bool isVersion37;
+  final bool is3Dot7;
 
   final _cache = <_Key, Solution>{};
 
-  SolutionCache({required bool version37}) : isVersion37 = version37;
+  SolutionCache({required this.is3Dot7});
 
   /// Returns a previously cached solution for formatting [root] with leading
   /// [indent] (and [subsequentIndent] for lines after the first) or produces a

lib/src/dart_formatter.dart

@@ -17,6 +17,7 @@ import 'package:pub_semver/pub_semver.dart';
 
 import 'exceptions.dart';
 import 'front_end/ast_node_visitor.dart';
+import 'front_end/formatting_style.dart';
 import 'short/source_visitor.dart';
 import 'source_code.dart';
 import 'string_compare.dart' as string_compare;
@@ -233,8 +234,16 @@ final class DartFormatter {
         }
       }
 
-      var visitor = AstNodeVisitor(this, lineInfo, unitSourceCode);
-      output = visitor.run(unitSourceCode, node, pageWidthFromComment);
+      var visitor = AstNodeVisitor(
+        FormattingStyle(
+          this,
+          languageVersion: sourceLanguageVersion,
+          pageWidth: pageWidthFromComment,
+        ),
+        lineInfo,
+        unitSourceCode,
+      );
+      output = visitor.run(unitSourceCode, node);
     } else {
       // Use the old style.
       var visitor = SourceVisitor(this, lineInfo, unitSourceCode);