From ac790526745615ff2d893e3c4b43929418e8e318 Mon Sep 17 00:00:00 2001 From: daria-trusca-solid Date: Wed, 3 Jun 2026 11:54:33 +0300 Subject: [PATCH 1/7] Updated avoid_unnecessary_return_variable rule and its visitor to use AnalysisRule. --- ...void_unnecessary_return_variable_rule.dart | 131 ++++-------------- ...d_unnecessary_return_variable_visitor.dart | 110 +++++++-------- .../return_variable_usage_visitor.dart | 73 ++++++++++ 3 files changed, 157 insertions(+), 157 deletions(-) create mode 100644 lib/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart diff --git a/lib/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart b/lib/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart index 90cd9c7d..7293c416 100644 --- a/lib/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart +++ b/lib/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart @@ -1,11 +1,8 @@ -import 'package:analyzer/dart/ast/ast.dart'; -import 'package:analyzer/dart/element/element.dart'; -import 'package:analyzer/error/listener.dart'; -import 'package:custom_lint_builder/custom_lint_builder.dart'; -import 'package:solid_lints/src/common/visitors/select_expression_identifiers_visitor.dart'; +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/error/error.dart'; import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart'; -import 'package:solid_lints/src/models/rule_config.dart'; -import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// An `avoid_unnecessary_return_variable` rule which forbids returning /// an immutable variable if it can be rewritten in return statement itself. @@ -32,109 +29,37 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// } /// ``` /// -class AvoidUnnecessaryReturnVariableRule extends SolidLintRule { - /// This lint rule represents the error - /// when unnecessary return variable statement is found +class AvoidUnnecessaryReturnVariableRule extends AnalysisRule { + /// The name of the lint rule. static const lintName = 'avoid_unnecessary_return_variable'; - AvoidUnnecessaryReturnVariableRule._(super.config); - - /// Creates a new instance of [AvoidUnnecessaryReturnVariableRule] - /// based on the lint configuration. - factory AvoidUnnecessaryReturnVariableRule.createRule( - CustomLintConfigs configs, - ) { - final rule = RuleConfig( - configs: configs, - name: lintName, - problemMessage: (_) => """ + /// The message shown when the lint is triggered. + static const String lintMessage = """ Avoid creating unnecessary variable only for return. -Rewrite the variable evaluation into return statement instead.""", - ); +Rewrite the variable evaluation into return statement instead."""; - return AvoidUnnecessaryReturnVariableRule._(rule); - } + /// Lint code. + static const LintCode _code = LintCode( + lintName, + lintMessage, + ); - @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, - ) { - context.registry.addReturnStatement( - (statement) { - _checkReturnStatement( - statement: statement, - reporter: reporter, - context: context, + /// Creates a new instance of [AvoidUnnecessaryReturnVariableRule] + AvoidUnnecessaryReturnVariableRule() + : super( + name: lintName, + description: lintMessage, ); - }, - ); - } - - void _checkReturnStatement({ - required ReturnStatement statement, - required DiagnosticReporter reporter, - required CustomLintContext context, - }) { - final expr = statement.expression; - if (expr is! SimpleIdentifier) return; - - final element = expr.element; - if (element is! LocalVariableElement) return; - final returnVariableElement = element; - - if (!returnVariableElement.isFinal && !returnVariableElement.isConst) { - return; - } - - final blockBody = statement.parent; - if (blockBody == null) return; - - final visitor = AvoidUnnecessaryReturnVariableVisitor( - returnVariableElement, - statement, - ); - blockBody.visitChildren(visitor); - - if (!visitor.hasBadStatementCount()) return; - //it is 100% bad if return statement follows declaration - if (!visitor.foundTokensBetweenDeclarationAndReturn) { - reporter.atNode(statement, code); - return; - } - - final declaration = visitor.variableDeclaration; - - //check if immutable - final initializer = declaration?.initializer; - if (initializer == null) return; - - if (!_isExpressionImmutable(initializer)) return; - - reporter.atNode(statement, code); - } - - bool _isExpressionImmutable(Expression expr) { - final visitor = SelectExpressionIdentifiersVisitor(); - expr.accept(visitor); - - return visitor.identifiers.every(_isSimpleIdentifierImmutable); - } - - bool _isSimpleIdentifierImmutable(SimpleIdentifier identifier) { - switch (identifier.element) { - case final VariableElement variable: - return variable.isFinal || variable.isConst; - - case ClassElement _: - return true; - - case GetterElement(:final PropertyInducingElement variable): - return variable.isFinal || variable.isConst; - } + @override + LintCode get diagnosticCode => _code; - return false; + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + final visitor = AvoidUnnecessaryReturnVariableVisitor(this); + registry.addReturnStatement(this, visitor); } } diff --git a/lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart b/lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart index bbc7455d..e6405e12 100644 --- a/lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart +++ b/lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart @@ -1,79 +1,81 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/dart/element/element.dart'; -import 'package:collection/collection.dart'; +import 'package:solid_lints/src/common/visitors/select_expression_identifiers_visitor.dart'; +import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart'; +import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart'; -/// This visitor is searches all uses of a single local variable, -/// including variable declaration. +/// Visitor for [AvoidUnnecessaryReturnVariableRule]. class AvoidUnnecessaryReturnVariableVisitor extends RecursiveAstVisitor { - /// The problem expects that exactly 1 mention of return variable. - /// VariableDeclarationStatement doesn't count when visiting SimpleIdentifier. - /// Any other amount of variable mentions implies that it is used somewhere - /// except return, so its existence is justified. - static const _badStatementCount = 1; + /// The rule associated with this visitor. + final AvoidUnnecessaryReturnVariableRule rule; - final LocalVariableElement _returnVariableElement; + /// Creates a new instance of [AvoidUnnecessaryReturnVariableVisitor]. + AvoidUnnecessaryReturnVariableVisitor(this.rule); - bool _foundTokensBetweenDeclarationAndReturn = false; - VariableDeclaration? _variableDeclaration; - int _variableStatementCounter = 0; + @override + void visitReturnStatement(ReturnStatement node) { + final expr = node.expression; + if (expr is! SimpleIdentifier) { + return; + } - final ReturnStatement _returnStatement; + final element = expr.element; + if (element is! LocalVariableElement) { + return; + } - /// After visiting holds info about whether there are any tokens - /// between variable declaration and return statement - bool get foundTokensBetweenDeclarationAndReturn => - _foundTokensBetweenDeclarationAndReturn; + if (!element.isFinal && !element.isConst) { + return; + } - /// Returns statement of local variable declaration - VariableDeclaration? get variableDeclaration => _variableDeclaration; + final block = node.parent; + if (block == null) return; - /// Creates a new instance of [AvoidUnnecessaryReturnVariableVisitor]. - AvoidUnnecessaryReturnVariableVisitor( - this._returnVariableElement, - this._returnStatement, - ); + final returnVariableUsageVisitor = + ReturnVariableUsageVisitor(node, element); - /// Defines whether the variables is used in return statement only. - bool hasBadStatementCount() => - _variableStatementCounter == _badStatementCount; + block.visitChildren(returnVariableUsageVisitor); + if (!returnVariableUsageVisitor.hasBadStatementCount()) return; - @override - void visitVariableDeclarationStatement(VariableDeclarationStatement node) { - if (_collectVariableDeclaration(node)) { - _checkTokensInBetween(node, _returnStatement); + //it is 100% bad if return statement follows declaration + if (!returnVariableUsageVisitor.foundTokensBetweenDeclarationAndReturn) { + rule.reportAtNode(node); + return; } - super.visitVariableDeclarationStatement(node); - } - @override - void visitSimpleIdentifier(SimpleIdentifier node) { - if (node.element?.id == _returnVariableElement.id) { - _variableStatementCounter++; - } + final declaration = returnVariableUsageVisitor.variableDeclaration; + + //check if immutable + final initializer = declaration?.initializer; + if (initializer == null) return; + + if (!_isExpressionImmutable(initializer)) return; - super.visitSimpleIdentifier(node); + rule.reportAtNode(node); + + super.visitReturnStatement(node); } - bool _collectVariableDeclaration(VariableDeclarationStatement node) { - final targetVariable = node.variables.variables.firstWhereOrNull( - (v) => v.declaredFragment?.element.id == _returnVariableElement.id, - ); - if (targetVariable == null) return false; + bool _isExpressionImmutable(Expression expr) { + final visitor = SelectExpressionIdentifiersVisitor(); + expr.accept(visitor); - _variableDeclaration = targetVariable; - return true; + return visitor.identifiers.every(_isSimpleIdentifierImmutable); } - void _checkTokensInBetween( - VariableDeclarationStatement variableDeclaration, - ReturnStatement returnStatement, - ) { - final tokenBeforeReturn = - _returnStatement.findPrevious(_returnStatement.beginToken); + bool _isSimpleIdentifierImmutable(SimpleIdentifier identifier) { + switch (identifier.element) { + case final VariableElement variable: + return variable.isFinal || variable.isConst; + + case ClassElement _: + return true; - if (tokenBeforeReturn != variableDeclaration.endToken) { - _foundTokensBetweenDeclarationAndReturn = true; + case GetterElement(:final PropertyInducingElement variable): + return variable.isFinal || variable.isConst; } + + return false; } } diff --git a/lib/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart b/lib/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart new file mode 100644 index 00000000..2e274732 --- /dev/null +++ b/lib/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart @@ -0,0 +1,73 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/element.dart'; +import 'package:collection/collection.dart'; + +/// The AST visitor that will collect every Variable Declaration statement +class ReturnVariableUsageVisitor extends RecursiveAstVisitor { + final ReturnStatement _returnStatement; + final LocalVariableElement _returnVariableElement; + + /// After visiting holds the declaration of return variable + VariableDeclaration? variableDeclaration; + + /// After visiting holds info about whether there are any tokens + bool foundTokensBetweenDeclarationAndReturn = false; + + /// The problem expects that exactly 1 mention of return variable. + /// VariableDeclarationStatement doesn't count when visiting SimpleIdentifier. + /// Any other amount of variable mentions implies that it is used somewhere + /// except return, so its existence is justified. + static const _badStatementCount = 1; + + int _variableStatementCounter = 0; + + /// Defines whether the variables is used in return statement only. + bool hasBadStatementCount() => + _variableStatementCounter == _badStatementCount; + + /// Creates a new instance of [ReturnVariableUsageVisitor]. + ReturnVariableUsageVisitor( + this._returnStatement, + this._returnVariableElement, + ); + + @override + void visitVariableDeclarationStatement(VariableDeclarationStatement node) { + if (_collectVariableDeclaration(node)) { + _checkTokensInBetween(node, _returnStatement); + } + super.visitVariableDeclarationStatement(node); + } + + @override + void visitSimpleIdentifier(SimpleIdentifier node) { + if (node.element?.id == _returnVariableElement.id) { + _variableStatementCounter++; + } + + super.visitSimpleIdentifier(node); + } + + bool _collectVariableDeclaration(VariableDeclarationStatement node) { + final targetVariable = node.variables.variables.firstWhereOrNull( + (v) => v.declaredFragment?.element.id == _returnVariableElement.id, + ); + if (targetVariable == null) return false; + variableDeclaration = targetVariable; + + return true; + } + + void _checkTokensInBetween( + VariableDeclarationStatement variableDeclaration, + ReturnStatement returnStatement, + ) { + final tokenBeforeReturn = + _returnStatement.findPrevious(_returnStatement.beginToken); + + if (tokenBeforeReturn != variableDeclaration.endToken) { + foundTokensBetweenDeclarationAndReturn = true; + } + } +} From e03d583e03ed43c143708f7ba438359e1769d3be Mon Sep 17 00:00:00 2001 From: daria-trusca-solid Date: Wed, 3 Jun 2026 11:55:00 +0300 Subject: [PATCH 2/7] Updated tests for avoid_unnecessary_return_variable rule --- ...unnecessary_return_variable_rule_test.dart | 180 ++++++++++++++++++ 1 file changed, 180 insertions(+) create mode 100644 test/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule_test.dart diff --git a/test/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule_test.dart b/test/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule_test.dart new file mode 100644 index 00000000..9bf78468 --- /dev/null +++ b/test/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule_test.dart @@ -0,0 +1,180 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(AvoidUnnecessaryReturnVariableTest); + }); +} + +@reflectiveTest +class AvoidUnnecessaryReturnVariableTest extends AnalysisRuleTest { + @override + void setUp() { + rule = AvoidUnnecessaryReturnVariableRule(); + super.setUp(); + } + + @override + String get analysisRule => AvoidUnnecessaryReturnVariableRule.lintName; + + void test_does_not_report_if_return_good_trivial() async { + await assertNoDiagnostics( + r''' +int returnVarTestTrivial() { + return 1; +} + ''', + ); + } + + void test_does_not_report_if_return_is_mutable() async { + await assertNoDiagnostics( + r''' +int returnVarTestMutable() { + var a = 1; + a++; + + return a; +} + ''', + ); + } + + void test_does_not_report_if_returns_parameter() async { + await assertNoDiagnostics( + r''' +int returnVarTestReturnParameter(int param) { + return param; +} + ''', + ); + } + + void test_does_not_report_if_return_is_cached_mutable() async { + await assertNoDiagnostics( + r''' +int returnVarTestCachedMutable() { + var a = 1; + final result = a; + _doNothing(); + + return result; +} + +void _doNothing() {} + ''', + ); + } + + void test_reports_if_return_follows_declaration() async { + await assertDiagnostics(r''' +int returnVarTestReturnFollowsDeclaration() { + var a = 1; + final result = a; + + //Some comment here + + return result; +} + ''', [lint(105, 14)]); + } + + void test_does_not_report_if_return_is_cached_another_method_result() async { + await assertNoDiagnostics( + r''' +int returnVarTestCachedAnotherMethodResult() { + var a = 1; + final result = _testValueEval(); + _doNothing(); + + return result; +} + +int _testValueEval() { + return 1; +} + +void _doNothing() {} +''', + ); + } + + void test_does_not_report_if_return_is_cached_object_field() async { + await assertNoDiagnostics( + r''' +int returnVarTestCachedObjectField() { + final obj = _TestClass(); + final result = obj.varField; + _doNothing(); + + return result; +} + +class _TestClass { + static const constValue = 1; + static final finalValue = 1; + //ignore: member_ordering + final finalField = 1; + var varField = 1; +} + +void _doNothing() {} +''', + ); + } + + void test_does_not_report_if_return_used_variable() async { + await assertNoDiagnostics( + r''' +int returnVarTestUsedVariable() { + var a = 1; + final result = 2; + a += result; + + return result; +} +''', + ); + } + + void test_reports_if_return_is_bad_trivial() async { + await assertDiagnostics(r''' +int returnVarTestBadTrivial() { + final result = 1; + + return result; +} +''', [lint(55, 14)]); + } + + void test_reports_if_return_is_bad_immutable_expression() async { + await assertDiagnostics(r''' +int returnVarTestBadImmutableExpression() { + const constLocal = 1; + final finalLocal = 1; + final testObj = _TestClass(); + final result = constLocal + + finalLocal + + 1 + //const literal + _TestClass.constValue + + _TestClass.finalValue + + testObj.finalField; + _doNothing(); + + return result; +} + +class _TestClass { + static const constValue = 1; + static final finalValue = 1; + //ignore: member_ordering + final finalField = 1; + var varField = 1; +} + +void _doNothing() {} +''', [lint(304, 14)]); + } +} From ea77879d9d5f8af95e3c1a076977215349c5f024 Mon Sep 17 00:00:00 2001 From: daria-trusca-solid Date: Wed, 3 Jun 2026 11:56:31 +0300 Subject: [PATCH 3/7] Deleted previous test file for avoid_unnecessary_return_variable rule. --- ...void_unnecessary_return_variable_test.dart | 125 ------------------ 1 file changed, 125 deletions(-) delete mode 100644 lint_test/avoid_unnecessary_return_variable_test.dart diff --git a/lint_test/avoid_unnecessary_return_variable_test.dart b/lint_test/avoid_unnecessary_return_variable_test.dart deleted file mode 100644 index 1e068180..00000000 --- a/lint_test/avoid_unnecessary_return_variable_test.dart +++ /dev/null @@ -1,125 +0,0 @@ -// ignore_for_file: unused_local_variable, newline_before_return, no_empty_block - -/// Test the avoid_unnecessary_return_variable. -/// Good code, trivial case. -int returnVarTestGoodTrivial() { - return 1; -} - -/// Test the avoid_unnecessary_return_variable. -/// Returning mutable variable should not trigger the lint. -int returnVarTestReturnMutable() { - var a = 1; - a++; - - return a; -} - -int returnVarTestReturnParameter(int param) { - return param; -} - -/// Test the avoid_unnecessary_return_variable. -/// Caching mutable variable value. -/// Unpredictable: may be useful to cache value -/// before operation can change it. -int returnVarTestCachedMutable() { - var a = 1; - final result = a; - _doNothing(); - - return result; -} - -/// Test the avoid_unnecessary_return_variable. -/// Caching mutable variable value, but return goes -/// right after declaration, which makes it bad. -int returnVarTestReturnFollowsDeclaration() { - var a = 1; - final result = a; - - //Some comment here - - //expect_lint: avoid_unnecessary_return_variable - return result; -} - -/// Test the avoid_unnecessary_return_variable. -/// Caching another method result. -/// Unpredictable: may be useful to cache value -/// before operation can change it. -int returnVarTestCachedAnotherMethodResult() { - var a = 1; - final result = _testValueEval(); - _doNothing(); - - return result; -} - -/// Test the avoid_unnecessary_return_variable. -/// Caching value of object's field. -/// Unpredictable: may be useful to cache value -/// before operation can change it. -int returnVarTestCachedObjectField() { - final obj = _TestClass(); - final result = obj.varField; - _doNothing(); - - return result; -} - -/// Test the avoid_unnecessary_return_variable. -/// Good: variable is created not only for return -/// but is used in following expressions as well. -int returnVarTestUsedVariable() { - var a = 1; - final result = 2; - a += result; - - return result; -} - -/// Test the avoid_unnecessary_return_variable. -/// Bad code, trivial example. -int returnVarTestBadTrivial() { - final result = 1; - - //expect_lint: avoid_unnecessary_return_variable - return result; -} - -/// Test the avoid_unnecessary_return_variable. -/// Bad code: result expression is immutable, -/// so can be written in return statement directly. -int returnVarTestBadImmutableExpression() { - const constLocal = 1; - final finalLocal = 1; - final testObj = _TestClass(); - final result = constLocal + - finalLocal + - 1 + //const literal - _TestClass.constValue + - _TestClass.finalValue + - testObj.finalField; - _doNothing(); - - //expect_lint: avoid_unnecessary_return_variable - return result; -} - -int _testValueEval() { - return 1; -} - -/// This method is a placeholder for unpredictable behaviour -/// which can potentially change any mutable variables -void _doNothing() {} - -//ignore: prefer_match_file_name -class _TestClass { - static const constValue = 1; - static final finalValue = 1; - //ignore: member_ordering - final finalField = 1; - var varField = 1; -} From 49c9fa7711f1ac02fc400ae799f2028ee044f9b9 Mon Sep 17 00:00:00 2001 From: daria-trusca-solid Date: Wed, 3 Jun 2026 12:05:32 +0300 Subject: [PATCH 4/7] Updated ReturnVariableUsageVisitor description. --- .../visitors/return_variable_usage_visitor.dart | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart b/lib/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart index 2e274732..ff46b0f0 100644 --- a/lib/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart +++ b/lib/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart @@ -2,8 +2,11 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/dart/element/element.dart'; import 'package:collection/collection.dart'; +import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart'; -/// The AST visitor that will collect every Variable Declaration statement +/// Visitor for [AvoidUnnecessaryReturnVariableRule] which checks whether the +/// return variable is used somewhere except return statement and +/// whether it is immutable. class ReturnVariableUsageVisitor extends RecursiveAstVisitor { final ReturnStatement _returnStatement; final LocalVariableElement _returnVariableElement; From 9abcdde2e2961814153b764c2b69ced9c1e7126c Mon Sep 17 00:00:00 2001 From: daria-trusca-solid Date: Wed, 3 Jun 2026 12:31:18 +0300 Subject: [PATCH 5/7] Updated avoid_unnecessary_return_variable visitor from recursive to simple visitor. --- .../visitors/avoid_unnecessary_return_variable_visitor.dart | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart b/lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart index e6405e12..cdcea4dd 100644 --- a/lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart +++ b/lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart @@ -6,7 +6,7 @@ import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/avoid_un import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart'; /// Visitor for [AvoidUnnecessaryReturnVariableRule]. -class AvoidUnnecessaryReturnVariableVisitor extends RecursiveAstVisitor { +class AvoidUnnecessaryReturnVariableVisitor extends SimpleAstVisitor { /// The rule associated with this visitor. final AvoidUnnecessaryReturnVariableRule rule; @@ -53,8 +53,6 @@ class AvoidUnnecessaryReturnVariableVisitor extends RecursiveAstVisitor { if (!_isExpressionImmutable(initializer)) return; rule.reportAtNode(node); - - super.visitReturnStatement(node); } bool _isExpressionImmutable(Expression expr) { From bb53722eab0ee4d678bdefb2ab6410b63cd82d6a Mon Sep 17 00:00:00 2001 From: daria-trusca-solid Date: Thu, 4 Jun 2026 10:33:35 +0300 Subject: [PATCH 6/7] Address PR suggestions --- ...void_unnecessary_return_variable_rule.dart | 16 ++--- ...d_unnecessary_return_variable_visitor.dart | 39 +++++------ .../return_variable_usage_visitor.dart | 19 +++--- pubspec.yaml | 2 +- ...unnecessary_return_variable_rule_test.dart | 65 +++++++++---------- 5 files changed, 64 insertions(+), 77 deletions(-) diff --git a/lib/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart b/lib/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart index 7293c416..4068d342 100644 --- a/lib/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart +++ b/lib/src/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule.dart @@ -31,25 +31,25 @@ import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/visitors /// class AvoidUnnecessaryReturnVariableRule extends AnalysisRule { /// The name of the lint rule. - static const lintName = 'avoid_unnecessary_return_variable'; + static const _lintName = 'avoid_unnecessary_return_variable'; /// The message shown when the lint is triggered. - static const String lintMessage = """ + static const String _lintMessage = """ Avoid creating unnecessary variable only for return. Rewrite the variable evaluation into return statement instead."""; /// Lint code. static const LintCode _code = LintCode( - lintName, - lintMessage, + _lintName, + _lintMessage, ); /// Creates a new instance of [AvoidUnnecessaryReturnVariableRule] AvoidUnnecessaryReturnVariableRule() - : super( - name: lintName, - description: lintMessage, - ); + : super( + name: _lintName, + description: _lintMessage, + ); @override LintCode get diagnosticCode => _code; diff --git a/lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart b/lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart index cdcea4dd..3bd64ea5 100644 --- a/lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart +++ b/lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart @@ -16,27 +16,23 @@ class AvoidUnnecessaryReturnVariableVisitor extends SimpleAstVisitor { @override void visitReturnStatement(ReturnStatement node) { final expr = node.expression; - if (expr is! SimpleIdentifier) { - return; - } + if (expr is! SimpleIdentifier) return; final element = expr.element; - if (element is! LocalVariableElement) { - return; - } + if (element is! LocalVariableElement) return; - if (!element.isFinal && !element.isConst) { - return; - } + if (!element.isFinal && !element.isConst) return; final block = node.parent; if (block == null) return; - final returnVariableUsageVisitor = - ReturnVariableUsageVisitor(node, element); + final returnVariableUsageVisitor = ReturnVariableUsageVisitor( + node, + element, + ); block.visitChildren(returnVariableUsageVisitor); - if (!returnVariableUsageVisitor.hasBadStatementCount()) return; + if (!returnVariableUsageVisitor.hasBadStatementCount) return; //it is 100% bad if return statement follows declaration if (!returnVariableUsageVisitor.foundTokensBetweenDeclarationAndReturn) { @@ -63,17 +59,12 @@ class AvoidUnnecessaryReturnVariableVisitor extends SimpleAstVisitor { } bool _isSimpleIdentifierImmutable(SimpleIdentifier identifier) { - switch (identifier.element) { - case final VariableElement variable: - return variable.isFinal || variable.isConst; - - case ClassElement _: - return true; - - case GetterElement(:final PropertyInducingElement variable): - return variable.isFinal || variable.isConst; - } - - return false; + return switch (identifier.element) { + ClassElement _ => true, + final VariableElement variable => variable.isFinal || variable.isConst, + GetterElement(:final PropertyInducingElement variable) => + variable.isFinal || variable.isConst, + _ => false, + }; } } diff --git a/lib/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart b/lib/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart index ff46b0f0..27702549 100644 --- a/lib/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart +++ b/lib/src/lints/avoid_unnecessary_return_variable/visitors/return_variable_usage_visitor.dart @@ -8,6 +8,12 @@ import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/avoid_un /// return variable is used somewhere except return statement and /// whether it is immutable. class ReturnVariableUsageVisitor extends RecursiveAstVisitor { + /// The problem expects that exactly 1 mention of return variable. + /// VariableDeclarationStatement doesn't count when visiting SimpleIdentifier. + /// Any other amount of variable mentions implies that it is used somewhere + /// except return, so its existence is justified. + static const _badStatementCount = 1; + final ReturnStatement _returnStatement; final LocalVariableElement _returnVariableElement; @@ -17,16 +23,10 @@ class ReturnVariableUsageVisitor extends RecursiveAstVisitor { /// After visiting holds info about whether there are any tokens bool foundTokensBetweenDeclarationAndReturn = false; - /// The problem expects that exactly 1 mention of return variable. - /// VariableDeclarationStatement doesn't count when visiting SimpleIdentifier. - /// Any other amount of variable mentions implies that it is used somewhere - /// except return, so its existence is justified. - static const _badStatementCount = 1; - int _variableStatementCounter = 0; /// Defines whether the variables is used in return statement only. - bool hasBadStatementCount() => + bool get hasBadStatementCount => _variableStatementCounter == _badStatementCount; /// Creates a new instance of [ReturnVariableUsageVisitor]. @@ -66,8 +66,9 @@ class ReturnVariableUsageVisitor extends RecursiveAstVisitor { VariableDeclarationStatement variableDeclaration, ReturnStatement returnStatement, ) { - final tokenBeforeReturn = - _returnStatement.findPrevious(_returnStatement.beginToken); + final tokenBeforeReturn = _returnStatement.findPrevious( + _returnStatement.beginToken, + ); if (tokenBeforeReturn != variableDeclaration.endToken) { foundTokensBetweenDeclarationAndReturn = true; diff --git a/pubspec.yaml b/pubspec.yaml index f85b1309..70168359 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -8,7 +8,7 @@ documentation: https://solid-software.github.io/solid_lints/docs/intro topics: [lints, linter, lint, analysis, analyzer] environment: - sdk: ">=3.5.0 <4.0.0" + sdk: ">=3.9.0 <4.0.0" dependencies: analyzer: ^10.0.1 diff --git a/test/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule_test.dart b/test/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule_test.dart index 9bf78468..d2e42b16 100644 --- a/test/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule_test.dart +++ b/test/lints/avoid_unnecessary_return_variable/avoid_unnecessary_return_variable_rule_test.dart @@ -17,44 +17,37 @@ class AvoidUnnecessaryReturnVariableTest extends AnalysisRuleTest { } @override - String get analysisRule => AvoidUnnecessaryReturnVariableRule.lintName; + String get analysisRule => rule.name; void test_does_not_report_if_return_good_trivial() async { - await assertNoDiagnostics( - r''' + await assertNoDiagnostics(r''' int returnVarTestTrivial() { return 1; } - ''', - ); + '''); } void test_does_not_report_if_return_is_mutable() async { - await assertNoDiagnostics( - r''' + await assertNoDiagnostics(r''' int returnVarTestMutable() { var a = 1; a++; return a; } - ''', - ); + '''); } void test_does_not_report_if_returns_parameter() async { - await assertNoDiagnostics( - r''' + await assertNoDiagnostics(r''' int returnVarTestReturnParameter(int param) { return param; } - ''', - ); + '''); } void test_does_not_report_if_return_is_cached_mutable() async { - await assertNoDiagnostics( - r''' + await assertNoDiagnostics(r''' int returnVarTestCachedMutable() { var a = 1; final result = a; @@ -64,12 +57,12 @@ int returnVarTestCachedMutable() { } void _doNothing() {} - ''', - ); + '''); } void test_reports_if_return_follows_declaration() async { - await assertDiagnostics(r''' + await assertDiagnostics( + r''' int returnVarTestReturnFollowsDeclaration() { var a = 1; final result = a; @@ -78,12 +71,13 @@ int returnVarTestReturnFollowsDeclaration() { return result; } - ''', [lint(105, 14)]); + ''', + [lint(105, 14)], + ); } void test_does_not_report_if_return_is_cached_another_method_result() async { - await assertNoDiagnostics( - r''' + await assertNoDiagnostics(r''' int returnVarTestCachedAnotherMethodResult() { var a = 1; final result = _testValueEval(); @@ -97,13 +91,11 @@ int _testValueEval() { } void _doNothing() {} -''', - ); +'''); } void test_does_not_report_if_return_is_cached_object_field() async { - await assertNoDiagnostics( - r''' + await assertNoDiagnostics(r''' int returnVarTestCachedObjectField() { final obj = _TestClass(); final result = obj.varField; @@ -121,13 +113,11 @@ class _TestClass { } void _doNothing() {} -''', - ); +'''); } void test_does_not_report_if_return_used_variable() async { - await assertNoDiagnostics( - r''' + await assertNoDiagnostics(r''' int returnVarTestUsedVariable() { var a = 1; final result = 2; @@ -135,22 +125,25 @@ int returnVarTestUsedVariable() { return result; } -''', - ); +'''); } void test_reports_if_return_is_bad_trivial() async { - await assertDiagnostics(r''' + await assertDiagnostics( + r''' int returnVarTestBadTrivial() { final result = 1; return result; } -''', [lint(55, 14)]); +''', + [lint(55, 14)], + ); } void test_reports_if_return_is_bad_immutable_expression() async { - await assertDiagnostics(r''' + await assertDiagnostics( + r''' int returnVarTestBadImmutableExpression() { const constLocal = 1; final finalLocal = 1; @@ -175,6 +168,8 @@ class _TestClass { } void _doNothing() {} -''', [lint(304, 14)]); +''', + [lint(304, 14)], + ); } } From 7a2bba5eb5a0cfcaf9257363e8235971a4b490be Mon Sep 17 00:00:00 2001 From: daria-trusca-solid Date: Thu, 4 Jun 2026 11:54:52 +0300 Subject: [PATCH 7/7] Updated rule to private in the visitor. --- .../avoid_unnecessary_return_variable_visitor.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart b/lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart index 3bd64ea5..ebfad8e4 100644 --- a/lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart +++ b/lib/src/lints/avoid_unnecessary_return_variable/visitors/avoid_unnecessary_return_variable_visitor.dart @@ -8,10 +8,10 @@ import 'package:solid_lints/src/lints/avoid_unnecessary_return_variable/visitors /// Visitor for [AvoidUnnecessaryReturnVariableRule]. class AvoidUnnecessaryReturnVariableVisitor extends SimpleAstVisitor { /// The rule associated with this visitor. - final AvoidUnnecessaryReturnVariableRule rule; + final AvoidUnnecessaryReturnVariableRule _rule; /// Creates a new instance of [AvoidUnnecessaryReturnVariableVisitor]. - AvoidUnnecessaryReturnVariableVisitor(this.rule); + AvoidUnnecessaryReturnVariableVisitor(this._rule); @override void visitReturnStatement(ReturnStatement node) { @@ -36,7 +36,7 @@ class AvoidUnnecessaryReturnVariableVisitor extends SimpleAstVisitor { //it is 100% bad if return statement follows declaration if (!returnVariableUsageVisitor.foundTokensBetweenDeclarationAndReturn) { - rule.reportAtNode(node); + _rule.reportAtNode(node); return; } @@ -48,7 +48,7 @@ class AvoidUnnecessaryReturnVariableVisitor extends SimpleAstVisitor { if (!_isExpressionImmutable(initializer)) return; - rule.reportAtNode(node); + _rule.reportAtNode(node); } bool _isExpressionImmutable(Expression expr) {