diff --git a/CHANGELOG.md b/CHANGELOG.md index f4ccc394..17527b5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - refactor: replace `avoid_unnecessary_type_casts` rule with dart analyzer's `unnecessary_cast` - Added `use_nearest_context` rule. +- Added `avoid_duplicate_code` rule. - Added `use_descriptive_names_for_type_parameters` rule. ## 0.3.3 diff --git a/lib/analysis_options.yaml b/lib/analysis_options.yaml index 7dbbcdd4..77071fad 100644 --- a/lib/analysis_options.yaml +++ b/lib/analysis_options.yaml @@ -36,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 f68bdf9d..ce9a5bfa 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -4,6 +4,7 @@ import 'package:analyzer/analysis_rule/analysis_rule.dart'; import 'package:solid_lints/src/common/parameter_parser/analysis_options_loader.dart'; import 'package:solid_lints/src/common/solid_lints_constants.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_global_state/avoid_global_state_rule.dart'; import 'package:solid_lints/src/lints/avoid_late_keyword/avoid_late_keyword_rule.dart'; @@ -58,6 +59,7 @@ class SolidLintsPlugin extends Plugin { final lintRules = [ AvoidDebugPrintInReleaseRule(), + AvoidDuplicateCodeRule(analysisOptionsLoader: analysisLoader), AvoidFinalWithGetterRule(), AvoidGlobalStateRule(), AvoidLateKeywordRule(analysisOptionsLoader: analysisLoader), diff --git a/lib/src/common/parameters/excluded_identifier_parameter.dart b/lib/src/common/parameters/excluded_identifier_parameter.dart index f55f9704..b77e2d3e 100644 --- a/lib/src/common/parameters/excluded_identifier_parameter.dart +++ b/lib/src/common/parameters/excluded_identifier_parameter.dart @@ -1,5 +1,7 @@ +import 'package:equatable/equatable.dart'; + /// Model class for ExcludeRule parameters -class ExcludedIdentifierParameter { +class ExcludedIdentifierParameter extends Equatable { /// The name of the method that should be excluded from the lint. final String? methodName; @@ -21,8 +23,19 @@ 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() => { + 'method_name': ?methodName, + 'class_name': ?className, + 'declaration_name': ?declarationName, + }; + + @override + List get props => [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 d7ca8244..691a5f9d 100644 --- a/lib/src/common/parameters/excluded_identifiers_list_parameter.dart +++ b/lib/src/common/parameters/excluded_identifiers_list_parameter.dart @@ -90,4 +90,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..e8b3676c --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/models/avoid_duplicate_code_parameters.dart @@ -0,0 +1,87 @@ +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) => + 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..a8889b3c --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/models/cross_file_match.dart @@ -0,0 +1,27 @@ +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, + }); +} + +/// Extension on an iterable of [CrossFileMatch] to group duplicates by hash. +extension CrossFileMatchIterableExtension on Iterable { + /// Converts this iterable of cross-file matches to a map of duplicates + /// grouped by hash. + Map> toDuplicatesByHash() => { + for (final match in this) match.currentEntry.hash: match.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..79e34d7d --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/models/file_cache_entry.dart @@ -0,0 +1,37 @@ +import 'package:solid_lints/src/lints/avoid_duplicate_code/models/hash_entry.dart'; +import 'package:solid_lints/src/utils/iterable_utils.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. + FileCacheEntry.fromJson(Map json) + : modificationStamp = (json['m'] as int?) ?? 0, + entries = switch (json['e']) { + final List eValue => + eValue + .whereType>() + .tryMap(HashEntry.fromJson) + .nonNulls + .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 new file mode 100644 index 00000000..f4fbdb2f --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/models/hash_entry.dart @@ -0,0 +1,47 @@ +/// 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; + + /// Returns the offset and length as a range tuple. + (int, int) get range => (offset, length); + + /// 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. + HashEntry.fromJson(Map json) + : 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..b77ac50d --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/services/global_hash_registry.dart @@ -0,0 +1,300 @@ +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/hash_entry_list_extension.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'; +import 'package:solid_lints/src/utils/iterable_utils.dart'; +import 'package:solid_lints/src/utils/set_dictionary.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 = SetDictionary(); + + 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, + ) => _hashToLocations.addAll(entries.asIndexEntries(absoluteFilePath)); + + void _removeFromInvertedIndex( + String absoluteFilePath, + List entries, + ) => _hashToLocations.removeAll(entries.asIndexEntries(absoluteFilePath)); + + void _ensureLoaded( + String packageRoot, [ + AvoidDuplicateCodeParameters? currentParams, + ]) { + final params = currentParams ?? AvoidDuplicateCodeParameters.empty(); + if (_loadedRoots[packageRoot] case final previousParams?) { + if (previousParams == params) return; + + _clearEntriesForRoot(packageRoot); + } + + _loadedRoots[packageRoot] = params; + + final cached = HashCacheStorage( + packageRoot: packageRoot, + resourceProvider: resourceProvider, + currentParams: params, + ).load(); + if (cached == null) return; + + final deletedFiles = !enablePhysicalFileCleanup + ? {} + : cached.keys.where((p) => !resourceProvider.getFile(p).exists).toSet(); + + final hashes = cached.keys.toSet().difference(deletedFiles); + + for (final k in hashes) { + _index[k] = cached[k]!; + _addToInvertedIndex(k, cached[k]!.entries); + } + + if (deletedFiles.isNotEmpty) _scheduleSave(packageRoot, params); + } + + void _clearEntriesForRoot(String packageRoot) { + _index.removeWhere((key, oldEntry) { + if (PathUtils.isWithinOrEqual(packageRoot, key)) { + _removeFromInvertedIndex(key, oldEntry.entries); + return true; + } + return false; + }); + } + + 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, + }) => _getCache(filePath, packageRoot, parameters)?.modificationStamp; + + /// Returns the cached entries for [filePath], or `null` if not indexed. + List? getFileEntries( + String filePath, { + AvoidDuplicateCodeParameters? parameters, + String? packageRoot, + }) => _getCache(filePath, packageRoot, parameters)?.entries; + + FileCacheEntry? _getCache( + String filePath, + String? packageRoot, + AvoidDuplicateCodeParameters? parameters, + ) => _index[_resolveAndLoad(filePath, packageRoot, parameters)]; + + /// 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); + + if (_index[absoluteFilePath] case final oldEntry?) { + _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) continue; + + for (final loc in locations) { + final key = loc.filePath; + if (key == absoluteCurrentFilePath) continue; + + final isInvalid = fileStatusCache.putIfAbsent( + key, + () => + (enablePhysicalFileCleanup && + !resourceProvider.getFile(key).exists) || + (isFileExcluded?.call(key) ?? false), + ); + + if (isInvalid) { + filesToRemove.add(key); + } else if (PathUtils.isWithinOrEqual(root, key) && + // guard against hash collisions + loc.entry.tokenCount == entry.tokenCount) { + duplicates.add(loc); + } + } + + if (duplicates.isNotEmpty) { + matches.add( + CrossFileMatch( + currentEntry: entry, + duplicates: duplicates, + ), + ); + } + } + + if (filesToRemove.isEmpty) return matches; + + for (final file in filesToRemove) { + if (_index.remove(file) case final oldEntry?) { + _removeFromInvertedIndex(file, oldEntry.entries); + } + } + + _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); + if (_index.remove(absoluteFilePath) case final oldEntry?) { + _removeFromInvertedIndex(absoluteFilePath, oldEntry.entries); + } + _scheduleSave(root, parameters); + } + + void _scheduleSave( + String packageRoot, [ + AvoidDuplicateCodeParameters? parameters, + ]) { + _saveDebouncers + .putIfAbsent(packageRoot, () => Debouncer(_saveDebounceDuration)) + .run(() { + _performSave(packageRoot, parameters); + }); + } + + void _performSave( + String packageRoot, [ + AvoidDuplicateCodeParameters? parameters, + ]) => + HashCacheStorage( + packageRoot: packageRoot, + resourceProvider: resourceProvider, + currentParams: + parameters ?? + _loadedRoots[packageRoot] ?? + AvoidDuplicateCodeParameters.empty(), + ).save( + // Filter _index for files belonging to this packageRoot + _index.entries.whereKey( + (k) => PathUtils.isWithinOrEqual(packageRoot, k), + ), + ); + + /// 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( + packageRoot: io.Directory.current.path, + resourceProvider: resourceProvider, + ).delete(); + 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..2bc466c7 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/services/hash_cache_storage.dart @@ -0,0 +1,93 @@ +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'; +import 'package:solid_lints/src/utils/function_utils.dart'; +import 'package:solid_lints/src/utils/resource_provider_utils.dart'; + +/// 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'; + + /// The root directory of the package being analyzed. + final String packageRoot; + + /// The resource provider used for file system operations. + final ResourceProvider resourceProvider; + + /// The current configuration parameters. + final AvoidDuplicateCodeParameters currentParams; + + File get _file => resourceProvider.getFile( + resourceProvider.pathContext.join( + packageRoot, + _cacheDirName, + _cacheFileName, + ), + ); + + /// Constructor for [HashCacheStorage]. + HashCacheStorage({ + required this.packageRoot, + required this.resourceProvider, + AvoidDuplicateCodeParameters? currentParams, + }) : currentParams = currentParams ?? AvoidDuplicateCodeParameters.empty(); + + /// Loads the cached index from disk for [packageRoot]. + /// + /// Returns `null` if the cache file does not exist, is corrupted, or fails + /// to load. + Map? load() => FunctionUtils.tryOrNull(_load); + + Map _load() { + if (jsonDecode(_file.readAsStringSync()) case { + 'config': final cachedConfig, + 'files': final Map filesMap, + }) { + if (!const DeepCollectionEquality().equals( + cachedConfig, + currentParams.toJson(), + )) { + throw ArgumentError('Configuration mismatch'); + } + + return { + for (final MapEntry(:key, :value) in filesMap.entries) + if (PathUtils.normalizePath(key, packageRoot) case final absoluteKey) + if (value is Map) + if (FunctionUtils.tryOrNull(() => FileCacheEntry.fromJson(value)) + case final parsed?) + absoluteKey: parsed, + }; + } + + throw ArgumentError('Bad json'); + } + + /// Saves the [index] to disk for [packageRoot]. + void save(Map index) => FunctionUtils.tryOrNull( + () { + resourceProvider.ensureFolderExists(packageRoot, _cacheDirName); + _file.writeAsStringSync( + jsonEncode({ + 'version': 1, + 'config': currentParams.toJson(), + 'files': index.map( + (k, v) => MapEntry( + PathUtils.relativePath(k, packageRoot), + v.toJson(), + ), + ), + }), + ); + }, + ); + + /// Deletes the cache file for [packageRoot] if it exists. + void delete() => FunctionUtils.tryOrNull(_file.delete); +} 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..11e33731 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/utils/context_root_extensions.dart @@ -0,0 +1,12 @@ +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) => + PathUtils.isWithinOrEqual(root.path, filePath) && !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/hash_entry_list_extension.dart b/lib/src/lints/avoid_duplicate_code/utils/hash_entry_list_extension.dart new file mode 100644 index 00000000..4656646e --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/utils/hash_entry_list_extension.dart @@ -0,0 +1,15 @@ +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'; + +/// Extension methods for [List] of [HashEntry]. +extension HashEntryListExtension on List { + /// Converts entries into (hash, DuplicateLocation) tuple pairs for indexing. + Iterable<(int, DuplicateLocation)> asIndexEntries( + String absoluteFilePath, + ) => map( + (e) => ( + e.hash, + DuplicateLocation(entry: e, filePath: absoluteFilePath), + ), + ); +} 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..7985fbce --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/utils/jenkins_hasher.dart @@ -0,0 +1,33 @@ +/// 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 { + static const _hashMask = 0x1fffffff; + static const _shift10Mask = 0x0007ffff; + static const _shift3Mask = 0x03ffffff; + static const _shift15Mask = 0x00003fff; + + 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 = _mix(_hashMask & (_hash + value), _shift10Mask, 10); + _hash ^= _hash >> 6; + } + + /// Adds all code units of [value] to the hash. + void addString(String value) => value.codeUnits.forEach(add); + + /// Finalizes and returns the computed hash. + int get hash { + final h = _mix(_hash, _shift3Mask, 3); + return _mix(h ^ (h >> 11), _shift15Mask, 15); + } + + static int _mix(int hash, int shiftMask, int shiftAmount) => + _hashMask & (hash + ((shiftMask & hash) << shiftAmount)); +} 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..a75a520e --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/utils/path_utils.dart @@ -0,0 +1,25 @@ +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)); + } + + /// Converts a file path to a relative path from [root]. + static String relativePath(String filePath, String root) { + if (p.isAbsolute(filePath)) { + return p.relative(filePath, from: root); + } + return 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..4331377b --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/utils/range_extension.dart @@ -0,0 +1,20 @@ +/// 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); + } +} + +/// Extension on an iterable of (offset, length) ranges. +extension RangeIterableExtension on Iterable<(int, int)> { + /// Returns `true` if any range in this iterable strictly contains the [child] + /// range. + bool anyContainsStrictly((int, int) child) { + return any(child.isStrictlyWithin); + } +} 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..7578392f --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/utils/token_utils.dart @@ -0,0 +1,17 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; + +/// Utility methods for token manipulation. +extension AstNodeTokenCount on AstNode { + /// Returns the total number of non-EOF tokens within this node. + int get tokenCount { + final end = endToken; + var count = 1; + + for (Token t = beginToken; t != end && t != t.next; t = t.next ?? end) { + count++; + } + + return count; + } +} 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..49ea8816 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/visitors/ast_structural_hash_visitor.dart @@ -0,0 +1,206 @@ +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 = {}; + static const _pipeAscii = 0x7C; // '|' + + final _hasher = JenkinsHasher(); + final _localVariableIds = {}; + final bool _ignoreLiterals; + final bool _ignoreIdentifiers; + + /// 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) + ..add(_pipeAscii); + + void _appendHash(int hashCode) => _hasher + ..add(hashCode) + ..add(_pipeAscii); + + @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 type = node.runtimeType; + _append(_typeNameCache.putIfAbsent(type, type.toString)); + node.visitChildren(this); + _append('^'); + } + + @override + void visitVariableDeclarationStatement(VariableDeclarationStatement node) { + if (node.variables.keyword?.lexeme case final l?) _append(l); + 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 (node case PrefixExpression( + operator: Token(type: TokenType.MINUS || TokenType.PLUS), + operand: IntegerLiteral() || DoubleLiteral(), + ) when _ignoreLiterals) { + node.operand.accept(this); + } else { + _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) { + final element = node.element; + + switch (element) { + // Smart ignore: ignore ONLY local variables and parameters. + // This preserves field names, getters, methods, class names, etc. + case LocalVariableElement() || FormalParameterElement() + when _ignoreIdentifiers: + // It is a local variable or parameter. + // Assign it a local ID (De Bruijn Indexing) to distinguish clones + // that wire their variables differently. + _appendHash( + _localVariableIds.putIfAbsent( + element!, + () => _localVariableIds.length, + ), + ); + default: + _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..c39435f5 --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/visitors/avoid_duplicate_code_visitor.dart @@ -0,0 +1,303 @@ +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:collection/collection.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/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/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'; + static final _packageRootCache = {}; + + 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) { + if (_filePath.isEmpty) return; + GlobalHashRegistry.instance.resourceProvider = _resourceProvider; + if (_tryReportFromCache( + _filePath, + _contextRoot?.root.path ?? _findPackageRoot(_filePath) ?? '', + )) { + return; + } + final hasher = AstStructuralHashVisitor( + ignoreLiterals: _parameters.ignoreLiterals, + ignoreIdentifiers: _parameters.ignoreIdentifiers, + ); + final candidates = _collectCandidates(node); + final candidateHashes = { + for (final BodyCandidate(:node) in candidates) + node: hasher.computeHash(node), + }; + final crossFileDuplicatesByHash = _findAndSaveCrossFileMatches( + _filePath, + candidates + .map( + (c) => HashEntry( + hash: candidateHashes[c.node]!, + lineNumber: node.lineInfo.getLocation(c.node.offset).lineNumber, + offset: c.node.offset, + length: c.node.length, + tokenCount: c.node.tokenCount, + ), + ) + .toList(), + _contextRoot?.root.path ?? _findPackageRoot(_filePath) ?? '', + ); + _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 = groupBy(cachedEntries, (entry) => entry.hash); + + 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 = crossMatches.toDuplicatesByHash(); + + for (final entry in cachedEntries) { + final group = hashGroups[entry.hash]; + if (suppressedRanges.anyContainsStrictly(entry.range) || + reportedOffsets.contains(entry.offset) || + group == null) { + continue; + } + + final activeGroup = group + .where((e) => !suppressedRanges.anyContainsStrictly(e.range)) + .toList(); + + final externalPartners = + crossFileDuplicatesByHash[entry.hash] ?? const []; + final internalPartners = activeGroup.where((e) => e != entry).toList(); + + if (internalPartners.isNotEmpty || externalPartners.isNotEmpty) { + final contextMessages = _buildContextMessages( + currentFilePath: filePath, + externalPartners: externalPartners, + internalPartners: internalPartners.map((e) => (e.offset, e.length)), + ); + + _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, + ) { + if (packageRoot.isEmpty) return const {}; + + final registry = GlobalHashRegistry.instance; + final crossMatches = registry.findCrossFileMatches( + filePath, + hashEntries, + parameters: _parameters, + packageRoot: packageRoot, + isFileExcluded: (p) => _contextRoot?.isFileExcluded(p) ?? false, + ); + registry.updateFile( + filePath, + hashEntries, + modificationStamp: _modificationStamp, + parameters: _parameters, + packageRoot: packageRoot, + ); + + return crossMatches.toDuplicatesByHash(); + } + + void _groupAndReportDuplicates( + String filePath, + List candidates, + Map candidateHashes, + AstStructuralHashVisitor hasher, + Map> crossFileDuplicatesByHash, + ) { + final groups = groupBy( + candidates, + (c) => candidateHashes[c.node] ?? hasher.computeHash(c.node), + ); + + final suppressed = {}; + + for (final candidate in candidates.toSet()) { + // Skip candidates that are descendants of an already reported duplicate + // block to prevent nested warnings. + if (suppressed.contains(candidate.node)) continue; + + final hash = + candidateHashes[candidate.node] ?? hasher.computeHash(candidate.node); + final internalPartners = groups[hash]?.toList(); + if (internalPartners == null) continue; + + final externalPartners = crossFileDuplicatesByHash[hash] ?? const []; + internalPartners + ..removeWhere((c) => suppressed.contains(c.node)) + ..remove(candidate); + + if (internalPartners.isEmpty && externalPartners.isEmpty) continue; + + _rule.reportAtNode( + candidate.node, + contextMessages: _buildContextMessages( + currentFilePath: filePath, + externalPartners: externalPartners, + internalPartners: internalPartners.map( + (c) => (c.node.offset, c.node.length), + ), + ), + ); + + _suppressDescendants(candidate.node, suppressed); + } + } + + void _suppressDescendants(AstNode root, Set suppressed) { + final descendantCollector = DescendantVisitor(suppressed, root); + root.accept(descendantCollector); + } + + List _buildContextMessages({ + required String currentFilePath, + required List externalPartners, + required Iterable<(int offset, int length)> internalPartners, + }) { + return [ + for (final dup in externalPartners) + SolidDiagnosticMessage( + filePath: dup.filePath, + offset: dup.entry.offset, + length: dup.entry.length, + message: _duplicateContextMessage, + ), + for (final (offset, length) in internalPartners) + SolidDiagnosticMessage( + filePath: currentFilePath, + offset: offset, + length: length, + message: _duplicateContextMessage, + ), + ]; + } + + /// 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..6b840a4d --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/visitors/candidate_visitor.dart @@ -0,0 +1,48 @@ +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. + !parameters.checkBlocks && node.parent is! BlockFunctionBody + ? super.visitBlock(node) + : _visit(node, super.visitBlock); + + @override + void visitExpressionFunctionBody(ExpressionFunctionBody node) => + _visit(node, super.visitExpressionFunctionBody); + + void _visit( + T node, + void Function(T) visitSuper, + ) { + if (node.tokenCount < parameters.minTokens) return; + + final declaration = node.thisOrAncestorOfType(); + if (declaration != null && parameters.exclude.shouldIgnore(declaration)) { + return; + } + + candidates.add(( + node: node, + enclosingDeclaration: declaration, + )); + + visitSuper(node); + } +} 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..3c22b9dc --- /dev/null +++ b/lib/src/lints/avoid_duplicate_code/visitors/descendant_visitor.dart @@ -0,0 +1,26 @@ +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) => _visit(node, super.visitBlock); + + @override + void visitExpressionFunctionBody(ExpressionFunctionBody node) => + _visit(node, super.visitExpressionFunctionBody); + + void _visit(T node, void Function(T) visitSuper) { + if (node != root) suppressed.add(node); + visitSuper(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/lib/src/utils/function_utils.dart b/lib/src/utils/function_utils.dart new file mode 100644 index 00000000..a1059251 --- /dev/null +++ b/lib/src/utils/function_utils.dart @@ -0,0 +1,12 @@ +/// Utility functions. +abstract final class FunctionUtils { + /// Executes the [f], returning its result or `null` if any + /// exception/error is thrown. + static T? tryOrNull(T Function() f) { + try { + return f(); + } catch (_) { + return null; + } + } +} diff --git a/lib/src/utils/iterable_utils.dart b/lib/src/utils/iterable_utils.dart index 3dcf538b..c5f195f6 100644 --- a/lib/src/utils/iterable_utils.dart +++ b/lib/src/utils/iterable_utils.dart @@ -26,6 +26,28 @@ extension IterablePairwise on Iterable { } } +/// Extension on [Iterable] that provides a [tryMap] method. +extension IterableTryMap on Iterable { + /// Maps each element using [f], catching exceptions and returning null for + /// those elements. + Iterable tryMap(U Function(T) f) => map((e) { + try { + return f(e); + } catch (_) { + return null; + } + }); +} + +/// Extension on [Iterable] of [MapEntry] to filter and convert to [Map]. +extension MapEntryIterableExtension on Iterable> { + /// Filters entries by key and returns a new [Map]. + Map whereKey(bool Function(K key) test) => { + for (final entry in this) + if (test(entry.key)) entry.key: entry.value, + }; +} + /// Extension on [Iterable] to zip elements with another iterable. extension IterableZip on Iterable { /// Zips this iterable with [other]. diff --git a/lib/src/utils/resource_provider_utils.dart b/lib/src/utils/resource_provider_utils.dart new file mode 100644 index 00000000..6541026e --- /dev/null +++ b/lib/src/utils/resource_provider_utils.dart @@ -0,0 +1,9 @@ +import 'package:analyzer/file_system/file_system.dart'; + +/// Extension on [ResourceProvider] to provide folder helpers. +extension ResourceProviderUtils on ResourceProvider { + /// Ensures that a folder at the path joined from [root] and [dir] exists, + /// creating it if it doesn't. + void ensureFolderExists(String root, String dir) => + getFolder(pathContext.join(root, dir)).create(); +} diff --git a/lib/src/utils/set_dictionary.dart b/lib/src/utils/set_dictionary.dart new file mode 100644 index 00000000..cb5b1050 --- /dev/null +++ b/lib/src/utils/set_dictionary.dart @@ -0,0 +1,46 @@ +/// A dictionary that maps a key to a set of values. +class SetDictionary { + final _map = >{}; + + /// Adds a [value] to the set associated with the [key]. + void add(K key, V value) { + (_map[key] ??= {}).add(value); + } + + /// Adds all [entries] (key-value pairs) to the dictionary. + void addAll(Iterable<(K, V)> entries) { + for (final (key, value) in entries) { + add(key, value); + } + } + + /// Removes a [value] from the set associated with the [key]. + /// + /// If the set becomes empty after removing the value, the [key] is removed + /// from the dictionary. + void remove(K key, V value) { + final set = _map[key]; + if (set == null) return; + + set.remove(value); + if (set.isEmpty) { + _map.remove(key); + } + } + + /// Removes all [entries] (key-value pairs) from the dictionary. + void removeAll(Iterable<(K, V)> entries) { + for (final (key, value) in entries) { + remove(key, value); + } + } + + /// Returns the set of values associated with the [key], or `null` if the + /// key is not in the dictionary. + Set? operator [](K key) => _map[key]; + + /// Removes all keys and values from the dictionary. + void clear() { + _map.clear(); + } +} diff --git a/pubspec.yaml b/pubspec.yaml index a5276bf7..09b05007 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -17,6 +17,7 @@ dependencies: analyzer: ^10.0.1 collection: ^1.19.0 analysis_server_plugin: ^0.3.3 + equatable: ^2.1.0 glob: ^2.1.3 path: ^1.9.1 yaml: ^3.1.3 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..04573387 --- /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 '../../utils/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..93c8de1e --- /dev/null +++ b/test/src/lints/avoid_duplicate_code/global_hash_registry_test.dart @@ -0,0 +1,548 @@ +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 + ..resourceProvider = memoryResourceProvider + ..clear() + ..enablePhysicalFileCleanup = false; + }); + + tearDown( + () => registry + ..clear() + ..resourceProvider = PhysicalResourceProvider.INSTANCE + ..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)], + ), + }; + + final storage = HashCacheStorage( + packageRoot: io.Directory.current.path, + resourceProvider: memoryResourceProvider, + ); + + storage.save(index); + + final loaded = storage.load(); + 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)); + + storage.delete(); + }); + + 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, + ); + + final storage1 = HashCacheStorage( + packageRoot: io.Directory.current.path, + resourceProvider: memoryResourceProvider, + currentParams: params1, + ); + + // Save with params1 + storage1.save(index); + + // Loading with params1 should succeed + final loaded1 = storage1.load(); + expect(loaded1, isNotNull); + + // Loading with params2 (different config) should return null + // (invalidated) + final storage2 = HashCacheStorage( + packageRoot: io.Directory.current.path, + resourceProvider: memoryResourceProvider, + currentParams: params2, + ); + final loaded2 = storage2.load(); + expect(loaded2, isNull); + + storage1.delete(); + }); + + 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', () { + final storage = HashCacheStorage( + packageRoot: io.Directory.current.path, + resourceProvider: memoryResourceProvider, + ); + + // Ensure any existing cache is deleted + storage.delete(); + + final loaded = storage.load(); + + expect(loaded, isNull); + }); + + test('HashCacheStorage.load returns null and does not throw when cache ' + 'file is corrupted', () { + final storage = HashCacheStorage( + packageRoot: io.Directory.current.path, + resourceProvider: memoryResourceProvider, + ); + + 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 = storage.load(); + expect(loaded, isNull); + + storage.delete(); + }); + + 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( + packageRoot: tempDir1, + resourceProvider: memoryResourceProvider, + ).load(); + final loaded2 = HashCacheStorage( + packageRoot: tempDir2, + resourceProvider: memoryResourceProvider, + ).load(); + + 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); + }); + }); +}