From 2e398567fac8759d2de1d6ee370e5a04ccd2d8d6 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Tue, 23 Jun 2026 14:41:25 +0300 Subject: [PATCH 1/8] 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/8] 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/8] 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/8] 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 && From 4be1427e0111d6d10ee90774eb08c89be7cfb451 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Fri, 26 Jun 2026 13:56:41 +0300 Subject: [PATCH 5/8] refactor: extract correction and iterable utilities and optimize named parameters ordering fix logic --- .../fixes/named_parameters_ordering_fix.dart | 138 +++++++----------- .../named_parameters_ordering_visitor.dart | 44 +++--- lib/src/utils/correction_utils.dart | 13 ++ lib/src/utils/iterable_utils.dart | 16 ++ 4 files changed, 102 insertions(+), 109 deletions(-) create mode 100644 lib/src/utils/correction_utils.dart create mode 100644 lib/src/utils/iterable_utils.dart 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 cf5dd648..d75a5144 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 @@ -2,13 +2,14 @@ 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:collection/collection.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:solid_lints/src/utils/correction_utils.dart'; /// A parameter block: the text of a parameter (including leading comments and /// indentation) and an optional trailing comment on the same line. @@ -44,23 +45,15 @@ class NamedParametersOrderingFix extends ResolvedCorrectionProducer { 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); - }); + final sortedNamedParams = namedParams.sortedBy( + (e) => parametersOrder.indexOf(ParameterType.fromParameter(e)), + ); // 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; - } - } + final isChanged = !const ListEquality().equals( + namedParams, + sortedNamedParams, + ); if (!isChanged) return; final isMultiline = utils @@ -71,23 +64,19 @@ class NamedParametersOrderingFix extends ResolvedCorrectionProducer { (p) => p.beginToken.precedingComments != null, ); + final sourceStart = namedParams.first.offset; + final sourceEnd = namedParams.last.end; + if (!isMultiline && !hasComments) { // 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 builder.addDartFileEdit(file, (builder) { + builder.addSimpleReplacement( + utils.createRange(sourceStart, sourceEnd), + sortedNamedParams + .map((p) => utils.getRangeText(p.sourceRange)) + .join(', '), + ); }); - return; } // Multiline: extract parameter blocks including leading and trailing @@ -103,52 +92,39 @@ class NamedParametersOrderingFix extends ResolvedCorrectionProducer { .toList(); // Determine if original had a trailing comma after the last param - final hasTrailingComma = namedParams.last.endToken.next?.lexeme == ','; + final tokenAfterEnd = namedParams.last.endToken.next; + final hasTrailingComma = tokenAfterEnd?.lexeme == ','; // Build replacement text preserving trailing comments - final buffer = StringBuffer(); - for (int i = 0; i < sortedBlocks.length; i++) { - buffer.write(sortedBlocks[i].text); - + final replacement = sortedBlocks.expandIndexed((i, e) { 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'); - } - } + final trailingComment = e.trailingComment; + return [ + e.text, + if (!isLast || hasTrailingComma) ',', + if (trailingComment != null) ' $trailingComment', + if (!isLast) '\n', + ]; + }).join(); // 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; - } + var rangeEnd = hasTrailingComma ? tokenAfterEnd!.end : sourceEnd; final upperBound = parameterList.rightDelimiter?.offset ?? parameterList.rightParenthesis.offset; if (rangeEnd < upperBound) { - final afterLast = utils.getRangeText( - SourceRange(rangeEnd, upperBound - rangeEnd), - ); + final afterLast = utils.getTextRange(rangeEnd, upperBound); final newlineIdx = afterLast.indexOf('\n'); if (newlineIdx != -1) { rangeEnd += newlineIdx; } } - final targetRange = SourceRange( - firstBlockStart, - rangeEnd - firstBlockStart, - ); + final targetRange = utils.createRange(firstBlockStart, rangeEnd); await builder.addDartFileEdit(file, (builder) { - builder.addSimpleReplacement(targetRange, buffer.toString()); + builder.addSimpleReplacement(targetRange, replacement); }); } @@ -165,13 +141,16 @@ class NamedParametersOrderingFix extends ResolvedCorrectionProducer { final blocks = <_ParamBlock>[]; int? firstStart; + final lowerBound = + parameterList.leftDelimiter?.end ?? parameterList.leftParenthesis.end; + final upperBound = + parameterList.rightDelimiter?.offset ?? + parameterList.rightParenthesis.offset; + 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; + final int minOffset = i == 0 ? lowerBound : namedParams[i - 1].end; // Find leading comment, skipping any trailing comment that belongs // to the previous parameter (same line as previous param). @@ -179,11 +158,9 @@ class NamedParametersOrderingFix extends ResolvedCorrectionProducer { Token? leadingComment = param.beginToken.precedingComments; if (i > 0) { while (leadingComment != null) { - final betweenText = utils.getRangeText( - SourceRange( - namedParams[i - 1].end, - leadingComment.offset - namedParams[i - 1].end, - ), + final betweenText = utils.getTextRange( + namedParams[i - 1].end, + leadingComment.offset, ); if (!betweenText.contains('\n')) { leadingComment = leadingComment.next; @@ -198,9 +175,7 @@ class NamedParametersOrderingFix extends ResolvedCorrectionProducer { blockStart = leadingComment.offset; } final lineStart = utils.getLineThis(blockStart); - final prefixText = utils.getRangeText( - SourceRange(lineStart, blockStart - lineStart), - ); + final prefixText = utils.getTextRange(lineStart, blockStart); if (prefixText.trim().isEmpty) { blockStart = lineStart; } @@ -209,16 +184,10 @@ class NamedParametersOrderingFix extends ResolvedCorrectionProducer { String? trailingComment; final nextParamStart = i < namedParams.length - 1 ? namedParams[i + 1].offset - : (parameterList.rightDelimiter?.offset ?? - parameterList.rightParenthesis.offset); + : upperBound; if (param.end < nextParamStart) { - final afterParam = utils.getRangeText( - SourceRange(param.end, nextParamStart - param.end), - ); - final newlineIdx = afterParam.indexOf('\n'); - final sameLine = newlineIdx == -1 - ? afterParam - : afterParam.substring(0, newlineIdx); + final afterParam = utils.getTextRange(param.end, nextParamStart); + final sameLine = afterParam.split('\n').first; final commentIdx = sameLine.indexOf('//'); if (commentIdx != -1) { trailingComment = sameLine.substring(commentIdx); @@ -227,9 +196,7 @@ class NamedParametersOrderingFix extends ResolvedCorrectionProducer { firstStart ??= blockStart; blocks.add(( - text: utils.getRangeText( - SourceRange(blockStart, param.end - blockStart), - ), + text: utils.getTextRange(blockStart, param.end), trailingComment: trailingComment, )); } @@ -248,9 +215,8 @@ class NamedParametersOrderingFix extends ResolvedCorrectionProducer { file, NamedParametersOrderingRule.lintName, ); - if (options != null) { - return NamedParametersOrderingParameters.fromJson(options).order; - } - return ParameterType.defaultOrder; + return options == null + ? ParameterType.defaultOrder + : NamedParametersOrderingParameters.fromJson(options).order; } } 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 90a8efd8..44d2e26d 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 @@ -25,6 +25,7 @@ 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_type.dart'; import 'package:solid_lints/src/lints/named_parameters_ordering/named_parameters_ordering_rule.dart'; +import 'package:solid_lints/src/utils/iterable_utils.dart'; /// AST Visitor which finds all methods, functions and constructor named /// parameters and checks if they are in order provided from rule config @@ -38,41 +39,38 @@ class NamedParametersOrderingVisitor extends SimpleAstVisitor { @override void visitFormalParameterList(FormalParameterList node) { - final namedParametersList = - node.parameters.where((p) => p.isNamed).toList(); + final namedParametersList = node.parameters + .where((p) => p.isNamed) + .toList(); if (namedParametersList.isEmpty) { return; } - final parametersInfo = []; + final parametersInfo = namedParametersList.map( + (p) => (parameter: p, type: ParameterType.fromParameter(p)), + ); - for (final parameter in namedParametersList) { - final parameterType = ParameterType.fromParameter(parameter); - final previousParameterType = parametersInfo.lastOrNull; + final badParameters = parametersInfo.pairwise().where( + (pair) => _isOrderingWrong(pair.$2.type, pair.$1.type), + ); - final isWrong = _isOrderingWrong(parameterType, previousParameterType); - - if (isWrong && previousParameterType != null) { - _rule.reportAtNode( - parameter, - arguments: [ - parameterType.displayName, - previousParameterType.displayName, - ], - ); - } - - parametersInfo.add(parameterType); + for (final (previous, current) in badParameters) { + _rule.reportAtNode( + current.parameter, + arguments: [ + current.type.displayName, + previous.type.displayName, + ], + ); } } bool _isOrderingWrong( ParameterType currentParameterType, - ParameterType? previousParameterType, + ParameterType previousParameterType, ) { - return previousParameterType != null && - _parametersOrder.indexOf(previousParameterType) > - _parametersOrder.indexOf(currentParameterType); + return _parametersOrder.indexOf(previousParameterType) > + _parametersOrder.indexOf(currentParameterType); } } diff --git a/lib/src/utils/correction_utils.dart b/lib/src/utils/correction_utils.dart new file mode 100644 index 00000000..c10179cc --- /dev/null +++ b/lib/src/utils/correction_utils.dart @@ -0,0 +1,13 @@ +import 'package:analysis_server_plugin/edit/correction_utils.dart'; +import 'package:analyzer/source/source_range.dart'; + +/// Extension on [CorrectionUtils] to provide utilities for creating [SourceRange]s. +extension CorrectionUtilsExtension on CorrectionUtils { + /// Creates a [SourceRange] from [start] and [end] offsets. + SourceRange createRange(int start, int end) => + SourceRange(start, end - start); + + /// Returns the text of the range from [start] to [end]. + String getTextRange(int start, int end) => + getRangeText(createRange(start, end)); +} diff --git a/lib/src/utils/iterable_utils.dart b/lib/src/utils/iterable_utils.dart new file mode 100644 index 00000000..4b9d54a1 --- /dev/null +++ b/lib/src/utils/iterable_utils.dart @@ -0,0 +1,16 @@ +/// Extension on [Iterable] that provides a [pairwise] method for grouping +/// elements. +extension IterablePairwise on Iterable { + /// Returns an iterable of overlapping pairs of elements. + /// For example, `[1, 2, 3].pairwise()` returns `[(1, 2), (2, 3)]`. + Iterable<(T, T)> pairwise() sync* { + final iterator = this.iterator; + if (!iterator.moveNext()) return; + + var previous = iterator.current; + while (iterator.moveNext()) { + yield (previous, iterator.current); + previous = iterator.current; + } + } +} From 09da30ef0211f015ade690e76645dc1a7980c910 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Fri, 26 Jun 2026 13:58:23 +0300 Subject: [PATCH 6/8] style: fix doc comment line length in correction_utils.dart --- lib/src/utils/correction_utils.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/src/utils/correction_utils.dart b/lib/src/utils/correction_utils.dart index c10179cc..ba68723e 100644 --- a/lib/src/utils/correction_utils.dart +++ b/lib/src/utils/correction_utils.dart @@ -1,7 +1,8 @@ import 'package:analysis_server_plugin/edit/correction_utils.dart'; import 'package:analyzer/source/source_range.dart'; -/// Extension on [CorrectionUtils] to provide utilities for creating [SourceRange]s. +/// Extension on [CorrectionUtils] to provide utilities for creating +/// [SourceRange]s. extension CorrectionUtilsExtension on CorrectionUtils { /// Creates a [SourceRange] from [start] and [end] offsets. SourceRange createRange(int start, int end) => From f9b09c1b3f6569d56b54be57c841fc9e92924fb0 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Fri, 26 Jun 2026 14:31:07 +0300 Subject: [PATCH 7/8] refactor: simplify pairwise implementation to use indexed element access --- lib/src/utils/iterable_utils.dart | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/src/utils/iterable_utils.dart b/lib/src/utils/iterable_utils.dart index 4b9d54a1..9f569e21 100644 --- a/lib/src/utils/iterable_utils.dart +++ b/lib/src/utils/iterable_utils.dart @@ -4,13 +4,8 @@ extension IterablePairwise on Iterable { /// Returns an iterable of overlapping pairs of elements. /// For example, `[1, 2, 3].pairwise()` returns `[(1, 2), (2, 3)]`. Iterable<(T, T)> pairwise() sync* { - final iterator = this.iterator; - if (!iterator.moveNext()) return; - - var previous = iterator.current; - while (iterator.moveNext()) { - yield (previous, iterator.current); - previous = iterator.current; + for (var i = 0; i + 1 < length; i++) { + yield (elementAt(i), elementAt(i + 1)); } } } From f3b77c084a5f8ba0e6cf01203347b5268d6cc0fd Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Tue, 30 Jun 2026 12:31:23 +0300 Subject: [PATCH 8/8] refactor: extract comment handling logic to correction utilities --- .../fixes/named_parameters_ordering_fix.dart | 79 ++++++------------- lib/src/utils/correction_utils.dart | 59 +++++++++++++- 2 files changed, 84 insertions(+), 54 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 d75a5144..4349e8d0 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,7 +1,6 @@ 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_plugin/utilities/change_builder/change_builder_core.dart'; import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; import 'package:collection/collection.dart'; @@ -11,9 +10,9 @@ import 'package:solid_lints/src/lints/named_parameters_ordering/models/parameter import 'package:solid_lints/src/lints/named_parameters_ordering/named_parameters_ordering_rule.dart'; import 'package:solid_lints/src/utils/correction_utils.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 parameter block: the offsets of a parameter (including leading comments +/// and indentation) and an optional trailing comment on the same line. +typedef _ParamBlock = ({int blockStart, int blockEnd, String? trailingComment}); /// A Quick fix for [NamedParametersOrderingRule] rule. class NamedParametersOrderingFix extends ResolvedCorrectionProducer { @@ -81,7 +80,7 @@ class NamedParametersOrderingFix extends ResolvedCorrectionProducer { // Multiline: extract parameter blocks including leading and trailing // comments - final (:blocks, :firstBlockStart) = _extractParamBlocks( + final blocks = _extractParamBlocks( namedParams, parameterList, ); @@ -99,8 +98,9 @@ class NamedParametersOrderingFix extends ResolvedCorrectionProducer { final replacement = sortedBlocks.expandIndexed((i, e) { final isLast = i == sortedBlocks.length - 1; final trailingComment = e.trailingComment; + final text = utils.getTextRange(e.blockStart, e.blockEnd); return [ - e.text, + text, if (!isLast || hasTrailingComma) ',', if (trailingComment != null) ' $trailingComment', if (!isLast) '\n', @@ -121,6 +121,7 @@ class NamedParametersOrderingFix extends ResolvedCorrectionProducer { } } + final firstBlockStart = blocks.first.blockStart; final targetRange = utils.createRange(firstBlockStart, rangeEnd); await builder.addDartFileEdit(file, (builder) { @@ -134,12 +135,11 @@ class NamedParametersOrderingFix extends ResolvedCorrectionProducer { /// /// 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<_ParamBlock> _extractParamBlocks( List namedParams, FormalParameterList parameterList, ) { final blocks = <_ParamBlock>[]; - int? firstStart; final lowerBound = parameterList.leftDelimiter?.end ?? parameterList.leftParenthesis.end; @@ -149,62 +149,35 @@ class NamedParametersOrderingFix extends ResolvedCorrectionProducer { for (int i = 0; i < namedParams.length; i++) { final param = namedParams[i]; - final int minOffset = i == 0 ? lowerBound : namedParams[i - 1].end; - // Find leading comment, skipping any trailing comment that belongs - // to the previous parameter (same line as previous param). - var blockStart = param.offset; - Token? leadingComment = param.beginToken.precedingComments; - if (i > 0) { - while (leadingComment != null) { - final betweenText = utils.getTextRange( - namedParams[i - 1].end, - leadingComment.offset, - ); - if (!betweenText.contains('\n')) { - leadingComment = leadingComment.next; - } else { - break; - } - } - } - if (leadingComment != null && - leadingComment.offset >= minOffset && - leadingComment.offset < param.offset) { - blockStart = leadingComment.offset; - } - final lineStart = utils.getLineThis(blockStart); - final prefixText = utils.getTextRange(lineStart, blockStart); - if (prefixText.trim().isEmpty) { - blockStart = lineStart; - } - - // Find trailing comment on the same line as this parameter. - String? trailingComment; final nextParamStart = i < namedParams.length - 1 ? namedParams[i + 1].offset : upperBound; - if (param.end < nextParamStart) { - final afterParam = utils.getTextRange(param.end, nextParamStart); - final sameLine = afterParam.split('\n').first; - final commentIdx = sameLine.indexOf('//'); - if (commentIdx != -1) { - trailingComment = sameLine.substring(commentIdx); - } - } + final previousParamEnd = i > 0 ? namedParams[i - 1].end : null; + + final leadingComment = utils.getLeadingComment( + node: param, + previousEnd: previousParamEnd, + ); + final blockStart = utils.getDeclarationStartOffset( + node: param, + leadingComment: leadingComment, + minOffset: minOffset, + ); + final trailingComment = utils.getTrailingComment( + node: param, + nextOffset: nextParamStart, + ); - firstStart ??= blockStart; blocks.add(( - text: utils.getTextRange(blockStart, param.end), + blockStart: blockStart, + blockEnd: param.end, trailingComment: trailingComment, )); } - return ( - blocks: blocks, - firstBlockStart: firstStart ?? namedParams.first.offset, - ); + return blocks; } List _getParametersOrder() { diff --git a/lib/src/utils/correction_utils.dart b/lib/src/utils/correction_utils.dart index ba68723e..80bfe87c 100644 --- a/lib/src/utils/correction_utils.dart +++ b/lib/src/utils/correction_utils.dart @@ -1,8 +1,10 @@ import 'package:analysis_server_plugin/edit/correction_utils.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; import 'package:analyzer/source/source_range.dart'; /// Extension on [CorrectionUtils] to provide utilities for creating -/// [SourceRange]s. +/// [SourceRange]s and parsing comments. extension CorrectionUtilsExtension on CorrectionUtils { /// Creates a [SourceRange] from [start] and [end] offsets. SourceRange createRange(int start, int end) => @@ -11,4 +13,59 @@ extension CorrectionUtilsExtension on CorrectionUtils { /// Returns the text of the range from [start] to [end]. String getTextRange(int start, int end) => getRangeText(createRange(start, end)); + + /// Finds trailing comment on the same line as this [node]. + String? getTrailingComment({ + required AstNode node, + required int nextOffset, + }) { + if (node.end >= nextOffset) return null; + + final sameLine = getTextRange(node.end, nextOffset) + .split('\n') + .first; + + final commentIdx = sameLine.indexOf('//'); + + return commentIdx != -1 ? sameLine.substring(commentIdx) : null; + } + + /// Finds leading comment, skipping any trailing comment that belongs + /// to the previous node (same line as previous node). + Token? getLeadingComment({ + required AstNode node, + int? previousEnd, + }) { + Token? comment = node.beginToken.precedingComments; + if (previousEnd == null) return comment; + + for (; comment != null; comment = comment.next) { + if (getTextRange(previousEnd, comment.offset).contains('\n')) { + return comment; + } + } + + return null; + } + + /// Computes the start offset of a [node]'s declaration, + /// including leading comments and indentation if applicable. + int getDeclarationStartOffset({ + required AstNode node, + required Token? leadingComment, + required int minOffset, + }) { + final hasValidComment = leadingComment != null && + leadingComment.offset >= minOffset && + leadingComment.offset < node.offset; + + final blockStart = hasValidComment ? leadingComment.offset : node.offset; + + final lineStart = getLineThis(blockStart); + final prefixText = getTextRange(lineStart, blockStart); + + final isLineStartWhitespace = prefixText.trim().isEmpty; + + return isLineStartWhitespace ? lineStart : blockStart; + } }