diff --git a/CHANGELOG.md b/CHANGELOG.md index 94224d91..0c761195 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 0.3.2 + +- Added `named_parameters_ordering` rule. + ## 0.3.1 - Added `allow_with_comments` parameter for `no_empty_block` lint. diff --git a/lib/solid_lints.dart b/lib/solid_lints.dart index 0ddfd587..b2a24a9d 100644 --- a/lib/solid_lints.dart +++ b/lib/solid_lints.dart @@ -17,6 +17,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/function_lines_of_code/function_lines_of_code_rule.dart'; import 'package:solid_lints/src/lints/member_ordering/member_ordering_rule.dart'; +import 'package:solid_lints/src/lints/named_parameters_ordering/named_parameters_ordering_rule.dart'; import 'package:solid_lints/src/lints/newline_before_return/newline_before_return_rule.dart'; import 'package:solid_lints/src/lints/no_empty_block/no_empty_block_rule.dart'; import 'package:solid_lints/src/lints/no_equal_then_else/no_equal_then_else_rule.dart'; @@ -65,6 +66,7 @@ class _SolidLints extends PluginBase { AvoidDebugPrintInReleaseRule.createRule(configs), PreferEarlyReturnRule.createRule(configs), AvoidFinalWithGetterRule.createRule(configs), + NamedParametersOrderingRule.createRule(configs), ]; // Return only enabled rules diff --git a/lib/src/lints/named_parameters_ordering/config_parser.dart b/lib/src/lints/named_parameters_ordering/config_parser.dart new file mode 100644 index 00000000..5c5f9570 --- /dev/null +++ b/lib/src/lints/named_parameters_ordering/config_parser.dart @@ -0,0 +1,44 @@ +// 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:solid_lints/src/lints/named_parameters_ordering/models/parameter_type.dart'; + +/// Helper class to parse member_ordering rule config +class NamedParametersConfigParser { + static const _defaultOrderList = [ + 'required_super', + 'super', + 'required', + 'nullable', + 'default', + ]; + + /// Parse rule config for regular class order rules + static List parseOrder(Object? orderConfig) { + final order = orderConfig is Iterable + ? List.from(orderConfig) + : _defaultOrderList; + + return order.map(ParameterType.fromType).nonNulls.toList(); + } +} diff --git a/lib/src/lints/named_parameters_ordering/models/named_parameters_ordering_parameters.dart b/lib/src/lints/named_parameters_ordering/models/named_parameters_ordering_parameters.dart new file mode 100644 index 00000000..7ff0009f --- /dev/null +++ b/lib/src/lints/named_parameters_ordering/models/named_parameters_ordering_parameters.dart @@ -0,0 +1,48 @@ +// 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:solid_lints/src/lints/named_parameters_ordering/config_parser.dart'; +import 'package:solid_lints/src/lints/named_parameters_ordering/models/parameter_type.dart'; + +/// A data model class that represents the parameter ordering input +/// parameters. +class NamedParametersOrderingParameters { + /// Config keys + static const _orderConfig = 'order'; + + /// Config used for order of named parameters + final List order; + + /// Constructor for [NamedParametersOrderingParameters] model + const NamedParametersOrderingParameters({required this.order}); + + /// Method for creating from json data + factory NamedParametersOrderingParameters.fromJson( + Map json, + ) => + NamedParametersOrderingParameters( + order: NamedParametersConfigParser.parseOrder( + json[_orderConfig], + ), + ); +} diff --git a/lib/src/lints/named_parameters_ordering/models/parameter_info.dart b/lib/src/lints/named_parameters_ordering/models/parameter_info.dart new file mode 100644 index 00000000..3aa7d398 --- /dev/null +++ b/lib/src/lints/named_parameters_ordering/models/parameter_info.dart @@ -0,0 +1,17 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:solid_lints/src/lints/named_parameters_ordering/models/parameter_ordering_info.dart'; + +/// Data class that holds AST function parameter and it's order info +class ParameterInfo { + /// AST instance of an [FormalParameter] + final FormalParameter formalParameter; + + /// Function parameter order info + final ParameterOrderingInfo parameterOrderingInfo; + + /// Creates instance of an [ParameterInfo] + const ParameterInfo({ + required this.formalParameter, + required this.parameterOrderingInfo, + }); +} diff --git a/lib/src/lints/named_parameters_ordering/models/parameter_ordering_info.dart b/lib/src/lints/named_parameters_ordering/models/parameter_ordering_info.dart new file mode 100644 index 00000000..d9b8efa5 --- /dev/null +++ b/lib/src/lints/named_parameters_ordering/models/parameter_ordering_info.dart @@ -0,0 +1,20 @@ +import 'package:solid_lints/src/lints/named_parameters_ordering/models/parameter_type.dart'; + +/// Data class holds information about parameter order info +class ParameterOrderingInfo { + /// Indicates if order is wrong + final bool isWrong; + + /// Info about current parameter parameter type + final ParameterType parameterType; + + /// Info about previous parameter parameter type + final ParameterType? previousParameterType; + + /// Creates instance of [ParameterOrderingInfo] + const ParameterOrderingInfo({ + required this.isWrong, + required this.parameterType, + required this.previousParameterType, + }); +} diff --git a/lib/src/lints/named_parameters_ordering/models/parameter_type.dart b/lib/src/lints/named_parameters_ordering/models/parameter_type.dart new file mode 100644 index 00000000..d08901a2 --- /dev/null +++ b/lib/src/lints/named_parameters_ordering/models/parameter_type.dart @@ -0,0 +1,32 @@ +import 'package:collection/collection.dart'; + +/// Represents a function parameter type +enum ParameterType { + /// Inherited (super) parameter type (super.parameterName) + inherited('super'), + + /// Required inherited (super) parameter type (required super.parameterName) + requiredInherited('required_super'), + + /// Required parameter type (required String parameterName) + required('required'), + + /// Nullable parameter type (String? parameterName) + nullable('nullable'), + + /// Default value parameter type (String parameterName = 'defaultValue') + defaultValue('default'); + + /// Returns [ParameterType] from type or null if not found + static ParameterType? fromType(String type) { + return values.firstWhereOrNull((o) => o.type == type); + } + + /// String representation of the parameter type + final String type; + + /// Display name of the parameter type + String get displayName => type.replaceAll('_', ' '); + + const ParameterType(this.type); +} diff --git a/lib/src/lints/named_parameters_ordering/named_parameters_ordering_rule.dart b/lib/src/lints/named_parameters_ordering/named_parameters_ordering_rule.dart new file mode 100644 index 00000000..013c19cd --- /dev/null +++ b/lib/src/lints/named_parameters_ordering/named_parameters_ordering_rule.dart @@ -0,0 +1,158 @@ +import 'package:analyzer/error/listener.dart'; +import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:solid_lints/src/lints/named_parameters_ordering/models/named_parameters_ordering_parameters.dart'; +import 'package:solid_lints/src/lints/named_parameters_ordering/models/parameter_ordering_info.dart'; +import 'package:solid_lints/src/lints/named_parameters_ordering/visitors/named_parameters_ordering_visitor.dart'; +import 'package:solid_lints/src/models/rule_config.dart'; +import 'package:solid_lints/src/models/solid_lint_rule.dart'; + +/// A lint which allows to enforce a particular named parameter ordering +/// conventions. +/// +/// ### Configuration format +/// ```yaml +/// - named_parameters_ordering: +/// order: +/// - (parameterType) +/// ``` +/// Where parameterType can be one of: +/// +/// - `super` +/// - `required_super` +/// - `required` +/// - `nullable` +/// - `default` +/// +/// ### Example: +/// +/// Assuming config: +/// +/// ```yaml +/// custom_lint: +/// rules: +/// - named_parameters_ordering: +/// order: +/// - required +/// - required_super +/// - default +/// - nullable +/// - super +/// ``` +/// +/// #### BAD: +/// +/// ```dart +/// class UserProfile extends User { +/// final String? age; +/// final String? country; +/// final String email; +/// final bool isActive; +/// final String name; +/// +/// UserProfile({ +/// this.age, +/// required super.accountType, // LINT, required super named parameters should be before nullable named parameters +/// required this.name, // LINT, required named parameters should be before super named parameters +/// super.userId, +/// this.country, // LINT, nullable named parameters should be before super named parameters +/// this.isActive = true, // LINT, default named parameters should be before nullable named parameters +/// required this.email, // LINT, required named parameters should be before default named parameters +/// }); +/// +/// void doSomething({ +/// required String name, +/// int? age, +/// bool isActive = true, // LINT, default named parameters should be before nullable named parameters +/// required String email, // LINT, required named parameters should be before default named parameters +/// }) { +/// return; +/// } +/// } +/// ``` +/// +/// #### GOOD: +/// +/// ```dart +/// class UserProfile extends User { +/// final String? age; +/// final String? country; +/// final String email; +/// final bool isActive; +/// final String name; +/// +/// UserProfile({ +/// required this.name, +/// required this.email, +/// required super.accountType, +/// this.isActive = true, +/// this.age, +/// this.country, +/// super.userId, +/// }); +/// +/// void doSomething({ +/// required String name, +/// required String email, +/// bool isActive = true, +/// int? age, +/// }) { +/// return; +/// } +/// } +/// ``` +class NamedParametersOrderingRule + extends SolidLintRule { + /// The name of this lint rule. + static const lintName = 'named_parameters_ordering'; + + late final _visitor = NamedParametersOrderingVisitor(config.parameters.order); + + NamedParametersOrderingRule._(super.config); + + /// Creates a new instance of [NamedParametersOrderingRule] + /// based on the lint configuration. + factory NamedParametersOrderingRule.createRule(CustomLintConfigs configs) { + final config = RuleConfig( + configs: configs, + name: lintName, + paramsParser: NamedParametersOrderingParameters.fromJson, + problemMessage: (_) => "Order of named parameter is wrong", + ); + + return NamedParametersOrderingRule._(config); + } + + @override + void run( + CustomLintResolver resolver, + ErrorReporter reporter, + CustomLintContext context, + ) { + context.registry.addFormalParameterList((node) { + final parametersInfo = _visitor.visitFormalParameterList(node); + + final wrongOrderParameters = parametersInfo.where( + (info) => info.parameterOrderingInfo.isWrong, + ); + + for (final parameterInfo in wrongOrderParameters) { + reporter.atNode( + parameterInfo.formalParameter, + _createWrongOrderLintCode(parameterInfo.parameterOrderingInfo), + ); + } + }); + } + + LintCode _createWrongOrderLintCode(ParameterOrderingInfo info) { + final parameterOrdering = info.parameterType; + final previousParameterOrdering = info.previousParameterType; + + return LintCode( + name: lintName, + problemMessage: "${parameterOrdering.displayName} named parameters" + " should be before " + "${previousParameterOrdering!.displayName} named parameters.", + ); + } +} diff --git a/lib/src/lints/named_parameters_ordering/visitors/named_parameters_ordering_visitor.dart b/lib/src/lints/named_parameters_ordering/visitors/named_parameters_ordering_visitor.dart new file mode 100644 index 00000000..4ec4598c --- /dev/null +++ b/lib/src/lints/named_parameters_ordering/visitors/named_parameters_ordering_visitor.dart @@ -0,0 +1,116 @@ +// 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' hide Annotation; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:solid_lints/src/lints/named_parameters_ordering/models/parameter_info.dart'; +import 'package:solid_lints/src/lints/named_parameters_ordering/models/parameter_ordering_info.dart'; +import 'package:solid_lints/src/lints/named_parameters_ordering/models/parameter_type.dart'; + +/// AST Visitor which finds all methods, functions and constructor named +/// parameters and checks if they are in order provided from rule config +/// or default config +class NamedParametersOrderingVisitor + extends RecursiveAstVisitor> { + final List _parametersOrder; + + final List _parametersInfo = []; + + /// Creates instance of [NamedParametersOrderingVisitor] + NamedParametersOrderingVisitor(this._parametersOrder); + + @override + List visitFormalParameterList(FormalParameterList node) { + super.visitFormalParameterList(node); + + _parametersInfo.clear(); + + final namedParametersList = + node.parameters.where((p) => p.isNamed).toList(); + + if (namedParametersList.isEmpty) { + return _parametersInfo; + } + + for (final parameter in namedParametersList) { + _parametersInfo.add( + ParameterInfo( + formalParameter: parameter, + parameterOrderingInfo: _getParameterOrderingInfo(parameter), + ), + ); + } + + return _parametersInfo; + } + + ParameterOrderingInfo _getParameterOrderingInfo(FormalParameter parameter) { + final parameterType = _getParameterType(parameter); + final previousParameterType = + _parametersInfo.lastOrNull?.parameterOrderingInfo.parameterType; + + return ParameterOrderingInfo( + isWrong: _isOrderingWrong(parameterType, previousParameterType), + parameterType: parameterType, + previousParameterType: previousParameterType, + ); + } + + ParameterType _getParameterType( + FormalParameter parameter, [ + bool hasDefaultValue = false, + ]) { + if (parameter is DefaultFormalParameter && + parameter.parameter is! DefaultFormalParameter) { + return _getParameterType( + parameter.parameter, + parameter.defaultValue != null, + ); + } + + switch (parameter) { + case SuperFormalParameter(:final isRequired): + return isRequired + ? ParameterType.requiredInherited + : ParameterType.inherited; + + case DefaultFormalParameter(): + case _ when hasDefaultValue: + return ParameterType.defaultValue; + + case FieldFormalParameter(:final isRequired) || + FunctionTypedFormalParameter(:final isRequired) || + SimpleFormalParameter(:final isRequired): + return isRequired ? ParameterType.required : ParameterType.nullable; + } + } + + bool _isOrderingWrong( + ParameterType currentParameterType, + ParameterType? previousParameterType, + ) { + return previousParameterType != null && + _parametersOrder.indexOf(previousParameterType) > + _parametersOrder.indexOf(currentParameterType); + } +} diff --git a/lint_test/analysis_options.yaml b/lint_test/analysis_options.yaml index 94fdbefb..45e4b7ef 100644 --- a/lint_test/analysis_options.yaml +++ b/lint_test/analysis_options.yaml @@ -82,3 +82,10 @@ custom_lint: - prefer_match_file_name - proper_super_calls - avoid_final_with_getter + - named_parameters_ordering: + order: + - required_super + - super + - required + - nullable + - default diff --git a/lint_test/avoid_unused_parameters_test.dart b/lint_test/avoid_unused_parameters_test.dart index b4eb7bd3..075269a6 100644 --- a/lint_test/avoid_unused_parameters_test.dart +++ b/lint_test/avoid_unused_parameters_test.dart @@ -1,4 +1,4 @@ -// ignore_for_file: prefer_const_declarations, prefer_match_file_name, avoid_global_state +// ignore_for_file: prefer_const_declarations, prefer_match_file_name, avoid_global_state, named_parameters_ordering // ignore_for_file: unnecessary_nullable_for_final_variable_declarations // ignore_for_file: unused_local_variable // ignore_for_file: unused_element diff --git a/lint_test/named_parameters_ordering_test.dart b/lint_test/named_parameters_ordering_test.dart new file mode 100644 index 00000000..3ea72264 --- /dev/null +++ b/lint_test/named_parameters_ordering_test.dart @@ -0,0 +1,133 @@ +// ignore_for_file: unused_field, prefer_match_file_name, proper_super_calls, number_of_parameters, avoid_unused_parameters, member_ordering +// ignore_for_file: unused_element +// ignore_for_file: no_empty_block + +/// Check the `named_parameters_ordering` rule + +class User { + final String accountType; + final String? userId; + + User({ + this.userId, + // expect_lint: named_parameters_ordering + required this.accountType, + }); +} + +class UserProfile extends User { + final String? age; + final String? country; + final String email; + final bool isActive; + final String name; + final String? profileId; + + // no lint + UserProfile.orderedConstructor( + this.profileId, { + required super.accountType, + super.userId, + required this.name, + required this.email, + this.age, + this.country, + this.isActive = true, + }); + + UserProfile.partiallyOrderedConstructor( + this.profileId, { + required super.accountType, + required this.email, + this.age, + // expect_lint: named_parameters_ordering + required this.name, + this.country, + // expect_lint: named_parameters_ordering + super.userId, + this.isActive = true, + }); + + UserProfile.unorderedConstructor( + String profileId, { + this.age, + // expect_lint: named_parameters_ordering + super.userId, + // expect_lint: named_parameters_ordering + required super.accountType, + this.country, + // expect_lint: named_parameters_ordering + required this.name, + this.isActive = true, + // expect_lint: named_parameters_ordering + required this.email, + }) : profileId = profileId; + + // no lint + void orderedMethod({ + required String name, + required String email, + int? age, + bool isActive = true, + }) { + return; + } + + void partiallyOrderedMethod({ + required String name, + int? age, + // expect_lint: named_parameters_ordering + required String email, + bool isActive = true, + }) { + return; + } + + void unorderedMethod({ + int? age, + // expect_lint: named_parameters_ordering + required String email, + bool isActive = true, + // expect_lint: named_parameters_ordering + required String name, + }) { + return; + } + + void mixedParameters( + String accountType, + String? userId, { + int? age, + // expect_lint: named_parameters_ordering + required String email, + bool isActive = true, + // expect_lint: named_parameters_ordering + required String name, + }) { + return; + } +} + +void functionExample({ + required String name, + bool isActive = true, + // expect_lint: named_parameters_ordering + int? age, + // expect_lint: named_parameters_ordering + required String email, +}) { + return; +} + +void mixedParameters( + String accountType, + String? userId, { + int? age, + // expect_lint: named_parameters_ordering + required String email, + bool isActive = true, + // expect_lint: named_parameters_ordering + required String name, +}) { + return; +}