diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d7261d4..fcf44b45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,14 +1,46 @@ ## 3.0.0-wip -* **Remove the old formatter executables and CLI options.** - - Before the `dart format` command was added to the core Dart SDK, users - accessed the formatter by running a separate `dartfmt` executable that was - included with the Dart SDK. That executable had a different CLI interface. - For example, you had to pass `-w` to get it to overwrite files and if you - passed no arguments at all, it silently sat there waiting for input on stdin. - When we added `dart format`, we took that opportunity to revamp the CLI - options. +* **Remove support for fixes and `--fix`.** The tools that come with the Dart + SDK provide two ways to apply automated changes to code: `dart format --fix` + and `dart fix`. The former is older and used to be faster. But it can only + apply a few fixes and hasn't been maintained in many years. The `dart fix` + command is actively maintained, can apply all of the fixes that + `dart format --fix` could apply and many many more. + + In order to avoid duplicate engineering effort, we decided to consolidate on + `dart fix` as the one way to make automated changes that go beyond the simple + formatting and style changes that `dart format` applies. + + The ability to apply fixes is also removed from the `DartFormatter()` library + API. + +* **Make the language version parameter to `DartFormatter()` mandatory.** This + way, the formatter always knows what language version the input is intended + to be treated as. Note that a `// @dart=` language version comment if present + overrides the specified language version. You can think of the version passed + to the `DartFormatter()` constructor as a "default" language version which + the file's contents may then override. + + If you don't particularly care about the version of what you're formatting, + you can pass in `DartFormatter.latestLanguageVersion` to unconditionally get + the latest language version that the formatter supports. Note that doing so + means you will also implicitly opt into the new tall style when that style + becomes available. + + This change only affects the library API. When using the formatter from the + command line, you can use `--language-version=` to specify a language version + or pass `--language-version=latest` to use the latest supported version. If + omitted, the formatter will look up the surrounding directories for a package + config file and infer the language version for the package from that, similar + to how other Dart tools behave like `dart analyze` and `dart run`. + +* **Remove the old formatter executables and CLI options.** Before the + `dart format` command was added to the core Dart SDK, users accessed the + formatter by running a separate `dartfmt` executable that was included with + the Dart SDK. That executable had a different CLI interface. For example, you + had to pass `-w` to get it to overwrite files and if you passed no arguments + at all, it silently sat there waiting for input on stdin. When we added + `dart format`, we took that opportunity to revamp the CLI options. However, the dart_style package still exposed an executable with the old CLI. If you ran `dart pub global activate dart_style`, this would give you a diff --git a/README.md b/README.md index dbab7cca..7480d915 100644 --- a/README.md +++ b/README.md @@ -32,29 +32,6 @@ if (tag == 'style' || The formatter will never break your code—you can safely invoke it automatically from build and presubmit scripts. -## Style fixes - -The formatter can also apply non-whitespace changes to make your code -consistently idiomatic. You must opt into these by passing either `--fix` which -applies all style fixes, or any of the `--fix-`-prefixed flags to apply specific -fixes. - -For example, running with `--fix-named-default-separator` changes this: - -```dart -greet(String name, {String title: "Captain"}) { - print("Greetings, $title $name!"); -} -``` - -into: - -```dart -greet(String name, {String title = "Captain"}) { - print("Greetings, $title $name!"); -} -``` - ## Using the formatter The formatter is part of the unified [`dart`][] developer tool included in the diff --git a/benchmark/directory.dart b/benchmark/directory.dart index bdfb1c37..d1d051d5 100644 --- a/benchmark/directory.dart +++ b/benchmark/directory.dart @@ -64,6 +64,7 @@ void main(List arguments) async { void _runFormatter(String source) { try { var formatter = DartFormatter( + languageVersion: DartFormatter.latestLanguageVersion, experimentFlags: [if (!_isShort) tallStyleExperimentFlag]); var result = formatter.format(source); diff --git a/benchmark/run.dart b/benchmark/run.dart index bf8da687..e19ff788 100644 --- a/benchmark/run.dart +++ b/benchmark/run.dart @@ -131,6 +131,7 @@ List _runTrials(String verb, Benchmark benchmark, int trials) { throwIfDiagnostics: false); var formatter = DartFormatter( + languageVersion: DartFormatter.latestLanguageVersion, pageWidth: benchmark.pageWidth, lineEnding: '\n', experimentFlags: [if (!_isShort) tallStyleExperimentFlag]); diff --git a/example/format.dart b/example/format.dart index d6d06035..ad2d004d 100644 --- a/example/format.dart +++ b/example/format.dart @@ -40,6 +40,7 @@ void _runFormatter(String source, int pageWidth, {required bool tall, required bool isCompilationUnit}) { try { var formatter = DartFormatter( + languageVersion: DartFormatter.latestLanguageVersion, pageWidth: pageWidth, experimentFlags: [if (tall) tallStyleExperimentFlag]); @@ -72,9 +73,9 @@ Future _runTest(String path, int line, var formatTest = testFile.tests.firstWhere((test) => test.line == line); var formatter = DartFormatter( + languageVersion: formatTest.languageVersion, pageWidth: testFile.pageWidth, indent: formatTest.leadingIndent, - fixes: formatTest.fixes, experimentFlags: tall ? const ['inline-class', tallStyleExperimentFlag] : const ['inline-class']); diff --git a/lib/dart_style.dart b/lib/dart_style.dart index d265a621..a730fc86 100644 --- a/lib/dart_style.dart +++ b/lib/dart_style.dart @@ -3,5 +3,4 @@ // BSD-style license that can be found in the LICENSE file. export 'src/dart_formatter.dart'; export 'src/exceptions.dart'; -export 'src/short/style_fix.dart'; export 'src/source_code.dart'; diff --git a/lib/src/cli/format_command.dart b/lib/src/cli/format_command.dart index 12732138..cba4d16c 100644 --- a/lib/src/cli/format_command.dart +++ b/lib/src/cli/format_command.dart @@ -9,7 +9,6 @@ import 'package:pub_semver/pub_semver.dart'; import '../dart_formatter.dart'; import '../io.dart'; -import '../short/style_fix.dart'; import 'formatter_options.dart'; import 'output.dart'; import 'show.dart'; @@ -77,18 +76,6 @@ class FormatCommand extends Command { negatable: false, help: 'Return exit code 1 if there are any formatting changes.'); - if (verbose) { - argParser.addSeparator('Non-whitespace fixes (off by default):'); - } - - argParser.addFlag('fix', - negatable: false, help: 'Apply all style fixes.', hide: !verbose); - - for (var fix in StyleFix.all) { - argParser.addFlag('fix-${fix.name}', - negatable: false, help: fix.description, hide: !verbose); - } - if (verbose) argParser.addSeparator('Other options:'); argParser.addOption('line-length', @@ -211,18 +198,6 @@ class FormatCommand extends Command { '"${argResults['indent']}".'); } - var fixes = []; - if (argResults['fix'] as bool) fixes.addAll(StyleFix.all); - for (var fix in StyleFix.all) { - if (argResults['fix-${fix.name}'] as bool) { - if (argResults['fix'] as bool) { - usageException('--fix-${fix.name} is redundant with --fix.'); - } - - fixes.add(fix); - } - } - List? selection; try { selection = _parseSelection(argResults, 'selection'); @@ -255,7 +230,6 @@ class FormatCommand extends Command { indent: indent, pageWidth: pageWidth, followLinks: followLinks, - fixes: fixes, show: show, output: output, summary: summary, diff --git a/lib/src/cli/formatter_options.dart b/lib/src/cli/formatter_options.dart index f0b04cdb..d20e4238 100644 --- a/lib/src/cli/formatter_options.dart +++ b/lib/src/cli/formatter_options.dart @@ -6,7 +6,6 @@ import 'dart:io'; import 'package:pub_semver/pub_semver.dart'; -import '../short/style_fix.dart'; import '../source_code.dart'; import 'output.dart'; import 'show.dart'; @@ -31,9 +30,6 @@ class FormatterOptions { /// Whether symlinks should be traversed when formatting a directory. final bool followLinks; - /// The style fixes to apply while formatting. - final List fixes; - /// Which affected files should be shown. final Show show; @@ -55,14 +51,12 @@ class FormatterOptions { this.indent = 0, this.pageWidth = 80, this.followLinks = false, - Iterable? fixes, this.show = Show.changed, this.output = Output.write, this.summary = Summary.none, this.setExitIfChanged = false, List? experimentFlags}) - : fixes = [...?fixes], - experimentFlags = [...?experimentFlags]; + : experimentFlags = [...?experimentFlags]; /// Called when [file] is about to be formatted. /// diff --git a/lib/src/dart_formatter.dart b/lib/src/dart_formatter.dart index 67a97b10..9394448d 100644 --- a/lib/src/dart_formatter.dart +++ b/lib/src/dart_formatter.dart @@ -4,7 +4,6 @@ import 'dart:math' as math; import 'package:analyzer/dart/analysis/features.dart'; -import 'package:analyzer/dart/analysis/results.dart'; import 'package:analyzer/dart/analysis/utilities.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/token.dart'; @@ -19,7 +18,6 @@ import 'constants.dart'; import 'exceptions.dart'; import 'front_end/ast_node_visitor.dart'; import 'short/source_visitor.dart'; -import 'short/style_fix.dart'; import 'source_code.dart'; import 'string_compare.dart' as string_compare; @@ -29,18 +27,11 @@ class DartFormatter { /// version of the formatter. static final latestLanguageVersion = Version(3, 3, 0); - /// The highest Dart language version without support for patterns. - static final _lastNonPatternsVersion = Version(2, 19, 0); - /// The Dart language version that formatted code should be parsed as. /// /// Note that a `// @dart=` comment inside the code overrides this. final Version languageVersion; - /// Whether the user passed in a non-`null` language version. - // TODO(rnystrom): Remove this when the language version is required. - final bool _omittedLanguageVersion; - /// The string that newlines should use. /// /// If not explicitly provided, this is inferred from the source text. If the @@ -54,8 +45,6 @@ class DartFormatter { /// The number of characters of indentation to prefix the output lines with. final int indent; - final Set fixes; - /// Flags to enable experimental language features. /// /// See dart.dev/go/experiments for details. @@ -63,34 +52,20 @@ class DartFormatter { /// Creates a new formatter for Dart code at [languageVersion]. /// - /// If [languageVersion] is omitted, then it defaults to - /// [latestLanguageVersion]. In a future major release of dart_style, the - /// language version will affect the applied formatting style. At that point, - /// this parameter will become required so that the applied style doesn't - /// change unexpectedly. It is optional now so that users can migrate to - /// versions of dart_style that accept this parameter and be ready for the - /// major version when it's released. - /// /// If [lineEnding] is given, that will be used for any newlines in the /// output. Otherwise, the line separator will be inferred from the line /// endings in the source file. /// /// If [indent] is given, that many levels of indentation will be prefixed /// before each resulting line in the output. - /// - /// While formatting, also applies any of the given [fixes]. DartFormatter( - {Version? languageVersion, + {required this.languageVersion, this.lineEnding, int? pageWidth, int? indent, - Iterable? fixes, List? experimentFlags}) - : languageVersion = languageVersion ?? latestLanguageVersion, - _omittedLanguageVersion = languageVersion == null, - pageWidth = pageWidth ?? 80, + : pageWidth = pageWidth ?? 80, indent = indent ?? 0, - fixes = {...?fixes}, experimentFlags = [...?experimentFlags]; /// Formats the given [source] string containing an entire Dart compilation @@ -141,33 +116,18 @@ class DartFormatter { ); } + // Don't pass the formatter's own experiment flag to the parser. + var experiments = experimentFlags.toList(); + experiments.remove(tallStyleExperimentFlag); + var featureSet = FeatureSet.fromEnableFlags2( + sdkLanguageVersion: languageVersion, flags: experiments); + // Parse it. - var parseResult = _parse(text, source.uri, languageVersion); - - // If we couldn't parse it, and the language version supports patterns, it - // may be because of the breaking syntax changes to switch cases. Try - // parsing it again without pattern support. - // TODO(rnystrom): This is a pretty big hack. Before Dart 3.0, every - // language version was a strict syntactic superset of all previous - // versions. When patterns were added, a small number of switch cases - // became syntax errors. - // - // For most of its history, the formatter simply parsed every file at the - // latest language version without having to detect each file's actual - // version. We are moving towards requiring the language version when - // formatting, but for now, try to degrade gracefully if the user omits the - // version. - // - // Remove this when the languageVersion constructor parameter is required. - if (_omittedLanguageVersion && parseResult.errors.isNotEmpty) { - var withoutPatternsResult = - _parse(text, source.uri, _lastNonPatternsVersion); - - // If we succeeded this time, use this parse instead. - if (withoutPatternsResult.errors.isEmpty) { - parseResult = withoutPatternsResult; - } - } + var parseResult = parseString( + content: text, + featureSet: featureSet, + path: source.uri, + throwIfDiagnostics: false); // Infer the line ending if not given one. Do it here since now we know // where the lines start. @@ -226,27 +186,10 @@ class DartFormatter { } // Sanity check that only whitespace was changed if that's all we expect. - if (fixes.isEmpty && - !string_compare.equalIgnoringWhitespace(source.text, output.text)) { + if (!string_compare.equalIgnoringWhitespace(source.text, output.text)) { throw UnexpectedOutputException(source.text, output.text); } return output; } - - /// Parse [source] from [uri] at language [version]. - ParseStringResult _parse(String source, String? uri, Version version) { - // Don't pass the formatter's own experiment flag to the parser. - var experiments = experimentFlags.toList(); - experiments.remove(tallStyleExperimentFlag); - - var featureSet = FeatureSet.fromEnableFlags2( - sdkLanguageVersion: version, flags: experiments); - - return parseString( - content: source, - featureSet: featureSet, - path: uri, - throwIfDiagnostics: false); - } } diff --git a/lib/src/io.dart b/lib/src/io.dart index 0032e612..96429ece 100644 --- a/lib/src/io.dart +++ b/lib/src/io.dart @@ -30,10 +30,10 @@ Future formatStdin( var input = StringBuffer(); stdin.transform(const Utf8Decoder()).listen(input.write, onDone: () { var formatter = DartFormatter( - languageVersion: options.languageVersion, + languageVersion: + options.languageVersion ?? DartFormatter.latestLanguageVersion, indent: options.indent, pageWidth: options.pageWidth, - fixes: options.fixes, experimentFlags: options.experimentFlags); try { options.beforeFile(null, name); @@ -157,7 +157,6 @@ Future _processFile( languageVersion: languageVersion ?? DartFormatter.latestLanguageVersion, indent: options.indent, pageWidth: options.pageWidth, - fixes: options.fixes, experimentFlags: options.experimentFlags); try { diff --git a/lib/src/short/chunk_builder.dart b/lib/src/short/chunk_builder.dart index 7083c2c8..a9620c6d 100644 --- a/lib/src/short/chunk_builder.dart +++ b/lib/src/short/chunk_builder.dart @@ -1,8 +1,6 @@ // Copyright (c) 2014, 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 'dart:math' as math; - import '../comment_type.dart'; import '../constants.dart'; import '../dart_formatter.dart'; @@ -15,21 +13,10 @@ import 'nesting_builder.dart'; import 'nesting_level.dart'; import 'rule/rule.dart'; import 'source_comment.dart'; -import 'style_fix.dart'; /// Matches if the last character of a string is an identifier character. final _trailingIdentifierChar = RegExp(r'[a-zA-Z0-9_]$'); -/// Matches a JavaDoc-style doc comment that starts with "/**" and ends with -/// "*/" or "**/". -final _javaDocComment = RegExp(r'^/\*\*([^*/][\s\S]*?)\*?\*/$'); - -/// Matches the leading "*" in a line in the middle of a JavaDoc-style comment. -final _javaDocLine = RegExp(r'^\s*\*(.*)'); - -/// Matches spaces at the beginning of as string. -final _leadingIndentation = RegExp(r'^(\s*)'); - /// Takes the incremental serialized output of [SourceVisitor]--the source text /// along with any comments and preserved whitespace--and produces a coherent /// tree of [Chunk]s which can then be split into physical lines. @@ -241,24 +228,6 @@ class ChunkBuilder { } } - // Edge case: if the previous output was also from a call to - // [writeComments()] which ended with a line comment, force a newline. - // Normally, comments are strictly interleaved with tokens and you never - // get two sequences of comments in a row. However, when applying a fix - // that removes a token (like `new`), it's possible to get two sets of - // comments in a row, as in: - // - // // a - // new // b - // Foo(); - // - // When that happens, we need to make sure to preserve the split at the end - // of the first sequence of comments if there is one. - if (_afterComment && _pendingNewlines > 0) { - comments.first.linesBefore = 1; - _pendingNewlines = 0; - } - // Edge case: if the comments are completely inline (i.e. just a series of // block comments with no newlines before, after, or between them), then // they will eat any pending newlines. Make sure that doesn't happen by @@ -311,7 +280,7 @@ class ChunkBuilder { _emitPendingWhitespace(isDouble: _needsBlankLineBeforeComment(comment)); } - _writeCommentText(comment, chunk); + _writeText(comment.text, chunk); if (comment.selectionStart != null) { startSelectionFromEnd(comment.text.length - comment.selectionStart!); @@ -352,66 +321,6 @@ class ChunkBuilder { _afterComment = true; } - /// Writes the text of [comment]. - /// - /// If it's a JavaDoc comment that should be fixed to use `///`, fixes it. - void _writeCommentText(SourceComment comment, [Chunk? chunk]) { - if (!_formatter.fixes.contains(StyleFix.docComments)) { - _writeText(comment.text, chunk); - return; - } - - // See if it's a JavaDoc comment. - var match = _javaDocComment.firstMatch(comment.text); - if (match == null) { - _writeText(comment.text, chunk); - return; - } - - var lines = match[1]!.split('\n').toList(); - var leastIndentation = comment.text.length; - - for (var i = 0; i < lines.length; i++) { - // Trim trailing whitespace and turn any all-whitespace lines to "". - var line = lines[i].trimRight(); - - // Remove any leading "*" from the middle lines. - if (i > 0 && i < lines.length - 1) { - var match = _javaDocLine.firstMatch(line); - if (match != null) { - line = match[1]!; - } - } - - // Find the line with the least indentation. - if (line.isNotEmpty) { - var indentation = _leadingIndentation.firstMatch(line)![1]!.length; - leastIndentation = math.min(leastIndentation, indentation); - } - - lines[i] = line; - } - - // Trim the first and last lines if empty. - if (lines.first.isEmpty) lines.removeAt(0); - if (lines.isNotEmpty && lines.last.isEmpty) lines.removeLast(); - - // Don't completely eliminate an empty block comment. - if (lines.isEmpty) lines.add(''); - - for (var line in lines) { - _writeText('///', chunk); - if (line.isNotEmpty) { - // Discard any indentation shared by all lines. - line = line.substring(leastIndentation); - _writeText(' $line', chunk); - } - - writeNewline(); - _emitPendingWhitespace(); - } - } - /// Creates a new indentation level [spaces] deeper than the current one. /// /// If omitted, [spaces] defaults to [Indent.block]. diff --git a/lib/src/short/source_visitor.dart b/lib/src/short/source_visitor.dart index a2a30174..4b964af7 100644 --- a/lib/src/short/source_visitor.dart +++ b/lib/src/short/source_visitor.dart @@ -7,8 +7,6 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/token.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/source/line_info.dart'; -// ignore: implementation_imports -import 'package:analyzer/src/clients/dart_style/rewrite_cascade.dart'; import '../ast_extensions.dart'; import '../comment_type.dart'; @@ -25,7 +23,6 @@ import 'rule/combinator.dart'; import 'rule/rule.dart'; import 'rule/type_argument.dart'; import 'source_comment.dart'; -import 'style_fix.dart'; /// Visits every token of the AST and passes all of the relevant bits to a /// [ChunkBuilder]. @@ -33,8 +30,6 @@ class SourceVisitor extends ThrowingAstVisitor { /// The builder for the block that is currently being visited. ChunkBuilder builder; - final DartFormatter _formatter; - /// Cached line info for calculating blank lines. final LineInfo _lineInfo; @@ -61,17 +56,6 @@ class SourceVisitor extends ThrowingAstVisitor { /// This is calculated and cached by [_findSelectionEnd]. int? _selectionEnd; - /// How many levels deep inside a constant context the visitor currently is. - int _constNesting = 0; - - /// Whether we are currently fixing a typedef declaration. - /// - /// Set to `true` while traversing the parameters of a typedef being converted - /// to the new syntax. The new syntax does not allow `int foo()` as a - /// parameter declaration, so it needs to be converted to `int Function() foo` - /// as part of the fix. - bool _insideNewTypedefFix = false; - /// A stack that tracks forcing nested collections to split. /// /// Each entry corresponds to a collection currently being visited and the @@ -103,14 +87,14 @@ class SourceVisitor extends ThrowingAstVisitor { final Map _blockRules = {}; final Map _blockPreviousChunks = {}; - /// Comments and new lines attached to tokens added here are suppressed - /// from the output. + /// Tracks tokens whose preceding comments have already been handled and + /// written and thus don't need to be written when the token is. final Set _suppressPrecedingCommentsAndNewLines = {}; /// Initialize a newly created visitor to write source code representing /// the visited nodes to the given [writer]. - SourceVisitor(this._formatter, this._lineInfo, this._source) - : builder = ChunkBuilder(_formatter, _source); + SourceVisitor(DartFormatter formatter, this._lineInfo, this._source) + : builder = ChunkBuilder(formatter, _source); /// Runs the visitor on [node], formatting its contents. /// @@ -127,8 +111,6 @@ class SourceVisitor extends ThrowingAstVisitor { // Output trailing comments. writePrecedingCommentsAndNewlines(node.endToken.next!); - assert(_constNesting == 0, 'Should have exited all const contexts.'); - Profile.end('SourceVisitor create Chunks'); // Finish writing and return the complete result. @@ -215,11 +197,8 @@ class SourceVisitor extends ThrowingAstVisitor { token(node.period); visit(node.constructorName); - if (node.arguments != null) { - // Metadata annotations are always const contexts. - _constNesting++; - visitArgumentList(node.arguments!, nestExpression: false); - _constNesting--; + if (node.arguments case var arguments?) { + visitArgumentList(arguments, nestExpression: false); } builder.unnest(); @@ -930,16 +909,9 @@ class SourceVisitor extends ThrowingAstVisitor { builder.startSpan(); builder.nestExpression(); - if (_formatter.fixes.contains(StyleFix.namedDefaultSeparator)) { - // Change the separator to "=". - space(); - writePrecedingCommentsAndNewlines(node.separator!); - _writeText('=', node.separator!); - } else { - // The '=' separator is preceded by a space, ":" is not. - if (node.separator!.type == TokenType.EQ) space(); - token(node.separator); - } + // The '=' separator is preceded by a space, ":" is not. + if (node.separator!.type == TokenType.EQ) space(); + token(node.separator); soloSplit(_assignmentCost(node.defaultValue!)); visit(node.defaultValue); @@ -1137,123 +1109,8 @@ class SourceVisitor extends ThrowingAstVisitor { token(node.semicolon); } - /// Parenthesize the target of the given statement's expression (assumed to - /// be a CascadeExpression) before removing the cascade. - void _fixCascadeByParenthesizingTarget(ExpressionStatement statement) { - var cascade = statement.expression as CascadeExpression; - assert(cascade.cascadeSections.length == 1); - - // Write any leading comments and whitespace immediately, as they should - // precede the new opening parenthesis, but then prevent them from being - // written again after the parenthesis. - writePrecedingCommentsAndNewlines(cascade.target.beginToken); - _suppressPrecedingCommentsAndNewLines.add(cascade.target.beginToken); - - // Finally, we can revisit a clone of this ExpressionStatement to actually - // remove the cascade. - visit( - fixCascadeByParenthesizingTarget( - expressionStatement: statement, - cascadeExpression: cascade, - ), - ); - } - - void _removeCascade(ExpressionStatement statement) { - var cascade = statement.expression as CascadeExpression; - var subexpression = cascade.cascadeSections.single; - builder.nestExpression(); - - if (subexpression is AssignmentExpression || - subexpression is MethodInvocation || - subexpression is PropertyAccess) { - // CascadeExpression("leftHandSide", "..", - // AssignmentExpression("target", "=", "rightHandSide")) - // - // transforms to - // - // AssignmentExpression( - // PropertyAccess("leftHandSide", ".", "target"), - // "=", - // "rightHandSide") - // - // CascadeExpression("leftHandSide", "..", - // MethodInvocation("target", ".", "methodName", ...)) - // - // transforms to - // - // MethodInvocation( - // PropertyAccess("leftHandSide", ".", "target"), - // ".", - // "methodName", ...) - // - // And similarly for PropertyAccess expressions. - visit(insertCascadeTargetIntoExpression( - expression: subexpression, cascadeTarget: cascade.target)); - } else { - throw UnsupportedError( - '--fix-single-cascade-statements: subexpression of cascade ' - '"$cascade" has unsupported type ${subexpression.runtimeType}.'); - } - - token(statement.semicolon); - builder.unnest(); - } - - /// Remove any unnecessary single cascade from the given expression statement, - /// which is assumed to contain a [CascadeExpression]. - /// - /// Returns true after applying the fix, which involves visiting the nested - /// expression. Callers must visit the nested expression themselves - /// if-and-only-if this method returns false. - bool _fixSingleCascadeStatement(ExpressionStatement statement) { - var cascade = statement.expression as CascadeExpression; - if (cascade.cascadeSections.length != 1) return false; - - var target = cascade.target; - if (target is AsExpression || - target is AwaitExpression || - target is BinaryExpression || - target is ConditionalExpression || - target is IsExpression || - target is PostfixExpression || - target is PrefixExpression) { - // In these cases, the cascade target needs to be parenthesized before - // removing the cascade, otherwise the semantics will change. - _fixCascadeByParenthesizingTarget(statement); - return true; - } else if (target is BooleanLiteral || - target is FunctionExpression || - target is IndexExpression || - target is InstanceCreationExpression || - target is IntegerLiteral || - target is ListLiteral || - target is NullLiteral || - target is MethodInvocation || - target is ParenthesizedExpression || - target is PrefixedIdentifier || - target is PropertyAccess || - target is SimpleIdentifier || - target is StringLiteral || - target is ThisExpression) { - // OK to simply remove the cascade. - _removeCascade(statement); - return true; - } else { - // If we get here, some new syntax was added to the language that the fix - // does not yet support. Leave it as is. - return false; - } - } - @override void visitExpressionStatement(ExpressionStatement node) { - if (_formatter.fixes.contains(StyleFix.singleCascadeStatements) && - node.expression is CascadeExpression && - _fixSingleCascadeStatement(node)) { - return; - } - _simpleStatement(node, () { visit(node.expression); }); @@ -1658,13 +1515,7 @@ class SourceVisitor extends ThrowingAstVisitor { @override void visitFunctionExpression(FunctionExpression node) { - // Inside a function body is no longer in the surrounding const context. - var oldConstNesting = _constNesting; - _constNesting = 0; - _visitFunctionBody(node.typeParameters, node.parameters, node.body); - - _constNesting = oldConstNesting; } @override @@ -1690,28 +1541,6 @@ class SourceVisitor extends ThrowingAstVisitor { @override void visitFunctionTypeAlias(FunctionTypeAlias node) { visitMetadata(node.metadata); - - if (_formatter.fixes.contains(StyleFix.functionTypedefs)) { - _simpleStatement(node, () { - // Inlined visitGenericTypeAlias - _visitGenericTypeAliasHeader( - node.typedefKeyword, - node.name, - node.typeParameters, - null, - node.returnType?.beginToken ?? node.name); - - space(); - - // Recursively convert function-arguments to Function syntax. - _insideNewTypedefFix = true; - _visitGenericFunctionType( - node.returnType, null, node.name, null, node.parameters); - _insideNewTypedefFix = false; - }); - return; - } - _simpleStatement(node, () { token(node.typedefKeyword); space(); @@ -1725,32 +1554,30 @@ class SourceVisitor extends ThrowingAstVisitor { @override void visitFunctionTypedFormalParameter(FunctionTypedFormalParameter node) { visitParameterMetadata(node.metadata, () { - if (!_insideNewTypedefFix) { - modifier(node.requiredKeyword); - modifier(node.covariantKeyword); - visit(node.returnType, after: space); - // Try to keep the function's parameters with its name. - builder.startSpan(); - token(node.name); - _visitParameterSignature(node.typeParameters, node.parameters); - token(node.question); - builder.endSpan(); - } else { - _beginFormalParameter(node); - _visitGenericFunctionType(node.returnType, null, node.name, - node.typeParameters, node.parameters); - token(node.question); - split(); - token(node.name); - _endFormalParameter(node); - } + modifier(node.requiredKeyword); + modifier(node.covariantKeyword); + visit(node.returnType, after: space); + // Try to keep the function's parameters with its name. + builder.startSpan(); + token(node.name); + _visitParameterSignature(node.typeParameters, node.parameters); + token(node.question); + builder.endSpan(); }); } @override void visitGenericFunctionType(GenericFunctionType node) { - _visitGenericFunctionType(node.returnType, node.functionKeyword, null, - node.typeParameters, node.parameters); + builder.startLazyRule(); + builder.nestExpression(); + + visit(node.returnType, after: split); + token(node.functionKeyword); + + builder.unnest(); + builder.endRule(); + _visitParameterSignature(node.typeParameters, node.parameters); + token(node.question); } @@ -1758,11 +1585,22 @@ class SourceVisitor extends ThrowingAstVisitor { void visitGenericTypeAlias(GenericTypeAlias node) { visitNodes(node.metadata, between: newline, after: newline); _simpleStatement(node, () { - _visitGenericTypeAliasHeader(node.typedefKeyword, node.name, - node.typeParameters, node.equals, null); - + token(node.typedefKeyword); space(); + // If the typedef's type parameters split, split after the "=" too, + // mainly to ensure the function's type parameters and parameters get + // end up on successive lines with the same indentation. + builder.startRule(); + + token(node.name); + visit(node.typeParameters); + split(); + token(node.equals); + + builder.endRule(); + + space(); visit(node.type); }); } @@ -2025,26 +1863,7 @@ class SourceVisitor extends ThrowingAstVisitor { void visitInstanceCreationExpression(InstanceCreationExpression node) { builder.startSpan(); - var includeKeyword = true; - - if (node.keyword != null) { - if (node.keyword!.keyword == Keyword.NEW && - _formatter.fixes.contains(StyleFix.optionalNew)) { - includeKeyword = false; - } else if (node.keyword!.keyword == Keyword.CONST && - _formatter.fixes.contains(StyleFix.optionalConst) && - _constNesting > 0) { - includeKeyword = false; - } - } - - if (includeKeyword) { - token(node.keyword, after: space); - } else { - // Don't lose comments before the discarded keyword, if any. - writePrecedingCommentsAndNewlines(node.keyword!); - } - + token(node.keyword, after: space); builder.startSpan(Cost.constructorName); // Start the expression nesting for the argument list here, in case this @@ -2053,14 +1872,10 @@ class SourceVisitor extends ThrowingAstVisitor { builder.nestExpression(); visit(node.constructorName); - _startPossibleConstContext(node.keyword); - builder.endSpan(); visitArgumentList(node.argumentList, nestExpression: false); builder.endSpan(); - _endPossibleConstContext(node.keyword); - builder.unnest(); } @@ -2218,9 +2033,7 @@ class SourceVisitor extends ThrowingAstVisitor { // code here has the same rules as in [visitInstanceCreationExpression]. // // That ensures that the way some code is formatted is not affected by the - // presence or absence of `new`/`const`. In particular, it means that if - // they run `dart format --fix`, and then run `dart format` *again*, the - // second run will not produce any additional changes. + // presence or absence of `new`/`const`. if (node.target == null || node.looksLikeStaticCall) { // Try to keep the entire method invocation one line. builder.nestExpression(); @@ -2735,34 +2548,11 @@ class SourceVisitor extends ThrowingAstVisitor { visitParameterMetadata(node.metadata, () { _beginFormalParameter(node); - if (_insideNewTypedefFix && node.type == null) { - // Parameters can use "var" instead of "dynamic". Since we are inserting - // "dynamic" in that case, remove the "var". - if (node.keyword != null) { - if (node.keyword!.type != Keyword.VAR) { - modifier(node.keyword); - } else { - // Keep any comment attached to "var". - writePrecedingCommentsAndNewlines(node.keyword!); - } - } - - // In function declarations and the old typedef syntax, you can have a - // parameter name without a type. In the new syntax, you can have a type - // without a name. Add "dynamic" in that case. + modifier(node.keyword); - // Ensure comments on the identifier comes before the inserted type. - token(node.name, before: () { - _writeText('dynamic', node.name!); - split(); - }); - } else { - modifier(node.keyword); - - visit(node.type); - if (node.name != null) _separatorBetweenTypeAndVariable(node.type); - token(node.name); - } + visit(node.type); + if (node.name != null) _separatorBetweenTypeAndVariable(node.type); + token(node.name); _endFormalParameter(node); }); @@ -3127,8 +2917,6 @@ class SourceVisitor extends ThrowingAstVisitor { builder.endSpan(); - _startPossibleConstContext(node.keyword); - // Use a single rule for all of the variables. If there are multiple // declarations, we will try to keep them all on one line. If that isn't // possible, we split after *every* declaration so that each is on its own @@ -3144,7 +2932,6 @@ class SourceVisitor extends ThrowingAstVisitor { if (node.variables.length > 1) builder.endBlockArgumentNesting(); builder.endRule(); - _endPossibleConstContext(node.keyword); } @override @@ -3625,15 +3412,7 @@ class SourceVisitor extends ThrowingAstVisitor { int? cost, bool splitOuterCollection = false, bool isRecord = false}) { - // See if `const` should be removed. - if (constKeyword != null && - _constNesting > 0 && - _formatter.fixes.contains(StyleFix.optionalConst)) { - // Don't lose comments before the discarded keyword, if any. - writePrecedingCommentsAndNewlines(constKeyword); - } else { - modifier(constKeyword); - } + modifier(constKeyword); // Don't use the normal type argument list formatting code because we don't // want to allow splitting before the "<" since there is no preceding @@ -3658,7 +3437,7 @@ class SourceVisitor extends ThrowingAstVisitor { // TODO(rnystrom): There is a bug in analyzer where the end token of a // nullable record type is the ")" and not the "?". This works around - // that. Remove that's fixed. + // that. Remove once that's fixed. if (comma?.lexeme == '?') comma = comma?.next; token(comma); @@ -3688,7 +3467,6 @@ class SourceVisitor extends ThrowingAstVisitor { } _beginBody(leftBracket); - _startPossibleConstContext(constKeyword); // If a collection contains a line comment, we assume it's a big complex // blob of data with some documented structure. In that case, the user @@ -3740,7 +3518,6 @@ class SourceVisitor extends ThrowingAstVisitor { var isSingleElementRecord = isRecord && elements.length == 1; if (elements.hasCommaAfter && !isSingleElementRecord) force = true; - _endPossibleConstContext(constKeyword); _endBody(rightBracket, forceSplit: force); } @@ -3826,61 +3603,6 @@ class SourceVisitor extends ThrowingAstVisitor { builder.endRule(); } - /// Writes a `Function` function type. - /// - /// Used also by a fix, so there may not be a [functionKeyword]. - /// In that case [functionKeywordPosition] should be the source position - /// used for the inserted "Function" text. - void _visitGenericFunctionType( - AstNode? returnType, - Token? functionKeyword, - Token? positionToken, - TypeParameterList? typeParameters, - FormalParameterList parameters) { - builder.startLazyRule(); - builder.nestExpression(); - - visit(returnType, after: split); - if (functionKeyword != null) { - token(functionKeyword); - } else { - _writeText('Function', positionToken!); - } - - builder.unnest(); - builder.endRule(); - _visitParameterSignature(typeParameters, parameters); - } - - /// Writes the header of a new-style typedef. - /// - /// Also used by a fix so there may not be an [equals] token. - /// If [equals] is `null`, then [equalsPosition] must be a - /// position to use for the inserted text "=". - void _visitGenericTypeAliasHeader(Token typedefKeyword, Token name, - AstNode? typeParameters, Token? equals, Token? positionToken) { - token(typedefKeyword); - space(); - - // If the typedef's type parameters split, split after the "=" too, - // mainly to ensure the function's type parameters and parameters get - // end up on successive lines with the same indentation. - builder.startRule(); - - token(name); - - visit(typeParameters); - split(); - - if (equals != null) { - token(equals); - } else { - _writeText('=', positionToken!); - } - - builder.endRule(); - } - /// Visits the `if ( [case [when ]])` header of an if /// statement or element. void _visitIfCondition(Token ifKeyword, Token leftParenthesis, @@ -4106,20 +3828,6 @@ class SourceVisitor extends ThrowingAstVisitor { builder.unnest(); } - /// If [keyword] is `const`, begins a new constant context. - void _startPossibleConstContext(Token? keyword) { - if (keyword != null && keyword.keyword == Keyword.CONST) { - _constNesting++; - } - } - - /// If [keyword] is `const`, ends the current outermost constant context. - void _endPossibleConstContext(Token? keyword) { - if (keyword != null && keyword.keyword == Keyword.CONST) { - _constNesting--; - } - } - /// Writes the simple statement or semicolon-delimited top-level declaration. /// /// Handles nesting if a line break occurs in the statement and writes the @@ -4382,7 +4090,7 @@ class SourceVisitor extends ThrowingAstVisitor { // actually needed. if (comment == null) return false; - // If the token's comments are being moved by a fix, do not write them here. + // If the token's comments are already handled, do not write them here. if (_suppressPrecedingCommentsAndNewLines.contains(token)) return false; var previousLine = _endLine(token.previous!); @@ -4457,8 +4165,8 @@ class SourceVisitor extends ThrowingAstVisitor { /// /// Also outputs the selection endpoints if needed. /// - /// Usually, [text] is simply [token]'s lexeme, but for fixes, multi-line - /// strings, or a couple of other cases, it will be different. + /// Usually, [text] is simply [token]'s lexeme, but for multiline strings, or + /// a couple of other cases, it will be different. /// /// If [offset] is given, uses that for calculating selection location. /// Otherwise, uses the offset of [token]. diff --git a/lib/src/short/style_fix.dart b/lib/src/short/style_fix.dart deleted file mode 100644 index 032cd313..00000000 --- a/lib/src/short/style_fix.dart +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright (c) 2018, 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. - -/// Enum-like class for the different syntactic fixes that can be applied while -/// formatting. -class StyleFix { - static const docComments = StyleFix._( - 'doc-comments', 'Use triple slash for documentation comments.'); - - static const functionTypedefs = StyleFix._( - 'function-typedefs', 'Use new syntax for function type typedefs.'); - - static const namedDefaultSeparator = StyleFix._('named-default-separator', - 'Use "=" as the separator before named parameter default values.'); - - static const optionalConst = StyleFix._( - 'optional-const', 'Remove "const" keyword inside constant context.'); - - static const optionalNew = - StyleFix._('optional-new', 'Remove "new" keyword.'); - - static const singleCascadeStatements = StyleFix._('single-cascade-statements', - 'Remove unnecessary single cascades from expression statements.'); - - static const all = [ - docComments, - functionTypedefs, - namedDefaultSeparator, - optionalConst, - optionalNew, - singleCascadeStatements, - ]; - - final String name; - final String description; - - const StyleFix._(this.name, this.description); -} diff --git a/lib/src/testing/test_file.dart b/lib/src/testing/test_file.dart index 23dc192d..01a3eec4 100644 --- a/lib/src/testing/test_file.dart +++ b/lib/src/testing/test_file.dart @@ -5,11 +5,12 @@ import 'dart:io'; import 'dart:isolate'; import 'package:path/path.dart' as p; +import 'package:pub_semver/pub_semver.dart'; import '../../dart_style.dart'; final _indentPattern = RegExp(r'\(indent (\d+)\)'); -final _fixPattern = RegExp(r'\(fix ([a-x-]+)\)'); +final _versionPattern = RegExp(r'\(version (\d+)\.(\d+)\)'); final _unicodeUnescapePattern = RegExp(r'×([0-9a-fA-F]{2,4})'); final _unicodeEscapePattern = RegExp('[\x0a\x0c\x0d]'); @@ -89,7 +90,6 @@ class TestFile { while (i < lines.length) { var lineNumber = i + 1; var description = readLine().replaceAll('>>>', ''); - var fixes = []; // Let the test specify a leading indentation. This is handy for // regression tests which often come from a chunk of nested code. @@ -99,9 +99,12 @@ class TestFile { return ''; }); - // Let the test specify fixes to apply. - description = description.replaceAllMapped(_fixPattern, (match) { - fixes.add(StyleFix.all.firstWhere((fix) => fix.name == match[1])); + // Let the test specify a language version to parse it at. + var languageVersion = DartFormatter.latestLanguageVersion; + description = description.replaceAllMapped(_versionPattern, (match) { + var major = int.parse(match[1]!); + var minor = int.parse(match[2]!); + languageVersion = Version(major, minor, 0); return ''; }); @@ -142,7 +145,7 @@ class TestFile { description.trim(), outputDescription.trim(), lineNumber, - fixes, + languageVersion, leadingIndent, inputComments, outputComments)); @@ -194,8 +197,8 @@ class FormatTest { /// The 1-based index of the line where this test begins. final int line; - /// The style fixes this test is applying. - final List fixes; + /// The language version the test code should be parsed at. + final Version languageVersion; /// The number of spaces of leading indentation that should be added to each /// line. @@ -207,7 +210,7 @@ class FormatTest { this.description, this.outputDescription, this.line, - this.fixes, + this.languageVersion, this.leadingIndent, this.inputComments, this.outputComments); diff --git a/test/README.md b/test/README.md index 7e0e6afa..0ec286fb 100644 --- a/test/README.md +++ b/test/README.md @@ -50,7 +50,6 @@ The older short style tests are organized like: ``` short/comments/ - Test comment handling. -short/fixes/ - Test `--fix`. short/regression/ - Regression tests. File names correspond to issues. short/selections/ - Test how the formatter preserves selection information. short/splitting/ - Test line splitting behavior. diff --git a/test/cli_test.dart b/test/cli_test.dart index 1b57e051..0ad70c34 100644 --- a/test/cli_test.dart +++ b/test/cli_test.dart @@ -297,7 +297,6 @@ void main() { await expectLater(process.stdout, emitsThrough(contains('-o, --output'))); await expectLater(process.stdout, emitsThrough(contains('--show'))); await expectLater(process.stdout, emitsThrough(contains('--summary'))); - await expectLater(process.stdout, emitsThrough(contains('--fix'))); await process.shouldExit(0); }); }); @@ -308,64 +307,6 @@ void main() { await process.shouldExit(64); }); - group('fix', () { - test('--fix applies all fixes', () async { - var process = await runFormatter(['--fix', '--output=show']); - process.stdin.writeln('foo({a:1}) {'); - process.stdin.writeln(' new Bar(const Baz(const []));}'); - await process.stdin.close(); - - expect(await process.stdout.next, 'foo({a = 1}) {'); - expect(await process.stdout.next, ' Bar(const Baz([]));'); - expect(await process.stdout.next, '}'); - await process.shouldExit(0); - }); - - test('--fix-named-default-separator', () async { - var process = await runFormatter( - ['--fix-named-default-separator', '--output=show']); - process.stdin.writeln('foo({a:1}) {'); - process.stdin.writeln(' new Bar();}'); - await process.stdin.close(); - - expect(await process.stdout.next, 'foo({a = 1}) {'); - expect(await process.stdout.next, ' new Bar();'); - expect(await process.stdout.next, '}'); - await process.shouldExit(0); - }); - - test('--fix-optional-const', () async { - var process = - await runFormatter(['--fix-optional-const', '--output=show']); - process.stdin.writeln('foo({a:1}) {'); - process.stdin.writeln(' const Bar(const Baz());}'); - await process.stdin.close(); - - expect(await process.stdout.next, 'foo({a: 1}) {'); - expect(await process.stdout.next, ' const Bar(Baz());'); - expect(await process.stdout.next, '}'); - await process.shouldExit(0); - }); - - test('--fix-optional-new', () async { - var process = await runFormatter(['--fix-optional-new', '--output=show']); - process.stdin.writeln('foo({a:1}) {'); - process.stdin.writeln(' new Bar();}'); - await process.stdin.close(); - - expect(await process.stdout.next, 'foo({a: 1}) {'); - expect(await process.stdout.next, ' Bar();'); - expect(await process.stdout.next, '}'); - await process.shouldExit(0); - }); - - test('errors with --fix and specific fix flag', () async { - var process = - await runFormatter(['--fix', '--fix-named-default-separator']); - await process.shouldExit(64); - }); - }); - group('--indent', () { test('sets the leading indentation of the output', () async { var process = await runFormatter(['--indent=3']); diff --git a/test/dart_formatter_test.dart b/test/dart_formatter_test.dart index f9140006..9b97a799 100644 --- a/test/dart_formatter_test.dart +++ b/test/dart_formatter_test.dart @@ -22,7 +22,7 @@ void _runTests({required bool isTall}) { DartFormatter makeFormatter( {Version? languageVersion, int? indent, String? lineEnding}) { return DartFormatter( - languageVersion: languageVersion, + languageVersion: languageVersion ?? DartFormatter.latestLanguageVersion, indent: indent, lineEnding: lineEnding, experimentFlags: [if (isTall) tallStyleExperimentFlag]); diff --git a/test/fix_test.dart b/test/fix_test.dart deleted file mode 100644 index fe33a046..00000000 --- a/test/fix_test.dart +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright (c) 2018, 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. - -@TestOn('vm') -library; - -import 'package:dart_style/dart_style.dart'; -import 'package:test/test.dart'; - -import 'utils.dart'; - -void main() async { - await testFile('short/fixes/named_default_separator.unit', - fixes: [StyleFix.namedDefaultSeparator]); - await testFile('short/fixes/doc_comments.stmt', - fixes: [StyleFix.docComments]); - await testFile('short/fixes/function_typedefs.unit', - fixes: [StyleFix.functionTypedefs]); - await testFile('short/fixes/optional_const.unit', - fixes: [StyleFix.optionalConst]); - await testFile('short/fixes/optional_new.stmt', - fixes: [StyleFix.optionalNew]); - await testFile('short/fixes/single_cascade_statements.stmt', - fixes: [StyleFix.singleCascadeStatements]); -} diff --git a/test/short/fixes/doc_comments.stmt b/test/short/fixes/doc_comments.stmt deleted file mode 100644 index c7b2cec7..00000000 --- a/test/short/fixes/doc_comments.stmt +++ /dev/null @@ -1,195 +0,0 @@ -40 columns | ->>> single line comment -/** doc */ m() {} -<<< -/// doc -m() {} ->>> multiline comment -/** - * multiline comment - * line 1 - */ -m() {} -<<< -/// multiline comment -/// line 1 -m() {} ->>> multiline comment without star -/** - multiline comment without star - line 1 - */ -m() {} -<<< -/// multiline comment without star -/// line 1 -m() {} ->>> strange indent - /** - * strange indent - * line 1 - */ -m() {} -<<< -/// strange indent -/// line 1 -m() {} ->>> comment on first line -/** comment on first line -* line 1 -*/ -m() {} -<<< -/// comment on first line -/// line 1 -m() {} ->>> empty comment -/** */ -m() {} -<<< -/// -m() {} ->>> empty multiline comment -/** - */ -m() {} -<<< -/// -m() {} ->>> multiline comment with sample -/** - * multiline comment with sample - * - * var a; - */ -m() {} -<<< -/// multiline comment with sample -/// -/// var a; -m() {} ->>> single line comment with "**/" as close -/** doc **/ m() {} -<<< -/// doc -m() {} ->>> multiline comment with "**/" as close -/** - * multiline comment - * line 1 - **/ -m() {} -<<< -/// multiline comment -/// line 1 -m() {} ->>> does not touch longer strings of "*" -/******* not a doc comment **/ -m() {} -<<< -/******* not a doc comment **/ -m() {} ->>> does not touch "/**/" -/**/ -m() {} -<<< -/**/ -m() {} ->>> does not touch "/***/" -/***/ -m() {} -<<< -/***/ -m() {} ->>> long box of "*" -/******* - * STUFF - *******/ -m() {} -<<< -/******* - * STUFF - *******/ -m() {} ->>> nested comment -/** - * Floo the grumshack. - * - * Example: - * ```dart - * /** Doc comment */ - * var grumshack = getGrumshack(); - * /* Do the floo */ - * grumshack.floo(); - * ``` -*/ -m() {} -<<< -/// Floo the grumshack. -/// -/// Example: -/// ```dart -/// /** Doc comment */ -/// var grumshack = getGrumshack(); -/// /* Do the floo */ -/// grumshack.floo(); -/// ``` -m() {} ->>> -/** Does a [foo](http://example.org/*example*/doc). **/ -m() {} -<<< -/// Does a [foo](http://example.org/*example*/doc). -m() {} ->>> -/** Does a [foo](http://example.org/*example*/doc). */ -m() {} -<<< -/// Does a [foo](http://example.org/*example*/doc). -m() {} ->>> non-leading "*" -/** - * Thing. - Another * thing. -*/ -m() {} -<<< -/// Thing. -/// Another * thing. -m() {} ->>> missing "*" but indented (#821) -/** -This is an ugly dartdoc comment that contains a code block. - - class Foo { - final int x; - Foo(this.x); - } - -The formatting gets messed up by `dart format --fix-doc-comments`. -*/ -m() {} -<<< -/// This is an ugly dartdoc comment that contains a code block. -/// -/// class Foo { -/// final int x; -/// Foo(this.x); -/// } -/// -/// The formatting gets messed up by `dart format --fix-doc-comments`. -m() {} ->>> strip leading indentation shared by all lines -/** 4 - 3 - * 5 - * 2 - 7 */ -m() {} -<<< -/// 4 -/// 3 -/// 5 -/// 2 -/// 7 -m() {} \ No newline at end of file diff --git a/test/short/fixes/function_typedefs.unit b/test/short/fixes/function_typedefs.unit deleted file mode 100644 index 3d784de1..00000000 --- a/test/short/fixes/function_typedefs.unit +++ /dev/null @@ -1,272 +0,0 @@ -40 columns | ->>> -typedef void A(); -<<< -typedef A = void Function(); ->>> -typedef void A(String name); -<<< -typedef A = void Function(String name); ->>> -typedef bool A(int count); -<<< -typedef A = bool Function(int count); ->>> -typedef void A(); -<<< -typedef A = void Function(); ->>> -typedef void A(T x); -<<< -typedef A = void Function(T x); ->>> -typedef void A(x); -<<< -typedef A = void Function(dynamic x); ->>> -typedef V A(); -<<< -typedef A = V Function(); ->>> -typedef V A(K key); -<<< -typedef A = V Function(K key); ->>> -typedef K A(V value); -<<< -typedef A = K Function(V value); ->>> -typedef F = void Function(T); -<<< -typedef F = void Function(T); ->>> -@meta typedef void X(y); -<<< -@meta -typedef X = void Function(dynamic y); ->>> split type parameters -typedef G = T Function(); -<<< -typedef G = T Function(); ->>> split all type parameters -typedef G = T Function(); -<<< -typedef G = T Function< - TypeOne, - TypeTwo, - TypeThree, - TypeFour, - TypeFive, - TypeSix>(); ->>> split type and value parameters -typedef G = T Function(TypeOne one, TypeTwo two, TypeThree three); -<<< -typedef G = T Function(TypeOne one, - TypeTwo two, TypeThree three); ->>> generic typedef parameters on one line -typedef Foo = Function(); -<<< -typedef Foo = Function(); ->>> generic typedef parameters that split -typedef LongfunctionType = Function(First first, Second second, Third third, Fourth fourth); -<<< -typedef LongfunctionType - = Function( - First first, - Second second, - Third third, - Fourth fourth); ->>> both type parameter lists split -typedef LongfunctionType = Function(First first, Second second, Third third, Fourth fourth); -<<< -typedef LongfunctionType - = Function( - First first, - Second second, - Third third, - Fourth fourth); ->>> all three parameter lists split -typedef LongfunctionType = Function(First first, Second second, Third third, Fourth fourth); -<<< -typedef LongfunctionType - = Function< - Seventh, - Eighth, - Ninth, - Tenth, - Eleventh, - Twelfth, - Thirteenth>( - First first, - Second second, - Third third, - Fourth fourth); ->>> old generic typedef syntax -typedef Foo < T ,S >(T t,S s); -<<< -typedef Foo = Function(T t, S s); ->>> non-generic in typedef -typedef SomeFunc=ReturnType Function(int param, double other); -<<< -typedef SomeFunc = ReturnType Function( - int param, double other); ->>> generic in typedef -typedef Generic = T Function(T param, double other); -<<< -typedef Generic = T Function( - T param, double other); ->>> no return type -typedef SomeFunc = Function(); -<<< -typedef SomeFunc = Function(); ->>> nested -typedef SomeFunc = Function(int first, Function(int first, bool second, String third) second, String third); -<<< -typedef SomeFunc = Function( - int first, - Function(int first, bool second, - String third) - second, - String third); ->>> without param names -typedef F = Function(int, bool, String); -<<< -typedef F = Function(int, bool, String); ->>> generic -typedef Foo < A ,B>=Function ( A a, B b ); -<<< -typedef Foo = Function(A a, B b); ->>> generic function -typedef Foo =Function < A ,B > ( A a,B b ); -<<< -typedef Foo = Function(A a, B b); ->>> simple typedef -typedef int Foo(int x); -<<< -typedef Foo = int Function(int x); ->>> generic typedef -typedef S Foo(S x); -<<< -typedef Foo = S Function( - S x); ->>> named argument -typedef int Foo({x}); -<<< -typedef Foo = int Function({dynamic x}); ->>> optional positional argument -typedef int Foo([x]); -<<< -typedef Foo = int Function([dynamic x]); ->>> metadata and keywords -typedef int Foo(@meta v1, final v2, var v3, /*c*/ v4); -<<< -typedef Foo = int Function( - @meta dynamic v1, - final dynamic v2, - dynamic v3, - /*c*/ dynamic v4); ->>> metadata and keywords optional positional -typedef int Foo([@meta v1, final v2, var v3, /*c*/ v4]); -<<< -typedef Foo = int Function( - [@meta dynamic v1, - final dynamic v2, - dynamic v3, - /*c*/ dynamic v4]); ->>> metadata and keywords named -typedef int Foo({@meta v1, final v2, var v3, /*c*/ v4}); -<<< -typedef Foo = int Function( - {@meta dynamic v1, - final dynamic v2, - dynamic v3, - /*c*/ dynamic v4}); ->>> function argument -typedef int Foo(int callback(int x)); -<<< -typedef Foo = int Function( - int Function(int x) callback); ->>> nested Function type -typedef int Foo(int cb(int Function(int) cb2)); -<<< -typedef Foo = int Function( - int Function(int Function(int) cb2) - cb); ->>> nested generic function -typedef T Foo(T cb(T cb2(S x))); -<<< -typedef Foo = T Function( - T Function(T Function(S x) cb2) - cb); ->>> new-style function type unchanged -typedef int Foo(int Function(int) cb); -<<< -typedef Foo = int Function( - int Function(int) cb); ->>> don't change correct typedef -typedef Foo = int Function(int); -<<< -typedef Foo = int Function(int); ->>> simple typedef, no types -typedef Foo(x); -<<< -typedef Foo = Function(dynamic x); ->>> nested function types, no types -typedef Foo(foo(x)); -<<< -typedef Foo = Function( - Function(dynamic x) foo); ->>> block comments in typedef -typedef/*0*/void/*1*/Foo/*2*//*3*/(/*4*/T/*5*/foo(/*6*/x)); -<<< -typedef /*1*/ Foo /*2*/ - = /*0*/ void Function /*3*/ ( - /*4*/ T Function( - /*6*/ dynamic x) /*5*/ - foo); ->>> eol comments in typedef -typedef//0 -void//1 -Foo//2 -//3 -(//4 -T//5 -foo(//6 -x)); -<<< -typedef //1 - Foo //2 - - = //0 - void Function //3 - ( - //4 - T Function( - //6 - dynamic x) //5 - foo); ->>> nullable old style function-typed formal -typedef Foo(foo()?); -<<< -typedef Foo = Function(Function()? foo); ->>> required parameters -typedef Foo({required a, required b()}); -<<< -typedef Foo = Function( - {required dynamic a, - required Function() b}); ->>> issue #826 -typedef void MyFunction(var parameter); -<<< -typedef MyFunction = void Function( - dynamic parameter); ->>> issue #826 with comment -typedef void MyFunction(/* comment */ var parameter); -<<< -typedef MyFunction = void Function( - /* comment */ dynamic parameter); \ No newline at end of file diff --git a/test/short/fixes/named_default_separator.unit b/test/short/fixes/named_default_separator.unit deleted file mode 100644 index de119b96..00000000 --- a/test/short/fixes/named_default_separator.unit +++ /dev/null @@ -1,46 +0,0 @@ -40 columns | ->>> -foo({int a, int c: 1, d: 2, e=3}) {} -<<< -foo({int a, int c = 1, d = 2, e = 3}) {} ->>> preserves block comments before -foo({a/* b */ /* c */:1}) {} -<<< -foo({a /* b */ /* c */ = 1}) {} ->>> preserves block comments after -foo({a : /* b */ /* c */1}) {} -<<< -foo({a = /* b */ /* c */ 1}) {} ->>> preserves line comments before -foo({a// b - - - // c - :1}) {} -<<< -foo( - {a // b - - // c - = 1}) {} ->>> preserves line comments after -foo({a : // b - - // c -1}) {} -<<< -foo( - {a = // b - - // c - 1}) {} ->>> on required parameter -class A { -foo({required int b: 1, required c(): f}) {} -} -<<< -class A { - foo( - {required int b = 1, - required c() = f}) {} -} \ No newline at end of file diff --git a/test/short/fixes/optional_const.unit b/test/short/fixes/optional_const.unit deleted file mode 100644 index 7823dfd7..00000000 --- a/test/short/fixes/optional_const.unit +++ /dev/null @@ -1,107 +0,0 @@ -40 columns | ->>> -main() { - const a = const A(const A.named()); - // Don't touch new. - new A(new A.named()); -} -<<< -main() { - const a = A(A.named()); - // Don't touch new. - new A(new A.named()); -} ->>> preserves block comments before -const c = /* a */ /* b */ const A(); -<<< -const c = /* a */ /* b */ A(); ->>> preserves block comments after -const c = const /* a */ /* b */A(); -<<< -const c = /* a */ /* b */ A(); ->>> preserves line comments before -const c = // a - // b -const A(); -<<< -const c = // a - // b - A(); ->>> preserves line comments after -const c = const // a - // b -A(); -<<< -const c = // a - // b - A(); ->>> merge surrounding comments -main() { - const c = /* a */ const /* b */ A(); - const d = /* a */ const // b - A(); - const e = // a - const /* b */ A(); - const f = // a - const // b - A(); -} -<<< -main() { - const c = /* a */ /* b */ A(); - const d = /* a */ // b - A(); - const e = // a - /* b */ A(); - const f = // a - // b - A(); -} ->>> handle already-removed keyword -const c = A(A.named()); -<<< -const c = A(A.named()); ->>> remove from collection literals -const c = const [const [], const {}, const {}]; -<<< -const c = [[], {}, {}]; ->>> const constructor call is a const context -var a = const A(const B()); -<<< -var a = const A(B()); ->>> const collection is a const context -var a = const [const B()]; -<<< -var a = const [B()]; ->>> const top-level variable is a const context -const c = const A(); -<<< -const c = A(); ->>> const local variable is a const context -main() { - const c = const A(); -} -<<< -main() { - const c = A(); -} ->>> metadata annotation is a const context -@Foo(const Bar()) -function() {} -<<< -@Foo(Bar()) -function() {} ->>> nested function is not a const context -const c = [const A(), () => const B(() { const C(); }), const D()]; -<<< -const c = [ - A(), - () => const B(() { - const C(); - }), - D() -]; ->>> do not remove erroneous new -const c = new A(); -<<< -const c = new A(); \ No newline at end of file diff --git a/test/short/fixes/optional_new.stmt b/test/short/fixes/optional_new.stmt deleted file mode 100644 index cdf81c27..00000000 --- a/test/short/fixes/optional_new.stmt +++ /dev/null @@ -1,79 +0,0 @@ -40 columns | ->>> -{ - new A(new A.named()); - new A(new A.named()); - // Don't touch const. - const A(); - const A.named(); -} -<<< -{ - A(A.named()); - A(A.named()); - // Don't touch const. - const A(); - const A.named(); -} ->>> preserves block comments before -/* a */ /* b */ new A(); -<<< -/* a */ /* b */ A(); ->>> preserves block comments after -new /* a */ /* b */A(); -<<< -/* a */ /* b */ A(); ->>> preserves line comments before -// a -// b -new A(); -<<< -// a -// b -A(); ->>> preserves line comments after -new // a -// b -A(); -<<< -// a -// b -A(); ->>> -var x = // 1 -new // 2 -Foo(); -<<< -var x = // 1 - // 2 - Foo(); ->>> merge surrounding comments -{ - /* a */ new /* b */ A(); - /* a */ new // b - A(); - // a - new /* b */ A(); - // a - new // b - A(); -} -<<< -{ - /* a */ /* b */ A(); - /* a */ // b - A(); - // a - /* b */ A(); - // a - // b - A(); -} ->>> handle already-removed keyword -A(A.named()); -<<< -A(A.named()); ->>> inside string interpolation -"before ${new Foo()} after"; -<<< -"before ${Foo()} after"; \ No newline at end of file diff --git a/test/short/fixes/single_cascade_statements.stmt b/test/short/fixes/single_cascade_statements.stmt deleted file mode 100644 index 2c769c84..00000000 --- a/test/short/fixes/single_cascade_statements.stmt +++ /dev/null @@ -1,149 +0,0 @@ -40 columns | ->>> -obj..method(); -<<< -obj.method(); ->>> -obj..getter; -<<< -obj.getter; ->>> -obj..setter = 3; -<<< -obj.setter = 3; ->>> -obj..[subscript] = 3; -<<< -obj[subscript] = 3; ->>> -obj?..[subscript] = 3; -<<< -obj?[subscript] = 3; ->>> -obj..index[subscript] = 3; -<<< -obj.index[subscript] = 3; ->>> -obj..index?[subscript] = 3; -<<< -obj.index?[subscript] = 3; ->>> targets that don't need parentheses -{ - null..method(); - 123..method(); - true..method(); - "str"..method(); - "str$interp"..method(); - [1]..method(); - () {}..method(); - - this..method(); - new C()..method(); - const C()..method(); - foo()..method(); - foo.bar()..method(); - foo?.bar()..method(); - super.foo()..method(); -} -<<< -{ - null.method(); - 123.method(); - true.method(); - "str".method(); - "str$interp".method(); - [1].method(); - () {}.method(); - - this.method(); - new C().method(); - const C().method(); - foo().method(); - foo.bar().method(); - foo?.bar().method(); - super.foo().method(); -} ->>> targets that do need parentheses -{ - a as C..method(); - a is C..method(); - c ? a : b..method(); - a ?? b..method(); - a && b..method(); - a || b..method(); - a == b..method(); - a != b..method(); - a < b..method(); - a > b..method(); - a <= b..method(); - a >= b..method(); - a ^ b..method(); - a | b..method(); - a << b..method(); - a >> b..method(); - a + b..method(); - a - b..method(); - a * b..method(); - a / b..method(); - a ~/ b..method(); - -a..method(); - !a..method(); - ~a..method(); - a++..method(); - a--..method(); - - foo() async { - await a..method(); - } -} -<<< -{ - (a as C).method(); - (a is C).method(); - (c ? a : b).method(); - (a ?? b).method(); - (a && b).method(); - (a || b).method(); - (a == b).method(); - (a != b).method(); - (a < b).method(); - (a > b).method(); - (a <= b).method(); - (a >= b).method(); - (a ^ b).method(); - (a | b).method(); - (a << b).method(); - (a >> b).method(); - (a + b).method(); - (a - b).method(); - (a * b).method(); - (a / b).method(); - (a ~/ b).method(); - (-a).method(); - (!a).method(); - (~a).method(); - (a++).method(); - (a--).method(); - - foo() async { - (await a).method(); - } -} ->>> unaffected expressions -{ - foo..bar()..method(); - a = b..method(); - a += b..method(); - () => a..method(); - throw a..method(); -} -<<< -{ - foo - ..bar() - ..method(); - a = b..method(); - a += b..method(); - () => a..method(); - throw a..method(); -} \ No newline at end of file diff --git a/test/short/regression/0700/0707.stmt b/test/short/regression/0700/0707.stmt index c9f464a5..fd599a2f 100644 --- a/test/short/regression/0700/0707.stmt +++ b/test/short/regression/0700/0707.stmt @@ -1,4 +1,6 @@ ->>> (indent 4) (fix optional-new) +### This test used to test --fix behavior but since that feature was removed, +### the test now just verifies that the formatter preserves the original code. +>>> (indent 4) sink.write('FILE ACCESSED ${new DateTime.now()}\n'); <<< - sink.write('FILE ACCESSED ${DateTime.now()}\n'); \ No newline at end of file + sink.write('FILE ACCESSED ${new DateTime.now()}\n'); \ No newline at end of file diff --git a/test/short/regression/0700/0720.unit b/test/short/regression/0700/0720.unit index 61b9faf9..d9a8926a 100644 --- a/test/short/regression/0700/0720.unit +++ b/test/short/regression/0700/0720.unit @@ -1,12 +1,14 @@ ->>> (fix optional-const) +### This test used to test --fix behavior but since that feature was removed, +### the test now just verifies that the formatter preserves the original code. +>>> @meta(const Foo(const [])) library foo; <<< -@meta(Foo([])) +@meta(const Foo(const [])) library foo; ->>> (fix optional-const) +>>> @meta(const Foo(const [])) import 'foo.dart'; <<< -@meta(Foo([])) +@meta(const Foo(const [])) import 'foo.dart'; \ No newline at end of file diff --git a/test/short/whitespace/switch.stmt b/test/short/whitespace/switch.stmt index 598f5910..be073431 100644 --- a/test/short/whitespace/switch.stmt +++ b/test/short/whitespace/switch.stmt @@ -227,7 +227,7 @@ var x = switch (obj) { 1 => 'one', var two => 'two' }; ->>> handle cases that are not valid patterns +>>> (version 2.19) handle cases in old code that are not valid patterns switch (obj) { case {1, 2}: case -pi: diff --git a/test/tall/statement/switch_legacy.stmt b/test/tall/statement/switch_legacy.stmt index 22e2debb..71771478 100644 --- a/test/tall/statement/switch_legacy.stmt +++ b/test/tall/statement/switch_legacy.stmt @@ -1,7 +1,7 @@ 40 columns | ### Tests syntax that used to be valid in a switch case before Dart 3.0 but is ### invalid in 3.0 and later. ->>> Handle cases that are not valid patterns. +>>> (version 2.19) Handle cases that are not valid patterns. switch (obj) { case {1, 2}: case -pi: diff --git a/test/utils.dart b/test/utils.dart index 45a3c5b1..b36cde16 100644 --- a/test/utils.dart +++ b/test/utils.dart @@ -78,14 +78,14 @@ Future runFormatterOnDir([List? args]) { } /// Run tests defined in "*.unit" and "*.stmt" files inside directory [path]. -Future testDirectory(String path, {Iterable? fixes}) async { +Future testDirectory(String path) async { for (var test in await TestFile.listDirectory(path)) { - _testFile(test, fixes); + _testFile(test); } } -Future testFile(String path, {Iterable? fixes}) async { - _testFile(await TestFile.read(path), fixes); +Future testFile(String path) async { + _testFile(await TestFile.read(path)); } /// Format all of the benchmarks and ensure they produce their expected outputs. @@ -96,6 +96,7 @@ Future testBenchmarks({required bool useTallStyle}) async { for (var benchmark in benchmarks) { test(benchmark.name, () { var formatter = DartFormatter( + languageVersion: DartFormatter.latestLanguageVersion, pageWidth: benchmark.pageWidth, experimentFlags: useTallStyle ? const ['inline-class', 'macros', tallStyleExperimentFlag] @@ -123,23 +124,17 @@ Future testBenchmarks({required bool useTallStyle}) async { }); } -void _testFile(TestFile testFile, Iterable? baseFixes) { +void _testFile(TestFile testFile) { var useTallStyle = testFile.path.startsWith('tall/') || testFile.path.startsWith('tall\\'); group(testFile.path, () { for (var formatTest in testFile.tests) { test(formatTest.label, () { - var fixes = [...?baseFixes, ...formatTest.fixes]; - - if (useTallStyle && fixes.isNotEmpty) { - fail('Test error: Tall style does not support applying fixes.'); - } - var formatter = DartFormatter( + languageVersion: formatTest.languageVersion, pageWidth: testFile.pageWidth, indent: formatTest.leadingIndent, - fixes: fixes, experimentFlags: useTallStyle ? const ['inline-class', 'macros', tallStyleExperimentFlag] : const ['inline-class', 'macros']); diff --git a/tool/update_tests.dart b/tool/update_tests.dart index 17cf1dd5..9c8affcd 100644 --- a/tool/update_tests.dart +++ b/tool/update_tests.dart @@ -87,23 +87,6 @@ Future _updateTestFile(TestFile testFile) async { // Write the file-level comments. _writeComments(buffer, testFile.comments); - // TODO(rnystrom): This is duplicating logic in fix_test.dart. Ideally, we'd - // move the fix markers into the tests themselves, but since --fix is - // probably going away, it's not worth it. - var baseFixes = const { - 'short/fixes/doc_comments.stmt': [StyleFix.docComments], - 'short/fixes/function_typedefs.unit': [StyleFix.functionTypedefs], - 'short/fixes/named_default_separator.unit': [ - StyleFix.namedDefaultSeparator - ], - 'short/fixes/optional_const.unit': [StyleFix.optionalConst], - 'short/fixes/optional_new.stmt': [StyleFix.optionalNew], - 'short/fixes/single_cascade_statements.stmt': [ - StyleFix.singleCascadeStatements - ], - }[testFile.path] ?? - const []; - var experiments = [ 'inline-class', 'macros', @@ -114,9 +97,9 @@ Future _updateTestFile(TestFile testFile) async { for (var formatTest in testFile.tests) { var formatter = DartFormatter( + languageVersion: formatTest.languageVersion, pageWidth: testFile.pageWidth, indent: formatTest.leadingIndent, - fixes: [...baseFixes, ...formatTest.fixes], experimentFlags: experiments); var actual = formatter.formatSource(formatTest.input); @@ -132,7 +115,9 @@ Future _updateTestFile(TestFile testFile) async { var descriptionParts = [ if (formatTest.leadingIndent != 0) '(indent ${formatTest.leadingIndent})', - for (var fix in formatTest.fixes) '(fix ${fix.name})', + if (formatTest.languageVersion != DartFormatter.latestLanguageVersion) + '(version ${formatTest.languageVersion.major}.' + '${formatTest.languageVersion.minor})', formatTest.description ];