diff --git a/lib/main.dart b/lib/main.dart index d9d338a8..96a02c48 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -14,6 +14,7 @@ import 'package:solid_lints/src/lints/cyclomatic_complexity/cyclomatic_complexit import 'package:solid_lints/src/lints/double_literal_format/double_literal_format_rule.dart'; import 'package:solid_lints/src/lints/double_literal_format/fixes/double_literal_format_fix.dart'; import 'package:solid_lints/src/lints/function_lines_of_code/function_lines_of_code_rule.dart'; +import 'package:solid_lints/src/lints/number_of_parameters/number_of_parameters_rule.dart'; import 'package:solid_lints/src/lints/prefer_first/fixes/prefer_first_fix.dart'; import 'package:solid_lints/src/lints/prefer_first/prefer_first_rule.dart'; import 'package:solid_lints/src/lints/proper_super_calls/proper_super_calls_rule.dart'; @@ -63,6 +64,9 @@ class SolidLintsPlugin extends Plugin { CyclomaticComplexityRule( analysisOptionsLoader: analysisLoader, ), + NumberOfParametersRule( + analysisOptionsLoader: analysisLoader, + ), UseNearestContextRule(), preferFirstRule, // TODO: Add more lint rules and use analysisLoader diff --git a/lib/src/lints/number_of_parameters/models/number_of_parameters_parameters.dart b/lib/src/lints/number_of_parameters/models/number_of_parameters_parameters.dart index 0ac631a4..2c0f0e89 100644 --- a/lib/src/lints/number_of_parameters/models/number_of_parameters_parameters.dart +++ b/lib/src/lints/number_of_parameters/models/number_of_parameters_parameters.dart @@ -17,6 +17,14 @@ class NumberOfParametersParameters { required this.exclude, }); + /// Empty [NumberOfParametersParameters] model. + factory NumberOfParametersParameters.empty() { + return NumberOfParametersParameters( + maxParameters: _defaultMaxParameters, + exclude: ExcludedIdentifiersListParameter(exclude: []), + ); + } + /// Method for creating from json data factory NumberOfParametersParameters.fromJson(Map json) => NumberOfParametersParameters( diff --git a/lib/src/lints/number_of_parameters/number_of_parameters_rule.dart b/lib/src/lints/number_of_parameters/number_of_parameters_rule.dart index c31bc78d..dbea292d 100644 --- a/lib/src/lints/number_of_parameters/number_of_parameters_rule.dart +++ b/lib/src/lints/number_of_parameters/number_of_parameters_rule.dart @@ -1,8 +1,8 @@ -import 'package:analyzer/dart/ast/ast.dart'; -import 'package:analyzer/error/listener.dart'; -import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/error/error.dart'; import 'package:solid_lints/src/lints/number_of_parameters/models/number_of_parameters_parameters.dart'; -import 'package:solid_lints/src/models/rule_config.dart'; +import 'package:solid_lints/src/lints/number_of_parameters/visitors/number_of_parameters_visitor.dart'; import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// A number of parameters metric which checks whether we didn't exceed @@ -14,10 +14,11 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// Assuming config: /// /// ```yaml -/// custom_lint: -/// rules: -/// - number_of_parameters: -/// max_parameters: 2 +/// plugins: +/// solid_lints: +/// diagnostics: +/// number_of_parameters: +/// max_parameters: 2 /// ``` /// /// #### BAD: @@ -37,50 +38,44 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// ``` class NumberOfParametersRule extends SolidLintRule { - /// This lint rule represents the error if number of - /// parameters reaches the maximum value. + /// The name of this lint rule. static const lintName = 'number_of_parameters'; - NumberOfParametersRule._(super.rule); + static const _code = LintCode( + lintName, + 'The maximum allowed number of parameters is {0}. ' + 'Try reducing the number of parameters.', + ); - /// Creates a new instance of [NumberOfParametersRule] - /// based on the lint configuration. - factory NumberOfParametersRule.createRule(CustomLintConfigs configs) { - final rule = RuleConfig( - configs: configs, - name: lintName, - paramsParser: NumberOfParametersParameters.fromJson, - problemMessage: (value) => - 'The maximum allowed number of parameters is ${value.maxParameters}. ' - 'Try reducing the number of parameters.', - ); + @override + DiagnosticCode get diagnosticCode => _code; - return NumberOfParametersRule._(rule); - } + /// Creates a new instance of [NumberOfParametersRule]. + NumberOfParametersRule({ + required super.analysisOptionsLoader, + }) : super.withParameters( + name: lintName, + description: + "Checks whether we didn't exceed the maximum allowed number " + 'of parameters for a function, method or constructor.', + parametersParser: NumberOfParametersParameters.fromJson, + ); @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, ) { - context.registry.addDeclaration((node) { - final isIgnored = config.parameters.exclude.shouldIgnore(node); - final parameters = switch (node) { - (final MethodDeclaration node) => - node.parameters?.parameters.length ?? 0, - (final FunctionDeclaration node) => - node.functionExpression.parameters?.parameters.length ?? 0, - _ => 0, - }; + super.registerNodeProcessors(registry, context); + + final parameters = + getParametersForContext(context) ?? + NumberOfParametersParameters.empty(); + + final visitor = NumberOfParametersVisitor(this, parameters); - if (!isIgnored && parameters > config.parameters.maxParameters) { - reporter.atOffset( - offset: node.firstTokenAfterCommentAndMetadata.offset, - length: node.end, - diagnosticCode: code, - ); - } - }); + registry.addFunctionDeclaration(this, visitor); + registry.addMethodDeclaration(this, visitor); + registry.addConstructorDeclaration(this, visitor); } } diff --git a/lib/src/lints/number_of_parameters/visitors/number_of_parameters_visitor.dart b/lib/src/lints/number_of_parameters/visitors/number_of_parameters_visitor.dart new file mode 100644 index 00000000..c898b798 --- /dev/null +++ b/lib/src/lints/number_of_parameters/visitors/number_of_parameters_visitor.dart @@ -0,0 +1,46 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:solid_lints/src/lints/number_of_parameters/models/number_of_parameters_parameters.dart'; +import 'package:solid_lints/src/lints/number_of_parameters/number_of_parameters_rule.dart'; +import 'package:solid_lints/src/utils/node_utils.dart'; + +/// A visitor that checks the number of parameters for functions, methods, and +/// constructors. +class NumberOfParametersVisitor extends SimpleAstVisitor { + final NumberOfParametersRule _rule; + final NumberOfParametersParameters _parameters; + + /// Creates a new instance of [NumberOfParametersVisitor]. + NumberOfParametersVisitor(this._rule, this._parameters); + + void _check(Declaration node, FormalParameterList? parameterList) { + if (parameterList == null) return; + + final isIgnored = _parameters.exclude.shouldIgnore(node); + if (isIgnored) return; + + final count = parameterList.parameters.length; + if (count > _parameters.maxParameters) { + _rule.reportAtNode( + parameterList, + arguments: [_parameters.maxParameters.toString()], + ); + } + } + + @override + void visitFunctionDeclaration(FunctionDeclaration node) { + _check(node, node.functionExpression.parameters); + } + + @override + void visitMethodDeclaration(MethodDeclaration node) { + if (isOverride(node.metadata)) return; + _check(node, node.parameters); + } + + @override + void visitConstructorDeclaration(ConstructorDeclaration node) { + _check(node, node.parameters); + } +} diff --git a/lint_test/number_of_parameters_copy_with_test/analysis_options.yaml b/lint_test/number_of_parameters_copy_with_test/analysis_options.yaml deleted file mode 100644 index 3fdd2d5e..00000000 --- a/lint_test/number_of_parameters_copy_with_test/analysis_options.yaml +++ /dev/null @@ -1 +0,0 @@ -include: ../../lib/analysis_options.yaml diff --git a/lint_test/number_of_parameters_copy_with_test/lib/src/number_of_parameters_copy_with_test.dart b/lint_test/number_of_parameters_copy_with_test/lib/src/number_of_parameters_copy_with_test.dart deleted file mode 100644 index 60a6f42f..00000000 --- a/lint_test/number_of_parameters_copy_with_test/lib/src/number_of_parameters_copy_with_test.dart +++ /dev/null @@ -1,90 +0,0 @@ -// ignore_for_file: prefer_match_file_name, public_member_api_docs -// Check number of parameters fail -/// -/// `number_of_parameters: max_parameters` - -class UserDto { - final String a; - final String b; - final String c; - final String d; - final String e; - final String f; - final String g; - final String h; - - const UserDto({ - required this.a, - required this.b, - required this.c, - required this.d, - required this.e, - required this.f, - required this.g, - required this.h, - }); - - /// Excluded by method_name - UserDto copyWith({ - String? a, - String? b, - String? c, - String? d, - String? e, - String? f, - String? g, - String? h, - }) { - return UserDto( - a: a ?? this.a, - b: b ?? this.b, - c: c ?? this.c, - d: d ?? this.d, - e: e ?? this.e, - f: f ?? this.f, - g: g ?? this.g, - h: h ?? this.h, - ); - } - - /// expect_lint: number_of_parameters - UserDto noCopyWith({ - String? a, - String? b, - String? c, - String? d, - String? e, - String? f, - String? g, - String? h, - }) { - return UserDto( - a: a ?? this.a, - b: b ?? this.b, - c: c ?? this.c, - d: d ?? this.d, - e: e ?? this.e, - f: f ?? this.f, - g: g ?? this.g, - h: h ?? this.h, - ); - } - - /// Allow - UserDto noLongCopyWith({ - String? a, - String? b, - String? c, - }) { - return UserDto( - a: a ?? this.a, - b: b ?? this.b, - c: c ?? this.c, - d: '', - e: '', - f: '', - g: '', - h: '', - ); - } -} diff --git a/lint_test/number_of_parameters_copy_with_test/pubspec.yaml b/lint_test/number_of_parameters_copy_with_test/pubspec.yaml deleted file mode 100644 index e753375a..00000000 --- a/lint_test/number_of_parameters_copy_with_test/pubspec.yaml +++ /dev/null @@ -1,15 +0,0 @@ -name: solid_lints_number_of_parameters_copy_with_test -description: A starting point for Dart libraries or applications. -publish_to: none - -environment: - sdk: '>=3.0.0 <4.0.0' - -dependencies: - flutter: - sdk: flutter - -dev_dependencies: - solid_lints: - path: ../../ - test: ^1.20.1 diff --git a/lint_test/number_of_parameters_test.dart b/lint_test/number_of_parameters_test.dart deleted file mode 100644 index 40f9b5b8..00000000 --- a/lint_test/number_of_parameters_test.dart +++ /dev/null @@ -1,20 +0,0 @@ -// ignore_for_file: prefer_match_file_name -// Check number of parameters fail -/// -/// `number_of_parameters: max_parameters` -/// expect_lint: number_of_parameters -String numberOfParameters(String a, String b, String c) { - return a + b + c; -} - -//no lint -String avoidNumberOfParameters(String a, String b, String c) { - return a + b + c; -} - -class Exclude { - //no lint - String avoidNumberOfParameters(String a, String b, String c) { - return a + b + c; - } -} diff --git a/test/src/lints/number_of_parameters/number_of_parameters_rule_test.dart b/test/src/lints/number_of_parameters/number_of_parameters_rule_test.dart new file mode 100644 index 00000000..04dd4197 --- /dev/null +++ b/test/src/lints/number_of_parameters/number_of_parameters_rule_test.dart @@ -0,0 +1,93 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:analyzer_testing/utilities/utilities.dart'; +import 'package:solid_lints/src/common/parameter_parser/analysis_options_loader.dart'; +import 'package:solid_lints/src/lints/number_of_parameters/number_of_parameters_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../../../lints/auto_test_lint_offsets.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(NumberOfParametersRuleTest); + }); +} + +@reflectiveTest +class NumberOfParametersRuleTest extends AnalysisRuleTest + with AutoTestLintOffsets { + static const _mockAnalysisOptionsContent = ''' +plugins: + solid_lints: + diagnostics: + number_of_parameters: + max_parameters: 2 + exclude: + - method_name: copyWith + - class_name: Base + method_name: myMethod + '''; + + @override + void setUp() { + rule = NumberOfParametersRule( + analysisOptionsLoader: AnalysisOptionsLoader( + resourceProvider: resourceProvider, + ), + ); + super.setUp(); + + newAnalysisOptionsYamlFile( + testPackageRootPath, + '''${analysisOptionsContent(rules: [rule.name])} +$_mockAnalysisOptionsContent''', + ); + } + + @override + String get analysisRule => NumberOfParametersRule.lintName; + + Future test_reports_on_exceeding_max_parameters() async { + await assertAutoDiagnostics(''' +void fun${expectLint(r'(int a, int b, int c)')} {} +'''); + } + + Future test_does_not_report_on_limit_or_below() async { + await assertNoDiagnostics(r''' +void fun(int a, int b) {} +void fun2(int a) {} +void fun3() {} +'''); + } + + Future test_does_not_report_on_excluded_copyWith() async { + await assertNoDiagnostics(r''' +class UserDto { + UserDto copyWith(int a, int b, int c) { + return UserDto(); + } +} +'''); + } + + Future test_reports_on_constructors_exceeding_max_parameters() async { + await assertAutoDiagnostics(''' +class Test { + Test${expectLint(r'(int a, int b, int c)')}; +} +'''); + } + + Future test_does_not_report_on_overridden_methods() async { + await assertNoDiagnostics(r''' +class Base { + void myMethod(int a, int b, int c) {} +} + +class Derived extends Base { + @override + void myMethod(int a, int b, int c) {} +} +'''); + } +}