From fef004537e232d9be57fce8c2b13e3e4038c2706 Mon Sep 17 00:00:00 2001 From: shaark Date: Thu, 5 Dec 2024 17:16:55 +0200 Subject: [PATCH] issue-93. added more test case --- ...consider_making_a_member_private_rule.dart | 8 ++ ...sider_making_a_member_private_visitor.dart | 111 ++++++++++++++---- ...id_debug_print_in_release_prefix_test.dart | 2 +- .../avoid_debug_print_in_release_test.dart | 2 +- ...void_unnecessary_type_assertions_test.dart | 2 +- .../avoid_unnecessary_type_casts_test.dart | 2 +- .../avoid_unrelated_type_assertions_test.dart | 2 +- .../external_source/lib/banned_library.dart | 1 + ...consider_making_a_member_private_test.dart | 54 ++++++++- lint_test/no_equal_then_else_test.dart | 2 +- ..._number_allowed_in_widget_params_test.dart | 2 +- lint_test/number_of_parameters_test.dart | 1 + ...tional_expressions_ignore_nested_test.dart | 2 +- .../prefer_conditional_expressions_test.dart | 2 +- lint_test/prefer_early_return_test.dart | 2 +- lint_test/prefer_first_test.dart | 1 + lint_test/prefer_last_test.dart | 1 + 17 files changed, 162 insertions(+), 35 deletions(-) diff --git a/lib/src/lints/consider_making_a_member_private/consider_making_a_member_private_rule.dart b/lib/src/lints/consider_making_a_member_private/consider_making_a_member_private_rule.dart index c8642457..323d4187 100644 --- a/lib/src/lints/consider_making_a_member_private/consider_making_a_member_private_rule.dart +++ b/lib/src/lints/consider_making_a_member_private/consider_making_a_member_private_rule.dart @@ -69,10 +69,18 @@ class ConsiderMakingAMemberPrivateRule extends SolidLintRule { node.visitChildren(visitor); final unusedMembers = visitor.unusedPublicMembers; + final unusedGlobalVariables = visitor.unusedGlobalVariables; + final unusedGlobalFunctions = visitor.unusedGlobalFunctions; for (final member in unusedMembers) { reporter.atNode(member, code); } + for (final variable in unusedGlobalVariables) { + reporter.atNode(variable, code); + } + for (final function in unusedGlobalFunctions) { + reporter.atNode(function, code); + } }); } } diff --git a/lib/src/lints/consider_making_a_member_private/visitor/consider_making_a_member_private_visitor.dart b/lib/src/lints/consider_making_a_member_private/visitor/consider_making_a_member_private_visitor.dart index c7710d01..509eba08 100644 --- a/lib/src/lints/consider_making_a_member_private/visitor/consider_making_a_member_private_visitor.dart +++ b/lib/src/lints/consider_making_a_member_private/visitor/consider_making_a_member_private_visitor.dart @@ -24,30 +24,48 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; -/// The AST visitor that will find lines with code. +/// AST Visitor for identifying unused public members, global functions, +/// and variables. class ConsiderMakingAMemberPrivateVisitor extends RecursiveAstVisitor { final CompilationUnit _node; final _unusedPublicMembers = {}; + final _unusedGlobalFunctions = {}; + final _unusedGlobalVariables = {}; - /// Returns public members that are not used as representatives of the class. + /// Returns unused public members of classes. Iterable get unusedPublicMembers => _unusedPublicMembers; - /// Creates a new instance of [ConsiderMakingAMemberPrivateVisitor]. + /// Returns unused global functions. + Iterable get unusedGlobalFunctions => + _unusedGlobalFunctions; + + /// Returns unused global variables. + Iterable get unusedGlobalVariables => + _unusedGlobalVariables; + + /// Constructor for [ConsiderMakingAMemberPrivateVisitor] ConsiderMakingAMemberPrivateVisitor(this._node); @override void visitClassDeclaration(ClassDeclaration node) { + // Check for unused public members in a class. final classMembers = node.members.where((member) { if (member is MethodDeclaration || member is FieldDeclaration) { final name = _getMemberName(member); return name != null && !_isPrivate(name); } + if (member is ConstructorDeclaration) { + final name = _getMemberName(member); + if (name == null) return true; + + return !_isPrivate(name); + } return false; }).toList(); for (final member in classMembers) { - final memberName = _getMemberName(member)!; + final memberName = _getMemberName(member); if (!_isUsedOutsideClass(memberName, node)) { _unusedPublicMembers.add(member); } @@ -56,7 +74,27 @@ class ConsiderMakingAMemberPrivateVisitor extends RecursiveAstVisitor { super.visitClassDeclaration(node); } - bool _isPrivate(String name) => name.startsWith('_'); + @override + void visitFunctionDeclaration(FunctionDeclaration node) { + final name = node.declaredElement?.name; + if (!_isPrivate(name) && !_isUsedEntity(name)) { + _unusedGlobalFunctions.add(node); + } + super.visitFunctionDeclaration(node); + } + + @override + void visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) { + for (final variable in node.variables.variables) { + final name = variable.declaredElement?.name; + if (!_isPrivate(name) && !_isUsedEntity(name)) { + _unusedGlobalVariables.add(variable); + } + } + super.visitTopLevelVariableDeclaration(node); + } + + bool _isPrivate(String? name) => name?.startsWith('_') ?? false; String? _getMemberName(ClassMember member) { if (member is MethodDeclaration) return member.declaredElement?.name; @@ -64,46 +102,77 @@ class ConsiderMakingAMemberPrivateVisitor extends RecursiveAstVisitor { final fields = member.fields.variables; return fields.isNotEmpty ? fields.first.declaredElement?.name : null; } + if (member is ConstructorDeclaration) { + return member.declaredElement?.name; + } return null; } - bool _isUsedOutsideClass(String memberName, ClassDeclaration classNode) { + bool _isUsedOutsideClass(String? memberName, ClassDeclaration classNode) { bool isUsedOutside = false; _node.visitChildren( - _PublicMemberUsageVisitor( - memberName: memberName, - classNode: classNode, + _GlobalEntityUsageVisitor( + entityName: memberName!, onUsageFound: () { isUsedOutside = true; }, ), ); + if (memberName.isEmpty) return true; + return isUsedOutside; } + + bool _isUsedEntity(String? entityName) { + bool isUsed = false; + + if (entityName == null) return false; + + _node.visitChildren( + _GlobalEntityUsageVisitor( + entityName: entityName, + onUsageFound: () { + isUsed = true; + }, + ), + ); + + return isUsed; + } } -class _PublicMemberUsageVisitor extends RecursiveAstVisitor { - final String memberName; - final ClassDeclaration classNode; +class _GlobalEntityUsageVisitor extends RecursiveAstVisitor { + final String entityName; final Function() onUsageFound; - _PublicMemberUsageVisitor({ - required this.memberName, - required this.classNode, + _GlobalEntityUsageVisitor({ + required this.entityName, required this.onUsageFound, }); @override void visitSimpleIdentifier(SimpleIdentifier identifier) { - if (identifier.name == memberName) { - final parentClass = identifier.thisOrAncestorOfType(); - if (parentClass != classNode) { - onUsageFound(); - } + if (identifier.name == entityName) { + onUsageFound(); } - super.visitSimpleIdentifier(identifier); } + + @override + void visitInstanceCreationExpression(InstanceCreationExpression node) { + if (node.constructorName.name?.name == entityName) { + onUsageFound(); + } + super.visitInstanceCreationExpression(node); + } + + @override + void visitMethodInvocation(MethodInvocation node) { + if (node.target != null && node.target.toString() == entityName) { + onUsageFound(); + } + super.visitMethodInvocation(node); + } } diff --git a/lint_test/avoid_debug_print_in_release_test/avoid_debug_print_in_release_prefix_test.dart b/lint_test/avoid_debug_print_in_release_test/avoid_debug_print_in_release_prefix_test.dart index 01b162aa..3a1d7d0a 100644 --- a/lint_test/avoid_debug_print_in_release_test/avoid_debug_print_in_release_prefix_test.dart +++ b/lint_test/avoid_debug_print_in_release_test/avoid_debug_print_in_release_prefix_test.dart @@ -1,4 +1,4 @@ -// ignore_for_file: unused_local_variable +// ignore_for_file: unused_local_variable, consider_making_a_member_private import 'package:flutter/foundation.dart' as f; diff --git a/lint_test/avoid_debug_print_in_release_test/avoid_debug_print_in_release_test.dart b/lint_test/avoid_debug_print_in_release_test/avoid_debug_print_in_release_test.dart index 9d6fa72a..b3a9f053 100644 --- a/lint_test/avoid_debug_print_in_release_test/avoid_debug_print_in_release_test.dart +++ b/lint_test/avoid_debug_print_in_release_test/avoid_debug_print_in_release_test.dart @@ -1,4 +1,4 @@ -// ignore_for_file: unused_local_variable +// ignore_for_file: unused_local_variable, consider_making_a_member_private import 'package:flutter/foundation.dart'; diff --git a/lint_test/avoid_unnecessary_type_assertions_test.dart b/lint_test/avoid_unnecessary_type_assertions_test.dart index b3f2f3c6..566592de 100644 --- a/lint_test/avoid_unnecessary_type_assertions_test.dart +++ b/lint_test/avoid_unnecessary_type_assertions_test.dart @@ -1,4 +1,4 @@ -// ignore_for_file: prefer_const_declarations, prefer_match_file_name, cyclomatic_complexity, unused_element +// ignore_for_file: prefer_const_declarations, prefer_match_file_name, cyclomatic_complexity, unused_element, consider_making_a_member_private // ignore_for_file: unnecessary_nullable_for_final_variable_declarations // ignore_for_file: unnecessary_type_check // ignore_for_file: unused_local_variable diff --git a/lint_test/avoid_unnecessary_type_casts_test.dart b/lint_test/avoid_unnecessary_type_casts_test.dart index dffa6903..97a2b512 100644 --- a/lint_test/avoid_unnecessary_type_casts_test.dart +++ b/lint_test/avoid_unnecessary_type_casts_test.dart @@ -1,4 +1,4 @@ -// ignore_for_file: prefer_const_declarations +// ignore_for_file: prefer_const_declarations, consider_making_a_member_private // ignore_for_file: unnecessary_nullable_for_final_variable_declarations // ignore_for_file: unnecessary_cast // ignore_for_file: unused_local_variable diff --git a/lint_test/avoid_unrelated_type_assertions_test.dart b/lint_test/avoid_unrelated_type_assertions_test.dart index 209a5615..5a0b0633 100644 --- a/lint_test/avoid_unrelated_type_assertions_test.dart +++ b/lint_test/avoid_unrelated_type_assertions_test.dart @@ -1,4 +1,4 @@ -// ignore_for_file: prefer_const_declarations, prefer_match_file_name, unused_element +// ignore_for_file: prefer_const_declarations, prefer_match_file_name, unused_element, consider_making_a_member_private // ignore_for_file: unnecessary_nullable_for_final_variable_declarations // ignore_for_file: unused_local_variable diff --git a/lint_test/avoid_using_api/external_source/lib/banned_library.dart b/lint_test/avoid_using_api/external_source/lib/banned_library.dart index abad4800..1978c36d 100644 --- a/lint_test/avoid_using_api/external_source/lib/banned_library.dart +++ b/lint_test/avoid_using_api/external_source/lib/banned_library.dart @@ -1,2 +1,3 @@ +//ignore_for_file:consider_making_a_member_private // expect_lint: avoid_global_state int banned = 5; diff --git a/lint_test/consider_making_a_member_private_test.dart b/lint_test/consider_making_a_member_private_test.dart index f49f122d..6d4ad5cf 100644 --- a/lint_test/consider_making_a_member_private_test.dart +++ b/lint_test/consider_making_a_member_private_test.dart @@ -1,19 +1,65 @@ -// ignore_for_file: no_empty_block, prefer_match_file_name, unused_element, member_ordering +// ignore_for_file: no_empty_block, prefer_match_file_name, unused_element, member_ordering, avoid_global_state, avoid_unused_parameters /// Check the `consider_making_a_member_private` rule /// +// expect_lint:consider_making_a_member_private +const int unusedGlobalVariables = 0; + +//no lint +const int usedGlobalVariables = 0; + +// expect_lint:consider_making_a_member_private +void unusedGLobalFunction() {} + +//no lint +void usedGLobalFunction() {} + class X { // expect_lint:consider_making_a_member_private void unusedX() {} -// no lint - void usedX() {} + // expect_lint:consider_making_a_member_private + final int unusedFinalX = 0; + // expect_lint:consider_making_a_member_private + static final int unusedStaticX = 0; + + // expect_lint:consider_making_a_member_private + int unusedMutableX = 0; + + // expect_lint:consider_making_a_member_private + int get unusedGetX => 0; + + // expect_lint:consider_making_a_member_private + set unusedSetX(int y) {} + + // no lint + void usedMethodX() {} + + // expect_lint:consider_making_a_member_private + static void unusedStaticMethodX() {} + + // no lint + X(); + + // expect_lint:consider_making_a_member_private + X.withValue({ + required this.unusedMutableX, + }); + + // expect_lint:consider_making_a_member_private + factory X.factory() => X(); + + // no lint + factory X.usedFactory() => X(); } class Y { // no lint void _y() { final x = X(); - x.usedX(); + X.usedFactory(); + x.usedMethodX(); + usedGLobalFunction(); + usedGlobalVariables; } } diff --git a/lint_test/no_equal_then_else_test.dart b/lint_test/no_equal_then_else_test.dart index c9caa40c..8870fbdc 100644 --- a/lint_test/no_equal_then_else_test.dart +++ b/lint_test/no_equal_then_else_test.dart @@ -1,4 +1,4 @@ -// ignore_for_file: unused_local_variable +// ignore_for_file: unused_local_variable, consider_making_a_member_private // ignore_for_file: cyclomatic_complexity // ignore_for_file: no_magic_number // ignore_for_file: prefer_conditional_expressions diff --git a/lint_test/no_magic_number_allowed_in_widget_params_test/no_magic_number_allowed_in_widget_params_test.dart b/lint_test/no_magic_number_allowed_in_widget_params_test/no_magic_number_allowed_in_widget_params_test.dart index 7d603d21..142cbe87 100644 --- a/lint_test/no_magic_number_allowed_in_widget_params_test/no_magic_number_allowed_in_widget_params_test.dart +++ b/lint_test/no_magic_number_allowed_in_widget_params_test/no_magic_number_allowed_in_widget_params_test.dart @@ -1,4 +1,4 @@ -// ignore_for_file: avoid_returning_widgets +// ignore_for_file: avoid_returning_widgets, consider_making_a_member_private // ignore_for_file: prefer_match_file_name // Allowed for numbers in a Widget subtype parameters. diff --git a/lint_test/number_of_parameters_test.dart b/lint_test/number_of_parameters_test.dart index 12fa6e21..fa2fa336 100644 --- a/lint_test/number_of_parameters_test.dart +++ b/lint_test/number_of_parameters_test.dart @@ -1,3 +1,4 @@ +// ignore_for_file: consider_making_a_member_private /// Check number of parameters fail /// /// `number_of_parameters: max_parameters` diff --git a/lint_test/prefer_conditional_expressions_ignore_nested_test/prefer_conditional_expressions_ignore_nested_test.dart b/lint_test/prefer_conditional_expressions_ignore_nested_test/prefer_conditional_expressions_ignore_nested_test.dart index 150b9e4b..cad19563 100644 --- a/lint_test/prefer_conditional_expressions_ignore_nested_test/prefer_conditional_expressions_ignore_nested_test.dart +++ b/lint_test/prefer_conditional_expressions_ignore_nested_test/prefer_conditional_expressions_ignore_nested_test.dart @@ -1,4 +1,4 @@ -// ignore_for_file: unused_local_variable +// ignore_for_file: unused_local_variable,consider_making_a_member_private // ignore_for_file: cyclomatic_complexity // ignore_for_file: no_equal_then_else diff --git a/lint_test/prefer_conditional_expressions_test.dart b/lint_test/prefer_conditional_expressions_test.dart index ee60f237..73c528d2 100644 --- a/lint_test/prefer_conditional_expressions_test.dart +++ b/lint_test/prefer_conditional_expressions_test.dart @@ -1,4 +1,4 @@ -// ignore_for_file: unused_local_variable +// ignore_for_file: unused_local_variable, consider_making_a_member_private // ignore_for_file: cyclomatic_complexity // ignore_for_file: no_equal_then_else // ignore_for_file: dead_code diff --git a/lint_test/prefer_early_return_test.dart b/lint_test/prefer_early_return_test.dart index d9108186..4446901b 100644 --- a/lint_test/prefer_early_return_test.dart +++ b/lint_test/prefer_early_return_test.dart @@ -1,4 +1,4 @@ -// ignore_for_file: dead_code, cyclomatic_complexity, no_equal_then_else, prefer_match_file_name +// ignore_for_file: dead_code, cyclomatic_complexity, no_equal_then_else, prefer_match_file_name, consider_making_a_member_private // ignore: no_empty_block void _doSomething() {} diff --git a/lint_test/prefer_first_test.dart b/lint_test/prefer_first_test.dart index 2527d828..8801d6fb 100644 --- a/lint_test/prefer_first_test.dart +++ b/lint_test/prefer_first_test.dart @@ -1,3 +1,4 @@ +// ignore_for_file: consider_making_a_member_private /// Check the `prefer_first` rule void fun() { const zero = 0; diff --git a/lint_test/prefer_last_test.dart b/lint_test/prefer_last_test.dart index 4b87476f..d9e65969 100644 --- a/lint_test/prefer_last_test.dart +++ b/lint_test/prefer_last_test.dart @@ -1,3 +1,4 @@ +// ignore_for_file: consider_making_a_member_private /// Check the `prefer_first` rule void fun() { final list = [0, 1, 2, 3];