Add new rule avoid_similar_names by solid-illiaaihistov · Pull Request #321 · solid-software/solid_lints

solid-illiaaihistov · 2026-07-16T07:32:24Z

Closes #95

… in avoid_similar_names lint

gemini-code-assist

Code Review

This pull request introduces a new lint rule, avoid_similar_names, which warns about variables or parameters with confusingly similar names within the same function scope. It includes the rule implementation, AST visitors, tokenization utilities, and corresponding tests. The feedback highlights three key areas for improvement: refining the tokenization regular expression to correctly handle acronyms followed by capitalized words, extending LocalVariablesVisitor to collect variables from for-in loops, and adjusting the type comparison logic to allow comparisons between nullable and non-nullable versions of the same type.

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.

…ort camelCase suffixes

…e collection logic

…es when comparing variable types

solid-illiaaihistov · 2026-07-16T08:22:10Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a new lint rule avoid_similar_names to detect confusingly similar variable and parameter names within the same scope. The feedback highlights two important improvements: changing the visitor base class to SimpleAstVisitor to avoid double-traversal performance issues, and extending LocalVariablesVisitor to support Dart 3.0 pattern variable declarations.

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.

… in AvoidSimilarNamesVisitor

…larations

solid-illiaaihistov · 2026-07-16T08:48:16Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a new lint rule, avoid_similar_names, which warns about variables or parameters with confusingly similar names within the same function scope. Feedback on the changes highlights a potential memory leak in AvoidSimilarNamesVisitor due to _reportedNodes never being cleared, and suggests registering FunctionExpression instead of FunctionDeclaration to support analyzing anonymous closures as well as named functions.

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.

…tale lint reports

solid-illiaaihistov · 2026-07-16T08:58:04Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a new lint rule, avoid_similar_names, to detect variables or parameters with confusingly similar names within the same function scope. The feedback recommends adding an explicit length check in isSubsetWithNonDescriptiveToken to prevent potential out-of-bounds errors. Additionally, it suggests registering and visiting FunctionExpression instead of FunctionDeclaration to extend the analysis to anonymous closures.

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.

… adding related tests

danylo-safonov-solid · 2026-07-16T17:28:34Z

  }
+
+  /// Compares this type with [other] ignoring nullability where applicable.
+  bool isDifferentIgnoringNullability(DartType? other) {


other != null && (element ?? this) != (other.element ?? other)

danylo-safonov-solid · 2026-07-16T17:36:41Z

+    registry.addMethodDeclaration(this, visitor);
+    registry.addConstructorDeclaration(this, visitor);
+    registry.addFunctionDeclaration(this, visitor);


Suggested change

registry.addMethodDeclaration(this, visitor);

registry.addConstructorDeclaration(this, visitor);

registry.addFunctionDeclaration(this, visitor);

registry

..addMethodDeclaration(this, visitor)

..addConstructorDeclaration(this, visitor)

..addFunctionDeclaration(this, visitor);

danylo-safonov-solid · 2026-07-16T17:41:40Z

+  void _report(ScopeVariable a, ScopeVariable b) {
+    if (_reportedNodes.add(a.node)) {
+      _rule.reportAtToken(a.nameToken);
+    }
+    if (_reportedNodes.add(b.node)) {
+      _rule.reportAtToken(b.nameToken);
+    }
+  }


void _report(ScopeVariable a, ScopeVariable b) => [a, b] .where((e) => _reportedNodes.add(e.node)) .forEach((e) => _rule.reportAtToken(e.nameToken));

danylo-safonov-solid · 2026-07-16T17:43:31Z

+    final (longer, shorter) = a.tokens.length > b.tokens.length
+        ? (a.tokens, b.tokens)
+        : (b.tokens, a.tokens);
+
+    if (NameTokenizer.isSubsetWithNonDescriptiveToken(
+      longer,
+      shorter,
+    )) {


Suggested change

final (longer, shorter) = a.tokens.length > b.tokens.length

? (a.tokens, b.tokens)

: (b.tokens, a.tokens);

if (NameTokenizer.isSubsetWithNonDescriptiveToken(

longer,

shorter,

)) {

final [shorter, longer] = [a.tokens, b.tokens].sortedBy((e) => e.length);

if (NameTokenizer.isSubsetWithNonDescriptiveToken(longer, shorter)) {

danylo-safonov-solid · 2026-07-16T17:53:32Z

+    }
+  }
+
+  void _checkSameLengthTokens(


final diffs = a.tokens.zipWithIndexed(b.tokens).where((e) => e.$2 != e.$3); if (diffs.length >= 2 && NameTokenizer.isNonDescriptiveToken(a.tokens[diffs.first.$1]) && NameTokenizer.isNonDescriptiveToken(b.tokens[diffs.first.$1])) { _report(a, b); }

extension<T> on Iterable<T> { Iterable<(T, U)> zipWith<U>(Iterable<U> other) sync* { for (var i = 0; i < length && i < other.length; i++) { yield (elementAt(i), other.elementAt(i)); } } Iterable<(int, T, U)> zipWithIndexed<U>(Iterable<U> other) sync* { for (var i = 0; i < length && i < other.length; i++) { yield (i, elementAt(i), other.elementAt(i)); } } }

I replaced the length check with singleOrNull to leverage lazy evaluation (as calculating the diff length would require iterating over all elements). However, for typical variable name scenarios, performance still dropped by 5–10x (depending on the number of tokens in the variable name) compared to a standard loop that doesn't generate collections. I kept the version with zipWithIndexed.

final diff = a.tokens .zipWithIndexed(b.tokens) .where((e) => e.$2 != e.$3) .singleOrNull; if (diff != null && NameTokenizer.isNonDescriptiveToken(a.tokens[diff.$1]) && NameTokenizer.isNonDescriptiveToken(b.tokens[diff.$1])) { _report(a, b); }

5–10x

I think we shouldn't worry about values like that. If we worry that performance decreased dramatically it'd be better to compare runtime over a real code base (e.g. worklog)

danylo-safonov-solid · 2026-07-16T17:56:08Z

+  void _comparePair(
+    ScopeVariable a,
+    ScopeVariable b,
+  ) {
+    if (a.type?.isDifferentIgnoringNullability(b.type) ?? false) {
+      return;
+    }
+
+    switch ((a.tokens.length - b.tokens.length).abs()) {
+      case 0:
+        _checkSameLengthTokens(a, b);
+      case 1:
+        _checkDifferentLengthTokens(a, b);
+    }
+  }


Suggested change

void _comparePair(

ScopeVariable a,

ScopeVariable b,

) {

if (a.type?.isDifferentIgnoringNullability(b.type) ?? false) {

return;

}

switch ((a.tokens.length - b.tokens.length).abs()) {

case 0:

_checkSameLengthTokens(a, b);

case 1:

_checkDifferentLengthTokens(a, b);

}

}

void _comparePair(ScopeVariable a, ScopeVariable b) =>

a.type?.isDifferentIgnoringNullability(b.type) ?? false

? null

: a.tokens.length == b.tokens.length

? _checkSameLengthTokens(a, b)

: _checkDifferentLengthTokens(a, b);

Implemented the following approach:

void _comparePair(ScopeVariable a, ScopeVariable b) => a.type?.isDifferentIgnoringNullability(b.type) ?? false ? null : switch ((a.tokens.length - b.tokens.length).abs()) { 0 => _checkSameLengthTokens(a, b), 1 => _checkSingleTokenLengthDifference(a, b), _ => null, };

why not ternary chain?

a ? b : c ? d : e

is idiom for

if (a) { b; } else if (c) { d; } else { e; }

I think we should make the _checkX return bool instead though.
Then we can completely remove that ternary, something like:

void _comparePair(ScopeVariable a, ScopeVariable b) { final shouldReport = a.type?.isDifferentIgnoringNullability(b.type) != true && (a.tokens.length == b.tokens.length && _checkSameLengthTokens(a, b) || _checkSingleTokenLengthDifference(a, b)); if (shouldReport) _report(a, b); } bool _checkSameLengthTokens(ScopeVariable a, ScopeVariable b) { final diff = a.tokens .zipWithIndexed(b.tokens) .where((e) => e.$2 != e.$3) .singleOrNull ?.$1; return diff != null && NameTokenizer.isNonDescriptiveToken(a.tokens[diff]) && NameTokenizer.isNonDescriptiveToken(b.tokens[diff]); } bool _checkSingleTokenLengthDifference(ScopeVariable a, ScopeVariable b) { final [shorter, longer] = [a.tokens, b.tokens].sortedBy((e) => e.length); return NameTokenizer.isSubsetWithNonDescriptiveToken(longer, shorter); }

danylo-safonov-solid · 2026-07-21T15:18:40Z

+  void visitMethodDeclaration(
+    MethodDeclaration node,
+  ) {
+    _checkScope(node.parameters, node.body);
+  }


Suggested change

void visitMethodDeclaration(

MethodDeclaration node,

) {

_checkScope(node.parameters, node.body);

}

void visitMethodDeclaration(MethodDeclaration node) =>

_checkScope(node.parameters, node.body);

danylo-safonov-solid · 2026-07-21T15:18:53Z

+  void visitConstructorDeclaration(
+    ConstructorDeclaration node,
+  ) {
+    _checkScope(node.parameters, node.body);
+  }


Suggested change

void visitConstructorDeclaration(

ConstructorDeclaration node,

) {

_checkScope(node.parameters, node.body);

}

void visitConstructorDeclaration(ConstructorDeclaration node) =>

_checkScope(node.parameters, node.body);

danylo-safonov-solid · 2026-07-21T15:19:11Z

+  void visitFunctionDeclaration(
+    FunctionDeclaration node,
+  ) {
+    _checkScope(
+      node.functionExpression.parameters,
+      node.functionExpression.body,
+    );
+  }


Suggested change

void visitFunctionDeclaration(

FunctionDeclaration node,

) {

_checkScope(

node.functionExpression.parameters,

node.functionExpression.body,

);

}

void visitFunctionDeclaration(FunctionDeclaration node) => _checkScope(

node.functionExpression.parameters,

node.functionExpression.body,

);

danylo-safonov-solid · 2026-07-21T15:20:13Z

+    final variables = [
+      ..._extractParameters(parameters),
+      ..._collector.variables,
+    ];
+
+    _compareVariables(variables);


Suggested change

final variables = [

..._extractParameters(parameters),

..._collector.variables,

];

_compareVariables(variables);

_compareVariables([

..._extractParameters(parameters),

..._collector.variables,

]);

danylo-safonov-solid · 2026-07-21T15:20:34Z

+  void _compareVariables(
+    List<ScopeVariable> variables,
+  ) {


Suggested change

void _compareVariables(

List<ScopeVariable> variables,

) {

void _compareVariables(List<ScopeVariable> variables) {

danylo-safonov-solid · 2026-07-21T15:25:44Z

we can extract something like:

Future<void> _assert(String source) => assertAutoDiagnostics('''void test() {$source}''');

instead of

Future<void> f(...) async { await f2(...); }

we should do

Future<void> f(...) => f2(...);

… in avoid_similar_names visitor

solid-danylosafonov

LGTM!

…o 95-add-avoid_similar_names

Illia Aihistov added 2 commits July 15, 2026 18:32

feat: add avoid_similar_names lint rule

a2cb420

refactor: optimize variable collection and improve tokenization logic…

a5c0076

… in avoid_similar_names lint

gemini-code-assist Bot reviewed Jul 16, 2026

View reviewed changes

Comment thread lib/src/lints/avoid_similar_names/utils/name_tokenizer.dart Outdated

Comment thread lib/src/lints/avoid_similar_names/visitors/local_variables_visitor.dart

Comment thread lib/src/lints/avoid_similar_names/visitors/avoid_similar_names_visitor.dart Outdated

Illia Aihistov added 3 commits July 16, 2026 10:43

fix: improve acronym tokenization in avoid_similar_names lint to supp…

19674b7

…ort camelCase suffixes

refactor: support linting of declared identifiers and extract variabl…

c9e7bb3

…e collection logic

feat: update avoid_similar_names lint to ignore nullability differenc…

6c569e8

…es when comparing variable types

gemini-code-assist Bot reviewed Jul 16, 2026

View reviewed changes

Comment thread lib/src/lints/avoid_similar_names/visitors/avoid_similar_names_visitor.dart Outdated

Comment thread lib/src/lints/avoid_similar_names/visitors/local_variables_visitor.dart

Illia Aihistov added 2 commits July 16, 2026 11:40

refactor: switch to SimpleAstVisitor and remove redundant super calls…

8a1407d

… in AvoidSimilarNamesVisitor

feat: add support for detecting similar names in pattern variable dec…

19e9eee

…larations

gemini-code-assist Bot reviewed Jul 16, 2026

View reviewed changes

Comment thread lib/src/lints/avoid_similar_names/visitors/avoid_similar_names_visitor.dart

Comment thread lib/src/lints/avoid_similar_names/visitors/avoid_similar_names_visitor.dart Outdated

Comment thread lib/src/lints/avoid_similar_names/avoid_similar_names_rule.dart Outdated

fix: clear reported nodes in avoid similar names visitor to prevent s…

03fe8ef

…tale lint reports

gemini-code-assist Bot reviewed Jul 16, 2026

View reviewed changes

Comment thread lib/src/lints/avoid_similar_names/utils/name_tokenizer.dart

Comment thread lib/src/lints/avoid_similar_names/avoid_similar_names_rule.dart

Comment thread lib/src/lints/avoid_similar_names/visitors/avoid_similar_names_visitor.dart Outdated

feat: improve name similarity detection by refining NameTokenizer and…

01d9bfd

… adding related tests

danylo-safonov-solid reviewed Jul 16, 2026

View reviewed changes

refactor: simplify avoid_similar_names visitor logic

a2a0265

danylo-safonov-solid reviewed Jul 21, 2026

View reviewed changes

Illia Aihistov added 3 commits July 22, 2026 10:59

refactor: decouple lint reporting logic from token comparison methods…

6d4504e

… in avoid_similar_names visitor

refactor: simplify avoid_similar_names visitor methods

b53508d

refactor: simplify avoid_similar_names test suite using helper methods

1d20eeb

solid-danylosafonov approved these changes Jul 22, 2026

View reviewed changes

Merge remote-tracking branch 'upstream/analysis_server_migration' int…

60f2cb8

…o 95-add-avoid_similar_names

solid-illiaaihistov merged commit 24d6168 into solid-software:analysis_server_migration Jul 22, 2026
1 check passed

solid-illiaaihistov deleted the 95-add-avoid_similar_names branch July 22, 2026 13:21

Uh oh!

Conversation

solid-illiaaihistov commented Jul 16, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

solid-illiaaihistov commented Jul 16, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

solid-illiaaihistov commented Jul 16, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

solid-illiaaihistov commented Jul 16, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment