From b40bee6819fca5de565264d653b7714472d13f33 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 24 Jun 2026 18:16:00 +0300 Subject: [PATCH 1/6] refactor: migrate no_magic_number rule --- lib/main.dart | 4 + .../models/no_magic_number_parameters.dart | 25 +- .../no_magic_number/no_magic_number_rule.dart | 196 +++---------- .../no_magic_number_rule_visitor.dart | 124 ++++++++ .../visitors/no_magic_number_visitor.dart | 45 --- .../no_magic_number_rule_test.dart | 275 ++++++++++++++++++ 6 files changed, 455 insertions(+), 214 deletions(-) create mode 100644 lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart delete mode 100644 lib/src/lints/no_magic_number/visitors/no_magic_number_visitor.dart create mode 100644 test/src/lints/no_magic_number/no_magic_number_rule_test.dart diff --git a/lib/main.dart b/lib/main.dart index d9d338a8..76e7f3e5 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -14,6 +14,7 @@ import 'package:solid_lints/src/lints/cyclomatic_complexity/cyclomatic_complexit 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/function_lines_of_code/function_lines_of_code_rule.dart'; +import 'package:solid_lints/src/lints/no_magic_number/no_magic_number_rule.dart'; import 'package:solid_lints/src/lints/prefer_first/fixes/prefer_first_fix.dart'; import 'package:solid_lints/src/lints/prefer_first/prefer_first_rule.dart'; import 'package:solid_lints/src/lints/proper_super_calls/proper_super_calls_rule.dart'; @@ -65,6 +66,9 @@ class SolidLintsPlugin extends Plugin { ), UseNearestContextRule(), preferFirstRule, + NoMagicNumberRule( + analysisOptionsLoader: analysisLoader, + ), // TODO: Add more lint rules and use analysisLoader // for rules that need parameters // For example: `CyclomaticComplexityRule(analysisLoader)` diff --git a/lib/src/lints/no_magic_number/models/no_magic_number_parameters.dart b/lib/src/lints/no_magic_number/models/no_magic_number_parameters.dart index f23ab0bc..907996ec 100644 --- a/lib/src/lints/no_magic_number/models/no_magic_number_parameters.dart +++ b/lib/src/lints/no_magic_number/models/no_magic_number_parameters.dart @@ -47,12 +47,23 @@ class NoMagicNumberParameters { required this.allowedInWidgetParams, }); - /// Method for creating from json data - factory NoMagicNumberParameters.fromJson(Map json) => - NoMagicNumberParameters( - allowedNumbers: - json[_allowedConfigName] as Iterable? ?? _defaultMagicNumbers, - allowedInWidgetParams: - json[_allowedInWidgetParamsConfigName] as bool? ?? false, + /// Creates an empty/default instance of [NoMagicNumberParameters] + factory NoMagicNumberParameters.empty() => const NoMagicNumberParameters( + allowedNumbers: _defaultMagicNumbers, + allowedInWidgetParams: false, ); + + /// Method for creating from json data + factory NoMagicNumberParameters.fromJson(Map json) { + final allowedRaw = json[_allowedConfigName]; + final allowedList = allowedRaw is Iterable + ? allowedRaw.whereType().toList() + : _defaultMagicNumbers; + + return NoMagicNumberParameters( + allowedNumbers: allowedList, + allowedInWidgetParams: + json[_allowedInWidgetParamsConfigName] as bool? ?? false, + ); + } } diff --git a/lib/src/lints/no_magic_number/no_magic_number_rule.dart b/lib/src/lints/no_magic_number/no_magic_number_rule.dart index a1355219..57e5e9f9 100644 --- a/lib/src/lints/no_magic_number/no_magic_number_rule.dart +++ b/lib/src/lints/no_magic_number/no_magic_number_rule.dart @@ -1,34 +1,8 @@ -// MIT License -// -// Copyright (c) 2020-2021 Dart Code Checker team -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -import 'package:analyzer/dart/ast/ast.dart'; -import 'package:analyzer/dart/element/type.dart'; -import 'package:analyzer/error/listener.dart'; -import 'package:collection/collection.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/no_magic_number/models/no_magic_number_parameters.dart'; -import 'package:solid_lints/src/lints/no_magic_number/visitors/no_magic_number_visitor.dart'; -import 'package:solid_lints/src/models/rule_config.dart'; +import 'package:solid_lints/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart'; import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// A `no_magic_number` rule which forbids having numbers without variable @@ -43,11 +17,12 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// ### Example config: /// /// ```yaml -/// custom_lint: -/// rules: -/// - no_magic_number: -/// allowed: [12, 42] -/// allowed_in_widget_params: true +/// plugins: +/// solid_lints: +/// diagnostics: +/// no_magic_number: +/// allowed: [12, 42] +/// allowed_in_widget_params: true /// ``` /// /// ### Example @@ -88,7 +63,7 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// const Circle({required this.r}); /// } /// const Circle(r: 5); -/// const circle = Circle(r: 10) +/// const circle = Circle(r: 10); /// ``` /// /// ### Allowed @@ -138,139 +113,36 @@ class NoMagicNumberRule extends SolidLintRule { /// the error when having magic number. static const String lintName = 'no_magic_number'; - NoMagicNumberRule._(super.config); - - /// Creates a new instance of [NoMagicNumberRule] - /// based on the lint configuration. - factory NoMagicNumberRule.createRule(CustomLintConfigs configs) { - final config = RuleConfig( - configs: configs, - name: lintName, - paramsParser: NoMagicNumberParameters.fromJson, - problemMessage: (_) => 'Avoid using magic numbers.' - 'Extract them to named constants or variables.', - ); - - return NoMagicNumberRule._(config); - } + static const _code = LintCode( + lintName, + 'Avoid using magic numbers. Extract them to named constants or variables.', + ); @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, - ) { - context.registry.addCompilationUnit((node) { - final visitor = NoMagicNumberVisitor(); - node.accept(visitor); - - final magicNumbers = visitor.literals - .where(_isMagicNumber) - .where(_isNotInsideVariable) - .where(_isNotInsideCollectionLiteral) - .where(_isNotInsideConstMap) - .where(_isNotInsideConstConstructor) - .where(_isNotInDateTime) - .where(_isNotInsideIndexExpression) - .where(_isNotInsideEnumConstantArguments) - .where(_isNotDefaultValue) - .where(_isNotInConstructorInitializer) - .where(_isNotWidgetParameter); - - for (final magicNumber in magicNumbers) { - reporter.atNode(magicNumber, code); - } - }); - } - - bool _isMagicNumber(Literal l) => - (l is DoubleLiteral && - !config.parameters.allowedNumbers.contains(l.value)) || - (l is IntegerLiteral && - !config.parameters.allowedNumbers.contains(l.value)); - - bool _isNotInsideVariable(Literal l) { - // Whether we encountered such node, - // This is tracked because [InstanceCreationExpression] can be - // inside [VariableDeclaration] removing unwanted literals - - bool isInstanceCreationExpression = false; - return l.thisOrAncestorMatching((ancestor) { - if (ancestor is InstanceCreationExpression) { - isInstanceCreationExpression = true; - } - if (isInstanceCreationExpression) { - return false; - } else { - return ancestor is VariableDeclaration; - } - }) == - null; - } - - bool _isNotInDateTime(Literal l) => - l.thisOrAncestorMatching( - (a) => - a is InstanceCreationExpression && - a.staticType?.getDisplayString() == 'DateTime', - ) == - null; - - bool _isNotInsideEnumConstantArguments(Literal l) { - final node = l.thisOrAncestorMatching( - (ancestor) => ancestor is EnumConstantArguments, - ); - - return node == null; - } + DiagnosticCode get diagnosticCode => _code; - bool _isNotInsideCollectionLiteral(Literal l) => l.parent is! TypedLiteral; - - bool _isNotInsideConstMap(Literal l) { - final grandParent = l.parent?.parent; - - return !(grandParent is SetOrMapLiteral && grandParent.isConst); - } - - bool _isNotInsideConstConstructor(Literal l) => - l.thisOrAncestorMatching((ancestor) { - return ancestor is InstanceCreationExpression && ancestor.isConst; - }) == - null; - - bool _isNotInsideIndexExpression(Literal l) => l.parent is! IndexExpression; - - bool _isNotDefaultValue(Literal literal) { - return literal.thisOrAncestorOfType() == null; - } - - bool _isNotInConstructorInitializer(Literal literal) { - return literal.thisOrAncestorOfType() == null; - } - - bool _isNotWidgetParameter(Literal literal) { - if (!config.parameters.allowedInWidgetParams) return true; - - final widgetCreationExpression = literal.thisOrAncestorMatching( - _isWidgetCreationExpression, - ); - - return widgetCreationExpression == null; - } + /// Creates a new instance of [NoMagicNumberRule] + NoMagicNumberRule({ + required super.analysisOptionsLoader, + }) : super.withParameters( + name: lintName, + description: 'Forbids having numbers without variable.', + parametersParser: NoMagicNumberParameters.fromJson, + ); - bool _isWidgetCreationExpression( - AstNode node, + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, ) { - if (node is! InstanceCreationExpression) return false; - - final staticType = node.staticType; + super.registerNodeProcessors(registry, context); - if (staticType is! InterfaceType) return false; + final parameters = + getParametersForContext(context) ?? NoMagicNumberParameters.empty(); - final widgetSupertype = staticType.allSupertypes.firstWhereOrNull( - (supertype) => supertype.getDisplayString() == 'Widget', - ); + final visitor = NoMagicNumberRuleVisitor(this, parameters); - return widgetSupertype != null; + registry.addDoubleLiteral(this, visitor); + registry.addIntegerLiteral(this, visitor); } } diff --git a/lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart b/lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart new file mode 100644 index 00000000..a201be5d --- /dev/null +++ b/lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart @@ -0,0 +1,124 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/type.dart'; +import 'package:collection/collection.dart'; +import 'package:solid_lints/src/lints/no_magic_number/models/no_magic_number_parameters.dart'; +import 'package:solid_lints/src/models/solid_lint_rule.dart'; + +/// The AST visitor that checks if double and integer literals are magic numbers. +class NoMagicNumberRuleVisitor extends SimpleAstVisitor { + final SolidLintRule _rule; + final NoMagicNumberParameters _parameters; + + /// Creates a new instance of [NoMagicNumberRuleVisitor]. + NoMagicNumberRuleVisitor(this._rule, this._parameters); + + @override + void visitDoubleLiteral(DoubleLiteral node) { + _checkLiteral(node); + } + + @override + void visitIntegerLiteral(IntegerLiteral node) { + _checkLiteral(node); + } + + void _checkLiteral(Literal node) { + 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; + + _rule.reportAtNode(node); + } + + bool _isMagicNumber(Literal l) => + (l is DoubleLiteral && !_parameters.allowedNumbers.contains(l.value)) || + (l is IntegerLiteral && !_parameters.allowedNumbers.contains(l.value)); + + /// Returns the effective parent of [l], skipping over a unary + /// [PrefixExpression] (e.g. the `-` in `-42`). + AstNode? _effectiveParent(Literal l) => + l.parent is PrefixExpression ? l.parent?.parent : l.parent; + + /// Returns `true` if the literal is NOT the direct initializer of a variable + /// declaration. + /// + /// Allows `var x = 42` (literal gets a name) but reports `var r = x + 42` + /// (42 is a magic number inside an expression). + bool _isNotInsideVariable(Literal l) => + _effectiveParent(l) is! VariableDeclaration; + + bool _isNotInDateTime(Literal l) => + l.thisOrAncestorMatching( + (a) => + a is InstanceCreationExpression && + a.staticType?.getDisplayString() == 'DateTime', + ) == + null; + + bool _isNotInsideEnumConstantArguments(Literal l) { + final node = l.thisOrAncestorMatching( + (ancestor) => ancestor is EnumConstantArguments, + ); + return node == null; + } + + /// Returns `true` if the literal is NOT inside a collection literal. + /// + /// Covers list elements (`[42]`, `[-42]`), set elements (`{42}`, `{-42}`), + /// and map keys/values (`{42: 'a'}`, `{-42: 'a'}`, `{'a': -42}`). + /// + /// Unary prefix expressions (e.g. `-42`) are skipped transparently so that + /// the effective parent—the collection literal—is checked instead of the + /// intermediate [PrefixExpression] node. + bool _isNotInsideCollectionLiteral(Literal l) { + final p = _effectiveParent(l); + return p is! TypedLiteral && p is! MapLiteralEntry; + } + + bool _isNotInsideConstConstructor(Literal l) => + l.thisOrAncestorMatching((ancestor) { + return ancestor is InstanceCreationExpression && ancestor.isConst; + }) == + null; + + bool _isNotInsideIndexExpression(Literal l) => l.parent is! IndexExpression; + + bool _isNotDefaultValue(Literal literal) { + return literal.thisOrAncestorOfType() == null; + } + + bool _isNotInConstructorInitializer(Literal literal) { + return literal.thisOrAncestorOfType() == null; + } + + bool _isNotWidgetParameter(Literal literal) { + if (!_parameters.allowedInWidgetParams) return true; + + final widgetCreationExpression = literal.thisOrAncestorMatching( + _isWidgetCreationExpression, + ); + + return widgetCreationExpression == null; + } + + bool _isWidgetCreationExpression(AstNode node) { + if (node is! InstanceCreationExpression) return false; + + final staticType = node.staticType; + if (staticType is! InterfaceType) return false; + + final widgetSupertype = staticType.allSupertypes.firstWhereOrNull( + (supertype) => supertype.getDisplayString() == 'Widget', + ); + + return widgetSupertype != null; + } +} diff --git a/lib/src/lints/no_magic_number/visitors/no_magic_number_visitor.dart b/lib/src/lints/no_magic_number/visitors/no_magic_number_visitor.dart deleted file mode 100644 index 7bec43f6..00000000 --- a/lib/src/lints/no_magic_number/visitors/no_magic_number_visitor.dart +++ /dev/null @@ -1,45 +0,0 @@ -// MIT License -// -// Copyright (c) 2020-2021 Dart Code Checker team -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -import 'package:analyzer/dart/ast/ast.dart'; -import 'package:analyzer/dart/ast/visitor.dart'; - -/// The AST visitor that will find all double and integer literals -class NoMagicNumberVisitor extends RecursiveAstVisitor { - final _literals = []; - - /// List of all double and integer literals - Iterable get literals => _literals; - - @override - void visitDoubleLiteral(DoubleLiteral node) { - _literals.add(node); - super.visitDoubleLiteral(node); - } - - @override - void visitIntegerLiteral(IntegerLiteral node) { - _literals.add(node); - super.visitIntegerLiteral(node); - } -} diff --git a/test/src/lints/no_magic_number/no_magic_number_rule_test.dart b/test/src/lints/no_magic_number/no_magic_number_rule_test.dart new file mode 100644 index 00000000..50cd8c22 --- /dev/null +++ b/test/src/lints/no_magic_number/no_magic_number_rule_test.dart @@ -0,0 +1,275 @@ +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/no_magic_number/no_magic_number_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../../../lints/auto_test_lint_offsets.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(NoMagicNumberRuleTest); + }); +} + +@reflectiveTest +class NoMagicNumberRuleTest extends AnalysisRuleTest with AutoTestLintOffsets { + static const _importFlutterWidgets = "import 'package:flutter/widgets.dart';"; + static const _mockFlutterWidgetsContent = ''' +abstract class Widget { + const Widget(); +} + +class SizedBox extends Widget { + final double? width; + final Widget? child; + const SizedBox({this.width, this.child}); +} +'''; + + @override + void setUp() { + rule = NoMagicNumberRule( + analysisOptionsLoader: AnalysisOptionsLoader( + resourceProvider: resourceProvider, + ), + ); + newPackage('flutter') + ..addFile('lib/widgets.dart', _mockFlutterWidgetsContent); + super.setUp(); + } + + Future test_reports_magic_number_in_expression() async { + await assertAutoDiagnostics(''' +int fn(int b) { + return b + ${expectLint('42')}; +} +'''); + } + + Future test_does_not_report_allowed_numbers() async { + await assertNoDiagnostics(r''' +void fn() { + var a = 0; + var b = 1; + var c = -1; +} +'''); + } + + Future test_does_not_report_consts() async { + await assertNoDiagnostics(r''' +const pi = 3.14; +void fn() { + var a = pi; +} +'''); + } + + Future test_does_not_report_in_collections() async { + await assertNoDiagnostics(r''' +void fn() { + var list = [1, 2, 3]; + var map = {1: 'a', 2: 'b'}; + var set = {1, 2, 3}; +} +'''); + } + + Future test_does_not_report_negative_numbers_in_list() async { + await assertNoDiagnostics(r''' +void fn() { + var list = [1, -42, 3]; +} +'''); + } + + Future test_does_not_report_negative_numbers_in_set() async { + await assertNoDiagnostics(r''' +void fn() { + var set = {-42, -99}; +} +'''); + } + + Future test_does_not_report_negative_numbers_in_map() async { + await assertNoDiagnostics(r''' +void fn() { + var map = {-42: 'a', 'b': -99}; +} +'''); + } + + Future test_reports_widget_params_when_flag_false() async { + newAnalysisOptionsYamlFile( + testPackageRootPath, + analysisOptionsContent(rules: [rule.name]), + ); + + await assertAutoDiagnostics(''' +$_importFlutterWidgets + +Widget build() { + return SizedBox(width: ${expectLint('42')}); +} +'''); + } + + Future test_does_not_report_widget_params_when_flag_true() async { + newAnalysisOptionsYamlFile(testPackageRootPath, ''' +${analysisOptionsContent(rules: [rule.name])} +plugins: + solid_lints: + diagnostics: + no_magic_number: + allowed_in_widget_params: true +'''); + + await assertNoDiagnostics(''' +$_importFlutterWidgets + +Widget build() { + return SizedBox(width: 42); +} +'''); + } + + Future test_does_not_report_in_datetime() async { + await assertNoDiagnostics(r''' +class DateTime { + const DateTime(int year, int month, int day); +} +void fn() { + final apocalypse = DateTime(2012, 12, 21); +} +'''); + } + + Future test_does_not_report_in_enum_constant_arguments() async { + await assertNoDiagnostics(r''' +enum ConstEnum { + one(1), + two(2); + + final int value; + const ConstEnum(this.value); +} +'''); + } + + Future test_does_not_report_default_formal_parameter_values() async { + await assertNoDiagnostics(r''' +class DefaultValues { + final int value; + DefaultValues.named({this.value = 2}); + DefaultValues.positional([this.value = 3]); + void methodWithNamedParam({int value = 4}) {} + void methodWithPositionalParam([int value = 5]) {} +} +void topLevelFunctionWithDefaultParam({int value = 6}) {} +'''); + } + + Future test_does_not_report_in_constructor_initializers() async { + await assertNoDiagnostics(r''' +class ConstructorInitializer { + final int value; + ConstructorInitializer() : value = 10; +} +'''); + } + + Future test_reports_expression_in_constructors_when_not_const() async { + await assertAutoDiagnostics(''' +class TestOperation { + final double res; + const TestOperation({required this.res}); +} +void fn() { + TestOperation(res: (${expectLint('5')} / ${expectLint('5')}) * ${expectLint('20')}); +} +'''); + } + + Future + test_does_not_report_expression_in_constructors_when_const() async { + await assertNoDiagnostics(r''' +class TestOperation { + final double res; + const TestOperation({required this.res}); +} +void fn() { + const TestOperation(res: (10 / 2)); +} +'''); + } + + Future test_does_not_report_variable_declarations() async { + await assertNoDiagnostics(r''' +void fn() { + var x = 42; + final y = 3.14; +} +'''); + } + + Future test_reports_magic_number_in_variable_expression() async { + await assertAutoDiagnostics(''' +double circumference(double radius) { + var result = ${expectLint('2')} * ${expectLint('3.14')} * radius; + return result; +} +'''); + } + + Future + test_reports_magic_number_in_binary_expression_in_variable() async { + await assertAutoDiagnostics(''' +void fn(int x) { + var result = x + ${expectLint('42')}; +} +'''); + } + + Future test_does_not_report_index_expressions() async { + await assertNoDiagnostics(r''' +void fn(List list) { + var x = list[42]; +} +'''); + } + + Future test_does_not_report_map_literal_keys_and_values() async { + await assertNoDiagnostics(r''' +void fn() { + print({42: 3.14}); +} +'''); + } + + Future test_does_not_report_const_map_keys_and_values() async { + await assertNoDiagnostics(r''' +void fn() { + const map = {42: 3.14}; +} +'''); + } + + Future test_does_not_report_custom_allowed_numbers() async { + newAnalysisOptionsYamlFile(testPackageRootPath, ''' +${analysisOptionsContent(rules: [rule.name])} +plugins: + solid_lints: + diagnostics: + no_magic_number: + allowed: [42, 100] +'''); + + await assertNoDiagnostics(r''' +void fn() { + print(42); + print(100); +} +'''); + } +} From 95ac8e17142fee62d3cc980add5b8fd80fc9c3fa Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 24 Jun 2026 18:40:44 +0300 Subject: [PATCH 2/6] fix: ignore magic numbers in annotations and update boolean configuration parsing --- .../models/no_magic_number_parameters.dart | 2 +- .../visitors/no_magic_number_rule_visitor.dart | 3 ++- .../no_magic_number_rule_test.dart | 16 ++++++++++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/src/lints/no_magic_number/models/no_magic_number_parameters.dart b/lib/src/lints/no_magic_number/models/no_magic_number_parameters.dart index 907996ec..50bdc93f 100644 --- a/lib/src/lints/no_magic_number/models/no_magic_number_parameters.dart +++ b/lib/src/lints/no_magic_number/models/no_magic_number_parameters.dart @@ -63,7 +63,7 @@ class NoMagicNumberParameters { return NoMagicNumberParameters( allowedNumbers: allowedList, allowedInWidgetParams: - json[_allowedInWidgetParamsConfigName] as bool? ?? false, + json[_allowedInWidgetParamsConfigName] == true, ); } } diff --git a/lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart b/lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart index a201be5d..b3411f01 100644 --- a/lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart +++ b/lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart @@ -85,7 +85,8 @@ class NoMagicNumberRuleVisitor extends SimpleAstVisitor { bool _isNotInsideConstConstructor(Literal l) => l.thisOrAncestorMatching((ancestor) { - return ancestor is InstanceCreationExpression && ancestor.isConst; + return (ancestor is InstanceCreationExpression && ancestor.isConst) || + ancestor is Annotation; }) == null; diff --git a/test/src/lints/no_magic_number/no_magic_number_rule_test.dart b/test/src/lints/no_magic_number/no_magic_number_rule_test.dart index 50cd8c22..0609d1b0 100644 --- a/test/src/lints/no_magic_number/no_magic_number_rule_test.dart +++ b/test/src/lints/no_magic_number/no_magic_number_rule_test.dart @@ -270,6 +270,22 @@ void fn() { print(42); print(100); } +'''); + } + + Future test_does_not_report_in_annotations() async { + await assertNoDiagnostics(r''' +class Timeout { + final Duration duration; + const Timeout(this.duration); +} +class Duration { + final int seconds; + const Duration({required this.seconds}); +} + +@Timeout(Duration(seconds: 30)) +void fn() {} '''); } } From e31a5d9f564da2c121860f434342cb3bcf97b51c Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 24 Jun 2026 18:49:10 +0300 Subject: [PATCH 3/6] fix: allow negative numbers in list index expressions for no_magic_number rule --- .../no_magic_number/visitors/no_magic_number_rule_visitor.dart | 3 ++- test/src/lints/no_magic_number/no_magic_number_rule_test.dart | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart b/lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart index b3411f01..1d1319f1 100644 --- a/lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart +++ b/lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart @@ -90,7 +90,8 @@ class NoMagicNumberRuleVisitor extends SimpleAstVisitor { }) == null; - bool _isNotInsideIndexExpression(Literal l) => l.parent is! IndexExpression; + bool _isNotInsideIndexExpression(Literal l) => + _effectiveParent(l) is! IndexExpression; bool _isNotDefaultValue(Literal literal) { return literal.thisOrAncestorOfType() == null; diff --git a/test/src/lints/no_magic_number/no_magic_number_rule_test.dart b/test/src/lints/no_magic_number/no_magic_number_rule_test.dart index 0609d1b0..ea5109c4 100644 --- a/test/src/lints/no_magic_number/no_magic_number_rule_test.dart +++ b/test/src/lints/no_magic_number/no_magic_number_rule_test.dart @@ -235,6 +235,7 @@ void fn(int x) { await assertNoDiagnostics(r''' void fn(List list) { var x = list[42]; + var y = list[-42]; } '''); } From cc0e0cf3e02a918097ee3e408ba30fc4fc4e24f0 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 24 Jun 2026 18:56:14 +0300 Subject: [PATCH 4/6] feat: ignore magic numbers within record literals and named record fields --- .../visitors/no_magic_number_rule_visitor.dart | 5 ++++- .../lints/no_magic_number/no_magic_number_rule_test.dart | 9 +++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart b/lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart index 1d1319f1..4608a628 100644 --- a/lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart +++ b/lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart @@ -80,7 +80,10 @@ class NoMagicNumberRuleVisitor extends SimpleAstVisitor { /// intermediate [PrefixExpression] node. bool _isNotInsideCollectionLiteral(Literal l) { final p = _effectiveParent(l); - return p is! TypedLiteral && p is! MapLiteralEntry; + return p is! TypedLiteral && + p is! MapLiteralEntry && + p is! RecordLiteral && + !(p is NamedExpression && p.parent is RecordLiteral); } bool _isNotInsideConstConstructor(Literal l) => diff --git a/test/src/lints/no_magic_number/no_magic_number_rule_test.dart b/test/src/lints/no_magic_number/no_magic_number_rule_test.dart index ea5109c4..04f67b94 100644 --- a/test/src/lints/no_magic_number/no_magic_number_rule_test.dart +++ b/test/src/lints/no_magic_number/no_magic_number_rule_test.dart @@ -287,6 +287,15 @@ class Duration { @Timeout(Duration(seconds: 30)) void fn() {} +'''); + } + + Future test_does_not_report_in_records() async { + await assertNoDiagnostics(r''' +void fn() { + var point = (10, 20); + var named = (x: 100, y: 200); +} '''); } } From 586fcae48e37bc0deb368ca4527be4a9edac97eb Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Thu, 25 Jun 2026 12:17:43 +0300 Subject: [PATCH 5/6] refactor: migrate no_magic_number test cases to unit tests and remove legacy integration tests --- .../analysis_options.yaml | 8 - ..._number_allowed_in_widget_params_test.dart | 33 ---- lint_test/no_magic_number_test.dart | 149 ------------------ .../no_magic_number_rule_test.dart | 92 +++++++++++ 4 files changed, 92 insertions(+), 190 deletions(-) delete mode 100644 lint_test/no_magic_number_allowed_in_widget_params_test/analysis_options.yaml delete mode 100644 lint_test/no_magic_number_allowed_in_widget_params_test/no_magic_number_allowed_in_widget_params_test.dart delete mode 100644 lint_test/no_magic_number_test.dart diff --git a/lint_test/no_magic_number_allowed_in_widget_params_test/analysis_options.yaml b/lint_test/no_magic_number_allowed_in_widget_params_test/analysis_options.yaml deleted file mode 100644 index 8c1644ac..00000000 --- a/lint_test/no_magic_number_allowed_in_widget_params_test/analysis_options.yaml +++ /dev/null @@ -1,8 +0,0 @@ -analyzer: - plugins: - - ../custom_lint - -custom_lint: - rules: - - no_magic_number: - allowed_in_widget_params: true diff --git a/lint_test/no_magic_number_allowed_in_widget_params_test/no_magic_number_allowed_in_widget_params_test.dart b/lint_test/no_magic_number_allowed_in_widget_params_test/no_magic_number_allowed_in_widget_params_test.dart deleted file mode 100644 index b9f21f1a..00000000 --- a/lint_test/no_magic_number_allowed_in_widget_params_test/no_magic_number_allowed_in_widget_params_test.dart +++ /dev/null @@ -1,33 +0,0 @@ -// ignore_for_file: avoid_returning_widgets -// ignore_for_file: prefer_match_file_name - -// Allowed for numbers in a Widget subtype parameters. -abstract interface class Widget {} - -class StatelessWidget implements Widget {} - -class MyWidget extends StatelessWidget { - final MyWidgetDecoration decoration; - final int value; - - MyWidget({ - required this.decoration, - required this.value, - }); -} - -class MyWidgetDecoration { - final int size; - - MyWidgetDecoration({required this.size}); -} - -Widget build() { - return MyWidget( - /// allowed_in_widget_params: true - decoration: MyWidgetDecoration(size: 12), - - /// allowed_in_widget_params: true - value: 23, - ); -} diff --git a/lint_test/no_magic_number_test.dart b/lint_test/no_magic_number_test.dart deleted file mode 100644 index 4bdc69e2..00000000 --- a/lint_test/no_magic_number_test.dart +++ /dev/null @@ -1,149 +0,0 @@ -// ignore_for_file: unused_local_variable, avoid_returning_widgets -// ignore_for_file: prefer_match_file_name -// ignore_for_file: avoid_unused_parameters -// ignore_for_file: no_empty_block - -/// Check the `no_magic_number` rule - -const pi = 3.14; -const radiusToDiameterCoefficient = 2; - -// expect_lint: no_magic_number -double circumference(double radius) => 2 * 3.14 * radius; - -double correctCircumference(double radius) => - radiusToDiameterCoefficient * pi * radius; - -bool canDrive(int age, {bool isUSA = false}) { - // expect_lint: no_magic_number - return isUSA ? age >= 16 : age > 18; -} - -const usaDrivingAge = 16; -const worldWideDrivingAge = 18; - -bool correctCanDrive(int age, {bool isUSA = false}) { - return isUSA ? age >= usaDrivingAge : age > worldWideDrivingAge; -} - -class ConstClass { - final int a; - - const ConstClass(this.a); -} - -enum ConstEnum { - // Allowed in enum arguments - one(1), - two(2); - - final int value; - - const ConstEnum(this.value); -} - -void fun() { - // Allowed in const constructors - const classInstance = ConstClass(1); - - // Allowed in list literals - final list = [1, 2, 3]; - - // Allowed int map literals - final map = {1: 'One', 2: 'Two'}; - - // Allowed in indexed expression - final result = list[1]; - - // Allowed in DateTime because it doesn't have const constructor - final apocalypse = DateTime(2012, 12, 21); -} - -// Allowed for defaults in constructors and methods. -class DefaultValues { - final int value; - - DefaultValues.named({ - this.value = 2, - }); - - DefaultValues.positional([ - this.value = 3, - ]); - - void methodWithNamedParam({int value = 4}) {} - - void methodWithPositionalParam([int value = 5]) {} -} - -void topLevelFunctionWithDefaultParam({int value = 6}) { - ({int value = 7}) {}; -} - -// Allowed for numbers in constructor initializer. -class ConstructorInitializer { - final int value; - - ConstructorInitializer() : value = 10; -} - -abstract interface class Widget {} - -class StatelessWidget implements Widget {} - -class MyWidget extends StatelessWidget { - final MyWidgetDecoration decoration; - final int value; - - MyWidget({ - required this.decoration, - required this.value, - }); -} - -class MyWidgetDecoration { - final int size; - - MyWidgetDecoration({required this.size}); -} - -Widget build() { - return MyWidget( - // expect_lint: no_magic_number - decoration: MyWidgetDecoration(size: 12), - - // expect_lint: no_magic_number - value: 23, - ); -} - -class TestOperation { - final double res; - - const TestOperation({required this.res}); -} - -const n = 15; -const m = 20; - -void checkOperationInConstructor() { - // expect_lint: no_magic_number - TestOperation(res: (5 / 5) * 20); - - // expect_lint: no_magic_number - var variable = TestOperation(res: (n / m) * 20); - - // expect_lint: no_magic_number - final finalVar = TestOperation(res: (10 / m + 4 + n)); - - // Allowed for constant expressions - const constVar = TestOperation(res: (10 / m + 4 + n)); - - var constVar2 = const TestOperation(res: 15 + (10 / n)); - - final l = [ - // expect_lint: no_magic_number - TestOperation(res: 8), - const TestOperation(res: 9), - ]; -} diff --git a/test/src/lints/no_magic_number/no_magic_number_rule_test.dart b/test/src/lints/no_magic_number/no_magic_number_rule_test.dart index 04f67b94..59df9c7f 100644 --- a/test/src/lints/no_magic_number/no_magic_number_rule_test.dart +++ b/test/src/lints/no_magic_number/no_magic_number_rule_test.dart @@ -47,6 +47,15 @@ int fn(int b) { '''); } + Future + test_reports_magic_numbers_in_ternary_comparison() async { + await assertAutoDiagnostics(''' +bool canDrive(int age, {bool isUSA = false}) { + return isUSA ? age >= ${expectLint('16')} : age > ${expectLint('18')}; +} +'''); + } + Future test_does_not_report_allowed_numbers() async { await assertNoDiagnostics(r''' void fn() { @@ -296,6 +305,89 @@ void fn() { var point = (10, 20); var named = (x: 100, y: 200); } +'''); + } + + Future + test_does_not_report_anonymous_function_default_param() async { + await assertNoDiagnostics(r''' +void fn() { + ({int value = 7}) {}; +} +'''); + } + + Future + test_reports_magic_number_mixed_with_consts_in_constructor() async { + await assertAutoDiagnostics(''' +class TestOperation { + final double res; + const TestOperation({required this.res}); +} +const n = 15; +const m = 20; +void fn() { + TestOperation(res: (n / m) * ${expectLint('20')}); +} +'''); + } + + Future + test_reports_multiple_magic_numbers_in_constructor_expression() async { + await assertAutoDiagnostics(''' +class TestOperation { + final double res; + const TestOperation({required this.res}); +} +const m = 20; +const n = 15; +void fn() { + final v = TestOperation(res: (${expectLint('10')} / m + ${expectLint('4')} + n)); +} +'''); + } + + Future + test_does_not_report_var_with_const_constructor_call() async { + await assertNoDiagnostics(r''' +class TestOperation { + final double res; + const TestOperation({required this.res}); +} +const n = 15; +void fn() { + var v = const TestOperation(res: 15 + (10 / n)); +} +'''); + } + + Future + test_reports_magic_number_in_constructor_inside_list() async { + await assertAutoDiagnostics(''' +class TestOperation { + final double res; + const TestOperation({required this.res}); +} +void fn() { + final l = [ + TestOperation(res: ${expectLint('8')}), + ]; +} +'''); + } + + Future + test_does_not_report_const_constructor_inside_list() async { + await assertNoDiagnostics(r''' +class TestOperation { + final double res; + const TestOperation({required this.res}); +} +void fn() { + final l = [ + const TestOperation(res: 9), + ]; +} '''); } } From 78b275c6c517805444d3c7d69b7bf726d5c9ba56 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Thu, 25 Jun 2026 17:19:02 +0300 Subject: [PATCH 6/6] refactor: move AST inspection logic to AstNode extension and simplify magic number validation checks --- .../no_magic_number_rule_visitor.dart | 85 ++++++------------- lib/src/utils/node_utils.dart | 42 +++++++++ 2 files changed, 69 insertions(+), 58 deletions(-) diff --git a/lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart b/lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart index 4608a628..efd6e13b 100644 --- a/lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart +++ b/lib/src/lints/no_magic_number/visitors/no_magic_number_rule_visitor.dart @@ -4,6 +4,7 @@ import 'package:analyzer/dart/element/type.dart'; import 'package:collection/collection.dart'; import 'package:solid_lints/src/lints/no_magic_number/models/no_magic_number_parameters.dart'; import 'package:solid_lints/src/models/solid_lint_rule.dart'; +import 'package:solid_lints/src/utils/node_utils.dart'; /// The AST visitor that checks if double and integer literals are magic numbers. class NoMagicNumberRuleVisitor extends SimpleAstVisitor { @@ -24,53 +25,39 @@ class NoMagicNumberRuleVisitor extends SimpleAstVisitor { } void _checkLiteral(Literal node) { - 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 (_isWidgetParameter(node)) return; + + if (node.isInsideConstConstructor) return; + if (node.isInDateTime) return; + if (node.isInsideIndexExpression) return; + if (node.isInsideEnumConstantArguments) return; + if (node.isDefaultValue) return; + if (node.isInConstructorInitializer) return; _rule.reportAtNode(node); } - bool _isMagicNumber(Literal l) => - (l is DoubleLiteral && !_parameters.allowedNumbers.contains(l.value)) || - (l is IntegerLiteral && !_parameters.allowedNumbers.contains(l.value)); + bool _isNotMagicNumber(Literal l) => + (l is DoubleLiteral && _parameters.allowedNumbers.contains(l.value)) || + (l is IntegerLiteral && _parameters.allowedNumbers.contains(l.value)); /// Returns the effective parent of [l], skipping over a unary /// [PrefixExpression] (e.g. the `-` in `-42`). AstNode? _effectiveParent(Literal l) => l.parent is PrefixExpression ? l.parent?.parent : l.parent; - /// Returns `true` if the literal is NOT the direct initializer of a variable + /// Returns `true` if the literal is the direct initializer of a variable /// declaration. /// /// Allows `var x = 42` (literal gets a name) but reports `var r = x + 42` /// (42 is a magic number inside an expression). - bool _isNotInsideVariable(Literal l) => - _effectiveParent(l) is! VariableDeclaration; - - bool _isNotInDateTime(Literal l) => - l.thisOrAncestorMatching( - (a) => - a is InstanceCreationExpression && - a.staticType?.getDisplayString() == 'DateTime', - ) == - null; - - bool _isNotInsideEnumConstantArguments(Literal l) { - final node = l.thisOrAncestorMatching( - (ancestor) => ancestor is EnumConstantArguments, - ); - return node == null; - } + bool _isInsideVariable(Literal l) => + _effectiveParent(l) is VariableDeclaration; - /// Returns `true` if the literal is NOT inside a collection literal. + /// Returns `true` if the literal is inside a collection literal. /// /// Covers list elements (`[42]`, `[-42]`), set elements (`{42}`, `{-42}`), /// and map keys/values (`{42: 'a'}`, `{-42: 'a'}`, `{'a': -42}`). @@ -78,40 +65,22 @@ class NoMagicNumberRuleVisitor extends SimpleAstVisitor { /// Unary prefix expressions (e.g. `-42`) are skipped transparently so that /// the effective parent—the collection literal—is checked instead of the /// intermediate [PrefixExpression] node. - bool _isNotInsideCollectionLiteral(Literal l) { + bool _isInsideCollectionLiteral(Literal l) { final p = _effectiveParent(l); - return p is! TypedLiteral && - p is! MapLiteralEntry && - p is! RecordLiteral && - !(p is NamedExpression && p.parent is RecordLiteral); - } - - bool _isNotInsideConstConstructor(Literal l) => - l.thisOrAncestorMatching((ancestor) { - return (ancestor is InstanceCreationExpression && ancestor.isConst) || - ancestor is Annotation; - }) == - null; - - bool _isNotInsideIndexExpression(Literal l) => - _effectiveParent(l) is! IndexExpression; - - bool _isNotDefaultValue(Literal literal) { - return literal.thisOrAncestorOfType() == null; - } - - bool _isNotInConstructorInitializer(Literal literal) { - return literal.thisOrAncestorOfType() == null; + return p is TypedLiteral || + p is MapLiteralEntry || + p is RecordLiteral || + (p is NamedExpression && p.parent is RecordLiteral); } - bool _isNotWidgetParameter(Literal literal) { - if (!_parameters.allowedInWidgetParams) return true; + bool _isWidgetParameter(Literal literal) { + if (!_parameters.allowedInWidgetParams) return false; final widgetCreationExpression = literal.thisOrAncestorMatching( _isWidgetCreationExpression, ); - return widgetCreationExpression == null; + return widgetCreationExpression != null; } bool _isWidgetCreationExpression(AstNode node) { diff --git a/lib/src/utils/node_utils.dart b/lib/src/utils/node_utils.dart index 2b008879..6c7d9816 100644 --- a/lib/src/utils/node_utils.dart +++ b/lib/src/utils/node_utils.dart @@ -61,3 +61,45 @@ extension SimpleIdentifierExtension on SimpleIdentifier { return declOffset >= body.offset && declOffset < body.end; } } + +/// Extension on [AstNode] to provide generic context/traversal checks. +extension AstNodeExtension on AstNode { + /// Returns `true` if the node is within the default value of a formal parameter. + bool get isDefaultValue => + thisOrAncestorOfType() != null; + + /// Returns `true` if the node is within a constructor initializer. + bool get isInConstructorInitializer => + thisOrAncestorOfType() != null; + + /// Returns `true` if the node is within a const constructor invocation or annotation. + bool get isInsideConstConstructor => + thisOrAncestorMatching((ancestor) { + return (ancestor is InstanceCreationExpression && ancestor.isConst) || + ancestor is Annotation; + }) != + null; + + /// Returns `true` if the node is within enum constant arguments. + bool get isInsideEnumConstantArguments => + thisOrAncestorMatching( + (ancestor) => ancestor is EnumConstantArguments, + ) != + null; + + /// Returns `true` if the node is within a DateTime constructor invocation. + bool get isInDateTime => + thisOrAncestorMatching( + (a) => + a is InstanceCreationExpression && + a.staticType?.getDisplayString() == 'DateTime', + ) != + null; + + /// Returns `true` if the node is the child (optionally wrapped in a prefix + /// expression) of an index expression (e.g. `list[42]` or `list[-42]`). + bool get isInsideIndexExpression { + final p = parent is PrefixExpression ? parent?.parent : parent; + return p is IndexExpression; + } +}