From 00129403d958582dacf22842ecaed71c67a45bcd Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 8 Jul 2026 16:02:29 +0300 Subject: [PATCH 01/28] feat: add avoid_duplicate_code lint rule --- lib/main.dart | 2 + .../avoid_duplicate_code_rule.dart | 69 +++ .../avoid_duplicate_code_parameters.dart | 60 +++ .../visitors/ast_structural_hasher.dart | 472 ++++++++++++++++++ .../avoid_duplicate_code_visitor.dart | 154 ++++++ .../avoid_duplicate_code_rule_test.dart | 338 +++++++++++++ 6 files changed, 1095 insertions(+) create mode 100644 lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart create mode 100644 lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart create mode 100644 lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hasher.dart create mode 100644 lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart create mode 100644 test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart diff --git a/lib/main.dart b/lib/main.dart index b7a6f5db..02da6662 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -2,6 +2,7 @@ import 'package:analysis_server_plugin/plugin.dart'; import 'package:analysis_server_plugin/registry.dart'; import 'package:solid_lints/src/common/parameter_parser/analysis_options_loader.dart'; import 'package:solid_lints/src/lints/avoid_debug_print_in_release/avoid_debug_print_in_release_rule.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart'; import 'package:solid_lints/src/lints/avoid_final_with_getter/avoid_final_with_getter_rule.dart'; import 'package:solid_lints/src/lints/avoid_final_with_getter/fixes/avoid_final_with_getter_fix.dart'; import 'package:solid_lints/src/lints/avoid_global_state/avoid_global_state_rule.dart'; @@ -70,6 +71,7 @@ class SolidLintsPlugin extends Plugin { AvoidNonNullAssertionRule(analysisOptionsLoader: analysisLoader), avoidUnnecessaryTypeAssertionsRule, AvoidDebugPrintInReleaseRule(), + AvoidDuplicateCodeRule(analysisOptionsLoader: analysisLoader), doubleLiteralFormatRule, ProperSuperCallsRule(), NamedParametersOrderingRule(analysisOptionsLoader: analysisLoader), diff --git a/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart b/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart new file mode 100644 index 00000000..19bf7a97 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart @@ -0,0 +1,69 @@ +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_duplicate_code/models/avoid_duplicate_code_parameters.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart'; +import 'package:solid_lints/src/models/solid_lint_rule.dart'; + +/// A lint rule that detects duplicated code blocks (clones) within a single +/// file. +/// +/// When two or more function/method/constructor bodies have structurally +/// identical AST subtrees (Type 2 clones — same structure, variable names +/// may differ), the rule reports all copies except the first occurrence. +/// +/// ### Example config: +/// +/// ```yaml +/// plugins: +/// solid_lints: +/// diagnostics: +/// avoid_duplicate_code: +/// min_statements: 3 +/// ignore_literals: false +/// ignore_identifiers: true +/// check_blocks: false +/// exclude: +/// - method_name: build +/// ``` +class AvoidDuplicateCodeRule + extends SolidLintRule { + /// Name of the lint. + static const lintName = 'avoid_duplicate_code'; + + static const _code = LintCode( + lintName, + 'This code is a duplicate of the function/method at line {0}. ' + 'Consider extracting the shared logic into a common function.', + ); + + @override + DiagnosticCode get diagnosticCode => _code; + + /// Creates a new instance of [AvoidDuplicateCodeRule]. + AvoidDuplicateCodeRule({ + required super.analysisOptionsLoader, + }) : super.withParameters( + name: lintName, + description: + 'Detects structurally identical function/method bodies ' + 'within a single file (code clones).', + parametersParser: AvoidDuplicateCodeParameters.fromJson, + ); + + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + super.registerNodeProcessors(registry, context); + + final parameters = + getParametersForContext(context) ?? + AvoidDuplicateCodeParameters.empty(); + + final visitor = AvoidDuplicateCodeVisitor(this, parameters); + + registry.addCompilationUnit(this, visitor); + } +} diff --git a/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart new file mode 100644 index 00000000..38dafd43 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart @@ -0,0 +1,60 @@ +import 'package:solid_lints/src/common/parameters/excluded_identifiers_list_parameter.dart'; + +/// Configuration parameters for the avoid_duplicate_code rule. +class AvoidDuplicateCodeParameters { + /// Minimum number of statements in a function body or block to be considered + /// for clone detection. Bodies/blocks shorter than this are ignored. + final int minStatements; + + /// When `true`, literal values (strings, numbers, booleans) are excluded + /// from the structural hash, making the check ignore literal differences. + final bool ignoreLiterals; + + /// When `true`, variable and method names (identifiers) are excluded + /// from the structural hash, allowing detection of renamed variables (Type 2). + final bool ignoreIdentifiers; + + /// When `true`, statement blocks (like if-blocks or loops) inside functions + /// are also checked for duplication. + final bool checkBlocks; + + /// A list of methods/functions that should be excluded from the lint. + final ExcludedIdentifiersListParameter exclude; + + static const _defaultMinStatements = 3; + + /// Constructor for [AvoidDuplicateCodeParameters] model. + const AvoidDuplicateCodeParameters({ + required this.minStatements, + required this.ignoreLiterals, + required this.ignoreIdentifiers, + required this.checkBlocks, + required this.exclude, + }); + + /// Empty [AvoidDuplicateCodeParameters] model with default values. + factory AvoidDuplicateCodeParameters.empty() => + AvoidDuplicateCodeParameters( + minStatements: _defaultMinStatements, + ignoreLiterals: false, + ignoreIdentifiers: true, + checkBlocks: false, + exclude: ExcludedIdentifiersListParameter(exclude: []), + ); + + /// Creates parameters from JSON configuration. + factory AvoidDuplicateCodeParameters.fromJson(Map json) => + AvoidDuplicateCodeParameters( + minStatements: + json['min_statements'] as int? ?? _defaultMinStatements, + ignoreLiterals: + json['ignore_literals'] as bool? ?? + json['ignore_literal_values'] as bool? ?? + false, + ignoreIdentifiers: + json['ignore_identifiers'] as bool? ?? true, + checkBlocks: + json['check_blocks'] as bool? ?? false, + exclude: ExcludedIdentifiersListParameter.defaultFromJson(json), + ); +} diff --git a/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hasher.dart b/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hasher.dart new file mode 100644 index 00000000..4c10eecd --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hasher.dart @@ -0,0 +1,472 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; + +/// A [RecursiveAstVisitor] that builds a structural fingerprint of an AST +/// subtree for clone detection. +/// +/// The fingerprint captures the structure of the code (node types, operators, +/// and optionally literal values) while ignoring identifier names, whitespace, +/// and comments. This enables Type 2 clone detection where two code blocks +/// with identical structure but different variable names are considered clones. +class AstStructuralHasher extends RecursiveAstVisitor { + final bool _ignoreLiterals; + final bool _ignoreIdentifiers; + final StringBuffer _buffer = StringBuffer(); + + /// Creates a new [AstStructuralHasher]. + AstStructuralHasher({ + required bool ignoreLiterals, + required bool ignoreIdentifiers, + }) : _ignoreLiterals = ignoreLiterals, + _ignoreIdentifiers = ignoreIdentifiers; + + /// Computes the structural hash for the given [node]. + /// + /// Visits the entire subtree of [node] and returns an integer hash + /// of the accumulated structural fingerprint. + int computeHash(AstNode node) { + _buffer.clear(); + node.accept(this); + return _buffer.toString().hashCode; + } + + /// Returns the raw structural fingerprint string for the given [node]. + /// + /// Useful for debugging and testing. + String computeFingerprint(AstNode node) { + _buffer.clear(); + node.accept(this); + return _buffer.toString(); + } + + // --- Structure nodes --- + + @override + void visitBlock(Block node) { + _append('Block'); + super.visitBlock(node); + } + + @override + void visitBlockFunctionBody(BlockFunctionBody node) { + _append('BlockFunctionBody'); + super.visitBlockFunctionBody(node); + } + + @override + void visitExpressionFunctionBody(ExpressionFunctionBody node) { + _append('ExpressionFunctionBody'); + super.visitExpressionFunctionBody(node); + } + + // --- Statements --- + + @override + void visitExpressionStatement(ExpressionStatement node) { + _append('ExpressionStatement'); + super.visitExpressionStatement(node); + } + + @override + void visitReturnStatement(ReturnStatement node) { + _append('ReturnStatement'); + super.visitReturnStatement(node); + } + + @override + void visitVariableDeclarationStatement(VariableDeclarationStatement node) { + _append('VariableDeclarationStatement'); + // Include keyword (final/var/const) + final keyword = node.variables.keyword; + if (keyword != null) { + _append(keyword.lexeme); + } + super.visitVariableDeclarationStatement(node); + } + + @override + void visitIfStatement(IfStatement node) { + _append('IfStatement'); + _append(node.elseKeyword != null ? 'withElse' : 'noElse'); + super.visitIfStatement(node); + } + + @override + void visitForStatement(ForStatement node) { + _append('ForStatement'); + super.visitForStatement(node); + } + + @override + void visitForEachPartsWithDeclaration(ForEachPartsWithDeclaration node) { + _append('ForEachPartsWithDeclaration'); + super.visitForEachPartsWithDeclaration(node); + } + + @override + void visitForPartsWithDeclarations(ForPartsWithDeclarations node) { + _append('ForPartsWithDeclarations'); + super.visitForPartsWithDeclarations(node); + } + + @override + void visitWhileStatement(WhileStatement node) { + _append('WhileStatement'); + super.visitWhileStatement(node); + } + + @override + void visitDoStatement(DoStatement node) { + _append('DoStatement'); + super.visitDoStatement(node); + } + + @override + void visitSwitchStatement(SwitchStatement node) { + _append('SwitchStatement'); + super.visitSwitchStatement(node); + } + + @override + void visitSwitchCase(SwitchCase node) { + _append('SwitchCase'); + super.visitSwitchCase(node); + } + + @override + void visitSwitchDefault(SwitchDefault node) { + _append('SwitchDefault'); + super.visitSwitchDefault(node); + } + + @override + void visitSwitchExpression(SwitchExpression node) { + _append('SwitchExpression'); + super.visitSwitchExpression(node); + } + + @override + void visitSwitchExpressionCase(SwitchExpressionCase node) { + _append('SwitchExpressionCase'); + super.visitSwitchExpressionCase(node); + } + + @override + void visitTryStatement(TryStatement node) { + _append('TryStatement'); + _append(node.finallyBlock != null ? 'withFinally' : 'noFinally'); + super.visitTryStatement(node); + } + + @override + void visitCatchClause(CatchClause node) { + _append('CatchClause'); + super.visitCatchClause(node); + } + + @override + void visitThrowExpression(ThrowExpression node) { + _append('ThrowExpression'); + super.visitThrowExpression(node); + } + + @override + void visitBreakStatement(BreakStatement node) { + _append('BreakStatement'); + super.visitBreakStatement(node); + } + + @override + void visitContinueStatement(ContinueStatement node) { + _append('ContinueStatement'); + super.visitContinueStatement(node); + } + + @override + void visitAssertStatement(AssertStatement node) { + _append('AssertStatement'); + super.visitAssertStatement(node); + } + + @override + void visitYieldStatement(YieldStatement node) { + _append('YieldStatement'); + _append(node.star != null ? 'star' : 'noStar'); + super.visitYieldStatement(node); + } + + // --- Expressions --- + + @override + void visitBinaryExpression(BinaryExpression node) { + _append('BinaryExpression'); + _append(node.operator.type.toString()); + super.visitBinaryExpression(node); + } + + @override + void visitPrefixExpression(PrefixExpression node) { + _append('PrefixExpression'); + _append(node.operator.type.toString()); + super.visitPrefixExpression(node); + } + + @override + void visitPostfixExpression(PostfixExpression node) { + _append('PostfixExpression'); + _append(node.operator.type.toString()); + super.visitPostfixExpression(node); + } + + @override + void visitAssignmentExpression(AssignmentExpression node) { + _append('AssignmentExpression'); + _append(node.operator.type.toString()); + super.visitAssignmentExpression(node); + } + + @override + void visitConditionalExpression(ConditionalExpression node) { + _append('ConditionalExpression'); + super.visitConditionalExpression(node); + } + + @override + void visitMethodInvocation(MethodInvocation node) { + _append('MethodInvocation'); + // Include method name as it's part of the API being called, + // not a local identifier + _append(node.methodName.name); + super.visitMethodInvocation(node); + } + + @override + void visitFunctionExpressionInvocation(FunctionExpressionInvocation node) { + _append('FunctionExpressionInvocation'); + super.visitFunctionExpressionInvocation(node); + } + + @override + void visitInstanceCreationExpression(InstanceCreationExpression node) { + _append('InstanceCreationExpression'); + _append(node.constructorName.type.name.lexeme); + super.visitInstanceCreationExpression(node); + } + + @override + void visitPropertyAccess(PropertyAccess node) { + _append('PropertyAccess'); + _append(node.propertyName.name); + super.visitPropertyAccess(node); + } + + @override + void visitPrefixedIdentifier(PrefixedIdentifier node) { + _append('PrefixedIdentifier'); + // Include the property name, but not the prefix (which is a local variable) + _append(node.identifier.name); + super.visitPrefixedIdentifier(node); + } + + @override + void visitIndexExpression(IndexExpression node) { + _append('IndexExpression'); + super.visitIndexExpression(node); + } + + @override + void visitCascadeExpression(CascadeExpression node) { + _append('CascadeExpression'); + super.visitCascadeExpression(node); + } + + @override + void visitAwaitExpression(AwaitExpression node) { + _append('AwaitExpression'); + super.visitAwaitExpression(node); + } + + @override + void visitAsExpression(AsExpression node) { + _append('AsExpression'); + super.visitAsExpression(node); + } + + @override + void visitIsExpression(IsExpression node) { + _append('IsExpression'); + _append(node.notOperator != null ? 'not' : 'is'); + super.visitIsExpression(node); + } + + @override + void visitParenthesizedExpression(ParenthesizedExpression node) { + _append('ParenthesizedExpression'); + super.visitParenthesizedExpression(node); + } + + @override + void visitListLiteral(ListLiteral node) { + _append('ListLiteral'); + super.visitListLiteral(node); + } + + @override + void visitSetOrMapLiteral(SetOrMapLiteral node) { + _append('SetOrMapLiteral'); + super.visitSetOrMapLiteral(node); + } + + @override + void visitMapLiteralEntry(MapLiteralEntry node) { + _append('MapLiteralEntry'); + super.visitMapLiteralEntry(node); + } + + @override + void visitSpreadElement(SpreadElement node) { + _append('SpreadElement'); + super.visitSpreadElement(node); + } + + @override + void visitNamedExpression(NamedExpression node) { + _append('NamedExpression'); + _append(node.name.label.name); + super.visitNamedExpression(node); + } + + // --- Literals --- + + @override + void visitIntegerLiteral(IntegerLiteral node) { + _append('IntegerLiteral'); + if (!_ignoreLiterals) { + _append(node.literal.lexeme); + } + } + + @override + void visitDoubleLiteral(DoubleLiteral node) { + _append('DoubleLiteral'); + if (!_ignoreLiterals) { + _append(node.literal.lexeme); + } + } + + @override + void visitSimpleStringLiteral(SimpleStringLiteral node) { + _append('StringLiteral'); + if (!_ignoreLiterals) { + _append(node.value); + } + } + + @override + void visitStringInterpolation(StringInterpolation node) { + _append('StringInterpolation'); + super.visitStringInterpolation(node); + } + + @override + void visitBooleanLiteral(BooleanLiteral node) { + _append('BooleanLiteral'); + if (!_ignoreLiterals) { + _append(node.value.toString()); + } + } + + @override + void visitNullLiteral(NullLiteral node) { + _append('NullLiteral'); + } + + // --- Identifiers (ignored for Type 2 clone detection) --- + + @override + void visitSimpleIdentifier(SimpleIdentifier node) { + _append('Identifier'); + if (!_ignoreIdentifiers) { + _append(node.name); + } + } + + // --- Type annotations --- + + @override + void visitNamedType(NamedType node) { + _append('NamedType'); + _append(node.name.lexeme); + super.visitNamedType(node); + } + + @override + void visitGenericFunctionType(GenericFunctionType node) { + _append('GenericFunctionType'); + super.visitGenericFunctionType(node); + } + + // --- Variable declarations --- + + @override + void visitVariableDeclaration(VariableDeclaration node) { + _append('VariableDeclaration'); + super.visitVariableDeclaration(node); + } + + @override + void visitVariableDeclarationList(VariableDeclarationList node) { + _append('VariableDeclarationList'); + super.visitVariableDeclarationList(node); + } + + // --- Arguments and parameters --- + + @override + void visitArgumentList(ArgumentList node) { + _append('ArgumentList'); + _append(node.arguments.length.toString()); + super.visitArgumentList(node); + } + + @override + void visitFormalParameterList(FormalParameterList node) { + _append('FormalParameterList'); + super.visitFormalParameterList(node); + } + + // --- Function expressions (closures) --- + + @override + void visitFunctionExpression(FunctionExpression node) { + _append('FunctionExpression'); + super.visitFunctionExpression(node); + } + + // --- Type arguments --- + + @override + void visitTypeArgumentList(TypeArgumentList node) { + _append('TypeArgumentList'); + super.visitTypeArgumentList(node); + } + + // --- Null-aware operators --- + + @override + void visitNullCheckPattern(NullCheckPattern node) { + _append('NullCheckPattern'); + super.visitNullCheckPattern(node); + } + + @override + void visitNullAssertPattern(NullAssertPattern node) { + _append('NullAssertPattern'); + super.visitNullAssertPattern(node); + } + + void _append(String value) { + _buffer.write(value); + _buffer.write('|'); + } +} diff --git a/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart new file mode 100644 index 00000000..1d56b31e --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart @@ -0,0 +1,154 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/visitors/ast_structural_hasher.dart'; + +/// A record representing a block or expression candidate for clone detection. +typedef _BodyCandidate = ({ + AstNode node, + Declaration? enclosingDeclaration, +}); + +/// A visitor that detects duplicate code blocks (at the function level and/or +/// statement block level) within a single compilation unit. +class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { + final AvoidDuplicateCodeRule _rule; + final AvoidDuplicateCodeParameters _parameters; + + /// Creates a new instance of [AvoidDuplicateCodeVisitor]. + AvoidDuplicateCodeVisitor(this._rule, this._parameters); + + @override + void visitCompilationUnit(CompilationUnit node) { + final collector = _CandidateCollector(_parameters); + node.accept(collector); + + final candidates = collector.candidates; + if (candidates.length < 2) return; + + final hasher = AstStructuralHasher( + ignoreLiterals: _parameters.ignoreLiterals, + ignoreIdentifiers: _parameters.ignoreIdentifiers, + ); + + // Group candidates by their structural hash + final groups = >{}; + for (final candidate in candidates) { + final hash = hasher.computeHash(candidate.node); + (groups[hash] ??= []).add(candidate); + } + + final lineInfo = node.lineInfo; + final suppressed = {}; + + // Candidates are collected pre-order (parent before children). + // By iterating in this order, we can process larger scopes first, + // and if a parent block is reported, suppress warnings on its children. + for (final candidate in candidates) { + if (suppressed.contains(candidate.node)) continue; + + final hash = hasher.computeHash(candidate.node); + final group = groups[hash]; + if (group == null) continue; + + // Filter group to only include non-suppressed candidates + final activeGroup = group + .where((c) => !suppressed.contains(c.node)) + .toList(); + if (activeGroup.length < 2) continue; + + final firstOccurrence = activeGroup.first; + final firstOffset = firstOccurrence.node.offset; + final firstLine = lineInfo.getLocation(firstOffset).lineNumber; + + for (var i = 1; i < activeGroup.length; i++) { + final duplicate = activeGroup[i]; + + // Report on the enclosing declaration if it's a function body, + // otherwise report directly on the statement block itself. + final isFuncBody = duplicate.node.parent is FunctionBody; + final reportNode = isFuncBody + ? (duplicate.enclosingDeclaration ?? duplicate.node) + : duplicate.node; + + _rule.reportAtNode( + reportNode, + arguments: [firstLine], + ); + + // Mark this duplicate block and all its descendants as suppressed + suppressed.add(duplicate.node); + _suppressDescendants(duplicate.node, suppressed); + } + } + } + + void _suppressDescendants(AstNode root, Set suppressed) { + root.accept(_DescendantCollector(suppressed, root)); + } +} + +/// A visitor that collects descendant candidate blocks to suppress them. +class _DescendantCollector extends RecursiveAstVisitor { + final Set suppressed; + final AstNode root; + + _DescendantCollector(this.suppressed, this.root); + + @override + void visitBlock(Block node) { + if (node != root) { + suppressed.add(node); + } + super.visitBlock(node); + } + + @override + void visitExpressionFunctionBody(ExpressionFunctionBody node) { + if (node != root) { + suppressed.add(node); + } + super.visitExpressionFunctionBody(node); + } +} + +class _CandidateCollector extends RecursiveAstVisitor { + final List<_BodyCandidate> candidates = []; + final AvoidDuplicateCodeParameters parameters; + + _CandidateCollector(this.parameters); + + @override + void visitBlock(Block node) { + // If checkBlocks is false, only consider blocks that represent function + // bodies. + if (!parameters.checkBlocks && node.parent is! BlockFunctionBody) { + super.visitBlock(node); + return; + } + + _checkAndCollect(node, node.statements.length); + super.visitBlock(node); + } + + @override + void visitExpressionFunctionBody(ExpressionFunctionBody node) { + _checkAndCollect(node, 1); + super.visitExpressionFunctionBody(node); + } + + void _checkAndCollect(AstNode node, int statementCount) { + if (statementCount < parameters.minStatements) return; + + final declaration = node.thisOrAncestorOfType(); + if (declaration != null && parameters.exclude.shouldIgnore(declaration)) { + return; + } + + candidates.add(( + node: node, + enclosingDeclaration: declaration, + )); + } +} diff --git a/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart b/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart new file mode 100644 index 00000000..be5e5dac --- /dev/null +++ b/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart @@ -0,0 +1,338 @@ +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/avoid_duplicate_code/avoid_duplicate_code_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../../../lints/auto_test_lint_offsets.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(AvoidDuplicateCodeRuleTest); + }); +} + +@reflectiveTest +class AvoidDuplicateCodeRuleTest extends AnalysisRuleTest + with AutoTestLintOffsets { + static const _mockAnalysisOptionsContent = ''' +plugins: + solid_lints: + diagnostics: + avoid_duplicate_code: + min_statements: 3 + check_blocks: true + exclude: + - method_name: excluded + '''; + + @override + void setUp() { + rule = AvoidDuplicateCodeRule( + analysisOptionsLoader: AnalysisOptionsLoader( + resourceProvider: resourceProvider, + ), + ); + super.setUp(); + + newAnalysisOptionsYamlFile( + testPackageRootPath, + '''${analysisOptionsContent(rules: [rule.name])} +$_mockAnalysisOptionsContent''', + ); + } + + // --- Base Tests (min_statements: 3, check_blocks: true, default) --- + + Future + test_reports_when_two_functions_have_identical_bodies() async { + await assertAutoDiagnostics(''' +void first() { + final x = 1; + if (x > 0) { + print(x); + } + print('done'); +} + +${expectLint(r'''void second() { + final x = 1; + if (x > 0) { + print(x); + } + print('done'); +}''')} +'''); + } + + Future + test_reports_when_functions_have_same_structure_different_names() async { + await assertAutoDiagnostics(''' +void fetchUsers() { + final response = 'data'; + if (response.isEmpty) { + throw Exception('error'); + } + print(response); +} + +${expectLint(r'''void fetchOrders() { + final result = 'data'; + if (result.isEmpty) { + throw Exception('error'); + } + print(result); +}''')} +'''); + } + + Future + test_does_not_report_when_functions_have_different_structure() async { + await assertNoDiagnostics(r''' +void first() { + final x = 1; + if (x > 0) { + print(x); + } + print('done'); +} + +void second() { + final x = 1; + while (x > 0) { + print(x); + } + return; +} +'''); + } + + Future + test_does_not_report_when_body_below_min_statements() async { + await assertNoDiagnostics(r''' +void first() { + print('hello'); + print('world'); +} + +void second() { + print('hello'); + print('world'); +} +'''); + } + + Future + test_reports_on_methods_in_same_class() async { + await assertAutoDiagnostics(''' +class MyClass { + void first() { + final x = 1; + if (x > 0) { + print(x); + } + print('done'); + } + + ${expectLint(r'''void second() { + final y = 1; + if (y > 0) { + print(y); + } + print('done'); + }''')} +} +'''); + } + + Future + test_reports_on_third_clone_also() async { + await assertAutoDiagnostics(''' +void first() { + final x = 1; + if (x > 0) { + print(x); + } + print('done'); +} + +${expectLint(r'''void second() { + final y = 1; + if (y > 0) { + print(y); + } + print('done'); +}''')} + +${expectLint(r'''void third() { + final z = 1; + if (z > 0) { + print(z); + } + print('done'); +}''')} +'''); + } + + // --- Ignore Literals Tests --- + + Future + test_reports_when_only_literal_values_differ() async { + newAnalysisOptionsYamlFile( + testPackageRootPath, + '''${analysisOptionsContent(rules: [rule.name])} +plugins: + solid_lints: + diagnostics: + avoid_duplicate_code: + min_statements: 3 + ignore_literals: true +''', + ); + await assertAutoDiagnostics(''' +void first() { + final x = 1; + if (x > 0) { + print('hello'); + } + print('world'); +} + +${expectLint(r'''void second() { + final y = 2; + if (y > 0) { + print('foo'); + } + print('bar'); +}''')} +'''); + } + + Future + test_does_not_report_when_literal_values_differ_without_flag() async { + await assertNoDiagnostics(r''' +void first() { + final x = 1; + if (x > 0) { + print('a'); + } + print('b'); +} + +void second() { + final y = 99; + if (y > 0) { + print('c'); + } + print('d'); +} +'''); + } + + // --- Ignore Identifiers Tests --- + + Future + test_does_not_report_when_identifiers_differ_without_flag() async { + newAnalysisOptionsYamlFile( + testPackageRootPath, + '''${analysisOptionsContent(rules: [rule.name])} +plugins: + solid_lints: + diagnostics: + avoid_duplicate_code: + min_statements: 3 + ignore_identifiers: false +''', + ); + await assertNoDiagnostics(r''' +void first() { + final x = 1; + if (x > 0) { + print(x); + } + print('done'); +} + +void second() { + final y = 1; + if (y > 0) { + print(y); + } + print('done'); +} +'''); + } + + // --- Exclude Tests --- + + Future + test_does_not_report_on_excluded_function() async { + await assertNoDiagnostics(r''' +void first() { + final x = 1; + if (x > 0) { + print(x); + } + print('done'); +} + +void excluded() { + final y = 1; + if (y > 0) { + print(y); + } + print('done'); +} +'''); + } + + // --- Check Blocks Tests --- + + Future + test_reports_duplicate_blocks_inside_different_functions() async { + await assertAutoDiagnostics(''' +void one() { + final x = 1; + if (x > 0) { + print('hello'); + print('world'); + print('done'); + } +} + +void two() { + final y = 2; + ${expectLint(r'''{ + print('hello'); + print('world'); + print('done'); + }''')} +} +'''); + } + + Future + test_reports_parent_but_not_nested_blocks_when_parent_is_reported() async { + await assertAutoDiagnostics(''' +void one() { + final x = 1; + if (x > 0) { + print('hello'); + print('world'); + print('done'); + } + print('done'); +} + +${expectLint(r'''void two() { + final y = 1; + if (y > 0) { + print('hello'); + print('world'); + print('done'); + } + print('done'); +}''')} +'''); + } +} From 77f24f878401b5c44c73da46dbcccda9cb23d4dc Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 8 Jul 2026 19:57:28 +0300 Subject: [PATCH 02/28] feat: increase default min_statements for avoid_duplicate_code from 3 to 10 --- .../lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart | 2 +- .../models/avoid_duplicate_code_parameters.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart b/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart index 19bf7a97..dc5ac313 100644 --- a/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart +++ b/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart @@ -19,7 +19,7 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// solid_lints: /// diagnostics: /// avoid_duplicate_code: -/// min_statements: 3 +/// min_statements: 10 /// ignore_literals: false /// ignore_identifiers: true /// check_blocks: false diff --git a/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart index 38dafd43..a45dec10 100644 --- a/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart +++ b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart @@ -21,7 +21,7 @@ class AvoidDuplicateCodeParameters { /// A list of methods/functions that should be excluded from the lint. final ExcludedIdentifiersListParameter exclude; - static const _defaultMinStatements = 3; + static const _defaultMinStatements = 10; /// Constructor for [AvoidDuplicateCodeParameters] model. const AvoidDuplicateCodeParameters({ From d8e267e9ec05903216ccde5e7c54b3cb9713edd9 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Thu, 9 Jul 2026 18:10:12 +0300 Subject: [PATCH 03/28] feat: enable cross-file duplicate code detection by implementing a global hash registry --- lib/analysis_options.yaml | 8 +- .../avoid_duplicate_code_rule.dart | 18 +- .../avoid_duplicate_code_parameters.dart | 3 +- .../models/cross_file_match.dart | 32 ++ .../models/file_cache_entry.dart | 40 +++ .../models/hash_entry.dart | 45 +++ .../services/global_hash_registry.dart | 306 ++++++++++++++++++ .../services/hash_cache_storage.dart | 122 +++++++ .../avoid_duplicate_code_visitor.dart | 305 +++++++++++++++-- .../avoid_duplicate_code_rule_test.dart | 77 ++++- .../global_hash_registry_test.dart | 217 +++++++++++++ 11 files changed, 1124 insertions(+), 49 deletions(-) create mode 100644 lib/src/lints/avoid_duplicate_code/models/cross_file_match.dart create mode 100644 lib/src/lints/avoid_duplicate_code/models/file_cache_entry.dart create mode 100644 lib/src/lints/avoid_duplicate_code/models/hash_entry.dart create mode 100644 lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart create mode 100644 lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart create mode 100644 test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart diff --git a/lib/analysis_options.yaml b/lib/analysis_options.yaml index dd2a9c95..1a86a8c6 100644 --- a/lib/analysis_options.yaml +++ b/lib/analysis_options.yaml @@ -1,11 +1,13 @@ analyzer: plugins: - - custom_lint + - solid_lints exclude: # General generated files - "**/*.g.dart" - "**/*.gr.dart" + - "src/lints/avoid_using_api/**" + # Flutter - "lib/generated_plugin_registrant.dart" @@ -38,6 +40,10 @@ analyzer: solid_lints: diagnostics: avoid_global_state: true + avoid_duplicate_code: + avoid_duplicate_code: true + min_statements: 3 + check_blocks: true avoid_late_keyword: allow_initialized: true diff --git a/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart b/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart index dc5ac313..08ce7209 100644 --- a/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart +++ b/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart @@ -6,12 +6,16 @@ import 'package:solid_lints/src/lints/avoid_duplicate_code/visitors/avoid_duplic import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// A lint rule that detects duplicated code blocks (clones) within a single -/// file. +/// file and across multiple files in the project. /// /// When two or more function/method/constructor bodies have structurally /// identical AST subtrees (Type 2 clones — same structure, variable names /// may differ), the rule reports all copies except the first occurrence. /// +/// Cross-file detection works on a "best-effort" basis: duplicates are +/// detected against files that have already been analyzed in the current +/// session. The detection improves as more files are analyzed. +/// /// ### Example config: /// /// ```yaml @@ -33,7 +37,7 @@ class AvoidDuplicateCodeRule static const _code = LintCode( lintName, - 'This code is a duplicate of the function/method at line {0}. ' + 'Perhaps this code is a duplicate.\n' 'Consider extracting the shared logic into a common function.', ); @@ -47,7 +51,7 @@ class AvoidDuplicateCodeRule name: lintName, description: 'Detects structurally identical function/method bodies ' - 'within a single file (code clones).', + 'within a single file and across files (code clones).', parametersParser: AvoidDuplicateCodeParameters.fromJson, ); @@ -62,7 +66,13 @@ class AvoidDuplicateCodeRule getParametersForContext(context) ?? AvoidDuplicateCodeParameters.empty(); - final visitor = AvoidDuplicateCodeVisitor(this, parameters); + final visitor = AvoidDuplicateCodeVisitor( + this, + parameters, + filePath: context.definingUnit.file.path, + modificationStamp: context.definingUnit.file.modificationStamp, + contextRoot: context.libraryElement?.session.analysisContext.contextRoot, + ); registry.addCompilationUnit(this, visitor); } diff --git a/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart index a45dec10..cfc5d623 100644 --- a/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart +++ b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart @@ -11,7 +11,8 @@ class AvoidDuplicateCodeParameters { final bool ignoreLiterals; /// When `true`, variable and method names (identifiers) are excluded - /// from the structural hash, allowing detection of renamed variables (Type 2). + /// from the structural hash, allowing detection of renamed variables + /// (Type 2). final bool ignoreIdentifiers; /// When `true`, statement blocks (like if-blocks or loops) inside functions diff --git a/lib/src/lints/avoid_duplicate_code/models/cross_file_match.dart b/lib/src/lints/avoid_duplicate_code/models/cross_file_match.dart new file mode 100644 index 00000000..54fd008f --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/models/cross_file_match.dart @@ -0,0 +1,32 @@ +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/hash_entry.dart'; + +/// Represents a location in the codebase where a duplicate was found. +class DuplicateLocation { + /// The matching hash entry in the other file. + final HashEntry entry; + + /// The absolute path of the other file. + final String filePath; + + /// Creates a new [DuplicateLocation]. + const DuplicateLocation({ + required this.entry, + required this.filePath, + }); +} + +/// Represents a structural duplicate found in other files. +class CrossFileMatch { + /// The hash entry in the current file. + final HashEntry currentEntry; + + /// The list of locations in other files where a duplicate of this entry + /// exists. + final List duplicates; + + /// Creates a new [CrossFileMatch]. + const CrossFileMatch({ + required this.currentEntry, + required this.duplicates, + }); +} diff --git a/lib/src/lints/avoid_duplicate_code/models/file_cache_entry.dart b/lib/src/lints/avoid_duplicate_code/models/file_cache_entry.dart new file mode 100644 index 00000000..23677fa6 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/models/file_cache_entry.dart @@ -0,0 +1,40 @@ +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/hash_entry.dart'; + +/// A cache entry containing the file's modification stamp and its structural hashes. +class FileCacheEntry { + /// The file modification stamp. + final int modificationStamp; + + /// The list of structural hash entries for this file. + final List entries; + + /// Creates a new [FileCacheEntry]. + const FileCacheEntry({ + required this.modificationStamp, + required this.entries, + }); + + /// Converts this [FileCacheEntry] to a JSON map. + Map toJson() => { + 'm': modificationStamp, + 'e': entries.map((e) => e.toJson()).toList(), + }; + + /// Parses a [FileCacheEntry] from a JSON map. + factory FileCacheEntry.fromJson(Map json) { + final entriesList = []; + if (json['e'] is List) { + for (final item in json['e'] as List) { + if (item is Map) { + try { + entriesList.add(HashEntry.fromJson(item)); + } catch (_) {} + } + } + } + return FileCacheEntry( + modificationStamp: (json['m'] as int?) ?? 0, + entries: entriesList, + ); + } +} diff --git a/lib/src/lints/avoid_duplicate_code/models/hash_entry.dart b/lib/src/lints/avoid_duplicate_code/models/hash_entry.dart new file mode 100644 index 00000000..a136905c --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/models/hash_entry.dart @@ -0,0 +1,45 @@ +/// A single structural hash entry representing a function body or block +/// candidate for cross-file clone detection. +class HashEntry { + /// The structural hash of the AST subtree. + final int hash; + + /// The line number where this candidate starts. + final int lineNumber; + + /// The character offset from the start of the file. + final int offset; + + /// The length of the code block. + final int length; + + /// The number of statements in the candidate body. + final int statementCount; + + /// Creates a new [HashEntry]. + const HashEntry({ + required this.hash, + required this.lineNumber, + required this.statementCount, + this.offset = 0, + this.length = 0, + }); + + /// Converts this [HashEntry] to a JSON-compatible map using shortened keys. + Map toJson() => { + 'h': hash, + 'n': lineNumber, + 'o': offset, + 'l': length, + 's': statementCount, + }; + + /// Creates a [HashEntry] from a JSON map, supporting both old and new keys. + factory HashEntry.fromJson(Map json) => HashEntry( + hash: (json['h'] ?? json['hash'])! as int, + lineNumber: (json['n'] ?? json['lineNumber'])! as int, + offset: (json['o'] ?? json['offset'] ?? 0) as int, + length: (json['l'] ?? json['length'] ?? 0) as int, + statementCount: (json['s'] ?? json['statementCount'])! as int, + ); +} diff --git a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart new file mode 100644 index 00000000..4b2b11ea --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart @@ -0,0 +1,306 @@ +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; +import 'package:path/path.dart' as p; +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/cross_file_match.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/file_cache_entry.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/hash_entry.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart'; + +/// A singleton registry that stores structural hashes for all analyzed files. +/// +/// This registry lives as long as the plugin isolate, enabling cross-file +/// clone detection on a "best-effort" basis — duplicates are detected +/// against files that have already been analyzed in the current session. +/// +/// When a file is analyzed, its hash entries are stored in the registry. +/// Subsequent file analyses check their hashes against the registry to find +/// cross-file duplicates. +class GlobalHashRegistry { + /// Singleton instance — lives as long as the plugin isolate. + static final instance = GlobalHashRegistry._(); + + /// Internal index: filePath → FileCacheEntry. + final _index = {}; + + /// Auxiliary inverted index: hash → Set. + final _hashToLocations = >{}; + + final _loadedRoots = {}; + + Timer? _saveTimer; + + /// Whether to automatically clean up files that do not exist physically on + /// disk. + /// + /// Set to `false` in tests using a virtual resource provider. + bool enablePhysicalFileCleanup = true; + + GlobalHashRegistry._(); + + void _addToInvertedIndex(String absoluteFilePath, List entries) { + for (final entry in entries) { + _hashToLocations.putIfAbsent(entry.hash, () => {}).add( + DuplicateLocation( + entry: entry, + filePath: absoluteFilePath, + ), + ); + } + } + + void _removeFromInvertedIndex( + String absoluteFilePath, + List entries, + ) { + for (final entry in entries) { + final set = _hashToLocations[entry.hash]; + if (set != null) { + set.removeWhere((loc) => loc.filePath == absoluteFilePath); + if (set.isEmpty) { + _hashToLocations.remove(entry.hash); + } + } + } + } + + void _ensureLoaded(String packageRoot) { + if (_loadedRoots.contains(packageRoot)) return; + _loadedRoots.add(packageRoot); + final cached = HashCacheStorage.load(packageRoot); + if (cached != null) { + _index.addAll(cached); + + // Clean up files that were physically deleted once upon loading cache + final deletedFiles = []; + for (final path in cached.keys) { + if (enablePhysicalFileCleanup && !File(path).existsSync()) { + deletedFiles.add(path); + } + } + for (final file in deletedFiles) { + _index.remove(file); + } + + for (final MapEntry(:key, :value) in _index.entries) { + _addToInvertedIndex(key, value.entries); + } + + if (deletedFiles.isNotEmpty) { + _scheduleSave(packageRoot); + } + } + } + + /// Returns the cached modification stamp for [filePath], or `null` if not indexed. + int? getModificationStamp(String filePath, {String? packageRoot}) { + final root = packageRoot ?? Directory.current.path; + _ensureLoaded(root); + final absoluteFilePath = p.isAbsolute(filePath) + ? p.normalize(filePath) + : p.normalize(p.join(root, filePath)); + return _index[absoluteFilePath]?.modificationStamp; + } + + /// Returns the cached entries for [filePath], or `null` if not indexed. + List? getFileEntries(String filePath, {String? packageRoot}) { + final root = packageRoot ?? Directory.current.path; + _ensureLoaded(root); + final absoluteFilePath = p.isAbsolute(filePath) + ? p.normalize(filePath) + : p.normalize(p.join(root, filePath)); + return _index[absoluteFilePath]?.entries; + } + + /// Updates the hash entries for [filePath], replacing any previous entries. + void updateFile( + String filePath, + List entries, { + required int modificationStamp, + String? packageRoot, + }) { + final root = packageRoot ?? Directory.current.path; + _ensureLoaded(root); + final absoluteFilePath = p.isAbsolute(filePath) + ? p.normalize(filePath) + : p.normalize(p.join(root, filePath)); + + final oldEntry = _index[absoluteFilePath]; + if (oldEntry != null) { + _removeFromInvertedIndex(absoluteFilePath, oldEntry.entries); + } + + _index[absoluteFilePath] = FileCacheEntry( + modificationStamp: modificationStamp, + entries: entries, + ); + _addToInvertedIndex(absoluteFilePath, entries); + _scheduleSave(root); + } + + /// Finds cross-file duplicates for [currentEntries] against all other + /// indexed files (excluding [currentFilePath]). + /// + /// Returns a list of [CrossFileMatch] objects, one for each duplicate found. + List findCrossFileMatches( + String currentFilePath, + List currentEntries, { + bool Function(String filePath)? isFileExcluded, + String? packageRoot, + }) { + final root = packageRoot ?? Directory.current.path; + _ensureLoaded(root); + + final absoluteCurrentFilePath = p.isAbsolute(currentFilePath) + ? p.normalize(currentFilePath) + : p.normalize(p.join(root, currentFilePath)); + + final matches = []; + final filesToRemove = {}; + + for (final entry in currentEntries) { + final duplicates = []; + + final locations = _hashToLocations[entry.hash]; + if (locations != null) { + for (final loc in locations) { + final key = loc.filePath; + if (key == absoluteCurrentFilePath) continue; + + // Check if deleted or excluded on demand only for matched locations + final isDeleted = enablePhysicalFileCleanup && !File(key).existsSync(); + final isExcluded = isFileExcluded != null && isFileExcluded(key); + + if (isDeleted || isExcluded) { + filesToRemove.add(key); + continue; + } + + if (!key.startsWith(root)) continue; + if (key.contains('.dart_tool/')) { + if (!key.contains('.dart_tool/generated_test/')) continue; + if (p.dirname(key) != p.dirname(absoluteCurrentFilePath)) continue; + } + + duplicates.add(loc); + } + } + + if (duplicates.isNotEmpty) { + matches.add( + CrossFileMatch( + currentEntry: entry, + duplicates: duplicates, + ), + ); + } + } + + if (filesToRemove.isNotEmpty) { + for (final file in filesToRemove) { + final oldEntry = _index[file]; + if (oldEntry != null) { + _removeFromInvertedIndex(file, oldEntry.entries); + } + _index.remove(file); + } + _scheduleSave(root); + } + + return matches; + } + + /// Removes [filePath] from the index. + void removeFile(String filePath, {String? packageRoot}) { + final root = packageRoot ?? Directory.current.path; + _ensureLoaded(root); + final absoluteFilePath = p.isAbsolute(filePath) + ? p.normalize(filePath) + : p.normalize(p.join(root, filePath)); + final oldEntry = _index[absoluteFilePath]; + if (oldEntry != null) { + _removeFromInvertedIndex(absoluteFilePath, oldEntry.entries); + } + _index.remove(absoluteFilePath); + _scheduleSave(root); + } + + /// The number of files currently indexed. + int get fileCount => _index.length; + + /// Clears the entire index and deletes the persistent cache file. + /// + /// Primarily used in tests to ensure test isolation. + void clear() { + _saveTimer?.cancel(); + _loadedRoots.clear(); + _index.clear(); + _hashToLocations.clear(); + HashCacheStorage.delete(Directory.current.path); + try { + final file = File( + '${Directory.current.path}/.dart_tool/solid_lints/duplicate_inverted_index.json', + ); + if (file.existsSync()) { + file.deleteSync(); + } + } catch (_) {} + } + + void _scheduleSave(String packageRoot) { + _saveTimer?.cancel(); + _saveTimer = Timer(const Duration(milliseconds: 500), () { + _performSave(packageRoot); + }); + } + + void _performSave(String packageRoot) { + // Filter _index for files belonging to this packageRoot + final subset = { + for (final MapEntry(:key, :value) in _index.entries) + if (key.startsWith(packageRoot)) key: value + }; + + HashCacheStorage.save(packageRoot, subset); + + // Save inverted index to duplicate_inverted_index.json + try { + final jsonInverted = >>{}; + _hashToLocations.forEach((hash, locations) { + final packageLocations = locations + .where((loc) => loc.filePath.startsWith(packageRoot)) + .toList(); + if (packageLocations.isEmpty) return; + + jsonInverted[hash.toString()] = packageLocations.map((loc) { + final relativePath = p.isAbsolute(loc.filePath) + ? p.relative(loc.filePath, from: packageRoot) + : loc.filePath; + return { + 'filePath': relativePath, + 'entry': loc.entry.toJson(), + }; + }).toList(); + }); + + final file = File( + '$packageRoot/.dart_tool/solid_lints/duplicate_inverted_index.json', + ); + if (jsonInverted.isEmpty) { + if (file.existsSync()) { + file.deleteSync(); + } + } else { + final directory = file.parent; + if (!directory.existsSync()) { + directory.createSync(recursive: true); + } + file.writeAsStringSync( + const JsonEncoder.withIndent(' ').convert(jsonInverted), + ); + } + } catch (_) { + // Fail silently to avoid breaking analysis server + } + } +} diff --git a/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart b/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart new file mode 100644 index 00000000..114bdcb6 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart @@ -0,0 +1,122 @@ +import 'dart:convert'; +import 'dart:io'; +import 'package:path/path.dart' as p; +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/file_cache_entry.dart'; + +/// Service responsible for persisting and loading the duplicate code hash +/// cache. +class HashCacheStorage { + static String _filePath(String packageRoot) => + '$packageRoot/.dart_tool/solid_lints/duplicate_index.json'; + + /// Loads the cached index from disk for the given [packageRoot]. + /// + /// Returns `null` if the cache file does not exist, is corrupted, or fails + /// to load. + static Map? load(String packageRoot) { + final stopwatch = Stopwatch()..start(); + final startTime = DateTime.now(); + try { + final file = File(_filePath(packageRoot)); + if (!file.existsSync()) return null; + + final content = file.readAsStringSync(); + final jsonMap = jsonDecode(content) as Map; + + final result = {}; + for (final MapEntry(:key, :value) in jsonMap.entries) { + final absoluteKey = p.isAbsolute(key) + ? p.normalize(key) + : p.normalize(p.join(packageRoot, key)); + + if (value is Map) { + try { + result[absoluteKey] = FileCacheEntry.fromJson(value); + } catch (_) { + // Skip corrupted entries + } + } + } + stopwatch.stop(); + _writeLoadLog( + packageRoot, + startTime, + stopwatch.elapsedMilliseconds, + result.length, + ); + return result; + } catch (_) { + stopwatch.stop(); + _writeLoadLog(packageRoot, startTime, stopwatch.elapsedMilliseconds, -1); + return null; + } + } + + static void _writeLoadLog( + String packageRoot, + DateTime time, + int elapsedMs, + int filesCount, + ) { + try { + final logFile = File( + '$packageRoot/.dart_tool/solid_lints/duplicate_index_load.log', + ); + final logLine = + '${time.toIso8601String()}: Loaded map in ${elapsedMs}ms (files: ${filesCount >= 0 ? filesCount : 'failed'})\n'; + logFile.writeAsStringSync(logLine, mode: FileMode.append); + } catch (_) { + // Fail silently + } + } + + /// Saves the [index] to disk for the given [packageRoot]. + static void save(String packageRoot, Map index) { + try { + final cacheDir = '$packageRoot/.dart_tool/solid_lints'; + final directory = Directory(cacheDir); + if (!directory.existsSync()) { + directory.createSync(recursive: true); + } + + final file = File(_filePath(packageRoot)); + if (index.isEmpty) { + file.writeAsStringSync('{}'); + return; + } + + final buffer = StringBuffer('{\n'); + final entriesList = index.entries.toList(); + for (var i = 0; i < entriesList.length; i++) { + final entry = entriesList[i]; + final relativeKey = p.isAbsolute(entry.key) + ? p.relative(entry.key, from: packageRoot) + : entry.key; + final listJson = jsonEncode(entry.value.toJson()); + + buffer.write(' ${jsonEncode(relativeKey)}: $listJson'); + if (i < entriesList.length - 1) { + buffer.write(',\n'); + } else { + buffer.write('\n'); + } + } + buffer.write('}'); + file.writeAsStringSync(buffer.toString()); + } catch (_) { + // Fail silently to avoid breaking analysis server + } + } + + /// Deletes the cache file for [packageRoot] if it exists. + static void delete(String packageRoot) { + try { + final file = File(_filePath(packageRoot)); + if (file.existsSync()) { + file.deleteSync(); + } + } catch (_) { + // Fail silently + } + } +} diff --git a/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart index 1d56b31e..cc31ce60 100644 --- a/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart +++ b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart @@ -1,7 +1,16 @@ +import 'dart:io'; + +import 'package:analyzer/dart/analysis/context_root.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/diagnostic/diagnostic.dart'; +import 'package:analyzer/src/diagnostic/diagnostic_message.dart'; +import 'package:path/path.dart' as path; import 'package:solid_lints/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/cross_file_match.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/hash_entry.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/services/global_hash_registry.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/visitors/ast_structural_hasher.dart'; /// A record representing a block or expression candidate for clone detection. @@ -11,82 +20,320 @@ typedef _BodyCandidate = ({ }); /// A visitor that detects duplicate code blocks (at the function level and/or -/// statement block level) within a single compilation unit. +/// statement block level) within a single compilation unit and across files. class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { + static int _processedFilesCount = 0; + static int _totalDurationMs = 0; + final AvoidDuplicateCodeRule _rule; final AvoidDuplicateCodeParameters _parameters; + final String _filePath; + final int _modificationStamp; + final ContextRoot? _contextRoot; /// Creates a new instance of [AvoidDuplicateCodeVisitor]. - AvoidDuplicateCodeVisitor(this._rule, this._parameters); + AvoidDuplicateCodeVisitor( + this._rule, + this._parameters, { + required String filePath, + required int modificationStamp, + ContextRoot? contextRoot, + }) : _filePath = filePath, + _modificationStamp = modificationStamp, + _contextRoot = contextRoot; @override void visitCompilationUnit(CompilationUnit node) { + final stopwatch = Stopwatch()..start(); + final startTime = DateTime.now(); + + final filePath = _filePath; + + // --- Phase 0: Try to use cached entries if file is unchanged and has no warnings --- + if (filePath.isNotEmpty) { + final contextRoot = _contextRoot; + final packageRoot = contextRoot?.root.path ?? _findPackageRoot(filePath) ?? ''; + if (packageRoot.isNotEmpty) { + final registry = GlobalHashRegistry.instance; + final cachedStamp = registry.getModificationStamp(filePath, packageRoot: packageRoot); + + if (cachedStamp == _modificationStamp) { + final cachedEntries = registry.getFileEntries(filePath, packageRoot: packageRoot); + if (cachedEntries != null) { + // Check if there are any cross-file matches + final crossMatches = registry.findCrossFileMatches( + filePath, + cachedEntries, + packageRoot: packageRoot, + isFileExcluded: (otherPath) { + if (contextRoot == null) return false; + final isWithin = otherPath.startsWith(contextRoot.root.path); + return isWithin && !contextRoot.isAnalyzed(otherPath); + }, + ); + + // Check if there are any intra-file duplicates + final hashCounts = {}; + for (final entry in cachedEntries) { + hashCounts[entry.hash] = (hashCounts[entry.hash] ?? 0) + 1; + } + final hasIntraDuplicates = hashCounts.values.any((count) => count > 1); + final hasCrossDuplicates = crossMatches.isNotEmpty; + + if (!hasIntraDuplicates && !hasCrossDuplicates) { + // 0 warnings and file is unchanged -> Skip entire AST traversal! + stopwatch.stop(); + _writeRunLog( + packageRoot, + startTime, + stopwatch.elapsedMilliseconds, + filePath, + cachedEntries.length, + 0, + cached: true, + ); + return; + } + } + } + } + } + final collector = _CandidateCollector(_parameters); node.accept(collector); - final candidates = collector.candidates; - if (candidates.length < 2) return; final hasher = AstStructuralHasher( ignoreLiterals: _parameters.ignoreLiterals, ignoreIdentifiers: _parameters.ignoreIdentifiers, ); - // Group candidates by their structural hash - final groups = >{}; + // --- Phase A: Build hash entries for cross-file registry --- + final hashEntries = []; + final candidateHashes = {}; + for (final candidate in candidates) { final hash = hasher.computeHash(candidate.node); + candidateHashes[candidate.node] = hash; + final line = node.lineInfo.getLocation(candidate.node.offset).lineNumber; + hashEntries.add( + HashEntry( + hash: hash, + lineNumber: line, + offset: candidate.node.offset, + length: candidate.node.length, + statementCount: _statementCount(candidate.node), + ), + ); + } + + // --- Phase B: Find Cross-file matches from registry --- + final crossFileDuplicatesByHash = >{}; + if (filePath.isNotEmpty) { + final contextRoot = _contextRoot; + final packageRoot = contextRoot?.root.path ?? _findPackageRoot(filePath) ?? ''; + + final registry = GlobalHashRegistry.instance; + final crossMatches = registry.findCrossFileMatches( + filePath, + hashEntries, + packageRoot: packageRoot, + isFileExcluded: (otherPath) { + if (contextRoot == null) return false; + final isWithin = otherPath.startsWith(contextRoot.root.path); + return isWithin && !contextRoot.isAnalyzed(otherPath); + }, + ); + for (final match in crossMatches) { + crossFileDuplicatesByHash[match.currentEntry.hash] = match.duplicates; + } + registry.updateFile( + filePath, + hashEntries, + modificationStamp: _modificationStamp, + packageRoot: packageRoot, + ); + } + + // --- Phase C: Group candidates by structural hash for intra-file --- + final groups = >{}; + for (final candidate in candidates) { + final hash = candidateHashes[candidate.node] ?? + hasher.computeHash(candidate.node); (groups[hash] ??= []).add(candidate); } - final lineInfo = node.lineInfo; final suppressed = {}; + final reported = {}; + var warningCount = 0; // Candidates are collected pre-order (parent before children). - // By iterating in this order, we can process larger scopes first, - // and if a parent block is reported, suppress warnings on its children. + // Process larger scopes first, and suppress warnings on children. for (final candidate in candidates) { if (suppressed.contains(candidate.node)) continue; + if (reported.contains(candidate.node)) continue; - final hash = hasher.computeHash(candidate.node); + final hash = candidateHashes[candidate.node] ?? + hasher.computeHash(candidate.node); final group = groups[hash]; if (group == null) continue; // Filter group to only include non-suppressed candidates - final activeGroup = group - .where((c) => !suppressed.contains(c.node)) - .toList(); - if (activeGroup.length < 2) continue; + final activeGroup = + group.where((c) => !suppressed.contains(c.node)).toList(); + + final externalPartners = crossFileDuplicatesByHash[hash] ?? const []; + final internalPartners = activeGroup.where((c) => c != candidate).toList(); - final firstOccurrence = activeGroup.first; - final firstOffset = firstOccurrence.node.offset; - final firstLine = lineInfo.getLocation(firstOffset).lineNumber; + final hasExternalDuplicates = externalPartners.isNotEmpty; + final hasInternalDuplicates = internalPartners.isNotEmpty; - for (var i = 1; i < activeGroup.length; i++) { - final duplicate = activeGroup[i]; + // Report a warning if there are any duplicates (internal or external) + final shouldReport = hasInternalDuplicates || hasExternalDuplicates; - // Report on the enclosing declaration if it's a function body, - // otherwise report directly on the statement block itself. - final isFuncBody = duplicate.node.parent is FunctionBody; - final reportNode = isFuncBody - ? (duplicate.enclosingDeclaration ?? duplicate.node) - : duplicate.node; + if (shouldReport) { + final reportNode = _findReportNode(candidate.node); + + final contextMessages = [ + // Add external partners + for (final dup in externalPartners) + DiagnosticMessageImpl( + filePath: dup.filePath, + offset: dup.entry.offset, + length: dup.entry.length, + message: 'Duplicate', + url: null, + ), + // Add internal partners + for (final other in internalPartners) + DiagnosticMessageImpl( + filePath: filePath, + offset: other.node.offset, + length: other.node.length, + message: 'Duplicate', + url: null, + ), + ]; _rule.reportAtNode( reportNode, - arguments: [firstLine], + contextMessages: contextMessages, ); + warningCount++; + + // Mark this duplicate block as reported. + reported.add(candidate.node); + // Mark all its descendants as suppressed. + _suppressDescendants(candidate.node, suppressed); + } + } - // Mark this duplicate block and all its descendants as suppressed - suppressed.add(duplicate.node); - _suppressDescendants(duplicate.node, suppressed); + stopwatch.stop(); + if (filePath.isNotEmpty) { + final contextRoot = _contextRoot; + final packageRoot = + contextRoot?.root.path ?? _findPackageRoot(filePath) ?? ''; + if (packageRoot.isNotEmpty) { + _writeRunLog( + packageRoot, + startTime, + stopwatch.elapsedMilliseconds, + filePath, + candidates.length, + warningCount, + cached: false, + ); } } } + /// Returns the number of statements in a body node. + static int _statementCount(AstNode node) { + if (node is Block) return node.statements.length; + if (node is ExpressionFunctionBody) return 1; + return 0; + } + void _suppressDescendants(AstNode root, Set suppressed) { root.accept(_DescendantCollector(suppressed, root)); } + + AstNode _findReportNode(AstNode node) { + final parent = node.parent; + if (parent is FunctionBody) { + final declaration = parent.parent; + if (declaration != null) { + if (declaration is FunctionDeclaration || + declaration is MethodDeclaration || + declaration is ConstructorDeclaration) { + return declaration; + } + if (declaration is FunctionExpression) { + final grandDeclaration = declaration.parent; + if (grandDeclaration is FunctionDeclaration) { + return grandDeclaration; + } + return declaration; + } + } + } + return node; + } + + String? _findPackageRoot(String filePath) { + if (filePath.isEmpty) return null; + var dir = File(filePath).parent; + while (true) { + final pubspec = File(path.join(dir.path, 'pubspec.yaml')); + if (pubspec.existsSync()) { + return dir.path; + } + final parent = dir.parent; + if (parent.path == dir.path) { + break; + } + dir = parent; + } + return null; + } + + void _writeRunLog( + String packageRoot, + DateTime startTime, + int elapsedMs, + String filePath, + int candidateCount, + int warningCount, { + required bool cached, + }) { + try { + _processedFilesCount++; + _totalDurationMs += elapsedMs; + final logFile = File( + '$packageRoot/.dart_tool/solid_lints/avoid_duplicate_code_run.log', + ); + + // Ensure the directory exists + final directory = logFile.parent; + if (!directory.existsSync()) { + directory.createSync(recursive: true); + } + + final relativePath = + packageRoot.isNotEmpty && filePath.startsWith(packageRoot) + ? path.relative(filePath, from: packageRoot) + : filePath; + final ramMb = ProcessInfo.currentRss / (1024 * 1024); + final cacheIndicator = cached ? ' [Cached]' : ''; + final logLine = + '${startTime.toIso8601String()}: Checked #$_processedFilesCount ($relativePath) ' + 'in ${elapsedMs}ms$cacheIndicator | Candidates: $candidateCount | Warnings: $warningCount | RAM: ${ramMb.toStringAsFixed(1)} MB ' + '(Total session time: ${_totalDurationMs}ms)\n'; + logFile.writeAsStringSync(logLine, mode: FileMode.append); + } catch (_) { + // Fail silently + } + } } /// A visitor that collects descendant candidate blocks to suppress them. diff --git a/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart b/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart index be5e5dac..18d7dc51 100644 --- a/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart +++ b/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart @@ -1,7 +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/common/parameters/excluded_identifiers_list_parameter.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/services/global_hash_registry.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart'; import 'package:test_reflective_loader/test_reflective_loader.dart'; import '../../../lints/auto_test_lint_offsets.dart'; @@ -28,6 +32,8 @@ plugins: @override void setUp() { + GlobalHashRegistry.instance.clear(); + GlobalHashRegistry.instance.enablePhysicalFileCleanup = false; rule = AvoidDuplicateCodeRule( analysisOptionsLoader: AnalysisOptionsLoader( resourceProvider: resourceProvider, @@ -42,18 +48,24 @@ $_mockAnalysisOptionsContent''', ); } + @override + Future tearDown() async { + GlobalHashRegistry.instance.clear(); + await super.tearDown(); + } + // --- Base Tests (min_statements: 3, check_blocks: true, default) --- Future test_reports_when_two_functions_have_identical_bodies() async { await assertAutoDiagnostics(''' -void first() { +${expectLint(r'''void first() { final x = 1; if (x > 0) { print(x); } print('done'); -} +}''')} ${expectLint(r'''void second() { final x = 1; @@ -68,13 +80,13 @@ ${expectLint(r'''void second() { Future test_reports_when_functions_have_same_structure_different_names() async { await assertAutoDiagnostics(''' -void fetchUsers() { +${expectLint(r'''void fetchUsers() { final response = 'data'; if (response.isEmpty) { throw Exception('error'); } print(response); -} +}''')} ${expectLint(r'''void fetchOrders() { final result = 'data'; @@ -126,13 +138,13 @@ void second() { test_reports_on_methods_in_same_class() async { await assertAutoDiagnostics(''' class MyClass { - void first() { + ${expectLint(r'''void first() { final x = 1; if (x > 0) { print(x); } print('done'); - } + }''')} ${expectLint(r'''void second() { final y = 1; @@ -148,13 +160,13 @@ class MyClass { Future test_reports_on_third_clone_also() async { await assertAutoDiagnostics(''' -void first() { +${expectLint(r'''void first() { final x = 1; if (x > 0) { print(x); } print('done'); -} +}''')} ${expectLint(r'''void second() { final y = 1; @@ -190,13 +202,13 @@ plugins: ''', ); await assertAutoDiagnostics(''' -void first() { +${expectLint(r'''void first() { final x = 1; if (x > 0) { print('hello'); } print('world'); -} +}''')} ${expectLint(r'''void second() { final y = 2; @@ -293,11 +305,11 @@ void excluded() { await assertAutoDiagnostics(''' void one() { final x = 1; - if (x > 0) { + if (x > 0) ${expectLint(r'''{ print('hello'); print('world'); print('done'); - } + }''')} } void two() { @@ -314,7 +326,7 @@ void two() { Future test_reports_parent_but_not_nested_blocks_when_parent_is_reported() async { await assertAutoDiagnostics(''' -void one() { +${expectLint(r'''void one() { final x = 1; if (x > 0) { print('hello'); @@ -322,7 +334,7 @@ void one() { print('done'); } print('done'); -} +}''')} ${expectLint(r'''void two() { final y = 1; @@ -333,6 +345,43 @@ ${expectLint(r'''void two() { } print('done'); }''')} +'''); + } + + Future test_reports_cross_file_duplicate_when_in_registry() async { + final otherFile = newFile('$testPackageLibPath/other.dart', ''' +void otherMethod() { + final x = 1; + if (x > 0) { + print(x); + } + print('done'); +} +'''); + + final resolvedOther = await resolveFile(otherFile.path); + final visitor = AvoidDuplicateCodeVisitor( + rule as AvoidDuplicateCodeRule, + AvoidDuplicateCodeParameters( + minStatements: 3, + ignoreLiterals: false, + ignoreIdentifiers: true, + checkBlocks: true, + exclude: ExcludedIdentifiersListParameter(exclude: []), + ), + filePath: otherFile.path, + modificationStamp: 1, + ); + resolvedOther.unit.accept(visitor); + + await assertAutoDiagnostics(''' +${expectLint(r'''void mainMethod() { + final x = 1; + if (x > 0) { + print(x); + } + print('done'); +}''')} '''); } } diff --git a/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart b/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart new file mode 100644 index 00000000..59484ecf --- /dev/null +++ b/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart @@ -0,0 +1,217 @@ +import 'dart:io'; +import 'package:path/path.dart' as p; +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/file_cache_entry.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/hash_entry.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/services/global_hash_registry.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart'; +import 'package:test/test.dart'; + +void main() { + group('GlobalHashRegistry', () { + late GlobalHashRegistry registry; + + setUp(() { + registry = GlobalHashRegistry.instance; + registry.clear(); + registry.enablePhysicalFileCleanup = false; + }); + + tearDown(() { + registry.clear(); + registry.enablePhysicalFileCleanup = true; + }); + + test('updateFile stores entries', () { + final entries = [ + const HashEntry(hash: 123, lineNumber: 10, statementCount: 5), + const HashEntry(hash: 456, lineNumber: 20, statementCount: 3), + ]; + + registry.updateFile('file_a.dart', entries, modificationStamp: 1); + + expect(registry.fileCount, equals(1)); + }); + + test('findCrossFileMatches finds duplicate in other files', () { + final fileAEntries = [ + const HashEntry(hash: 123, lineNumber: 10, statementCount: 5), + ]; + final fileBEntries = [ + const HashEntry(hash: 123, lineNumber: 15, statementCount: 5), + ]; + + registry.updateFile('file_a.dart', fileAEntries, modificationStamp: 1); + + final matches = registry.findCrossFileMatches('file_b.dart', fileBEntries); + + expect(matches, hasLength(1)); + expect(matches.first.duplicates, hasLength(1)); + final expectedPath = p.normalize(p.join(Directory.current.path, 'file_a.dart')); + expect(matches.first.duplicates.first.filePath, equals(expectedPath)); + expect(matches.first.duplicates.first.entry.hash, equals(123)); + expect(matches.first.duplicates.first.entry.lineNumber, equals(10)); + }); + + test('findCrossFileMatches ignores same file', () { + final entries = [ + const HashEntry(hash: 123, lineNumber: 10, statementCount: 5), + ]; + + registry.updateFile('file_a.dart', entries, modificationStamp: 1); + + final matches = registry.findCrossFileMatches('file_a.dart', entries); + + expect(matches, isEmpty); + }); + + test('updateFile replaces previous entries', () { + final oldEntries = [ + const HashEntry(hash: 123, lineNumber: 10, statementCount: 5), + ]; + final newEntries = [ + const HashEntry(hash: 456, lineNumber: 20, statementCount: 3), + ]; + + registry.updateFile('file_a.dart', oldEntries, modificationStamp: 1); + registry.updateFile('file_a.dart', newEntries, modificationStamp: 2); + + expect(registry.fileCount, equals(1)); + + // File B tries to match against the old hash 123, should find nothing + final matches1 = registry.findCrossFileMatches('file_b.dart', [ + const HashEntry(hash: 123, lineNumber: 15, statementCount: 5), + ]); + expect(matches1, isEmpty); + + // File B tries to match against the new hash 456, should match + final matches2 = registry.findCrossFileMatches('file_b.dart', [ + const HashEntry(hash: 456, lineNumber: 25, statementCount: 3), + ]); + expect(matches2, hasLength(1)); + }); + + test('removeFile clears entries for specific file', () { + registry.updateFile('file_a.dart', [ + const HashEntry(hash: 123, lineNumber: 10, statementCount: 5), + ], modificationStamp: 1); + registry.updateFile('file_b.dart', [ + const HashEntry(hash: 456, lineNumber: 20, statementCount: 5), + ], modificationStamp: 1); + + expect(registry.fileCount, equals(2)); + + registry.removeFile('file_a.dart'); + + expect(registry.fileCount, equals(1)); + + final matches = registry.findCrossFileMatches('file_c.dart', [ + const HashEntry(hash: 123, lineNumber: 30, statementCount: 5), + ]); + expect(matches, isEmpty); + }); + + test('clear empties the registry', () { + registry.updateFile('file_a.dart', [ + const HashEntry(hash: 123, lineNumber: 10, statementCount: 5), + ], modificationStamp: 1); + expect(registry.fileCount, equals(1)); + + registry.clear(); + + expect(registry.fileCount, equals(0)); + }); + + test('findCrossFileMatches groups multiple duplicate locations', () { + registry.updateFile('file_a.dart', [ + const HashEntry(hash: 123, lineNumber: 10, statementCount: 5), + ], modificationStamp: 1); + registry.updateFile('file_b.dart', [ + const HashEntry(hash: 123, lineNumber: 20, statementCount: 5), + ], modificationStamp: 1); + + final matches = registry.findCrossFileMatches('file_c.dart', [ + const HashEntry(hash: 123, lineNumber: 30, statementCount: 5), + ]); + + expect(matches, hasLength(1)); + expect(matches.first.duplicates, hasLength(2)); + final expectedPathA = p.normalize(p.join(Directory.current.path, 'file_a.dart')); + final expectedPathB = p.normalize(p.join(Directory.current.path, 'file_b.dart')); + expect(matches.first.duplicates[0].filePath, equals(expectedPathA)); + expect(matches.first.duplicates[1].filePath, equals(expectedPathB)); + }); + + test('HashCacheStorage saves and loads index', () { + final absoluteFilePath = p.normalize(p.join(Directory.current.path, 'file_a.dart')); + final index = { + absoluteFilePath: const FileCacheEntry( + modificationStamp: 123456, + entries: [ + HashEntry( + hash: 123, + lineNumber: 10, + statementCount: 5, + ), + ], + ), + }; + + HashCacheStorage.save(Directory.current.path, index); + + final loaded = HashCacheStorage.load(Directory.current.path); + expect(loaded, isNotNull); + expect(loaded!.length, equals(1)); + expect(loaded[absoluteFilePath]!.entries, hasLength(1)); + expect(loaded[absoluteFilePath]!.modificationStamp, equals(123456)); + + final entry = loaded[absoluteFilePath]!.entries.first; + expect(entry.hash, equals(123)); + expect(entry.lineNumber, equals(10)); + expect(entry.statementCount, equals(5)); + + HashCacheStorage.delete(Directory.current.path); + }); + + test('findCrossFileMatches cleans up absolute paths of deleted files', () { + registry.enablePhysicalFileCleanup = true; + final tempFile = File('${Directory.systemTemp.path}/temp_test_file.dart'); + tempFile.writeAsStringSync('void main() {}'); + + registry.updateFile(tempFile.path, [ + const HashEntry(hash: 123, lineNumber: 10, statementCount: 5), + ], modificationStamp: 1); + expect(registry.fileCount, equals(1)); + + // Delete the file physically + tempFile.deleteSync(); + + // Trigger matching, which should clean up the deleted tempFile from registry + final matches = registry.findCrossFileMatches('other_file.dart', [ + const HashEntry(hash: 123, lineNumber: 10, statementCount: 5), + ]); + + expect(matches, isEmpty); + expect(registry.fileCount, equals(0)); + }); + + test('findCrossFileMatches cleans up absolute paths of excluded files', () { + final absoluteExcludedPath = + p.normalize('/workspace/project/lib/excluded.dart'); + + registry.updateFile(absoluteExcludedPath, [ + const HashEntry(hash: 123, lineNumber: 10, statementCount: 5), + ], modificationStamp: 1); + expect(registry.fileCount, equals(1)); + + // Trigger matching with a callback that considers absoluteExcludedPath as excluded + final matches = registry.findCrossFileMatches( + 'other_file.dart', + [const HashEntry(hash: 123, lineNumber: 10, statementCount: 5)], + isFileExcluded: (path) => path == absoluteExcludedPath, + ); + + expect(matches, isEmpty); + expect(registry.fileCount, equals(0)); + }); + }); +} From f57a2848b18e9a278c1629cafc32894c116e5639 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Fri, 10 Jul 2026 14:09:43 +0300 Subject: [PATCH 04/28] chore: remove avoid_using_api from analysis exclude list --- lib/analysis_options.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/analysis_options.yaml b/lib/analysis_options.yaml index 1a86a8c6..ac94649a 100644 --- a/lib/analysis_options.yaml +++ b/lib/analysis_options.yaml @@ -6,8 +6,6 @@ analyzer: - "**/*.g.dart" - "**/*.gr.dart" - - "src/lints/avoid_using_api/**" - # Flutter - "lib/generated_plugin_registrant.dart" From 6c3900b44aad635f4b823d7b234a0a85fd962ab8 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Mon, 13 Jul 2026 10:52:03 +0300 Subject: [PATCH 05/28] refactor: introduce SolidDiagnosticMessage to encapsulate diagnostic reporting and improve code formatting --- .../models/file_cache_entry.dart | 14 +++-- .../services/global_hash_registry.dart | 12 ++-- .../services/hash_cache_storage.dart | 3 +- .../avoid_duplicate_code_visitor.dart | 58 ++++++++++++------- lib/src/models/solid_diagnostic_message.dart | 32 ++++++++++ 5 files changed, 86 insertions(+), 33 deletions(-) create mode 100644 lib/src/models/solid_diagnostic_message.dart diff --git a/lib/src/lints/avoid_duplicate_code/models/file_cache_entry.dart b/lib/src/lints/avoid_duplicate_code/models/file_cache_entry.dart index 23677fa6..65f21cc2 100644 --- a/lib/src/lints/avoid_duplicate_code/models/file_cache_entry.dart +++ b/lib/src/lints/avoid_duplicate_code/models/file_cache_entry.dart @@ -1,6 +1,7 @@ import 'package:solid_lints/src/lints/avoid_duplicate_code/models/hash_entry.dart'; -/// A cache entry containing the file's modification stamp and its structural hashes. +/// A cache entry containing the file's modification stamp and its structural +/// hashes. class FileCacheEntry { /// The file modification stamp. final int modificationStamp; @@ -16,15 +17,16 @@ class FileCacheEntry { /// Converts this [FileCacheEntry] to a JSON map. Map toJson() => { - 'm': modificationStamp, - 'e': entries.map((e) => e.toJson()).toList(), - }; + 'm': modificationStamp, + 'e': entries.map((e) => e.toJson()).toList(), + }; /// Parses a [FileCacheEntry] from a JSON map. factory FileCacheEntry.fromJson(Map json) { final entriesList = []; - if (json['e'] is List) { - for (final item in json['e'] as List) { + final eValue = json['e']; + if (eValue is List) { + for (final item in eValue) { if (item is Map) { try { entriesList.add(HashEntry.fromJson(item)); diff --git a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart index 4b2b11ea..888842fd 100644 --- a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart +++ b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart @@ -40,7 +40,9 @@ class GlobalHashRegistry { void _addToInvertedIndex(String absoluteFilePath, List entries) { for (final entry in entries) { - _hashToLocations.putIfAbsent(entry.hash, () => {}).add( + _hashToLocations + .putIfAbsent(entry.hash, () => {}) + .add( DuplicateLocation( entry: entry, filePath: absoluteFilePath, @@ -92,7 +94,8 @@ class GlobalHashRegistry { } } - /// Returns the cached modification stamp for [filePath], or `null` if not indexed. + /// Returns the cached modification stamp for [filePath], or `null` if no + /// indexed. int? getModificationStamp(String filePath, {String? packageRoot}) { final root = packageRoot ?? Directory.current.path; _ensureLoaded(root); @@ -168,7 +171,8 @@ class GlobalHashRegistry { if (key == absoluteCurrentFilePath) continue; // Check if deleted or excluded on demand only for matched locations - final isDeleted = enablePhysicalFileCleanup && !File(key).existsSync(); + final isDeleted = + enablePhysicalFileCleanup && !File(key).existsSync(); final isExcluded = isFileExcluded != null && isFileExcluded(key); if (isDeleted || isExcluded) { @@ -258,7 +262,7 @@ class GlobalHashRegistry { // Filter _index for files belonging to this packageRoot final subset = { for (final MapEntry(:key, :value) in _index.entries) - if (key.startsWith(packageRoot)) key: value + if (key.startsWith(packageRoot)) key: value, }; HashCacheStorage.save(packageRoot, subset); diff --git a/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart b/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart index 114bdcb6..e1a62544 100644 --- a/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart +++ b/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart @@ -63,7 +63,8 @@ class HashCacheStorage { '$packageRoot/.dart_tool/solid_lints/duplicate_index_load.log', ); final logLine = - '${time.toIso8601String()}: Loaded map in ${elapsedMs}ms (files: ${filesCount >= 0 ? filesCount : 'failed'})\n'; + '${time.toIso8601String()}: Loaded map in ${elapsedMs}ms ' + '(files: ${filesCount >= 0 ? filesCount : 'failed'})\n'; logFile.writeAsStringSync(logLine, mode: FileMode.append); } catch (_) { // Fail silently diff --git a/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart index cc31ce60..d4eaa53f 100644 --- a/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart +++ b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart @@ -4,7 +4,6 @@ import 'package:analyzer/dart/analysis/context_root.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/diagnostic/diagnostic.dart'; -import 'package:analyzer/src/diagnostic/diagnostic_message.dart'; import 'package:path/path.dart' as path; import 'package:solid_lints/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart'; @@ -12,6 +11,7 @@ import 'package:solid_lints/src/lints/avoid_duplicate_code/models/cross_file_mat import 'package:solid_lints/src/lints/avoid_duplicate_code/models/hash_entry.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/services/global_hash_registry.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/visitors/ast_structural_hasher.dart'; +import 'package:solid_lints/src/models/solid_diagnostic_message.dart'; /// A record representing a block or expression candidate for clone detection. typedef _BodyCandidate = ({ @@ -49,16 +49,24 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { final filePath = _filePath; - // --- Phase 0: Try to use cached entries if file is unchanged and has no warnings --- + // --- Phase 0: Try to use cached entries if file is unchanged and has no + // warnings if (filePath.isNotEmpty) { final contextRoot = _contextRoot; - final packageRoot = contextRoot?.root.path ?? _findPackageRoot(filePath) ?? ''; + final packageRoot = + contextRoot?.root.path ?? _findPackageRoot(filePath) ?? ''; if (packageRoot.isNotEmpty) { final registry = GlobalHashRegistry.instance; - final cachedStamp = registry.getModificationStamp(filePath, packageRoot: packageRoot); + final cachedStamp = registry.getModificationStamp( + filePath, + packageRoot: packageRoot, + ); if (cachedStamp == _modificationStamp) { - final cachedEntries = registry.getFileEntries(filePath, packageRoot: packageRoot); + final cachedEntries = registry.getFileEntries( + filePath, + packageRoot: packageRoot, + ); if (cachedEntries != null) { // Check if there are any cross-file matches final crossMatches = registry.findCrossFileMatches( @@ -77,7 +85,9 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { for (final entry in cachedEntries) { hashCounts[entry.hash] = (hashCounts[entry.hash] ?? 0) + 1; } - final hasIntraDuplicates = hashCounts.values.any((count) => count > 1); + final hasIntraDuplicates = hashCounts.values.any( + (count) => count > 1, + ); final hasCrossDuplicates = crossMatches.isNotEmpty; if (!hasIntraDuplicates && !hasCrossDuplicates) { @@ -131,7 +141,8 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { final crossFileDuplicatesByHash = >{}; if (filePath.isNotEmpty) { final contextRoot = _contextRoot; - final packageRoot = contextRoot?.root.path ?? _findPackageRoot(filePath) ?? ''; + final packageRoot = + contextRoot?.root.path ?? _findPackageRoot(filePath) ?? ''; final registry = GlobalHashRegistry.instance; final crossMatches = registry.findCrossFileMatches( @@ -158,8 +169,8 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { // --- Phase C: Group candidates by structural hash for intra-file --- final groups = >{}; for (final candidate in candidates) { - final hash = candidateHashes[candidate.node] ?? - hasher.computeHash(candidate.node); + final hash = + candidateHashes[candidate.node] ?? hasher.computeHash(candidate.node); (groups[hash] ??= []).add(candidate); } @@ -173,17 +184,20 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { if (suppressed.contains(candidate.node)) continue; if (reported.contains(candidate.node)) continue; - final hash = candidateHashes[candidate.node] ?? - hasher.computeHash(candidate.node); + final hash = + candidateHashes[candidate.node] ?? hasher.computeHash(candidate.node); final group = groups[hash]; if (group == null) continue; // Filter group to only include non-suppressed candidates - final activeGroup = - group.where((c) => !suppressed.contains(c.node)).toList(); + final activeGroup = group + .where((c) => !suppressed.contains(c.node)) + .toList(); final externalPartners = crossFileDuplicatesByHash[hash] ?? const []; - final internalPartners = activeGroup.where((c) => c != candidate).toList(); + final internalPartners = activeGroup + .where((c) => c != candidate) + .toList(); final hasExternalDuplicates = externalPartners.isNotEmpty; final hasInternalDuplicates = internalPartners.isNotEmpty; @@ -197,21 +211,19 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { final contextMessages = [ // Add external partners for (final dup in externalPartners) - DiagnosticMessageImpl( + SolidDiagnosticMessage( filePath: dup.filePath, offset: dup.entry.offset, length: dup.entry.length, message: 'Duplicate', - url: null, ), // Add internal partners for (final other in internalPartners) - DiagnosticMessageImpl( + SolidDiagnosticMessage( filePath: filePath, offset: other.node.offset, length: other.node.length, message: 'Duplicate', - url: null, ), ]; @@ -321,13 +333,15 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { final relativePath = packageRoot.isNotEmpty && filePath.startsWith(packageRoot) - ? path.relative(filePath, from: packageRoot) - : filePath; + ? path.relative(filePath, from: packageRoot) + : filePath; final ramMb = ProcessInfo.currentRss / (1024 * 1024); final cacheIndicator = cached ? ' [Cached]' : ''; final logLine = - '${startTime.toIso8601String()}: Checked #$_processedFilesCount ($relativePath) ' - 'in ${elapsedMs}ms$cacheIndicator | Candidates: $candidateCount | Warnings: $warningCount | RAM: ${ramMb.toStringAsFixed(1)} MB ' + '${startTime.toIso8601String()}: Checked ' + '#$_processedFilesCount ($relativePath) in ${elapsedMs}ms' + '$cacheIndicator | Candidates: $candidateCount | ' + 'Warnings: $warningCount | RAM: ${ramMb.toStringAsFixed(1)} MB ' '(Total session time: ${_totalDurationMs}ms)\n'; logFile.writeAsStringSync(logLine, mode: FileMode.append); } catch (_) { diff --git a/lib/src/models/solid_diagnostic_message.dart b/lib/src/models/solid_diagnostic_message.dart new file mode 100644 index 00000000..85d2ce09 --- /dev/null +++ b/lib/src/models/solid_diagnostic_message.dart @@ -0,0 +1,32 @@ +import 'package:analyzer/diagnostic/diagnostic.dart'; + +/// A concrete implementation of [DiagnosticMessage] for reporting diagnostics +/// with context messages in solid_lints rules. +class SolidDiagnosticMessage implements DiagnosticMessage { + @override + final String filePath; + + @override + final int length; + + @override + final int offset; + + @override + String? get url => null; + + final String _message; + + /// Creates a new instance of [SolidDiagnosticMessage]. + SolidDiagnosticMessage({ + required this.filePath, + required this.length, + required String message, + required this.offset, + }) : _message = message; + + @override + String messageText({required bool includeUrl}) { + return _message; + } +} From 59e68848fe964c05204b9d2f8701cd791029c5e0 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Mon, 13 Jul 2026 11:46:30 +0300 Subject: [PATCH 06/28] feat: register SolidLintsPlugin in pubspec and remove redundant analyzer plugin configuration --- lib/analysis_options.yaml | 2 -- pubspec.yaml | 5 +++++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/analysis_options.yaml b/lib/analysis_options.yaml index ac94649a..9dbe3b51 100644 --- a/lib/analysis_options.yaml +++ b/lib/analysis_options.yaml @@ -1,6 +1,4 @@ analyzer: - plugins: - - solid_lints exclude: # General generated files - "**/*.g.dart" diff --git a/pubspec.yaml b/pubspec.yaml index 56c55842..65345c99 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -27,3 +27,8 @@ dev_dependencies: args: ^2.6.0 analyzer_testing: ^0.1.9 test_reflective_loader: ^0.3.0 + +plugin: + platforms: + dart: + pluginClass: SolidLintsPlugin \ No newline at end of file From 74b6e93358603f41e52d2fe43bb62b1b1284ccec Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Mon, 13 Jul 2026 17:47:18 +0300 Subject: [PATCH 07/28] refactor: optimize duplicate code detection with Jenkins hashing, structural config validation, and improved caching settings. --- lib/analysis_options.yaml | 12 +- lib/analysis_options_test.yaml | 3 + .../excluded_identifier_parameter.dart | 10 +- .../avoid_duplicate_code_rule.dart | 5 +- .../avoid_duplicate_code_parameters.dart | 77 ++-- .../models/hash_entry.dart | 32 +- .../services/global_hash_registry.dart | 114 ++--- .../services/hash_cache_storage.dart | 89 ++-- .../utils/jenkins_hasher.dart | 35 ++ .../visitors/ast_structural_hasher.dart | 400 +++--------------- .../avoid_duplicate_code_visitor.dart | 216 +++++----- .../avoid_duplicate_code_rule_test.dart | 40 +- .../global_hash_registry_test.dart | 92 ++-- .../utils/jenkins_hasher_test.dart | 72 ++++ 14 files changed, 538 insertions(+), 659 deletions(-) create mode 100644 lib/src/lints/avoid_duplicate_code/utils/jenkins_hasher.dart create mode 100644 test/src/lints/avoid_duplicate_code/utils/jenkins_hasher_test.dart diff --git a/lib/analysis_options.yaml b/lib/analysis_options.yaml index 9dbe3b51..e96a501c 100644 --- a/lib/analysis_options.yaml +++ b/lib/analysis_options.yaml @@ -4,6 +4,9 @@ analyzer: - "**/*.g.dart" - "**/*.gr.dart" + # Exclude unmigrated rules + - "src/lints/avoid_using_api/**" + # Flutter - "lib/generated_plugin_registrant.dart" @@ -37,9 +40,14 @@ solid_lints: diagnostics: avoid_global_state: true avoid_duplicate_code: - avoid_duplicate_code: true - min_statements: 3 + min_tokens: 50 check_blocks: true + exclude: + - method_name: initState + - method_name: dispose + - method_name: didChangeDependencies + - method_name: didUpdateWidget + - method_name: build avoid_late_keyword: allow_initialized: true diff --git a/lib/analysis_options_test.yaml b/lib/analysis_options_test.yaml index 19ddb7ec..10d57108 100644 --- a/lib/analysis_options_test.yaml +++ b/lib/analysis_options_test.yaml @@ -51,3 +51,6 @@ solid_lints: avoid_late_keyword: false # It's acceptable to include stubs or other helper classes into the test file. prefer_match_file_name: false +# Test files inherently contain a lot of duplicated setups, teardowns, and mock structures. +# Running duplicate code detection on them usually produces a lot of noise. + avoid_duplicate_code: false diff --git a/lib/src/common/parameters/excluded_identifier_parameter.dart b/lib/src/common/parameters/excluded_identifier_parameter.dart index f55f9704..839658f2 100644 --- a/lib/src/common/parameters/excluded_identifier_parameter.dart +++ b/lib/src/common/parameters/excluded_identifier_parameter.dart @@ -21,8 +21,16 @@ class ExcludedIdentifierParameter { Map json, ) { return ExcludedIdentifierParameter( - methodName: json['method_name'] as String, + methodName: json['method_name'] as String?, className: json['class_name'] as String?, + declarationName: json['declaration_name'] as String?, ); } + + /// Method to convert parameter to JSON Map. + Map toJson() => { + if (methodName != null) 'method_name': methodName, + if (className != null) 'class_name': className, + if (declarationName != null) 'declaration_name': declarationName, + }; } diff --git a/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart b/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart index 08ce7209..1e83660a 100644 --- a/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart +++ b/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart @@ -10,7 +10,8 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// /// When two or more function/method/constructor bodies have structurally /// identical AST subtrees (Type 2 clones — same structure, variable names -/// may differ), the rule reports all copies except the first occurrence. +/// may differ), the rule reports all copies and provides context messages +/// linking to the other occurrences. /// /// Cross-file detection works on a "best-effort" basis: duplicates are /// detected against files that have already been analyzed in the current @@ -23,7 +24,7 @@ import 'package:solid_lints/src/models/solid_lint_rule.dart'; /// solid_lints: /// diagnostics: /// avoid_duplicate_code: -/// min_statements: 10 +/// min_tokens: 50 /// ignore_literals: false /// ignore_identifiers: true /// check_blocks: false diff --git a/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart index cfc5d623..e771eb79 100644 --- a/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart +++ b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart @@ -1,10 +1,11 @@ +import 'package:solid_lints/src/common/parameters/excluded_identifier_parameter.dart'; import 'package:solid_lints/src/common/parameters/excluded_identifiers_list_parameter.dart'; /// Configuration parameters for the avoid_duplicate_code rule. class AvoidDuplicateCodeParameters { - /// Minimum number of statements in a function body or block to be considered + /// Minimum number of tokens in a function body or block to be considered /// for clone detection. Bodies/blocks shorter than this are ignored. - final int minStatements; + final int minTokens; /// When `true`, literal values (strings, numbers, booleans) are excluded /// from the structural hash, making the check ignore literal differences. @@ -22,11 +23,21 @@ class AvoidDuplicateCodeParameters { /// A list of methods/functions that should be excluded from the lint. final ExcludedIdentifiersListParameter exclude; - static const _defaultMinStatements = 10; + static const _defaultMinTokens = 50; + + static final _defaultExclude = ExcludedIdentifiersListParameter( + exclude: const [ + ExcludedIdentifierParameter(methodName: 'initState'), + ExcludedIdentifierParameter(methodName: 'dispose'), + ExcludedIdentifierParameter(methodName: 'didChangeDependencies'), + ExcludedIdentifierParameter(methodName: 'didUpdateWidget'), + ExcludedIdentifierParameter(methodName: 'build'), + ], + ); /// Constructor for [AvoidDuplicateCodeParameters] model. const AvoidDuplicateCodeParameters({ - required this.minStatements, + required this.minTokens, required this.ignoreLiterals, required this.ignoreIdentifiers, required this.checkBlocks, @@ -34,28 +45,42 @@ class AvoidDuplicateCodeParameters { }); /// Empty [AvoidDuplicateCodeParameters] model with default values. - factory AvoidDuplicateCodeParameters.empty() => - AvoidDuplicateCodeParameters( - minStatements: _defaultMinStatements, - ignoreLiterals: false, - ignoreIdentifiers: true, - checkBlocks: false, - exclude: ExcludedIdentifiersListParameter(exclude: []), - ); + factory AvoidDuplicateCodeParameters.empty() => AvoidDuplicateCodeParameters( + minTokens: _defaultMinTokens, + ignoreLiterals: false, + ignoreIdentifiers: true, + checkBlocks: false, + exclude: _defaultExclude, + ); /// Creates parameters from JSON configuration. - factory AvoidDuplicateCodeParameters.fromJson(Map json) => - AvoidDuplicateCodeParameters( - minStatements: - json['min_statements'] as int? ?? _defaultMinStatements, - ignoreLiterals: - json['ignore_literals'] as bool? ?? - json['ignore_literal_values'] as bool? ?? - false, - ignoreIdentifiers: - json['ignore_identifiers'] as bool? ?? true, - checkBlocks: - json['check_blocks'] as bool? ?? false, - exclude: ExcludedIdentifiersListParameter.defaultFromJson(json), - ); + factory AvoidDuplicateCodeParameters.fromJson(Map json) { + final baseExclude = ExcludedIdentifiersListParameter.defaultFromJson(json); + final combinedExclude = ExcludedIdentifiersListParameter( + exclude: [ + ..._defaultExclude.exclude, + ...baseExclude.exclude, + ], + ); + + return AvoidDuplicateCodeParameters( + minTokens: json['min_tokens'] as int? ?? _defaultMinTokens, + ignoreLiterals: + json['ignore_literals'] as bool? ?? + json['ignore_literal_values'] as bool? ?? + false, + ignoreIdentifiers: json['ignore_identifiers'] as bool? ?? true, + checkBlocks: json['check_blocks'] as bool? ?? false, + exclude: combinedExclude, + ); + } + + /// Converts the parameters to a JSON-compatible Map. + Map toJson() => { + 'min_tokens': minTokens, + 'ignore_literals': ignoreLiterals, + 'ignore_identifiers': ignoreIdentifiers, + 'check_blocks': checkBlocks, + 'exclude': exclude.exclude.map((e) => e.toJson()).toList(), + }; } diff --git a/lib/src/lints/avoid_duplicate_code/models/hash_entry.dart b/lib/src/lints/avoid_duplicate_code/models/hash_entry.dart index a136905c..8f265e14 100644 --- a/lib/src/lints/avoid_duplicate_code/models/hash_entry.dart +++ b/lib/src/lints/avoid_duplicate_code/models/hash_entry.dart @@ -13,33 +13,33 @@ class HashEntry { /// The length of the code block. final int length; - /// The number of statements in the candidate body. - final int statementCount; + /// The number of tokens in the candidate body. + final int tokenCount; /// Creates a new [HashEntry]. const HashEntry({ required this.hash, required this.lineNumber, - required this.statementCount, + required this.tokenCount, this.offset = 0, this.length = 0, }); /// Converts this [HashEntry] to a JSON-compatible map using shortened keys. Map toJson() => { - 'h': hash, - 'n': lineNumber, - 'o': offset, - 'l': length, - 's': statementCount, - }; + 'h': hash, + 'n': lineNumber, + 'o': offset, + 'l': length, + 't': tokenCount, + }; - /// Creates a [HashEntry] from a JSON map, supporting both old and new keys. + /// Creates a [HashEntry] from a JSON map. factory HashEntry.fromJson(Map json) => HashEntry( - hash: (json['h'] ?? json['hash'])! as int, - lineNumber: (json['n'] ?? json['lineNumber'])! as int, - offset: (json['o'] ?? json['offset'] ?? 0) as int, - length: (json['l'] ?? json['length'] ?? 0) as int, - statementCount: (json['s'] ?? json['statementCount'])! as int, - ); + hash: json['h']! as int, + lineNumber: json['n']! as int, + offset: (json['o'] ?? 0) as int, + length: (json['l'] ?? 0) as int, + tokenCount: (json['t'] ?? json['s'] ?? 0) as int, + ); } diff --git a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart index 888842fd..7315e7db 100644 --- a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart +++ b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart @@ -1,7 +1,7 @@ import 'dart:async'; -import 'dart:convert'; import 'dart:io'; import 'package:path/path.dart' as p; +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/cross_file_match.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/file_cache_entry.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/hash_entry.dart'; @@ -29,6 +29,7 @@ class GlobalHashRegistry { final _loadedRoots = {}; Timer? _saveTimer; + AvoidDuplicateCodeParameters? _currentParams; /// Whether to automatically clean up files that do not exist physically on /// disk. @@ -66,10 +67,15 @@ class GlobalHashRegistry { } } - void _ensureLoaded(String packageRoot) { + void _ensureLoaded( + String packageRoot, [ + AvoidDuplicateCodeParameters? currentParams, + ]) { if (_loadedRoots.contains(packageRoot)) return; _loadedRoots.add(packageRoot); - final cached = HashCacheStorage.load(packageRoot); + final params = currentParams ?? AvoidDuplicateCodeParameters.empty(); + _currentParams = params; + final cached = HashCacheStorage.load(packageRoot, params); if (cached != null) { _index.addAll(cached); @@ -89,16 +95,20 @@ class GlobalHashRegistry { } if (deletedFiles.isNotEmpty) { - _scheduleSave(packageRoot); + _scheduleSave(packageRoot, params); } } } /// Returns the cached modification stamp for [filePath], or `null` if no /// indexed. - int? getModificationStamp(String filePath, {String? packageRoot}) { + int? getModificationStamp( + String filePath, { + AvoidDuplicateCodeParameters? parameters, + String? packageRoot, + }) { final root = packageRoot ?? Directory.current.path; - _ensureLoaded(root); + _ensureLoaded(root, parameters); final absoluteFilePath = p.isAbsolute(filePath) ? p.normalize(filePath) : p.normalize(p.join(root, filePath)); @@ -106,9 +116,13 @@ class GlobalHashRegistry { } /// Returns the cached entries for [filePath], or `null` if not indexed. - List? getFileEntries(String filePath, {String? packageRoot}) { + List? getFileEntries( + String filePath, { + AvoidDuplicateCodeParameters? parameters, + String? packageRoot, + }) { final root = packageRoot ?? Directory.current.path; - _ensureLoaded(root); + _ensureLoaded(root, parameters); final absoluteFilePath = p.isAbsolute(filePath) ? p.normalize(filePath) : p.normalize(p.join(root, filePath)); @@ -120,10 +134,11 @@ class GlobalHashRegistry { String filePath, List entries, { required int modificationStamp, + AvoidDuplicateCodeParameters? parameters, String? packageRoot, }) { final root = packageRoot ?? Directory.current.path; - _ensureLoaded(root); + _ensureLoaded(root, parameters); final absoluteFilePath = p.isAbsolute(filePath) ? p.normalize(filePath) : p.normalize(p.join(root, filePath)); @@ -138,7 +153,7 @@ class GlobalHashRegistry { entries: entries, ); _addToInvertedIndex(absoluteFilePath, entries); - _scheduleSave(root); + _scheduleSave(root, parameters); } /// Finds cross-file duplicates for [currentEntries] against all other @@ -148,11 +163,12 @@ class GlobalHashRegistry { List findCrossFileMatches( String currentFilePath, List currentEntries, { + AvoidDuplicateCodeParameters? parameters, bool Function(String filePath)? isFileExcluded, String? packageRoot, }) { final root = packageRoot ?? Directory.current.path; - _ensureLoaded(root); + _ensureLoaded(root, parameters); final absoluteCurrentFilePath = p.isAbsolute(currentFilePath) ? p.normalize(currentFilePath) @@ -208,16 +224,20 @@ class GlobalHashRegistry { } _index.remove(file); } - _scheduleSave(root); + _scheduleSave(root, parameters); } return matches; } /// Removes [filePath] from the index. - void removeFile(String filePath, {String? packageRoot}) { + void removeFile( + String filePath, { + AvoidDuplicateCodeParameters? parameters, + String? packageRoot, + }) { final root = packageRoot ?? Directory.current.path; - _ensureLoaded(root); + _ensureLoaded(root, parameters); final absoluteFilePath = p.isAbsolute(filePath) ? p.normalize(filePath) : p.normalize(p.join(root, filePath)); @@ -226,7 +246,7 @@ class GlobalHashRegistry { _removeFromInvertedIndex(absoluteFilePath, oldEntry.entries); } _index.remove(absoluteFilePath); - _scheduleSave(root); + _scheduleSave(root, parameters); } /// The number of files currently indexed. @@ -241,70 +261,30 @@ class GlobalHashRegistry { _index.clear(); _hashToLocations.clear(); HashCacheStorage.delete(Directory.current.path); - try { - final file = File( - '${Directory.current.path}/.dart_tool/solid_lints/duplicate_inverted_index.json', - ); - if (file.existsSync()) { - file.deleteSync(); - } - } catch (_) {} } - void _scheduleSave(String packageRoot) { + void _scheduleSave( + String packageRoot, [ + AvoidDuplicateCodeParameters? parameters, + ]) { _saveTimer?.cancel(); _saveTimer = Timer(const Duration(milliseconds: 500), () { - _performSave(packageRoot); + _performSave(packageRoot, parameters); }); } - void _performSave(String packageRoot) { + void _performSave( + String packageRoot, [ + AvoidDuplicateCodeParameters? parameters, + ]) { // Filter _index for files belonging to this packageRoot final subset = { for (final MapEntry(:key, :value) in _index.entries) if (key.startsWith(packageRoot)) key: value, }; - HashCacheStorage.save(packageRoot, subset); - - // Save inverted index to duplicate_inverted_index.json - try { - final jsonInverted = >>{}; - _hashToLocations.forEach((hash, locations) { - final packageLocations = locations - .where((loc) => loc.filePath.startsWith(packageRoot)) - .toList(); - if (packageLocations.isEmpty) return; - - jsonInverted[hash.toString()] = packageLocations.map((loc) { - final relativePath = p.isAbsolute(loc.filePath) - ? p.relative(loc.filePath, from: packageRoot) - : loc.filePath; - return { - 'filePath': relativePath, - 'entry': loc.entry.toJson(), - }; - }).toList(); - }); - - final file = File( - '$packageRoot/.dart_tool/solid_lints/duplicate_inverted_index.json', - ); - if (jsonInverted.isEmpty) { - if (file.existsSync()) { - file.deleteSync(); - } - } else { - final directory = file.parent; - if (!directory.existsSync()) { - directory.createSync(recursive: true); - } - file.writeAsStringSync( - const JsonEncoder.withIndent(' ').convert(jsonInverted), - ); - } - } catch (_) { - // Fail silently to avoid breaking analysis server - } + final params = + parameters ?? _currentParams ?? AvoidDuplicateCodeParameters.empty(); + HashCacheStorage.save(packageRoot, subset, params); } } diff --git a/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart b/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart index e1a62544..e914f72e 100644 --- a/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart +++ b/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart @@ -1,6 +1,8 @@ import 'dart:convert'; import 'dart:io'; +import 'package:collection/collection.dart'; import 'package:path/path.dart' as p; +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/file_cache_entry.dart'; /// Service responsible for persisting and loading the duplicate code hash @@ -13,9 +15,10 @@ class HashCacheStorage { /// /// Returns `null` if the cache file does not exist, is corrupted, or fails /// to load. - static Map? load(String packageRoot) { - final stopwatch = Stopwatch()..start(); - final startTime = DateTime.now(); + static Map? load( + String packageRoot, + AvoidDuplicateCodeParameters currentParams, + ) { try { final file = File(_filePath(packageRoot)); if (!file.existsSync()) return null; @@ -23,8 +26,25 @@ class HashCacheStorage { final content = file.readAsStringSync(); final jsonMap = jsonDecode(content) as Map; + // Validate config + final cachedConfig = jsonMap['config']; + if (cachedConfig is! Map) { + return null; + } + + final currentConfig = currentParams.toJson(); + if (!const DeepCollectionEquality().equals(cachedConfig, currentConfig)) { + // Configuration mismatch -> discard cache + return null; + } + + final filesMap = jsonMap['files']; + if (filesMap is! Map) { + return null; + } + final result = {}; - for (final MapEntry(:key, :value) in jsonMap.entries) { + for (final MapEntry(:key, :value) in filesMap.entries) { final absoluteKey = p.isAbsolute(key) ? p.normalize(key) : p.normalize(p.join(packageRoot, key)); @@ -37,42 +57,18 @@ class HashCacheStorage { } } } - stopwatch.stop(); - _writeLoadLog( - packageRoot, - startTime, - stopwatch.elapsedMilliseconds, - result.length, - ); return result; } catch (_) { - stopwatch.stop(); - _writeLoadLog(packageRoot, startTime, stopwatch.elapsedMilliseconds, -1); return null; } } - static void _writeLoadLog( + /// Saves the [index] to disk for the given [packageRoot]. + static void save( String packageRoot, - DateTime time, - int elapsedMs, - int filesCount, + Map index, + AvoidDuplicateCodeParameters currentParams, ) { - try { - final logFile = File( - '$packageRoot/.dart_tool/solid_lints/duplicate_index_load.log', - ); - final logLine = - '${time.toIso8601String()}: Loaded map in ${elapsedMs}ms ' - '(files: ${filesCount >= 0 ? filesCount : 'failed'})\n'; - logFile.writeAsStringSync(logLine, mode: FileMode.append); - } catch (_) { - // Fail silently - } - } - - /// Saves the [index] to disk for the given [packageRoot]. - static void save(String packageRoot, Map index) { try { final cacheDir = '$packageRoot/.dart_tool/solid_lints'; final directory = Directory(cacheDir); @@ -81,29 +77,22 @@ class HashCacheStorage { } final file = File(_filePath(packageRoot)); - if (index.isEmpty) { - file.writeAsStringSync('{}'); - return; - } - final buffer = StringBuffer('{\n'); - final entriesList = index.entries.toList(); - for (var i = 0; i < entriesList.length; i++) { - final entry = entriesList[i]; + final filesMap = {}; + for (final entry in index.entries) { final relativeKey = p.isAbsolute(entry.key) ? p.relative(entry.key, from: packageRoot) : entry.key; - final listJson = jsonEncode(entry.value.toJson()); - - buffer.write(' ${jsonEncode(relativeKey)}: $listJson'); - if (i < entriesList.length - 1) { - buffer.write(',\n'); - } else { - buffer.write('\n'); - } + filesMap[relativeKey] = entry.value.toJson(); } - buffer.write('}'); - file.writeAsStringSync(buffer.toString()); + + final data = { + 'version': 1, + 'config': currentParams.toJson(), + 'files': filesMap, + }; + + file.writeAsStringSync(jsonEncode(data)); } catch (_) { // Fail silently to avoid breaking analysis server } diff --git a/lib/src/lints/avoid_duplicate_code/utils/jenkins_hasher.dart b/lib/src/lints/avoid_duplicate_code/utils/jenkins_hasher.dart new file mode 100644 index 00000000..89d9b1a4 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/utils/jenkins_hasher.dart @@ -0,0 +1,35 @@ +/// A custom implementation of the Jenkins One-at-a-Time hash algorithm. +/// +/// This incrementally computes a hash without allocating large strings, +/// significantly reducing GC pressure during tree traversal. +class JenkinsHasher { + int _hash = 0; + + /// Resets the hash to its initial state. + void reset() { + _hash = 0; + } + + /// Adds a single 32-bit integer [value] to the hash. + void add(int value) { + _hash = 0x1fffffff & (_hash + value); + _hash = 0x1fffffff & (_hash + ((0x0007ffff & _hash) << 10)); + _hash = _hash ^ (_hash >> 6); + } + + /// Adds all code units of [value] to the hash. + void addString(String value) { + for (var i = 0; i < value.length; i++) { + add(value.codeUnitAt(i)); + } + } + + /// Finalizes and returns the computed hash. + int get hash { + var h = _hash; + h = 0x1fffffff & (h + ((0x03ffffff & h) << 3)); + h = h ^ (h >> 11); + h = 0x1fffffff & (h + ((0x00003fff & h) << 15)); + return h; + } +} diff --git a/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hasher.dart b/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hasher.dart index 4c10eecd..11199ad6 100644 --- a/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hasher.dart +++ b/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hasher.dart @@ -1,82 +1,55 @@ import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/element.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/utils/jenkins_hasher.dart'; -/// A [RecursiveAstVisitor] that builds a structural fingerprint of an AST +/// A [UnifyingAstVisitor] that builds a structural fingerprint of an AST /// subtree for clone detection. /// /// The fingerprint captures the structure of the code (node types, operators, -/// and optionally literal values) while ignoring identifier names, whitespace, -/// and comments. This enables Type 2 clone detection where two code blocks -/// with identical structure but different variable names are considered clones. -class AstStructuralHasher extends RecursiveAstVisitor { +/// and optionally literal values) while ignoring identifier names (like local +/// variables), whitespace, and comments. This enables Type 2 clone detection +/// where two code blocks with identical structure but different variable names +/// are considered clones. +class AstStructuralHasher extends UnifyingAstVisitor { final bool _ignoreLiterals; final bool _ignoreIdentifiers; - final StringBuffer _buffer = StringBuffer(); + final JenkinsHasher _hasher = JenkinsHasher(); + final Map _localVariableIds = {}; /// Creates a new [AstStructuralHasher]. AstStructuralHasher({ required bool ignoreLiterals, required bool ignoreIdentifiers, - }) : _ignoreLiterals = ignoreLiterals, - _ignoreIdentifiers = ignoreIdentifiers; + }) : _ignoreLiterals = ignoreLiterals, + _ignoreIdentifiers = ignoreIdentifiers; /// Computes the structural hash for the given [node]. /// /// Visits the entire subtree of [node] and returns an integer hash /// of the accumulated structural fingerprint. int computeHash(AstNode node) { - _buffer.clear(); + _hasher.reset(); + _localVariableIds.clear(); node.accept(this); - return _buffer.toString().hashCode; + return _hasher.hash; } - /// Returns the raw structural fingerprint string for the given [node]. - /// - /// Useful for debugging and testing. - String computeFingerprint(AstNode node) { - _buffer.clear(); - node.accept(this); - return _buffer.toString(); - } - - // --- Structure nodes --- - - @override - void visitBlock(Block node) { - _append('Block'); - super.visitBlock(node); - } - - @override - void visitBlockFunctionBody(BlockFunctionBody node) { - _append('BlockFunctionBody'); - super.visitBlockFunctionBody(node); - } - - @override - void visitExpressionFunctionBody(ExpressionFunctionBody node) { - _append('ExpressionFunctionBody'); - super.visitExpressionFunctionBody(node); - } - - // --- Statements --- - - @override - void visitExpressionStatement(ExpressionStatement node) { - _append('ExpressionStatement'); - super.visitExpressionStatement(node); + void _append(String value) { + _hasher.addString(value); + _hasher.add(0x7C); // ASCII code for '|' } @override - void visitReturnStatement(ReturnStatement node) { - _append('ReturnStatement'); - super.visitReturnStatement(node); + void visitNode(AstNode node) { + _append(node.runtimeType.toString()); + node.visitChildren(this); + _append('^'); } @override void visitVariableDeclarationStatement(VariableDeclarationStatement node) { - _append('VariableDeclarationStatement'); - // Include keyword (final/var/const) final keyword = node.variables.keyword; if (keyword != null) { _append(keyword.lexeme); @@ -86,252 +59,61 @@ class AstStructuralHasher extends RecursiveAstVisitor { @override void visitIfStatement(IfStatement node) { - _append('IfStatement'); _append(node.elseKeyword != null ? 'withElse' : 'noElse'); super.visitIfStatement(node); } - @override - void visitForStatement(ForStatement node) { - _append('ForStatement'); - super.visitForStatement(node); - } - - @override - void visitForEachPartsWithDeclaration(ForEachPartsWithDeclaration node) { - _append('ForEachPartsWithDeclaration'); - super.visitForEachPartsWithDeclaration(node); - } - - @override - void visitForPartsWithDeclarations(ForPartsWithDeclarations node) { - _append('ForPartsWithDeclarations'); - super.visitForPartsWithDeclarations(node); - } - - @override - void visitWhileStatement(WhileStatement node) { - _append('WhileStatement'); - super.visitWhileStatement(node); - } - - @override - void visitDoStatement(DoStatement node) { - _append('DoStatement'); - super.visitDoStatement(node); - } - - @override - void visitSwitchStatement(SwitchStatement node) { - _append('SwitchStatement'); - super.visitSwitchStatement(node); - } - - @override - void visitSwitchCase(SwitchCase node) { - _append('SwitchCase'); - super.visitSwitchCase(node); - } - - @override - void visitSwitchDefault(SwitchDefault node) { - _append('SwitchDefault'); - super.visitSwitchDefault(node); - } - - @override - void visitSwitchExpression(SwitchExpression node) { - _append('SwitchExpression'); - super.visitSwitchExpression(node); - } - - @override - void visitSwitchExpressionCase(SwitchExpressionCase node) { - _append('SwitchExpressionCase'); - super.visitSwitchExpressionCase(node); - } - @override void visitTryStatement(TryStatement node) { - _append('TryStatement'); _append(node.finallyBlock != null ? 'withFinally' : 'noFinally'); super.visitTryStatement(node); } - @override - void visitCatchClause(CatchClause node) { - _append('CatchClause'); - super.visitCatchClause(node); - } - - @override - void visitThrowExpression(ThrowExpression node) { - _append('ThrowExpression'); - super.visitThrowExpression(node); - } - - @override - void visitBreakStatement(BreakStatement node) { - _append('BreakStatement'); - super.visitBreakStatement(node); - } - - @override - void visitContinueStatement(ContinueStatement node) { - _append('ContinueStatement'); - super.visitContinueStatement(node); - } - - @override - void visitAssertStatement(AssertStatement node) { - _append('AssertStatement'); - super.visitAssertStatement(node); - } - @override void visitYieldStatement(YieldStatement node) { - _append('YieldStatement'); _append(node.star != null ? 'star' : 'noStar'); super.visitYieldStatement(node); } - // --- Expressions --- - @override void visitBinaryExpression(BinaryExpression node) { - _append('BinaryExpression'); _append(node.operator.type.toString()); super.visitBinaryExpression(node); } @override void visitPrefixExpression(PrefixExpression node) { - _append('PrefixExpression'); + if (_ignoreLiterals && + (node.operator.type == TokenType.MINUS || + node.operator.type == TokenType.PLUS) && + (node.operand is IntegerLiteral || node.operand is DoubleLiteral)) { + node.operand.accept(this); + return; + } _append(node.operator.type.toString()); super.visitPrefixExpression(node); } @override void visitPostfixExpression(PostfixExpression node) { - _append('PostfixExpression'); _append(node.operator.type.toString()); super.visitPostfixExpression(node); } @override void visitAssignmentExpression(AssignmentExpression node) { - _append('AssignmentExpression'); _append(node.operator.type.toString()); super.visitAssignmentExpression(node); } - @override - void visitConditionalExpression(ConditionalExpression node) { - _append('ConditionalExpression'); - super.visitConditionalExpression(node); - } - - @override - void visitMethodInvocation(MethodInvocation node) { - _append('MethodInvocation'); - // Include method name as it's part of the API being called, - // not a local identifier - _append(node.methodName.name); - super.visitMethodInvocation(node); - } - - @override - void visitFunctionExpressionInvocation(FunctionExpressionInvocation node) { - _append('FunctionExpressionInvocation'); - super.visitFunctionExpressionInvocation(node); - } - - @override - void visitInstanceCreationExpression(InstanceCreationExpression node) { - _append('InstanceCreationExpression'); - _append(node.constructorName.type.name.lexeme); - super.visitInstanceCreationExpression(node); - } - - @override - void visitPropertyAccess(PropertyAccess node) { - _append('PropertyAccess'); - _append(node.propertyName.name); - super.visitPropertyAccess(node); - } - - @override - void visitPrefixedIdentifier(PrefixedIdentifier node) { - _append('PrefixedIdentifier'); - // Include the property name, but not the prefix (which is a local variable) - _append(node.identifier.name); - super.visitPrefixedIdentifier(node); - } - - @override - void visitIndexExpression(IndexExpression node) { - _append('IndexExpression'); - super.visitIndexExpression(node); - } - - @override - void visitCascadeExpression(CascadeExpression node) { - _append('CascadeExpression'); - super.visitCascadeExpression(node); - } - - @override - void visitAwaitExpression(AwaitExpression node) { - _append('AwaitExpression'); - super.visitAwaitExpression(node); - } - - @override - void visitAsExpression(AsExpression node) { - _append('AsExpression'); - super.visitAsExpression(node); - } - @override void visitIsExpression(IsExpression node) { - _append('IsExpression'); _append(node.notOperator != null ? 'not' : 'is'); super.visitIsExpression(node); } - @override - void visitParenthesizedExpression(ParenthesizedExpression node) { - _append('ParenthesizedExpression'); - super.visitParenthesizedExpression(node); - } - - @override - void visitListLiteral(ListLiteral node) { - _append('ListLiteral'); - super.visitListLiteral(node); - } - - @override - void visitSetOrMapLiteral(SetOrMapLiteral node) { - _append('SetOrMapLiteral'); - super.visitSetOrMapLiteral(node); - } - - @override - void visitMapLiteralEntry(MapLiteralEntry node) { - _append('MapLiteralEntry'); - super.visitMapLiteralEntry(node); - } - - @override - void visitSpreadElement(SpreadElement node) { - _append('SpreadElement'); - super.visitSpreadElement(node); - } - @override void visitNamedExpression(NamedExpression node) { - _append('NamedExpression'); _append(node.name.label.name); super.visitNamedExpression(node); } @@ -340,133 +122,81 @@ class AstStructuralHasher extends RecursiveAstVisitor { @override void visitIntegerLiteral(IntegerLiteral node) { - _append('IntegerLiteral'); if (!_ignoreLiterals) { _append(node.literal.lexeme); } + super.visitIntegerLiteral(node); } @override void visitDoubleLiteral(DoubleLiteral node) { - _append('DoubleLiteral'); if (!_ignoreLiterals) { _append(node.literal.lexeme); } + super.visitDoubleLiteral(node); } @override void visitSimpleStringLiteral(SimpleStringLiteral node) { - _append('StringLiteral'); if (!_ignoreLiterals) { _append(node.value); } + super.visitSimpleStringLiteral(node); } @override - void visitStringInterpolation(StringInterpolation node) { - _append('StringInterpolation'); - super.visitStringInterpolation(node); + void visitInterpolationString(InterpolationString node) { + if (!_ignoreLiterals) { + _append(node.value); + } + super.visitInterpolationString(node); } @override void visitBooleanLiteral(BooleanLiteral node) { - _append('BooleanLiteral'); if (!_ignoreLiterals) { _append(node.value.toString()); } + super.visitBooleanLiteral(node); } - @override - void visitNullLiteral(NullLiteral node) { - _append('NullLiteral'); - } - - // --- Identifiers (ignored for Type 2 clone detection) --- + // --- Identifiers --- @override void visitSimpleIdentifier(SimpleIdentifier node) { - _append('Identifier'); if (!_ignoreIdentifiers) { _append(node.name); + } else { + // Smart ignore: ignore ONLY local variables and parameters. + // This preserves field names, getters, methods, class names, etc. + final element = node.element; + if (element != null) { + if (element is! LocalVariableElement && + element is! FormalParameterElement) { + _append(node.name); + } else { + // It is a local variable or parameter. + // Assign it a local ID (De Bruijn Indexing) to distinguish clones + // that wire their variables differently. + final id = _localVariableIds.putIfAbsent( + element, + () => _localVariableIds.length, + ); + _append('VAR_$id'); + } + } else { + // If element is null, we are not sure what it is. + // It could be an unresolved local variable or an unresolved property. + // We ignore it to preserve standard Type-2 clone behavior (less false + // negatives). + } } + super.visitSimpleIdentifier(node); } - // --- Type annotations --- - @override void visitNamedType(NamedType node) { - _append('NamedType'); _append(node.name.lexeme); super.visitNamedType(node); } - - @override - void visitGenericFunctionType(GenericFunctionType node) { - _append('GenericFunctionType'); - super.visitGenericFunctionType(node); - } - - // --- Variable declarations --- - - @override - void visitVariableDeclaration(VariableDeclaration node) { - _append('VariableDeclaration'); - super.visitVariableDeclaration(node); - } - - @override - void visitVariableDeclarationList(VariableDeclarationList node) { - _append('VariableDeclarationList'); - super.visitVariableDeclarationList(node); - } - - // --- Arguments and parameters --- - - @override - void visitArgumentList(ArgumentList node) { - _append('ArgumentList'); - _append(node.arguments.length.toString()); - super.visitArgumentList(node); - } - - @override - void visitFormalParameterList(FormalParameterList node) { - _append('FormalParameterList'); - super.visitFormalParameterList(node); - } - - // --- Function expressions (closures) --- - - @override - void visitFunctionExpression(FunctionExpression node) { - _append('FunctionExpression'); - super.visitFunctionExpression(node); - } - - // --- Type arguments --- - - @override - void visitTypeArgumentList(TypeArgumentList node) { - _append('TypeArgumentList'); - super.visitTypeArgumentList(node); - } - - // --- Null-aware operators --- - - @override - void visitNullCheckPattern(NullCheckPattern node) { - _append('NullCheckPattern'); - super.visitNullCheckPattern(node); - } - - @override - void visitNullAssertPattern(NullAssertPattern node) { - _append('NullAssertPattern'); - super.visitNullAssertPattern(node); - } - - void _append(String value) { - _buffer.write(value); - _buffer.write('|'); - } } diff --git a/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart index d4eaa53f..295f2ce0 100644 --- a/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart +++ b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart @@ -22,9 +22,6 @@ typedef _BodyCandidate = ({ /// A visitor that detects duplicate code blocks (at the function level and/or /// statement block level) within a single compilation unit and across files. class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { - static int _processedFilesCount = 0; - static int _totalDurationMs = 0; - final AvoidDuplicateCodeRule _rule; final AvoidDuplicateCodeParameters _parameters; final String _filePath; @@ -44,9 +41,6 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { @override void visitCompilationUnit(CompilationUnit node) { - final stopwatch = Stopwatch()..start(); - final startTime = DateTime.now(); - final filePath = _filePath; // --- Phase 0: Try to use cached entries if file is unchanged and has no @@ -59,12 +53,14 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { final registry = GlobalHashRegistry.instance; final cachedStamp = registry.getModificationStamp( filePath, + parameters: _parameters, packageRoot: packageRoot, ); if (cachedStamp == _modificationStamp) { final cachedEntries = registry.getFileEntries( filePath, + parameters: _parameters, packageRoot: packageRoot, ); if (cachedEntries != null) { @@ -72,6 +68,7 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { final crossMatches = registry.findCrossFileMatches( filePath, cachedEntries, + parameters: _parameters, packageRoot: packageRoot, isFileExcluded: (otherPath) { if (contextRoot == null) return false; @@ -81,29 +78,93 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { ); // Check if there are any intra-file duplicates - final hashCounts = {}; + final hashGroups = >{}; for (final entry in cachedEntries) { - hashCounts[entry.hash] = (hashCounts[entry.hash] ?? 0) + 1; + (hashGroups[entry.hash] ??= []).add(entry); } - final hasIntraDuplicates = hashCounts.values.any( - (count) => count > 1, + + final hasIntraDuplicates = hashGroups.values.any( + (group) => group.length > 1, ); final hasCrossDuplicates = crossMatches.isNotEmpty; if (!hasIntraDuplicates && !hasCrossDuplicates) { - // 0 warnings and file is unchanged -> Skip entire AST traversal! - stopwatch.stop(); - _writeRunLog( - packageRoot, - startTime, - stopwatch.elapsedMilliseconds, - filePath, - cachedEntries.length, - 0, - cached: true, - ); return; } + + // We have warnings, but we can report them directly from the cache! + + final suppressedRanges = <(int offset, int length)>[]; + final reportedOffsets = {}; + + final crossFileDuplicatesByHash = >{}; + for (final match in crossMatches) { + crossFileDuplicatesByHash[match.currentEntry.hash] = + match.duplicates; + } + + for (final entry in cachedEntries) { + // Check if suppressed + final isSuppressed = suppressedRanges.any( + (range) => + entry.offset >= range.$1 && + (entry.offset + entry.length) <= (range.$1 + range.$2), + ); + if (isSuppressed) continue; + + // Also check if we already reported this exact entry + if (reportedOffsets.contains(entry.offset)) continue; + + final group = hashGroups[entry.hash]; + if (group == null) continue; + + final activeGroup = group.where((e) { + return !suppressedRanges.any( + (range) => + e.offset >= range.$1 && + (e.offset + e.length) <= (range.$1 + range.$2), + ); + }).toList(); + + final externalPartners = + crossFileDuplicatesByHash[entry.hash] ?? const []; + final internalPartners = activeGroup + .where((e) => e != entry) + .toList(); + + final shouldReport = + internalPartners.isNotEmpty || externalPartners.isNotEmpty; + + if (shouldReport) { + final contextMessages = [ + for (final dup in externalPartners) + SolidDiagnosticMessage( + filePath: dup.filePath, + offset: dup.entry.offset, + length: dup.entry.length, + message: 'Duplicate', + ), + for (final other in internalPartners) + SolidDiagnosticMessage( + filePath: filePath, + offset: other.offset, + length: other.length, + message: 'Duplicate', + ), + ]; + + _rule.reportAtOffset( + entry.offset, + entry.length, + contextMessages: contextMessages, + ); + + reportedOffsets.add(entry.offset); + suppressedRanges.add((entry.offset, entry.length)); + } + } + + return; } } } @@ -132,7 +193,7 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { lineNumber: line, offset: candidate.node.offset, length: candidate.node.length, - statementCount: _statementCount(candidate.node), + tokenCount: _tokenCount(candidate.node), ), ); } @@ -148,6 +209,7 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { final crossMatches = registry.findCrossFileMatches( filePath, hashEntries, + parameters: _parameters, packageRoot: packageRoot, isFileExcluded: (otherPath) { if (contextRoot == null) return false; @@ -162,6 +224,7 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { filePath, hashEntries, modificationStamp: _modificationStamp, + parameters: _parameters, packageRoot: packageRoot, ); } @@ -176,7 +239,6 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { final suppressed = {}; final reported = {}; - var warningCount = 0; // Candidates are collected pre-order (parent before children). // Process larger scopes first, and suppress warnings on children. @@ -206,8 +268,6 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { final shouldReport = hasInternalDuplicates || hasExternalDuplicates; if (shouldReport) { - final reportNode = _findReportNode(candidate.node); - final contextMessages = [ // Add external partners for (final dup in externalPartners) @@ -228,10 +288,9 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { ]; _rule.reportAtNode( - reportNode, + candidate.node, contextMessages: contextMessages, ); - warningCount++; // Mark this duplicate block as reported. reported.add(candidate.node); @@ -239,59 +298,24 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { _suppressDescendants(candidate.node, suppressed); } } - - stopwatch.stop(); - if (filePath.isNotEmpty) { - final contextRoot = _contextRoot; - final packageRoot = - contextRoot?.root.path ?? _findPackageRoot(filePath) ?? ''; - if (packageRoot.isNotEmpty) { - _writeRunLog( - packageRoot, - startTime, - stopwatch.elapsedMilliseconds, - filePath, - candidates.length, - warningCount, - cached: false, - ); - } - } } - /// Returns the number of statements in a body node. - static int _statementCount(AstNode node) { - if (node is Block) return node.statements.length; - if (node is ExpressionFunctionBody) return 1; - return 0; + /// Returns the number of tokens in a body node. + static int _tokenCount(AstNode node) { + int count = 0; + var token = node.beginToken; + final end = node.endToken; + while (token != end) { + count++; + token = token.next!; + } + return count + 1; } void _suppressDescendants(AstNode root, Set suppressed) { root.accept(_DescendantCollector(suppressed, root)); } - AstNode _findReportNode(AstNode node) { - final parent = node.parent; - if (parent is FunctionBody) { - final declaration = parent.parent; - if (declaration != null) { - if (declaration is FunctionDeclaration || - declaration is MethodDeclaration || - declaration is ConstructorDeclaration) { - return declaration; - } - if (declaration is FunctionExpression) { - final grandDeclaration = declaration.parent; - if (grandDeclaration is FunctionDeclaration) { - return grandDeclaration; - } - return declaration; - } - } - } - return node; - } - String? _findPackageRoot(String filePath) { if (filePath.isEmpty) return null; var dir = File(filePath).parent; @@ -308,46 +332,6 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { } return null; } - - void _writeRunLog( - String packageRoot, - DateTime startTime, - int elapsedMs, - String filePath, - int candidateCount, - int warningCount, { - required bool cached, - }) { - try { - _processedFilesCount++; - _totalDurationMs += elapsedMs; - final logFile = File( - '$packageRoot/.dart_tool/solid_lints/avoid_duplicate_code_run.log', - ); - - // Ensure the directory exists - final directory = logFile.parent; - if (!directory.existsSync()) { - directory.createSync(recursive: true); - } - - final relativePath = - packageRoot.isNotEmpty && filePath.startsWith(packageRoot) - ? path.relative(filePath, from: packageRoot) - : filePath; - final ramMb = ProcessInfo.currentRss / (1024 * 1024); - final cacheIndicator = cached ? ' [Cached]' : ''; - final logLine = - '${startTime.toIso8601String()}: Checked ' - '#$_processedFilesCount ($relativePath) in ${elapsedMs}ms' - '$cacheIndicator | Candidates: $candidateCount | ' - 'Warnings: $warningCount | RAM: ${ramMb.toStringAsFixed(1)} MB ' - '(Total session time: ${_totalDurationMs}ms)\n'; - logFile.writeAsStringSync(logLine, mode: FileMode.append); - } catch (_) { - // Fail silently - } - } } /// A visitor that collects descendant candidate blocks to suppress them. @@ -389,18 +373,18 @@ class _CandidateCollector extends RecursiveAstVisitor { return; } - _checkAndCollect(node, node.statements.length); + _checkAndCollect(node, AvoidDuplicateCodeVisitor._tokenCount(node)); super.visitBlock(node); } @override void visitExpressionFunctionBody(ExpressionFunctionBody node) { - _checkAndCollect(node, 1); + _checkAndCollect(node, AvoidDuplicateCodeVisitor._tokenCount(node)); super.visitExpressionFunctionBody(node); } - void _checkAndCollect(AstNode node, int statementCount) { - if (statementCount < parameters.minStatements) return; + void _checkAndCollect(AstNode node, int tokenCount) { + if (tokenCount < parameters.minTokens) return; final declaration = node.thisOrAncestorOfType(); if (declaration != null && parameters.exclude.shouldIgnore(declaration)) { diff --git a/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart b/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart index 18d7dc51..72ad70e7 100644 --- a/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart +++ b/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart @@ -24,7 +24,7 @@ plugins: solid_lints: diagnostics: avoid_duplicate_code: - min_statements: 3 + min_tokens: 15 check_blocks: true exclude: - method_name: excluded @@ -54,12 +54,12 @@ $_mockAnalysisOptionsContent''', await super.tearDown(); } - // --- Base Tests (min_statements: 3, check_blocks: true, default) --- + // --- Base Tests (min_tokens: 15, check_blocks: true, default) --- Future test_reports_when_two_functions_have_identical_bodies() async { await assertAutoDiagnostics(''' -${expectLint(r'''void first() { +void first() ${expectLint(r'''{ final x = 1; if (x > 0) { print(x); @@ -67,7 +67,7 @@ ${expectLint(r'''void first() { print('done'); }''')} -${expectLint(r'''void second() { +void second() ${expectLint(r'''{ final x = 1; if (x > 0) { print(x); @@ -80,7 +80,7 @@ ${expectLint(r'''void second() { Future test_reports_when_functions_have_same_structure_different_names() async { await assertAutoDiagnostics(''' -${expectLint(r'''void fetchUsers() { +void fetchUsers() ${expectLint(r'''{ final response = 'data'; if (response.isEmpty) { throw Exception('error'); @@ -88,7 +88,7 @@ ${expectLint(r'''void fetchUsers() { print(response); }''')} -${expectLint(r'''void fetchOrders() { +void fetchOrders() ${expectLint(r'''{ final result = 'data'; if (result.isEmpty) { throw Exception('error'); @@ -120,7 +120,7 @@ void second() { } Future - test_does_not_report_when_body_below_min_statements() async { + test_does_not_report_when_body_below_min_tokens() async { await assertNoDiagnostics(r''' void first() { print('hello'); @@ -138,7 +138,7 @@ void second() { test_reports_on_methods_in_same_class() async { await assertAutoDiagnostics(''' class MyClass { - ${expectLint(r'''void first() { + void first() ${expectLint(r'''{ final x = 1; if (x > 0) { print(x); @@ -146,7 +146,7 @@ class MyClass { print('done'); }''')} - ${expectLint(r'''void second() { + void second() ${expectLint(r'''{ final y = 1; if (y > 0) { print(y); @@ -160,7 +160,7 @@ class MyClass { Future test_reports_on_third_clone_also() async { await assertAutoDiagnostics(''' -${expectLint(r'''void first() { +void first() ${expectLint(r'''{ final x = 1; if (x > 0) { print(x); @@ -168,7 +168,7 @@ ${expectLint(r'''void first() { print('done'); }''')} -${expectLint(r'''void second() { +void second() ${expectLint(r'''{ final y = 1; if (y > 0) { print(y); @@ -176,7 +176,7 @@ ${expectLint(r'''void second() { print('done'); }''')} -${expectLint(r'''void third() { +void third() ${expectLint(r'''{ final z = 1; if (z > 0) { print(z); @@ -197,12 +197,12 @@ plugins: solid_lints: diagnostics: avoid_duplicate_code: - min_statements: 3 + min_tokens: 15 ignore_literals: true ''', ); await assertAutoDiagnostics(''' -${expectLint(r'''void first() { +void first() ${expectLint(r'''{ final x = 1; if (x > 0) { print('hello'); @@ -210,7 +210,7 @@ ${expectLint(r'''void first() { print('world'); }''')} -${expectLint(r'''void second() { +void second() ${expectLint(r'''{ final y = 2; if (y > 0) { print('foo'); @@ -252,7 +252,7 @@ plugins: solid_lints: diagnostics: avoid_duplicate_code: - min_statements: 3 + min_tokens: 15 ignore_identifiers: false ''', ); @@ -326,7 +326,7 @@ void two() { Future test_reports_parent_but_not_nested_blocks_when_parent_is_reported() async { await assertAutoDiagnostics(''' -${expectLint(r'''void one() { +void one() ${expectLint(r'''{ final x = 1; if (x > 0) { print('hello'); @@ -336,7 +336,7 @@ ${expectLint(r'''void one() { print('done'); }''')} -${expectLint(r'''void two() { +void two() ${expectLint(r'''{ final y = 1; if (y > 0) { print('hello'); @@ -363,7 +363,7 @@ void otherMethod() { final visitor = AvoidDuplicateCodeVisitor( rule as AvoidDuplicateCodeRule, AvoidDuplicateCodeParameters( - minStatements: 3, + minTokens: 15, ignoreLiterals: false, ignoreIdentifiers: true, checkBlocks: true, @@ -375,7 +375,7 @@ void otherMethod() { resolvedOther.unit.accept(visitor); await assertAutoDiagnostics(''' -${expectLint(r'''void mainMethod() { +void mainMethod() ${expectLint(r'''{ final x = 1; if (x > 0) { print(x); diff --git a/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart b/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart index 59484ecf..27b93030 100644 --- a/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart +++ b/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart @@ -1,5 +1,6 @@ import 'dart:io'; import 'package:path/path.dart' as p; +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/file_cache_entry.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/hash_entry.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/services/global_hash_registry.dart'; @@ -23,8 +24,8 @@ void main() { test('updateFile stores entries', () { final entries = [ - const HashEntry(hash: 123, lineNumber: 10, statementCount: 5), - const HashEntry(hash: 456, lineNumber: 20, statementCount: 3), + const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), + const HashEntry(hash: 456, lineNumber: 20, tokenCount: 3), ]; registry.updateFile('file_a.dart', entries, modificationStamp: 1); @@ -34,10 +35,10 @@ void main() { test('findCrossFileMatches finds duplicate in other files', () { final fileAEntries = [ - const HashEntry(hash: 123, lineNumber: 10, statementCount: 5), + const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), ]; final fileBEntries = [ - const HashEntry(hash: 123, lineNumber: 15, statementCount: 5), + const HashEntry(hash: 123, lineNumber: 15, tokenCount: 5), ]; registry.updateFile('file_a.dart', fileAEntries, modificationStamp: 1); @@ -54,7 +55,7 @@ void main() { test('findCrossFileMatches ignores same file', () { final entries = [ - const HashEntry(hash: 123, lineNumber: 10, statementCount: 5), + const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), ]; registry.updateFile('file_a.dart', entries, modificationStamp: 1); @@ -66,10 +67,10 @@ void main() { test('updateFile replaces previous entries', () { final oldEntries = [ - const HashEntry(hash: 123, lineNumber: 10, statementCount: 5), + const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), ]; final newEntries = [ - const HashEntry(hash: 456, lineNumber: 20, statementCount: 3), + const HashEntry(hash: 456, lineNumber: 20, tokenCount: 3), ]; registry.updateFile('file_a.dart', oldEntries, modificationStamp: 1); @@ -79,23 +80,23 @@ void main() { // File B tries to match against the old hash 123, should find nothing final matches1 = registry.findCrossFileMatches('file_b.dart', [ - const HashEntry(hash: 123, lineNumber: 15, statementCount: 5), + const HashEntry(hash: 123, lineNumber: 15, tokenCount: 5), ]); expect(matches1, isEmpty); // File B tries to match against the new hash 456, should match final matches2 = registry.findCrossFileMatches('file_b.dart', [ - const HashEntry(hash: 456, lineNumber: 25, statementCount: 3), + const HashEntry(hash: 456, lineNumber: 25, tokenCount: 3), ]); expect(matches2, hasLength(1)); }); test('removeFile clears entries for specific file', () { registry.updateFile('file_a.dart', [ - const HashEntry(hash: 123, lineNumber: 10, statementCount: 5), + const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), ], modificationStamp: 1); registry.updateFile('file_b.dart', [ - const HashEntry(hash: 456, lineNumber: 20, statementCount: 5), + const HashEntry(hash: 456, lineNumber: 20, tokenCount: 5), ], modificationStamp: 1); expect(registry.fileCount, equals(2)); @@ -105,14 +106,14 @@ void main() { expect(registry.fileCount, equals(1)); final matches = registry.findCrossFileMatches('file_c.dart', [ - const HashEntry(hash: 123, lineNumber: 30, statementCount: 5), + const HashEntry(hash: 123, lineNumber: 30, tokenCount: 5), ]); expect(matches, isEmpty); }); test('clear empties the registry', () { registry.updateFile('file_a.dart', [ - const HashEntry(hash: 123, lineNumber: 10, statementCount: 5), + const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), ], modificationStamp: 1); expect(registry.fileCount, equals(1)); @@ -123,14 +124,14 @@ void main() { test('findCrossFileMatches groups multiple duplicate locations', () { registry.updateFile('file_a.dart', [ - const HashEntry(hash: 123, lineNumber: 10, statementCount: 5), + const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), ], modificationStamp: 1); registry.updateFile('file_b.dart', [ - const HashEntry(hash: 123, lineNumber: 20, statementCount: 5), + const HashEntry(hash: 123, lineNumber: 20, tokenCount: 5), ], modificationStamp: 1); final matches = registry.findCrossFileMatches('file_c.dart', [ - const HashEntry(hash: 123, lineNumber: 30, statementCount: 5), + const HashEntry(hash: 123, lineNumber: 30, tokenCount: 5), ]); expect(matches, hasLength(1)); @@ -150,15 +151,22 @@ void main() { HashEntry( hash: 123, lineNumber: 10, - statementCount: 5, + tokenCount: 5, ), ], ), }; - HashCacheStorage.save(Directory.current.path, index); + HashCacheStorage.save( + Directory.current.path, + index, + AvoidDuplicateCodeParameters.empty(), + ); - final loaded = HashCacheStorage.load(Directory.current.path); + final loaded = HashCacheStorage.load( + Directory.current.path, + AvoidDuplicateCodeParameters.empty(), + ); expect(loaded, isNotNull); expect(loaded!.length, equals(1)); expect(loaded[absoluteFilePath]!.entries, hasLength(1)); @@ -167,7 +175,7 @@ void main() { final entry = loaded[absoluteFilePath]!.entries.first; expect(entry.hash, equals(123)); expect(entry.lineNumber, equals(10)); - expect(entry.statementCount, equals(5)); + expect(entry.tokenCount, equals(5)); HashCacheStorage.delete(Directory.current.path); }); @@ -178,7 +186,7 @@ void main() { tempFile.writeAsStringSync('void main() {}'); registry.updateFile(tempFile.path, [ - const HashEntry(hash: 123, lineNumber: 10, statementCount: 5), + const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), ], modificationStamp: 1); expect(registry.fileCount, equals(1)); @@ -187,7 +195,7 @@ void main() { // Trigger matching, which should clean up the deleted tempFile from registry final matches = registry.findCrossFileMatches('other_file.dart', [ - const HashEntry(hash: 123, lineNumber: 10, statementCount: 5), + const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), ]); expect(matches, isEmpty); @@ -199,19 +207,55 @@ void main() { p.normalize('/workspace/project/lib/excluded.dart'); registry.updateFile(absoluteExcludedPath, [ - const HashEntry(hash: 123, lineNumber: 10, statementCount: 5), + const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), ], modificationStamp: 1); expect(registry.fileCount, equals(1)); // Trigger matching with a callback that considers absoluteExcludedPath as excluded final matches = registry.findCrossFileMatches( 'other_file.dart', - [const HashEntry(hash: 123, lineNumber: 10, statementCount: 5)], + [const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5)], isFileExcluded: (path) => path == absoluteExcludedPath, ); expect(matches, isEmpty); expect(registry.fileCount, equals(0)); }); + + test('HashCacheStorage invalidates cache on config change', () { + final absoluteFilePath = p.normalize( + p.join(Directory.current.path, 'file.dart'), + ); + final index = { + absoluteFilePath: FileCacheEntry( + modificationStamp: 123456, + entries: const [ + HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), + ], + ), + }; + + final params1 = AvoidDuplicateCodeParameters.empty(); + final params2 = AvoidDuplicateCodeParameters( + minTokens: 5, + ignoreLiterals: true, + ignoreIdentifiers: false, + checkBlocks: true, + exclude: params1.exclude, + ); + + // Save with params1 + HashCacheStorage.save(Directory.current.path, index, params1); + + // Loading with params1 should succeed + final loaded1 = HashCacheStorage.load(Directory.current.path, params1); + expect(loaded1, isNotNull); + + // Loading with params2 (different config) should return null (invalidated) + final loaded2 = HashCacheStorage.load(Directory.current.path, params2); + expect(loaded2, isNull); + + HashCacheStorage.delete(Directory.current.path); + }); }); } diff --git a/test/src/lints/avoid_duplicate_code/utils/jenkins_hasher_test.dart b/test/src/lints/avoid_duplicate_code/utils/jenkins_hasher_test.dart new file mode 100644 index 00000000..a2055031 --- /dev/null +++ b/test/src/lints/avoid_duplicate_code/utils/jenkins_hasher_test.dart @@ -0,0 +1,72 @@ +import 'package:solid_lints/src/lints/avoid_duplicate_code/utils/jenkins_hasher.dart'; +import 'package:test/test.dart'; + +void main() { + group('JenkinsHasher', () { + late JenkinsHasher hasher; + + setUp(() { + hasher = JenkinsHasher(); + }); + + test('initial hash is 0', () { + expect(hasher.hash, 0); + }); + + test('addString updates hash consistently', () { + hasher.addString('hello'); + final hash1 = hasher.hash; + + hasher.reset(); + hasher.addString('hello'); + final hash2 = hasher.hash; + + expect(hash1, hash2); + expect(hash1, isNot(0)); + }); + + test('different strings produce different hashes', () { + hasher.addString('hello'); + final hash1 = hasher.hash; + + hasher.reset(); + hasher.addString('world'); + final hash2 = hasher.hash; + + expect(hash1, isNot(hash2)); + }); + + test('addString is equivalent to adding characters sequentially', () { + hasher.addString('hello'); + final combinedHash = hasher.hash; + + hasher.reset(); + hasher.addString('he'); + hasher.addString('llo'); + final sequentialHash = hasher.hash; + + expect(combinedHash, sequentialHash); + }); + + test('add updates hash correctly', () { + hasher.add(1); + hasher.add(2); + final hash1 = hasher.hash; + + hasher.reset(); + hasher.add(1); + hasher.add(2); + final hash2 = hasher.hash; + + expect(hash1, hash2); + }); + + test('reset clears the state', () { + hasher.addString('some data'); + expect(hasher.hash, isNot(0)); + + hasher.reset(); + expect(hasher.hash, 0); + }); + }); +} From 94b1d74d598bf79beb60c2474a29bfaf274fff16 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Mon, 13 Jul 2026 18:55:08 +0300 Subject: [PATCH 08/28] refactor: simplify avoid_duplicate_code parameters by removing default method exclusions and cleaning up JSON parsing logic --- .../avoid_duplicate_code_parameters.dart | 24 +++---------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart index e771eb79..b99af47b 100644 --- a/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart +++ b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart @@ -1,4 +1,3 @@ -import 'package:solid_lints/src/common/parameters/excluded_identifier_parameter.dart'; import 'package:solid_lints/src/common/parameters/excluded_identifiers_list_parameter.dart'; /// Configuration parameters for the avoid_duplicate_code rule. @@ -26,13 +25,7 @@ class AvoidDuplicateCodeParameters { static const _defaultMinTokens = 50; static final _defaultExclude = ExcludedIdentifiersListParameter( - exclude: const [ - ExcludedIdentifierParameter(methodName: 'initState'), - ExcludedIdentifierParameter(methodName: 'dispose'), - ExcludedIdentifierParameter(methodName: 'didChangeDependencies'), - ExcludedIdentifierParameter(methodName: 'didUpdateWidget'), - ExcludedIdentifierParameter(methodName: 'build'), - ], + exclude: const [], ); /// Constructor for [AvoidDuplicateCodeParameters] model. @@ -55,23 +48,12 @@ class AvoidDuplicateCodeParameters { /// Creates parameters from JSON configuration. factory AvoidDuplicateCodeParameters.fromJson(Map json) { - final baseExclude = ExcludedIdentifiersListParameter.defaultFromJson(json); - final combinedExclude = ExcludedIdentifiersListParameter( - exclude: [ - ..._defaultExclude.exclude, - ...baseExclude.exclude, - ], - ); - return AvoidDuplicateCodeParameters( minTokens: json['min_tokens'] as int? ?? _defaultMinTokens, - ignoreLiterals: - json['ignore_literals'] as bool? ?? - json['ignore_literal_values'] as bool? ?? - false, + ignoreLiterals: json['ignore_literals'] as bool? ?? false, ignoreIdentifiers: json['ignore_identifiers'] as bool? ?? true, checkBlocks: json['check_blocks'] as bool? ?? false, - exclude: combinedExclude, + exclude: ExcludedIdentifiersListParameter.defaultFromJson(json), ); } From f4918d321af96b28475c39ba255ad68916ea81bd Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Tue, 14 Jul 2026 10:57:13 +0300 Subject: [PATCH 09/28] refactor: simplify duplicate code detection logic by extracting visitor helper methods and improving cache handling --- lib/analysis_options.yaml | 5 +- .../avoid_duplicate_code_parameters.dart | 9 +- .../services/global_hash_registry.dart | 63 +-- .../services/hash_cache_storage.dart | 14 +- .../visitors/ast_structural_hasher.dart | 27 +- .../avoid_duplicate_code_visitor.dart | 376 +++++++++--------- 6 files changed, 264 insertions(+), 230 deletions(-) diff --git a/lib/analysis_options.yaml b/lib/analysis_options.yaml index e96a501c..8ff26b0e 100644 --- a/lib/analysis_options.yaml +++ b/lib/analysis_options.yaml @@ -40,14 +40,11 @@ solid_lints: diagnostics: avoid_global_state: true avoid_duplicate_code: - min_tokens: 50 + min_tokens: 30 check_blocks: true exclude: - method_name: initState - method_name: dispose - - method_name: didChangeDependencies - - method_name: didUpdateWidget - - method_name: build avoid_late_keyword: allow_initialized: true diff --git a/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart index b99af47b..754075dc 100644 --- a/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart +++ b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart @@ -10,9 +10,10 @@ class AvoidDuplicateCodeParameters { /// from the structural hash, making the check ignore literal differences. final bool ignoreLiterals; - /// When `true`, variable and method names (identifiers) are excluded + /// When `true`, local variable and parameter names are excluded /// from the structural hash, allowing detection of renamed variables - /// (Type 2). + /// (Type 2). Note that method, class, and field names are NOT ignored + /// to prevent excessive false positives. final bool ignoreIdentifiers; /// When `true`, statement blocks (like if-blocks or loops) inside functions @@ -42,7 +43,7 @@ class AvoidDuplicateCodeParameters { minTokens: _defaultMinTokens, ignoreLiterals: false, ignoreIdentifiers: true, - checkBlocks: false, + checkBlocks: true, exclude: _defaultExclude, ); @@ -52,7 +53,7 @@ class AvoidDuplicateCodeParameters { minTokens: json['min_tokens'] as int? ?? _defaultMinTokens, ignoreLiterals: json['ignore_literals'] as bool? ?? false, ignoreIdentifiers: json['ignore_identifiers'] as bool? ?? true, - checkBlocks: json['check_blocks'] as bool? ?? false, + checkBlocks: json['check_blocks'] as bool? ?? true, exclude: ExcludedIdentifiersListParameter.defaultFromJson(json), ); } diff --git a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart index 7315e7db..a46f3647 100644 --- a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart +++ b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart @@ -27,8 +27,8 @@ class GlobalHashRegistry { final _hashToLocations = >{}; final _loadedRoots = {}; + final _saveDebouncer = _Debouncer(const Duration(milliseconds: 500)); - Timer? _saveTimer; AvoidDuplicateCodeParameters? _currentParams; /// Whether to automatically clean up files that do not exist physically on @@ -39,6 +39,12 @@ class GlobalHashRegistry { GlobalHashRegistry._(); + String _normalizePath(String filePath, String root) { + return p.isAbsolute(filePath) + ? p.normalize(filePath) + : p.normalize(p.join(root, filePath)); + } + void _addToInvertedIndex(String absoluteFilePath, List entries) { for (final entry in entries) { _hashToLocations @@ -109,9 +115,7 @@ class GlobalHashRegistry { }) { final root = packageRoot ?? Directory.current.path; _ensureLoaded(root, parameters); - final absoluteFilePath = p.isAbsolute(filePath) - ? p.normalize(filePath) - : p.normalize(p.join(root, filePath)); + final absoluteFilePath = _normalizePath(filePath, root); return _index[absoluteFilePath]?.modificationStamp; } @@ -123,9 +127,7 @@ class GlobalHashRegistry { }) { final root = packageRoot ?? Directory.current.path; _ensureLoaded(root, parameters); - final absoluteFilePath = p.isAbsolute(filePath) - ? p.normalize(filePath) - : p.normalize(p.join(root, filePath)); + final absoluteFilePath = _normalizePath(filePath, root); return _index[absoluteFilePath]?.entries; } @@ -139,9 +141,7 @@ class GlobalHashRegistry { }) { final root = packageRoot ?? Directory.current.path; _ensureLoaded(root, parameters); - final absoluteFilePath = p.isAbsolute(filePath) - ? p.normalize(filePath) - : p.normalize(p.join(root, filePath)); + final absoluteFilePath = _normalizePath(filePath, root); final oldEntry = _index[absoluteFilePath]; if (oldEntry != null) { @@ -170,12 +170,11 @@ class GlobalHashRegistry { final root = packageRoot ?? Directory.current.path; _ensureLoaded(root, parameters); - final absoluteCurrentFilePath = p.isAbsolute(currentFilePath) - ? p.normalize(currentFilePath) - : p.normalize(p.join(root, currentFilePath)); + final absoluteCurrentFilePath = _normalizePath(currentFilePath, root); final matches = []; final filesToRemove = {}; + final fileStatusCache = {}; for (final entry in currentEntries) { final duplicates = []; @@ -187,11 +186,16 @@ class GlobalHashRegistry { if (key == absoluteCurrentFilePath) continue; // Check if deleted or excluded on demand only for matched locations - final isDeleted = - enablePhysicalFileCleanup && !File(key).existsSync(); - final isExcluded = isFileExcluded != null && isFileExcluded(key); + bool? isInvalid = fileStatusCache[key]; + if (isInvalid == null) { + final isDeleted = + enablePhysicalFileCleanup && !File(key).existsSync(); + final isExcluded = isFileExcluded != null && isFileExcluded(key); + isInvalid = isDeleted || isExcluded; + fileStatusCache[key] = isInvalid; + } - if (isDeleted || isExcluded) { + if (isInvalid) { filesToRemove.add(key); continue; } @@ -238,9 +242,7 @@ class GlobalHashRegistry { }) { final root = packageRoot ?? Directory.current.path; _ensureLoaded(root, parameters); - final absoluteFilePath = p.isAbsolute(filePath) - ? p.normalize(filePath) - : p.normalize(p.join(root, filePath)); + final absoluteFilePath = _normalizePath(filePath, root); final oldEntry = _index[absoluteFilePath]; if (oldEntry != null) { _removeFromInvertedIndex(absoluteFilePath, oldEntry.entries); @@ -256,7 +258,7 @@ class GlobalHashRegistry { /// /// Primarily used in tests to ensure test isolation. void clear() { - _saveTimer?.cancel(); + _saveDebouncer.cancel(); _loadedRoots.clear(); _index.clear(); _hashToLocations.clear(); @@ -267,8 +269,7 @@ class GlobalHashRegistry { String packageRoot, [ AvoidDuplicateCodeParameters? parameters, ]) { - _saveTimer?.cancel(); - _saveTimer = Timer(const Duration(milliseconds: 500), () { + _saveDebouncer.run(() { _performSave(packageRoot, parameters); }); } @@ -288,3 +289,19 @@ class GlobalHashRegistry { HashCacheStorage.save(packageRoot, subset, params); } } + +class _Debouncer { + final Duration delay; + Timer? _timer; + + _Debouncer(this.delay); + + void run(void Function() action) { + _timer?.cancel(); + _timer = Timer(delay, action); + } + + void cancel() { + _timer?.cancel(); + } +} diff --git a/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart b/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart index e914f72e..e27f9315 100644 --- a/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart +++ b/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart @@ -8,8 +8,11 @@ import 'package:solid_lints/src/lints/avoid_duplicate_code/models/file_cache_ent /// Service responsible for persisting and loading the duplicate code hash /// cache. class HashCacheStorage { + static const _cacheDirName = '.dart_tool/solid_lints'; + static const _cacheFileName = 'duplicate_index.json'; + static String _filePath(String packageRoot) => - '$packageRoot/.dart_tool/solid_lints/duplicate_index.json'; + p.join(packageRoot, _cacheDirName, _cacheFileName); /// Loads the cached index from disk for the given [packageRoot]. /// @@ -58,7 +61,7 @@ class HashCacheStorage { } } return result; - } catch (_) { + } on Exception { return null; } } @@ -70,8 +73,7 @@ class HashCacheStorage { AvoidDuplicateCodeParameters currentParams, ) { try { - final cacheDir = '$packageRoot/.dart_tool/solid_lints'; - final directory = Directory(cacheDir); + final directory = Directory(p.join(packageRoot, _cacheDirName)); if (!directory.existsSync()) { directory.createSync(recursive: true); } @@ -93,7 +95,7 @@ class HashCacheStorage { }; file.writeAsStringSync(jsonEncode(data)); - } catch (_) { + } on FileSystemException { // Fail silently to avoid breaking analysis server } } @@ -105,7 +107,7 @@ class HashCacheStorage { if (file.existsSync()) { file.deleteSync(); } - } catch (_) { + } on FileSystemException { // Fail silently } } diff --git a/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hasher.dart b/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hasher.dart index 11199ad6..66424670 100644 --- a/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hasher.dart +++ b/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hasher.dart @@ -41,9 +41,14 @@ class AstStructuralHasher extends UnifyingAstVisitor { _hasher.add(0x7C); // ASCII code for '|' } + void _appendHash(int hashCode) { + _hasher.add(hashCode); + _hasher.add(0x7C); // ASCII code for '|' + } + @override void visitNode(AstNode node) { - _append(node.runtimeType.toString()); + _appendHash(node.runtimeType.hashCode); node.visitChildren(this); _append('^'); } @@ -59,25 +64,25 @@ class AstStructuralHasher extends UnifyingAstVisitor { @override void visitIfStatement(IfStatement node) { - _append(node.elseKeyword != null ? 'withElse' : 'noElse'); + _appendHash(node.elseKeyword != null ? 1 : 0); super.visitIfStatement(node); } @override void visitTryStatement(TryStatement node) { - _append(node.finallyBlock != null ? 'withFinally' : 'noFinally'); + _appendHash(node.finallyBlock != null ? 1 : 0); super.visitTryStatement(node); } @override void visitYieldStatement(YieldStatement node) { - _append(node.star != null ? 'star' : 'noStar'); + _appendHash(node.star != null ? 1 : 0); super.visitYieldStatement(node); } @override void visitBinaryExpression(BinaryExpression node) { - _append(node.operator.type.toString()); + _appendHash(node.operator.type.hashCode); super.visitBinaryExpression(node); } @@ -90,25 +95,25 @@ class AstStructuralHasher extends UnifyingAstVisitor { node.operand.accept(this); return; } - _append(node.operator.type.toString()); + _appendHash(node.operator.type.hashCode); super.visitPrefixExpression(node); } @override void visitPostfixExpression(PostfixExpression node) { - _append(node.operator.type.toString()); + _appendHash(node.operator.type.hashCode); super.visitPostfixExpression(node); } @override void visitAssignmentExpression(AssignmentExpression node) { - _append(node.operator.type.toString()); + _appendHash(node.operator.type.hashCode); super.visitAssignmentExpression(node); } @override void visitIsExpression(IsExpression node) { - _append(node.notOperator != null ? 'not' : 'is'); + _appendHash(node.notOperator != null ? 1 : 0); super.visitIsExpression(node); } @@ -155,7 +160,7 @@ class AstStructuralHasher extends UnifyingAstVisitor { @override void visitBooleanLiteral(BooleanLiteral node) { if (!_ignoreLiterals) { - _append(node.value.toString()); + _appendHash(node.value ? 1 : 0); } super.visitBooleanLiteral(node); } @@ -182,7 +187,7 @@ class AstStructuralHasher extends UnifyingAstVisitor { element, () => _localVariableIds.length, ); - _append('VAR_$id'); + _appendHash(id); } } else { // If element is null, we are not sure what it is. diff --git a/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart index 295f2ce0..c6ab0c74 100644 --- a/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart +++ b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart @@ -13,6 +13,14 @@ import 'package:solid_lints/src/lints/avoid_duplicate_code/services/global_hash_ import 'package:solid_lints/src/lints/avoid_duplicate_code/visitors/ast_structural_hasher.dart'; import 'package:solid_lints/src/models/solid_diagnostic_message.dart'; +extension _RangeExtension on (int, int) { + bool isStrictlyWithin((int, int) parent) { + return this.$1 >= parent.$1 && + (this.$1 + this.$2) <= (parent.$1 + parent.$2) && + !(this.$1 == parent.$1 && this.$2 == parent.$2); + } +} + /// A record representing a block or expression candidate for clone detection. typedef _BodyCandidate = ({ AstNode node, @@ -42,144 +50,22 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { @override void visitCompilationUnit(CompilationUnit node) { final filePath = _filePath; + if (filePath.isEmpty) return; - // --- Phase 0: Try to use cached entries if file is unchanged and has no - // warnings - if (filePath.isNotEmpty) { - final contextRoot = _contextRoot; - final packageRoot = - contextRoot?.root.path ?? _findPackageRoot(filePath) ?? ''; - if (packageRoot.isNotEmpty) { - final registry = GlobalHashRegistry.instance; - final cachedStamp = registry.getModificationStamp( - filePath, - parameters: _parameters, - packageRoot: packageRoot, - ); + final packageRoot = + _contextRoot?.root.path ?? _findPackageRoot(filePath) ?? ''; - if (cachedStamp == _modificationStamp) { - final cachedEntries = registry.getFileEntries( - filePath, - parameters: _parameters, - packageRoot: packageRoot, - ); - if (cachedEntries != null) { - // Check if there are any cross-file matches - final crossMatches = registry.findCrossFileMatches( - filePath, - cachedEntries, - parameters: _parameters, - packageRoot: packageRoot, - isFileExcluded: (otherPath) { - if (contextRoot == null) return false; - final isWithin = otherPath.startsWith(contextRoot.root.path); - return isWithin && !contextRoot.isAnalyzed(otherPath); - }, - ); - - // Check if there are any intra-file duplicates - final hashGroups = >{}; - for (final entry in cachedEntries) { - (hashGroups[entry.hash] ??= []).add(entry); - } - - final hasIntraDuplicates = hashGroups.values.any( - (group) => group.length > 1, - ); - final hasCrossDuplicates = crossMatches.isNotEmpty; - - if (!hasIntraDuplicates && !hasCrossDuplicates) { - return; - } - - // We have warnings, but we can report them directly from the cache! - - final suppressedRanges = <(int offset, int length)>[]; - final reportedOffsets = {}; - - final crossFileDuplicatesByHash = >{}; - for (final match in crossMatches) { - crossFileDuplicatesByHash[match.currentEntry.hash] = - match.duplicates; - } - - for (final entry in cachedEntries) { - // Check if suppressed - final isSuppressed = suppressedRanges.any( - (range) => - entry.offset >= range.$1 && - (entry.offset + entry.length) <= (range.$1 + range.$2), - ); - if (isSuppressed) continue; - - // Also check if we already reported this exact entry - if (reportedOffsets.contains(entry.offset)) continue; - - final group = hashGroups[entry.hash]; - if (group == null) continue; - - final activeGroup = group.where((e) { - return !suppressedRanges.any( - (range) => - e.offset >= range.$1 && - (e.offset + e.length) <= (range.$1 + range.$2), - ); - }).toList(); - - final externalPartners = - crossFileDuplicatesByHash[entry.hash] ?? const []; - final internalPartners = activeGroup - .where((e) => e != entry) - .toList(); - - final shouldReport = - internalPartners.isNotEmpty || externalPartners.isNotEmpty; - - if (shouldReport) { - final contextMessages = [ - for (final dup in externalPartners) - SolidDiagnosticMessage( - filePath: dup.filePath, - offset: dup.entry.offset, - length: dup.entry.length, - message: 'Duplicate', - ), - for (final other in internalPartners) - SolidDiagnosticMessage( - filePath: filePath, - offset: other.offset, - length: other.length, - message: 'Duplicate', - ), - ]; - - _rule.reportAtOffset( - entry.offset, - entry.length, - contextMessages: contextMessages, - ); - - reportedOffsets.add(entry.offset); - suppressedRanges.add((entry.offset, entry.length)); - } - } - - return; - } - } - } + if (_tryReportFromCache(filePath, packageRoot)) { + return; } - final collector = _CandidateCollector(_parameters); - node.accept(collector); - final candidates = collector.candidates; + final candidates = _collectCandidates(node); final hasher = AstStructuralHasher( ignoreLiterals: _parameters.ignoreLiterals, ignoreIdentifiers: _parameters.ignoreIdentifiers, ); - // --- Phase A: Build hash entries for cross-file registry --- final hashEntries = []; final candidateHashes = {}; @@ -198,38 +84,173 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { ); } - // --- Phase B: Find Cross-file matches from registry --- + final crossFileDuplicatesByHash = _findAndSaveCrossFileMatches( + filePath, + hashEntries, + packageRoot, + ); + + _groupAndReportDuplicates( + filePath, + candidates, + candidateHashes, + hasher, + crossFileDuplicatesByHash, + ); + } + + bool _tryReportFromCache(String filePath, String packageRoot) { + if (packageRoot.isEmpty) return false; + + final registry = GlobalHashRegistry.instance; + final cachedStamp = registry.getModificationStamp( + filePath, + parameters: _parameters, + packageRoot: packageRoot, + ); + + if (cachedStamp != _modificationStamp) return false; + + final cachedEntries = registry.getFileEntries( + filePath, + parameters: _parameters, + packageRoot: packageRoot, + ); + if (cachedEntries == null) return false; + + final crossMatches = registry.findCrossFileMatches( + filePath, + cachedEntries, + parameters: _parameters, + packageRoot: packageRoot, + isFileExcluded: _isFileExcluded, + ); + + final hashGroups = >{}; + for (final entry in cachedEntries) { + (hashGroups[entry.hash] ??= []).add(entry); + } + + final hasIntraDuplicates = hashGroups.values.any( + (group) => group.length > 1, + ); + final hasCrossDuplicates = crossMatches.isNotEmpty; + + if (!hasIntraDuplicates && !hasCrossDuplicates) { + return true; // We checked the cache, and there are no duplicates. + } + + final suppressedRanges = <(int, int)>[]; + final reportedOffsets = {}; + final crossFileDuplicatesByHash = >{}; - if (filePath.isNotEmpty) { - final contextRoot = _contextRoot; - final packageRoot = - contextRoot?.root.path ?? _findPackageRoot(filePath) ?? ''; - - final registry = GlobalHashRegistry.instance; - final crossMatches = registry.findCrossFileMatches( - filePath, - hashEntries, - parameters: _parameters, - packageRoot: packageRoot, - isFileExcluded: (otherPath) { - if (contextRoot == null) return false; - final isWithin = otherPath.startsWith(contextRoot.root.path); - return isWithin && !contextRoot.isAnalyzed(otherPath); - }, + for (final match in crossMatches) { + crossFileDuplicatesByHash[match.currentEntry.hash] = match.duplicates; + } + + for (final entry in cachedEntries) { + final isSuppressed = suppressedRanges.any( + (range) => (entry.offset, entry.length).isStrictlyWithin(range), ); - for (final match in crossMatches) { - crossFileDuplicatesByHash[match.currentEntry.hash] = match.duplicates; + if (isSuppressed) continue; + + if (reportedOffsets.contains(entry.offset)) continue; + + final group = hashGroups[entry.hash]; + if (group == null) continue; + + final activeGroup = group.where((e) { + return !suppressedRanges.any( + (range) => (e.offset, e.length).isStrictlyWithin(range), + ); + }).toList(); + + final externalPartners = + crossFileDuplicatesByHash[entry.hash] ?? const []; + final internalPartners = activeGroup.where((e) => e != entry).toList(); + + if (internalPartners.isNotEmpty || externalPartners.isNotEmpty) { + final contextMessages = [ + for (final dup in externalPartners) + SolidDiagnosticMessage( + filePath: dup.filePath, + offset: dup.entry.offset, + length: dup.entry.length, + message: 'Duplicate', + ), + for (final other in internalPartners) + SolidDiagnosticMessage( + filePath: filePath, + offset: other.offset, + length: other.length, + message: 'Duplicate', + ), + ]; + + _rule.reportAtOffset( + entry.offset, + entry.length, + contextMessages: contextMessages, + ); + + reportedOffsets.add(entry.offset); + suppressedRanges.add((entry.offset, entry.length)); } - registry.updateFile( - filePath, - hashEntries, - modificationStamp: _modificationStamp, - parameters: _parameters, - packageRoot: packageRoot, - ); } - // --- Phase C: Group candidates by structural hash for intra-file --- + return true; // Cache hit and handled + } + + bool _isFileExcluded(String otherPath) { + final contextRoot = _contextRoot; + if (contextRoot == null) return false; + final isWithin = otherPath.startsWith(contextRoot.root.path); + return isWithin && !contextRoot.isAnalyzed(otherPath); + } + + List<_BodyCandidate> _collectCandidates(CompilationUnit node) { + final collector = _CandidateCollector(_parameters); + node.accept(collector); + return collector.candidates; + } + + Map> _findAndSaveCrossFileMatches( + String filePath, + List hashEntries, + String packageRoot, + ) { + final crossFileDuplicatesByHash = >{}; + if (packageRoot.isEmpty) return crossFileDuplicatesByHash; + + final registry = GlobalHashRegistry.instance; + final crossMatches = registry.findCrossFileMatches( + filePath, + hashEntries, + parameters: _parameters, + packageRoot: packageRoot, + isFileExcluded: _isFileExcluded, + ); + for (final match in crossMatches) { + crossFileDuplicatesByHash[match.currentEntry.hash] = match.duplicates; + } + registry.updateFile( + filePath, + hashEntries, + modificationStamp: _modificationStamp, + parameters: _parameters, + packageRoot: packageRoot, + ); + + return crossFileDuplicatesByHash; + } + + void _groupAndReportDuplicates( + String filePath, + List<_BodyCandidate> candidates, + Map candidateHashes, + AstStructuralHasher hasher, + Map> crossFileDuplicatesByHash, + ) { final groups = >{}; for (final candidate in candidates) { final hash = @@ -240,8 +261,6 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { final suppressed = {}; final reported = {}; - // Candidates are collected pre-order (parent before children). - // Process larger scopes first, and suppress warnings on children. for (final candidate in candidates) { if (suppressed.contains(candidate.node)) continue; if (reported.contains(candidate.node)) continue; @@ -251,7 +270,6 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { final group = groups[hash]; if (group == null) continue; - // Filter group to only include non-suppressed candidates final activeGroup = group .where((c) => !suppressed.contains(c.node)) .toList(); @@ -261,15 +279,8 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { .where((c) => c != candidate) .toList(); - final hasExternalDuplicates = externalPartners.isNotEmpty; - final hasInternalDuplicates = internalPartners.isNotEmpty; - - // Report a warning if there are any duplicates (internal or external) - final shouldReport = hasInternalDuplicates || hasExternalDuplicates; - - if (shouldReport) { + if (internalPartners.isNotEmpty || externalPartners.isNotEmpty) { final contextMessages = [ - // Add external partners for (final dup in externalPartners) SolidDiagnosticMessage( filePath: dup.filePath, @@ -277,7 +288,6 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { length: dup.entry.length, message: 'Duplicate', ), - // Add internal partners for (final other in internalPartners) SolidDiagnosticMessage( filePath: filePath, @@ -292,15 +302,12 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { contextMessages: contextMessages, ); - // Mark this duplicate block as reported. reported.add(candidate.node); - // Mark all its descendants as suppressed. _suppressDescendants(candidate.node, suppressed); } } } - /// Returns the number of tokens in a body node. static int _tokenCount(AstNode node) { int count = 0; var token = node.beginToken; @@ -316,21 +323,26 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { root.accept(_DescendantCollector(suppressed, root)); } + static final _packageRootCache = {}; + String? _findPackageRoot(String filePath) { if (filePath.isEmpty) return null; - var dir = File(filePath).parent; - while (true) { - final pubspec = File(path.join(dir.path, 'pubspec.yaml')); - if (pubspec.existsSync()) { - return dir.path; - } - final parent = dir.parent; - if (parent.path == dir.path) { - break; + final dirPath = path.dirname(filePath); + return _packageRootCache.putIfAbsent(dirPath, () { + var dir = Directory(dirPath); + while (true) { + final pubspec = File(path.join(dir.path, 'pubspec.yaml')); + if (pubspec.existsSync()) { + return dir.path; + } + final parent = dir.parent; + if (parent.path == dir.path) { + break; + } + dir = parent; } - dir = parent; - } - return null; + return null; + }); } } From a18f796508a1041685ed61f370dcf813fc331b9d Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Tue, 14 Jul 2026 16:55:29 +0300 Subject: [PATCH 10/28] refactor: modularize duplicate detection logic by introducing specialized visitors and utility extensions --- .../models/body_candidate.dart | 7 + .../models/cross_file_match.dart | 16 +-- .../models/duplicate_location.dart | 16 +++ .../services/global_hash_registry.dart | 111 +++++++-------- .../services/hash_cache_storage.dart | 5 +- .../utils/context_root_extensions.dart | 13 ++ .../avoid_duplicate_code/utils/debouncer.dart | 29 ++++ .../utils/path_utils.dart | 9 ++ .../utils/range_extension.dart | 11 ++ .../utils/token_utils.dart | 13 ++ ....dart => ast_structural_hash_visitor.dart} | 22 ++- .../avoid_duplicate_code_visitor.dart | 133 +++--------------- .../visitors/candidate_visitor.dart | 50 +++++++ .../visitors/descendant_visitor.dart | 30 ++++ .../avoid_duplicate_code_rule_test.dart | 117 +++++++++++++-- .../global_hash_registry_test.dart | 83 +++++++---- 16 files changed, 423 insertions(+), 242 deletions(-) create mode 100644 lib/src/lints/avoid_duplicate_code/models/body_candidate.dart create mode 100644 lib/src/lints/avoid_duplicate_code/models/duplicate_location.dart create mode 100644 lib/src/lints/avoid_duplicate_code/utils/context_root_extensions.dart create mode 100644 lib/src/lints/avoid_duplicate_code/utils/debouncer.dart create mode 100644 lib/src/lints/avoid_duplicate_code/utils/path_utils.dart create mode 100644 lib/src/lints/avoid_duplicate_code/utils/range_extension.dart create mode 100644 lib/src/lints/avoid_duplicate_code/utils/token_utils.dart rename lib/src/lints/avoid_duplicate_code/visitors/{ast_structural_hasher.dart => ast_structural_hash_visitor.dart} (91%) create mode 100644 lib/src/lints/avoid_duplicate_code/visitors/candidate_visitor.dart create mode 100644 lib/src/lints/avoid_duplicate_code/visitors/descendant_visitor.dart diff --git a/lib/src/lints/avoid_duplicate_code/models/body_candidate.dart b/lib/src/lints/avoid_duplicate_code/models/body_candidate.dart new file mode 100644 index 00000000..8e0b2b83 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/models/body_candidate.dart @@ -0,0 +1,7 @@ +import 'package:analyzer/dart/ast/ast.dart'; + +/// A record representing a block or expression candidate for clone detection. +typedef BodyCandidate = ({ + AstNode node, + Declaration? enclosingDeclaration, +}); diff --git a/lib/src/lints/avoid_duplicate_code/models/cross_file_match.dart b/lib/src/lints/avoid_duplicate_code/models/cross_file_match.dart index 54fd008f..2e7c9b3c 100644 --- a/lib/src/lints/avoid_duplicate_code/models/cross_file_match.dart +++ b/lib/src/lints/avoid_duplicate_code/models/cross_file_match.dart @@ -1,20 +1,6 @@ +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/duplicate_location.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/hash_entry.dart'; -/// Represents a location in the codebase where a duplicate was found. -class DuplicateLocation { - /// The matching hash entry in the other file. - final HashEntry entry; - - /// The absolute path of the other file. - final String filePath; - - /// Creates a new [DuplicateLocation]. - const DuplicateLocation({ - required this.entry, - required this.filePath, - }); -} - /// Represents a structural duplicate found in other files. class CrossFileMatch { /// The hash entry in the current file. diff --git a/lib/src/lints/avoid_duplicate_code/models/duplicate_location.dart b/lib/src/lints/avoid_duplicate_code/models/duplicate_location.dart new file mode 100644 index 00000000..8599938f --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/models/duplicate_location.dart @@ -0,0 +1,16 @@ +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/hash_entry.dart'; + +/// Represents a location in the codebase where a duplicate was found. +class DuplicateLocation { + /// The matching hash entry in the other file. + final HashEntry entry; + + /// The absolute path of the other file. + final String filePath; + + /// Creates a new [DuplicateLocation]. + const DuplicateLocation({ + required this.entry, + required this.filePath, + }); +} diff --git a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart index a46f3647..ba11c0d8 100644 --- a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart +++ b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart @@ -1,11 +1,13 @@ -import 'dart:async'; import 'dart:io'; -import 'package:path/path.dart' as p; + import 'package:solid_lints/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/cross_file_match.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/duplicate_location.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/file_cache_entry.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/hash_entry.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/utils/debouncer.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/utils/path_utils.dart'; /// A singleton registry that stores structural hashes for all analyzed files. /// @@ -17,9 +19,17 @@ import 'package:solid_lints/src/lints/avoid_duplicate_code/services/hash_cache_s /// Subsequent file analyses check their hashes against the registry to find /// cross-file duplicates. class GlobalHashRegistry { + static const _saveDebounceDuration = Duration(milliseconds: 500); + /// Singleton instance — lives as long as the plugin isolate. static final instance = GlobalHashRegistry._(); + /// Whether to automatically clean up files that do not exist physically on + /// disk. + /// + /// Set to `false` in tests using a virtual resource provider. + bool enablePhysicalFileCleanup = true; + /// Internal index: filePath → FileCacheEntry. final _index = {}; @@ -27,23 +37,16 @@ class GlobalHashRegistry { final _hashToLocations = >{}; final _loadedRoots = {}; - final _saveDebouncer = _Debouncer(const Duration(milliseconds: 500)); + late final _saveDebouncer = Debouncer(_saveDebounceDuration); AvoidDuplicateCodeParameters? _currentParams; - /// Whether to automatically clean up files that do not exist physically on - /// disk. - /// - /// Set to `false` in tests using a virtual resource provider. - bool enablePhysicalFileCleanup = true; - GlobalHashRegistry._(); - String _normalizePath(String filePath, String root) { - return p.isAbsolute(filePath) - ? p.normalize(filePath) - : p.normalize(p.join(root, filePath)); - } + /// The number of files currently indexed. + int get fileCount => _index.length; + + String _getRoot(String? packageRoot) => packageRoot ?? Directory.current.path; void _addToInvertedIndex(String absoluteFilePath, List entries) { for (final entry in entries) { @@ -106,6 +109,16 @@ class GlobalHashRegistry { } } + String _resolveAndLoad( + String filePath, + String? packageRoot, + AvoidDuplicateCodeParameters? parameters, + ) { + final root = _getRoot(packageRoot); + _ensureLoaded(root, parameters); + return normalizePath(filePath, root); + } + /// Returns the cached modification stamp for [filePath], or `null` if no /// indexed. int? getModificationStamp( @@ -113,9 +126,7 @@ class GlobalHashRegistry { AvoidDuplicateCodeParameters? parameters, String? packageRoot, }) { - final root = packageRoot ?? Directory.current.path; - _ensureLoaded(root, parameters); - final absoluteFilePath = _normalizePath(filePath, root); + final absoluteFilePath = _resolveAndLoad(filePath, packageRoot, parameters); return _index[absoluteFilePath]?.modificationStamp; } @@ -125,9 +136,7 @@ class GlobalHashRegistry { AvoidDuplicateCodeParameters? parameters, String? packageRoot, }) { - final root = packageRoot ?? Directory.current.path; - _ensureLoaded(root, parameters); - final absoluteFilePath = _normalizePath(filePath, root); + final absoluteFilePath = _resolveAndLoad(filePath, packageRoot, parameters); return _index[absoluteFilePath]?.entries; } @@ -139,9 +148,8 @@ class GlobalHashRegistry { AvoidDuplicateCodeParameters? parameters, String? packageRoot, }) { - final root = packageRoot ?? Directory.current.path; - _ensureLoaded(root, parameters); - final absoluteFilePath = _normalizePath(filePath, root); + final root = _getRoot(packageRoot); + final absoluteFilePath = _resolveAndLoad(filePath, packageRoot, parameters); final oldEntry = _index[absoluteFilePath]; if (oldEntry != null) { @@ -167,10 +175,12 @@ class GlobalHashRegistry { bool Function(String filePath)? isFileExcluded, String? packageRoot, }) { - final root = packageRoot ?? Directory.current.path; - _ensureLoaded(root, parameters); - - final absoluteCurrentFilePath = _normalizePath(currentFilePath, root); + final root = _getRoot(packageRoot); + final absoluteCurrentFilePath = _resolveAndLoad( + currentFilePath, + packageRoot, + parameters, + ); final matches = []; final filesToRemove = {}; @@ -201,11 +211,6 @@ class GlobalHashRegistry { } if (!key.startsWith(root)) continue; - if (key.contains('.dart_tool/')) { - if (!key.contains('.dart_tool/generated_test/')) continue; - if (p.dirname(key) != p.dirname(absoluteCurrentFilePath)) continue; - } - duplicates.add(loc); } } @@ -240,9 +245,8 @@ class GlobalHashRegistry { AvoidDuplicateCodeParameters? parameters, String? packageRoot, }) { - final root = packageRoot ?? Directory.current.path; - _ensureLoaded(root, parameters); - final absoluteFilePath = _normalizePath(filePath, root); + final root = _getRoot(packageRoot); + final absoluteFilePath = _resolveAndLoad(filePath, packageRoot, parameters); final oldEntry = _index[absoluteFilePath]; if (oldEntry != null) { _removeFromInvertedIndex(absoluteFilePath, oldEntry.entries); @@ -251,20 +255,6 @@ class GlobalHashRegistry { _scheduleSave(root, parameters); } - /// The number of files currently indexed. - int get fileCount => _index.length; - - /// Clears the entire index and deletes the persistent cache file. - /// - /// Primarily used in tests to ensure test isolation. - void clear() { - _saveDebouncer.cancel(); - _loadedRoots.clear(); - _index.clear(); - _hashToLocations.clear(); - HashCacheStorage.delete(Directory.current.path); - } - void _scheduleSave( String packageRoot, [ AvoidDuplicateCodeParameters? parameters, @@ -288,20 +278,15 @@ class GlobalHashRegistry { parameters ?? _currentParams ?? AvoidDuplicateCodeParameters.empty(); HashCacheStorage.save(packageRoot, subset, params); } -} - -class _Debouncer { - final Duration delay; - Timer? _timer; - - _Debouncer(this.delay); - - void run(void Function() action) { - _timer?.cancel(); - _timer = Timer(delay, action); - } - void cancel() { - _timer?.cancel(); + /// Clears the entire index and deletes the persistent cache file. + /// + /// Primarily used in tests to ensure test isolation. + void clear() { + _saveDebouncer.cancel(); + _loadedRoots.clear(); + _index.clear(); + _hashToLocations.clear(); + HashCacheStorage.delete(Directory.current.path); } } diff --git a/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart b/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart index e27f9315..e4116d6e 100644 --- a/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart +++ b/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart @@ -4,6 +4,7 @@ import 'package:collection/collection.dart'; import 'package:path/path.dart' as p; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/file_cache_entry.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/utils/path_utils.dart'; /// Service responsible for persisting and loading the duplicate code hash /// cache. @@ -48,9 +49,7 @@ class HashCacheStorage { final result = {}; for (final MapEntry(:key, :value) in filesMap.entries) { - final absoluteKey = p.isAbsolute(key) - ? p.normalize(key) - : p.normalize(p.join(packageRoot, key)); + final absoluteKey = normalizePath(key, packageRoot); if (value is Map) { try { diff --git a/lib/src/lints/avoid_duplicate_code/utils/context_root_extensions.dart b/lib/src/lints/avoid_duplicate_code/utils/context_root_extensions.dart new file mode 100644 index 00000000..7dc8e055 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/utils/context_root_extensions.dart @@ -0,0 +1,13 @@ +import 'package:analyzer/dart/analysis/context_root.dart'; + +/// Extension methods for [ContextRoot] to help with file analysis checks. +extension ContextRootExtensions on ContextRoot { + /// Checks if [filePath] is excluded from analysis by this context root. + /// + /// Returns `true` only if the file is within the context root but is + /// explicitly excluded (e.g., via analysis_options.yaml). + bool isFileExcluded(String filePath) { + final isWithin = filePath.startsWith(root.path); + return isWithin && !isAnalyzed(filePath); + } +} diff --git a/lib/src/lints/avoid_duplicate_code/utils/debouncer.dart b/lib/src/lints/avoid_duplicate_code/utils/debouncer.dart new file mode 100644 index 00000000..8500a155 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/utils/debouncer.dart @@ -0,0 +1,29 @@ +import 'dart:async'; + +/// A utility class for debouncing quick, successive operations. +/// +/// It guarantees that an action is executed only once after a specified [delay] +/// of inactivity, cancelling any previously scheduled execution if a new one is +/// requested before the delay completes. +class Debouncer { + /// The duration to wait before executing the action. + final Duration delay; + Timer? _timer; + + /// Creates a new [Debouncer] with the given [delay]. + Debouncer(this.delay); + + /// Schedules [action] to be executed after the configured [delay]. + /// + /// If [run] is called again before the delay expires, the previous timer + /// is cancelled and a new one is started. + void run(void Function() action) { + _timer?.cancel(); + _timer = Timer(delay, action); + } + + /// Cancels any scheduled action from executing. + void cancel() { + _timer?.cancel(); + } +} diff --git a/lib/src/lints/avoid_duplicate_code/utils/path_utils.dart b/lib/src/lints/avoid_duplicate_code/utils/path_utils.dart new file mode 100644 index 00000000..4247b571 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/utils/path_utils.dart @@ -0,0 +1,9 @@ +import 'package:path/path.dart' as p; + +/// Normalizes a file path, ensuring it is absolute and resolved correctly +/// relative to the given [root] if it isn't already absolute. +String normalizePath(String filePath, String root) { + return p.isAbsolute(filePath) + ? p.normalize(filePath) + : p.normalize(p.join(root, filePath)); +} diff --git a/lib/src/lints/avoid_duplicate_code/utils/range_extension.dart b/lib/src/lints/avoid_duplicate_code/utils/range_extension.dart new file mode 100644 index 00000000..2fdd72ed --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/utils/range_extension.dart @@ -0,0 +1,11 @@ +/// Extension on an (offset, length) record to check if it is strictly +/// within another parent (offset, length) range. +extension RangeExtension on (int, int) { + /// Returns `true` if this range is strictly inside the [parent] range + /// (meaning it is within the bounds but not exactly equal to the parent). + bool isStrictlyWithin((int, int) parent) { + return this.$1 >= parent.$1 && + (this.$1 + this.$2) <= (parent.$1 + parent.$2) && + !(this.$1 == parent.$1 && this.$2 == parent.$2); + } +} diff --git a/lib/src/lints/avoid_duplicate_code/utils/token_utils.dart b/lib/src/lints/avoid_duplicate_code/utils/token_utils.dart new file mode 100644 index 00000000..7b023ff0 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/utils/token_utils.dart @@ -0,0 +1,13 @@ +import 'package:analyzer/dart/ast/ast.dart'; + +/// Returns the total number of non-EOF tokens within this node. +int getTokenCount(AstNode node) { + int count = 0; + var token = node.beginToken; + final end = node.endToken; + while (token != end) { + count++; + token = token.next!; + } + return count + 1; +} diff --git a/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hasher.dart b/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hash_visitor.dart similarity index 91% rename from lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hasher.dart rename to lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hash_visitor.dart index 66424670..dceaceaa 100644 --- a/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hasher.dart +++ b/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hash_visitor.dart @@ -12,14 +12,15 @@ import 'package:solid_lints/src/lints/avoid_duplicate_code/utils/jenkins_hasher. /// variables), whitespace, and comments. This enables Type 2 clone detection /// where two code blocks with identical structure but different variable names /// are considered clones. -class AstStructuralHasher extends UnifyingAstVisitor { +class AstStructuralHashVisitor extends UnifyingAstVisitor { + final JenkinsHasher _hasher = JenkinsHasher(); + final bool _ignoreLiterals; final bool _ignoreIdentifiers; - final JenkinsHasher _hasher = JenkinsHasher(); final Map _localVariableIds = {}; - /// Creates a new [AstStructuralHasher]. - AstStructuralHasher({ + /// Creates a new [AstStructuralHashVisitor]. + AstStructuralHashVisitor({ required bool ignoreLiterals, required bool ignoreIdentifiers, }) : _ignoreLiterals = ignoreLiterals, @@ -176,10 +177,8 @@ class AstStructuralHasher extends UnifyingAstVisitor { // This preserves field names, getters, methods, class names, etc. final element = node.element; if (element != null) { - if (element is! LocalVariableElement && - element is! FormalParameterElement) { - _append(node.name); - } else { + if (element is LocalVariableElement || + element is FormalParameterElement) { // It is a local variable or parameter. // Assign it a local ID (De Bruijn Indexing) to distinguish clones // that wire their variables differently. @@ -188,12 +187,9 @@ class AstStructuralHasher extends UnifyingAstVisitor { () => _localVariableIds.length, ); _appendHash(id); + } else { + _append(node.name); } - } else { - // If element is null, we are not sure what it is. - // It could be an unresolved local variable or an unresolved property. - // We ignore it to preserve standard Type-2 clone behavior (less false - // negatives). } } super.visitSimpleIdentifier(node); diff --git a/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart index c6ab0c74..41a1aa51 100644 --- a/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart +++ b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart @@ -7,29 +7,23 @@ import 'package:analyzer/diagnostic/diagnostic.dart'; import 'package:path/path.dart' as path; import 'package:solid_lints/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart'; -import 'package:solid_lints/src/lints/avoid_duplicate_code/models/cross_file_match.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/body_candidate.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/duplicate_location.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/hash_entry.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/services/global_hash_registry.dart'; -import 'package:solid_lints/src/lints/avoid_duplicate_code/visitors/ast_structural_hasher.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/utils/context_root_extensions.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/utils/range_extension.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/utils/token_utils.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/visitors/ast_structural_hash_visitor.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/visitors/candidate_visitor.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/visitors/descendant_visitor.dart'; import 'package:solid_lints/src/models/solid_diagnostic_message.dart'; -extension _RangeExtension on (int, int) { - bool isStrictlyWithin((int, int) parent) { - return this.$1 >= parent.$1 && - (this.$1 + this.$2) <= (parent.$1 + parent.$2) && - !(this.$1 == parent.$1 && this.$2 == parent.$2); - } -} - -/// A record representing a block or expression candidate for clone detection. -typedef _BodyCandidate = ({ - AstNode node, - Declaration? enclosingDeclaration, -}); - /// A visitor that detects duplicate code blocks (at the function level and/or /// statement block level) within a single compilation unit and across files. class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { + static const _duplicateContextMessage = 'Duplicate'; + final AvoidDuplicateCodeRule _rule; final AvoidDuplicateCodeParameters _parameters; final String _filePath; @@ -61,7 +55,7 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { final candidates = _collectCandidates(node); - final hasher = AstStructuralHasher( + final hasher = AstStructuralHashVisitor( ignoreLiterals: _parameters.ignoreLiterals, ignoreIdentifiers: _parameters.ignoreIdentifiers, ); @@ -79,7 +73,7 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { lineNumber: line, offset: candidate.node.offset, length: candidate.node.length, - tokenCount: _tokenCount(candidate.node), + tokenCount: getTokenCount(candidate.node), ), ); } @@ -123,7 +117,7 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { cachedEntries, parameters: _parameters, packageRoot: packageRoot, - isFileExcluded: _isFileExcluded, + isFileExcluded: (p) => _contextRoot?.isFileExcluded(p) ?? false, ); final hashGroups = >{}; @@ -201,15 +195,8 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { return true; // Cache hit and handled } - bool _isFileExcluded(String otherPath) { - final contextRoot = _contextRoot; - if (contextRoot == null) return false; - final isWithin = otherPath.startsWith(contextRoot.root.path); - return isWithin && !contextRoot.isAnalyzed(otherPath); - } - - List<_BodyCandidate> _collectCandidates(CompilationUnit node) { - final collector = _CandidateCollector(_parameters); + List _collectCandidates(CompilationUnit node) { + final collector = CandidateVisitor(_parameters); node.accept(collector); return collector.candidates; } @@ -228,7 +215,7 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { hashEntries, parameters: _parameters, packageRoot: packageRoot, - isFileExcluded: _isFileExcluded, + isFileExcluded: (p) => _contextRoot?.isFileExcluded(p) ?? false, ); for (final match in crossMatches) { crossFileDuplicatesByHash[match.currentEntry.hash] = match.duplicates; @@ -246,12 +233,12 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { void _groupAndReportDuplicates( String filePath, - List<_BodyCandidate> candidates, + List candidates, Map candidateHashes, - AstStructuralHasher hasher, + AstStructuralHashVisitor hasher, Map> crossFileDuplicatesByHash, ) { - final groups = >{}; + final groups = >{}; for (final candidate in candidates) { final hash = candidateHashes[candidate.node] ?? hasher.computeHash(candidate.node); @@ -286,14 +273,14 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { filePath: dup.filePath, offset: dup.entry.offset, length: dup.entry.length, - message: 'Duplicate', + message: _duplicateContextMessage, ), for (final other in internalPartners) SolidDiagnosticMessage( filePath: filePath, offset: other.node.offset, length: other.node.length, - message: 'Duplicate', + message: _duplicateContextMessage, ), ]; @@ -308,19 +295,9 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { } } - static int _tokenCount(AstNode node) { - int count = 0; - var token = node.beginToken; - final end = node.endToken; - while (token != end) { - count++; - token = token.next!; - } - return count + 1; - } - void _suppressDescendants(AstNode root, Set suppressed) { - root.accept(_DescendantCollector(suppressed, root)); + final descendantCollector = DescendantVisitor(suppressed, root); + root.accept(descendantCollector); } static final _packageRootCache = {}; @@ -345,67 +322,3 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { }); } } - -/// A visitor that collects descendant candidate blocks to suppress them. -class _DescendantCollector extends RecursiveAstVisitor { - final Set suppressed; - final AstNode root; - - _DescendantCollector(this.suppressed, this.root); - - @override - void visitBlock(Block node) { - if (node != root) { - suppressed.add(node); - } - super.visitBlock(node); - } - - @override - void visitExpressionFunctionBody(ExpressionFunctionBody node) { - if (node != root) { - suppressed.add(node); - } - super.visitExpressionFunctionBody(node); - } -} - -class _CandidateCollector extends RecursiveAstVisitor { - final List<_BodyCandidate> candidates = []; - final AvoidDuplicateCodeParameters parameters; - - _CandidateCollector(this.parameters); - - @override - void visitBlock(Block node) { - // If checkBlocks is false, only consider blocks that represent function - // bodies. - if (!parameters.checkBlocks && node.parent is! BlockFunctionBody) { - super.visitBlock(node); - return; - } - - _checkAndCollect(node, AvoidDuplicateCodeVisitor._tokenCount(node)); - super.visitBlock(node); - } - - @override - void visitExpressionFunctionBody(ExpressionFunctionBody node) { - _checkAndCollect(node, AvoidDuplicateCodeVisitor._tokenCount(node)); - super.visitExpressionFunctionBody(node); - } - - void _checkAndCollect(AstNode node, int tokenCount) { - if (tokenCount < parameters.minTokens) return; - - final declaration = node.thisOrAncestorOfType(); - if (declaration != null && parameters.exclude.shouldIgnore(declaration)) { - return; - } - - candidates.add(( - node: node, - enclosingDeclaration: declaration, - )); - } -} diff --git a/lib/src/lints/avoid_duplicate_code/visitors/candidate_visitor.dart b/lib/src/lints/avoid_duplicate_code/visitors/candidate_visitor.dart new file mode 100644 index 00000000..fae430ab --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/visitors/candidate_visitor.dart @@ -0,0 +1,50 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/body_candidate.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/utils/token_utils.dart'; + +/// A visitor that collects all block-level duplicate code candidates. +class CandidateVisitor extends RecursiveAstVisitor { + /// The list of collected candidates. + final List candidates = []; + + /// The parameters for the lint rule. + final AvoidDuplicateCodeParameters parameters; + + /// Creates a new instance of [CandidateVisitor]. + CandidateVisitor(this.parameters); + + @override + void visitBlock(Block node) { + // If checkBlocks is false, only consider blocks that represent function + // bodies. + if (!parameters.checkBlocks && node.parent is! BlockFunctionBody) { + super.visitBlock(node); + return; + } + + _checkAndCollect(node, getTokenCount(node)); + super.visitBlock(node); + } + + @override + void visitExpressionFunctionBody(ExpressionFunctionBody node) { + _checkAndCollect(node, getTokenCount(node)); + super.visitExpressionFunctionBody(node); + } + + void _checkAndCollect(AstNode node, int tokenCount) { + if (tokenCount < parameters.minTokens) return; + + final declaration = node.thisOrAncestorOfType(); + if (declaration != null && parameters.exclude.shouldIgnore(declaration)) { + return; + } + + candidates.add(( + node: node, + enclosingDeclaration: declaration, + )); + } +} diff --git a/lib/src/lints/avoid_duplicate_code/visitors/descendant_visitor.dart b/lib/src/lints/avoid_duplicate_code/visitors/descendant_visitor.dart new file mode 100644 index 00000000..467b2a32 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/visitors/descendant_visitor.dart @@ -0,0 +1,30 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; + +/// A visitor that collects descendant candidate blocks to suppress them. +class DescendantVisitor extends RecursiveAstVisitor { + /// The set of suppressed nodes. + final Set suppressed; + + /// The root node from which to start the descent. + final AstNode root; + + /// Creates a new instance of [DescendantVisitor]. + DescendantVisitor(this.suppressed, this.root); + + @override + void visitBlock(Block node) { + if (node != root) { + suppressed.add(node); + } + super.visitBlock(node); + } + + @override + void visitExpressionFunctionBody(ExpressionFunctionBody node) { + if (node != root) { + suppressed.add(node); + } + super.visitExpressionFunctionBody(node); + } +} diff --git a/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart b/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart index 72ad70e7..161429bf 100644 --- a/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart +++ b/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart @@ -56,8 +56,7 @@ $_mockAnalysisOptionsContent''', // --- Base Tests (min_tokens: 15, check_blocks: true, default) --- - Future - test_reports_when_two_functions_have_identical_bodies() async { + Future test_reports_when_two_functions_have_identical_bodies() async { await assertAutoDiagnostics(''' void first() ${expectLint(r'''{ final x = 1; @@ -119,8 +118,7 @@ void second() { '''); } - Future - test_does_not_report_when_body_below_min_tokens() async { + Future test_does_not_report_when_body_below_min_tokens() async { await assertNoDiagnostics(r''' void first() { print('hello'); @@ -134,8 +132,7 @@ void second() { '''); } - Future - test_reports_on_methods_in_same_class() async { + Future test_reports_on_methods_in_same_class() async { await assertAutoDiagnostics(''' class MyClass { void first() ${expectLint(r'''{ @@ -157,8 +154,7 @@ class MyClass { '''); } - Future - test_reports_on_third_clone_also() async { + Future test_reports_on_third_clone_also() async { await assertAutoDiagnostics(''' void first() ${expectLint(r'''{ final x = 1; @@ -188,8 +184,7 @@ void third() ${expectLint(r'''{ // --- Ignore Literals Tests --- - Future - test_reports_when_only_literal_values_differ() async { + Future test_reports_when_only_literal_values_differ() async { newAnalysisOptionsYamlFile( testPackageRootPath, '''${analysisOptionsContent(rules: [rule.name])} @@ -277,8 +272,7 @@ void second() { // --- Exclude Tests --- - Future - test_does_not_report_on_excluded_function() async { + Future test_does_not_report_on_excluded_function() async { await assertNoDiagnostics(r''' void first() { final x = 1; @@ -382,6 +376,105 @@ void mainMethod() ${expectLint(r'''{ } print('done'); }''')} +'''); + } + + Future test_reports_on_expression_function_bodies() async { + // 15+ tokens are needed. We repeat a pattern to ensure token count. + await assertAutoDiagnostics(''' +int first(int a, int b) ${expectLint(r'''=> a + b + a + b + + a + b + a + b + a + b + a + b;''')} + +int second(int x, int y) ${expectLint(r'''=> x + y + x + y + + x + y + x + y + x + y + x + y;''')} +'''); + } + + Future test_reports_despite_different_comments_and_formatting() async { + await assertAutoDiagnostics(''' +void first() ${expectLint(r'''{ + final x = 1; + // This is a comment + if (x > 0) { + print(x); + } + print('done'); +}''')} + +void second() ${expectLint(r'''{ + final y = 1; + + + if (y > 0) { + /* multiline + comment */ + print(y); + } + + print('done'); +}''')} +'''); + } + + Future test_ignores_nested_blocks_when_check_blocks_false() async { + newAnalysisOptionsYamlFile( + testPackageRootPath, + '''${analysisOptionsContent(rules: [rule.name])} +plugins: + solid_lints: + diagnostics: + avoid_duplicate_code: + min_tokens: 15 + check_blocks: false +''', + ); + + // The nested blocks are identical (>15 tokens), but check_blocks is false, + // and the outer function bodies differ significantly. + await assertNoDiagnostics(r''' +void one() { + final x = 1; + if (x > 0) { + print('hello'); + print('world'); + print('done'); + } +} + +void two() { + print('completely different start'); + if (true) { + print('hello'); + print('world'); + print('done'); + } +} +'''); + } + + Future + test_does_not_report_when_identifiers_are_different_method_calls() async { + // Tests that ignore_identifiers=true still differentiates method calls. + // Local variables are ignored, but external method names are preserved. + await assertNoDiagnostics(''' +void doSomething(int x) {} +void doAnotherThing(int x) {} + +void first() { + final x = 1; + if (x > 0) { + doSomething(x); + } + print('done'); +} + +void second() { + final y = 1; + if (y > 0) { + doAnotherThing(y); + } + print('done'); +} '''); } } diff --git a/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart b/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart index 27b93030..e5b99f87 100644 --- a/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart +++ b/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart @@ -43,11 +43,16 @@ void main() { registry.updateFile('file_a.dart', fileAEntries, modificationStamp: 1); - final matches = registry.findCrossFileMatches('file_b.dart', fileBEntries); + final matches = registry.findCrossFileMatches( + 'file_b.dart', + fileBEntries, + ); expect(matches, hasLength(1)); expect(matches.first.duplicates, hasLength(1)); - final expectedPath = p.normalize(p.join(Directory.current.path, 'file_a.dart')); + final expectedPath = p.normalize( + p.join(Directory.current.path, 'file_a.dart'), + ); expect(matches.first.duplicates.first.filePath, equals(expectedPath)); expect(matches.first.duplicates.first.entry.hash, equals(123)); expect(matches.first.duplicates.first.entry.lineNumber, equals(10)); @@ -136,24 +141,24 @@ void main() { expect(matches, hasLength(1)); expect(matches.first.duplicates, hasLength(2)); - final expectedPathA = p.normalize(p.join(Directory.current.path, 'file_a.dart')); - final expectedPathB = p.normalize(p.join(Directory.current.path, 'file_b.dart')); + final expectedPathA = p.normalize( + p.join(Directory.current.path, 'file_a.dart'), + ); + final expectedPathB = p.normalize( + p.join(Directory.current.path, 'file_b.dart'), + ); expect(matches.first.duplicates[0].filePath, equals(expectedPathA)); expect(matches.first.duplicates[1].filePath, equals(expectedPathB)); }); test('HashCacheStorage saves and loads index', () { - final absoluteFilePath = p.normalize(p.join(Directory.current.path, 'file_a.dart')); + final absoluteFilePath = p.normalize( + p.join(Directory.current.path, 'file_a.dart'), + ); final index = { absoluteFilePath: const FileCacheEntry( modificationStamp: 123456, - entries: [ - HashEntry( - hash: 123, - lineNumber: 10, - tokenCount: 5, - ), - ], + entries: [HashEntry(hash: 123, lineNumber: 10, tokenCount: 5)], ), }; @@ -193,7 +198,8 @@ void main() { // Delete the file physically tempFile.deleteSync(); - // Trigger matching, which should clean up the deleted tempFile from registry + // Trigger matching, which should clean up the deleted tempFile + // from registry final matches = registry.findCrossFileMatches('other_file.dart', [ const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), ]); @@ -203,20 +209,20 @@ void main() { }); test('findCrossFileMatches cleans up absolute paths of excluded files', () { - final absoluteExcludedPath = - p.normalize('/workspace/project/lib/excluded.dart'); + final absoluteExcludedPath = p.normalize( + '/workspace/project/lib/excluded.dart', + ); registry.updateFile(absoluteExcludedPath, [ const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), ], modificationStamp: 1); expect(registry.fileCount, equals(1)); - // Trigger matching with a callback that considers absoluteExcludedPath as excluded - final matches = registry.findCrossFileMatches( - 'other_file.dart', - [const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5)], - isFileExcluded: (path) => path == absoluteExcludedPath, - ); + // Trigger matching with a callback that considers absoluteExcludedPath + // as excluded + final matches = registry.findCrossFileMatches('other_file.dart', [ + const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), + ], isFileExcluded: (path) => path == absoluteExcludedPath); expect(matches, isEmpty); expect(registry.fileCount, equals(0)); @@ -227,11 +233,9 @@ void main() { p.join(Directory.current.path, 'file.dart'), ); final index = { - absoluteFilePath: FileCacheEntry( + absoluteFilePath: const FileCacheEntry( modificationStamp: 123456, - entries: const [ - HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), - ], + entries: [HashEntry(hash: 123, lineNumber: 10, tokenCount: 5)], ), }; @@ -251,11 +255,38 @@ void main() { final loaded1 = HashCacheStorage.load(Directory.current.path, params1); expect(loaded1, isNotNull); - // Loading with params2 (different config) should return null (invalidated) + // Loading with params2 (different config) should return null + // (invalidated) final loaded2 = HashCacheStorage.load(Directory.current.path, params2); expect(loaded2, isNull); HashCacheStorage.delete(Directory.current.path); }); + + test('findCrossFileMatches processes multiple candidates correctly', () { + registry.updateFile('file_a.dart', [ + const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), + ], modificationStamp: 1); + + final matches = registry.findCrossFileMatches('file_b.dart', [ + const HashEntry(hash: 123, lineNumber: 20, tokenCount: 5), // Match + const HashEntry(hash: 999, lineNumber: 30, tokenCount: 10), // No match + ]); + + expect(matches, hasLength(1)); + expect(matches.first.duplicates.first.entry.hash, equals(123)); + }); + + test('HashCacheStorage.load returns null when cache file is missing', () { + // Ensure any existing cache is deleted + HashCacheStorage.delete(Directory.current.path); + + final loaded = HashCacheStorage.load( + Directory.current.path, + AvoidDuplicateCodeParameters.empty(), + ); + + expect(loaded, isNull); + }); }); } From f1b870e7cb137372a8112a04ebc845d674b954a4 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Tue, 14 Jul 2026 16:58:22 +0300 Subject: [PATCH 11/28] chore: remove exclusion for avoid_using_api lint rules --- lib/analysis_options.yaml | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/analysis_options.yaml b/lib/analysis_options.yaml index 8ff26b0e..0517da06 100644 --- a/lib/analysis_options.yaml +++ b/lib/analysis_options.yaml @@ -4,9 +4,6 @@ analyzer: - "**/*.g.dart" - "**/*.gr.dart" - # Exclude unmigrated rules - - "src/lints/avoid_using_api/**" - # Flutter - "lib/generated_plugin_registrant.dart" From 80f888afb0afc91b0c04c358942cde2c256437fb Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Tue, 14 Jul 2026 18:56:56 +0300 Subject: [PATCH 12/28] fix: use stable strings instead of runtime hash codes in AST structural hashing to prevent cache invalidation --- .../visitors/ast_structural_hash_visitor.dart | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hash_visitor.dart b/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hash_visitor.dart index dceaceaa..0133281f 100644 --- a/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hash_visitor.dart +++ b/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hash_visitor.dart @@ -49,7 +49,11 @@ class AstStructuralHashVisitor extends UnifyingAstVisitor { @override void visitNode(AstNode node) { - _appendHash(node.runtimeType.hashCode); + // Use the type name string instead of runtimeType.hashCode, because + // identity-based hashCode changes between VM/isolate restarts, making + // hashes stored in the persistent cache incompatible with newly computed + // ones. This caused "flapping" warnings that appeared and disappeared. + _append(node.runtimeType.toString()); node.visitChildren(this); _append('^'); } @@ -83,7 +87,7 @@ class AstStructuralHashVisitor extends UnifyingAstVisitor { @override void visitBinaryExpression(BinaryExpression node) { - _appendHash(node.operator.type.hashCode); + _append(node.operator.lexeme); super.visitBinaryExpression(node); } @@ -96,19 +100,19 @@ class AstStructuralHashVisitor extends UnifyingAstVisitor { node.operand.accept(this); return; } - _appendHash(node.operator.type.hashCode); + _append(node.operator.lexeme); super.visitPrefixExpression(node); } @override void visitPostfixExpression(PostfixExpression node) { - _appendHash(node.operator.type.hashCode); + _append(node.operator.lexeme); super.visitPostfixExpression(node); } @override void visitAssignmentExpression(AssignmentExpression node) { - _appendHash(node.operator.type.hashCode); + _append(node.operator.lexeme); super.visitAssignmentExpression(node); } @@ -190,6 +194,12 @@ class AstStructuralHashVisitor extends UnifyingAstVisitor { } else { _append(node.name); } + } else { + // Fallback for unresolved ASTs: + // When the IDE first opens a file, it may send an unresolved AST where + // element is null. To ensure the hash doesn't change drastically, + // we append the identifier name. + _append(node.name); } } super.visitSimpleIdentifier(node); From d0611f9e82a2abbb62dc09dd287495b5cc2753b0 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Tue, 14 Jul 2026 19:18:21 +0300 Subject: [PATCH 13/28] fix: improve accuracy and stability of duplicate code detection by implementing robust cache invalidation, parameter equality checks, and collision guarding. --- .../avoid_duplicate_code_parameters.dart | 17 +++++++++ .../models/duplicate_location.dart | 11 ++++++ .../services/global_hash_registry.dart | 38 +++++++++++++++++-- .../utils/token_utils.dart | 8 ++-- .../visitors/ast_structural_hash_visitor.dart | 11 +++++- .../avoid_duplicate_code_visitor.dart | 4 ++ 6 files changed, 82 insertions(+), 7 deletions(-) diff --git a/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart index 754075dc..b4fec8d3 100644 --- a/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart +++ b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart @@ -66,4 +66,21 @@ class AvoidDuplicateCodeParameters { 'check_blocks': checkBlocks, 'exclude': exclude.exclude.map((e) => e.toJson()).toList(), }; + + @override + bool operator ==(Object other) => + identical(this, other) || + other is AvoidDuplicateCodeParameters && + other.minTokens == minTokens && + other.ignoreLiterals == ignoreLiterals && + other.ignoreIdentifiers == ignoreIdentifiers && + other.checkBlocks == checkBlocks; + + @override + int get hashCode => Object.hash( + minTokens, + ignoreLiterals, + ignoreIdentifiers, + checkBlocks, + ); } diff --git a/lib/src/lints/avoid_duplicate_code/models/duplicate_location.dart b/lib/src/lints/avoid_duplicate_code/models/duplicate_location.dart index 8599938f..d4c90cd2 100644 --- a/lib/src/lints/avoid_duplicate_code/models/duplicate_location.dart +++ b/lib/src/lints/avoid_duplicate_code/models/duplicate_location.dart @@ -13,4 +13,15 @@ class DuplicateLocation { required this.entry, required this.filePath, }); + + @override + bool operator ==(Object other) => + identical(this, other) || + other is DuplicateLocation && + other.filePath == filePath && + other.entry.hash == entry.hash && + other.entry.offset == entry.offset; + + @override + int get hashCode => Object.hash(filePath, entry.hash, entry.offset); } diff --git a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart index ba11c0d8..cb297453 100644 --- a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart +++ b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart @@ -8,6 +8,7 @@ import 'package:solid_lints/src/lints/avoid_duplicate_code/models/hash_entry.dar import 'package:solid_lints/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/utils/debouncer.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/utils/path_utils.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart'; /// A singleton registry that stores structural hashes for all analyzed files. /// @@ -36,7 +37,7 @@ class GlobalHashRegistry { /// Auxiliary inverted index: hash → Set. final _hashToLocations = >{}; - final _loadedRoots = {}; + final _loadedRoots = {}; late final _saveDebouncer = Debouncer(_saveDebounceDuration); AvoidDuplicateCodeParameters? _currentParams; @@ -80,9 +81,18 @@ class GlobalHashRegistry { String packageRoot, [ AvoidDuplicateCodeParameters? currentParams, ]) { - if (_loadedRoots.contains(packageRoot)) return; - _loadedRoots.add(packageRoot); final params = currentParams ?? AvoidDuplicateCodeParameters.empty(); + final previousParams = _loadedRoots[packageRoot]; + + // If already loaded with the same parameters, skip. + if (previousParams != null && previousParams == params) return; + + // If parameters changed, clear entries for this root first. + if (previousParams != null) { + _clearEntriesForRoot(packageRoot); + } + + _loadedRoots[packageRoot] = params; _currentParams = params; final cached = HashCacheStorage.load(packageRoot, params); if (cached != null) { @@ -109,6 +119,23 @@ class GlobalHashRegistry { } } + void _clearEntriesForRoot(String packageRoot) { + final keysToRemove = []; + for (final key in _index.keys) { + if (key.startsWith(packageRoot)) { + keysToRemove.add(key); + } + } + for (final key in keysToRemove) { + final oldEntry = _index[key]; + if (oldEntry != null) { + _removeFromInvertedIndex(key, oldEntry.entries); + } + _index.remove(key); + } + } + + String _resolveAndLoad( String filePath, String? packageRoot, @@ -211,6 +238,10 @@ class GlobalHashRegistry { } if (!key.startsWith(root)) continue; + + // Verify tokenCount to guard against hash collisions. + if (loc.entry.tokenCount != entry.tokenCount) continue; + duplicates.add(loc); } } @@ -288,5 +319,6 @@ class GlobalHashRegistry { _index.clear(); _hashToLocations.clear(); HashCacheStorage.delete(Directory.current.path); + AvoidDuplicateCodeVisitor.clearPackageRootCache(); } } diff --git a/lib/src/lints/avoid_duplicate_code/utils/token_utils.dart b/lib/src/lints/avoid_duplicate_code/utils/token_utils.dart index 7b023ff0..483d819f 100644 --- a/lib/src/lints/avoid_duplicate_code/utils/token_utils.dart +++ b/lib/src/lints/avoid_duplicate_code/utils/token_utils.dart @@ -1,13 +1,15 @@ import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; /// Returns the total number of non-EOF tokens within this node. int getTokenCount(AstNode node) { int count = 0; - var token = node.beginToken; + Token? token = node.beginToken; final end = node.endToken; - while (token != end) { + while (token != null && token != end) { count++; - token = token.next!; + if (token == token.next) break; // Prevent infinite loop if AST is cyclical + token = token.next; } return count + 1; } diff --git a/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hash_visitor.dart b/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hash_visitor.dart index 0133281f..bb51b0c7 100644 --- a/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hash_visitor.dart +++ b/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hash_visitor.dart @@ -13,6 +13,8 @@ import 'package:solid_lints/src/lints/avoid_duplicate_code/utils/jenkins_hasher. /// where two code blocks with identical structure but different variable names /// are considered clones. class AstStructuralHashVisitor extends UnifyingAstVisitor { + static final _typeNameCache = {}; + final JenkinsHasher _hasher = JenkinsHasher(); final bool _ignoreLiterals; @@ -53,7 +55,14 @@ class AstStructuralHashVisitor extends UnifyingAstVisitor { // identity-based hashCode changes between VM/isolate restarts, making // hashes stored in the persistent cache incompatible with newly computed // ones. This caused "flapping" warnings that appeared and disappeared. - _append(node.runtimeType.toString()); + // + // The result is cached in a static map to avoid repeated toString() + // reflection calls during tree traversal. + final typeName = _typeNameCache.putIfAbsent( + node.runtimeType, + () => node.runtimeType.toString(), + ); + _append(typeName); node.visitChildren(this); _append('^'); } diff --git a/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart index 41a1aa51..75ad122a 100644 --- a/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart +++ b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart @@ -302,6 +302,10 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { static final _packageRootCache = {}; + /// Clears the cached package root lookups. Should be called when + /// the registry is cleared to avoid stale project path references. + static void clearPackageRootCache() => _packageRootCache.clear(); + String? _findPackageRoot(String filePath) { if (filePath.isEmpty) return null; final dirPath = path.dirname(filePath); From ce36e025e427567723dd1db2450392d16614487b Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Tue, 14 Jul 2026 19:19:03 +0300 Subject: [PATCH 14/28] refactor: remove unnecessary empty line in global_hash_registry.dart --- .../avoid_duplicate_code/services/global_hash_registry.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart index cb297453..054e8226 100644 --- a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart +++ b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart @@ -135,7 +135,6 @@ class GlobalHashRegistry { } } - String _resolveAndLoad( String filePath, String? packageRoot, From 1b557280613e56492304c1292e3b6fc0f1cb6b4d Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Tue, 14 Jul 2026 19:19:30 +0300 Subject: [PATCH 15/28] refactor: fix indentation in AvoidDuplicateCodeParameters hashCode method --- .../models/avoid_duplicate_code_parameters.dart | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart index b4fec8d3..4a699817 100644 --- a/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart +++ b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart @@ -78,9 +78,9 @@ class AvoidDuplicateCodeParameters { @override int get hashCode => Object.hash( - minTokens, - ignoreLiterals, - ignoreIdentifiers, - checkBlocks, - ); + minTokens, + ignoreLiterals, + ignoreIdentifiers, + checkBlocks, + ); } From 5e8e39371e9f0ee10ee3837af9af034a69e03a7a Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 15 Jul 2026 11:30:17 +0300 Subject: [PATCH 16/28] docs: update changelog with avoid_duplicate_code rule and formatting improvements --- CHANGELOG.md | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c8c46d4..67ae0738 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,10 @@ ## 1.0.0 - feat!: migrate to analyzer_server_plugin -- refactor: replace `avoid_unnecessary_type_casts` rule with dart analyzer's `unnecessary_cast` +- refactor: replace `avoid_unnecessary_type_casts` rule with dart analyzer's + `unnecessary_cast` - Added `use_nearest_context` rule. +- Added `avoid_duplicate_code` rule. ## 0.3.3 @@ -17,9 +19,9 @@ - Added `allow_with_comments` parameter for `no_empty_block` lint. - Added extension support for `avoid_using_api` -- Added `exclude_entity` parameter for `prefer_match_file_name` lint. - It is now possible to configure this lint to ignore `enum`, - `extension` and `mixin` declarations via `analysis_options.yaml`. +- Added `exclude_entity` parameter for `prefer_match_file_name` lint. It is now + possible to configure this lint to ignore `enum`, `extension` and `mixin` + declarations via `analysis_options.yaml`. ## 0.3.0 @@ -30,7 +32,8 @@ - `function_lines_of_code` - `no_empty_bloc` - `number_of_parameters` -- BREAKING CHANGE: Renamed `excludeNames` parameter to `exclude` for `function_lines_of_code` lint. +- BREAKING CHANGE: Renamed `excludeNames` parameter to `exclude` for + `function_lines_of_code` lint. - Fixed an issue with `prefer_early_retrun` for throw expression - `number_of_parameters` lint: added `copyWith` to the default exclude list. - Update dependencies: @@ -53,28 +56,36 @@ ## 0.2.0 - Added `avoid_final_with_getter` rule -- Improve `avoid_late_keyword` - `ignored_types` to support ignoring subtype of the node type () -- Abstract methods should be omitted by `proper_super_calls` () -- Add a rule prefer_guard_clause for reversing nested if statements () -- add exclude params support to avoid_returning_widgets rule () -- add quick fix to avoid_final_with_getter () +- Improve `avoid_late_keyword` - `ignored_types` to support ignoring subtype of + the node type () +- Abstract methods should be omitted by `proper_super_calls` + () +- Add a rule prefer_guard_clause for reversing nested if statements + () +- add exclude params support to avoid_returning_widgets rule + () +- add quick fix to avoid_final_with_getter + () - Renamed `avoid_debug_print` to `avoid_debug_print_in_release` -- The `avoid_debug_print_in_release` no longer reports a warning if the `debugPrint` call is wrapped in a `!kReleaseMode` check. +- The `avoid_debug_print_in_release` no longer reports a warning if the + `debugPrint` call is wrapped in a `!kReleaseMode` check. - Update custom_lints to work with newer Flutter ## 0.1.5 - Added `avoid_debug_print` rule - Fixed an issue with no_magic_number lint -- Fixed `avoid_unused_parameters` to report positional parameters from typedef if their name are not underscores. +- Fixed `avoid_unused_parameters` to report positional parameters from typedef + if their name are not underscores. - Improvement for `avoid_returning_widget` lint: - ignores methods that override ones that return widget (build() for example) - no longer allows returning widgets from methods/functions named build - Fixed unexpected avoid_unnecessary_type_assertions - Added `excludeNames` param for `function_lines_of_code` lint - Improved `avoid_unrelated_type_assertions` to support true and false results -- Set default `cyclomatic_complexity` to 10 () - Credits: Arthur Miranda () +- Set default `cyclomatic_complexity` to 10 + () Credits: Arthur + Miranda () ## 0.1.4 @@ -108,8 +119,7 @@ - avoid_unnecessary_type_casts - avoid_unrelated_type_assertions - avoid_unused_parameters - - avoid_using_api - Credits: getBoolean () + - avoid_using_api Credits: getBoolean () - cyclomatic_complexity - double_literal_format - function_lines_of_code @@ -176,7 +186,8 @@ ## 0.0.13 -- enable use_colored_box & use_decorated_box + add more comments about specific lints +- enable use_colored_box & use_decorated_box + add more comments about specific + lints ## 0.0.12 From 2faba00b0e83e8b3e28230470939e3b2a3f3c432 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 15 Jul 2026 11:38:54 +0300 Subject: [PATCH 17/28] chore: update default minimum tokens to 30 for avoid_duplicate_code lint --- .../models/avoid_duplicate_code_parameters.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart index 4a699817..5f542a91 100644 --- a/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart +++ b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart @@ -23,7 +23,7 @@ class AvoidDuplicateCodeParameters { /// A list of methods/functions that should be excluded from the lint. final ExcludedIdentifiersListParameter exclude; - static const _defaultMinTokens = 50; + static const _defaultMinTokens = 30; static final _defaultExclude = ExcludedIdentifiersListParameter( exclude: const [], From 44de4fab97eb119e1aaf7e7574453cdd1909deb4 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 15 Jul 2026 12:10:57 +0300 Subject: [PATCH 18/28] feat: add equality operators and hashCode overrides to parameter models for reliable comparison --- .../excluded_identifier_parameter.dart | 15 ++++++ .../excluded_identifiers_list_parameter.dart | 13 +++++ .../avoid_duplicate_code_parameters.dart | 4 +- .../global_hash_registry_test.dart | 49 +++++++++++++++++++ 4 files changed, 80 insertions(+), 1 deletion(-) diff --git a/lib/src/common/parameters/excluded_identifier_parameter.dart b/lib/src/common/parameters/excluded_identifier_parameter.dart index 839658f2..87bc6961 100644 --- a/lib/src/common/parameters/excluded_identifier_parameter.dart +++ b/lib/src/common/parameters/excluded_identifier_parameter.dart @@ -33,4 +33,19 @@ class ExcludedIdentifierParameter { if (className != null) 'class_name': className, if (declarationName != null) 'declaration_name': declarationName, }; + + @override + bool operator ==(Object other) => + identical(this, other) || + other is ExcludedIdentifierParameter && + other.methodName == methodName && + other.className == className && + other.declarationName == declarationName; + + @override + int get hashCode => Object.hash( + methodName, + className, + declarationName, + ); } diff --git a/lib/src/common/parameters/excluded_identifiers_list_parameter.dart b/lib/src/common/parameters/excluded_identifiers_list_parameter.dart index 484c49cc..12ca955a 100644 --- a/lib/src/common/parameters/excluded_identifiers_list_parameter.dart +++ b/lib/src/common/parameters/excluded_identifiers_list_parameter.dart @@ -88,4 +88,17 @@ class ExcludedIdentifiersListParameter { classDeclaration.namePart.typeName.lexeme == className; } } + + @override + bool operator ==(Object other) => + identical(this, other) || + other is ExcludedIdentifiersListParameter && + const ListEquality().equals( + other.exclude, + exclude, + ); + + @override + int get hashCode => + const ListEquality().hash(exclude); } diff --git a/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart index 5f542a91..371f485b 100644 --- a/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart +++ b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart @@ -74,7 +74,8 @@ class AvoidDuplicateCodeParameters { other.minTokens == minTokens && other.ignoreLiterals == ignoreLiterals && other.ignoreIdentifiers == ignoreIdentifiers && - other.checkBlocks == checkBlocks; + other.checkBlocks == checkBlocks && + other.exclude == exclude; @override int get hashCode => Object.hash( @@ -82,5 +83,6 @@ class AvoidDuplicateCodeParameters { ignoreLiterals, ignoreIdentifiers, checkBlocks, + exclude, ); } diff --git a/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart b/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart index e5b99f87..ad00b1e9 100644 --- a/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart +++ b/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart @@ -1,5 +1,7 @@ import 'dart:io'; import 'package:path/path.dart' as p; +import 'package:solid_lints/src/common/parameters/excluded_identifier_parameter.dart'; +import 'package:solid_lints/src/common/parameters/excluded_identifiers_list_parameter.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/file_cache_entry.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/hash_entry.dart'; @@ -288,5 +290,52 @@ void main() { expect(loaded, isNull); }); + + test('AvoidDuplicateCodeParameters value equality', () { + final params1 = AvoidDuplicateCodeParameters( + minTokens: 30, + ignoreLiterals: false, + ignoreIdentifiers: true, + checkBlocks: true, + exclude: ExcludedIdentifiersListParameter( + exclude: [ + const ExcludedIdentifierParameter( + methodName: 'foo', + className: 'Bar', + ), + ], + ), + ); + + final params2 = AvoidDuplicateCodeParameters( + minTokens: 30, + ignoreLiterals: false, + ignoreIdentifiers: true, + checkBlocks: true, + exclude: ExcludedIdentifiersListParameter( + exclude: [ + const ExcludedIdentifierParameter( + methodName: 'foo', + className: 'Bar', + ), + ], + ), + ); + + final paramsDifferentExclude = AvoidDuplicateCodeParameters( + minTokens: 30, + ignoreLiterals: false, + ignoreIdentifiers: true, + checkBlocks: true, + exclude: ExcludedIdentifiersListParameter( + exclude: [const ExcludedIdentifierParameter(methodName: 'different')], + ), + ); + + expect(params1, equals(params2)); + expect(params1.hashCode, equals(params2.hashCode)); + + expect(params1, isNot(equals(paramsDifferentExclude))); + }); }); } From 6b6fde655e725e983a4df1fa020de9daacbf6a39 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 15 Jul 2026 12:13:44 +0300 Subject: [PATCH 19/28] fix: handle corrupted cache files in HashCacheStorage and add regression test --- .../services/hash_cache_storage.dart | 2 +- .../global_hash_registry_test.dart | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart b/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart index e4116d6e..2069b793 100644 --- a/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart +++ b/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart @@ -60,7 +60,7 @@ class HashCacheStorage { } } return result; - } on Exception { + } catch (_) { return null; } } diff --git a/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart b/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart index ad00b1e9..80b769c5 100644 --- a/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart +++ b/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart @@ -291,6 +291,20 @@ void main() { expect(loaded, isNull); }); + test('HashCacheStorage.load returns null and does not throw when cache file is corrupted', () { + final cacheFile = File(p.join(Directory.current.path, '.dart_tool', 'solid_lints', 'duplicate_index.json')); + cacheFile.createSync(recursive: true); + cacheFile.writeAsStringSync('["invalid", "json", "structure", "not", "a", "map"]'); + + final loaded = HashCacheStorage.load( + Directory.current.path, + AvoidDuplicateCodeParameters.empty(), + ); + expect(loaded, isNull); + + HashCacheStorage.delete(Directory.current.path); + }); + test('AvoidDuplicateCodeParameters value equality', () { final params1 = AvoidDuplicateCodeParameters( minTokens: 30, From 8a766a60bb7f7d4943b863a75debdae44b40b732 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 15 Jul 2026 12:22:58 +0300 Subject: [PATCH 20/28] test: update avoid_duplicate_code_rule_test to include excluded identifiers and context root --- .../avoid_duplicate_code_rule_test.dart | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart b/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart index 161429bf..f92fbf20 100644 --- a/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart +++ b/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart @@ -1,6 +1,7 @@ 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/common/parameters/excluded_identifier_parameter.dart'; import 'package:solid_lints/src/common/parameters/excluded_identifiers_list_parameter.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart'; @@ -361,10 +362,13 @@ void otherMethod() { ignoreLiterals: false, ignoreIdentifiers: true, checkBlocks: true, - exclude: ExcludedIdentifiersListParameter(exclude: []), + exclude: ExcludedIdentifiersListParameter( + exclude: [ExcludedIdentifierParameter(methodName: 'excluded')], + ), ), filePath: otherFile.path, modificationStamp: 1, + contextRoot: resolvedOther.session.analysisContext.contextRoot, ); resolvedOther.unit.accept(visitor); From 4c88728ca8ffd7147f0122505eafbdcf5e102cfe Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 15 Jul 2026 12:38:14 +0300 Subject: [PATCH 21/28] fix: use set for deletedFiles and skip them when building the inverted index --- .../avoid_duplicate_code/services/global_hash_registry.dart | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart index 054e8226..315a45de 100644 --- a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart +++ b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart @@ -99,7 +99,7 @@ class GlobalHashRegistry { _index.addAll(cached); // Clean up files that were physically deleted once upon loading cache - final deletedFiles = []; + final deletedFiles = {}; for (final path in cached.keys) { if (enablePhysicalFileCleanup && !File(path).existsSync()) { deletedFiles.add(path); @@ -109,7 +109,8 @@ class GlobalHashRegistry { _index.remove(file); } - for (final MapEntry(:key, :value) in _index.entries) { + for (final MapEntry(:key, :value) in cached.entries) { + if (deletedFiles.contains(key)) continue; _addToInvertedIndex(key, value.entries); } From c2af6ecce21849a9f5a4b0610ee5ac6f4b1293ab Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 15 Jul 2026 12:52:40 +0300 Subject: [PATCH 22/28] fix: prevent infinite loop in token counting by returning count when a cycle is detected --- lib/src/lints/avoid_duplicate_code/utils/token_utils.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/lints/avoid_duplicate_code/utils/token_utils.dart b/lib/src/lints/avoid_duplicate_code/utils/token_utils.dart index 483d819f..d239cf2c 100644 --- a/lib/src/lints/avoid_duplicate_code/utils/token_utils.dart +++ b/lib/src/lints/avoid_duplicate_code/utils/token_utils.dart @@ -8,7 +8,7 @@ int getTokenCount(AstNode node) { final end = node.endToken; while (token != null && token != end) { count++; - if (token == token.next) break; // Prevent infinite loop if AST is cyclical + if (token == token.next) return count; token = token.next; } return count + 1; From 46ef176a94a7c8efd1c9e580a2627acb1d5c0cc9 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 15 Jul 2026 13:03:38 +0300 Subject: [PATCH 23/28] fix: improve path precision in duplicate code detection to avoid sibling directory collisions and encapsulate utility methods. --- .../services/global_hash_registry.dart | 8 +- .../services/hash_cache_storage.dart | 2 +- .../utils/context_root_extensions.dart | 3 +- .../utils/path_utils.dart | 20 +++-- .../utils/token_utils.dart | 23 ++--- .../avoid_duplicate_code_visitor.dart | 2 +- .../visitors/candidate_visitor.dart | 4 +- .../global_hash_registry_test.dart | 86 ++++++++++++++++--- 8 files changed, 110 insertions(+), 38 deletions(-) diff --git a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart index 315a45de..6159c58a 100644 --- a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart +++ b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart @@ -123,7 +123,7 @@ class GlobalHashRegistry { void _clearEntriesForRoot(String packageRoot) { final keysToRemove = []; for (final key in _index.keys) { - if (key.startsWith(packageRoot)) { + if (PathUtils.isWithinOrEqual(packageRoot, key)) { keysToRemove.add(key); } } @@ -143,7 +143,7 @@ class GlobalHashRegistry { ) { final root = _getRoot(packageRoot); _ensureLoaded(root, parameters); - return normalizePath(filePath, root); + return PathUtils.normalizePath(filePath, root); } /// Returns the cached modification stamp for [filePath], or `null` if no @@ -237,7 +237,7 @@ class GlobalHashRegistry { continue; } - if (!key.startsWith(root)) continue; + if (!PathUtils.isWithinOrEqual(root, key)) continue; // Verify tokenCount to guard against hash collisions. if (loc.entry.tokenCount != entry.tokenCount) continue; @@ -302,7 +302,7 @@ class GlobalHashRegistry { // Filter _index for files belonging to this packageRoot final subset = { for (final MapEntry(:key, :value) in _index.entries) - if (key.startsWith(packageRoot)) key: value, + if (PathUtils.isWithinOrEqual(packageRoot, key)) key: value, }; final params = diff --git a/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart b/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart index 2069b793..1246eff4 100644 --- a/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart +++ b/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart @@ -49,7 +49,7 @@ class HashCacheStorage { final result = {}; for (final MapEntry(:key, :value) in filesMap.entries) { - final absoluteKey = normalizePath(key, packageRoot); + final absoluteKey = PathUtils.normalizePath(key, packageRoot); if (value is Map) { try { diff --git a/lib/src/lints/avoid_duplicate_code/utils/context_root_extensions.dart b/lib/src/lints/avoid_duplicate_code/utils/context_root_extensions.dart index 7dc8e055..b04a0a82 100644 --- a/lib/src/lints/avoid_duplicate_code/utils/context_root_extensions.dart +++ b/lib/src/lints/avoid_duplicate_code/utils/context_root_extensions.dart @@ -1,4 +1,5 @@ import 'package:analyzer/dart/analysis/context_root.dart'; +import 'package:solid_lints/src/lints/avoid_duplicate_code/utils/path_utils.dart'; /// Extension methods for [ContextRoot] to help with file analysis checks. extension ContextRootExtensions on ContextRoot { @@ -7,7 +8,7 @@ extension ContextRootExtensions on ContextRoot { /// Returns `true` only if the file is within the context root but is /// explicitly excluded (e.g., via analysis_options.yaml). bool isFileExcluded(String filePath) { - final isWithin = filePath.startsWith(root.path); + final isWithin = PathUtils.isWithinOrEqual(root.path, filePath); return isWithin && !isAnalyzed(filePath); } } diff --git a/lib/src/lints/avoid_duplicate_code/utils/path_utils.dart b/lib/src/lints/avoid_duplicate_code/utils/path_utils.dart index 4247b571..fb93a331 100644 --- a/lib/src/lints/avoid_duplicate_code/utils/path_utils.dart +++ b/lib/src/lints/avoid_duplicate_code/utils/path_utils.dart @@ -1,9 +1,17 @@ import 'package:path/path.dart' as p; -/// Normalizes a file path, ensuring it is absolute and resolved correctly -/// relative to the given [root] if it isn't already absolute. -String normalizePath(String filePath, String root) { - return p.isAbsolute(filePath) - ? p.normalize(filePath) - : p.normalize(p.join(root, filePath)); +/// Utility methods for path manipulation and comparison. +abstract final class PathUtils { + /// Normalizes a file path, ensuring it is absolute and resolved correctly + /// relative to the given [root] if it isn't already absolute. + static String normalizePath(String filePath, String root) { + return p.isAbsolute(filePath) + ? p.normalize(filePath) + : p.normalize(p.join(root, filePath)); + } + + /// Checks if [filePath] is equal to [parentPath] or is located within it. + static bool isWithinOrEqual(String parentPath, String filePath) { + return p.equals(parentPath, filePath) || p.isWithin(parentPath, filePath); + } } diff --git a/lib/src/lints/avoid_duplicate_code/utils/token_utils.dart b/lib/src/lints/avoid_duplicate_code/utils/token_utils.dart index d239cf2c..2eae335d 100644 --- a/lib/src/lints/avoid_duplicate_code/utils/token_utils.dart +++ b/lib/src/lints/avoid_duplicate_code/utils/token_utils.dart @@ -1,15 +1,18 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/token.dart'; -/// Returns the total number of non-EOF tokens within this node. -int getTokenCount(AstNode node) { - int count = 0; - Token? token = node.beginToken; - final end = node.endToken; - while (token != null && token != end) { - count++; - if (token == token.next) return count; - token = token.next; +/// Utility methods for token manipulation. +abstract final class TokenUtils { + /// Returns the total number of non-EOF tokens within this node. + static int getTokenCount(AstNode node) { + int count = 0; + Token? token = node.beginToken; + final end = node.endToken; + while (token != null && token != end) { + count++; + if (token == token.next) return count; + token = token.next; + } + return count + 1; } - return count + 1; } diff --git a/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart index 75ad122a..cfe4c1ad 100644 --- a/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart +++ b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart @@ -73,7 +73,7 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { lineNumber: line, offset: candidate.node.offset, length: candidate.node.length, - tokenCount: getTokenCount(candidate.node), + tokenCount: TokenUtils.getTokenCount(candidate.node), ), ); } diff --git a/lib/src/lints/avoid_duplicate_code/visitors/candidate_visitor.dart b/lib/src/lints/avoid_duplicate_code/visitors/candidate_visitor.dart index fae430ab..c9d3c7f0 100644 --- a/lib/src/lints/avoid_duplicate_code/visitors/candidate_visitor.dart +++ b/lib/src/lints/avoid_duplicate_code/visitors/candidate_visitor.dart @@ -24,13 +24,13 @@ class CandidateVisitor extends RecursiveAstVisitor { return; } - _checkAndCollect(node, getTokenCount(node)); + _checkAndCollect(node, TokenUtils.getTokenCount(node)); super.visitBlock(node); } @override void visitExpressionFunctionBody(ExpressionFunctionBody node) { - _checkAndCollect(node, getTokenCount(node)); + _checkAndCollect(node, TokenUtils.getTokenCount(node)); super.visitExpressionFunctionBody(node); } diff --git a/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart b/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart index 80b769c5..5031296e 100644 --- a/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart +++ b/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart @@ -291,19 +291,31 @@ void main() { expect(loaded, isNull); }); - test('HashCacheStorage.load returns null and does not throw when cache file is corrupted', () { - final cacheFile = File(p.join(Directory.current.path, '.dart_tool', 'solid_lints', 'duplicate_index.json')); - cacheFile.createSync(recursive: true); - cacheFile.writeAsStringSync('["invalid", "json", "structure", "not", "a", "map"]'); - - final loaded = HashCacheStorage.load( - Directory.current.path, - AvoidDuplicateCodeParameters.empty(), - ); - expect(loaded, isNull); - - HashCacheStorage.delete(Directory.current.path); - }); + test( + 'HashCacheStorage.load returns null and does not throw when cache file is corrupted', + () { + final cacheFile = File( + p.join( + Directory.current.path, + '.dart_tool', + 'solid_lints', + 'duplicate_index.json', + ), + ); + cacheFile.createSync(recursive: true); + cacheFile.writeAsStringSync( + '["invalid", "json", "structure", "not", "a", "map"]', + ); + + final loaded = HashCacheStorage.load( + Directory.current.path, + AvoidDuplicateCodeParameters.empty(), + ); + expect(loaded, isNull); + + HashCacheStorage.delete(Directory.current.path); + }, + ); test('AvoidDuplicateCodeParameters value equality', () { final params1 = AvoidDuplicateCodeParameters( @@ -351,5 +363,53 @@ void main() { expect(params1, isNot(equals(paramsDifferentExclude))); }); + + test( + 'does not match or clear files from sibling directories with prefixing names', + () { + final currentRoot = Directory.current.path; + final siblingRoot = '${currentRoot}_sibling'; + final siblingFilePath = p.normalize(p.join(siblingRoot, 'file.dart')); + final projectFilePath = p.normalize(p.join(currentRoot, 'file.dart')); + + registry.updateFile(projectFilePath, [ + const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), + ], modificationStamp: 1); + + registry.updateFile(siblingFilePath, [ + const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), + ], modificationStamp: 1); + + expect(registry.fileCount, equals(2)); + + // 1. findCrossFileMatches should not find duplicate in siblingFilePath if limited to currentRoot. + final matches = registry.findCrossFileMatches(projectFilePath, [ + const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), + ], packageRoot: currentRoot); + expect(matches, isEmpty); + + // 2. clearEntriesForRoot should not clear siblingFilePath when clearing currentRoot. + final newParams = AvoidDuplicateCodeParameters( + minTokens: 40, + ignoreLiterals: false, + ignoreIdentifiers: false, + checkBlocks: true, + exclude: AvoidDuplicateCodeParameters.empty().exclude, + ); + + registry.updateFile( + projectFilePath, + [const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5)], + modificationStamp: 1, + parameters: newParams, + packageRoot: currentRoot, + ); + + expect( + registry.getFileEntries(siblingFilePath, packageRoot: siblingRoot), + isNotNull, + ); + }, + ); }); } From 5aff0b588c6b85b736d78954c7a83a6232adc686 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 15 Jul 2026 13:17:08 +0300 Subject: [PATCH 24/28] refactor: enable independent debounce logic for different package roots in GlobalHashRegistry --- .../services/global_hash_registry.dart | 15 ++++-- .../global_hash_registry_test.dart | 53 +++++++++++++++++++ 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart index 6159c58a..6d14cc68 100644 --- a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart +++ b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart @@ -38,7 +38,7 @@ class GlobalHashRegistry { final _hashToLocations = >{}; final _loadedRoots = {}; - late final _saveDebouncer = Debouncer(_saveDebounceDuration); + final _saveDebouncers = {}; AvoidDuplicateCodeParameters? _currentParams; @@ -290,9 +290,11 @@ class GlobalHashRegistry { String packageRoot, [ AvoidDuplicateCodeParameters? parameters, ]) { - _saveDebouncer.run(() { - _performSave(packageRoot, parameters); - }); + _saveDebouncers + .putIfAbsent(packageRoot, () => Debouncer(_saveDebounceDuration)) + .run(() { + _performSave(packageRoot, parameters); + }); } void _performSave( @@ -314,7 +316,10 @@ class GlobalHashRegistry { /// /// Primarily used in tests to ensure test isolation. void clear() { - _saveDebouncer.cancel(); + for (final debouncer in _saveDebouncers.values) { + debouncer.cancel(); + } + _saveDebouncers.clear(); _loadedRoots.clear(); _index.clear(); _hashToLocations.clear(); diff --git a/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart b/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart index 5031296e..6abf432c 100644 --- a/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart +++ b/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart @@ -411,5 +411,58 @@ void main() { ); }, ); + + test( + 'debounces save operations independently for different package roots', + () async { + final tempDir1 = Directory.systemTemp.createTempSync('package1_'); + final tempDir2 = Directory.systemTemp.createTempSync('package2_'); + + try { + final file1 = p.normalize(p.join(tempDir1.path, 'file.dart')); + final file2 = p.normalize(p.join(tempDir2.path, 'file.dart')); + + registry.updateFile( + file1, + [const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5)], + modificationStamp: 1, + packageRoot: tempDir1.path, + ); + + registry.updateFile( + file2, + [const HashEntry(hash: 456, lineNumber: 10, tokenCount: 5)], + modificationStamp: 1, + packageRoot: tempDir2.path, + ); + + // Wait for debounce duration (500ms + some buffer) + await Future.delayed(const Duration(milliseconds: 600)); + + // Both caches should be saved on disk + final loaded1 = HashCacheStorage.load( + tempDir1.path, + AvoidDuplicateCodeParameters.empty(), + ); + final loaded2 = HashCacheStorage.load( + tempDir2.path, + AvoidDuplicateCodeParameters.empty(), + ); + + expect(loaded1, isNotNull); + expect(loaded1!.keys.first, equals(file1)); + + expect(loaded2, isNotNull); + expect(loaded2!.keys.first, equals(file2)); + } finally { + try { + tempDir1.deleteSync(recursive: true); + } catch (_) {} + try { + tempDir2.deleteSync(recursive: true); + } catch (_) {} + } + }, + ); }); } From 3ee9826ccdc1546f77b301c03c4c82b4021cddf5 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 15 Jul 2026 13:21:43 +0300 Subject: [PATCH 25/28] fix: catch all exceptions during file operations in hash cache storage to prevent analysis crashes --- .../avoid_duplicate_code/services/hash_cache_storage.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart b/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart index 1246eff4..f075cdea 100644 --- a/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart +++ b/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart @@ -94,7 +94,7 @@ class HashCacheStorage { }; file.writeAsStringSync(jsonEncode(data)); - } on FileSystemException { + } catch (_) { // Fail silently to avoid breaking analysis server } } @@ -106,7 +106,7 @@ class HashCacheStorage { if (file.existsSync()) { file.deleteSync(); } - } on FileSystemException { + } catch (_) { // Fail silently } } From 1a62878eacba31dcae0ec00f1afd68bb9dc4985b Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 15 Jul 2026 13:28:34 +0300 Subject: [PATCH 26/28] fix: target specific DuplicateLocation for removal in global hash registry instead of filtering by path --- .../services/global_hash_registry.dart | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart index 6d14cc68..c89965db 100644 --- a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart +++ b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart @@ -69,7 +69,12 @@ class GlobalHashRegistry { for (final entry in entries) { final set = _hashToLocations[entry.hash]; if (set != null) { - set.removeWhere((loc) => loc.filePath == absoluteFilePath); + set.remove( + DuplicateLocation( + entry: entry, + filePath: absoluteFilePath, + ), + ); if (set.isEmpty) { _hashToLocations.remove(entry.hash); } From 89b205124b790147b26dadb14e25694d4b44bc7a Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 15 Jul 2026 13:38:06 +0300 Subject: [PATCH 27/28] fix: resolve package-specific parameter retrieval during debounced cache saves in multi-package workspaces --- .../services/global_hash_registry.dart | 7 +- .../global_hash_registry_test.dart | 72 +++++++++++++++++++ 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart index c89965db..6dc5f751 100644 --- a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart +++ b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart @@ -40,8 +40,6 @@ class GlobalHashRegistry { final _loadedRoots = {}; final _saveDebouncers = {}; - AvoidDuplicateCodeParameters? _currentParams; - GlobalHashRegistry._(); /// The number of files currently indexed. @@ -98,7 +96,6 @@ class GlobalHashRegistry { } _loadedRoots[packageRoot] = params; - _currentParams = params; final cached = HashCacheStorage.load(packageRoot, params); if (cached != null) { _index.addAll(cached); @@ -313,7 +310,9 @@ class GlobalHashRegistry { }; final params = - parameters ?? _currentParams ?? AvoidDuplicateCodeParameters.empty(); + parameters ?? + _loadedRoots[packageRoot] ?? + AvoidDuplicateCodeParameters.empty(); HashCacheStorage.save(packageRoot, subset, params); } diff --git a/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart b/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart index 6abf432c..0ae3902e 100644 --- a/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart +++ b/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart @@ -1,3 +1,4 @@ +import 'dart:convert'; import 'dart:io'; import 'package:path/path.dart' as p; import 'package:solid_lints/src/common/parameters/excluded_identifier_parameter.dart'; @@ -464,5 +465,76 @@ void main() { } }, ); + + test('uses correct package-specific parameters during debounced save ' + 'in multi-package workspace', () async { + final tempDir1 = Directory.systemTemp.createTempSync('package1_'); + final tempDir2 = Directory.systemTemp.createTempSync('package2_'); + + try { + final file1 = p.normalize(p.join(tempDir1.path, 'file.dart')); + final file2 = p.normalize(p.join(tempDir2.path, 'file.dart')); + + final params1 = AvoidDuplicateCodeParameters( + minTokens: 30, + ignoreLiterals: false, + ignoreIdentifiers: false, + checkBlocks: true, + exclude: AvoidDuplicateCodeParameters.empty().exclude, + ); + + final params2 = AvoidDuplicateCodeParameters( + minTokens: 40, + ignoreLiterals: false, + ignoreIdentifiers: false, + checkBlocks: true, + exclude: AvoidDuplicateCodeParameters.empty().exclude, + ); + + registry.updateFile( + file1, + [const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5)], + modificationStamp: 1, + parameters: params1, + packageRoot: tempDir1.path, + ); + + registry.updateFile( + file2, + [const HashEntry(hash: 456, lineNumber: 10, tokenCount: 5)], + modificationStamp: 1, + parameters: params2, + packageRoot: tempDir2.path, + ); + + // Wait for debounce duration (500ms + some buffer) + await Future.delayed(const Duration(milliseconds: 600)); + + final cacheFile1 = File( + p.join(tempDir1.path, '.dart_tool/solid_lints/duplicate_index.json'), + ); + final cacheFile2 = File( + p.join(tempDir2.path, '.dart_tool/solid_lints/duplicate_index.json'), + ); + + expect(cacheFile1.existsSync(), isTrue); + expect(cacheFile2.existsSync(), isTrue); + + final content1 = + jsonDecode(cacheFile1.readAsStringSync()) as Map; + final content2 = + jsonDecode(cacheFile2.readAsStringSync()) as Map; + + expect(content1['config']?['min_tokens'], equals(30)); + expect(content2['config']?['min_tokens'], equals(40)); + } finally { + try { + tempDir1.deleteSync(recursive: true); + } catch (_) {} + try { + tempDir2.deleteSync(recursive: true); + } catch (_) {} + } + }); }); } From c9f6df2bf6b9562dd1994cd5bf27bfeaa3035255 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 15 Jul 2026 13:47:27 +0300 Subject: [PATCH 28/28] refactor: inject ResourceProvider into avoid_duplicate_code services to support testability --- .../avoid_duplicate_code_rule.dart | 1 + .../services/global_hash_registry.dart | 23 +- .../services/hash_cache_storage.dart | 54 ++- .../avoid_duplicate_code_visitor.dart | 22 +- .../avoid_duplicate_code_rule_test.dart | 1 + .../global_hash_registry_test.dart | 317 ++++++++++-------- 6 files changed, 240 insertions(+), 178 deletions(-) diff --git a/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart b/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart index 1e83660a..688a31ad 100644 --- a/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart +++ b/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart @@ -73,6 +73,7 @@ class AvoidDuplicateCodeRule filePath: context.definingUnit.file.path, modificationStamp: context.definingUnit.file.modificationStamp, contextRoot: context.libraryElement?.session.analysisContext.contextRoot, + resourceProvider: context.definingUnit.file.provider, ); registry.addCompilationUnit(this, visitor); diff --git a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart index 6dc5f751..08ff0407 100644 --- a/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart +++ b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart @@ -1,4 +1,7 @@ -import 'dart:io'; +import 'dart:io' as io; + +import 'package:analyzer/file_system/file_system.dart'; +import 'package:analyzer/file_system/physical_file_system.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/cross_file_match.dart'; @@ -31,6 +34,9 @@ class GlobalHashRegistry { /// Set to `false` in tests using a virtual resource provider. bool enablePhysicalFileCleanup = true; + /// The resource provider used for file system operations. + ResourceProvider resourceProvider = PhysicalResourceProvider.INSTANCE; + /// Internal index: filePath → FileCacheEntry. final _index = {}; @@ -45,7 +51,8 @@ class GlobalHashRegistry { /// The number of files currently indexed. int get fileCount => _index.length; - String _getRoot(String? packageRoot) => packageRoot ?? Directory.current.path; + String _getRoot(String? packageRoot) => + packageRoot ?? io.Directory.current.path; void _addToInvertedIndex(String absoluteFilePath, List entries) { for (final entry in entries) { @@ -96,14 +103,15 @@ class GlobalHashRegistry { } _loadedRoots[packageRoot] = params; - final cached = HashCacheStorage.load(packageRoot, params); + final cached = HashCacheStorage.load(packageRoot, params, resourceProvider); if (cached != null) { _index.addAll(cached); // Clean up files that were physically deleted once upon loading cache final deletedFiles = {}; for (final path in cached.keys) { - if (enablePhysicalFileCleanup && !File(path).existsSync()) { + if (enablePhysicalFileCleanup && + !resourceProvider.getFile(path).exists) { deletedFiles.add(path); } } @@ -228,7 +236,8 @@ class GlobalHashRegistry { bool? isInvalid = fileStatusCache[key]; if (isInvalid == null) { final isDeleted = - enablePhysicalFileCleanup && !File(key).existsSync(); + enablePhysicalFileCleanup && + !resourceProvider.getFile(key).exists; final isExcluded = isFileExcluded != null && isFileExcluded(key); isInvalid = isDeleted || isExcluded; fileStatusCache[key] = isInvalid; @@ -313,7 +322,7 @@ class GlobalHashRegistry { parameters ?? _loadedRoots[packageRoot] ?? AvoidDuplicateCodeParameters.empty(); - HashCacheStorage.save(packageRoot, subset, params); + HashCacheStorage.save(packageRoot, subset, params, resourceProvider); } /// Clears the entire index and deletes the persistent cache file. @@ -327,7 +336,7 @@ class GlobalHashRegistry { _loadedRoots.clear(); _index.clear(); _hashToLocations.clear(); - HashCacheStorage.delete(Directory.current.path); + HashCacheStorage.delete(io.Directory.current.path, resourceProvider); AvoidDuplicateCodeVisitor.clearPackageRootCache(); } } diff --git a/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart b/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart index f075cdea..97465d83 100644 --- a/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart +++ b/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart @@ -1,19 +1,25 @@ import 'dart:convert'; -import 'dart:io'; + +import 'package:analyzer/file_system/file_system.dart'; import 'package:collection/collection.dart'; -import 'package:path/path.dart' as p; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/file_cache_entry.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/utils/path_utils.dart'; /// Service responsible for persisting and loading the duplicate code hash /// cache. -class HashCacheStorage { +abstract final class HashCacheStorage { static const _cacheDirName = '.dart_tool/solid_lints'; static const _cacheFileName = 'duplicate_index.json'; - static String _filePath(String packageRoot) => - p.join(packageRoot, _cacheDirName, _cacheFileName); + static String _filePath( + String packageRoot, + ResourceProvider resourceProvider, + ) => resourceProvider.pathContext.join( + packageRoot, + _cacheDirName, + _cacheFileName, + ); /// Loads the cached index from disk for the given [packageRoot]. /// @@ -22,10 +28,13 @@ class HashCacheStorage { static Map? load( String packageRoot, AvoidDuplicateCodeParameters currentParams, + ResourceProvider resourceProvider, ) { try { - final file = File(_filePath(packageRoot)); - if (!file.existsSync()) return null; + final file = resourceProvider.getFile( + _filePath(packageRoot, resourceProvider), + ); + if (!file.exists) return null; final content = file.readAsStringSync(); final jsonMap = jsonDecode(content) as Map; @@ -70,19 +79,25 @@ class HashCacheStorage { String packageRoot, Map index, AvoidDuplicateCodeParameters currentParams, + ResourceProvider resourceProvider, ) { try { - final directory = Directory(p.join(packageRoot, _cacheDirName)); - if (!directory.existsSync()) { - directory.createSync(recursive: true); + final pathContext = resourceProvider.pathContext; + final directory = resourceProvider.getFolder( + pathContext.join(packageRoot, _cacheDirName), + ); + if (!directory.exists) { + directory.create(); } - final file = File(_filePath(packageRoot)); + final file = resourceProvider.getFile( + _filePath(packageRoot, resourceProvider), + ); final filesMap = {}; for (final entry in index.entries) { - final relativeKey = p.isAbsolute(entry.key) - ? p.relative(entry.key, from: packageRoot) + final relativeKey = pathContext.isAbsolute(entry.key) + ? pathContext.relative(entry.key, from: packageRoot) : entry.key; filesMap[relativeKey] = entry.value.toJson(); } @@ -100,11 +115,16 @@ class HashCacheStorage { } /// Deletes the cache file for [packageRoot] if it exists. - static void delete(String packageRoot) { + static void delete( + String packageRoot, + ResourceProvider resourceProvider, + ) { try { - final file = File(_filePath(packageRoot)); - if (file.existsSync()) { - file.deleteSync(); + final file = resourceProvider.getFile( + _filePath(packageRoot, resourceProvider), + ); + if (file.exists) { + file.delete(); } } catch (_) { // Fail silently diff --git a/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart index cfe4c1ad..853916c6 100644 --- a/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart +++ b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart @@ -1,10 +1,9 @@ -import 'dart:io'; - import 'package:analyzer/dart/analysis/context_root.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/diagnostic/diagnostic.dart'; -import 'package:path/path.dart' as path; +import 'package:analyzer/file_system/file_system.dart'; +import 'package:analyzer/file_system/physical_file_system.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart'; import 'package:solid_lints/src/lints/avoid_duplicate_code/models/body_candidate.dart'; @@ -29,6 +28,7 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { final String _filePath; final int _modificationStamp; final ContextRoot? _contextRoot; + final ResourceProvider _resourceProvider; /// Creates a new instance of [AvoidDuplicateCodeVisitor]. AvoidDuplicateCodeVisitor( @@ -37,15 +37,20 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { required String filePath, required int modificationStamp, ContextRoot? contextRoot, + ResourceProvider? resourceProvider, }) : _filePath = filePath, _modificationStamp = modificationStamp, - _contextRoot = contextRoot; + _contextRoot = contextRoot, + _resourceProvider = + resourceProvider ?? PhysicalResourceProvider.INSTANCE; @override void visitCompilationUnit(CompilationUnit node) { final filePath = _filePath; if (filePath.isEmpty) return; + GlobalHashRegistry.instance.resourceProvider = _resourceProvider; + final packageRoot = _contextRoot?.root.path ?? _findPackageRoot(filePath) ?? ''; @@ -308,12 +313,13 @@ class AvoidDuplicateCodeVisitor extends RecursiveAstVisitor { String? _findPackageRoot(String filePath) { if (filePath.isEmpty) return null; - final dirPath = path.dirname(filePath); + final pathContext = _resourceProvider.pathContext; + final dirPath = pathContext.dirname(filePath); return _packageRootCache.putIfAbsent(dirPath, () { - var dir = Directory(dirPath); + var dir = _resourceProvider.getFolder(dirPath); while (true) { - final pubspec = File(path.join(dir.path, 'pubspec.yaml')); - if (pubspec.existsSync()) { + final pubspec = dir.getChildAssumingFile('pubspec.yaml'); + if (pubspec.exists) { return dir.path; } final parent = dir.parent; diff --git a/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart b/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart index f92fbf20..5a66fcc5 100644 --- a/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart +++ b/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart @@ -369,6 +369,7 @@ void otherMethod() { filePath: otherFile.path, modificationStamp: 1, contextRoot: resolvedOther.session.analysisContext.contextRoot, + resourceProvider: resourceProvider, ); resolvedOther.unit.accept(visitor); diff --git a/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart b/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart index 0ae3902e..7351ca8d 100644 --- a/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart +++ b/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart @@ -1,5 +1,8 @@ import 'dart:convert'; -import 'dart:io'; +import 'dart:io' as io; + +import 'package:analyzer/file_system/memory_file_system.dart'; +import 'package:analyzer/file_system/physical_file_system.dart'; import 'package:path/path.dart' as p; import 'package:solid_lints/src/common/parameters/excluded_identifier_parameter.dart'; import 'package:solid_lints/src/common/parameters/excluded_identifiers_list_parameter.dart'; @@ -13,15 +16,19 @@ import 'package:test/test.dart'; void main() { group('GlobalHashRegistry', () { late GlobalHashRegistry registry; + late MemoryResourceProvider memoryResourceProvider; setUp(() { + memoryResourceProvider = MemoryResourceProvider(); registry = GlobalHashRegistry.instance; + registry.resourceProvider = memoryResourceProvider; registry.clear(); registry.enablePhysicalFileCleanup = false; }); tearDown(() { registry.clear(); + registry.resourceProvider = PhysicalResourceProvider.INSTANCE; registry.enablePhysicalFileCleanup = true; }); @@ -54,7 +61,7 @@ void main() { expect(matches, hasLength(1)); expect(matches.first.duplicates, hasLength(1)); final expectedPath = p.normalize( - p.join(Directory.current.path, 'file_a.dart'), + p.join(io.Directory.current.path, 'file_a.dart'), ); expect(matches.first.duplicates.first.filePath, equals(expectedPath)); expect(matches.first.duplicates.first.entry.hash, equals(123)); @@ -145,10 +152,10 @@ void main() { expect(matches, hasLength(1)); expect(matches.first.duplicates, hasLength(2)); final expectedPathA = p.normalize( - p.join(Directory.current.path, 'file_a.dart'), + p.join(io.Directory.current.path, 'file_a.dart'), ); final expectedPathB = p.normalize( - p.join(Directory.current.path, 'file_b.dart'), + p.join(io.Directory.current.path, 'file_b.dart'), ); expect(matches.first.duplicates[0].filePath, equals(expectedPathA)); expect(matches.first.duplicates[1].filePath, equals(expectedPathB)); @@ -156,7 +163,7 @@ void main() { test('HashCacheStorage saves and loads index', () { final absoluteFilePath = p.normalize( - p.join(Directory.current.path, 'file_a.dart'), + p.join(io.Directory.current.path, 'file_a.dart'), ); final index = { absoluteFilePath: const FileCacheEntry( @@ -166,14 +173,16 @@ void main() { }; HashCacheStorage.save( - Directory.current.path, + io.Directory.current.path, index, AvoidDuplicateCodeParameters.empty(), + memoryResourceProvider, ); final loaded = HashCacheStorage.load( - Directory.current.path, + io.Directory.current.path, AvoidDuplicateCodeParameters.empty(), + memoryResourceProvider, ); expect(loaded, isNotNull); expect(loaded!.length, equals(1)); @@ -185,23 +194,28 @@ void main() { expect(entry.lineNumber, equals(10)); expect(entry.tokenCount, equals(5)); - HashCacheStorage.delete(Directory.current.path); + HashCacheStorage.delete( + io.Directory.current.path, + memoryResourceProvider, + ); }); test('findCrossFileMatches cleans up absolute paths of deleted files', () { registry.enablePhysicalFileCleanup = true; - final tempFile = File('${Directory.systemTemp.path}/temp_test_file.dart'); - tempFile.writeAsStringSync('void main() {}'); + final tempPath = p.normalize( + p.join(io.Directory.systemTemp.path, 'temp_test_file.dart'), + ); + memoryResourceProvider.newFile(tempPath, 'void main() {}'); - registry.updateFile(tempFile.path, [ + registry.updateFile(tempPath, [ const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), ], modificationStamp: 1); expect(registry.fileCount, equals(1)); - // Delete the file physically - tempFile.deleteSync(); + // Delete the file from the memory resource provider + memoryResourceProvider.deleteFile(tempPath); - // Trigger matching, which should clean up the deleted tempFile + // Trigger matching, which should clean up the deleted tempPath // from registry final matches = registry.findCrossFileMatches('other_file.dart', [ const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), @@ -233,7 +247,7 @@ void main() { test('HashCacheStorage invalidates cache on config change', () { final absoluteFilePath = p.normalize( - p.join(Directory.current.path, 'file.dart'), + p.join(io.Directory.current.path, 'file.dart'), ); final index = { absoluteFilePath: const FileCacheEntry( @@ -252,18 +266,34 @@ void main() { ); // Save with params1 - HashCacheStorage.save(Directory.current.path, index, params1); + HashCacheStorage.save( + io.Directory.current.path, + index, + params1, + memoryResourceProvider, + ); // Loading with params1 should succeed - final loaded1 = HashCacheStorage.load(Directory.current.path, params1); + final loaded1 = HashCacheStorage.load( + io.Directory.current.path, + params1, + memoryResourceProvider, + ); expect(loaded1, isNotNull); // Loading with params2 (different config) should return null // (invalidated) - final loaded2 = HashCacheStorage.load(Directory.current.path, params2); + final loaded2 = HashCacheStorage.load( + io.Directory.current.path, + params2, + memoryResourceProvider, + ); expect(loaded2, isNull); - HashCacheStorage.delete(Directory.current.path); + HashCacheStorage.delete( + io.Directory.current.path, + memoryResourceProvider, + ); }); test('findCrossFileMatches processes multiple candidates correctly', () { @@ -282,41 +312,47 @@ void main() { test('HashCacheStorage.load returns null when cache file is missing', () { // Ensure any existing cache is deleted - HashCacheStorage.delete(Directory.current.path); + HashCacheStorage.delete( + io.Directory.current.path, + memoryResourceProvider, + ); final loaded = HashCacheStorage.load( - Directory.current.path, + io.Directory.current.path, AvoidDuplicateCodeParameters.empty(), + memoryResourceProvider, ); expect(loaded, isNull); }); - test( - 'HashCacheStorage.load returns null and does not throw when cache file is corrupted', - () { - final cacheFile = File( - p.join( - Directory.current.path, - '.dart_tool', - 'solid_lints', - 'duplicate_index.json', - ), - ); - cacheFile.createSync(recursive: true); - cacheFile.writeAsStringSync( - '["invalid", "json", "structure", "not", "a", "map"]', - ); + test('HashCacheStorage.load returns null and does not throw when cache ' + 'file is corrupted', () { + final cachePath = p.normalize( + p.join( + io.Directory.current.path, + '.dart_tool', + 'solid_lints', + 'duplicate_index.json', + ), + ); + memoryResourceProvider.newFile( + cachePath, + '["invalid", "json", "structure", "not", "a", "map"]', + ); - final loaded = HashCacheStorage.load( - Directory.current.path, - AvoidDuplicateCodeParameters.empty(), - ); - expect(loaded, isNull); + final loaded = HashCacheStorage.load( + io.Directory.current.path, + AvoidDuplicateCodeParameters.empty(), + memoryResourceProvider, + ); + expect(loaded, isNull); - HashCacheStorage.delete(Directory.current.path); - }, - ); + HashCacheStorage.delete( + io.Directory.current.path, + memoryResourceProvider, + ); + }); test('AvoidDuplicateCodeParameters value equality', () { final params1 = AvoidDuplicateCodeParameters( @@ -368,7 +404,7 @@ void main() { test( 'does not match or clear files from sibling directories with prefixing names', () { - final currentRoot = Directory.current.path; + final currentRoot = io.Directory.current.path; final siblingRoot = '${currentRoot}_sibling'; final siblingFilePath = p.normalize(p.join(siblingRoot, 'file.dart')); final projectFilePath = p.normalize(p.join(currentRoot, 'file.dart')); @@ -383,13 +419,15 @@ void main() { expect(registry.fileCount, equals(2)); - // 1. findCrossFileMatches should not find duplicate in siblingFilePath if limited to currentRoot. + // 1. findCrossFileMatches should not find duplicate in siblingFilePath + // if limited to currentRoot. final matches = registry.findCrossFileMatches(projectFilePath, [ const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), ], packageRoot: currentRoot); expect(matches, isEmpty); - // 2. clearEntriesForRoot should not clear siblingFilePath when clearing currentRoot. + // 2. clearEntriesForRoot should not clear siblingFilePath when + // clearing currentRoot. final newParams = AvoidDuplicateCodeParameters( minTokens: 40, ignoreLiterals: false, @@ -416,125 +454,112 @@ void main() { test( 'debounces save operations independently for different package roots', () async { - final tempDir1 = Directory.systemTemp.createTempSync('package1_'); - final tempDir2 = Directory.systemTemp.createTempSync('package2_'); - - try { - final file1 = p.normalize(p.join(tempDir1.path, 'file.dart')); - final file2 = p.normalize(p.join(tempDir2.path, 'file.dart')); - - registry.updateFile( - file1, - [const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5)], - modificationStamp: 1, - packageRoot: tempDir1.path, - ); - - registry.updateFile( - file2, - [const HashEntry(hash: 456, lineNumber: 10, tokenCount: 5)], - modificationStamp: 1, - packageRoot: tempDir2.path, - ); - - // Wait for debounce duration (500ms + some buffer) - await Future.delayed(const Duration(milliseconds: 600)); - - // Both caches should be saved on disk - final loaded1 = HashCacheStorage.load( - tempDir1.path, - AvoidDuplicateCodeParameters.empty(), - ); - final loaded2 = HashCacheStorage.load( - tempDir2.path, - AvoidDuplicateCodeParameters.empty(), - ); - - expect(loaded1, isNotNull); - expect(loaded1!.keys.first, equals(file1)); - - expect(loaded2, isNotNull); - expect(loaded2!.keys.first, equals(file2)); - } finally { - try { - tempDir1.deleteSync(recursive: true); - } catch (_) {} - try { - tempDir2.deleteSync(recursive: true); - } catch (_) {} - } - }, - ); - - test('uses correct package-specific parameters during debounced save ' - 'in multi-package workspace', () async { - final tempDir1 = Directory.systemTemp.createTempSync('package1_'); - final tempDir2 = Directory.systemTemp.createTempSync('package2_'); - - try { - final file1 = p.normalize(p.join(tempDir1.path, 'file.dart')); - final file2 = p.normalize(p.join(tempDir2.path, 'file.dart')); + final tempDir1 = '/temp/package1'; + final tempDir2 = '/temp/package2'; - final params1 = AvoidDuplicateCodeParameters( - minTokens: 30, - ignoreLiterals: false, - ignoreIdentifiers: false, - checkBlocks: true, - exclude: AvoidDuplicateCodeParameters.empty().exclude, - ); - - final params2 = AvoidDuplicateCodeParameters( - minTokens: 40, - ignoreLiterals: false, - ignoreIdentifiers: false, - checkBlocks: true, - exclude: AvoidDuplicateCodeParameters.empty().exclude, - ); + final file1 = p.normalize(p.join(tempDir1, 'file.dart')); + final file2 = p.normalize(p.join(tempDir2, 'file.dart')); registry.updateFile( file1, [const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5)], modificationStamp: 1, - parameters: params1, - packageRoot: tempDir1.path, + packageRoot: tempDir1, ); registry.updateFile( file2, [const HashEntry(hash: 456, lineNumber: 10, tokenCount: 5)], modificationStamp: 1, - parameters: params2, - packageRoot: tempDir2.path, + packageRoot: tempDir2, ); // Wait for debounce duration (500ms + some buffer) await Future.delayed(const Duration(milliseconds: 600)); - final cacheFile1 = File( - p.join(tempDir1.path, '.dart_tool/solid_lints/duplicate_index.json'), + // Both caches should be saved on disk + final loaded1 = HashCacheStorage.load( + tempDir1, + AvoidDuplicateCodeParameters.empty(), + memoryResourceProvider, ); - final cacheFile2 = File( - p.join(tempDir2.path, '.dart_tool/solid_lints/duplicate_index.json'), + final loaded2 = HashCacheStorage.load( + tempDir2, + AvoidDuplicateCodeParameters.empty(), + memoryResourceProvider, ); - expect(cacheFile1.existsSync(), isTrue); - expect(cacheFile2.existsSync(), isTrue); - - final content1 = - jsonDecode(cacheFile1.readAsStringSync()) as Map; - final content2 = - jsonDecode(cacheFile2.readAsStringSync()) as Map; - - expect(content1['config']?['min_tokens'], equals(30)); - expect(content2['config']?['min_tokens'], equals(40)); - } finally { - try { - tempDir1.deleteSync(recursive: true); - } catch (_) {} - try { - tempDir2.deleteSync(recursive: true); - } catch (_) {} - } + expect(loaded1, isNotNull); + expect(loaded1!.keys.first, equals(file1)); + + expect(loaded2, isNotNull); + expect(loaded2!.keys.first, equals(file2)); + }, + ); + + test('uses correct package-specific parameters during debounced save ' + 'in multi-package workspace', () async { + final tempDir1 = '/temp/package1'; + final tempDir2 = '/temp/package2'; + + final file1 = p.normalize(p.join(tempDir1, 'file.dart')); + final file2 = p.normalize(p.join(tempDir2, 'file.dart')); + + final params1 = AvoidDuplicateCodeParameters( + minTokens: 30, + ignoreLiterals: false, + ignoreIdentifiers: false, + checkBlocks: true, + exclude: AvoidDuplicateCodeParameters.empty().exclude, + ); + + final params2 = AvoidDuplicateCodeParameters( + minTokens: 40, + ignoreLiterals: false, + ignoreIdentifiers: false, + checkBlocks: true, + exclude: AvoidDuplicateCodeParameters.empty().exclude, + ); + + registry.updateFile( + file1, + [const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5)], + modificationStamp: 1, + parameters: params1, + packageRoot: tempDir1, + ); + + registry.updateFile( + file2, + [const HashEntry(hash: 456, lineNumber: 10, tokenCount: 5)], + modificationStamp: 1, + parameters: params2, + packageRoot: tempDir2, + ); + + // Wait for debounce duration (500ms + some buffer) + await Future.delayed(const Duration(milliseconds: 600)); + + final cachePath1 = p.normalize( + p.join(tempDir1, '.dart_tool/solid_lints/duplicate_index.json'), + ); + final cachePath2 = p.normalize( + p.join(tempDir2, '.dart_tool/solid_lints/duplicate_index.json'), + ); + + final cacheFile1 = memoryResourceProvider.getFile(cachePath1); + final cacheFile2 = memoryResourceProvider.getFile(cachePath2); + + expect(cacheFile1.exists, isTrue); + expect(cacheFile2.exists, isTrue); + + final content1 = + jsonDecode(cacheFile1.readAsStringSync()) as Map; + final content2 = + jsonDecode(cacheFile2.readAsStringSync()) as Map; + + expect(content1['config']?['min_tokens'], equals(30)); + expect(content2['config']?['min_tokens'], equals(40)); }); }); }