refactor: migrate named_parameters_ordering by solid-illiaaihistov · Pull Request #298 · solid-software/solid_lints

solid-illiaaihistov · 2026-06-23T15:26:08Z

Closes #253

solid-illiaaihistov · 2026-06-23T15:27:04Z

/gemini review

gemini-code-assist

Code Review

This pull request implements the named_parameters_ordering lint rule and its corresponding quick fix to enforce consistent ordering of named parameters. The feedback highlights a critical bug where omitting parameter types in custom configurations leads to sorting errors, and another bug where the quick fix deletes trailing comments. Additionally, the reviewer suggests reusing the configuration parser in the quick fix to prevent duplicate logic and removing an unreachable case in the parameter classification switch statement.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-06-23T15:28:06Z

  static List<ParameterType> parseOrder(Object? orderConfig) {
-    final order = orderConfig is Iterable
-        ? List<String>.from(orderConfig)
-        : _defaultOrderList;
+    if (orderConfig is! Iterable) {
+      return ParameterType.defaultOrder;
+    }

+    final order = List<String>.from(orderConfig);
    return order.map(ParameterType.fromType).nonNulls.toList();
  }


Missing Parameter Types Bug in Custom Configuration

If a user configures a custom order in analysis_options.yaml but omits some of the parameter types (e.g., only specifying required and nullable), the omitted types will not be present in the parsed _parametersOrder list.

As a result, _parametersOrder.indexOf(type) will return -1 for those omitted types. This causes severe bugs:

In _isOrderingWrong, -1 is compared with other indices, leading to incorrect lint reports (e.g., reporting that a default parameter should be before a required parameter because -1 < 0).

In the quick fix, sorting will place omitted types at the very beginning of the parameter list.

To fix this, we should append any missing parameter types (from ParameterType.defaultOrder) to the end of the parsed list so that every ParameterType has a valid, non-negative index.

static List<ParameterType> parseOrder(Object? orderConfig) { if (orderConfig is! Iterable) { return ParameterType.defaultOrder; } final order = List<String>.from(orderConfig); final parsed = order.map(ParameterType.fromType).nonNulls.toList(); final missing = ParameterType.defaultOrder.where((type) => !parsed.contains(type)); return [...parsed, ...missing]; }

gemini-code-assist · 2026-06-23T15:28:06Z

+  ({List<String> blockTexts, int firstBlockStart}) _extractParamBlocks(
+    List<FormalParameter> namedParams,
+    FormalParameterList parameterList,
+  ) {
+    final blocks = <String>[];
+    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,
+    );
+  }


Trailing Comments Deletion Bug

When extracting parameter blocks in _extractParamBlocks, the block end is determined by param.end. However, any trailing comments on the same line (e.g., required super.accountType, // some comment) occur after param.end but before the next parameter's start.

Because the quick fix replaces the entire range from firstBlockStart to namedParams.last.end, any trailing comments on the same line as a parameter will be completely deleted when the fix is applied.

To fix this, you should either:

Detect if there are any trailing comments on the same line as any of the named parameters and abort the quick fix if so.

Or, adjust the block extraction logic to find the end of the line (or the start of the next parameter's leading comments/whitespace) and handle the commas carefully to preserve trailing comments.

gemini-code-assist · 2026-06-23T15:28:06Z

+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';


Import NamedParametersConfigParser to reuse the configuration parsing logic and ensure consistent behavior between the lint rule and the quick fix.

Suggested change

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';

import 'package:solid_lints/src/lints/named_parameters_ordering/config_parser.dart';

import 'package:solid_lints/src/lints/named_parameters_ordering/models/parameter_type.dart';

import 'package:solid_lints/src/lints/named_parameters_ordering/named_parameters_ordering_rule.dart';

import 'package:yaml/yaml.dart';

gemini-code-assist · 2026-06-23T15:28:06Z

+            final order = orderList
+                .map((e) => e is String ? ParameterType.fromType(e) : null)
+                .whereType<ParameterType>()
+                .toList();
+            if (order.isNotEmpty) {
+              return order;
+            }


Reuse NamedParametersConfigParser.parseOrder to parse the custom order list. This avoids duplicating the parsing logic and ensures that any omitted parameter types are correctly appended to the end of the list.

Suggested change

final order = orderList

.map((e) => e is String ? ParameterType.fromType(e) : null)

.whereType<ParameterType>()

.toList();

if (order.isNotEmpty) {

return order;

}

final order = NamedParametersConfigParser.parseOrder(orderList);

if (order.isNotEmpty) {

return order;

}

gemini-code-assist · 2026-06-23T15:28:06Z

+      case DefaultFormalParameter():
+      case _ when hasDefaultValue:
+        return ParameterType.defaultValue;


Unreachable Case in Switch Statement

The case DefaultFormalParameter(): is unreachable because any DefaultFormalParameter is already unwrapped and handled recursively by the if statement at the beginning of the method:

if (parameter is DefaultFormalParameter && ...)

You can safely remove this case to keep the switch statement clean and maintainable.

case _ when hasDefaultValue: return ParameterType.defaultValue;

gemini-code-assist

Code Review

This pull request introduces a new quick fix (NamedParametersOrderingFix) and refactors the rule and visitor (NamedParametersOrderingRule, NamedParametersOrderingVisitor) to enforce and automatically sort named parameters in Dart. It also adds comprehensive unit tests. The review feedback highlights three critical issues: first, unlisted parameter types in the sorting configuration can cause indexOf to return -1, leading to incorrect sorting in the quick fix; second, a similar issue in the visitor's ordering check results in incorrect lint reports; and third, extracting parameter blocks with comments can incorrectly group trailing comments from previous lines, potentially corrupting the code replacement.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-06-23T15:30:28Z

+    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);
+    });


When sorting the named parameters, if a parameter type is not present in the configured parametersOrder list, indexOf will return -1. This causes unlisted parameter types to be sorted to the very beginning of the list, which is counter-intuitive and contradicts the expected behavior (where unlisted types should ideally be placed at the end).

To fix this, we should treat -1 as a large index (e.g., parametersOrder.length) so that unlisted parameters are consistently sorted to the end.

Suggested change

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);

});

sortedNamedParams.sort((a, b) {

final typeA = ParameterType.fromParameter(a);

final typeB = ParameterType.fromParameter(b);

var indexA = parametersOrder.indexOf(typeA);

var indexB = parametersOrder.indexOf(typeB);

if (indexA == -1) indexA = parametersOrder.length;

if (indexB == -1) indexB = parametersOrder.length;

return indexA.compareTo(indexB);

});

gemini-code-assist · 2026-06-23T15:30:28Z

+      final isWrong = _isOrderingWrong(parameterType, previousParameterType);
+
+      if (isWrong && previousParameterType != null) {


The helper method _isOrderingWrong (defined on line 70) compares the indices of previousParameterType and currentParameterType in _parametersOrder using indexOf.

If any of these types are not present in the configured _parametersOrder list, indexOf returns -1. This leads to incorrect lint reports (e.g., treating unlisted parameters as if they must be placed at the very beginning of the list).

Please update _isOrderingWrong to treat -1 as _parametersOrder.length so that unlisted parameter types are consistently treated as being at the end of the order:

bool _isOrderingWrong( ParameterType currentParameterType, ParameterType? previousParameterType, ) { if (previousParameterType == null) return false; var indexPrev = _parametersOrder.indexOf(previousParameterType); var indexCurr = _parametersOrder.indexOf(currentParameterType); if (indexPrev == -1) indexPrev = _parametersOrder.length; if (indexCurr == -1) indexCurr = _parametersOrder.length; return indexPrev > indexCurr; }

gemini-code-assist · 2026-06-23T15:30:28Z

+      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;


There is a potential bug when extracting parameter blocks with comments.

If a comment is a trailing comment on the same line as the previous parameter (e.g., required super.accountType, // comment), it will be treated as a preceding comment of the current parameter (this.age) because there are no other tokens between them.

This causes blockStart for the current parameter to be set to the comment's offset on the previous line, resulting in overlapping blocks and corrupted code replacement.

To prevent this, we should only associate a preceding comment with the current parameter if there is a newline between the end of the previous parameter and the start of the comment. Additionally, we can use utils.getLineStart(blockStart) to safely and cleanly find the start of the line instead of manual subtraction.

Suggested change

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;

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) {

final textBetween = utils.getRangeText(SourceRange(minOffset, comment.offset - minOffset));

if (textBetween.contains('\\n')) {

blockStart = comment.offset;

}

}

blockStart = utils.getLineStart(blockStart);

refactor: migrate named_parameters_ordering

2e39856

solid-illiaaihistov linked an issue Jun 23, 2026 that may be closed by this pull request

Migrate named_parameters_ordering #253

Open

gemini-code-assist Bot reviewed Jun 23, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: migrate named_parameters_ordering#298

refactor: migrate named_parameters_ordering#298
solid-illiaaihistov wants to merge 1 commit into
solid-software:analysis_server_migrationfrom
solid-illiaaihistov:253-migrate-named_parameters_ordering

solid-illiaaihistov commented Jun 23, 2026

Uh oh!

solid-illiaaihistov commented Jun 23, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		final isWrong = _isOrderingWrong(parameterType, previousParameterType);

		if (isWrong && previousParameterType != null) {

Conversation

solid-illiaaihistov commented Jun 23, 2026

Uh oh!

solid-illiaaihistov commented Jun 23, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Choose a reason for hiding this comment

Missing Parameter Types Bug in Custom Configuration

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Choose a reason for hiding this comment

Trailing Comments Deletion Bug

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Choose a reason for hiding this comment

Unreachable Case in Switch Statement

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant