From 2e398567fac8759d2de1d6ee370e5a04ccd2d8d6 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Tue, 23 Jun 2026 14:41:25 +0300 Subject: [PATCH 1/4] refactor: migrate named_parameters_ordering --- lib/main.dart | 10 + .../config_parser.dart | 15 +- .../fixes/named_parameters_ordering_fix.dart | 196 +++++++++++++ .../models/parameter_type.dart | 43 +++ .../named_parameters_ordering_rule.dart | 94 +++--- .../named_parameters_ordering_visitor.dart | 90 ++---- .../named_parameters_ordering_rule_test.dart | 273 ++++++++++++++++++ 7 files changed, 593 insertions(+), 128 deletions(-) create mode 100644 lib/src/lints/named_parameters_ordering/fixes/named_parameters_ordering_fix.dart create mode 100644 test/src/lints/named_parameters_ordering/named_parameters_ordering_rule_test.dart diff --git a/lib/main.dart b/lib/main.dart index 54efe979..b2ee14f2 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -12,6 +12,8 @@ import 'package:solid_lints/src/lints/avoid_unnecessary_type_assertions/fixes/av import 'package:solid_lints/src/lints/avoid_unused_parameters/avoid_unused_parameters_rule.dart'; 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/named_parameters_ordering/fixes/named_parameters_ordering_fix.dart'; +import 'package:solid_lints/src/lints/named_parameters_ordering/named_parameters_ordering_rule.dart'; import 'package:solid_lints/src/lints/proper_super_calls/proper_super_calls_rule.dart'; import 'package:solid_lints/src/lints/use_nearest_context/fixes/rename_nearest_context_parameter_fix.dart'; import 'package:solid_lints/src/lints/use_nearest_context/fixes/replace_with_nearest_context_parameter_fix.dart'; @@ -53,6 +55,9 @@ class SolidLintsPlugin extends Plugin { AvoidUnusedParametersRule( analysisOptionsLoader: analysisLoader, ), + NamedParametersOrderingRule( + analysisOptionsLoader: analysisLoader, + ), UseNearestContextRule(), ]; @@ -83,5 +88,10 @@ class SolidLintsPlugin extends Plugin { UseNearestContextRule.code, ReplaceWithNearestContextParameterFix.new, ); + + registry.registerFixForRule( + NamedParametersOrderingRule.code, + NamedParametersOrderingFix.new, + ); } } diff --git a/lib/src/lints/named_parameters_ordering/config_parser.dart b/lib/src/lints/named_parameters_ordering/config_parser.dart index 5c5f9570..59ab1d22 100644 --- a/lib/src/lints/named_parameters_ordering/config_parser.dart +++ b/lib/src/lints/named_parameters_ordering/config_parser.dart @@ -25,20 +25,13 @@ import 'package:solid_lints/src/lints/named_parameters_ordering/models/parameter /// 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; + if (orderConfig is! Iterable) { + return ParameterType.defaultOrder; + } + final order = List.from(orderConfig); return order.map(ParameterType.fromType).nonNulls.toList(); } } diff --git a/lib/src/lints/named_parameters_ordering/fixes/named_parameters_ordering_fix.dart b/lib/src/lints/named_parameters_ordering/fixes/named_parameters_ordering_fix.dart new file mode 100644 index 00000000..627ac589 --- /dev/null +++ b/lib/src/lints/named_parameters_ordering/fixes/named_parameters_ordering_fix.dart @@ -0,0 +1,196 @@ +import 'package:analysis_server_plugin/edit/dart/correction_producer.dart'; +import 'package:analysis_server_plugin/edit/dart/dart_fix_kind_priority.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/source/source_range.dart'; +import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; +import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; +import 'package:solid_lints/src/lints/named_parameters_ordering/models/parameter_type.dart'; +import 'package:solid_lints/src/lints/named_parameters_ordering/named_parameters_ordering_rule.dart'; +import 'package:yaml/yaml.dart'; + +/// A Quick fix for [NamedParametersOrderingRule] rule. +class NamedParametersOrderingFix extends ResolvedCorrectionProducer { + static const _fixKind = FixKind( + 'solid_lints.fix.named_parameters_ordering', + DartFixKindPriority.standard, + "Sort named parameters", + ); + + /// Creates a new instance of [NamedParametersOrderingFix]. + NamedParametersOrderingFix({required super.context}); + + @override + FixKind get fixKind => _fixKind; + + @override + CorrectionApplicability get applicability => + CorrectionApplicability.automatically; + + @override + Future compute(ChangeBuilder builder) async { + final parameterList = node.thisOrAncestorOfType(); + if (parameterList == null) return; + + final namedParams = parameterList.parameters + .where((p) => p.isNamed) + .toList(); + if (namedParams.length < 2) return; + + final parametersOrder = _getParametersOrder(); + + final sortedNamedParams = [...namedParams]; + sortedNamedParams.sort((a, b) { + final typeA = ParameterType.fromParameter(a); + final typeB = ParameterType.fromParameter(b); + final indexA = parametersOrder.indexOf(typeA); + final indexB = parametersOrder.indexOf(typeB); + return indexA.compareTo(indexB); + }); + + // Check if the order is already correct (if sorting changed nothing) + bool isChanged = false; + for (int i = 0; i < namedParams.length; i++) { + if (namedParams[i] != sortedNamedParams[i]) { + isChanged = true; + break; + } + } + if (!isChanged) return; + + final isMultiline = utils + .getRangeText(parameterList.sourceRange) + .contains('\n'); + + if (!isMultiline) { + // Single-line: no leading comments, simple text replacement + final sortedTexts = sortedNamedParams + .map((p) => utils.getRangeText(p.sourceRange)) + .toList(); + + final replacementText = sortedTexts.join(', '); + + final targetRange = SourceRange( + namedParams.first.offset, + namedParams.last.end - namedParams.first.offset, + ); + + await builder.addDartFileEdit(file, (builder) { + builder.addSimpleReplacement(targetRange, replacementText); + }); + return; + } + + // Multiline: extract parameter blocks including leading comments + final (:blockTexts, :firstBlockStart) = _extractParamBlocks( + namedParams, + parameterList, + ); + + // Map sorted parameters to their corresponding block texts + final sortedBlockTexts = sortedNamedParams + .map((p) => blockTexts[namedParams.indexOf(p)]) + .toList(); + + final replacementText = sortedBlockTexts.join(',\n'); + + final targetRange = SourceRange( + firstBlockStart, + namedParams.last.end - firstBlockStart, + ); + + await builder.addDartFileEdit(file, (builder) { + builder.addSimpleReplacement(targetRange, replacementText); + }); + } + + /// Extracts text blocks for each named parameter, including any leading + /// comments that belong to that parameter. + /// + /// Returns the block texts and the start offset of the first block + /// (used as the replacement range start). + ({List blockTexts, int firstBlockStart}) _extractParamBlocks( + List namedParams, + FormalParameterList parameterList, + ) { + final blocks = []; + int? firstStart; + + for (int i = 0; i < namedParams.length; i++) { + final param = namedParams[i]; + + final int minOffset = i == 0 + ? (parameterList.leftDelimiter?.end ?? + parameterList.leftParenthesis.end) + : namedParams[i - 1].end; + + // Find block start: use leading comment offset if present, + // then expand to line start for proper indentation + var blockStart = param.offset; + final comment = param.beginToken.precedingComments; + if (comment != null && + comment.offset >= minOffset && + comment.offset < param.offset) { + blockStart = comment.offset; + } + final linePrefix = utils.getLinePrefix(blockStart); + blockStart -= linePrefix.length; + + firstStart ??= blockStart; + blocks.add( + utils.getRangeText(SourceRange(blockStart, param.end - blockStart)), + ); + } + + return ( + blockTexts: blocks, + firstBlockStart: firstStart ?? namedParams.first.offset, + ); + } + + List _getParametersOrder() { + final pathContext = resourceProvider.pathContext; + String currentDirectoryPath = pathContext.dirname(file); + + while (pathContext.dirname(currentDirectoryPath) != currentDirectoryPath) { + final candidatePath = pathContext.join( + currentDirectoryPath, + 'analysis_options.yaml', + ); + final candidateFile = resourceProvider.getFile(candidatePath); + + if (candidateFile.exists) { + try { + final content = candidateFile.readAsStringSync(); + final yaml = loadYaml(content); + if (yaml case { + 'plugins': { + 'solid_lints': { + 'diagnostics': { + NamedParametersOrderingRule.lintName: { + 'order': final List orderList, + }, + }, + }, + }, + }) { + final order = orderList + .map((e) => e is String ? ParameterType.fromType(e) : null) + .whereType() + .toList(); + if (order.isNotEmpty) { + return order; + } + } + } catch (_) { + // ignore parsing error, fallback to default order + } + break; + } + + final parentDir = pathContext.dirname(currentDirectoryPath); + currentDirectoryPath = parentDir; + } + + return ParameterType.defaultOrder; + } +} diff --git a/lib/src/lints/named_parameters_ordering/models/parameter_type.dart b/lib/src/lints/named_parameters_ordering/models/parameter_type.dart index d08901a2..93b29d47 100644 --- a/lib/src/lints/named_parameters_ordering/models/parameter_type.dart +++ b/lib/src/lints/named_parameters_ordering/models/parameter_type.dart @@ -1,3 +1,4 @@ +import 'package:analyzer/dart/ast/ast.dart'; import 'package:collection/collection.dart'; /// Represents a function parameter type @@ -17,11 +18,53 @@ enum ParameterType { /// Default value parameter type (String parameterName = 'defaultValue') defaultValue('default'); + /// The default ordering of parameter types. + static const defaultOrder = [ + ParameterType.requiredInherited, + ParameterType.inherited, + ParameterType.required, + ParameterType.nullable, + ParameterType.defaultValue, + ]; + /// Returns [ParameterType] from type or null if not found static ParameterType? fromType(String type) { return values.firstWhereOrNull((o) => o.type == type); } + /// Classifies a [FormalParameter] into a [ParameterType]. + /// + /// Recursively unwraps [DefaultFormalParameter] wrappers to determine + /// the underlying parameter kind. + static ParameterType fromParameter( + FormalParameter parameter, { + bool hasDefaultValue = false, + }) { + if (parameter is DefaultFormalParameter && + parameter.parameter is! DefaultFormalParameter) { + return fromParameter( + parameter.parameter, + hasDefaultValue: 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; + } + } + /// String representation of the parameter type final String 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 index 845a0c74..110ee5de 100644 --- a/lib/src/lints/named_parameters_ordering/named_parameters_ordering_rule.dart +++ b/lib/src/lints/named_parameters_ordering/named_parameters_ordering_rule.dart @@ -1,9 +1,9 @@ -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/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/models/parameter_type.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 @@ -28,15 +28,16 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// Assuming config: /// /// ```yaml -/// custom_lint: -/// rules: -/// - named_parameters_ordering: -/// order: -/// - required -/// - required_super -/// - default -/// - nullable -/// - super +/// plugins: +/// solid_lints: +/// diagnostics: +/// named_parameters_ordering: +/// order: +/// - required +/// - required_super +/// - default +/// - nullable +/// - super /// ``` /// /// #### BAD: @@ -105,54 +106,41 @@ class NamedParametersOrderingRule /// The name of this lint rule. static const lintName = 'named_parameters_ordering'; - late final _visitor = NamedParametersOrderingVisitor(config.parameters.order); + /// The [LintCode] for this rule. + static const code = LintCode( + lintName, + '{0} named parameters should be before {1} named parameters.', + ); - 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", - ); + @override + LintCode get diagnosticCode => code; - return NamedParametersOrderingRule._(config); - } + /// Creates a new instance of [NamedParametersOrderingRule]. + NamedParametersOrderingRule({ + required super.analysisOptionsLoader, + }) : super.withParameters( + name: lintName, + description: + 'A lint which allows to enforce a particular named parameter ' + 'ordering conventions.', + parametersParser: NamedParametersOrderingParameters.fromJson, + ); @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, ) { - context.registry.addFormalParameterList((node) { - final parametersInfo = _visitor.visitFormalParameterList(node); + super.registerNodeProcessors(registry, context); - final wrongOrderParameters = parametersInfo.where( - (info) => info.parameterOrderingInfo.isWrong, - ); - - for (final parameterInfo in wrongOrderParameters) { - reporter.atNode( - parameterInfo.formalParameter, - _createWrongOrderLintCode(parameterInfo.parameterOrderingInfo), + final parameters = + getParametersForContext(context) ?? + const NamedParametersOrderingParameters( + order: ParameterType.defaultOrder, ); - } - }); - } - LintCode _createWrongOrderLintCode(ParameterOrderingInfo info) { - final parameterOrdering = info.parameterType; - final previousParameterOrdering = info.previousParameterType; + final visitor = NamedParametersOrderingVisitor(this, parameters.order); - return LintCode( - name: lintName, - problemMessage: "${parameterOrdering.displayName} named parameters" - " should be before " - "${previousParameterOrdering!.displayName} named parameters.", - ); + registry.addFormalParameterList(this, visitor); } } 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 index 4ec4598c..90a8efd8 100644 --- 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 @@ -21,87 +21,49 @@ // 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/ast.dart'; 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'; +import 'package:solid_lints/src/lints/named_parameters_ordering/named_parameters_ordering_rule.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> { +/// or default config. +class NamedParametersOrderingVisitor extends SimpleAstVisitor { + final NamedParametersOrderingRule _rule; final List _parametersOrder; - final List _parametersInfo = []; - /// Creates instance of [NamedParametersOrderingVisitor] - NamedParametersOrderingVisitor(this._parametersOrder); + NamedParametersOrderingVisitor(this._rule, this._parametersOrder); @override - List visitFormalParameterList(FormalParameterList node) { - super.visitFormalParameterList(node); - - _parametersInfo.clear(); - + void visitFormalParameterList(FormalParameterList node) { 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; } - return _parametersInfo; - } - - ParameterOrderingInfo _getParameterOrderingInfo(FormalParameter parameter) { - final parameterType = _getParameterType(parameter); - final previousParameterType = - _parametersInfo.lastOrNull?.parameterOrderingInfo.parameterType; + final parametersInfo = []; - 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; + for (final parameter in namedParametersList) { + final parameterType = ParameterType.fromParameter(parameter); + final previousParameterType = parametersInfo.lastOrNull; + + final isWrong = _isOrderingWrong(parameterType, previousParameterType); + + if (isWrong && previousParameterType != null) { + _rule.reportAtNode( + parameter, + arguments: [ + parameterType.displayName, + previousParameterType.displayName, + ], + ); + } + + parametersInfo.add(parameterType); } } diff --git a/test/src/lints/named_parameters_ordering/named_parameters_ordering_rule_test.dart b/test/src/lints/named_parameters_ordering/named_parameters_ordering_rule_test.dart new file mode 100644 index 00000000..e1a90824 --- /dev/null +++ b/test/src/lints/named_parameters_ordering/named_parameters_ordering_rule_test.dart @@ -0,0 +1,273 @@ +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/named_parameters_ordering/named_parameters_ordering_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../../../lints/auto_test_lint_offsets.dart'; + +void main() { + defineRefSuite(); +} + +void defineRefSuite() { + defineReflectiveSuite(() { + defineReflectiveTests(NamedParametersOrderingRuleTest); + }); +} + +@reflectiveTest +class NamedParametersOrderingRuleTest extends AnalysisRuleTest + with AutoTestLintOffsets { + static const _customAnalysisOptionsContent = ''' +plugins: + solid_lints: + diagnostics: + named_parameters_ordering: + order: + - required + - required_super + - default + - nullable + - super +'''; + + @override + void setUp() { + rule = NamedParametersOrderingRule( + analysisOptionsLoader: AnalysisOptionsLoader( + resourceProvider: resourceProvider, + ), + ); + super.setUp(); + newAnalysisOptionsYamlFile( + testPackageRootPath, + analysisOptionsContent(rules: [rule.name]), + ); + } + + @override + String get analysisRule => NamedParametersOrderingRule.lintName; + + Future test_does_not_report_correct_constructor_ordering() async { + await assertNoDiagnostics(r''' +class Base { + final String accountType; + final String? userId; + + Base({ + required this.accountType, + this.userId, + }); +} + +class User extends Base { + final String name; + final String email; + final String? age; + final String? country; + final bool isActive; + + User({ + required super.accountType, + super.userId, + required this.name, + required this.email, + this.age, + this.country, + this.isActive = true, + }); +} +'''); + } + + Future test_reports_incorrect_constructor_ordering() async { + await assertAutoDiagnostics(''' +class User { + final String accountType; + final String? userId; + + User({ + this.userId, + ${expectLint('required this.accountType')}, + }); +} +'''); + } + + Future test_reports_incorrect_constructor_ordering_complex() async { + await assertAutoDiagnostics(''' +class Base { + final String accountType; + final String? userId; + + Base({ + required this.accountType, + this.userId, + }); +} + +class User extends Base { + final String name; + final String email; + final String? age; + final bool isActive; + + User({ + required super.accountType, + this.age, + ${expectLint('super.userId')}, + required this.name, + this.isActive = true, + ${expectLint('required this.email')}, + }); +} +'''); + } + + Future test_does_not_report_correct_method_ordering() async { + await assertNoDiagnostics(r''' +class UserProfile { + void orderedMethod({ + required String name, + required String email, + int? age, + bool isActive = true, + }) { + return; + } +} +'''); + } + + Future test_reports_incorrect_method_ordering() async { + await assertAutoDiagnostics(''' +class UserProfile { + void partiallyOrderedMethod({ + required String name, + int? age, + ${expectLint('required String email')}, + bool isActive = true, + }) { + return; + } +} +'''); + } + + Future test_reports_incorrect_function_ordering() async { + await assertAutoDiagnostics(''' +void functionExample({ + required String name, + bool isActive = true, + ${expectLint('int? age')}, + ${expectLint('required String email')}, +}) { + return; +} +'''); + } + + Future test_reports_mixed_positional_and_named_parameters() async { + await assertAutoDiagnostics(''' +void mixedParameters( + String accountType, + String? userId, { + int? age, + ${expectLint('required String email')}, + bool isActive = true, + ${expectLint('required String name')}, +}) { + return; +} +'''); + } + + Future test_reports_incorrect_ordering_with_custom_config() async { + newAnalysisOptionsYamlFile(testPackageRootPath, ''' +${analysisOptionsContent(rules: [rule.name])} +$_customAnalysisOptionsContent'''); + await assertAutoDiagnostics(''' +class User { + final String accountType; + final String? userId; + + User({ + this.userId, + ${expectLint('required this.accountType')}, + }); +} +'''); + } + + Future + test_reports_incorrect_ordering_with_custom_config_super() async { + newAnalysisOptionsYamlFile(testPackageRootPath, ''' +${analysisOptionsContent(rules: [rule.name])} +$_customAnalysisOptionsContent'''); + await assertAutoDiagnostics(''' +class Base { + final String? name; + Base({this.name}); +} + +class User extends Base { + final String? email; + User({ + super.name, + ${expectLint('required this.email')}, + }); +} +'''); + } + + Future test_reports_incorrect_ordering_with_callback_parameter() async { + await assertAutoDiagnostics(''' +void example({ + int? age, + ${expectLint('required void Function() onTap')}, +}) { + return; +} +'''); + } + + Future test_does_not_report_correct_ordering_with_callback() async { + await assertNoDiagnostics(r''' +void example({ + required void Function() onTap, + int? age, +}) { + return; +} +'''); + } + + Future test_reports_incorrect_ordering_with_comments() async { + await assertAutoDiagnostics(''' +void example({ + /* Whether active */ + bool isActive = true, + // Email comment + ${expectLint('required String email')}, + /// The age of the user. + /// Can be null if unknown. + int? age, +}) { + return; +} +'''); + } + + Future test_reports_incorrect_ordering_with_complex_defaults() async { + await assertAutoDiagnostics(''' +void example({ + List items = const [], + int count = 1 + 2, + ${expectLint('required String name')}, +}) { + return; +} +'''); + } +} From 4b8d852d1787baa659659cd1a09d777ca4bb3197 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 24 Jun 2026 12:38:17 +0300 Subject: [PATCH 2/4] refactor: improve named parameters ordering fix to preserve trailing comments and fix configuration parsing logic --- .../analysis_options_loader.dart | 16 ++ .../config_parser.dart | 13 +- .../fixes/named_parameters_ordering_fix.dart | 185 +++++++++++------- .../named_parameters_ordering_rule_test.dart | 79 ++++++++ 4 files changed, 223 insertions(+), 70 deletions(-) diff --git a/lib/src/common/parameter_parser/analysis_options_loader.dart b/lib/src/common/parameter_parser/analysis_options_loader.dart index ab22ecc9..f4fb74e8 100644 --- a/lib/src/common/parameter_parser/analysis_options_loader.dart +++ b/lib/src/common/parameter_parser/analysis_options_loader.dart @@ -21,6 +21,22 @@ class AnalysisOptionsLoader { (path) => _rulesCache[path]?.rules[ruleName], ); + /// Gets the options for a specific rule by looking up the nearest + /// `analysis_options.yaml` from the given [filePath]'s directory. + /// + /// Unlike [getRuleOptions], this method does not require a [RuleContext] + /// and can be used from quick fixes. + Map? getRuleOptionsForFile( + String filePath, + String ruleName, + ) { + final dirPath = _resourceProvider.pathContext.dirname(filePath); + final yamlPath = _findNearestAnalysisOptionsFilePath(dirPath); + if (yamlPath == null) return null; + _loadRulesOptionsIfNewer(yamlPath); + return _rulesCache[yamlPath]?.rules[ruleName]; + } + /// Loads lint rules from the analysis options file for all rules /// using the provided [RuleContext]. void loadRulesOptionsFromContext(RuleContext context) => diff --git a/lib/src/lints/named_parameters_ordering/config_parser.dart b/lib/src/lints/named_parameters_ordering/config_parser.dart index 59ab1d22..21fedaa1 100644 --- a/lib/src/lints/named_parameters_ordering/config_parser.dart +++ b/lib/src/lints/named_parameters_ordering/config_parser.dart @@ -23,7 +23,7 @@ import 'package:solid_lints/src/lints/named_parameters_ordering/models/parameter_type.dart'; -/// Helper class to parse member_ordering rule config +/// Helper class to parse named_parameters_ordering rule config class NamedParametersConfigParser { /// Parse rule config for regular class order rules static List parseOrder(Object? orderConfig) { @@ -31,7 +31,14 @@ class NamedParametersConfigParser { return ParameterType.defaultOrder; } - final order = List.from(orderConfig); - return order.map(ParameterType.fromType).nonNulls.toList(); + final parsed = orderConfig + .whereType() + .map(ParameterType.fromType) + .nonNulls + .toList(); + final missing = ParameterType.defaultOrder.where( + (type) => !parsed.contains(type), + ); + return [...parsed, ...missing]; } } diff --git a/lib/src/lints/named_parameters_ordering/fixes/named_parameters_ordering_fix.dart b/lib/src/lints/named_parameters_ordering/fixes/named_parameters_ordering_fix.dart index 627ac589..c0b0713c 100644 --- a/lib/src/lints/named_parameters_ordering/fixes/named_parameters_ordering_fix.dart +++ b/lib/src/lints/named_parameters_ordering/fixes/named_parameters_ordering_fix.dart @@ -1,12 +1,18 @@ import 'package:analysis_server_plugin/edit/dart/correction_producer.dart'; import 'package:analysis_server_plugin/edit/dart/dart_fix_kind_priority.dart'; import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; import 'package:analyzer/source/source_range.dart'; import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; +import 'package:solid_lints/src/common/parameter_parser/analysis_options_loader.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_type.dart'; import 'package:solid_lints/src/lints/named_parameters_ordering/named_parameters_ordering_rule.dart'; -import 'package:yaml/yaml.dart'; + +/// A parameter block: the text of a parameter (including leading comments and +/// indentation) and an optional trailing comment on the same line. +typedef _ParamBlock = ({String text, String? trailingComment}); /// A Quick fix for [NamedParametersOrderingRule] rule. class NamedParametersOrderingFix extends ResolvedCorrectionProducer { @@ -80,39 +86,76 @@ class NamedParametersOrderingFix extends ResolvedCorrectionProducer { return; } - // Multiline: extract parameter blocks including leading comments - final (:blockTexts, :firstBlockStart) = _extractParamBlocks( + // Multiline: extract parameter blocks including leading and trailing + // comments + final (:blocks, :firstBlockStart) = _extractParamBlocks( namedParams, parameterList, ); - // Map sorted parameters to their corresponding block texts - final sortedBlockTexts = sortedNamedParams - .map((p) => blockTexts[namedParams.indexOf(p)]) + // Map sorted parameters to their corresponding blocks + final sortedBlocks = sortedNamedParams + .map((p) => blocks[namedParams.indexOf(p)]) .toList(); - final replacementText = sortedBlockTexts.join(',\n'); + // Determine if original had a trailing comma after the last param + final hasTrailingComma = namedParams.last.endToken.next?.lexeme == ','; + + // Build replacement text preserving trailing comments + final buffer = StringBuffer(); + for (int i = 0; i < sortedBlocks.length; i++) { + buffer.write(sortedBlocks[i].text); + + final isLast = i == sortedBlocks.length - 1; + if (!isLast || hasTrailingComma) { + buffer.write(','); + } + final trailingComment = sortedBlocks[i].trailingComment; + if (trailingComment != null) { + buffer.write(' $trailingComment'); + } + if (!isLast) { + buffer.write('\n'); + } + } + + // Extend range to include the original trailing comma and any trailing + // comment on the original last parameter's line. + var rangeEnd = namedParams.last.end; + final upperBound = + parameterList.rightDelimiter?.offset ?? + parameterList.rightParenthesis.offset; + if (rangeEnd < upperBound) { + final afterLast = utils.getRangeText( + SourceRange(rangeEnd, upperBound - rangeEnd), + ); + final newlineIdx = afterLast.indexOf('\n'); + if (newlineIdx != -1) { + rangeEnd += newlineIdx; + } + } final targetRange = SourceRange( firstBlockStart, - namedParams.last.end - firstBlockStart, + rangeEnd - firstBlockStart, ); await builder.addDartFileEdit(file, (builder) { - builder.addSimpleReplacement(targetRange, replacementText); + builder.addSimpleReplacement(targetRange, buffer.toString()); }); } /// Extracts text blocks for each named parameter, including any leading - /// comments that belong to that parameter. + /// comments that belong to that parameter, and detects trailing comments + /// on the same line. /// - /// Returns the block texts and the start offset of the first block - /// (used as the replacement range start). - ({List blockTexts, int firstBlockStart}) _extractParamBlocks( + /// Trailing comments (e.g., `// comment` after a parameter on the same line) + /// are attributed to the parameter they follow, not the next parameter. + ({List<_ParamBlock> blocks, int firstBlockStart}) _extractParamBlocks( List namedParams, FormalParameterList parameterList, ) { - final blocks = []; + final blocks = <_ParamBlock>[]; int? firstStart; for (int i = 0; i < namedParams.length; i++) { @@ -123,74 +166,82 @@ class NamedParametersOrderingFix extends ResolvedCorrectionProducer { parameterList.leftParenthesis.end) : namedParams[i - 1].end; - // Find block start: use leading comment offset if present, - // then expand to line start for proper indentation + // Find leading comment, skipping any trailing comment that belongs + // to the previous parameter (same line as previous param). var blockStart = param.offset; - final comment = param.beginToken.precedingComments; - if (comment != null && - comment.offset >= minOffset && - comment.offset < param.offset) { - blockStart = comment.offset; + Token? leadingComment = param.beginToken.precedingComments; + if (i > 0 && leadingComment != null) { + final betweenText = utils.getRangeText( + SourceRange( + namedParams[i - 1].end, + leadingComment.offset - namedParams[i - 1].end, + ), + ); + if (!betweenText.contains('\n')) { + // This comment is a trailing comment of the previous param. + // Try the next comment in the chain as our leading comment. + final nextComment = leadingComment.next; + leadingComment = + nextComment != null && + nextComment.offset >= minOffset && + nextComment.offset < param.offset + ? nextComment + : null; + } + } + if (leadingComment != null && + leadingComment.offset >= minOffset && + leadingComment.offset < param.offset) { + blockStart = leadingComment.offset; } final linePrefix = utils.getLinePrefix(blockStart); blockStart -= linePrefix.length; + // Find trailing comment on the same line as this parameter. + String? trailingComment; + final searchBound = + parameterList.rightDelimiter?.offset ?? + parameterList.rightParenthesis.offset; + if (param.end < searchBound) { + final afterParam = utils.getRangeText( + SourceRange(param.end, searchBound - param.end), + ); + final newlineIdx = afterParam.indexOf('\n'); + final sameLine = newlineIdx == -1 + ? afterParam + : afterParam.substring(0, newlineIdx); + final commentIdx = sameLine.indexOf('//'); + if (commentIdx != -1) { + trailingComment = sameLine.substring(commentIdx); + } + } + firstStart ??= blockStart; - blocks.add( - utils.getRangeText(SourceRange(blockStart, param.end - blockStart)), - ); + blocks.add(( + text: utils.getRangeText( + SourceRange(blockStart, param.end - blockStart), + ), + trailingComment: trailingComment, + )); } return ( - blockTexts: blocks, + blocks: blocks, firstBlockStart: firstStart ?? namedParams.first.offset, ); } List _getParametersOrder() { - final pathContext = resourceProvider.pathContext; - String currentDirectoryPath = pathContext.dirname(file); - - while (pathContext.dirname(currentDirectoryPath) != currentDirectoryPath) { - final candidatePath = pathContext.join( - currentDirectoryPath, - 'analysis_options.yaml', - ); - final candidateFile = resourceProvider.getFile(candidatePath); - - if (candidateFile.exists) { - try { - final content = candidateFile.readAsStringSync(); - final yaml = loadYaml(content); - if (yaml case { - 'plugins': { - 'solid_lints': { - 'diagnostics': { - NamedParametersOrderingRule.lintName: { - 'order': final List orderList, - }, - }, - }, - }, - }) { - final order = orderList - .map((e) => e is String ? ParameterType.fromType(e) : null) - .whereType() - .toList(); - if (order.isNotEmpty) { - return order; - } - } - } catch (_) { - // ignore parsing error, fallback to default order - } - break; - } - - final parentDir = pathContext.dirname(currentDirectoryPath); - currentDirectoryPath = parentDir; + final loader = AnalysisOptionsLoader( + resourceProvider: resourceProvider, + ); + final options = loader.getRuleOptionsForFile( + file, + NamedParametersOrderingRule.lintName, + ); + if (options != null) { + return NamedParametersOrderingParameters.fromJson(options).order; } - return ParameterType.defaultOrder; } } diff --git a/test/src/lints/named_parameters_ordering/named_parameters_ordering_rule_test.dart b/test/src/lints/named_parameters_ordering/named_parameters_ordering_rule_test.dart index e1a90824..7a1e82cd 100644 --- a/test/src/lints/named_parameters_ordering/named_parameters_ordering_rule_test.dart +++ b/test/src/lints/named_parameters_ordering/named_parameters_ordering_rule_test.dart @@ -259,6 +259,35 @@ void example({ '''); } + Future + test_reports_incorrect_ordering_with_trailing_comments() async { + await assertAutoDiagnostics(''' +void example({ + int? age, // the age + ${expectLint('required String name')}, // the name + bool isActive = true, // active flag +}) { + return; +} +'''); + } + + Future + test_reports_incorrect_ordering_with_mixed_comments() async { + await assertAutoDiagnostics(''' +void example({ + /// The age of the user. + /// Can be null if unknown. + int? age, // optional + // The user's name + ${expectLint('required String name')}, // must not be empty + bool isActive = true, +}) { + return; +} +'''); + } + Future test_reports_incorrect_ordering_with_complex_defaults() async { await assertAutoDiagnostics(''' void example({ @@ -268,6 +297,56 @@ void example({ }) { return; } +'''); + } + + Future + test_does_not_report_with_partial_custom_config() async { + newAnalysisOptionsYamlFile(testPackageRootPath, ''' +${analysisOptionsContent(rules: [rule.name])} +plugins: + solid_lints: + diagnostics: + named_parameters_ordering: + order: + - required + - nullable +'''); + // 'default' is omitted from config but should not cause false positives. + // It should be appended after 'nullable' automatically. + await assertNoDiagnostics(r''' +void example({ + required String name, + int? age, + bool isActive = true, +}) { + return; +} +'''); + } + + Future + test_reports_incorrect_ordering_with_partial_custom_config() async { + newAnalysisOptionsYamlFile(testPackageRootPath, ''' +${analysisOptionsContent(rules: [rule.name])} +plugins: + solid_lints: + diagnostics: + named_parameters_ordering: + order: + - required + - nullable +'''); + // Order is [required, nullable, default]. + // 'default' is appended automatically after 'nullable'. + await assertAutoDiagnostics(''' +void example({ + int? age, + ${expectLint('required String name')}, + bool isActive = true, +}) { + return; +} '''); } } From 3d97b4220554e98adf4b5e80f1bd2b223b68a52d Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 24 Jun 2026 13:01:47 +0300 Subject: [PATCH 3/4] fix: prevent incorrect parameter reordering when comments are present and ensure unique parameter types in config parser --- .../config_parser.dart | 1 + .../fixes/named_parameters_ordering_fix.dart | 29 ++++++++++++++----- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/lib/src/lints/named_parameters_ordering/config_parser.dart b/lib/src/lints/named_parameters_ordering/config_parser.dart index 21fedaa1..52d94922 100644 --- a/lib/src/lints/named_parameters_ordering/config_parser.dart +++ b/lib/src/lints/named_parameters_ordering/config_parser.dart @@ -35,6 +35,7 @@ class NamedParametersConfigParser { .whereType() .map(ParameterType.fromType) .nonNulls + .toSet() .toList(); final missing = ParameterType.defaultOrder.where( (type) => !parsed.contains(type), diff --git a/lib/src/lints/named_parameters_ordering/fixes/named_parameters_ordering_fix.dart b/lib/src/lints/named_parameters_ordering/fixes/named_parameters_ordering_fix.dart index c0b0713c..f7bcc5ab 100644 --- a/lib/src/lints/named_parameters_ordering/fixes/named_parameters_ordering_fix.dart +++ b/lib/src/lints/named_parameters_ordering/fixes/named_parameters_ordering_fix.dart @@ -67,7 +67,11 @@ class NamedParametersOrderingFix extends ResolvedCorrectionProducer { .getRangeText(parameterList.sourceRange) .contains('\n'); - if (!isMultiline) { + final hasComments = namedParams.any( + (p) => p.beginToken.precedingComments != null, + ); + + if (!isMultiline && !hasComments) { // Single-line: no leading comments, simple text replacement final sortedTexts = sortedNamedParams .map((p) => utils.getRangeText(p.sourceRange)) @@ -122,6 +126,9 @@ class NamedParametersOrderingFix extends ResolvedCorrectionProducer { // Extend range to include the original trailing comma and any trailing // comment on the original last parameter's line. var rangeEnd = namedParams.last.end; + if (hasTrailingComma) { + rangeEnd = namedParams.last.endToken.next!.end; + } final upperBound = parameterList.rightDelimiter?.offset ?? parameterList.rightParenthesis.offset; @@ -194,17 +201,23 @@ class NamedParametersOrderingFix extends ResolvedCorrectionProducer { leadingComment.offset < param.offset) { blockStart = leadingComment.offset; } - final linePrefix = utils.getLinePrefix(blockStart); - blockStart -= linePrefix.length; + final lineStart = utils.getLineThis(blockStart); + final prefixText = utils.getRangeText( + SourceRange(lineStart, blockStart - lineStart), + ); + if (prefixText.trim().isEmpty) { + blockStart = lineStart; + } // Find trailing comment on the same line as this parameter. String? trailingComment; - final searchBound = - parameterList.rightDelimiter?.offset ?? - parameterList.rightParenthesis.offset; - if (param.end < searchBound) { + final nextParamStart = i < namedParams.length - 1 + ? namedParams[i + 1].offset + : (parameterList.rightDelimiter?.offset ?? + parameterList.rightParenthesis.offset); + if (param.end < nextParamStart) { final afterParam = utils.getRangeText( - SourceRange(param.end, searchBound - param.end), + SourceRange(param.end, nextParamStart - param.end), ); final newlineIdx = afterParam.indexOf('\n'); final sameLine = newlineIdx == -1 From 6550bb659515b0975bdd5942d55a9f31c65086d9 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 24 Jun 2026 13:12:19 +0300 Subject: [PATCH 4/4] fix: improve trailing comment detection by iterating through the entire comment chain in named parameters fix --- .../fixes/named_parameters_ordering_fix.dart | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/lib/src/lints/named_parameters_ordering/fixes/named_parameters_ordering_fix.dart b/lib/src/lints/named_parameters_ordering/fixes/named_parameters_ordering_fix.dart index f7bcc5ab..cf5dd648 100644 --- a/lib/src/lints/named_parameters_ordering/fixes/named_parameters_ordering_fix.dart +++ b/lib/src/lints/named_parameters_ordering/fixes/named_parameters_ordering_fix.dart @@ -177,23 +177,19 @@ class NamedParametersOrderingFix extends ResolvedCorrectionProducer { // to the previous parameter (same line as previous param). var blockStart = param.offset; Token? leadingComment = param.beginToken.precedingComments; - if (i > 0 && leadingComment != null) { - final betweenText = utils.getRangeText( - SourceRange( - namedParams[i - 1].end, - leadingComment.offset - namedParams[i - 1].end, - ), - ); - if (!betweenText.contains('\n')) { - // This comment is a trailing comment of the previous param. - // Try the next comment in the chain as our leading comment. - final nextComment = leadingComment.next; - leadingComment = - nextComment != null && - nextComment.offset >= minOffset && - nextComment.offset < param.offset - ? nextComment - : null; + if (i > 0) { + while (leadingComment != null) { + final betweenText = utils.getRangeText( + SourceRange( + namedParams[i - 1].end, + leadingComment.offset - namedParams[i - 1].end, + ), + ); + if (!betweenText.contains('\n')) { + leadingComment = leadingComment.next; + } else { + break; + } } } if (leadingComment != null &&