From c20a85d127a860a84a66527a2cb32b6f4d84cafd Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Thu, 18 Jun 2026 16:39:49 +0300 Subject: [PATCH 1/8] refactor: migrate function_lines_of_code rule (#251) --- lib/main.dart | 6 + .../function_lines_of_code_rule.dart | 113 ++-- .../function_lines_of_code_parameters.dart | 8 + .../function_lines_of_code_rule_visitor.dart | 76 +++ .../analysis_options.yaml | 14 - .../function_lines_of_code_test.dart | 535 ------------------ .../function_lines_of_code_rule_test.dart | 134 +++++ 7 files changed, 260 insertions(+), 626 deletions(-) create mode 100644 lib/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart delete mode 100644 lint_test/function_lines_of_code_test/analysis_options.yaml delete mode 100644 lint_test/function_lines_of_code_test/function_lines_of_code_test.dart create mode 100644 test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart diff --git a/lib/main.dart b/lib/main.dart index 018d598f..6e7694bb 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -12,6 +12,8 @@ import 'package:solid_lints/src/lints/avoid_unnecessary_type_assertions/avoid_un import 'package:solid_lints/src/lints/avoid_unnecessary_type_assertions/fixes/avoid_unnecessary_type_assertions_fix.dart'; import 'package:solid_lints/src/lints/double_literal_format/double_literal_format_rule.dart'; import 'package:solid_lints/src/lints/double_literal_format/fixes/double_literal_format_fix.dart'; +import 'package:solid_lints/src/lints/function_lines_of_code/function_lines_of_code_rule.dart'; +import 'package:solid_lints/src/lints/function_lines_of_code/models/function_lines_of_code_parameters.dart'; import 'package:solid_lints/src/lints/proper_super_calls/proper_super_calls_rule.dart'; /// The entry point for the Solid Lints analyser server plugin. @@ -47,6 +49,10 @@ class SolidLintsPlugin extends Plugin { analysisOptionsLoader: analysisLoader, parametersParser: AvoidReturningWidgetsParameters.fromJson, ), + FunctionLinesOfCodeRule( + analysisOptionsLoader: analysisLoader, + parametersParser: FunctionLinesOfCodeParameters.fromJson, + ), ]; for (final lintRule in lintRules) { diff --git a/lib/src/lints/function_lines_of_code/function_lines_of_code_rule.dart b/lib/src/lints/function_lines_of_code/function_lines_of_code_rule.dart index a8e7c89e..0aa33e1b 100644 --- a/lib/src/lints/function_lines_of_code/function_lines_of_code_rule.dart +++ b/lib/src/lints/function_lines_of_code/function_lines_of_code_rule.dart @@ -1,9 +1,8 @@ -import 'package:analyzer/dart/ast/ast.dart'; -import 'package:analyzer/error/listener.dart'; -import 'package:custom_lint_builder/custom_lint_builder.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/function_lines_of_code/models/function_lines_of_code_parameters.dart'; -import 'package:solid_lints/src/lints/function_lines_of_code/visitors/function_lines_of_code_visitor.dart'; -import 'package:solid_lints/src/models/rule_config.dart'; +import 'package:solid_lints/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart'; import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// An approximate metric of meaningful lines of source code inside a function, @@ -12,90 +11,50 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// ### Example config: /// /// ```yaml -/// custom_lint: -/// rules: -/// - function_lines_of_code: -/// max_lines: 100 -/// excludeNames: -/// - "Build" +/// plugins: +/// solid_lints: +/// diagnostics: +/// function_lines_of_code: +/// max_lines: 100 +/// exclude: +/// - "Build" /// ``` class FunctionLinesOfCodeRule extends SolidLintRule { - /// This lint rule represents the error if number of - /// parameters reaches the maximum value. + /// This lint rule name. static const lintName = 'function_lines_of_code'; - FunctionLinesOfCodeRule._(super.config); - - /// Creates a new instance of [FunctionLinesOfCodeRule] - /// based on the lint configuration. - factory FunctionLinesOfCodeRule.createRule(CustomLintConfigs configs) { - final rule = RuleConfig( - configs: configs, - name: lintName, - paramsParser: FunctionLinesOfCodeParameters.fromJson, - problemMessage: (value) => - 'The maximum allowed number of lines is ${value.maxLines}. ' - 'Try splitting this function into smaller parts.', - ); - - return FunctionLinesOfCodeRule._(rule); - } + static const _code = LintCode( + lintName, + 'The maximum allowed number of lines is {0}. ' + 'Try splitting this function into smaller parts.', + ); @override - void run( - CustomLintResolver resolver, - DiagnosticReporter reporter, - CustomLintContext context, - ) { - void checkNode(AstNode node) => _checkNode(resolver, reporter, node); + DiagnosticCode get diagnosticCode => _code; - void checkDeclarationNode(Declaration node) { - final isIgnored = config.parameters.exclude.shouldIgnore(node); - if (isIgnored) { - return; - } - checkNode(node); - } - - // Check for an anonymous function - void checkFunctionExpressionNode(FunctionExpression node) { - // If a FunctionExpression is an immediate child of a FunctionDeclaration - // this means it's a named function, which are already check as part of - // addFunctionDeclaration call. - if (node.parent is FunctionDeclaration) { - return; - } - checkNode(node); - } - - context.registry.addFunctionDeclaration(checkDeclarationNode); - context.registry.addMethodDeclaration(checkDeclarationNode); - context.registry.addFunctionExpression(checkFunctionExpressionNode); - } + /// Creates a new instance of [FunctionLinesOfCodeRule] + FunctionLinesOfCodeRule({ + required super.analysisOptionsLoader, + required super.parametersParser, + }) : super.withParameters( + name: lintName, + description: _code.problemMessage, + ); - void _checkNode( - CustomLintResolver resolver, - DiagnosticReporter reporter, - AstNode node, + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, ) { - final visitor = FunctionLinesOfCodeVisitor(resolver.lineInfo); - node.visitChildren(visitor); + super.registerNodeProcessors(registry, context); - if (visitor.linesWithCode.length > config.parameters.maxLines) { - if (node is! AnnotatedNode) { - reporter.atNode(node, code); - return; - } + final parameters = + getParametersForContext(context) ?? + FunctionLinesOfCodeParameters.empty(); - final startOffset = node.firstTokenAfterCommentAndMetadata.offset; - final lengthDifference = startOffset - node.offset; + final visitor = FunctionLinesOfCodeRuleVisitor(this, context, parameters); - reporter.atOffset( - offset: startOffset, - length: node.length - lengthDifference, - diagnosticCode: code, - ); - } + registry.addCompilationUnit(this, visitor); } } diff --git a/lib/src/lints/function_lines_of_code/models/function_lines_of_code_parameters.dart b/lib/src/lints/function_lines_of_code/models/function_lines_of_code_parameters.dart index 7df6da2a..3e748c49 100644 --- a/lib/src/lints/function_lines_of_code/models/function_lines_of_code_parameters.dart +++ b/lib/src/lints/function_lines_of_code/models/function_lines_of_code_parameters.dart @@ -18,6 +18,14 @@ class FunctionLinesOfCodeParameters { required this.exclude, }); + /// Empty [FunctionLinesOfCodeParameters] model with default max lines. + factory FunctionLinesOfCodeParameters.empty() { + return FunctionLinesOfCodeParameters( + maxLines: _defaultMaxLines, + exclude: ExcludedIdentifiersListParameter(exclude: []), + ); + } + /// Method for creating from json data factory FunctionLinesOfCodeParameters.fromJson(Map json) => FunctionLinesOfCodeParameters( diff --git a/lib/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart b/lib/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart new file mode 100644 index 00000000..e1901435 --- /dev/null +++ b/lib/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart @@ -0,0 +1,76 @@ +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:solid_lints/src/lints/function_lines_of_code/function_lines_of_code_rule.dart'; +import 'package:solid_lints/src/lints/function_lines_of_code/models/function_lines_of_code_parameters.dart'; +import 'package:solid_lints/src/lints/function_lines_of_code/visitors/function_lines_of_code_visitor.dart'; + +/// A visitor that reports on functions/methods exceeding the max line limit. +class FunctionLinesOfCodeRuleVisitor extends RecursiveAstVisitor { + final FunctionLinesOfCodeRule _rule; + final RuleContext _context; + final FunctionLinesOfCodeParameters _parameters; + + /// Creates a new instance of [FunctionLinesOfCodeRuleVisitor]. + FunctionLinesOfCodeRuleVisitor(this._rule, this._context, this._parameters); + + @override + void visitFunctionDeclaration(FunctionDeclaration node) { + final isIgnored = _parameters.exclude.shouldIgnore(node); + if (!isIgnored) { + _checkNode(node); + } + super.visitFunctionDeclaration(node); + } + + @override + void visitMethodDeclaration(MethodDeclaration node) { + final isIgnored = _parameters.exclude.shouldIgnore(node); + if (!isIgnored) { + _checkNode(node); + } + super.visitMethodDeclaration(node); + } + + @override + void visitFunctionExpression(FunctionExpression node) { + super.visitFunctionExpression(node); + + if (node.parent is FunctionDeclaration) { + return; + } + _checkNode(node); + } + + void _checkNode(AstNode node) { + final lineInfo = _context.currentUnit?.unit.lineInfo; + if (lineInfo == null) return; + + final visitor = FunctionLinesOfCodeVisitor(lineInfo); + node.visitChildren(visitor); + + if (visitor.linesWithCode.length > _parameters.maxLines) { + final reporter = _context.currentUnit?.diagnosticReporter; + if (reporter == null) return; + + if (node is! AnnotatedNode) { + reporter.atNode( + node, + _rule.diagnosticCode, + arguments: [_parameters.maxLines], + ); + return; + } + + final startOffset = node.firstTokenAfterCommentAndMetadata.offset; + final lengthDifference = startOffset - node.offset; + + reporter.atOffset( + offset: startOffset, + length: node.length - lengthDifference, + diagnosticCode: _rule.diagnosticCode, + arguments: [_parameters.maxLines], + ); + } + } +} diff --git a/lint_test/function_lines_of_code_test/analysis_options.yaml b/lint_test/function_lines_of_code_test/analysis_options.yaml deleted file mode 100644 index 459a4d79..00000000 --- a/lint_test/function_lines_of_code_test/analysis_options.yaml +++ /dev/null @@ -1,14 +0,0 @@ -analyzer: - plugins: - - ../custom_lint - -custom_lint: - rules: - - function_lines_of_code: - max_lines: 5 - exclude: - - class_name: ClassWithLongMethods - method_name: longMethodExcluded - - method_name: longFunctionExcluded - - longFunctionExcludedByDeclarationName - - longMethodExcludedByDeclarationName diff --git a/lint_test/function_lines_of_code_test/function_lines_of_code_test.dart b/lint_test/function_lines_of_code_test/function_lines_of_code_test.dart deleted file mode 100644 index 032f5cd6..00000000 --- a/lint_test/function_lines_of_code_test/function_lines_of_code_test.dart +++ /dev/null @@ -1,535 +0,0 @@ -// ignore_for_file: prefer_match_file_name - -class ClassWithLongMethods { - // expect_lint: function_lines_of_code - int longMethod() { - var i = 0; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - - return i; - } - - // Excluded by method_name - int longMethodExcluded() { - var i = 0; - i++; - i++; - i++; - i++; - - return i; - } - -// Excluded by declaration_name - int longMethodExcludedByDeclarationName() { - var i = 0; - i++; - i++; - i++; - i++; - - return i; - } - - int notLongMethod() { - var i = 0; - i++; - i++; - i++; - - return i; - } -} - -int notLongFunction() { - var i = 0; - i++; - i++; - i++; - - return i; -} - -// expect_lint: function_lines_of_code -int longFunction() { - var i = 0; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - i++; - - return i; -} - -// Excluded by method_name -int longFunctionExcluded() { - var i = 0; - i++; - i++; - i++; - i++; - - return i; -} - -// Excluded by declaration_name -int longFunctionExcludedByDeclarationName() { - var i = 0; - i++; - i++; - i++; - i++; - - return i; -} - -// expect_lint: function_lines_of_code -final longAnonymousFunction = () { - var i = 0; - i++; - i++; - i++; - i++; - - return i; -}; - -final notLongAnonymousFunction = () { - var i = 0; - i++; - i++; - i++; - - return i; -}; diff --git a/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart b/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart new file mode 100644 index 00000000..d8b1d643 --- /dev/null +++ b/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart @@ -0,0 +1,134 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:analyzer_testing/utilities/utilities.dart'; +import 'package:solid_lints/src/common/parameter_parser/analysis_options_loader.dart'; +import 'package:solid_lints/src/lints/function_lines_of_code/function_lines_of_code_rule.dart'; +import 'package:solid_lints/src/lints/function_lines_of_code/models/function_lines_of_code_parameters.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(FunctionLinesOfCodeRuleTest); + }); +} + +@reflectiveTest +class FunctionLinesOfCodeRuleTest extends AnalysisRuleTest { + static const _mockAnalysisOptionsContent = ''' +plugins: + solid_lints: + diagnostics: + function_lines_of_code: + max_lines: 4 + exclude: + - class_name: ClassWithLongMethods + method_name: longMethodExcluded + - method_name: longMethodExcludedByDeclarationName + - method_name: longFunctionExcluded + - method_name: longFunctionExcludedByDeclarationName + - longMethodExcludedByString + '''; + + @override + void setUp() { + rule = FunctionLinesOfCodeRule( + analysisOptionsLoader: AnalysisOptionsLoader( + resourceProvider: resourceProvider, + ), + parametersParser: FunctionLinesOfCodeParameters.fromJson, + ); + super.setUp(); + + newAnalysisOptionsYamlFile( + testPackageRootPath, + '''${analysisOptionsContent(rules: [rule.name])} +$_mockAnalysisOptionsContent''', + ); + } + + Future test_reports_when_lines_exceed_threshold() async { + await assertDiagnostics( + r''' +int longFunction() { + var i = 0; + i++; + i++; + i++; + + return i; +} +''', + [lint(0, 69)], + ); + } + + Future test_does_not_report_when_lines_within_threshold() async { + await assertNoDiagnostics(r''' +int shortFunction() { + var i = 0; + i++; + i++; + + return i; +} +'''); + } + + Future test_does_not_report_on_excluded_method() async { + await assertNoDiagnostics(r''' +class ClassWithLongMethods { + int longMethodExcluded() { + var i = 0; + i++; + i++; + i++; + + return i; + } +} +'''); + } + + Future test_does_not_report_on_excluded_top_level_function() async { + await assertNoDiagnostics(r''' +int longFunctionExcluded() { + var i = 0; + i++; + i++; + i++; + + return i; +} +'''); + } + + Future test_reports_on_anonymous_functions() async { + await assertDiagnostics( + r''' +final longAnonymousFunction = () { + var i = 0; + i++; + i++; + i++; + + return i; +}; +''', + [lint(30, 53)], + ); + } + + Future test_does_not_report_on_method_excluded_by_string() async { + await assertNoDiagnostics(r''' +class SomeClass { + int longMethodExcludedByString() { + var i = 0; + i++; + i++; + i++; + + return i; + } +} +'''); + } +} From 7dca4849a10ada96bbf0d917c49740c6fd91e684 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Fri, 19 Jun 2026 11:56:05 +0300 Subject: [PATCH 2/8] refactor: update function expression visitor logic --- .../visitors/function_lines_of_code_rule_visitor.dart | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart b/lib/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart index e1901435..0b6df2b3 100644 --- a/lib/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart +++ b/lib/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart @@ -34,12 +34,10 @@ class FunctionLinesOfCodeRuleVisitor extends RecursiveAstVisitor { @override void visitFunctionExpression(FunctionExpression node) { - super.visitFunctionExpression(node); - - if (node.parent is FunctionDeclaration) { - return; + if (node.parent is! FunctionDeclaration) { + _checkNode(node); } - _checkNode(node); + super.visitFunctionExpression(node); } void _checkNode(AstNode node) { From 7683e733a352f4d0891dae608db77ab1525631fe Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Fri, 19 Jun 2026 12:02:04 +0300 Subject: [PATCH 3/8] refactor: migrate function_lines_of_code tests to AutoTestLintOffsets --- .../function_lines_of_code_rule_test.dart | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart b/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart index d8b1d643..1bcb3574 100644 --- a/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart +++ b/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart @@ -5,6 +5,8 @@ import 'package:solid_lints/src/lints/function_lines_of_code/function_lines_of_c import 'package:solid_lints/src/lints/function_lines_of_code/models/function_lines_of_code_parameters.dart'; import 'package:test_reflective_loader/test_reflective_loader.dart'; +import '../../../lints/auto_test_lint_offsets.dart'; + void main() { defineReflectiveSuite(() { defineReflectiveTests(FunctionLinesOfCodeRuleTest); @@ -12,7 +14,8 @@ void main() { } @reflectiveTest -class FunctionLinesOfCodeRuleTest extends AnalysisRuleTest { +class FunctionLinesOfCodeRuleTest extends AnalysisRuleTest + with AutoTestLintOffsets { static const _mockAnalysisOptionsContent = ''' plugins: solid_lints: @@ -46,19 +49,16 @@ $_mockAnalysisOptionsContent''', } Future test_reports_when_lines_exceed_threshold() async { - await assertDiagnostics( - r''' -int longFunction() { + await assertAutoDiagnostics(''' +${expectLint(r'''int longFunction() { var i = 0; i++; i++; i++; return i; -} -''', - [lint(0, 69)], - ); +}''')} +'''); } Future test_does_not_report_when_lines_within_threshold() async { @@ -102,19 +102,16 @@ int longFunctionExcluded() { } Future test_reports_on_anonymous_functions() async { - await assertDiagnostics( - r''' -final longAnonymousFunction = () { + await assertAutoDiagnostics(''' +final longAnonymousFunction = ${expectLint(r'''() { var i = 0; i++; i++; i++; return i; -}; -''', - [lint(30, 53)], - ); +}''')}; +'''); } Future test_does_not_report_on_method_excluded_by_string() async { From ad21fe8c76a4c72df1a2eba75f194a5037b26463 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Fri, 19 Jun 2026 12:12:51 +0300 Subject: [PATCH 4/8] refactor: optimize currentUnit access in function lines of code visitor --- .../visitors/function_lines_of_code_rule_visitor.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart b/lib/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart index 0b6df2b3..497931e9 100644 --- a/lib/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart +++ b/lib/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart @@ -41,15 +41,15 @@ class FunctionLinesOfCodeRuleVisitor extends RecursiveAstVisitor { } void _checkNode(AstNode node) { - final lineInfo = _context.currentUnit?.unit.lineInfo; - if (lineInfo == null) return; + final currentUnit = _context.currentUnit; + if (currentUnit == null) return; + final lineInfo = currentUnit.unit.lineInfo; final visitor = FunctionLinesOfCodeVisitor(lineInfo); node.visitChildren(visitor); if (visitor.linesWithCode.length > _parameters.maxLines) { - final reporter = _context.currentUnit?.diagnosticReporter; - if (reporter == null) return; + final reporter = currentUnit.diagnosticReporter; if (node is! AnnotatedNode) { reporter.atNode( From b64df6a93e082c036aa387d9d01d02c71778c034 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Fri, 19 Jun 2026 12:26:07 +0300 Subject: [PATCH 5/8] feat: update description and add test case for function lines of code excluding comments --- .../function_lines_of_code_rule.dart | 4 +++- .../function_lines_of_code_rule_test.dart | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/lib/src/lints/function_lines_of_code/function_lines_of_code_rule.dart b/lib/src/lints/function_lines_of_code/function_lines_of_code_rule.dart index 0aa33e1b..dd033c6d 100644 --- a/lib/src/lints/function_lines_of_code/function_lines_of_code_rule.dart +++ b/lib/src/lints/function_lines_of_code/function_lines_of_code_rule.dart @@ -39,7 +39,9 @@ class FunctionLinesOfCodeRule required super.parametersParser, }) : super.withParameters( name: lintName, - description: _code.problemMessage, + description: + 'An approximate metric of meaningful lines of source code ' + 'inside a function, excluding blank lines and comments.', ); @override diff --git a/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart b/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart index 1bcb3574..d6afbd3e 100644 --- a/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart +++ b/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart @@ -126,6 +126,24 @@ class SomeClass { return i; } } +'''); + } + + Future + test_does_not_report_on_function_with_comments_and_blank_lines() async { + await assertNoDiagnostics(r''' +int shortFunctionWithComments() { + // This is a single-line comment. + var i = 0; + i++; + i++; + + /* + * This is a multi-line comment. + */ + + return i; +} '''); } } From e6ba1894d0f1a894ee021e7d779cc8499ce69446 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Tue, 23 Jun 2026 15:04:00 +0300 Subject: [PATCH 6/8] refactor: simplify lines of code visitor logic --- .../function_lines_of_code_rule_visitor.dart | 37 +++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/lib/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart b/lib/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart index 497931e9..28e9eda8 100644 --- a/lib/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart +++ b/lib/src/lints/function_lines_of_code/visitors/function_lines_of_code_rule_visitor.dart @@ -47,28 +47,27 @@ class FunctionLinesOfCodeRuleVisitor extends RecursiveAstVisitor { final lineInfo = currentUnit.unit.lineInfo; final visitor = FunctionLinesOfCodeVisitor(lineInfo); node.visitChildren(visitor); + if (visitor.linesWithCode.length <= _parameters.maxLines) return; - if (visitor.linesWithCode.length > _parameters.maxLines) { - final reporter = currentUnit.diagnosticReporter; - - if (node is! AnnotatedNode) { - reporter.atNode( - node, - _rule.diagnosticCode, - arguments: [_parameters.maxLines], - ); - return; - } - - final startOffset = node.firstTokenAfterCommentAndMetadata.offset; - final lengthDifference = startOffset - node.offset; - - reporter.atOffset( - offset: startOffset, - length: node.length - lengthDifference, - diagnosticCode: _rule.diagnosticCode, + final reporter = currentUnit.diagnosticReporter; + if (node is! AnnotatedNode) { + reporter.atNode( + node, + _rule.diagnosticCode, arguments: [_parameters.maxLines], ); + + return; } + + final startOffset = node.firstTokenAfterCommentAndMetadata.offset; + final lengthDifference = startOffset - node.offset; + + reporter.atOffset( + offset: startOffset, + length: node.length - lengthDifference, + diagnosticCode: _rule.diagnosticCode, + arguments: [_parameters.maxLines], + ); } } From 5417f80cf8fc11893f2feb72a982c305f6c1b3a7 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Tue, 23 Jun 2026 17:11:01 +0300 Subject: [PATCH 7/8] refactor: migrate function_lines_of_code rule tests to a table-driven approach --- .../function_lines_of_code_rule_test.dart | 219 ++++++++++-------- .../models/test_case.dart | 38 +++ test/src/utils/code_generators.dart | 5 + test/src/utils/table_test_types.dart | 20 ++ 4 files changed, 179 insertions(+), 103 deletions(-) create mode 100644 test/src/lints/function_lines_of_code/models/test_case.dart create mode 100644 test/src/utils/code_generators.dart create mode 100644 test/src/utils/table_test_types.dart diff --git a/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart b/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart index d6afbd3e..5e31f875 100644 --- a/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart +++ b/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart @@ -3,9 +3,13 @@ import 'package:analyzer_testing/utilities/utilities.dart'; import 'package:solid_lints/src/common/parameter_parser/analysis_options_loader.dart'; import 'package:solid_lints/src/lints/function_lines_of_code/function_lines_of_code_rule.dart'; import 'package:solid_lints/src/lints/function_lines_of_code/models/function_lines_of_code_parameters.dart'; +import 'package:test/test.dart'; import 'package:test_reflective_loader/test_reflective_loader.dart'; import '../../../lints/auto_test_lint_offsets.dart'; +import '../../utils/code_generators.dart'; +import '../../utils/table_test_types.dart'; +import 'models/test_case.dart'; void main() { defineReflectiveSuite(() { @@ -15,22 +19,67 @@ void main() { @reflectiveTest class FunctionLinesOfCodeRuleTest extends AnalysisRuleTest - with AutoTestLintOffsets { - static const _mockAnalysisOptionsContent = ''' + with AutoTestLintOffsets, AnalyzerContextResetMixin { + static const _excludedClassName = 'ExcludedClass'; + static const _nonExcludedClassName = 'NonExcludedClass'; + static const _excludedMethodName = 'excludedMethod'; + static const _excludedMethodByDeclaration = 'excludedMethodByDeclaration'; + static const _excludedFunctionName = 'excludedFunction'; + static const _excludedFunctionByDeclaration = 'excludedFunctionByDeclaration'; + static const _excludedByString = 'excludedByString'; + + static const _mockAnalysisOptionsContent = + ''' plugins: solid_lints: diagnostics: function_lines_of_code: max_lines: 4 exclude: - - class_name: ClassWithLongMethods - method_name: longMethodExcluded - - method_name: longMethodExcludedByDeclarationName - - method_name: longFunctionExcluded - - method_name: longFunctionExcludedByDeclarationName - - longMethodExcludedByString + - class_name: $_excludedClassName + method_name: $_excludedMethodName + - method_name: $_excludedMethodByDeclaration + - method_name: $_excludedFunctionName + - method_name: $_excludedFunctionByDeclaration + - $_excludedByString '''; + /// All test cases for the function_lines_of_code rule. + static const testTable = { + // --- Threshold: fail when code lines > max (4) --- + TestCase(codeLines: 5): ExpectedResult.fail, + TestCase(codeLines: 5, comments: true): ExpectedResult.fail, + + // --- Threshold: pass when code lines ≤ max (4) --- + TestCase(codeLines: 4): ExpectedResult.pass, + TestCase(codeLines: 3): ExpectedResult.pass, + TestCase(codeLines: 4, comments: true): ExpectedResult.pass, + + // --- Exclude config --- + TestCase( + codeLines: 5, + className: _excludedClassName, + methodName: _excludedMethodName, + ): ExpectedResult.pass, + TestCase( + codeLines: 5, + className: _excludedClassName, + methodName: _excludedMethodByDeclaration, + ): ExpectedResult.pass, + TestCase(codeLines: 5, methodName: _excludedFunctionName): + ExpectedResult.pass, + TestCase(codeLines: 5, methodName: _excludedFunctionByDeclaration): + ExpectedResult.pass, + TestCase( + codeLines: 5, + className: _nonExcludedClassName, + methodName: _excludedByString, + ): ExpectedResult.pass, + + // --- Anonymous functions --- + TestCase(codeLines: 5, anonymous: true): ExpectedResult.fail, + }; + @override void setUp() { rule = FunctionLinesOfCodeRule( @@ -48,102 +97,66 @@ $_mockAnalysisOptionsContent''', ); } - Future test_reports_when_lines_exceed_threshold() async { - await assertAutoDiagnostics(''' -${expectLint(r'''int longFunction() { - var i = 0; - i++; - i++; - i++; - - return i; -}''')} -'''); - } - - Future test_does_not_report_when_lines_within_threshold() async { - await assertNoDiagnostics(r''' -int shortFunction() { - var i = 0; - i++; - i++; - - return i; -} -'''); - } - - Future test_does_not_report_on_excluded_method() async { - await assertNoDiagnostics(r''' -class ClassWithLongMethods { - int longMethodExcluded() { - var i = 0; - i++; - i++; - i++; - - return i; - } -} -'''); - } - - Future test_does_not_report_on_excluded_top_level_function() async { - await assertNoDiagnostics(r''' -int longFunctionExcluded() { - var i = 0; - i++; - i++; - i++; - - return i; -} -'''); - } - - Future test_reports_on_anonymous_functions() async { - await assertAutoDiagnostics(''' -final longAnonymousFunction = ${expectLint(r'''() { - var i = 0; - i++; - i++; - i++; - - return i; -}''')}; -'''); + /// Generates test source code for the given [testCase]. + /// + /// Returns the full [source] to analyze and the [lintTarget] substring + /// that should trigger the lint diagnostic (for fail cases). + static ({String source, String lintTarget}) _generateCode(TestCase testCase) { + final indent = testCase.className != null ? ' ' : ' '; + final singleComment = testCase.comments + ? '$indent// This is a single-line comment.\n' + : ''; + final multiComment = testCase.comments + ? '\n$indent/*\n$indent * This is a multi-line comment.\n$indent */\n' + : ''; + final extra = testCase.codeLines - 2; + final stmts = extra > 0 + ? '\n${repeatLines('${indent}i++;', extra)}\n' + : '\n'; + final body = + '$singleComment${indent}var i = 0;$stmts$multiComment\n${indent}return i;\n'; + + if (testCase.anonymous) { + final fn = '() {\n$body}'; + return (source: 'final longAnonymousFunction = $fn;\n', lintTarget: fn); + } + + final name = testCase.methodName ?? 'function'; + + if (testCase.className != null) { + final method = ' int $name() {\n$body }'; + return ( + source: '\nclass ${testCase.className} {\n$method\n}\n', + lintTarget: method, + ); + } + + final fn = 'int $name() {\n$body}'; + return (source: '$fn\n', lintTarget: fn); } - Future test_does_not_report_on_method_excluded_by_string() async { - await assertNoDiagnostics(r''' -class SomeClass { - int longMethodExcludedByString() { - var i = 0; - i++; - i++; - i++; - - return i; - } -} -'''); - } - - Future - test_does_not_report_on_function_with_comments_and_blank_lines() async { - await assertNoDiagnostics(r''' -int shortFunctionWithComments() { - // This is a single-line comment. - var i = 0; - i++; - i++; - - /* - * This is a multi-line comment. - */ - - return i; -} -'''); + Future test_function_lines_of_code_cases() async { + for (final MapEntry(key: testCase, value: expected) in testTable.entries) { + final (:source, :lintTarget) = _generateCode(testCase); + + try { + switch (expected) { + case ExpectedResult.pass: + await assertNoDiagnostics(source); + case ExpectedResult.fail: + final marked = source.replaceFirst( + lintTarget, + expectLint(lintTarget), + ); + await assertAutoDiagnostics(marked); + } + } on TestFailure catch (e) { + fail('Case $testCase: $e'); + } + + // Reset the analysis context between test cases to bypass the analyzer's + // internal caching and ensure the newly generated code is re-analyzed. + await resetAnalyzerContext(); + } } } diff --git a/test/src/lints/function_lines_of_code/models/test_case.dart b/test/src/lints/function_lines_of_code/models/test_case.dart new file mode 100644 index 00000000..49b36416 --- /dev/null +++ b/test/src/lints/function_lines_of_code/models/test_case.dart @@ -0,0 +1,38 @@ +/// Parameters for a single function_lines_of_code test case. +class TestCase { + /// Number of code lines inside the function body. + /// + /// `var i = 0;` and `return i;` are always present, so [codeLines] of 4 + /// means two extra `i++;` statements. + final int codeLines; + + /// Whether to include single-line and multi-line comments (not counted). + final bool comments; + + /// If set, wraps the function inside `class [className] { ... }`. + final String? className; + + /// Overrides the default function name (`function`). + final String? methodName; + + /// If true, generates an anonymous function literal instead. + final bool anonymous; + + const TestCase({ + required this.codeLines, + this.comments = false, + this.className, + this.methodName, + this.anonymous = false, + }); + + @override + String toString() { + final parts = ['codeLines: $codeLines']; + if (comments) parts.add('comments: true'); + if (className != null) parts.add('className: $className'); + if (methodName != null) parts.add('methodName: $methodName'); + if (anonymous) parts.add('anonymous: true'); + return '(${parts.join(', ')})'; + } +} diff --git a/test/src/utils/code_generators.dart b/test/src/utils/code_generators.dart new file mode 100644 index 00000000..d331ee4a --- /dev/null +++ b/test/src/utils/code_generators.dart @@ -0,0 +1,5 @@ +/// Repeats [string] [times] times, joining with newlines. +/// +/// Useful for generating test Dart code with a specific number of lines. +String repeatLines(String string, int times) => + List.generate(times, (_) => string).join('\n'); diff --git a/test/src/utils/table_test_types.dart b/test/src/utils/table_test_types.dart new file mode 100644 index 00000000..785e9d79 --- /dev/null +++ b/test/src/utils/table_test_types.dart @@ -0,0 +1,20 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; + +/// Result expected from a table-driven test case. +enum ExpectedResult { + /// The test case should pass without diagnostics. + pass, + + /// The test case should fail with a lint diagnostic. + fail, +} + +/// Mixin to allow manual analyzer context resets in table-driven tests. +mixin AnalyzerContextResetMixin on AnalysisRuleTest { + /// Disposes and recreates the analysis context. + Future resetAnalyzerContext() async { + // ignore: invalid_use_of_visible_for_testing_member + await super.tearDown(); + setUp(); + } +} From 02ac214a3a7581470f56887a04d568b2e362bd09 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Tue, 23 Jun 2026 17:33:33 +0300 Subject: [PATCH 8/8] refactor: introduce TableDrivenRuleTest base class to consolidate table-driven test logic --- .../function_lines_of_code_rule_test.dart | 34 ++---------- test/src/utils/table_driven_rule_test.dart | 53 +++++++++++++++++++ test/src/utils/table_test_types.dart | 20 ------- 3 files changed, 58 insertions(+), 49 deletions(-) create mode 100644 test/src/utils/table_driven_rule_test.dart delete mode 100644 test/src/utils/table_test_types.dart diff --git a/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart b/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart index 5e31f875..8a9c4665 100644 --- a/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart +++ b/test/src/lints/function_lines_of_code/function_lines_of_code_rule_test.dart @@ -1,14 +1,11 @@ -import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; import 'package:analyzer_testing/utilities/utilities.dart'; import 'package:solid_lints/src/common/parameter_parser/analysis_options_loader.dart'; import 'package:solid_lints/src/lints/function_lines_of_code/function_lines_of_code_rule.dart'; import 'package:solid_lints/src/lints/function_lines_of_code/models/function_lines_of_code_parameters.dart'; -import 'package:test/test.dart'; import 'package:test_reflective_loader/test_reflective_loader.dart'; -import '../../../lints/auto_test_lint_offsets.dart'; import '../../utils/code_generators.dart'; -import '../../utils/table_test_types.dart'; +import '../../utils/table_driven_rule_test.dart'; import 'models/test_case.dart'; void main() { @@ -18,8 +15,7 @@ void main() { } @reflectiveTest -class FunctionLinesOfCodeRuleTest extends AnalysisRuleTest - with AutoTestLintOffsets, AnalyzerContextResetMixin { +class FunctionLinesOfCodeRuleTest extends TableDrivenRuleTest { static const _excludedClassName = 'ExcludedClass'; static const _nonExcludedClassName = 'NonExcludedClass'; static const _excludedMethodName = 'excludedMethod'; @@ -101,7 +97,8 @@ $_mockAnalysisOptionsContent''', /// /// Returns the full [source] to analyze and the [lintTarget] substring /// that should trigger the lint diagnostic (for fail cases). - static ({String source, String lintTarget}) _generateCode(TestCase testCase) { + @override + ({String source, String lintTarget}) generateCode(TestCase testCase) { final indent = testCase.className != null ? ' ' : ' '; final singleComment = testCase.comments ? '$indent// This is a single-line comment.\n' @@ -136,27 +133,6 @@ $_mockAnalysisOptionsContent''', } Future test_function_lines_of_code_cases() async { - for (final MapEntry(key: testCase, value: expected) in testTable.entries) { - final (:source, :lintTarget) = _generateCode(testCase); - - try { - switch (expected) { - case ExpectedResult.pass: - await assertNoDiagnostics(source); - case ExpectedResult.fail: - final marked = source.replaceFirst( - lintTarget, - expectLint(lintTarget), - ); - await assertAutoDiagnostics(marked); - } - } on TestFailure catch (e) { - fail('Case $testCase: $e'); - } - - // Reset the analysis context between test cases to bypass the analyzer's - // internal caching and ensure the newly generated code is re-analyzed. - await resetAnalyzerContext(); - } + await runTableTests(testTable); } } diff --git a/test/src/utils/table_driven_rule_test.dart b/test/src/utils/table_driven_rule_test.dart new file mode 100644 index 00000000..8f282f27 --- /dev/null +++ b/test/src/utils/table_driven_rule_test.dart @@ -0,0 +1,53 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:test/test.dart' hide setUp; + +import '../../lints/auto_test_lint_offsets.dart'; + +/// Result expected from a table-driven test case. +enum ExpectedResult { + /// The test case should pass without diagnostics. + pass, + + /// The test case should fail with a lint diagnostic. + fail, +} + +/// Base class for table-driven lint rule tests. +abstract class TableDrivenRuleTest extends AnalysisRuleTest + with AutoTestLintOffsets { + /// Disposes and recreates the analysis context. + Future resetAnalyzerContext() async { + // ignore: invalid_use_of_visible_for_testing_member + await super.tearDown(); + setUp(); + } + + /// Generates the test source code and the lint target for a given [testCase]. + ({String source, String lintTarget}) generateCode(T testCase); + + /// Executes all test cases defined in the [testTable] map. + Future runTableTests(Map testTable) async { + for (final MapEntry(key: testCase, value: expected) in testTable.entries) { + final (:source, :lintTarget) = generateCode(testCase); + + try { + switch (expected) { + case ExpectedResult.pass: + await assertNoDiagnostics(source); + case ExpectedResult.fail: + final marked = source.replaceFirst( + lintTarget, + expectLint(lintTarget), + ); + await assertAutoDiagnostics(marked); + } + } on TestFailure catch (e) { + fail('Case $testCase: $e'); + } + + // Reset the analysis context between test cases to bypass the analyzer's + // internal caching and ensure the newly generated code is re-analyzed. + await resetAnalyzerContext(); + } + } +} diff --git a/test/src/utils/table_test_types.dart b/test/src/utils/table_test_types.dart deleted file mode 100644 index 785e9d79..00000000 --- a/test/src/utils/table_test_types.dart +++ /dev/null @@ -1,20 +0,0 @@ -import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; - -/// Result expected from a table-driven test case. -enum ExpectedResult { - /// The test case should pass without diagnostics. - pass, - - /// The test case should fail with a lint diagnostic. - fail, -} - -/// Mixin to allow manual analyzer context resets in table-driven tests. -mixin AnalyzerContextResetMixin on AnalysisRuleTest { - /// Disposes and recreates the analysis context. - Future resetAnalyzerContext() async { - // ignore: invalid_use_of_visible_for_testing_member - await super.tearDown(); - setUp(); - } -}