Merge d3e8923 into 4b3ebb9

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## Unreleased (2.4.0)
+
+- Add static code diagnostic avoid-unused-parameters
+
 ## 2.3.2
 
 - Add Gitlab Code Quality support in Code Climate report

README.md

@@ -126,6 +126,7 @@ If you want command line tool to check rules, you should add configuration to yo
 
 ### Common
 
+- [avoid-unused-parameters](https://github.com/wrike/dart-code-metrics/blob/master/doc/rules/avoid_unused_parameters.md)
 - [binary-expression-operand-order](https://github.com/wrike/dart-code-metrics/blob/master/doc/rules/binary_expression_operand_order.md)   ![Has auto-fix](https://img.shields.io/badge/-has%20auto--fix-success)
 - [double-literal-format](https://github.com/wrike/dart-code-metrics/blob/master/doc/rules/double_literal_format.md)   ![Has auto-fix](https://img.shields.io/badge/-has%20auto--fix-success)
 - [member-ordering](https://github.com/wrike/dart-code-metrics/blob/master/doc/rules/member_ordering.md)   ![Configurable](https://img.shields.io/badge/-configurable-informational)

doc/rules/avoid_unused_parameters.md

@@ -0,0 +1,54 @@
+# Avoid unused parameters
+
+## Rule id
+
+avoid-unused-parameters
+
+## Description
+
+Checks for unused parameters inside a function or method body.
+For overridden methods suggests renaming unused parameters to \_, \_\_, etc.
+
+Note: abstract classes are completely ignored by the rule to avoid redundant checks for potentially overridden methods.
+
+### Example
+
+Bad:
+
+```dart
+void someFunction(String s) {
+  return;
+}
+
+class SomeClass {
+  void method(String s) {
+    return;
+  }
+}
+```
+
+Good:
+
+```dart
+void someFunction() {
+  return;
+}
+
+class SomeClass {
+  void method() {
+    return;
+  }
+}
+
+void someFunction(String s) {
+  print(s);
+  return;
+}
+
+class SomeClass {
+  void method(String s) {
+    print(s);
+    return;
+  }
+}
+```

lib/src/rules/avoid_unused_parameters.dart

@@ -0,0 +1,164 @@
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/syntactic_entity.dart';
+import 'package:analyzer/dart/ast/visitor.dart';
+
+import '../models/code_issue.dart';
+import '../models/code_issue_severity.dart';
+import '../models/source.dart';
+import 'base_rule.dart';
+import 'rule_utils.dart';
+
+// Inspired by PVS-Studio (https://www.viva64.com/en/w/v6022/)
+
+class AvoidUnusedParameters extends BaseRule {
+  static const String ruleId = 'avoid-unused-parameters';
+  static const _documentationUrl = 'https://git.io/JL153';
+
+  static const _warningMessage = 'Parameter is unused';
+  static const _renameMessage =
+      'Parameter is unused, consider renaming it to _, __, etc.';
+
+  AvoidUnusedParameters({
+    Map<String, Object> config = const {},
+  }) : super(
+          id: ruleId,
+          documentation: Uri.parse(_documentationUrl),
+          severity: CodeIssueSeverity.fromJson(config['severity'] as String) ??
+              CodeIssueSeverity.warning,
+        );
+
+  @override
+  Iterable<CodeIssue> check(Source source) {
+    final _visitor = _Visitor();
+
+    source.compilationUnit.visitChildren(_visitor);
+
+    return [
+      ..._visitor.unusedParameters
+          .map((parameter) => createIssue(
+                this,
+                _warningMessage,
+                null,
+                null,
+                source.url,
+                source.content,
+                source.compilationUnit.lineInfo,
+                parameter,
+              ))
+          .toList(growable: false),
+      ..._visitor.renameSuggestions
+          .map((parameter) => createIssue(
+                this,
+                _renameMessage,
+                null,
+                null,
+                source.url,
+                source.content,
+                source.compilationUnit.lineInfo,
+                parameter,
+              ))
+          .toList(),
+    ];
+  }
+}
+
+class _Visitor extends RecursiveAstVisitor<void> {
+  final _unusedParameters = <FormalParameter>[];
+  final _renameSuggestions = <FormalParameter>[];
+
+  Iterable<FormalParameter> get unusedParameters => _unusedParameters;
+
+  Iterable<FormalParameter> get renameSuggestions => _renameSuggestions;
+
+  @override
+  void visitMethodDeclaration(MethodDeclaration node) {
+    super.visitMethodDeclaration(node);
+
+    final parent = node.parent;
+
+    if (parent is ClassDeclaration && parent.isAbstract) {
+      return;
+    }
+
+    final isOverride = node.metadata.firstWhere(
+      (node) => node.name.name == 'override' && node.atSign.type.lexeme == '@',
+      orElse: () => null,
+    );
+
+    if (isOverride != null) {
+      _renameSuggestions.addAll(
+        _checkParameters(
+          node.body.childEntities,
+          node.parameters.parameters,
+        ).where(
+          (parameter) =>
+              parameter.identifier.name.replaceAll('_', '').isNotEmpty,
+        ),
+      );
+    } else {
+      _unusedParameters.addAll(
+        _checkParameters(
+          node.body.childEntities,
+          node.parameters.parameters,
+        ),
+      );
+    }
+  }
+
+  @override
+  void visitFunctionDeclaration(FunctionDeclaration node) {
+    super.visitFunctionDeclaration(node);
+
+    _unusedParameters.addAll(
+      _checkParameters(
+        node.functionExpression.body.childEntities,
+        node.functionExpression.parameters.parameters,
+      ),
+    );
+  }
+
+  Iterable<FormalParameter> _checkParameters(
+    Iterable<SyntacticEntity> children,
+    NodeList<FormalParameter> parameters,
+  ) {
+    final result = <FormalParameter>[];
+
+    final usages = _getAllUsages(children, [])
+        .map((identifier) => identifier.name)
+        .toList();
+
+    for (final parameter in parameters) {
+      if (!usages.contains(parameter.identifier.name)) {
+        result.add(parameter);
+      }
+    }
+
+    return result;
+  }
+
+  Iterable<Identifier> _getAllUsages(
+    Iterable<SyntacticEntity> children,
+    Iterable<String> ignoredNames,
+  ) {
+    final usages = <Identifier>[];
+    final ignored = [...ignoredNames];
+
+    for (final child in children) {
+      if (child is FunctionExpression) {
+        for (final parameter in child.parameters.parameters) {
+          ignored.add(parameter.identifier.name);
+        }
+      } else if (child is Identifier &&
+          !ignored.contains(child.name) &&
+          child.parent is! PropertyAccess) {
+        usages.add(child);
+      }
+
+      if (child is AstNode) {
+        usages.addAll(_getAllUsages(child.childEntities, ignored));
+      }
+    }
+
+    return usages;
+  }
+}

lib/src/rules_factory.dart

@@ -1,4 +1,5 @@
 import 'rules/avoid_preserve_whitespace_false.dart';
+import 'rules/avoid_unused_parameters.dart';
 import 'rules/base_rule.dart';
 import 'rules/binary_expression_operand_order_rule.dart';
 import 'rules/component_annotation_arguments_ordering.dart';
@@ -22,6 +23,8 @@ import 'rules/provide_correct_intl_args.dart';
 final _implementedRules = <String, BaseRule Function(Map<String, Object>)>{
   AvoidPreserveWhitespaceFalseRule.ruleId: (config) =>
       AvoidPreserveWhitespaceFalseRule(config: config),
+  AvoidUnusedParameters.ruleId: (config) =>
+      AvoidUnusedParameters(config: config),
   BinaryExpressionOperandOrderRule.ruleId: (config) =>
       BinaryExpressionOperandOrderRule(config: config),
   ComponentAnnotationArgumentsOrderingRule.ruleId: (config) =>

test/rules/avoid_unused_parameters/avoid_unused_parameters_test.dart

@@ -0,0 +1,109 @@
+import 'dart:io';
+
+import 'package:analyzer/dart/analysis/utilities.dart';
+import 'package:dart_code_metrics/src/models/code_issue_severity.dart';
+import 'package:dart_code_metrics/src/models/source.dart';
+import 'package:dart_code_metrics/src/rules/avoid_unused_parameters.dart';
+import 'package:test/test.dart';
+
+const correctExamplePath =
+    'test/rules/avoid_unused_parameters/examples/correct_example.dart';
+const incorrectExamplePath =
+    'test/rules/avoid_unused_parameters/examples/incorrect_example.dart';
+
+void main() {
+  group('AvoidUnusedParameters', () {
+    test('initialization', () async {
+      final path = File(correctExamplePath).absolute.path;
+      final sourceUrl = Uri.parse(path);
+
+      final parseResult = await resolveFile(path: path);
+
+      final issues = AvoidUnusedParameters().check(Source(
+        sourceUrl,
+        parseResult.content,
+        parseResult.unit,
+      ));
+
+      expect(
+        issues.every((issue) => issue.ruleId == 'avoid-unused-parameters'),
+        isTrue,
+      );
+      expect(
+        issues.every((issue) => issue.severity == CodeIssueSeverity.warning),
+        isTrue,
+      );
+    });
+
+    test('reports no issues', () async {
+      final path = File(correctExamplePath).absolute.path;
+      final sourceUrl = Uri.parse(path);
+
+      final parseResult = await resolveFile(path: path);
+
+      final issues = AvoidUnusedParameters().check(Source(
+        sourceUrl,
+        parseResult.content,
+        parseResult.unit,
+      ));
+
+      expect(issues.isEmpty, isTrue);
+    });
+
+    test('reports about found issues', () async {
+      final path = File(incorrectExamplePath).absolute.path;
+      final sourceUrl = Uri.parse(path);
+
+      final parseResult = await resolveFile(path: path);
+
+      final issues = AvoidUnusedParameters().check(Source(
+        sourceUrl,
+        parseResult.content,
+        parseResult.unit,
+      ));
+
+      expect(
+        issues.map((issue) => issue.sourceSpan.start.offset),
+        equals([132, 190, 341, 379, 607, 493, 677, 94]),
+      );
+      expect(
+        issues.map((issue) => issue.sourceSpan.start.line),
+        equals([7, 9, 18, 20, 29, 24, 34, 5]),
+      );
+      expect(
+        issues.map((issue) => issue.sourceSpan.start.column),
+        equals([20, 40, 20, 20, 28, 38, 20, 23]),
+      );
+      expect(
+        issues.map((issue) => issue.sourceSpan.end.offset),
+        equals([145, 209, 354, 397, 623, 506, 693, 107]),
+      );
+      expect(
+        issues.map((issue) => issue.sourceSpan.text),
+        equals([
+          'String string',
+          'String secondString',
+          'String string',
+          'String firstString',
+          'TestClass object',
+          'String string',
+          'TestClass object',
+          'String string',
+        ]),
+      );
+      expect(
+        issues.map((issue) => issue.message),
+        equals([
+          'Parameter is unused',
+          'Parameter is unused',
+          'Parameter is unused',
+          'Parameter is unused',
+          'Parameter is unused',
+          'Parameter is unused',
+          'Parameter is unused',
+          'Parameter is unused, consider renaming it to _, __, etc.',
+        ]),
+      );
+    });
+  });
+}