Added no magic number rule (#57)

* Add no-magic-number rule

* Add tests for no-magic-number rule

* Fix import lost in merge

* Ignore no-magic-number in test

---------

Co-authored-by: Denis Bogatirov <denis.bohatyrov@computools.com>

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<num> allowedNumbers;
+
+  /// Constructor for [NoMagicNumberParameters] model
+  const NoMagicNumberParameters({
+    required this.allowedNumbers,
+  });
+
+  /// Method for creating from json data
+  factory NoMagicNumberParameters.fromJson(Map<String, Object?> json) =>
+      NoMagicNumberParameters(
+        allowedNumbers:
+            json[_allowedConfigName] as Iterable<num>? ?? _defaultMagicNumbers,
+      );
+}

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<NoMagicNumberParameters> {
+  /// 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<NoMagicNumberParameters>(
+      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;
+}

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<void> {
+  final _literals = <Literal>[];
+
+  /// List of all double and integer literals
+  Iterable<Literal> 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);
+  }
+}

lib/solid_lints.dart

@@ -17,6 +17,7 @@ import 'package:solid_lints/lints/member_ordering/member_ordering_rule.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_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';
 
@@ -45,6 +46,7 @@ class _SolidLints extends PluginBase {
       NoEmptyBlockRule.createRule(configs),
       NoEqualThenElseRule.createRule(configs),
       MemberOrderingRule.createRule(configs),
+      NoMagicNumberRule.createRule(configs),
     ];
 
     // Return only enabled rules

lint_test/analysis_options.yaml

@@ -48,3 +48,4 @@ custom_lint:
         - did-change-dependencies-method
         - did-update-widget-method
         - dispose-method
+    - no-magic-number

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

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

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