From c944ec1b0070cb17906600b4c7747e93595c1781 Mon Sep 17 00:00:00 2001 From: DenisBogatirov Date: Fri, 15 Sep 2023 19:50:19 +0300 Subject: [PATCH] Add avoid unused parameters rule (#49) * Add avoid-unused-parameters rule * Add tests for avoid-unused-parameters rule * Fix function name, remove null check operator and remove negation * Fix multiple reporting * Add more test cases * Fix formatting * Rename method * Add constructors and factories unused params handling * Fix constructors with named field parameters * Simplify rule run method * Fix tests * Fix tests after merge * Fix tests after merge --------- Co-authored-by: Denis Bogatirov --- .../avoid_unused_parameters_rule.dart | 45 +++++ .../avoid_unused_parameters_visitor.dart | 190 ++++++++++++++++++ lib/solid_lints.dart | 2 + lib/utils/node_utils.dart | 8 + lib/utils/parameter_utils.dart | 10 + lint_test/analysis_options.yaml | 1 + lint_test/avoid_unused_parameters_test.dart | 111 ++++++++++ lint_test/newline_before_return_test.dart | 1 + lint_test/no_empty_block_test.dart | 1 + 9 files changed, 369 insertions(+) create mode 100644 lib/lints/avoid_unused_parameters/avoid_unused_parameters_rule.dart create mode 100644 lib/lints/avoid_unused_parameters/avoid_unused_parameters_visitor.dart create mode 100644 lib/utils/node_utils.dart create mode 100644 lib/utils/parameter_utils.dart create mode 100644 lint_test/avoid_unused_parameters_test.dart diff --git a/lib/lints/avoid_unused_parameters/avoid_unused_parameters_rule.dart b/lib/lints/avoid_unused_parameters/avoid_unused_parameters_rule.dart new file mode 100644 index 00000000..5c608b28 --- /dev/null +++ b/lib/lints/avoid_unused_parameters/avoid_unused_parameters_rule.dart @@ -0,0 +1,45 @@ +import 'package:analyzer/error/listener.dart'; +import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:solid_lints/lints/avoid_unused_parameters/avoid_unused_parameters_visitor.dart'; +import 'package:solid_lints/models/rule_config.dart'; +import 'package:solid_lints/models/solid_lint_rule.dart'; + +/// A `avoid-unused-parameters` rule which +/// warns about unused parameters +class AvoidUnusedParametersRule extends SolidLintRule { + /// The [LintCode] of this lint rule that represents + /// the error whether we use bad formatted double literals. + static const String lintName = 'avoid-unused-parameters'; + + AvoidUnusedParametersRule._(super.config); + + /// Creates a new instance of [AvoidUnusedParametersRule] + /// based on the lint configuration. + factory AvoidUnusedParametersRule.createRule( + CustomLintConfigs configs, + ) { + final rule = RuleConfig( + configs: configs, + name: lintName, + problemMessage: (_) => 'Parameter is unused.', + ); + + return AvoidUnusedParametersRule._(rule); + } + + @override + void run( + CustomLintResolver resolver, + ErrorReporter reporter, + CustomLintContext context, + ) { + context.registry.addCompilationUnit((node) { + final visitor = AvoidUnusedParametersVisitor(); + node.accept(visitor); + + for (final element in visitor.unusedParameters) { + reporter.reportErrorForNode(code, element); + } + }); + } +} diff --git a/lib/lints/avoid_unused_parameters/avoid_unused_parameters_visitor.dart b/lib/lints/avoid_unused_parameters/avoid_unused_parameters_visitor.dart new file mode 100644 index 00000000..21760e13 --- /dev/null +++ b/lib/lints/avoid_unused_parameters/avoid_unused_parameters_visitor.dart @@ -0,0 +1,190 @@ +// MIT License +// +// Copyright (c) 2020-2021 Dart Code Checker team +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +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 'package:solid_lints/utils/node_utils.dart'; +import 'package:solid_lints/utils/parameter_utils.dart'; + +/// AST Visitor which finds all is expressions and checks if they are +/// unrelated (result always false) +class AvoidUnusedParametersVisitor extends RecursiveAstVisitor { + final _unusedParameters = []; + + /// List of unused parameters + Iterable get unusedParameters => _unusedParameters; + + @override + void visitConstructorDeclaration(ConstructorDeclaration node) { + super.visitConstructorDeclaration(node); + + final parent = node.parent; + final parameters = node.parameters; + + if (parent is ClassDeclaration && parent.abstractKeyword != null || + node.externalKeyword != null || + parameters.parameters.isEmpty) { + return; + } + + _unusedParameters.addAll( + _getUnusedParameters( + node.body, + parameters.parameters, + ).whereNot(nameConsistsOfUnderscoresOnly), + ); + } + + @override + void visitMethodDeclaration(MethodDeclaration node) { + super.visitMethodDeclaration(node); + + final parent = node.parent; + final parameters = node.parameters; + + if (parent is ClassDeclaration && parent.abstractKeyword != null || + node.isAbstract || + node.externalKeyword != null || + (parameters == null || parameters.parameters.isEmpty)) { + return; + } + + final isTearOff = _usedAsTearOff(node); + + if (!isOverride(node.metadata) && !isTearOff) { + _unusedParameters.addAll( + _getUnusedParameters( + node.body, + parameters.parameters, + ).whereNot(nameConsistsOfUnderscoresOnly), + ); + } + } + + @override + void visitFunctionDeclaration(FunctionDeclaration node) { + super.visitFunctionDeclaration(node); + + final parameters = node.functionExpression.parameters; + + if (node.externalKeyword != null || + (parameters == null || parameters.parameters.isEmpty)) { + return; + } + + _unusedParameters.addAll( + _getUnusedParameters( + node.functionExpression.body, + parameters.parameters, + ).whereNot(nameConsistsOfUnderscoresOnly), + ); + } + + Set _getUnusedParameters( + AstNode body, + Iterable parameters, + ) { + final result = {}; + final visitor = _IdentifiersVisitor(); + body.visitChildren(visitor); + + final allIdentifierElements = visitor.elements; + + for (final parameter in parameters) { + final name = parameter.name; + final isPresentInAll = allIdentifierElements.contains( + parameter.declaredElement, + ); + + /// Variables declared and initialized as 'Foo(this.param)' + bool isFieldFormalParameter = parameter is FieldFormalParameter; + + /// Variables declared and initialized as 'Foo(super.param)' + bool isSuperFormalParameter = parameter is SuperFormalParameter; + + if (parameter is DefaultFormalParameter) { + /// Variables as 'Foo({super.param})' or 'Foo({this.param})' + /// is being reported as [DefaultFormalParameter] instead + /// of [SuperFormalParameter] it seems to be an issue in DartSDK + isFieldFormalParameter = parameter.toSource().contains('this.'); + isSuperFormalParameter = parameter.toSource().contains('super.'); + } + + if (name != null && + !isPresentInAll && + !isFieldFormalParameter && + !isSuperFormalParameter) { + result.add(parameter); + } + } + + return result; + } + + bool _usedAsTearOff(MethodDeclaration node) { + final name = node.name.lexeme; + if (!Identifier.isPrivateName(name)) { + return false; + } + + final visitor = _InvocationsVisitor(name); + node.root.visitChildren(visitor); + + return visitor.hasTearOffInvocations; + } +} + +class _IdentifiersVisitor extends RecursiveAstVisitor { + final elements = {}; + + @override + void visitSimpleIdentifier(SimpleIdentifier node) { + super.visitSimpleIdentifier(node); + + final element = node.staticElement; + if (element != null) { + elements.add(element); + } + } +} + +class _InvocationsVisitor extends RecursiveAstVisitor { + final String methodName; + + bool hasTearOffInvocations = false; + + _InvocationsVisitor(this.methodName); + + @override + void visitSimpleIdentifier(SimpleIdentifier node) { + if (node.name == methodName && + node.staticElement is MethodElement && + node.parent is ArgumentList) { + hasTearOffInvocations = true; + } + + super.visitSimpleIdentifier(node); + } +} diff --git a/lib/solid_lints.dart b/lib/solid_lints.dart index 7c05fee6..aa460093 100644 --- a/lib/solid_lints.dart +++ b/lib/solid_lints.dart @@ -9,6 +9,7 @@ import 'package:solid_lints/lints/avoid_unnecessary_setstate/avoid_unnecessary_s import 'package:solid_lints/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart'; import 'package:solid_lints/lints/avoid_unnecessary_type_casts/avoid_unnecessary_type_casts_rule.dart'; import 'package:solid_lints/lints/avoid_unrelated_type_assertions/avoid_unrelated_type_assertions_rule.dart'; +import 'package:solid_lints/lints/avoid_unused_parameters/avoid_unused_parameters_rule.dart'; import 'package:solid_lints/lints/cyclomatic_complexity/cyclomatic_complexity_metric.dart'; import 'package:solid_lints/lints/double_literal_format/double_literal_format_rule.dart'; import 'package:solid_lints/lints/function_lines_of_code/function_lines_of_code_metric.dart'; @@ -39,6 +40,7 @@ class _SolidLints extends PluginBase { AvoidUnnecessarySetStateRule.createRule(configs), AvoidUnnecessaryTypeCastsRule.createRule(configs), AvoidUnrelatedTypeAssertionsRule.createRule(configs), + AvoidUnusedParametersRule.createRule(configs), NewlineBeforeReturnRule.createRule(configs), NoEmptyBlockRule.createRule(configs), NoEqualThenElseRule.createRule(configs), diff --git a/lib/utils/node_utils.dart b/lib/utils/node_utils.dart new file mode 100644 index 00000000..72589f94 --- /dev/null +++ b/lib/utils/node_utils.dart @@ -0,0 +1,8 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; + +/// Check node is override method from its metadata +bool isOverride(List metadata) => metadata.any( + (node) => + node.name.name == 'override' && node.atSign.type == TokenType.AT, + ); diff --git a/lib/utils/parameter_utils.dart b/lib/utils/parameter_utils.dart new file mode 100644 index 00000000..2fc20b8e --- /dev/null +++ b/lib/utils/parameter_utils.dart @@ -0,0 +1,10 @@ +import 'package:analyzer/dart/ast/ast.dart'; + +/// Checks if parameter name consists only of underscores +bool nameConsistsOfUnderscoresOnly(FormalParameter parameter) { + final paramName = parameter.name; + + if (paramName == null) return false; + + return paramName.lexeme.replaceAll('_', '').isEmpty; +} diff --git a/lint_test/analysis_options.yaml b/lint_test/analysis_options.yaml index 7704fc95..d7666bbf 100644 --- a/lint_test/analysis_options.yaml +++ b/lint_test/analysis_options.yaml @@ -19,6 +19,7 @@ custom_lint: - avoid-unnecessary-type-assertions - avoid-unnecessary-type-casts - avoid-unrelated-type-assertions + - avoid-unused-parameters - newline-before-return - no-empty-block - no-equal-then-else diff --git a/lint_test/avoid_unused_parameters_test.dart b/lint_test/avoid_unused_parameters_test.dart new file mode 100644 index 00000000..15c20b51 --- /dev/null +++ b/lint_test/avoid_unused_parameters_test.dart @@ -0,0 +1,111 @@ +// ignore_for_file: prefer_const_declarations +// ignore_for_file: unnecessary_nullable_for_final_variable_declarations +// ignore_for_file: unused_local_variable +// ignore_for_file: unused_element +// ignore_for_file: newline-before-return +// ignore_for_file: no-empty-block +// ignore_for_file: member-ordering + +import 'package:flutter/material.dart'; + +/// Check the `avoid-unused-parameters` rule + +// expect_lint: avoid-unused-parameters +void fun(String s) { + return; +} + +class TestClass { + // expect_lint: avoid-unused-parameters + static void staticMethod(int a) {} + + // expect_lint: avoid-unused-parameters + void method(String s) { + return; + } + + void methodWithUnderscores(int _) {} +} + +class TestClass2 { + void method(String _) { + return; + } +} + +class SomeOtherClass { + // expect_lint: avoid-unused-parameters + void method(String s) { + return; + } +} + +class SomeAnotherClass extends SomeOtherClass { + @override + void method(String s) {} +} + +void someOtherFunction(String s) { + print(s); + return; +} + +class SomeOtherAnotherClass { + void method(String s) { + print(s); + return; + } + + // expect_lint: avoid-unused-parameters + void anonymousCallback(Function(int a) cb) {} +} + +// expect_lint: avoid-unused-parameters +void closure(int a) { + void internal(int a) { + print(a); + } +} + +typedef MaxFun = int Function(int a, int b); + +// Allowed same way as override +final MaxFun maxFunInstance = (int a, int b) => 1; + +class Foo { + final int a; + final int? b; + + Foo._(this.a, this.b); + + Foo.name(this.a, this.b); + + Foo.coolName({required this.a, required this.b}); + + // expect_lint: avoid-unused-parameters + Foo.another({required int c}) + : a = 1, + b = 0; + + // expect_lint: avoid-unused-parameters + factory Foo.aOnly(int a) { + return Foo._(1, null); + } +} + +class Bar extends Foo { + Bar.name(super.a, super.b) : super.name(); +} + +class TestWidget extends StatelessWidget { + const TestWidget({ + super.key, + // expect_lint: avoid-unused-parameters + int a = 1, + }); + + @override + Widget build(BuildContext context) { + return const Placeholder(); + } +} diff --git a/lint_test/newline_before_return_test.dart b/lint_test/newline_before_return_test.dart index 3e061583..382f1326 100644 --- a/lint_test/newline_before_return_test.dart +++ b/lint_test/newline_before_return_test.dart @@ -1,5 +1,6 @@ // ignore_for_file: unused_local_variable // ignore_for_file: member-ordering +// ignore_for_file: avoid-unused-parameters /// Check the `newline-before-return` rule class Foo { diff --git a/lint_test/no_empty_block_test.dart b/lint_test/no_empty_block_test.dart index 51e3f6b9..014ad2f4 100644 --- a/lint_test/no_empty_block_test.dart +++ b/lint_test/no_empty_block_test.dart @@ -1,6 +1,7 @@ // ignore_for_file: prefer_const_declarations // ignore_for_file: unused_local_variable // ignore_for_file: cyclomatic_complexity +// ignore_for_file: avoid-unused-parameters /// Check the `no-empty-block` rule