feat: add static code diagnostic arguments-ordering

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## Unreleased
+
+* feat: add static code diagnostic [`arguments-ordering`](https://dartcodemetrics.dev/docs/rules/common/arguments-ordering).
+
 ## 5.0.1
 
 * fix: correctly available check rule names.

lib/src/analyzers/lint_analyzer/rules/rules_factory.dart

@@ -1,5 +1,6 @@
 import 'models/rule.dart';
 import 'rules_list/always_remove_listener/always_remove_listener_rule.dart';
+import 'rules_list/arguments_ordering/arguments_ordering_rule.dart';
 import 'rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart';
 import 'rules_list/avoid_border_all/avoid_border_all_rule.dart';
 import 'rules_list/avoid_cascade_after_if_null/avoid_cascade_after_if_null_rule.dart';
@@ -68,6 +69,7 @@ import 'rules_list/tag_name/tag_name_rule.dart';
 
 final _implementedRules = <String, Rule Function(Map<String, Object>)>{
   AlwaysRemoveListenerRule.ruleId: AlwaysRemoveListenerRule.new,
+  ArgumentsOrderingRule.ruleId: ArgumentsOrderingRule.new,
   AvoidBannedImportsRule.ruleId: AvoidBannedImportsRule.new,
   AvoidBorderAllRule.ruleId: AvoidBorderAllRule.new,
   AvoidCascadeAfterIfNullRule.ruleId: AvoidCascadeAfterIfNullRule.new,

lib/src/analyzers/lint_analyzer/rules/rules_list/arguments_ordering/arguments_ordering_rule.dart

@@ -0,0 +1,58 @@
+// ignore_for_file: public_member_api_docs
+
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/visitor.dart';
+import 'package:analyzer/dart/element/element.dart';
+import 'package:collection/collection.dart';
+
+import '../../../../../utils/node_utils.dart';
+import '../../../lint_utils.dart';
+import '../../../models/internal_resolved_unit_result.dart';
+import '../../../models/issue.dart';
+import '../../../models/replacement.dart';
+import '../../../models/severity.dart';
+import '../../models/common_rule.dart';
+import '../../rule_utils.dart';
+
+part 'config_parser.dart';
+part 'visitor.dart';
+
+class ArgumentsOrderingRule extends CommonRule {
+  static const String ruleId = 'arguments-ordering';
+
+  static const _warningMessage =
+      'Named arguments should be sorted to match parameters declaration order.';
+  static const _replaceComment = 'Sort arguments';
+
+  final bool _childLast;
+
+  ArgumentsOrderingRule([Map<String, Object> config = const {}])
+      : _childLast = _ConfigParser.parseChildLast(config),
+        super(
+          id: ruleId,
+          severity: readSeverity(config, Severity.style),
+          excludes: readExcludes(config),
+          includes: readIncludes(config),
+        );
+
+  @override
+  Iterable<Issue> check(InternalResolvedUnitResult source) {
+    final visitor = _Visitor(childLast: _childLast);
+
+    source.unit.visitChildren(visitor);
+
+    return visitor.issues
+        .map(
+          (issue) => createIssue(
+            rule: this,
+            location: nodeLocation(node: issue.argumentList, source: source),
+            message: _warningMessage,
+            replacement: Replacement(
+              comment: _replaceComment,
+              replacement: issue.replacement,
+            ),
+          ),
+        )
+        .toList(growable: false);
+  }
+}

lib/src/analyzers/lint_analyzer/rules/rules_list/arguments_ordering/config_parser.dart

@@ -0,0 +1,9 @@
+part of 'arguments_ordering_rule.dart';
+
+class _ConfigParser {
+  static const _childLast = 'child-last';
+  static const _childLastDefault = false;
+
+  static bool parseChildLast(Map<String, Object> config) =>
+      config[_childLast] as bool? ?? _childLastDefault;
+}

lib/src/analyzers/lint_analyzer/rules/rules_list/arguments_ordering/visitor.dart

@@ -0,0 +1,105 @@
+part of 'arguments_ordering_rule.dart';
+
+class _Visitor extends RecursiveAstVisitor<void> {
+  final bool childLast;
+
+  static const _childArg = 'child';
+  static const _childrenArg = 'children';
+  static const _childArgs = [_childArg, _childrenArg];
+
+  final _issues = <_IssueDetails>[];
+
+  Iterable<_IssueDetails> get issues => _issues;
+
+  _Visitor({required this.childLast});
+
+  @override
+  void visitInstanceCreationExpression(InstanceCreationExpression node) {
+    super.visitInstanceCreationExpression(node);
+
+    _checkOrder(
+      node.argumentList,
+      node.constructorName.staticElement?.parameters ?? [],
+    );
+  }
+
+  @override
+  void visitMethodInvocation(MethodInvocation node) {
+    super.visitMethodInvocation(node);
+    final staticElement = node.methodName.staticElement;
+    if (staticElement is FunctionElement) {
+      _checkOrder(
+        node.argumentList,
+        staticElement.parameters,
+      );
+    }
+  }
+
+  void _checkOrder(
+    ArgumentList argumentList,
+    List<ParameterElement> parameters,
+  ) {
+    final sortedArguments = argumentList.arguments.sorted((a, b) {
+      if (a is! NamedExpression && b is! NamedExpression) {
+        return 0;
+      }
+      if (a is NamedExpression && b is! NamedExpression) {
+        return 1;
+      }
+      if (a is! NamedExpression && b is NamedExpression) {
+        return -1;
+      }
+      if (a is NamedExpression && b is NamedExpression) {
+        final aName = a.name.label.name;
+        final bName = b.name.label.name;
+
+        if (aName == bName) {
+          return 0;
+        }
+
+        // We use simplified version for "child" argument check from "sort_child_properties_last" rule
+        // https://github.com/dart-lang/linter/blob/1933b2a2969380e5db35d6aec524fb21b0ed028b/lib/src/rules/sort_child_properties_last.dart#L140
+        // Hopefully, this will be enough for our current needs.
+        if (childLast &&
+            _childArgs.any((name) => name == aName || name == bName)) {
+          return (_childArgs.contains(aName) && !_childArgs.contains(bName)) ||
+                  (aName == _childArg)
+              ? 1
+              : -1;
+        }
+
+        return _parameterIndex(parameters, a)
+            .compareTo(_parameterIndex(parameters, b));
+      }
+
+      return 0;
+    });
+
+    if (argumentList.arguments.toString() != sortedArguments.toString()) {
+      _issues.add(
+        _IssueDetails(
+          argumentList: argumentList,
+          replacement: '(${sortedArguments.join(', ')})',
+        ),
+      );
+    }
+  }
+
+  static int _parameterIndex(
+    List<ParameterElement> parameters,
+    NamedExpression argumentExpression,
+  ) =>
+      parameters.indexWhere(
+        (parameter) => parameter.name == argumentExpression.name.label.name,
+      );
+}
+
+class _IssueDetails {
+  const _IssueDetails({
+    required this.argumentList,
+    required this.replacement,
+  });
+
+  final ArgumentList argumentList;
+  final String replacement;
+}

test/src/analyzers/lint_analyzer/rules/rules_list/arguments_ordering/arguments_ordering_rule_test.dart

@@ -0,0 +1,163 @@
+import 'package:dart_code_metrics/src/analyzers/lint_analyzer/models/severity.dart';
+import 'package:dart_code_metrics/src/analyzers/lint_analyzer/rules/rules_list/arguments_ordering/arguments_ordering_rule.dart';
+import 'package:test/test.dart';
+
+import '../../../../../helpers/rule_test_helper.dart';
+
+const _classExamplePath = 'arguments_ordering/examples/class_example.dart';
+const _functionExamplePath =
+    'arguments_ordering/examples/function_example.dart';
+const _widgetExamplePath = 'arguments_ordering/examples/widget_example.dart';
+const _widgetExampleChildLastPath =
+    'arguments_ordering/examples/widget_example_child_last.dart';
+const _mixedExamplePath = 'arguments_ordering/examples/mixed_example.dart';
+
+void main() {
+  group('ArgumentsOrderingRule', () {
+    test('initialization', () async {
+      final unit = await RuleTestHelper.resolveFromFile(_classExamplePath);
+      final issues = ArgumentsOrderingRule().check(unit);
+
+      RuleTestHelper.verifyInitialization(
+        issues: issues,
+        ruleId: 'arguments-ordering',
+        severity: Severity.style,
+      );
+    });
+
+    test('reports issues with class constructor arguments', () async {
+      final unit = await RuleTestHelper.resolveFromFile(_classExamplePath);
+      final issues = ArgumentsOrderingRule().check(unit);
+
+      RuleTestHelper.verifyIssues(
+        issues: issues,
+        startLines: [16, 17, 18, 19, 20, 21],
+        startColumns: [26, 26, 26, 26, 26, 26],
+        locationTexts: [
+          "(name: 42, surname: '', age: '')",
+          "(surname: '', name: '', age: 42)",
+          "(surname: '', age: 42, name: '')",
+          "(age: 42, surname: '', name: '')",
+          "(age: 42, name: '', surname: '')",
+          "(age: 42, name: '')",
+        ],
+        replacements: [
+          "(name: 42, age: '', surname: '')",
+          "(name: '', age: 42, surname: '')",
+          "(name: '', age: 42, surname: '')",
+          "(name: '', age: 42, surname: '')",
+          "(name: '', age: 42, surname: '')",
+          "(name: '', age: 42)",
+        ],
+      );
+    });
+
+    test('reports issues with function arguments', () async {
+      final unit = await RuleTestHelper.resolveFromFile(_functionExamplePath);
+      final issues = ArgumentsOrderingRule().check(unit);
+
+      RuleTestHelper.verifyIssues(
+        issues: issues,
+        startLines: [8, 9, 10, 11, 12, 13],
+        startColumns: [32, 32, 32, 32, 32, 32],
+        locationTexts: [
+          "(name: 42, surname: '', age: '')",
+          "(surname: '', name: '', age: 42)",
+          "(surname: '', age: 42, name: '')",
+          "(age: 42, surname: '', name: '')",
+          "(age: 42, name: '', surname: '')",
+          "(age: 42, name: '')",
+        ],
+        replacements: [
+          "(name: 42, age: '', surname: '')",
+          "(name: '', age: 42, surname: '')",
+          "(name: '', age: 42, surname: '')",
+          "(name: '', age: 42, surname: '')",
+          "(name: '', age: 42, surname: '')",
+          "(name: '', age: 42)",
+        ],
+      );
+    });
+
+    test('reports issues with Flutter widget arguments', () async {
+      final unit = await RuleTestHelper.resolveFromFile(_widgetExamplePath);
+      final issues = ArgumentsOrderingRule().check(unit);
+
+      RuleTestHelper.verifyIssues(
+        issues: issues,
+        startLines: [17, 18, 19, 20, 21],
+        startColumns: [32, 32, 32, 32, 32],
+        locationTexts: [
+          "(name: '', age: 42, child: Container())",
+          "(child: Container(), name: '', age: 42)",
+          "(child: Container(), age: 42, name: '')",
+          "(age: 42, child: Container(), name: '')",
+          "(age: 42, name: '', child: Container())",
+        ],
+        replacements: [
+          "(name: '', child: Container(), age: 42)",
+          "(name: '', child: Container(), age: 42)",
+          "(name: '', child: Container(), age: 42)",
+          "(name: '', child: Container(), age: 42)",
+          "(name: '', child: Container(), age: 42)",
+        ],
+      );
+    });
+
+    test(
+      'reports issues with Flutter widget arguments and "childLast" config option',
+      () async {
+        final unit =
+            await RuleTestHelper.resolveFromFile(_widgetExampleChildLastPath);
+        final issues = ArgumentsOrderingRule({'child-last': true}).check(unit);
+
+        RuleTestHelper.verifyIssues(
+          issues: issues,
+          startLines: [15, 18, 20, 22, 24, 26],
+          startColumns: [32, 17, 17, 17, 17, 17],
+          locationTexts: [
+            "(name: '', child: Container(), children: [])",
+            "(name: '', child: Container(), children: [])",
+            "(child: Container(), children: [], name: '')",
+            "(child: Container(), name: '', children: [])",
+            "(children: [], child: Container(), name: '')",
+            "(children: [], name: '', child: Container())",
+          ],
+          replacements: [
+            "(name: '', children: [], child: Container())",
+            "(name: '', children: [], child: Container())",
+            "(name: '', children: [], child: Container())",
+            "(name: '', children: [], child: Container())",
+            "(name: '', children: [], child: Container())",
+            "(name: '', children: [], child: Container())",
+          ],
+        );
+      },
+    );
+
+    test('reports issues with mixed (named + unnamed) arguments', () async {
+      final unit = await RuleTestHelper.resolveFromFile(_mixedExamplePath);
+      final issues = ArgumentsOrderingRule().check(unit);
+
+      RuleTestHelper.verifyIssues(
+        issues: issues,
+        startLines: [6, 7, 8, 9, 10],
+        startColumns: [20, 20, 20, 20, 20],
+        locationTexts: [
+          "('a', c: 'c', 'b', d: 'd')",
+          "('a', d: 'd', 'b', c: 'c')",
+          "('a', 'b', d: 'd', c: 'c')",
+          "('a', c: 'c', 'b')",
+          "(c: 'c', 'a', 'b')",
+        ],
+        replacements: [
+          "('a', 'b', c: 'c', d: 'd')",
+          "('a', 'b', c: 'c', d: 'd')",
+          "('a', 'b', c: 'c', d: 'd')",
+          "('a', 'b', c: 'c')",
+          "('a', 'b', c: 'c')",
+        ],
+      );
+    });
+  });
+}

test/src/analyzers/lint_analyzer/rules/rules_list/arguments_ordering/examples/class_example.dart

@@ -0,0 +1,21 @@
+class Person {
+  Person({
+    required this.name,
+    required this.age,
+    this.surname,
+  });
+
+  final String name;
+  final int age;
+  final String? surname;
+}
+
+final goodPerson1 = Person(name: '', age: 42, surname: '');
+final goodPerson2 = Person(name: '', age: 42);
+
+final badPerson1 = Person(name: 42, surname: '', age: ''); // LINT
+final badPerson2 = Person(surname: '', name: '', age: 42); // LINT
+final badPerson3 = Person(surname: '', age: 42, name: ''); // LINT
+final badPerson4 = Person(age: 42, surname: '', name: ''); // LINT
+final badPerson5 = Person(age: 42, name: '', surname: ''); // LINT
+final badPerson6 = Person(age: 42, name: ''); // LINT

test/src/analyzers/lint_analyzer/rules/rules_list/arguments_ordering/examples/function_example.dart

@@ -0,0 +1,13 @@
+Person? createPerson(
+        {required String name, required int age, String? surname}) =>
+    null;
+
+final goodPerson1 = createPerson(name: '', age: 42, surname: '');
+final goodPerson2 = createPerson(name: '', age: 42);
+
+final badPerson1 = createPerson(name: 42, surname: '', age: ''); // LINT
+final badPerson2 = createPerson(surname: '', name: '', age: 42); // LINT
+final badPerson3 = createPerson(surname: '', age: 42, name: ''); // LINT
+final badPerson4 = createPerson(age: 42, surname: '', name: ''); // LINT
+final badPerson5 = createPerson(age: 42, name: '', surname: ''); // LINT
+final badPerson6 = createPerson(age: 42, name: ''); // LINT