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 diff --git a/lib/analysis_options.yaml b/lib/analysis_options.yaml index dd2a9c95..0517da06 100644 --- a/lib/analysis_options.yaml +++ b/lib/analysis_options.yaml @@ -1,6 +1,4 @@ analyzer: - plugins: - - custom_lint exclude: # General generated files - "**/*.g.dart" @@ -38,6 +36,12 @@ analyzer: solid_lints: diagnostics: avoid_global_state: true + avoid_duplicate_code: + min_tokens: 30 + check_blocks: true + exclude: + - method_name: initState + - method_name: dispose 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/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/common/parameters/excluded_identifier_parameter.dart b/lib/src/common/parameters/excluded_identifier_parameter.dart index f55f9704..87bc6961 100644 --- a/lib/src/common/parameters/excluded_identifier_parameter.dart +++ b/lib/src/common/parameters/excluded_identifier_parameter.dart @@ -21,8 +21,31 @@ 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, + }; + + @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/avoid_duplicate_code_rule.dart b/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart new file mode 100644 index 00000000..688a31ad --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule.dart @@ -0,0 +1,81 @@ +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 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 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 +/// session. The detection improves as more files are analyzed. +/// +/// ### Example config: +/// +/// ```yaml +/// plugins: +/// solid_lints: +/// diagnostics: +/// avoid_duplicate_code: +/// min_tokens: 50 +/// 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, + 'Perhaps this code is a duplicate.\n' + '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 and across files (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, + 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/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..371f485b --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart @@ -0,0 +1,88 @@ +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 tokens in a function body or block to be considered + /// for clone detection. Bodies/blocks shorter than this are ignored. + final int minTokens; + + /// When `true`, literal values (strings, numbers, booleans) are excluded + /// from the structural hash, making the check ignore literal differences. + final bool ignoreLiterals; + + /// When `true`, local variable and parameter names are excluded + /// from the structural hash, allowing detection of renamed variables + /// (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 + /// 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 _defaultMinTokens = 30; + + static final _defaultExclude = ExcludedIdentifiersListParameter( + exclude: const [], + ); + + /// Constructor for [AvoidDuplicateCodeParameters] model. + const AvoidDuplicateCodeParameters({ + required this.minTokens, + required this.ignoreLiterals, + required this.ignoreIdentifiers, + required this.checkBlocks, + required this.exclude, + }); + + /// Empty [AvoidDuplicateCodeParameters] model with default values. + factory AvoidDuplicateCodeParameters.empty() => AvoidDuplicateCodeParameters( + minTokens: _defaultMinTokens, + ignoreLiterals: false, + ignoreIdentifiers: true, + checkBlocks: true, + exclude: _defaultExclude, + ); + + /// Creates parameters from JSON configuration. + factory AvoidDuplicateCodeParameters.fromJson(Map json) { + return 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? ?? true, + exclude: ExcludedIdentifiersListParameter.defaultFromJson(json), + ); + } + + /// 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(), + }; + + @override + bool operator ==(Object other) => + identical(this, other) || + other is AvoidDuplicateCodeParameters && + other.minTokens == minTokens && + other.ignoreLiterals == ignoreLiterals && + other.ignoreIdentifiers == ignoreIdentifiers && + other.checkBlocks == checkBlocks && + other.exclude == exclude; + + @override + int get hashCode => Object.hash( + minTokens, + ignoreLiterals, + ignoreIdentifiers, + checkBlocks, + exclude, + ); +} 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 new file mode 100644 index 00000000..2e7c9b3c --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/models/cross_file_match.dart @@ -0,0 +1,18 @@ +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 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/duplicate_location.dart b/lib/src/lints/avoid_duplicate_code/models/duplicate_location.dart new file mode 100644 index 00000000..d4c90cd2 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/models/duplicate_location.dart @@ -0,0 +1,27 @@ +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, + }); + + @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/models/file_cache_entry.dart b/lib/src/lints/avoid_duplicate_code/models/file_cache_entry.dart new file mode 100644 index 00000000..65f21cc2 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/models/file_cache_entry.dart @@ -0,0 +1,42 @@ +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 = []; + final eValue = json['e']; + if (eValue is List) { + for (final item in eValue) { + 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..8f265e14 --- /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 tokens in the candidate body. + final int tokenCount; + + /// Creates a new [HashEntry]. + const HashEntry({ + required this.hash, + required this.lineNumber, + 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, + 't': tokenCount, + }; + + /// Creates a [HashEntry] from a JSON map. + factory HashEntry.fromJson(Map json) => HashEntry( + 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 new file mode 100644 index 00000000..08ff0407 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart @@ -0,0 +1,342 @@ +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'; +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'; +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. +/// +/// 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 { + 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; + + /// The resource provider used for file system operations. + ResourceProvider resourceProvider = PhysicalResourceProvider.INSTANCE; + + /// Internal index: filePath → FileCacheEntry. + final _index = {}; + + /// Auxiliary inverted index: hash → Set. + final _hashToLocations = >{}; + + final _loadedRoots = {}; + final _saveDebouncers = {}; + + GlobalHashRegistry._(); + + /// The number of files currently indexed. + int get fileCount => _index.length; + + String _getRoot(String? packageRoot) => + packageRoot ?? io.Directory.current.path; + + 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.remove( + DuplicateLocation( + entry: entry, + filePath: absoluteFilePath, + ), + ); + if (set.isEmpty) { + _hashToLocations.remove(entry.hash); + } + } + } + } + + void _ensureLoaded( + String packageRoot, [ + AvoidDuplicateCodeParameters? currentParams, + ]) { + 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; + 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 && + !resourceProvider.getFile(path).exists) { + deletedFiles.add(path); + } + } + for (final file in deletedFiles) { + _index.remove(file); + } + + for (final MapEntry(:key, :value) in cached.entries) { + if (deletedFiles.contains(key)) continue; + _addToInvertedIndex(key, value.entries); + } + + if (deletedFiles.isNotEmpty) { + _scheduleSave(packageRoot, params); + } + } + } + + void _clearEntriesForRoot(String packageRoot) { + final keysToRemove = []; + for (final key in _index.keys) { + if (PathUtils.isWithinOrEqual(packageRoot, key)) { + 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, + AvoidDuplicateCodeParameters? parameters, + ) { + final root = _getRoot(packageRoot); + _ensureLoaded(root, parameters); + return PathUtils.normalizePath(filePath, root); + } + + /// Returns the cached modification stamp for [filePath], or `null` if no + /// indexed. + int? getModificationStamp( + String filePath, { + AvoidDuplicateCodeParameters? parameters, + String? packageRoot, + }) { + final absoluteFilePath = _resolveAndLoad(filePath, packageRoot, parameters); + return _index[absoluteFilePath]?.modificationStamp; + } + + /// Returns the cached entries for [filePath], or `null` if not indexed. + List? getFileEntries( + String filePath, { + AvoidDuplicateCodeParameters? parameters, + String? packageRoot, + }) { + final absoluteFilePath = _resolveAndLoad(filePath, packageRoot, parameters); + return _index[absoluteFilePath]?.entries; + } + + /// Updates the hash entries for [filePath], replacing any previous entries. + void updateFile( + String filePath, + List entries, { + required int modificationStamp, + AvoidDuplicateCodeParameters? parameters, + String? packageRoot, + }) { + final root = _getRoot(packageRoot); + final absoluteFilePath = _resolveAndLoad(filePath, packageRoot, parameters); + + final oldEntry = _index[absoluteFilePath]; + if (oldEntry != null) { + _removeFromInvertedIndex(absoluteFilePath, oldEntry.entries); + } + + _index[absoluteFilePath] = FileCacheEntry( + modificationStamp: modificationStamp, + entries: entries, + ); + _addToInvertedIndex(absoluteFilePath, entries); + _scheduleSave(root, parameters); + } + + /// 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, { + AvoidDuplicateCodeParameters? parameters, + bool Function(String filePath)? isFileExcluded, + String? packageRoot, + }) { + final root = _getRoot(packageRoot); + final absoluteCurrentFilePath = _resolveAndLoad( + currentFilePath, + packageRoot, + parameters, + ); + + final matches = []; + final filesToRemove = {}; + final fileStatusCache = {}; + + 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 + bool? isInvalid = fileStatusCache[key]; + if (isInvalid == null) { + final isDeleted = + enablePhysicalFileCleanup && + !resourceProvider.getFile(key).exists; + final isExcluded = isFileExcluded != null && isFileExcluded(key); + isInvalid = isDeleted || isExcluded; + fileStatusCache[key] = isInvalid; + } + + if (isInvalid) { + filesToRemove.add(key); + continue; + } + + if (!PathUtils.isWithinOrEqual(root, key)) continue; + + // Verify tokenCount to guard against hash collisions. + if (loc.entry.tokenCount != entry.tokenCount) 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, parameters); + } + + return matches; + } + + /// Removes [filePath] from the index. + void removeFile( + String filePath, { + AvoidDuplicateCodeParameters? parameters, + String? packageRoot, + }) { + final root = _getRoot(packageRoot); + final absoluteFilePath = _resolveAndLoad(filePath, packageRoot, parameters); + final oldEntry = _index[absoluteFilePath]; + if (oldEntry != null) { + _removeFromInvertedIndex(absoluteFilePath, oldEntry.entries); + } + _index.remove(absoluteFilePath); + _scheduleSave(root, parameters); + } + + void _scheduleSave( + String packageRoot, [ + AvoidDuplicateCodeParameters? parameters, + ]) { + _saveDebouncers + .putIfAbsent(packageRoot, () => Debouncer(_saveDebounceDuration)) + .run(() { + _performSave(packageRoot, parameters); + }); + } + + 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 (PathUtils.isWithinOrEqual(packageRoot, key)) key: value, + }; + + final params = + parameters ?? + _loadedRoots[packageRoot] ?? + AvoidDuplicateCodeParameters.empty(); + HashCacheStorage.save(packageRoot, subset, params, resourceProvider); + } + + /// Clears the entire index and deletes the persistent cache file. + /// + /// Primarily used in tests to ensure test isolation. + void clear() { + for (final debouncer in _saveDebouncers.values) { + debouncer.cancel(); + } + _saveDebouncers.clear(); + _loadedRoots.clear(); + _index.clear(); + _hashToLocations.clear(); + 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 new file mode 100644 index 00000000..97465d83 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart @@ -0,0 +1,133 @@ +import 'dart:convert'; + +import 'package:analyzer/file_system/file_system.dart'; +import 'package:collection/collection.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/utils/path_utils.dart'; + +/// Service responsible for persisting and loading the duplicate code hash +/// cache. +abstract final class HashCacheStorage { + static const _cacheDirName = '.dart_tool/solid_lints'; + static const _cacheFileName = 'duplicate_index.json'; + + static String _filePath( + String packageRoot, + ResourceProvider resourceProvider, + ) => resourceProvider.pathContext.join( + packageRoot, + _cacheDirName, + _cacheFileName, + ); + + /// 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, + AvoidDuplicateCodeParameters currentParams, + ResourceProvider resourceProvider, + ) { + try { + final file = resourceProvider.getFile( + _filePath(packageRoot, resourceProvider), + ); + if (!file.exists) return null; + + 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 filesMap.entries) { + final absoluteKey = PathUtils.normalizePath(key, packageRoot); + + if (value is Map) { + try { + result[absoluteKey] = FileCacheEntry.fromJson(value); + } catch (_) { + // Skip corrupted entries + } + } + } + return result; + } catch (_) { + return null; + } + } + + /// Saves the [index] to disk for the given [packageRoot]. + static void save( + String packageRoot, + Map index, + AvoidDuplicateCodeParameters currentParams, + ResourceProvider resourceProvider, + ) { + try { + final pathContext = resourceProvider.pathContext; + final directory = resourceProvider.getFolder( + pathContext.join(packageRoot, _cacheDirName), + ); + if (!directory.exists) { + directory.create(); + } + + final file = resourceProvider.getFile( + _filePath(packageRoot, resourceProvider), + ); + + final filesMap = {}; + for (final entry in index.entries) { + final relativeKey = pathContext.isAbsolute(entry.key) + ? pathContext.relative(entry.key, from: packageRoot) + : entry.key; + filesMap[relativeKey] = entry.value.toJson(); + } + + final data = { + 'version': 1, + 'config': currentParams.toJson(), + 'files': filesMap, + }; + + file.writeAsStringSync(jsonEncode(data)); + } catch (_) { + // Fail silently to avoid breaking analysis server + } + } + + /// Deletes the cache file for [packageRoot] if it exists. + static void delete( + String packageRoot, + ResourceProvider resourceProvider, + ) { + try { + final file = resourceProvider.getFile( + _filePath(packageRoot, resourceProvider), + ); + if (file.exists) { + file.delete(); + } + } catch (_) { + // Fail silently + } + } +} 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..b04a0a82 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/utils/context_root_extensions.dart @@ -0,0 +1,14 @@ +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 { + /// 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 = PathUtils.isWithinOrEqual(root.path, filePath); + 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/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/utils/path_utils.dart b/lib/src/lints/avoid_duplicate_code/utils/path_utils.dart new file mode 100644 index 00000000..fb93a331 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/utils/path_utils.dart @@ -0,0 +1,17 @@ +import 'package:path/path.dart' as p; + +/// 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/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..2eae335d --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/utils/token_utils.dart @@ -0,0 +1,18 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; + +/// 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; + } +} 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 new file mode 100644 index 00000000..bb51b0c7 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hash_visitor.dart @@ -0,0 +1,222 @@ +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 [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 (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 AstStructuralHashVisitor extends UnifyingAstVisitor { + static final _typeNameCache = {}; + + final JenkinsHasher _hasher = JenkinsHasher(); + + final bool _ignoreLiterals; + final bool _ignoreIdentifiers; + final Map _localVariableIds = {}; + + /// Creates a new [AstStructuralHashVisitor]. + AstStructuralHashVisitor({ + 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) { + _hasher.reset(); + _localVariableIds.clear(); + node.accept(this); + return _hasher.hash; + } + + void _append(String value) { + _hasher.addString(value); + _hasher.add(0x7C); // ASCII code for '|' + } + + void _appendHash(int hashCode) { + _hasher.add(hashCode); + _hasher.add(0x7C); // ASCII code for '|' + } + + @override + void visitNode(AstNode node) { + // 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. + // + // 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('^'); + } + + @override + void visitVariableDeclarationStatement(VariableDeclarationStatement node) { + final keyword = node.variables.keyword; + if (keyword != null) { + _append(keyword.lexeme); + } + super.visitVariableDeclarationStatement(node); + } + + @override + void visitIfStatement(IfStatement node) { + _appendHash(node.elseKeyword != null ? 1 : 0); + super.visitIfStatement(node); + } + + @override + void visitTryStatement(TryStatement node) { + _appendHash(node.finallyBlock != null ? 1 : 0); + super.visitTryStatement(node); + } + + @override + void visitYieldStatement(YieldStatement node) { + _appendHash(node.star != null ? 1 : 0); + super.visitYieldStatement(node); + } + + @override + void visitBinaryExpression(BinaryExpression node) { + _append(node.operator.lexeme); + super.visitBinaryExpression(node); + } + + @override + void visitPrefixExpression(PrefixExpression node) { + 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.lexeme); + super.visitPrefixExpression(node); + } + + @override + void visitPostfixExpression(PostfixExpression node) { + _append(node.operator.lexeme); + super.visitPostfixExpression(node); + } + + @override + void visitAssignmentExpression(AssignmentExpression node) { + _append(node.operator.lexeme); + super.visitAssignmentExpression(node); + } + + @override + void visitIsExpression(IsExpression node) { + _appendHash(node.notOperator != null ? 1 : 0); + super.visitIsExpression(node); + } + + @override + void visitNamedExpression(NamedExpression node) { + _append(node.name.label.name); + super.visitNamedExpression(node); + } + + // --- Literals --- + + @override + void visitIntegerLiteral(IntegerLiteral node) { + if (!_ignoreLiterals) { + _append(node.literal.lexeme); + } + super.visitIntegerLiteral(node); + } + + @override + void visitDoubleLiteral(DoubleLiteral node) { + if (!_ignoreLiterals) { + _append(node.literal.lexeme); + } + super.visitDoubleLiteral(node); + } + + @override + void visitSimpleStringLiteral(SimpleStringLiteral node) { + if (!_ignoreLiterals) { + _append(node.value); + } + super.visitSimpleStringLiteral(node); + } + + @override + void visitInterpolationString(InterpolationString node) { + if (!_ignoreLiterals) { + _append(node.value); + } + super.visitInterpolationString(node); + } + + @override + void visitBooleanLiteral(BooleanLiteral node) { + if (!_ignoreLiterals) { + _appendHash(node.value ? 1 : 0); + } + super.visitBooleanLiteral(node); + } + + // --- Identifiers --- + + @override + void visitSimpleIdentifier(SimpleIdentifier node) { + 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) { + // 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, + ); + _appendHash(id); + } 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); + } + + @override + void visitNamedType(NamedType node) { + _append(node.name.lexeme); + super.visitNamedType(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 new file mode 100644 index 00000000..853916c6 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart @@ -0,0 +1,334 @@ +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/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'; +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/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'; + +/// 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; + final int _modificationStamp; + final ContextRoot? _contextRoot; + final ResourceProvider _resourceProvider; + + /// Creates a new instance of [AvoidDuplicateCodeVisitor]. + AvoidDuplicateCodeVisitor( + this._rule, + this._parameters, { + required String filePath, + required int modificationStamp, + ContextRoot? contextRoot, + ResourceProvider? resourceProvider, + }) : _filePath = filePath, + _modificationStamp = modificationStamp, + _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) ?? ''; + + if (_tryReportFromCache(filePath, packageRoot)) { + return; + } + + final candidates = _collectCandidates(node); + + final hasher = AstStructuralHashVisitor( + ignoreLiterals: _parameters.ignoreLiterals, + ignoreIdentifiers: _parameters.ignoreIdentifiers, + ); + + 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, + tokenCount: TokenUtils.getTokenCount(candidate.node), + ), + ); + } + + 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: (p) => _contextRoot?.isFileExcluded(p) ?? false, + ); + + 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 = >{}; + 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), + ); + 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)); + } + } + + return true; // Cache hit and handled + } + + List _collectCandidates(CompilationUnit node) { + final collector = CandidateVisitor(_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: (p) => _contextRoot?.isFileExcluded(p) ?? false, + ); + 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 candidates, + Map candidateHashes, + AstStructuralHashVisitor hasher, + Map> crossFileDuplicatesByHash, + ) { + final groups = >{}; + for (final candidate in candidates) { + final hash = + candidateHashes[candidate.node] ?? hasher.computeHash(candidate.node); + (groups[hash] ??= []).add(candidate); + } + + final suppressed = {}; + final reported = {}; + + for (final candidate in candidates) { + if (suppressed.contains(candidate.node)) continue; + if (reported.contains(candidate.node)) continue; + + final hash = + candidateHashes[candidate.node] ?? hasher.computeHash(candidate.node); + final group = groups[hash]; + if (group == null) continue; + + final activeGroup = group + .where((c) => !suppressed.contains(c.node)) + .toList(); + + final externalPartners = crossFileDuplicatesByHash[hash] ?? const []; + final internalPartners = activeGroup + .where((c) => c != candidate) + .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: _duplicateContextMessage, + ), + for (final other in internalPartners) + SolidDiagnosticMessage( + filePath: filePath, + offset: other.node.offset, + length: other.node.length, + message: _duplicateContextMessage, + ), + ]; + + _rule.reportAtNode( + candidate.node, + contextMessages: contextMessages, + ); + + reported.add(candidate.node); + _suppressDescendants(candidate.node, suppressed); + } + } + } + + void _suppressDescendants(AstNode root, Set suppressed) { + final descendantCollector = DescendantVisitor(suppressed, root); + root.accept(descendantCollector); + } + + 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 pathContext = _resourceProvider.pathContext; + final dirPath = pathContext.dirname(filePath); + return _packageRootCache.putIfAbsent(dirPath, () { + var dir = _resourceProvider.getFolder(dirPath); + while (true) { + final pubspec = dir.getChildAssumingFile('pubspec.yaml'); + if (pubspec.exists) { + return dir.path; + } + final parent = dir.parent; + if (parent.path == dir.path) { + break; + } + dir = parent; + } + return null; + }); + } +} 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..c9d3c7f0 --- /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, TokenUtils.getTokenCount(node)); + super.visitBlock(node); + } + + @override + void visitExpressionFunctionBody(ExpressionFunctionBody node) { + _checkAndCollect(node, TokenUtils.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/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; + } +} 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 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..5a66fcc5 --- /dev/null +++ b/test/src/lints/avoid_duplicate_code/avoid_duplicate_code_rule_test.dart @@ -0,0 +1,485 @@ +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'; +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'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(AvoidDuplicateCodeRuleTest); + }); +} + +@reflectiveTest +class AvoidDuplicateCodeRuleTest extends AnalysisRuleTest + with AutoTestLintOffsets { + static const _mockAnalysisOptionsContent = ''' +plugins: + solid_lints: + diagnostics: + avoid_duplicate_code: + min_tokens: 15 + check_blocks: true + exclude: + - method_name: excluded + '''; + + @override + void setUp() { + GlobalHashRegistry.instance.clear(); + GlobalHashRegistry.instance.enablePhysicalFileCleanup = false; + rule = AvoidDuplicateCodeRule( + analysisOptionsLoader: AnalysisOptionsLoader( + resourceProvider: resourceProvider, + ), + ); + super.setUp(); + + newAnalysisOptionsYamlFile( + testPackageRootPath, + '''${analysisOptionsContent(rules: [rule.name])} +$_mockAnalysisOptionsContent''', + ); + } + + @override + Future tearDown() async { + GlobalHashRegistry.instance.clear(); + await super.tearDown(); + } + + // --- Base Tests (min_tokens: 15, check_blocks: true, default) --- + + Future test_reports_when_two_functions_have_identical_bodies() async { + await assertAutoDiagnostics(''' +void first() ${expectLint(r'''{ + final x = 1; + if (x > 0) { + print(x); + } + print('done'); +}''')} + +void second() ${expectLint(r'''{ + 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() ${expectLint(r'''{ + final response = 'data'; + if (response.isEmpty) { + throw Exception('error'); + } + print(response); +}''')} + +void fetchOrders() ${expectLint(r'''{ + 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_tokens() 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() ${expectLint(r'''{ + final x = 1; + if (x > 0) { + print(x); + } + print('done'); + }''')} + + void second() ${expectLint(r'''{ + final y = 1; + if (y > 0) { + print(y); + } + print('done'); + }''')} +} +'''); + } + + Future test_reports_on_third_clone_also() async { + await assertAutoDiagnostics(''' +void first() ${expectLint(r'''{ + final x = 1; + if (x > 0) { + print(x); + } + print('done'); +}''')} + +void second() ${expectLint(r'''{ + final y = 1; + if (y > 0) { + print(y); + } + print('done'); +}''')} + +void third() ${expectLint(r'''{ + 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_tokens: 15 + ignore_literals: true +''', + ); + await assertAutoDiagnostics(''' +void first() ${expectLint(r'''{ + final x = 1; + if (x > 0) { + print('hello'); + } + print('world'); +}''')} + +void second() ${expectLint(r'''{ + 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_tokens: 15 + 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) ${expectLint(r'''{ + 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() ${expectLint(r'''{ + final x = 1; + if (x > 0) { + print('hello'); + print('world'); + print('done'); + } + print('done'); +}''')} + +void two() ${expectLint(r'''{ + final y = 1; + if (y > 0) { + print('hello'); + print('world'); + print('done'); + } + 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( + minTokens: 15, + ignoreLiterals: false, + ignoreIdentifiers: true, + checkBlocks: true, + exclude: ExcludedIdentifiersListParameter( + exclude: [ExcludedIdentifierParameter(methodName: 'excluded')], + ), + ), + filePath: otherFile.path, + modificationStamp: 1, + contextRoot: resolvedOther.session.analysisContext.contextRoot, + resourceProvider: resourceProvider, + ); + resolvedOther.unit.accept(visitor); + + await assertAutoDiagnostics(''' +void mainMethod() ${expectLint(r'''{ + final x = 1; + if (x > 0) { + print(x); + } + 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 new file mode 100644 index 00000000..7351ca8d --- /dev/null +++ b/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart @@ -0,0 +1,565 @@ +import 'dart:convert'; +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'; +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'; +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; + 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; + }); + + test('updateFile stores entries', () { + final entries = [ + const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), + const HashEntry(hash: 456, lineNumber: 20, tokenCount: 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, tokenCount: 5), + ]; + final fileBEntries = [ + const HashEntry(hash: 123, lineNumber: 15, tokenCount: 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(io.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, tokenCount: 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, tokenCount: 5), + ]; + final newEntries = [ + const HashEntry(hash: 456, lineNumber: 20, tokenCount: 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, 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, tokenCount: 3), + ]); + expect(matches2, hasLength(1)); + }); + + test('removeFile clears entries for specific file', () { + registry.updateFile('file_a.dart', [ + const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), + ], modificationStamp: 1); + registry.updateFile('file_b.dart', [ + const HashEntry(hash: 456, lineNumber: 20, tokenCount: 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, tokenCount: 5), + ]); + expect(matches, isEmpty); + }); + + test('clear empties the registry', () { + registry.updateFile('file_a.dart', [ + const HashEntry(hash: 123, lineNumber: 10, tokenCount: 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, tokenCount: 5), + ], modificationStamp: 1); + registry.updateFile('file_b.dart', [ + const HashEntry(hash: 123, lineNumber: 20, tokenCount: 5), + ], modificationStamp: 1); + + final matches = registry.findCrossFileMatches('file_c.dart', [ + const HashEntry(hash: 123, lineNumber: 30, tokenCount: 5), + ]); + + expect(matches, hasLength(1)); + expect(matches.first.duplicates, hasLength(2)); + final expectedPathA = p.normalize( + p.join(io.Directory.current.path, 'file_a.dart'), + ); + final expectedPathB = p.normalize( + 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)); + }); + + test('HashCacheStorage saves and loads index', () { + final absoluteFilePath = p.normalize( + p.join(io.Directory.current.path, 'file_a.dart'), + ); + final index = { + absoluteFilePath: const FileCacheEntry( + modificationStamp: 123456, + entries: [HashEntry(hash: 123, lineNumber: 10, tokenCount: 5)], + ), + }; + + HashCacheStorage.save( + io.Directory.current.path, + index, + AvoidDuplicateCodeParameters.empty(), + memoryResourceProvider, + ); + + final loaded = HashCacheStorage.load( + io.Directory.current.path, + AvoidDuplicateCodeParameters.empty(), + memoryResourceProvider, + ); + 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.tokenCount, equals(5)); + + HashCacheStorage.delete( + io.Directory.current.path, + memoryResourceProvider, + ); + }); + + test('findCrossFileMatches cleans up absolute paths of deleted files', () { + registry.enablePhysicalFileCleanup = true; + final tempPath = p.normalize( + p.join(io.Directory.systemTemp.path, 'temp_test_file.dart'), + ); + memoryResourceProvider.newFile(tempPath, 'void main() {}'); + + registry.updateFile(tempPath, [ + const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5), + ], modificationStamp: 1); + expect(registry.fileCount, equals(1)); + + // Delete the file from the memory resource provider + memoryResourceProvider.deleteFile(tempPath); + + // 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), + ]); + + 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, 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); + + expect(matches, isEmpty); + expect(registry.fileCount, equals(0)); + }); + + test('HashCacheStorage invalidates cache on config change', () { + final absoluteFilePath = p.normalize( + p.join(io.Directory.current.path, 'file.dart'), + ); + final index = { + absoluteFilePath: const FileCacheEntry( + modificationStamp: 123456, + entries: [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( + io.Directory.current.path, + index, + params1, + memoryResourceProvider, + ); + + // Loading with params1 should succeed + 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( + io.Directory.current.path, + params2, + memoryResourceProvider, + ); + expect(loaded2, isNull); + + HashCacheStorage.delete( + io.Directory.current.path, + memoryResourceProvider, + ); + }); + + 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( + io.Directory.current.path, + memoryResourceProvider, + ); + + final loaded = HashCacheStorage.load( + 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 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( + io.Directory.current.path, + AvoidDuplicateCodeParameters.empty(), + memoryResourceProvider, + ); + expect(loaded, isNull); + + HashCacheStorage.delete( + io.Directory.current.path, + memoryResourceProvider, + ); + }); + + 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))); + }); + + test( + 'does not match or clear files from sibling directories with prefixing names', + () { + 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')); + + 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, + ); + }, + ); + + test( + 'debounces save operations independently for different package roots', + () 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')); + + registry.updateFile( + file1, + [const HashEntry(hash: 123, lineNumber: 10, tokenCount: 5)], + modificationStamp: 1, + packageRoot: tempDir1, + ); + + registry.updateFile( + file2, + [const HashEntry(hash: 456, lineNumber: 10, tokenCount: 5)], + modificationStamp: 1, + packageRoot: tempDir2, + ); + + // 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, + AvoidDuplicateCodeParameters.empty(), + memoryResourceProvider, + ); + final loaded2 = HashCacheStorage.load( + tempDir2, + AvoidDuplicateCodeParameters.empty(), + memoryResourceProvider, + ); + + 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)); + }); + }); +} 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); + }); + }); +}