implement "avoid unnecessary setstate" rule (#44)

* implement "avoid unnecessary setstate" rule

* fix pr comments

* added idea files to gitignore

* fix pr comments

* Fix merge conflict

* Fix tests and improve GitHub workflow

* Fix workflow file

* Fix conflicts after merge

* Remove non-existing import

* Add missing rule to tests and remove unnecessary GitHub actions step

---------

Co-authored-by: Yaroslav Laptiev <[email protected]>
Co-authored-by: vladimir-beloded <[email protected]>

.github/workflows/flutter.yaml

@@ -1,5 +1,5 @@
 name: Application ON Push & PR DO Code check
-on: [push, pull_request]
+on: [ push, pull_request ]
 
 jobs:
   code-check:
@@ -17,7 +17,9 @@ jobs:
         run: flutter --version
 
       - name: Get dependencies
-        run: flutter pub get
+        run: |
+          flutter pub get
+          flutter pub get lint_test
 
       - name: Check formatting
         run: dart format . --set-exit-if-changed

.gitignore

@@ -1,3 +1,6 @@
+#IDEA Files
+/.idea/
+
 # Files and directories created by pub.
 .dart_tool/
 .packages

example/analysis_options.yaml

@@ -16,5 +16,6 @@ custom_lint:
     - avoid_late_keyword
     - avoid_global_state
     - avoid_returning_widgets
+    - avoid_unnecessary_setstate
     - double-literal-format
     - avoid-unnecessary-type-assertions

lib/lints/avoid_unnecessary_setstate/avoid_unnecessary_set_state_rule.dart

@@ -0,0 +1,45 @@
+import 'package:analyzer/error/listener.dart';
+import 'package:custom_lint_builder/custom_lint_builder.dart';
+import 'package:solid_lints/lints/avoid_unnecessary_setstate/visitor/avoid_unnecessary_set_state_visitor.dart';
+import 'package:solid_lints/models/rule_config.dart';
+import 'package:solid_lints/models/solid_lint_rule.dart';
+
+/// A rule which warns when setState is called inside initState, didUpdateWidget
+/// or build methods and when it's called from a sync method that is called
+/// inside those methods.
+class AvoidUnnecessarySetStateRule extends SolidLintRule {
+  /// The lint name of this lint rule that represents
+  /// the error whether we use setState in inappropriate way.
+  static const lintName = 'avoid_unnecessary_setstate';
+
+  AvoidUnnecessarySetStateRule._(super.config);
+
+  /// Creates a new instance of [AvoidUnnecessarySetStateRule]
+  /// based on the lint configuration.
+  factory AvoidUnnecessarySetStateRule.createRule(CustomLintConfigs configs) {
+    final rule = RuleConfig(
+      name: lintName,
+      configs: configs,
+      problemMessage: (_) => ''
+          'Avoid calling unnecessary setState. '
+          'Consider changing the state directly.',
+    );
+    return AvoidUnnecessarySetStateRule._(rule);
+  }
+
+  @override
+  void run(
+    CustomLintResolver resolver,
+    ErrorReporter reporter,
+    CustomLintContext context,
+  ) {
+    final visitor = AvoidUnnecessarySetStateVisitor();
+
+    context.registry.addClassDeclaration((node) {
+      visitor.visitClassDeclaration(node);
+      for (final element in visitor.setStateInvocations) {
+        reporter.reportErrorForNode(code, element);
+      }
+    });
+  }
+}

lib/lints/avoid_unnecessary_setstate/visitor/avoid_unnecessary_set_state_method_visitor.dart

@@ -0,0 +1,62 @@
+// 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';
+
+/// AST Visitor which finds all setState invocations and checks if they are
+/// necessary
+class AvoidUnnecessarySetStateMethodVisitor extends RecursiveAstVisitor<void> {
+  final Set<String> _classMethodsNames;
+  final Iterable<FunctionBody> _bodies;
+
+  final _setStateInvocations = <MethodInvocation>[];
+
+  /// All setState invocations
+  Iterable<MethodInvocation> get setStateInvocations => _setStateInvocations;
+
+  /// Constructor for AvoidUnnecessarySetStateMethodVisitor
+  AvoidUnnecessarySetStateMethodVisitor(this._classMethodsNames, this._bodies);
+
+  @override
+  void visitMethodInvocation(MethodInvocation node) {
+    super.visitMethodInvocation(node);
+
+    final name = node.methodName.name;
+    final notInBody = _isNotInFunctionBody(node);
+
+    if (name == 'setState' && notInBody) {
+      _setStateInvocations.add(node);
+    } else if (_classMethodsNames.contains(name) &&
+        notInBody &&
+        node.realTarget == null) {
+      _setStateInvocations.add(node);
+    }
+  }
+
+  bool _isNotInFunctionBody(MethodInvocation node) =>
+      node.thisOrAncestorMatching(
+        (parent) => parent is FunctionBody && !_bodies.contains(parent),
+      ) ==
+      null;
+}

lib/lints/avoid_unnecessary_setstate/visitor/avoid_unnecessary_set_state_visitor.dart

@@ -0,0 +1,70 @@
+// 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';
+import 'package:solid_lints/lints/avoid_unnecessary_setstate/visitor/avoid_unnecessary_set_state_method_visitor.dart';
+import 'package:solid_lints/utils/types_utils.dart';
+
+/// AST visitor which checks if class is State, in case yes checks its methods
+class AvoidUnnecessarySetStateVisitor extends RecursiveAstVisitor<void> {
+  static const _checkedMethods = [
+    'initState',
+    'didUpdateWidget',
+    'didChangeDependencies',
+    'build',
+  ];
+
+  final _setStateInvocations = <MethodInvocation>[];
+
+  /// All setState invocations in checkedMethods
+  Iterable<MethodInvocation> get setStateInvocations => _setStateInvocations;
+
+  @override
+  void visitClassDeclaration(ClassDeclaration node) {
+    super.visitClassDeclaration(node);
+
+    final type = node.extendsClause?.superclass.type;
+    if (type == null || !isWidgetStateOrSubclass(type)) {
+      return;
+    }
+
+    final declarations = node.members.whereType<MethodDeclaration>().toList();
+    final classMethodsNames =
+        declarations.map((declaration) => declaration.name.lexeme).toSet();
+    final bodies = declarations.map((declaration) => declaration.body).toList();
+    final methods = declarations
+        .where((member) => _checkedMethods.contains(member.name.lexeme))
+        .toList();
+
+    for (final method in methods) {
+      final visitor =
+          AvoidUnnecessarySetStateMethodVisitor(classMethodsNames, bodies);
+      method.visitChildren(visitor);
+
+      _setStateInvocations.addAll([
+        ...visitor.setStateInvocations,
+      ]);
+    }
+  }
+}

lib/solid_lints.dart

@@ -5,6 +5,7 @@ import 'package:solid_lints/lints/avoid_global_state/avoid_global_state_rule.dar
 import 'package:solid_lints/lints/avoid_late_keyword/avoid_late_keyword_rule.dart';
 import 'package:solid_lints/lints/avoid_non_null_assertion/avoid_non_null_assertion_rule.dart';
 import 'package:solid_lints/lints/avoid_returning_widgets/avoid_returning_widgets_rule.dart';
+import 'package:solid_lints/lints/avoid_unnecessary_setstate/avoid_unnecessary_set_state_rule.dart';
 import 'package:solid_lints/lints/avoid_unnecessary_type_assertions/avoid_unnecessary_type_assertions_rule.dart';
 import 'package:solid_lints/lints/cyclomatic_complexity/cyclomatic_complexity_metric.dart';
 import 'package:solid_lints/lints/double_literal_format/double_literal_format_rule.dart';
@@ -29,6 +30,7 @@ class _SolidLints extends PluginBase {
       AvoidReturningWidgetsRule.createRule(configs),
       DoubleLiteralFormatRule.createRule(configs),
       AvoidUnnecessaryTypeAssertions.createRule(configs),
+      AvoidUnnecessarySetStateRule.createRule(configs),
     ];
 
     // Return only enabled rules

lint_test/analysis_options.yaml

@@ -0,0 +1,19 @@
+analyzer:
+  plugins:
+    - custom_lint
+
+custom_lint:
+  rules:
+    - cyclomatic_complexity:
+      max_complexity: 4
+    - number_of_parameters:
+      max_parameters: 2
+    - function_lines_of_code:
+      max_lines: 50
+    - avoid_non_null_assertion
+    - avoid_late_keyword
+    - avoid_global_state
+    - avoid_returning_widgets
+    - avoid_unnecessary_setstate
+    - double-literal-format
+    - avoid-unnecessary-type-assertions

example/test/avoid_returning_widget_test.dart → lint_test/avoid_returning_widget_test.dart

@@ -3,7 +3,7 @@
 /// Check returning a widget fail
 /// `avoid_returning_widgets`
 
-import 'package:flutter/widgets.dart';
+import 'package:flutter/material.dart';
 
 // expect_lint: avoid_returning_widgets
 Widget avoidReturningWidgets() => const SizedBox();