-
Notifications
You must be signed in to change notification settings - Fork 24
Add new rule avoid_duplicate_code #320
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 23 commits
0012940
77f24f8
d8e267e
f57a284
6c3900b
59e6884
74b6e93
94b1d74
f4918d3
a18f796
f1b870e
80f888a
d0611f9
ce36e02
1b55728
5e8e393
2faba00
44de4fa
6b6fde6
8a766a6
4c88728
c2af6ec
46ef176
5aff0b5
3ee9826
1a62878
89b2051
c9f6df2
8ac5326
bd85f23
d3c6c59
73647e8
60b97fc
31082d3
24fcb93
546e6d7
1f39df8
175c09f
a7b544a
4e4fba0
7e33e9c
301a012
8a4a384
f8f0ecb
6559760
4cfaeb9
35fcead
fe21164
b4843f6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,8 +21,31 @@ class ExcludedIdentifierParameter { | |
| Map<dynamic, dynamic> 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<String, Object?> 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, | ||
| ); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @illia-romanenko can we use https://pub.dev/packages/equatable here?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I don't see why not.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| 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<AvoidDuplicateCodeParameters> { | ||
| /// 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, | ||
|
solid-illiaaihistov marked this conversation as resolved.
|
||
| ); | ||
|
|
||
| registry.addCompilationUnit(this, visitor); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -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<String, Object?> 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), | ||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implemented similarly to the parameters of the other rules. |
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| /// Converts the parameters to a JSON-compatible Map. | ||||||||||||||||||||||||||||||||
| Map<String, Object?> 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, | ||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||
|
solid-illiaaihistov marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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, | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<DuplicateLocation> duplicates; | ||
|
|
||
| /// Creates a new [CrossFileMatch]. | ||
| const CrossFileMatch({ | ||
| required this.currentEntry, | ||
| required this.duplicates, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!