linter: Adjust require_trailing_commas reported token, and description

Fixes https://github.com/dart-lang/linter/issues/4918

I migrated relevant tests.

Description fixes:

* Don't use "function calls" or "declarations". First, I don't think
  we use the term "function call" much, I could be wrong. Second, it's
  not about function calls or declarations. It's about parameter lists
  and argument lists.
* Remove more mixing up of parameters and arguments. E.g. A "final
  parameter" cannot be a list literal.
* Fix naming of "literal array" to "list literal" and a few others.

Change-Id: I15844f4a34dc4fe08f15dc00bce2592ec12ce5c7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/359841
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>

pkg/linter/lib/src/rules/require_trailing_commas.dart

@@ -8,12 +8,13 @@ import 'package:analyzer/dart/ast/visitor.dart';
 
 import '../analyzer.dart';
 
-const _desc = r'Use trailing commas for all function calls and declarations.';
+const _desc =
+    r'Use trailing commas for all parameter lists and argument lists.';
 
 const _details = r'''
-**DO** use trailing commas for all function calls and declarations unless the
-function call or definition, from the start of the function name up to the
-closing parenthesis, fits in a single line.
+**DO** use trailing commas for all multi-line parameter lists and argument
+lists. A parameter list or argument list that fits on one line, including the
+opening parenthesis and closing parenthesis, does not require a trailing comma.
 
 **BAD:**
 ```dart
@@ -33,13 +34,14 @@ void run() {
 }
 ```
 
-**EXCEPTION:** If the final parameter/argument is positional (vs named) and is
-either a function literal implemented using curly braces, a literal map, a
-literal set or a literal array. This exception only applies if the final
-parameter does not fit entirely on one line.
+**EXCEPTION:** If the final argument in an argument list is positional (vs
+named) and is either a function literal with curly braces, a map literal, a set
+literal, or a list literal, then a trailing comma is not required.
+This exception only applies if the final argument does not fit entirely on one
+line.
 
-**NOTE:** This lint rule assumes `dart format` has been run over the code and
-may produce false positives until that has happened.
+**NOTE:** This lint rule assumes that code has been formatted with `dart format`
+and may produce false positives on unformatted code.
 
 ''';
 
@@ -93,9 +95,9 @@ class _Visitor extends SimpleAstVisitor<void> {
     super.visitArgumentList(node);
     if (node.arguments.isEmpty) return;
     _checkTrailingComma(
-      node.leftParenthesis,
-      node.rightParenthesis,
-      node.arguments.last,
+      openingToken: node.leftParenthesis,
+      closingToken: node.rightParenthesis,
+      lastNode: node.arguments.last,
     );
   }
 
@@ -103,9 +105,9 @@ class _Visitor extends SimpleAstVisitor<void> {
   void visitAssertInitializer(AssertInitializer node) {
     super.visitAssertInitializer(node);
     _checkTrailingComma(
-      node.leftParenthesis,
-      node.rightParenthesis,
-      node.message ?? node.condition,
+      openingToken: node.leftParenthesis,
+      closingToken: node.rightParenthesis,
+      lastNode: node.message ?? node.condition,
     );
   }
 
@@ -113,9 +115,9 @@ class _Visitor extends SimpleAstVisitor<void> {
   void visitAssertStatement(AssertStatement node) {
     super.visitAssertStatement(node);
     _checkTrailingComma(
-      node.leftParenthesis,
-      node.rightParenthesis,
-      node.message ?? node.condition,
+      openingToken: node.leftParenthesis,
+      closingToken: node.rightParenthesis,
+      lastNode: node.message ?? node.condition,
     );
   }
 
@@ -127,9 +129,10 @@ class _Visitor extends SimpleAstVisitor<void> {
     super.visitFormalParameterList(node);
     if (node.parameters.isEmpty) return;
     _checkTrailingComma(
-      node.leftParenthesis,
-      node.rightParenthesis,
-      node.parameters.last,
+      openingToken: node.leftParenthesis,
+      closingToken: node.rightParenthesis,
+      lastNode: node.parameters.last,
+      errorToken: node.rightDelimiter ?? node.rightParenthesis,
     );
   }
 
@@ -138,9 +141,9 @@ class _Visitor extends SimpleAstVisitor<void> {
     super.visitListLiteral(node);
     if (node.elements.isNotEmpty) {
       _checkTrailingComma(
-        node.leftBracket,
-        node.rightBracket,
-        node.elements.last,
+        openingToken: node.leftBracket,
+        closingToken: node.rightBracket,
+        lastNode: node.elements.last,
       );
     }
   }
@@ -150,18 +153,21 @@ class _Visitor extends SimpleAstVisitor<void> {
     super.visitSetOrMapLiteral(node);
     if (node.elements.isNotEmpty) {
       _checkTrailingComma(
-        node.leftBracket,
-        node.rightBracket,
-        node.elements.last,
+        openingToken: node.leftBracket,
+        closingToken: node.rightBracket,
+        lastNode: node.elements.last,
       );
     }
   }
 
-  void _checkTrailingComma(
-    Token leftParenthesis,
-    Token rightParenthesis,
-    AstNode lastNode,
-  ) {
+  void _checkTrailingComma({
+    required Token openingToken,
+    required Token closingToken,
+    required AstNode lastNode,
+    Token? errorToken,
+  }) {
+    errorToken ??= closingToken;
+
     // Early exit if trailing comma is present.
     if (lastNode.endToken.next?.type == TokenType.COMMA) return;
 
@@ -170,12 +176,12 @@ class _Visitor extends SimpleAstVisitor<void> {
     // right parenthesis are on the same line is sufficient since `dart format`
     // places the left parenthesis right after the identifier (on the same
     // line).
-    if (_isSameLine(leftParenthesis, rightParenthesis)) return;
+    if (_isSameLine(openingToken, closingToken)) return;
 
     // Check the last parameter to determine if there are any exceptions.
     if (_shouldAllowTrailingCommaException(lastNode)) return;
 
-    rule.reportLintForToken(rightParenthesis, errorCode: _trailingCommaCode);
+    rule.reportLintForToken(errorToken, errorCode: _trailingCommaCode);
   }
 
   bool _isSameLine(Token token1, Token token2) =>
@@ -194,7 +200,8 @@ class _Visitor extends SimpleAstVisitor<void> {
       return true;
     }
 
-    // Exception is allowed if the last argument is a (multiline) string literal.
+    // Exception is allowed if the last argument is a (multiline) string
+    // literal.
     if (lastNode is StringLiteral) return true;
 
     // Exception is allowed if the last argument is a anonymous function call.

pkg/linter/test/rules/all.dart

@@ -177,6 +177,7 @@ import 'prefer_void_to_null_test.dart' as prefer_void_to_null;
 import 'provide_deprecation_message_test.dart' as provide_deprecation_message;
 import 'public_member_api_docs_test.dart' as public_member_api_docs;
 import 'recursive_getters_test.dart' as recursive_getters;
+import 'require_trailing_commas_test.dart' as require_trailing_commas;
 import 'secure_pubspec_urls_test.dart' as secure_pubspec_urls;
 import 'sized_box_for_whitespace_test.dart' as sized_box_for_whitespace;
 import 'sized_box_shrink_expand_test.dart' as sized_box_shrink_expand;
@@ -388,6 +389,7 @@ void main() {
   provide_deprecation_message.main();
   public_member_api_docs.main();
   recursive_getters.main();
+  require_trailing_commas.main();
   secure_pubspec_urls.main();
   sized_box_for_whitespace.main();
   sized_box_shrink_expand.main();

pkg/linter/test/rules/require_trailing_commas_test.dart

@@ -0,0 +1,81 @@
+// 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:test_reflective_loader/test_reflective_loader.dart';
+
+import '../rule_test_support.dart';
+
+main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(RequireTrailingCommasTest);
+  });
+}
+
+@reflectiveTest
+class RequireTrailingCommasTest extends LintRuleTest {
+  @override
+  String get lintRule => 'require_trailing_commas';
+
+  test_function_parameters_multiLine() async {
+    await assertDiagnostics(r'''
+void method4(int one,
+    int two) {}
+''', [
+      lint(33, 1),
+    ]);
+  }
+
+  test_function_parameters_multiLine_withComma() async {
+    await assertNoDiagnostics(r'''
+void f(
+  int one,
+  int two,
+) {}
+''');
+  }
+
+  test_function_parameters_withNamed_mulitLine_withComma() async {
+    await assertNoDiagnostics(r'''
+void f(
+  int one, {
+  int? two,
+}) {}
+''');
+  }
+
+  test_function_parameters_withNamed_multiLine() async {
+    await assertDiagnostics(r'''
+void f(int one,
+    {int two = 2}) {}
+''', [
+      lint(32, 1),
+    ]);
+  }
+
+  test_function_parameters_withNamed_singleLine() async {
+    await assertNoDiagnostics(r'''
+  void method1(Object p1, Object p2, {Object? param3, Object? param4}) {}
+''');
+  }
+
+  test_functionLiteral_parameters_multiLine() async {
+    await assertDiagnostics(r'''
+void f() {
+  (int one,
+      int two)
+      {};
+}
+''', [
+      lint(36, 1),
+    ]);
+  }
+
+  test_functionLiteral_parameters_singleLine() async {
+    await assertNoDiagnostics(r'''
+void f() {
+  (a, b) {};
+}
+''');
+  }
+}

pkg/linter/test_data/rules/require_trailing_commas.dart

@@ -37,40 +37,6 @@ class RequireTrailingCommasExample {
       : assert(true,
             'A very very very very very very very very long string'); // LINT
 
-  void method1(Object p1, Object p2, {Object? param3, Object? param4}) {}
-
-  void method2(
-    Object param1,
-    Object param2,
-    Object param3,
-    Object param4,
-    Object param5,
-  ) {}
-
-  void method3(
-    Object param1,
-    Object param2, {
-    Object? param3,
-    Object? param4,
-    Object? param5,
-  }) {}
-
-  void method4(Object param1, Object param2, Object param3, Object param4,
-      Object param5) {} // LINT
-
-  void method5(Object param1, Object param2,
-      {Object? param3, Object? param4, Object? param5}) {} // LINT
-
-  void method6(Object param1, Object param2,
-      {Object? param3,
-      Object? param4,
-      Object? param5,
-      Object? param6,
-      Object? param7}) {} // LINT
-
-  void method7(Object param1, Object param2, Object param3,
-      {Object namedParam = true}) {} // LINT
-
   void run() {
     void test(Object param1, Object param2, {Object? param3}) {}