From a5d6b89423710fb53eb991e5b47ffca6031ddb5f Mon Sep 17 00:00:00 2001 From: Denis Bogatirov Date: Fri, 15 Sep 2023 16:41:08 +0300 Subject: [PATCH 1/4] Add no-magic-number rule --- .../models/no_magic_number_parameters.dart | 21 +++ .../no_magic_number/no_magic_number_rule.dart | 125 ++++++++++++++++++ .../no_magic_number_visitor.dart | 45 +++++++ lib/solid_lints.dart | 2 + 4 files changed, 193 insertions(+) create mode 100644 lib/lints/no_magic_number/models/no_magic_number_parameters.dart create mode 100644 lib/lints/no_magic_number/no_magic_number_rule.dart create mode 100644 lib/lints/no_magic_number/no_magic_number_visitor.dart diff --git a/lib/lints/no_magic_number/models/no_magic_number_parameters.dart b/lib/lints/no_magic_number/models/no_magic_number_parameters.dart new file mode 100644 index 00000000..783d7b31 --- /dev/null +++ b/lib/lints/no_magic_number/models/no_magic_number_parameters.dart @@ -0,0 +1,21 @@ +/// A data model class that represents the "no magic numbers" input +/// parameters. +class NoMagicNumberParameters { + static const _allowedConfigName = 'allowed'; + static const _defaultMagicNumbers = [-1, 0, 1]; + + /// List of allowed numbers + final Iterable allowedNumbers; + + /// Constructor for [NoMagicNumberParameters] model + const NoMagicNumberParameters({ + required this.allowedNumbers, + }); + + /// Method for creating from json data + factory NoMagicNumberParameters.fromJson(Map json) => + NoMagicNumberParameters( + allowedNumbers: + json[_allowedConfigName] as Iterable? ?? _defaultMagicNumbers, + ); +} diff --git a/lib/lints/no_magic_number/no_magic_number_rule.dart b/lib/lints/no_magic_number/no_magic_number_rule.dart new file mode 100644 index 00000000..1fd64405 --- /dev/null +++ b/lib/lints/no_magic_number/no_magic_number_rule.dart @@ -0,0 +1,125 @@ +// 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/error/listener.dart'; +import 'package:custom_lint_builder/custom_lint_builder.dart'; +import 'package:solid_lints/lints/no_magic_number/models/no_magic_number_parameters.dart'; +import 'package:solid_lints/lints/no_magic_number/no_magic_number_visitor.dart'; +import 'package:solid_lints/models/rule_config.dart'; +import 'package:solid_lints/models/solid_lint_rule.dart'; + +/// A `no-magic-number` rule which forbids having numbers without variable +class NoMagicNumberRule extends SolidLintRule { + /// The [LintCode] of this lint rule that represents + /// 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); + } + + @override + void run( + CustomLintResolver resolver, + ErrorReporter 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); + + for (final magicNumber in magicNumbers) { + reporter.reportErrorForNode(code, magicNumber); + } + }); + } + + 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) => + l.thisOrAncestorMatching( + (ancestor) => ancestor is VariableDeclaration, + ) == + null; + + bool _isNotInDateTime(Literal l) => + l.thisOrAncestorMatching( + (a) => + a is InstanceCreationExpression && + a.staticType?.getDisplayString(withNullability: false) == + 'DateTime', + ) == + null; + + bool _isNotInsideEnumConstantArguments(Literal l) { + final node = l.thisOrAncestorMatching( + (ancestor) => ancestor is EnumConstantArguments, + ); + + return node == null; + } + + 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) => + ancestor is InstanceCreationExpression && ancestor.isConst, + ) == + null; + + bool _isNotInsideIndexExpression(Literal l) => l.parent is! IndexExpression; +} diff --git a/lib/lints/no_magic_number/no_magic_number_visitor.dart b/lib/lints/no_magic_number/no_magic_number_visitor.dart new file mode 100644 index 00000000..7bec43f6 --- /dev/null +++ b/lib/lints/no_magic_number/no_magic_number_visitor.dart @@ -0,0 +1,45 @@ +// 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/lib/solid_lints.dart b/lib/solid_lints.dart index 13960c6d..e16fdc09 100644 --- a/lib/solid_lints.dart +++ b/lib/solid_lints.dart @@ -14,6 +14,7 @@ import 'package:solid_lints/lints/double_literal_format/double_literal_format_ru import 'package:solid_lints/lints/function_lines_of_code/function_lines_of_code_metric.dart'; import 'package:solid_lints/lints/newline_before_return/newline_before_return_rule.dart'; import 'package:solid_lints/lints/no_empty_block/no_empty_block_rule.dart'; +import 'package:solid_lints/lints/no_magic_number/no_magic_number_rule.dart'; import 'package:solid_lints/lints/number_of_parameters/number_of_parameters_metric.dart'; import 'package:solid_lints/models/solid_lint_rule.dart'; @@ -39,6 +40,7 @@ class _SolidLints extends PluginBase { AvoidUnrelatedTypeAssertionsRule.createRule(configs), NewlineBeforeReturnRule.createRule(configs), NoEmptyBlockRule.createRule(configs), + NoMagicNumberRule.createRule(configs), ]; // Return only enabled rules From 164399fb517756a798a2b9dc77974a548d206586 Mon Sep 17 00:00:00 2001 From: Denis Bogatirov Date: Fri, 15 Sep 2023 16:41:29 +0300 Subject: [PATCH 2/4] Add tests for no-magic-number rule --- lint_test/analysis_options.yaml | 1 + lint_test/lines_of_code_test.dart | 2 + lint_test/no_magic_number_test.dart | 57 +++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+) create mode 100644 lint_test/no_magic_number_test.dart diff --git a/lint_test/analysis_options.yaml b/lint_test/analysis_options.yaml index 6d666af3..7b1222e1 100644 --- a/lint_test/analysis_options.yaml +++ b/lint_test/analysis_options.yaml @@ -21,3 +21,4 @@ custom_lint: - avoid-unrelated-type-assertions - newline-before-return - no-empty-block + - no-magic-number diff --git a/lint_test/lines_of_code_test.dart b/lint_test/lines_of_code_test.dart index 5f5cfbde..b8d14eb9 100644 --- a/lint_test/lines_of_code_test.dart +++ b/lint_test/lines_of_code_test.dart @@ -1,3 +1,5 @@ +// ignore_for_file: no-magic-number + import 'package:test/test.dart'; /// Check number of lines fail diff --git a/lint_test/no_magic_number_test.dart b/lint_test/no_magic_number_test.dart new file mode 100644 index 00000000..42a384b3 --- /dev/null +++ b/lint_test/no_magic_number_test.dart @@ -0,0 +1,57 @@ +// ignore_for_file: unused_local_variable + +/// 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 cons constructor + final apocalypse = DateTime(2012, 12, 21); +} From c0a476ee14a595cfec8985a7f742852454bc65eb Mon Sep 17 00:00:00 2001 From: Denis Bogatirov Date: Fri, 15 Sep 2023 16:44:09 +0300 Subject: [PATCH 3/4] Fix import lost in merge --- lib/solid_lints.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/solid_lints.dart b/lib/solid_lints.dart index 9c5c8b5f..ec201fd5 100644 --- a/lib/solid_lints.dart +++ b/lib/solid_lints.dart @@ -15,6 +15,7 @@ import 'package:solid_lints/lints/function_lines_of_code/function_lines_of_code_ import 'package:solid_lints/lints/newline_before_return/newline_before_return_rule.dart'; import 'package:solid_lints/lints/no_empty_block/no_empty_block_rule.dart'; import 'package:solid_lints/lints/no_equal_then_else/no_equal_then_else_rule.dart'; +import 'package:solid_lints/lints/no_magic_number/no_magic_number_rule.dart'; import 'package:solid_lints/lints/number_of_parameters/number_of_parameters_metric.dart'; import 'package:solid_lints/models/solid_lint_rule.dart'; From 8a4defa6b5a5a81953368a345962c306a227fc23 Mon Sep 17 00:00:00 2001 From: Denis Bogatirov Date: Fri, 15 Sep 2023 16:50:17 +0300 Subject: [PATCH 4/4] Ignore no-magic-number in test --- lint_test/no_equal_then_else_test.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/lint_test/no_equal_then_else_test.dart b/lint_test/no_equal_then_else_test.dart index 74930dfd..358bf6f2 100644 --- a/lint_test/no_equal_then_else_test.dart +++ b/lint_test/no_equal_then_else_test.dart @@ -1,5 +1,6 @@ // ignore_for_file: unused_local_variable // ignore_for_file: cyclomatic_complexity +// ignore_for_file: no-magic-number /// Check the `no-equal-then-else` rule void fun() {