Skip to content

Commit

Permalink
Add avoid unused parameters rule (#49)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
2 people authored and vova-beloded-solid committed Nov 10, 2023
1 parent 39255dc commit c944ec1
Show file tree
Hide file tree
Showing 9 changed files with 369 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -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);
}
});
}
}
190 changes: 190 additions & 0 deletions lib/lints/avoid_unused_parameters/avoid_unused_parameters_visitor.dart
Original file line number Diff line number Diff line change
@@ -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<void> {
final _unusedParameters = <FormalParameter>[];

/// List of unused parameters
Iterable<FormalParameter> 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<FormalParameter> _getUnusedParameters(
AstNode body,
Iterable<FormalParameter> parameters,
) {
final result = <FormalParameter>{};
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<void> {
final elements = <Element>{};

@override
void visitSimpleIdentifier(SimpleIdentifier node) {
super.visitSimpleIdentifier(node);

final element = node.staticElement;
if (element != null) {
elements.add(element);
}
}
}

class _InvocationsVisitor extends RecursiveAstVisitor<void> {
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);
}
}
2 changes: 2 additions & 0 deletions lib/solid_lints.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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),
Expand Down
8 changes: 8 additions & 0 deletions lib/utils/node_utils.dart
Original file line number Diff line number Diff line change
@@ -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<Annotation> metadata) => metadata.any(
(node) =>
node.name.name == 'override' && node.atSign.type == TokenType.AT,
);
10 changes: 10 additions & 0 deletions lib/utils/parameter_utils.dart
Original file line number Diff line number Diff line change
@@ -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;
}
1 change: 1 addition & 0 deletions lint_test/analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
111 changes: 111 additions & 0 deletions lint_test/avoid_unused_parameters_test.dart
Original file line number Diff line number Diff line change
@@ -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();
}
}
1 change: 1 addition & 0 deletions lint_test/newline_before_return_test.dart
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
Loading

0 comments on commit c944ec1

Please sign in to comment.