From 3e7945c75f2bb585dec2121ce978c63b08178e9c Mon Sep 17 00:00:00 2001 From: Yaroslav <43727448+laptevw@users.noreply.github.com> Date: Mon, 4 Sep 2023 14:15:59 +0300 Subject: [PATCH] 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 Co-authored-by: vladimir-beloded --- .github/workflows/flutter.yaml | 6 +- .gitignore | 3 + .idea/.gitignore | 8 -- .idea/dictionaries | 6 -- .idea/encodings.xml | 6 -- .idea/libraries/Dart_SDK.xml | 28 ------ .idea/misc.xml | 6 -- .idea/modules.xml | 8 -- .idea/vcs.xml | 6 -- example/analysis_options.yaml | 1 + .../avoid_unnecessary_set_state_rule.dart | 45 +++++++++ ..._unnecessary_set_state_method_visitor.dart | 62 ++++++++++++ .../avoid_unnecessary_set_state_visitor.dart | 70 +++++++++++++ lib/solid_lints.dart | 2 + lint_test/analysis_options.yaml | 19 ++++ .../avoid_global_state_test.dart | 0 .../avoid_late_keyword_test.dart | 0 .../avoid_non_null_assertion_test.dart | 0 .../avoid_returning_widget_test.dart | 2 +- .../avoid_unnecessary_setstate_test.dart | 99 +++++++++++++++++++ ...void_unnecessary_type_assertions_test.dart | 0 .../cyclomatic_complexity_test.dart | 0 .../double_literal_format_test.dart | 0 .../lines_of_code_test.dart | 0 .../number_of_parameters_test.dart | 0 lint_test/pubspec.yaml | 15 +++ pubspec.yaml | 7 ++ 27 files changed, 328 insertions(+), 71 deletions(-) delete mode 100644 .idea/.gitignore delete mode 100644 .idea/dictionaries delete mode 100644 .idea/encodings.xml delete mode 100644 .idea/libraries/Dart_SDK.xml delete mode 100644 .idea/misc.xml delete mode 100644 .idea/modules.xml delete mode 100644 .idea/vcs.xml create mode 100644 lib/lints/avoid_unnecessary_setstate/avoid_unnecessary_set_state_rule.dart create mode 100644 lib/lints/avoid_unnecessary_setstate/visitor/avoid_unnecessary_set_state_method_visitor.dart create mode 100644 lib/lints/avoid_unnecessary_setstate/visitor/avoid_unnecessary_set_state_visitor.dart create mode 100644 lint_test/analysis_options.yaml rename {example/test => lint_test}/avoid_global_state_test.dart (100%) rename {example/test => lint_test}/avoid_late_keyword_test.dart (100%) rename {example/test => lint_test}/avoid_non_null_assertion_test.dart (100%) rename {example/test => lint_test}/avoid_returning_widget_test.dart (94%) create mode 100644 lint_test/avoid_unnecessary_setstate_test.dart rename {example/test => lint_test}/avoid_unnecessary_type_assertions_test.dart (100%) rename {example/test => lint_test}/cyclomatic_complexity_test.dart (100%) rename {example/test => lint_test}/double_literal_format_test.dart (100%) rename {example/test => lint_test}/lines_of_code_test.dart (100%) rename {example/test => lint_test}/number_of_parameters_test.dart (100%) create mode 100644 lint_test/pubspec.yaml diff --git a/.github/workflows/flutter.yaml b/.github/workflows/flutter.yaml index c4324493..e0068f71 100644 --- a/.github/workflows/flutter.yaml +++ b/.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 diff --git a/.gitignore b/.gitignore index 2c1b5848..a273ea4e 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,6 @@ +#IDEA Files +/.idea/ + # Files and directories created by pub. .dart_tool/ .packages diff --git a/.idea/.gitignore b/.idea/.gitignore deleted file mode 100644 index 13566b81..00000000 --- a/.idea/.gitignore +++ /dev/null @@ -1,8 +0,0 @@ -# Default ignored files -/shelf/ -/workspace.xml -# Editor-based HTTP Client requests -/httpRequests/ -# Datasource local storage ignored files -/dataSources/ -/dataSources.local.xml diff --git a/.idea/dictionaries b/.idea/dictionaries deleted file mode 100644 index 60179635..00000000 --- a/.idea/dictionaries +++ /dev/null @@ -1,6 +0,0 @@ - - - - - - \ No newline at end of file diff --git a/.idea/encodings.xml b/.idea/encodings.xml deleted file mode 100644 index 97626ba4..00000000 --- a/.idea/encodings.xml +++ /dev/null @@ -1,6 +0,0 @@ - - - - - - \ No newline at end of file diff --git a/.idea/libraries/Dart_SDK.xml b/.idea/libraries/Dart_SDK.xml deleted file mode 100644 index 62be7eab..00000000 --- a/.idea/libraries/Dart_SDK.xml +++ /dev/null @@ -1,28 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - \ No newline at end of file diff --git a/.idea/misc.xml b/.idea/misc.xml deleted file mode 100644 index 639900d1..00000000 --- a/.idea/misc.xml +++ /dev/null @@ -1,6 +0,0 @@ - - - - - - \ No newline at end of file diff --git a/.idea/modules.xml b/.idea/modules.xml deleted file mode 100644 index e821027b..00000000 --- a/.idea/modules.xml +++ /dev/null @@ -1,8 +0,0 @@ - - - - - - - - \ No newline at end of file diff --git a/.idea/vcs.xml b/.idea/vcs.xml deleted file mode 100644 index 94a25f7f..00000000 --- a/.idea/vcs.xml +++ /dev/null @@ -1,6 +0,0 @@ - - - - - - \ No newline at end of file diff --git a/example/analysis_options.yaml b/example/analysis_options.yaml index ddb52521..ccddb430 100644 --- a/example/analysis_options.yaml +++ b/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 diff --git a/lib/lints/avoid_unnecessary_setstate/avoid_unnecessary_set_state_rule.dart b/lib/lints/avoid_unnecessary_setstate/avoid_unnecessary_set_state_rule.dart new file mode 100644 index 00000000..4a61352c --- /dev/null +++ b/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); + } + }); + } +} diff --git a/lib/lints/avoid_unnecessary_setstate/visitor/avoid_unnecessary_set_state_method_visitor.dart b/lib/lints/avoid_unnecessary_setstate/visitor/avoid_unnecessary_set_state_method_visitor.dart new file mode 100644 index 00000000..36546be8 --- /dev/null +++ b/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 { + final Set _classMethodsNames; + final Iterable _bodies; + + final _setStateInvocations = []; + + /// All setState invocations + Iterable 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; +} diff --git a/lib/lints/avoid_unnecessary_setstate/visitor/avoid_unnecessary_set_state_visitor.dart b/lib/lints/avoid_unnecessary_setstate/visitor/avoid_unnecessary_set_state_visitor.dart new file mode 100644 index 00000000..b9d79824 --- /dev/null +++ b/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 { + static const _checkedMethods = [ + 'initState', + 'didUpdateWidget', + 'didChangeDependencies', + 'build', + ]; + + final _setStateInvocations = []; + + /// All setState invocations in checkedMethods + Iterable 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().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, + ]); + } + } +} diff --git a/lib/solid_lints.dart b/lib/solid_lints.dart index 6ce8a3fc..41088533 100644 --- a/lib/solid_lints.dart +++ b/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 diff --git a/lint_test/analysis_options.yaml b/lint_test/analysis_options.yaml new file mode 100644 index 00000000..f940145c --- /dev/null +++ b/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 diff --git a/example/test/avoid_global_state_test.dart b/lint_test/avoid_global_state_test.dart similarity index 100% rename from example/test/avoid_global_state_test.dart rename to lint_test/avoid_global_state_test.dart diff --git a/example/test/avoid_late_keyword_test.dart b/lint_test/avoid_late_keyword_test.dart similarity index 100% rename from example/test/avoid_late_keyword_test.dart rename to lint_test/avoid_late_keyword_test.dart diff --git a/example/test/avoid_non_null_assertion_test.dart b/lint_test/avoid_non_null_assertion_test.dart similarity index 100% rename from example/test/avoid_non_null_assertion_test.dart rename to lint_test/avoid_non_null_assertion_test.dart diff --git a/example/test/avoid_returning_widget_test.dart b/lint_test/avoid_returning_widget_test.dart similarity index 94% rename from example/test/avoid_returning_widget_test.dart rename to lint_test/avoid_returning_widget_test.dart index 53e28cb7..48d414d3 100644 --- a/example/test/avoid_returning_widget_test.dart +++ b/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(); diff --git a/lint_test/avoid_unnecessary_setstate_test.dart b/lint_test/avoid_unnecessary_setstate_test.dart new file mode 100644 index 00000000..2d550dfe --- /dev/null +++ b/lint_test/avoid_unnecessary_setstate_test.dart @@ -0,0 +1,99 @@ +// 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:flutter/material.dart'; + +/// Check unnecessary setstate fail +/// `avoid_unnecessary_setstate` +class MyWidget extends StatefulWidget { + @override + _MyWidgetState createState() => _MyWidgetState(); +} + +class _MyWidgetState extends State { + String _myString = ''; + final bool _condition = true; + + @override + void initState() { + super.initState(); + + // expect_lint: avoid_unnecessary_setstate + setState(() { + _myString = "Hello"; + }); + + if (_condition) { + // expect_lint: avoid_unnecessary_setstate + setState(() { + _myString = "Hello"; + }); + } + + // expect_lint: avoid_unnecessary_setstate + myStateUpdateMethod(); + } + + @override + void didUpdateWidget(MyWidget oldWidget) { + super.didUpdateWidget(oldWidget); + // expect_lint: avoid_unnecessary_setstate + setState(() { + _myString = "Hello"; + }); + } + + void myStateUpdateMethod() { + setState(() { + _myString = "Hello"; + }); + } + + @override + Widget build(BuildContext context) { + // expect_lint: avoid_unnecessary_setstate + setState(() { + _myString = "Hello"; + }); + + if (_condition) { + // expect_lint: avoid_unnecessary_setstate + setState(() { + _myString = "Hello"; + }); + } + + // expect_lint: avoid_unnecessary_setstate + myStateUpdateMethod(); + + return ElevatedButton( + onPressed: myStateUpdateMethod, + onLongPress: () { + setState(() { + _myString = 'data'; + }); + }, + child: Text(_myString), + ); + } +} diff --git a/example/test/avoid_unnecessary_type_assertions_test.dart b/lint_test/avoid_unnecessary_type_assertions_test.dart similarity index 100% rename from example/test/avoid_unnecessary_type_assertions_test.dart rename to lint_test/avoid_unnecessary_type_assertions_test.dart diff --git a/example/test/cyclomatic_complexity_test.dart b/lint_test/cyclomatic_complexity_test.dart similarity index 100% rename from example/test/cyclomatic_complexity_test.dart rename to lint_test/cyclomatic_complexity_test.dart diff --git a/example/test/double_literal_format_test.dart b/lint_test/double_literal_format_test.dart similarity index 100% rename from example/test/double_literal_format_test.dart rename to lint_test/double_literal_format_test.dart diff --git a/example/test/lines_of_code_test.dart b/lint_test/lines_of_code_test.dart similarity index 100% rename from example/test/lines_of_code_test.dart rename to lint_test/lines_of_code_test.dart diff --git a/example/test/number_of_parameters_test.dart b/lint_test/number_of_parameters_test.dart similarity index 100% rename from example/test/number_of_parameters_test.dart rename to lint_test/number_of_parameters_test.dart diff --git a/lint_test/pubspec.yaml b/lint_test/pubspec.yaml new file mode 100644 index 00000000..cb37cd0b --- /dev/null +++ b/lint_test/pubspec.yaml @@ -0,0 +1,15 @@ +name: solid_lints_test +description: A starting point for Dart libraries or applications. +publish_to: none + +environment: + sdk: '>=3.0.0 <4.0.0' + +dependencies: + flutter: + sdk: flutter + +dev_dependencies: + solid_lints: + path: ../ + test: ^1.20.1 diff --git a/pubspec.yaml b/pubspec.yaml index 6995c6ab..649539c7 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -12,3 +12,10 @@ dependencies: analyzer: ^5.12.0 collection: ^1.17.2 custom_lint_builder: ^0.5.0 + +dev_dependencies: +# These packages are mandatory for some of tests + flutter: + sdk: flutter + test: ^1.24.6 +