refactor: migrate no_magic_number rule by solid-illiaaihistov · Pull Request #300 · solid-software/solid_lints

solid-illiaaihistov · 2026-06-24T15:21:14Z

Closes #255

gemini-code-assist

Code Review

This pull request migrates the no_magic_number lint rule to the new custom_lint API structure, introducing a new NoMagicNumberRuleVisitor to replace the old recursive visitor, adding safe configuration parsing, and introducing comprehensive unit tests. The review feedback suggests two valuable improvements: first, to make configuration parsing more robust against invalid types by replacing as bool? with a safe == true comparison; second, to prevent false positives by ignoring magic numbers located inside annotations.

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.

…tion parsing

solid-illiaaihistov · 2026-06-24T15:41:37Z

/gemini review

gemini-code-assist

Code Review

This pull request migrates the no_magic_number lint rule to the new analyzer rule system, refactoring the logic into NoMagicNumberRuleVisitor and adding comprehensive tests. The reviewer identified a bug in _isNotInsideIndexExpression where negative numbers inside index expressions (e.g., list[-42]) would be incorrectly flagged as magic numbers because l.parent is used instead of _effectiveParent(l). Suggestions were provided to use _effectiveParent(l) and to add a corresponding test case.

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.

…mber rule

solid-illiaaihistov · 2026-06-24T15:49:35Z

/gemini review

gemini-code-assist

Code Review

This pull request refactors the no_magic_number lint rule to use the new analyzer APIs, delegating AST traversal to a new NoMagicNumberRuleVisitor and adding comprehensive unit tests. Feedback on the changes suggests extending the collection literal check in the visitor to also ignore RecordLiteral nodes, preventing false positives when using Dart 3 records.

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.

…elds

solid-illiaaihistov · 2026-06-24T15:56:31Z

/gemini review

gemini-code-assist

Code Review

This pull request registers and refactors the no_magic_number lint rule to use the new rule-visitor architecture. Specifically, it introduces NoMagicNumberRuleVisitor to handle AST traversal, refactors NoMagicNumberRule to delegate to this visitor, updates NoMagicNumberParameters with safe JSON parsing and an empty factory, and adds a comprehensive test suite covering various magic number scenarios. There are no review comments, and I have no additional feedback to provide.

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.

… legacy integration tests

solid-illiaaihistov · 2026-06-25T09:18:29Z

/gemini review

gemini-code-assist

Code Review

This pull request refactors the no_magic_number lint rule to align with the updated custom lint framework, introducing NoMagicNumberRuleVisitor to handle AST node processing and adding comprehensive unit tests. The feedback suggests improving the _effectiveParent helper method in the visitor to recursively skip parenthesized expressions and unary prefix expressions, preventing potential false positives when magic numbers are wrapped in parentheses.

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.

solid-danylosafonov

I think we can refactor the Visitor a bit

solid-danylosafonov · 2026-06-25T11:46:10Z

+    if (!_isMagicNumber(node)) return;
+    if (!_isNotInsideVariable(node)) return;
+    if (!_isNotInsideCollectionLiteral(node)) return;
+    if (!_isNotInsideConstConstructor(node)) return;
+    if (!_isNotInDateTime(node)) return;
+    if (!_isNotInsideIndexExpression(node)) return;
+    if (!_isNotInsideEnumConstantArguments(node)) return;
+    if (!_isNotDefaultValue(node)) return;
+    if (!_isNotInConstructorInitializer(node)) return;
+    if (!_isNotWidgetParameter(node)) return;


why don't we invert them?

Suggested change

if (!_isMagicNumber(node)) return;

if (!_isNotInsideVariable(node)) return;

if (!_isNotInsideCollectionLiteral(node)) return;

if (!_isNotInsideConstConstructor(node)) return;

if (!_isNotInDateTime(node)) return;

if (!_isNotInsideIndexExpression(node)) return;

if (!_isNotInsideEnumConstantArguments(node)) return;

if (!_isNotDefaultValue(node)) return;

if (!_isNotInConstructorInitializer(node)) return;

if (!_isNotWidgetParameter(node)) return;

if (_isNotMagicNumber(node)) return;

if (_isInsideVariable(node)) return;

if (_isInsideCollectionLiteral(node)) return;

if (_isInsideConstConstructor(node)) return;

if (_isInDateTime(node)) return;

if (_isInsideIndexExpression(node)) return;

if (_isInsideEnumConstantArguments(node)) return;

if (_isDefaultValue(node)) return;

if (_isInConstructorInitializer(node)) return;

if (_isWidgetParameter(node)) return;

solid-danylosafonov · 2026-06-25T11:48:29Z

+  bool _isNotDefaultValue(Literal literal) {
+    return literal.thisOrAncestorOfType<DefaultFormalParameter>() == null;
+  }
+
+  bool _isNotInConstructorInitializer(Literal literal) {
+    return literal.thisOrAncestorOfType<ConstructorInitializer>() == null;
+  }


I think we can extract the methods like those (that aren't dependent on _rule or _parameters) to an extension and put it somewhere in utils folder

I've moved the methods that look reusable into an extension.

… magic number validation checks

solid-danylosafonov

lgtm

refactor: migrate no_magic_number rule

b40bee6

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

Migrate no_magic_number #255

Open

gemini-code-assist Bot reviewed Jun 24, 2026

View reviewed changes

Comment thread lib/src/lints/no_magic_number/models/no_magic_number_parameters.dart Outdated

Comment thread lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart Outdated

fix: ignore magic numbers in annotations and update boolean configura…

95ac8e1

…tion parsing

gemini-code-assist Bot reviewed Jun 24, 2026

View reviewed changes

Comment thread lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart Outdated

Comment thread test/src/lints/no_magic_number/no_magic_number_rule_test.dart

fix: allow negative numbers in list index expressions for no_magic_nu…

e31a5d9

…mber rule

gemini-code-assist Bot reviewed Jun 24, 2026

View reviewed changes

Comment thread lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart Outdated

feat: ignore magic numbers within record literals and named record fi…

cc0e0cf

…elds

gemini-code-assist Bot reviewed Jun 24, 2026

View reviewed changes

refactor: migrate no_magic_number test cases to unit tests and remove…

586fcae

… legacy integration tests

gemini-code-assist Bot reviewed Jun 25, 2026

View reviewed changes

Comment thread lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart

solid-danylosafonov reviewed Jun 25, 2026

View reviewed changes

refactor: move AST inspection logic to AstNode extension and simplify…

78b275c

… magic number validation checks

solid-danylosafonov approved these changes Jun 25, 2026

View reviewed changes

Uh oh!

Conversation

solid-illiaaihistov commented Jun 24, 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 Jun 24, 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 Jun 24, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

solid-illiaaihistov commented Jun 24, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

solid-illiaaihistov commented Jun 25, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

solid-danylosafonov left a comment

Choose a reason for hiding this comment

Uh oh!

solid-danylosafonov Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

solid-illiaaihistov Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

solid-danylosafonov Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

solid-illiaaihistov Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

solid-danylosafonov left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants