feat: add static code diagnostic prefer-static-class (#1086)

Roman Petrov · web-flow · commit 4900412d56d2 · 2022-12-01T17:23:26.000+04:00
* feat: add static code diagnostic prefer-static-class

* fix: address review comments

* fix: review comment

* fix: review comment
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,8 @@
 * feat: add static code diagnostic [`arguments-ordering`](https://dartcodemetrics.dev/docs/rules/common/arguments-ordering).
 * feat: add method call chains support for [`ban-name`](https://dartcodemetrics.dev/docs/rules/common/ban-name).
 * fix: update `dart_all.yaml` preset to contain missing rules.
+* feat: add static code diagnostic [`prefer-static-class`](https://dartcodemetrics.dev/docs/rules/common/prefer-static-class).
+* feat: ignore `hcwidget` annotations in ['avoid-returning-widgets'](https://dartcodemetrics.dev/docs/rules/common/avoid-returning-widgets) rule by default.
 
 ## 5.0.1
 
diff --git a/lib/presets/dart_all.yaml b/lib/presets/dart_all.yaml
@@ -46,6 +46,6 @@ dart_code_metrics:
     - prefer-last
     - prefer-match-file-name
     - prefer-moving-to-variable
-    - prefer-trailing-comma
+    - prefer-static-class
     - prefer-trailing-comma
     - tag-name
diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart b/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart
@@ -63,6 +63,7 @@ import 'rules_list/prefer_match_file_name/prefer_match_file_name_rule.dart';
 import 'rules_list/prefer_moving_to_variable/prefer_moving_to_variable_rule.dart';
 import 'rules_list/prefer_on_push_cd_strategy/prefer_on_push_cd_strategy_rule.dart';
 import 'rules_list/prefer_single_widget_per_file/prefer_single_widget_per_file_rule.dart';
+import 'rules_list/prefer_static_class/prefer_static_class_rule.dart';
 import 'rules_list/prefer_trailing_comma/prefer_trailing_comma_rule.dart';
 import 'rules_list/provide_correct_intl_args/provide_correct_intl_args_rule.dart';
 import 'rules_list/tag_name/tag_name_rule.dart';
@@ -141,6 +142,7 @@ final _implementedRules = <String, Rule Function(Map<String, Object>)>{
   PreferMovingToVariableRule.ruleId: PreferMovingToVariableRule.new,
   PreferOnPushCdStrategyRule.ruleId: PreferOnPushCdStrategyRule.new,
   PreferSingleWidgetPerFileRule.ruleId: PreferSingleWidgetPerFileRule.new,
+  PreferStaticClassRule.ruleId: PreferStaticClassRule.new,
   PreferTrailingCommaRule.ruleId: PreferTrailingCommaRule.new,
   ProvideCorrectIntlArgsRule.ruleId: ProvideCorrectIntlArgsRule.new,
   TagNameRule.ruleId: TagNameRule.new,
diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_returning_widgets/avoid_returning_widgets_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_returning_widgets/avoid_returning_widgets_rule.dart
@@ -13,6 +13,7 @@ import '../../../models/issue.dart';
 import '../../../models/severity.dart';
 import '../../models/flutter_rule.dart';
 import '../../rule_utils.dart';
+import '../common_config.dart';
 
 part 'config_parser.dart';
 part 'visit_declaration.dart';
diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_returning_widgets/config_parser.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_returning_widgets/config_parser.dart
@@ -4,18 +4,12 @@ class _ConfigParser {
   static const _ignoredNamesConfig = 'ignored-names';
   static const _ignoredAnnotationsConfig = 'ignored-annotations';
 
-  static const _defaultIgnoredAnnotations = [
-    'FunctionalWidget',
-    'swidget',
-    'hwidget',
-  ];
-
   static Iterable<String> getIgnoredNames(Map<String, Object> config) =>
       _getIterable(config, _ignoredNamesConfig) ?? [];
 
   static Iterable<String> getIgnoredAnnotations(Map<String, Object> config) =>
       _getIterable(config, _ignoredAnnotationsConfig) ??
-      _defaultIgnoredAnnotations;
+      functionalWidgetAnnotations;
 
   static Iterable<String>? _getIterable(
     Map<String, Object> config,
diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/common_config.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/common_config.dart
@@ -0,0 +1,6 @@
+const functionalWidgetAnnotations = [
+  'FunctionalWidget',
+  'swidget',
+  'hwidget',
+  'hcwidget',
+];
diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_static_class/config_parser.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_static_class/config_parser.dart
@@ -0,0 +1,31 @@
+part of 'prefer_static_class_rule.dart';
+
+class _ConfigParser {
+  static const _ignorePrivate = 'ignore-private';
+  static const _ignoreNames = 'ignore-names';
+  static const _ignoreAnnotations = 'ignore-annotations';
+
+  static const _defaultIgnorePrivate = false;
+  static const _defaultIgnoreNames = <String>[];
+  static const _defaultIgnoreAnnotations = [
+    ...functionalWidgetAnnotations,
+    'riverpod',
+  ];
+
+  static bool getIgnorePrivate(Map<String, Object> config) =>
+      config[_ignorePrivate] as bool? ?? _defaultIgnorePrivate;
+
+  static Iterable<String> getIgnoreNames(Map<String, Object> config) =>
+      _getIterable(config, _ignoreNames) ?? _defaultIgnoreNames;
+
+  static Iterable<String> getIgnoreAnnotations(Map<String, Object> config) =>
+      _getIterable(config, _ignoreAnnotations) ?? _defaultIgnoreAnnotations;
+
+  static Iterable<String>? _getIterable(
+    Map<String, Object> config,
+    String name,
+  ) =>
+      config[name] is Iterable
+          ? List<String>.from(config[name] as Iterable)
+          : null;
+}
diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_static_class/prefer_static_class_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_static_class/prefer_static_class_rule.dart
@@ -0,0 +1,71 @@
+// ignore_for_file: public_member_api_docs
+
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/token.dart';
+import 'package:analyzer/dart/ast/visitor.dart';
+
+import '../../../../../utils/node_utils.dart';
+import '../../../lint_utils.dart';
+import '../../../models/internal_resolved_unit_result.dart';
+import '../../../models/issue.dart';
+import '../../../models/severity.dart';
+import '../../models/common_rule.dart';
+import '../../rule_utils.dart';
+import '../common_config.dart';
+
+part 'config_parser.dart';
+part 'visitor.dart';
+
+class PreferStaticClassRule extends CommonRule {
+  static const String ruleId = 'prefer-static-class';
+
+  static const _warningMessage =
+      'Prefer declaring static class members instead of global functions, variables and constants.';
+
+  final bool _ignorePrivate;
+  final Iterable<String> _ignoreNames;
+  final Iterable<String> _ignoreAnnotations;
+
+  PreferStaticClassRule([Map<String, Object> config = const {}])
+      : _ignorePrivate = _ConfigParser.getIgnorePrivate(config),
+        _ignoreAnnotations = _ConfigParser.getIgnoreAnnotations(config),
+        _ignoreNames = _ConfigParser.getIgnoreNames(config),
+        super(
+          id: ruleId,
+          severity: readSeverity(config, Severity.style),
+          excludes: readExcludes(config),
+          includes: readIncludes(config),
+        );
+
+  @override
+  Map<String, Object?> toJson() {
+    final json = super.toJson();
+    json[_ConfigParser._ignorePrivate] = _ignorePrivate;
+    json[_ConfigParser._ignoreNames] = _ignoreNames.toList();
+    json[_ConfigParser._ignoreAnnotations] = _ignoreAnnotations.toList();
+
+    return json;
+  }
+
+  @override
+  Iterable<Issue> check(InternalResolvedUnitResult source) {
+    final visitor = _Visitor(
+      ignorePrivate: _ignorePrivate,
+      ignoreNames: _ignoreNames,
+      ignoreAnnotations: _ignoreAnnotations,
+    );
+
+    source.unit.visitChildren(visitor);
+
+    return visitor.declarations
+        .map((declaration) => createIssue(
+              rule: this,
+              location: nodeLocation(
+                node: declaration,
+                source: source,
+              ),
+              message: _warningMessage,
+            ))
+        .toList(growable: false);
+  }
+}
diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_static_class/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_static_class/visitor.dart
@@ -0,0 +1,52 @@
+part of 'prefer_static_class_rule.dart';
+
+class _Visitor extends RecursiveAstVisitor<void> {
+  _Visitor({
+    required bool ignorePrivate,
+    required Iterable<String> ignoreNames,
+    required Iterable<String> ignoreAnnotations,
+  })  : _ignorePrivate = ignorePrivate,
+        _ignoreNames = ignoreNames.map(RegExp.new),
+        _ignoreAnnotations = ignoreAnnotations;
+
+  final bool _ignorePrivate;
+  final Iterable<RegExp> _ignoreNames;
+  final Iterable<String> _ignoreAnnotations;
+
+  final _declarations = <AstNode>[];
+
+  Iterable<AstNode> get declarations => _declarations;
+
+  @override
+  void visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) {
+    super.visitTopLevelVariableDeclaration(node);
+
+    if (!_hasIgnoredAnnotation(node) &&
+        !node.variables.variables
+            .every((variable) => _shouldIgnoreName(variable.name.lexeme))) {
+      _declarations.add(node);
+    }
+  }
+
+  @override
+  void visitFunctionDeclaration(FunctionDeclaration node) {
+    super.visitFunctionDeclaration(node);
+    final nodeName = node.name.lexeme;
+    if (node.parent is CompilationUnit &&
+        !_hasIgnoredAnnotation(node) &&
+        !_shouldIgnoreName(nodeName) &&
+        !isEntrypoint(nodeName, node.metadata)) {
+      _declarations.add(node);
+    }
+  }
+
+  bool _shouldIgnoreName(String name) =>
+      (_ignorePrivate && Identifier.isPrivateName(name)) ||
+      _ignoreNames.any((element) => element.hasMatch(name));
+
+  bool _hasIgnoredAnnotation(Declaration node) => node.metadata.any(
+        (node) =>
+            _ignoreAnnotations.contains(node.name.name) &&
+            node.atSign.type == TokenType.AT,
+      );
+}
diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_returning_widgets/avoid_returning_widgets_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_returning_widgets/avoid_returning_widgets_rule_test.dart
@@ -27,7 +27,7 @@ void main() {
 
       RuleTestHelper.verifyIssues(
         issues: issues,
-        startLines: [36, 38, 40, 42, 44, 47, 16, 53, 67],
+        startLines: [36, 38, 40, 42, 44, 47, 16, 53, 70],
         startColumns: [5, 5, 5, 5, 5, 14, 3, 1, 1],
         locationTexts: [
           '_localBuildMyWidget()',
@@ -73,8 +73,8 @@ void main() {
 
       RuleTestHelper.verifyIssues(
         issues: issues,
-        startLines: [36, 38, 40, 16, 53, 57, 60, 63],
-        startColumns: [5, 5, 5, 3, 1, 1, 1, 1],
+        startLines: [36, 38, 40, 16, 53, 57, 60, 63, 66],
+        startColumns: [5, 5, 5, 3, 1, 1, 1, 1, 1],
         locationTexts: [
           '_localBuildMyWidget()',
           '_getWidgetsIterable()',
@@ -89,6 +89,8 @@ void main() {
               'Widget _getStatelessFunctionalWidget() => Container();',
           '@hwidget\n'
               'Widget _getHookFunctionalWidget() => Container();',
+          '@hcwidget\n'
+              'Widget _getHookConsumerFunctionalWidget() => Container();',
         ],
         messages: [
           'Avoid returning widgets from a function.',
@@ -99,6 +101,7 @@ void main() {
           'Avoid returning widgets from a global function.',
           'Avoid returning widgets from a global function.',
           'Avoid returning widgets from a global function.',
+          'Avoid returning widgets from a global function.',
         ],
       );
     });
diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_returning_widgets/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_returning_widgets/examples/example.dart
@@ -63,6 +63,9 @@ Widget _getStatelessFunctionalWidget() => Container();
 @hwidget
 Widget _getHookFunctionalWidget() => Container();
 
+@hcwidget
+Widget _getHookConsumerFunctionalWidget() => Container();
+
 // LINT
 @ignoredAnnotation
 Widget _getWidgetWithIgnoredAnnotation() => Container();
@@ -73,6 +76,7 @@ class FunctionalWidget {
 
 const swidget = FunctionalWidget();
 const hwidget = FunctionalWidget();
+const hcwidget = FunctionalWidget();
 
 class IgnoredAnnotation {
   const IgnoredAnnotation();
diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_static_class/examples/correct_example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_static_class/examples/correct_example.dart
@@ -0,0 +1,34 @@
+void main() {}
+
+Future<void> main() {}
+
+class Example {
+  void classMethod() {}
+  static void staticClassMethod() {}
+  var classVariable = 42;
+  static var staticClassVariable = 42;
+  static final staticClassFinalVariable = 42;
+  static const classConstant = 42;
+
+  void _privateClassMethod() {}
+  static void _privateStaticClassMethod() {}
+  var _privateClassVariable = 42;
+  static var _privateStaticClassVariable = 42;
+  static final _privateStaticClassFinalVariable = 42;
+  static const _privateClassConstant = 42;
+}
+
+@FunctionalWidget
+Widget functionalWidget() {}
+
+@swidget
+Widget statelessWidget() {}
+
+@hwidget
+Widget hookWidget() {}
+
+@hcwidget
+Widget hookConsumerWidget() {}
+
+@riverpod
+int riverpodFunction(riverpodFunction ref) => 0;
diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_static_class/examples/correct_ignore_annotation_example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_static_class/examples/correct_ignore_annotation_example.dart
@@ -0,0 +1,23 @@
+@ignoredAnnotation
+void globalFunction() {}
+
+@ignoredAnnotation
+var globalVariable = 42;
+
+@ignoredAnnotation
+final globalFinalVariable = 42;
+
+@ignoredAnnotation
+const globalConstant = 42;
+
+@ignoredAnnotation
+void _privateGlobalFunction() {}
+
+@ignoredAnnotation
+var _privateGlobalVariable = 42;
+
+@ignoredAnnotation
+final _privateGlobalFinalVariable = 42;
+
+@ignoredAnnotation
+const _privateGlobalConstant = 42;
diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_static_class/examples/correct_ignore_names_example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_static_class/examples/correct_ignore_names_example.dart
@@ -0,0 +1,3 @@
+final someRiverpodProvider = 42;
+
+final useSomeRiverpodHook = 42;
diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_static_class/examples/correct_ignore_private_example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_static_class/examples/correct_ignore_private_example.dart
@@ -0,0 +1,7 @@
+void _privateGlobalFunction() {}
+
+var _privateGlobalVariable = 42;
+
+final _privateGlobalFinalVariable = 42;
+
+const _privateGlobalConstant = 42;
diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_static_class/examples/incorrect_example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_static_class/examples/incorrect_example.dart
@@ -0,0 +1,9 @@
+void globalFunction() {} // LINT
+var globalVariable = 42; // LINT
+final globalFinalVariable = 42; // LINT
+const globalConstant = 42; // LINT
+
+void _privateGlobalFunction() {} // LINT
+var _privateGlobalVariable = 42; // LINT
+final _privateGlobalFinalVariable = 42; // LINT
+const _privateGlobalConstant = 42; // LINT
diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_static_class/prefer_static_class_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_static_class/prefer_static_class_rule_test.dart
diff --git a/website/docs/rules/common/prefer-static-class.mdx b/website/docs/rules/common/prefer-static-class.mdx
diff --git a/website/docs/rules/index.mdx b/website/docs/rules/index.mdx
