From e225a300ed6a8849f44093cbdc868d4a0031a058 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 09:49:27 +0800 Subject: [PATCH 01/50] add doc --- website/docs/rules/common/ban-name.md | 42 +++++++++++++++++++++++++++ website/docs/rules/overview.md | 4 +++ 2 files changed, 46 insertions(+) create mode 100644 website/docs/rules/common/ban-name.md diff --git a/website/docs/rules/common/ban-name.md b/website/docs/rules/common/ban-name.md new file mode 100644 index 0000000000..a30dd3dfb9 --- /dev/null +++ b/website/docs/rules/common/ban-name.md @@ -0,0 +1,42 @@ +# Ban name + +## Rule id {#rule-id} + +ban-name + +## Severity {#severity} + +Warning + +## Description {#description} + +Configure some names that you want to ban. + +For example, in my own app, I want to ban `showDialog` and force to use `myShowDialog` instead. This is because (let me explain the simplified case) I added logging in `myShowDialog`, and I do want every dialog to be logged. + +### Example {#example} + +Bad: + +```dart +// suppose the user configures `showDialog` to be banned +showDialog(); // LINT +``` + +Good: + +```dart +myShowDialog(); +``` + +### Config example {#config-example} + +```yaml +dart_code_metrics: + ... + rules: + ... + - ban-name: + - ident: showDialog + description: Please use myShowDialog in this package +``` diff --git a/website/docs/rules/overview.md b/website/docs/rules/overview.md index 6c64b1dd4e..666140fa0e 100644 --- a/website/docs/rules/overview.md +++ b/website/docs/rules/overview.md @@ -59,6 +59,10 @@ Rules configuration is [described here](../getting-started/configuration#configu Checks for unused parameters inside a function or method body. +- [ban-name](./common/ban-name.md)   [![Configurable](https://img.shields.io/badge/-configurable-informational)](./common/ban-name.md#config-example) + + Configure some names that you want to ban. + - [binary-expression-operand-order](./common/binary-expression-operand-order.md)   ![Has auto-fix](https://img.shields.io/badge/-has%20auto--fix-success) Warns when a literal value is on the left hand side in a binary expressions. From dd561c15ceeb5092175b680185bdd17cac1fa377 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 09:51:42 +0800 Subject: [PATCH 02/50] add empty ban_name_rule --- .../lint_analyzer/rules/rules_factory.dart | 2 ++ .../rules_list/ban_name/ban_name_rule.dart | 20 +++++++++++++++++++ .../rules/rules_list/ban_name/visitor.dart | 3 +++ 3 files changed, 25 insertions(+) create mode 100644 lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart create mode 100644 lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/visitor.dart diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart b/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart index 7e9cba5f06..472bf2bc34 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart @@ -17,6 +17,7 @@ import 'rules_list/avoid_unnecessary_type_casts/avoid_unnecessary_type_casts_rul import 'rules_list/avoid_unrelated_type_assertions/avoid_unrelated_type_assertions_rule.dart'; import 'rules_list/avoid_unused_parameters/avoid_unused_parameters_rule.dart'; import 'rules_list/avoid_wrapping_in_padding/avoid_wrapping_in_padding_rule.dart'; +import 'rules_list/ban_name/ban_name_rule.dart'; import 'rules_list/binary_expression_operand_order/binary_expression_operand_order_rule.dart'; import 'rules_list/component_annotation_arguments_ordering/component_annotation_arguments_ordering_rule.dart'; import 'rules_list/double_literal_format/double_literal_format_rule.dart'; @@ -77,6 +78,7 @@ final _implementedRules = )>{ AvoidUnusedParametersRule(config), AvoidWrappingInPaddingRule.ruleId: (config) => AvoidWrappingInPaddingRule(config), + BanNameRule.ruleId: (config) => BanNameRule(config), BinaryExpressionOperandOrderRule.ruleId: (config) => BinaryExpressionOperandOrderRule(config), ComponentAnnotationArgumentsOrderingRule.ruleId: (config) => diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart new file mode 100644 index 0000000000..be743b6309 --- /dev/null +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart @@ -0,0 +1,20 @@ +// ignore_for_file: public_member_api_docs + +import 'package:analyzer/dart/ast/visitor.dart'; + +import '../../../lint_utils.dart'; +import '../../../models/severity.dart'; +import '../../models/common_rule.dart'; + +part 'visitor.dart'; + +class BanNameRule extends CommonRule { + static const String ruleId = 'ban-name'; + + BanNameRule([Map config = const {}]) + : super( + id: ruleId, + severity: readSeverity(config, Severity.style), + excludes: readExcludes(config), + ); +} diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/visitor.dart new file mode 100644 index 0000000000..4b0fce91dc --- /dev/null +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/visitor.dart @@ -0,0 +1,3 @@ +part of 'ban_name_rule.dart'; + +class _Visitor extends RecursiveAstVisitor {} From a1db691a6af3a014234f9353ed2459951e19b1a7 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 09:56:44 +0800 Subject: [PATCH 03/50] add tests --- .../rules_list/ban_name/ban_name_rule.dart | 41 +++++++++++++++++++ .../rules_list/ban_name/examples/example.dart | 7 ++++ website/docs/rules/common/ban-name.md | 1 + 3 files changed, 49 insertions(+) create mode 100644 test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart create mode 100644 test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/examples/example.dart diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart new file mode 100644 index 0000000000..0fba5fc702 --- /dev/null +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart @@ -0,0 +1,41 @@ +import 'package:dart_code_metrics/src/analyzers/lint_analyzer/models/severity.dart'; +import 'package:dart_code_metrics/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart'; +import 'package:test/test.dart'; + +import '../../../../../helpers/rule_test_helper.dart'; + +const _examplePath = 'always_remove_listener/examples/example.dart'; + +void main() { + group('BanNameRule', () { + test('initialization', () async { + final unit = await RuleTestHelper.resolveFromFile(_examplePath); + final issues = BanNameRule().check(unit); + + RuleTestHelper.verifyInitialization( + issues: issues, + ruleId: 'prefer-correct-type-name', + severity: Severity.style, + ); + }); + + test('reports about all found issues in example.dart', () async { + final unit = await RuleTestHelper.resolveFromFile(_examplePath); + + final issues = BanNameRule({ + 'entries': [ + {'ident': 'showDialog', 'description': 'Please use myShowDialog'}, + {'ident': 'showSnackBar', 'description': 'What about myShowSnackBar'}, + ], + }).check(unit); + + RuleTestHelper.verifyIssues( + issues: issues, + startLines: [], + startColumns: [], + locationTexts: [], + messages: [], + ); + }); + }); +} diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/examples/example.dart new file mode 100644 index 0000000000..83d5879861 --- /dev/null +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/examples/example.dart @@ -0,0 +1,7 @@ +void func() { + myShowDialog('some_arguments', 'another_argument'); + showDialog('some_arguments', 'another_argument'); // LINT + + myShowSnackBar(42); + showSnackBar(42); // LINT +} \ No newline at end of file diff --git a/website/docs/rules/common/ban-name.md b/website/docs/rules/common/ban-name.md index a30dd3dfb9..b3117e8a89 100644 --- a/website/docs/rules/common/ban-name.md +++ b/website/docs/rules/common/ban-name.md @@ -37,6 +37,7 @@ dart_code_metrics: rules: ... - ban-name: + entries: - ident: showDialog description: Please use myShowDialog in this package ``` From 059c8039ca8ceb666e2ddaf148770918b1dbf5db Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 10:02:43 +0800 Subject: [PATCH 04/50] config parser --- .../rules_list/ban_name/ban_name_rule.dart | 1 + .../ban_name/utils/config_parser.dart | 28 +++++++++++++++++++ .../rules_list/ban_name/examples/example.dart | 2 +- 3 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/utils/config_parser.dart diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart index be743b6309..24af2887cd 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart @@ -7,6 +7,7 @@ import '../../../models/severity.dart'; import '../../models/common_rule.dart'; part 'visitor.dart'; +part 'utils/config_parser.dart'; class BanNameRule extends CommonRule { static const String ruleId = 'ban-name'; diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/utils/config_parser.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/utils/config_parser.dart new file mode 100644 index 0000000000..38e9cc3bdc --- /dev/null +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/utils/config_parser.dart @@ -0,0 +1,28 @@ +part of '../ban_name_rule.dart'; + +const _entriesLabel = 'entries'; +const _identLabel = 'ident'; +const _descriptionLabel = 'description'; + +/// Parser for rule configuration. +class _ConfigParser { + static List<_BanNameConfigEntry> readEntries(Map config) => + _parseEntryConfig(config[_entriesLabel]); + + static List<_BanNameConfigEntry> _parseEntryConfig(Object? object) => + (object as Iterable).map((entry) { + final entryMap = entry as Map; + + return _BanNameConfigEntry( + ident: entryMap[_identLabel] as String, + description: entryMap[_descriptionLabel] as String, + ); + }).toList(); +} + +class _BanNameConfigEntry { + final String ident; + final String description; + + _BanNameConfigEntry({required this.ident, required this.description}); +} diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/examples/example.dart index 83d5879861..631f8a007d 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/examples/example.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/examples/example.dart @@ -4,4 +4,4 @@ void func() { myShowSnackBar(42); showSnackBar(42); // LINT -} \ No newline at end of file +} From 27e338c2979ca579c3d51b761bfad49d6ee050be Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 10:06:13 +0800 Subject: [PATCH 05/50] implement rule (without visitor) --- .../rules_list/ban_name/ban_name_rule.dart | 27 ++++++++++++++++++- .../rules/rules_list/ban_name/visitor.dart | 17 +++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart index 24af2887cd..7ea289d8d9 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart @@ -1,10 +1,15 @@ // ignore_for_file: public_member_api_docs +import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; +import '../../../../../utils/node_utils.dart'; import '../../../lint_utils.dart'; +import '../../../models/internal_resolved_unit_result.dart'; +import '../../../models/issue.dart'; import '../../../models/severity.dart'; import '../../models/common_rule.dart'; +import '../../rule_utils.dart'; part 'visitor.dart'; part 'utils/config_parser.dart'; @@ -12,10 +17,30 @@ part 'utils/config_parser.dart'; class BanNameRule extends CommonRule { static const String ruleId = 'ban-name'; + final List<_BanNameConfigEntry> _entries; + BanNameRule([Map config = const {}]) - : super( + : _entries = _ConfigParser._parseEntryConfig(config), + super( id: ruleId, severity: readSeverity(config, Severity.style), excludes: readExcludes(config), ); + + @override + Iterable check(InternalResolvedUnitResult source) { + final visitor = _Visitor(_entries); + + source.unit.visitChildren(visitor); + + return visitor.nodes + .map( + (node) => createIssue( + rule: this, + location: nodeLocation(node: node.node, source: source), + message: node.message, + ), + ) + .toList(growable: false); + } } diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/visitor.dart index 4b0fce91dc..d03b39839c 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/visitor.dart @@ -1,3 +1,18 @@ part of 'ban_name_rule.dart'; -class _Visitor extends RecursiveAstVisitor {} +class _Visitor extends RecursiveAstVisitor { + final List<_BanNameConfigEntry> _entries; + + final _nodes = <_NodeWithMessage>[]; + + Iterable<_NodeWithMessage> get nodes => _nodes; + + _Visitor(this._entries); +} + +class _NodeWithMessage { + final Expression node; + final String message; + + _NodeWithMessage(this.node, this.message); +} From d81fc297b57b7104b2e1d5475aa4ddd0f9d21e17 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 10:20:43 +0800 Subject: [PATCH 06/50] try to implement visitor --- .../rules/rules_list/ban_name/visitor.dart | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/visitor.dart index d03b39839c..3316ba6236 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/visitor.dart @@ -2,12 +2,41 @@ part of 'ban_name_rule.dart'; class _Visitor extends RecursiveAstVisitor { final List<_BanNameConfigEntry> _entries; + final Map _entryMap; final _nodes = <_NodeWithMessage>[]; Iterable<_NodeWithMessage> get nodes => _nodes; - _Visitor(this._entries); + _Visitor(this._entries) : _entryMap = Map.fromEntries(_entries.map((e) => MapEntry(e.ident, e))); + + @override + void visitSimpleIdentifier(SimpleIdentifier node) { + super.visitSimpleIdentifier(node); + _visitIdent(node, node); + } + + @override + void visitPrefixedIdentifier(PrefixedIdentifier node) { + super.visitPrefixedIdentifier(node); + _visitIdent(node, node.identifier); + _visitIdent(node, node.prefix); + } + + @override + void visitLibraryIdentifier(LibraryIdentifier node) { + super.visitLibraryIdentifier(node); + for (final component in node.components) { + _visitIdent(node, component); + } + } + + void _visitIdent(Expression node, SimpleIdentifier ident) { + final name = ident.name; + if(_entryMap.containsKey(name)) { + _nodes.add(_NodeWithMessage(node, '${_entryMap[name]!.description} ($name is banned)')); + } + } } class _NodeWithMessage { From ed5edb34472deff8d1aa9ab298dcf2ccbcfda375 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 10:22:25 +0800 Subject: [PATCH 07/50] fix bugs --- .../rules_list/ban_name/utils/config_parser.dart | 8 +++----- .../rules/rules_list/ban_name/visitor.dart | 11 +++++++---- .../rules/rules_list/ban_name/ban_name_rule.dart | 13 ++++++++----- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/utils/config_parser.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/utils/config_parser.dart index 38e9cc3bdc..e9bbb559e8 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/utils/config_parser.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/utils/config_parser.dart @@ -6,11 +6,9 @@ const _descriptionLabel = 'description'; /// Parser for rule configuration. class _ConfigParser { - static List<_BanNameConfigEntry> readEntries(Map config) => - _parseEntryConfig(config[_entriesLabel]); - - static List<_BanNameConfigEntry> _parseEntryConfig(Object? object) => - (object as Iterable).map((entry) { + static List<_BanNameConfigEntry> _parseEntryConfig( + Map config) => + (config[_entriesLabel] as Iterable? ?? []).map((entry) { final entryMap = entry as Map; return _BanNameConfigEntry( diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/visitor.dart index 3316ba6236..7550a93e7c 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/visitor.dart @@ -1,14 +1,14 @@ part of 'ban_name_rule.dart'; class _Visitor extends RecursiveAstVisitor { - final List<_BanNameConfigEntry> _entries; final Map _entryMap; final _nodes = <_NodeWithMessage>[]; Iterable<_NodeWithMessage> get nodes => _nodes; - _Visitor(this._entries) : _entryMap = Map.fromEntries(_entries.map((e) => MapEntry(e.ident, e))); + _Visitor(List<_BanNameConfigEntry> entries) + : _entryMap = Map.fromEntries(entries.map((e) => MapEntry(e.ident, e))); @override void visitSimpleIdentifier(SimpleIdentifier node) { @@ -33,8 +33,11 @@ class _Visitor extends RecursiveAstVisitor { void _visitIdent(Expression node, SimpleIdentifier ident) { final name = ident.name; - if(_entryMap.containsKey(name)) { - _nodes.add(_NodeWithMessage(node, '${_entryMap[name]!.description} ($name is banned)')); + if (_entryMap.containsKey(name)) { + _nodes.add(_NodeWithMessage( + node, + '${_entryMap[name]!.description} ($name is banned)', + )); } } } diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart index 0fba5fc702..f797e2e52d 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart @@ -4,7 +4,7 @@ import 'package:test/test.dart'; import '../../../../../helpers/rule_test_helper.dart'; -const _examplePath = 'always_remove_listener/examples/example.dart'; +const _examplePath = 'ban_name/examples/example.dart'; void main() { group('BanNameRule', () { @@ -31,10 +31,13 @@ void main() { RuleTestHelper.verifyIssues( issues: issues, - startLines: [], - startColumns: [], - locationTexts: [], - messages: [], + startLines: [3, 6], + startColumns: [3, 3], + locationTexts: ['showDialog', 'showSnackBar'], + messages: [ + 'Please use myShowDialog (showDialog is banned)', + 'What about myShowSnackBar (showSnackBar is banned)', + ], ); }); }); From 6c30d46d8f84d9f97c78032405fd4345bbea51a8 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 10:27:05 +0800 Subject: [PATCH 08/50] enhance tests --- .../rules/rules_list/ban_name/ban_name_rule.dart | 11 ++++++++--- .../rules/rules_list/ban_name/examples/example.dart | 4 ++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart index f797e2e52d..5c9e3465a0 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart @@ -31,10 +31,15 @@ void main() { RuleTestHelper.verifyIssues( issues: issues, - startLines: [3, 6], - startColumns: [3, 3], - locationTexts: ['showDialog', 'showSnackBar'], + startLines: [6, 7, 10], + startColumns: [3, 12, 3], + locationTexts: [ + 'showDialog', + 'showDialog', + 'showSnackBar', + ], messages: [ + 'Please use myShowDialog (showDialog is banned)', 'Please use myShowDialog (showDialog is banned)', 'What about myShowSnackBar (showSnackBar is banned)', ], diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/examples/example.dart index 631f8a007d..dbea378b3f 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/examples/example.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/examples/example.dart @@ -1,6 +1,10 @@ +import 'package:flutter/material.dart'; +import 'package:flutter/material.dart' as material; + void func() { myShowDialog('some_arguments', 'another_argument'); showDialog('some_arguments', 'another_argument'); // LINT + material.showDialog('some_arguments', 'another_argument'); // LINT myShowSnackBar(42); showSnackBar(42); // LINT From b41cfa9b6aa00eda58572668e20fcc1777df0de7 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Sat, 26 Feb 2022 08:58:15 +0800 Subject: [PATCH 09/50] fix wrong ruleid --- .../lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart index 5c9e3465a0..22c50a27e2 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart @@ -14,7 +14,7 @@ void main() { RuleTestHelper.verifyInitialization( issues: issues, - ruleId: 'prefer-correct-type-name', + ruleId: 'ban-name', severity: Severity.style, ); }); From eafaaa5e110cabc1e2845a8ebe88b5a024b31610 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Sat, 26 Feb 2022 08:59:11 +0800 Subject: [PATCH 10/50] change severity --- website/docs/rules/common/ban-name.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/rules/common/ban-name.md b/website/docs/rules/common/ban-name.md index b3117e8a89..4fa148fe70 100644 --- a/website/docs/rules/common/ban-name.md +++ b/website/docs/rules/common/ban-name.md @@ -6,7 +6,7 @@ ban-name ## Severity {#severity} -Warning +Style ## Description {#description} From 9113a63ab49a054559830aac38409aee136c3da1 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Sat, 26 Feb 2022 09:00:29 +0800 Subject: [PATCH 11/50] rephrase --- website/docs/rules/common/ban-name.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/rules/common/ban-name.md b/website/docs/rules/common/ban-name.md index 4fa148fe70..8ea725c8ee 100644 --- a/website/docs/rules/common/ban-name.md +++ b/website/docs/rules/common/ban-name.md @@ -12,7 +12,7 @@ Style Configure some names that you want to ban. -For example, in my own app, I want to ban `showDialog` and force to use `myShowDialog` instead. This is because (let me explain the simplified case) I added logging in `myShowDialog`, and I do want every dialog to be logged. +Example: When you add some extra functionalities to built-in Flutter functions (such as logging for `showDialog`), you may want to ban the original Flutter function and use your own version. ### Example {#example} From 3aa5b1cda5af8c15f8c8df4cc7432da817ff28f8 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Sat, 26 Feb 2022 09:03:53 +0800 Subject: [PATCH 12/50] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3979f04abf..7c549787a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * **Breaking Change:** cli arguments `--fatal-unused` and `--fatal-warnings` activate by default. * chore: restrict `analyzer` version to `>=2.8.0 <3.3.0`. +* feat: add static code diagnostic `ban-name` ## 4.11.0 From 0139ef24e9164f53721218d4df954e5cd2df854a Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Sat, 26 Feb 2022 20:20:05 +0800 Subject: [PATCH 13/50] add more tests --- .../rules_list/ban_name/ban_name_rule.dart | 17 ++++++++++++----- .../rules_list/ban_name/examples/example.dart | 10 ++++++++-- website/docs/rules/common/ban-name.md | 19 +++++++++++++++++-- 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart index 22c50a27e2..292d92fae9 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart @@ -25,23 +25,30 @@ void main() { final issues = BanNameRule({ 'entries': [ {'ident': 'showDialog', 'description': 'Please use myShowDialog'}, - {'ident': 'showSnackBar', 'description': 'What about myShowSnackBar'}, + {'ident': 'strangeName', 'description': 'The name is too strange'}, + {'ident': 'AnotherStrangeName', 'description': 'Oops'}, ], }).check(unit); RuleTestHelper.verifyIssues( issues: issues, - startLines: [6, 7, 10], - startColumns: [3, 12, 3], + startLines: [6, 7, 9, 12, 15, 16], + startColumns: [3, 12, 7, 6, 7, 12], locationTexts: [ 'showDialog', 'showDialog', - 'showSnackBar', + 'strangeName', + 'strangeName', + 'AnotherStrangeName', + 'strangeName', ], messages: [ 'Please use myShowDialog (showDialog is banned)', 'Please use myShowDialog (showDialog is banned)', - 'What about myShowSnackBar (showSnackBar is banned)', + 'The name is too strange (strangeName is banned)', + 'The name is too strange (strangeName is banned)', + 'Oops (AnotherStrangeName is banned)', + 'The name is too strange (strangeName is banned)', ], ); }); diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/examples/example.dart index dbea378b3f..4fc335aec3 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/examples/example.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/examples/example.dart @@ -6,6 +6,12 @@ void func() { showDialog('some_arguments', 'another_argument'); // LINT material.showDialog('some_arguments', 'another_argument'); // LINT - myShowSnackBar(42); - showSnackBar(42); // LINT + var strangeName = 42; // LINT +} + +void strangeName() {} // LINT + +// LINT +class AnotherStrangeName { + late var strangeName; // LINT } diff --git a/website/docs/rules/common/ban-name.md b/website/docs/rules/common/ban-name.md index 8ea725c8ee..af4955c6fe 100644 --- a/website/docs/rules/common/ban-name.md +++ b/website/docs/rules/common/ban-name.md @@ -19,8 +19,19 @@ Example: When you add some extra functionalities to built-in Flutter functions ( Bad: ```dart -// suppose the user configures `showDialog` to be banned -showDialog(); // LINT +// suppose the configuration is the one shown below + +showDialog('some_arguments', 'another_argument'); // LINT +material.showDialog('some_arguments', 'another_argument'); // LINT + +var strangeName = 42; // LINT + +void strangeName() {} // LINT + +// LINT +class AnotherStrangeName { + late var strangeName; // LINT +} ``` Good: @@ -40,4 +51,8 @@ dart_code_metrics: entries: - ident: showDialog description: Please use myShowDialog in this package + - ident: strangeName + description: The name is too strange + - ident: AnotherStrangeName + description: Oops ``` From 6fd7c99534338c1f8897c53f7af040b9ee32dbff Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Sat, 26 Feb 2022 20:30:51 +0800 Subject: [PATCH 14/50] scaffold for tag-name --- .../rules_list/tag_name/tag_name_rule.dart | 45 +++++++++++++++++++ .../rules/rules_list/tag_name/visitor.dart | 4 ++ website/docs/rules/common/tag-name.md | 45 +++++++++++++++++++ website/docs/rules/overview.md | 4 ++ 4 files changed, 98 insertions(+) create mode 100644 lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule.dart create mode 100644 lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart create mode 100644 website/docs/rules/common/tag-name.md diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule.dart new file mode 100644 index 0000000000..2da111e048 --- /dev/null +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule.dart @@ -0,0 +1,45 @@ +// ignore_for_file: public_member_api_docs + +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; + +import '../../../../../utils/node_utils.dart'; +import '../../../lint_utils.dart'; +import '../../../models/internal_resolved_unit_result.dart'; +import '../../../models/issue.dart'; +import '../../../models/severity.dart'; +import '../../models/common_rule.dart'; +import '../../rule_utils.dart'; + +part 'visitor.dart'; + +class TagNameRule extends CommonRule { + static const String ruleId = 'tag-name'; + + static const _warning = 'Incorrect tag name'; + + TagNameRule([Map config = const {}]) + : super( + id: ruleId, + severity: readSeverity(config, Severity.warning), + excludes: readExcludes(config), + ); + + @override + Iterable check(InternalResolvedUnitResult source) { + final visitor = _Visitor(); + + source.unit.visitChildren(visitor); + + return visitor.nodes + .map((node) => createIssue( + rule: this, + location: nodeLocation( + node: node, + source: source, + ), + message: _warning, + )) + .toList(growable: false); + } +} diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart new file mode 100644 index 0000000000..920a6c1c58 --- /dev/null +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart @@ -0,0 +1,4 @@ +part of 'tag_name_rule.dart'; + +class _Visitor extends RecursiveAstVisitor { +} diff --git a/website/docs/rules/common/tag-name.md b/website/docs/rules/common/tag-name.md new file mode 100644 index 0000000000..d473dc2fd5 --- /dev/null +++ b/website/docs/rules/common/tag-name.md @@ -0,0 +1,45 @@ +# Tag name + +## Rule id {#rule-id} + +tag-name + +## Severity {#severity} + +Warning + +## Description {#description} + +Warns when tag name does not match class name. + +### Config example {#config-example} + +We recommend exclude the `test` folder. + +```yaml +dart_code_metrics: + ... + rules: + ... + - tag-name: + var_names: [_kTag] + ... +``` + +### Example {#example} + +Bad: + +```dart +class Apple { + static const _kTag = 'Orange'; // LINT +} +``` + +Good: + +```dart +class Apple { + static const _kTag = 'Apple'; +} +``` diff --git a/website/docs/rules/overview.md b/website/docs/rules/overview.md index 6c64b1dd4e..baf4d045b1 100644 --- a/website/docs/rules/overview.md +++ b/website/docs/rules/overview.md @@ -139,6 +139,10 @@ Rules configuration is [described here](../getting-started/configuration#configu Check for trailing comma for arguments, parameters, enum values and collections. +- [tag-name](common/tag-name.md)   [![Configurable](https://img.shields.io/badge/-configurable-informational)](./common/prefer-trailing-comma.md#config-example)   ![Has auto-fix](https://img.shields.io/badge/-has%20auto--fix-success) + + Warns when tag name does not match class name. + ## Flutter specific {#flutter-specific} - [always-remove-listener](./flutter/always-remove-listener.md) From 0250bf51ce79845381127c8c18d5bdf93449be21 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Sat, 26 Feb 2022 20:32:51 +0800 Subject: [PATCH 15/50] add tests --- .../rules_list/tag_name/examples/example.dart | 11 ++++++ .../tag_name/tag_name_rule_test.dart | 37 +++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/examples/example.dart create mode 100644 test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule_test.dart diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/examples/example.dart new file mode 100644 index 0000000000..d556f3526d --- /dev/null +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/examples/example.dart @@ -0,0 +1,11 @@ +class _Fruit { + static const _kTag = '_Fruit'; +} + +class Apple extends _Fruit { + static const _kTag = 'Orange'; // LINT +} + +class Orange extends _Fruit { + static const _kTag = 'Orange'; +} diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule_test.dart new file mode 100644 index 0000000000..b4a46e053b --- /dev/null +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule_test.dart @@ -0,0 +1,37 @@ +import 'package:dart_code_metrics/src/analyzers/lint_analyzer/models/severity.dart'; +import 'package:dart_code_metrics/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule.dart'; +import 'package:test/test.dart'; + +import '../../../../../helpers/rule_test_helper.dart'; + +const _examplePath = 'tag_name/examples/example.dart'; + +void main() { + group('TagNameRule', () { + test('initialization', () async { + final unit = await RuleTestHelper.resolveFromFile(_examplePath); + final issues = TagNameRule().check(unit); + + RuleTestHelper.verifyInitialization( + issues: issues, + ruleId: 'tag-name', + severity: Severity.warning, + ); + }); + + test('reports about found issues', () async { + final unit = await RuleTestHelper.resolveFromFile(_examplePath); + final issues = TagNameRule().check(unit); + + RuleTestHelper.verifyIssues( + issues: issues, + startLines: [], + startColumns: [], + locationTexts: [], + replacementComments: [], + replacements: [], + messages: [], + ); + }); + }); +} From e9fbd2c6a69b7e22870072988710f346f3ba26d5 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Sat, 26 Feb 2022 20:42:33 +0800 Subject: [PATCH 16/50] partial impl --- .../rules/rules_list/tag_name/tag_name_rule.dart | 8 ++++++-- .../rules_list/tag_name/utils/config_parser.dart | 16 ++++++++++++++++ .../rules/rules_list/tag_name/visitor.dart | 16 ++++++++++++++++ .../rules_list/tag_name/tag_name_rule_test.dart | 4 +++- website/docs/rules/common/tag-name.md | 2 +- 5 files changed, 42 insertions(+), 4 deletions(-) create mode 100644 lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/utils/config_parser.dart diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule.dart index 2da111e048..48449427c4 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule.dart @@ -12,14 +12,18 @@ import '../../models/common_rule.dart'; import '../../rule_utils.dart'; part 'visitor.dart'; +part 'utils/config_parser.dart'; class TagNameRule extends CommonRule { static const String ruleId = 'tag-name'; static const _warning = 'Incorrect tag name'; + final _ParsedConfig _parsedConfig; + TagNameRule([Map config = const {}]) - : super( + : _parsedConfig = _ConfigParser._parseConfig(config), + super( id: ruleId, severity: readSeverity(config, Severity.warning), excludes: readExcludes(config), @@ -27,7 +31,7 @@ class TagNameRule extends CommonRule { @override Iterable check(InternalResolvedUnitResult source) { - final visitor = _Visitor(); + final visitor = _Visitor(_parsedConfig); source.unit.visitChildren(visitor); diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/utils/config_parser.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/utils/config_parser.dart new file mode 100644 index 0000000000..b27af45bd7 --- /dev/null +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/utils/config_parser.dart @@ -0,0 +1,16 @@ +part of '../tag_name_rule.dart'; + +const _varNamesLabel = 'var-names'; + +/// Parser for rule configuration. +class _ConfigParser { + static _ParsedConfig _parseConfig(Map value) => _ParsedConfig( + varNames: (value[_varNamesLabel] as List? ?? []).map((item) => item as String).toList(), + ); +} + +class _ParsedConfig { + final List varNames; + + _ParsedConfig({required this.varNames}); +} diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart index 920a6c1c58..80f0749102 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart @@ -1,4 +1,20 @@ part of 'tag_name_rule.dart'; class _Visitor extends RecursiveAstVisitor { + final _ParsedConfig _parsedConfig; + + _Visitor(this._parsedConfig); + + @override + void visitFieldDeclaration(FieldDeclaration node) { + super.visitFieldDeclaration(node); + + for (final fieldVariable in node.fields.variables) { + final name = fieldVariable.name.name; + if (_parsedConfig.varNames.contains(name)) { + // TODO + print('hi $name $node'); + } + } + } } diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule_test.dart index b4a46e053b..b29f5ed457 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule_test.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule_test.dart @@ -21,7 +21,9 @@ void main() { test('reports about found issues', () async { final unit = await RuleTestHelper.resolveFromFile(_examplePath); - final issues = TagNameRule().check(unit); + final issues = TagNameRule({ + 'var-names': ['_kTag', 'tag', 'TAG'], + }).check(unit); RuleTestHelper.verifyIssues( issues: issues, diff --git a/website/docs/rules/common/tag-name.md b/website/docs/rules/common/tag-name.md index d473dc2fd5..1bd613a5f3 100644 --- a/website/docs/rules/common/tag-name.md +++ b/website/docs/rules/common/tag-name.md @@ -22,7 +22,7 @@ dart_code_metrics: rules: ... - tag-name: - var_names: [_kTag] + var-names: [_kTag] ... ``` From c124eff4c5cd3c27255dae2d15eba260b4b62217 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Sat, 26 Feb 2022 20:43:41 +0800 Subject: [PATCH 17/50] more implementations --- .../rules_list/tag_name/tag_name_rule.dart | 7 ++- .../rules/rules_list/tag_name/visitor.dart | 49 +++++++++++++++++-- 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule.dart index 48449427c4..5163f4f9e3 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule.dart @@ -2,11 +2,13 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:meta/meta_meta.dart'; import '../../../../../utils/node_utils.dart'; import '../../../lint_utils.dart'; import '../../../models/internal_resolved_unit_result.dart'; import '../../../models/issue.dart'; +import '../../../models/replacement.dart'; import '../../../models/severity.dart'; import '../../models/common_rule.dart'; import '../../rule_utils.dart'; @@ -17,7 +19,7 @@ part 'utils/config_parser.dart'; class TagNameRule extends CommonRule { static const String ruleId = 'tag-name'; - static const _warning = 'Incorrect tag name'; + static const _warning = 'Tag name should match class name'; final _ParsedConfig _parsedConfig; @@ -39,10 +41,11 @@ class TagNameRule extends CommonRule { .map((node) => createIssue( rule: this, location: nodeLocation( - node: node, + node: node.initializer, source: source, ), message: _warning, + replacement: node.replacement, )) .toList(growable: false); } diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart index 80f0749102..3de4c9e460 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart @@ -5,16 +5,55 @@ class _Visitor extends RecursiveAstVisitor { _Visitor(this._parsedConfig); + final _nodes = <_NodeWithMessage>[]; + + Iterable<_NodeWithMessage> get nodes => _nodes; + @override void visitFieldDeclaration(FieldDeclaration node) { super.visitFieldDeclaration(node); for (final fieldVariable in node.fields.variables) { - final name = fieldVariable.name.name; - if (_parsedConfig.varNames.contains(name)) { - // TODO - print('hi $name $node'); - } + _checkField(node, fieldVariable); } } + + void _checkField(FieldDeclaration node, VariableDeclaration fieldVariable) { + final fieldName = fieldVariable.name.name; + final fieldType = fieldVariable.declaredElement?.type; + + if (!(fieldType != null && fieldType.isDartCoreString && _parsedConfig.varNames.contains(fieldName))) { + return; + } + + final fieldInitializer = fieldVariable.initializer; + if (fieldInitializer is! StringLiteral) { + return; + } + final fieldInitValue = fieldInitializer.stringValue; + + final classDeclaration = node.thisOrAncestorOfType(); + final className = classDeclaration?.name.name; + if (className == null) { + return; + } + final expectFieldValue = className; + + if (fieldInitValue != expectFieldValue) { + _nodes.add(_NodeWithMessage( + fieldInitializer, + Replacement( + comment: 'Replace with $expectFieldValue', + replacement: "'$expectFieldValue'", + ), + )); + } + } +} + +class _NodeWithMessage { + final StringLiteral initializer; + final Replacement replacement; + + _NodeWithMessage(this.initializer, this.replacement); } From 0eeb81234d3271348b3cb67b5c19798f0b37f614 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Sat, 26 Feb 2022 21:01:52 +0800 Subject: [PATCH 18/50] pass tests --- .../rules/rules_list/tag_name/visitor.dart | 2 +- .../rules/rules_list/tag_name/examples/example.dart | 4 +++- .../rules_list/tag_name/tag_name_rule_test.dart | 12 ++++++------ 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart index 3de4c9e460..2be11294f1 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart @@ -43,7 +43,7 @@ class _Visitor extends RecursiveAstVisitor { _nodes.add(_NodeWithMessage( fieldInitializer, Replacement( - comment: 'Replace with $expectFieldValue', + comment: "Replace with '$expectFieldValue'", replacement: "'$expectFieldValue'", ), )); diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/examples/example.dart index d556f3526d..0f8a3b2a3d 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/examples/example.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/examples/example.dart @@ -7,5 +7,7 @@ class Apple extends _Fruit { } class Orange extends _Fruit { - static const _kTag = 'Orange'; + static const TAG = 'Orange'; } + +static const TAG = 'FileLevelTagIsIgnored'; \ No newline at end of file diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule_test.dart index b29f5ed457..926150d846 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule_test.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule_test.dart @@ -27,12 +27,12 @@ void main() { RuleTestHelper.verifyIssues( issues: issues, - startLines: [], - startColumns: [], - locationTexts: [], - replacementComments: [], - replacements: [], - messages: [], + startLines: [6], + startColumns: [24], + locationTexts: ["'Orange'"], + messages: ['Tag name should match class name'], + replacements: ["'Apple'"], + replacementComments: ["Replace with 'Apple'"], ); }); }); From 712b0f7b9e500e22733335e1a404533b29c60b95 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Sat, 26 Feb 2022 21:02:34 +0800 Subject: [PATCH 19/50] enhance tests and still pass tests --- .../rules/rules_list/tag_name/examples/example.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/examples/example.dart index 0f8a3b2a3d..492ce6d165 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/examples/example.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/examples/example.dart @@ -7,7 +7,7 @@ class Apple extends _Fruit { } class Orange extends _Fruit { - static const TAG = 'Orange'; + static const TAG = 'Or' + 'an' + 'ge'; } static const TAG = 'FileLevelTagIsIgnored'; \ No newline at end of file From 8e21f5ce1ab900c49bb29fb28325603368c29641 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Sat, 26 Feb 2022 21:03:20 +0800 Subject: [PATCH 20/50] format code --- .../rules/rules_list/tag_name/utils/config_parser.dart | 4 +++- .../lint_analyzer/rules/rules_list/tag_name/visitor.dart | 4 +++- .../rules/rules_list/tag_name/examples/example.dart | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/utils/config_parser.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/utils/config_parser.dart index b27af45bd7..8df2782023 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/utils/config_parser.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/utils/config_parser.dart @@ -5,7 +5,9 @@ const _varNamesLabel = 'var-names'; /// Parser for rule configuration. class _ConfigParser { static _ParsedConfig _parseConfig(Map value) => _ParsedConfig( - varNames: (value[_varNamesLabel] as List? ?? []).map((item) => item as String).toList(), + varNames: (value[_varNamesLabel] as List? ?? []) + .map((item) => item as String) + .toList(), ); } diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart index 2be11294f1..e299b41767 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart @@ -22,7 +22,9 @@ class _Visitor extends RecursiveAstVisitor { final fieldName = fieldVariable.name.name; final fieldType = fieldVariable.declaredElement?.type; - if (!(fieldType != null && fieldType.isDartCoreString && _parsedConfig.varNames.contains(fieldName))) { + if (!(fieldType != null && + fieldType.isDartCoreString && + _parsedConfig.varNames.contains(fieldName))) { return; } diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/examples/example.dart index 492ce6d165..7bb3c18f41 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/examples/example.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/examples/example.dart @@ -10,4 +10,4 @@ class Orange extends _Fruit { static const TAG = 'Or' + 'an' + 'ge'; } -static const TAG = 'FileLevelTagIsIgnored'; \ No newline at end of file +const TAG = 'FileLevelTagIsIgnored'; From f53b303c408dcb675b08ac78a8c48f3adc222787 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Tue, 1 Mar 2022 11:54:38 +0800 Subject: [PATCH 21/50] update changelog --- .gitignore | 1 + CHANGELOG.md | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 3dadc6a9e9..bea1987ff8 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,7 @@ # JetBrains IDEs .idea/ +*.iml # Files and directories created by pub .dart_tool/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 4281e06981..ae6578e041 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,11 +3,9 @@ ## Unreleased * **Breaking Change:** cli arguments `--fatal-unused` and `--fatal-warnings` activate by default. -* chore: restrict `analyzer` version to `>=2.8.0 <3.3.0`. -* feat: add static code diagnostics `avoid-collection-methods-with-unrelated-types`, `ban-name`. * chore: restrict `analyzer` version to `>=3.3.0 <3.4.0`. * chore: restrict `analyzer_plugin` version to `>=0.9.0 <0.10.0`. -* feat: add static code diagnostic `avoid-collection-methods-with-unrelated-types` +* feat: add static code diagnostics `avoid-collection-methods-with-unrelated-types`, `ban-name`, `tag-name`. ## 4.11.0 From a6a65f553e05593834986ffa4cdbe0c520e189d8 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Tue, 1 Mar 2022 11:58:03 +0800 Subject: [PATCH 22/50] revert bumping analyzer, otherwise flutter stable version cannot work --- CHANGELOG.md | 2 +- .../rules/rules_list/avoid_returning_widgets/visitor.dart | 2 +- .../rules/rules_list/avoid_unnecessary_setstate/visitor.dart | 2 +- .../rules/rules_list/prefer_extracting_callbacks/visitor.dart | 2 +- .../rules_list/prefer_single_widget_per_file/visitor.dart | 2 +- pubspec.yaml | 4 ++-- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae6578e041..c16d7f4c5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ## Unreleased * **Breaking Change:** cli arguments `--fatal-unused` and `--fatal-warnings` activate by default. -* chore: restrict `analyzer` version to `>=3.3.0 <3.4.0`. +* chore: restrict `analyzer` version to `>=2.8.0 <3.3.0`. * chore: restrict `analyzer_plugin` version to `>=0.9.0 <0.10.0`. * feat: add static code diagnostics `avoid-collection-methods-with-unrelated-types`, `ban-name`, `tag-name`. diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_returning_widgets/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_returning_widgets/visitor.dart index f852ff6f10..456e5be364 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_returning_widgets/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_returning_widgets/visitor.dart @@ -36,7 +36,7 @@ class _Visitor extends RecursiveAstVisitor { @override void visitClassDeclaration(ClassDeclaration node) { - final classType = node.extendsClause?.superclass.type; + final classType = node.extendsClause?.superclass2.type; if (!isWidgetOrSubclass(classType) && !isWidgetStateOrSubclass(classType)) { return; } diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_unnecessary_setstate/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_unnecessary_setstate/visitor.dart index 8bb6ba0d6e..4b220889a8 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_unnecessary_setstate/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_unnecessary_setstate/visitor.dart @@ -14,7 +14,7 @@ class _Visitor extends RecursiveAstVisitor { void visitClassDeclaration(ClassDeclaration node) { super.visitClassDeclaration(node); - final type = node.extendsClause?.superclass.type; + final type = node.extendsClause?.superclass2.type; if (type == null || !isWidgetStateOrSubclass(type)) { return; } diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/visitor.dart index ac0147326c..64bce38799 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/visitor.dart @@ -11,7 +11,7 @@ class _Visitor extends SimpleAstVisitor { @override void visitClassDeclaration(ClassDeclaration node) { - final classType = node.extendsClause?.superclass.type; + final classType = node.extendsClause?.superclass2.type; if (!isWidgetOrSubclass(classType) && !isWidgetStateOrSubclass(classType)) { return; } diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_single_widget_per_file/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_single_widget_per_file/visitor.dart index a55d2842d3..09f4746ef3 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_single_widget_per_file/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_single_widget_per_file/visitor.dart @@ -15,7 +15,7 @@ class _Visitor extends SimpleAstVisitor { void visitClassDeclaration(ClassDeclaration node) { super.visitClassDeclaration(node); - final classType = node.extendsClause?.superclass.type; + final classType = node.extendsClause?.superclass2.type; if (isWidgetOrSubclass(classType) && (!_ignorePrivateWidgets || !Identifier.isPrivateName(node.name.name))) { _nodes.add(node); diff --git a/pubspec.yaml b/pubspec.yaml index c1139d35e3..f4fa5fd492 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -10,8 +10,8 @@ environment: sdk: ">=2.14.0 <3.0.0" dependencies: - analyzer: ">=3.3.0 <3.4.0" - analyzer_plugin: ">=0.9.0 <0.10.0" + analyzer: ">=2.8.0 <3.3.0" + analyzer_plugin: ">=0.8.0 <0.10.0" ansicolor: ^2.0.1 args: ^2.0.0 collection: ^1.15.0 From 85c48f450228ad5b21fc0a8512f1fc20ca46e5b1 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Tue, 1 Mar 2022 12:03:34 +0800 Subject: [PATCH 23/50] fix linter issue --- .../rules/rules_list/ban_name/utils/config_parser.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/utils/config_parser.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/utils/config_parser.dart index e9bbb559e8..faa2119921 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/utils/config_parser.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/ban_name/utils/config_parser.dart @@ -7,7 +7,8 @@ const _descriptionLabel = 'description'; /// Parser for rule configuration. class _ConfigParser { static List<_BanNameConfigEntry> _parseEntryConfig( - Map config) => + Map config, + ) => (config[_entriesLabel] as Iterable? ?? []).map((entry) { final entryMap = entry as Map; From a643e7eab80c924423fd8685011117407d9d166b Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Tue, 1 Mar 2022 12:18:14 +0800 Subject: [PATCH 24/50] regsiter TagNameRule --- lib/src/analyzers/lint_analyzer/rules/rules_factory.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart b/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart index 33c0bdc0a8..5eb6a2695d 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart @@ -46,6 +46,7 @@ import 'rules_list/prefer_on_push_cd_strategy/prefer_on_push_cd_strategy_rule.da import 'rules_list/prefer_single_widget_per_file/prefer_single_widget_per_file_rule.dart'; import 'rules_list/prefer_trailing_comma/prefer_trailing_comma_rule.dart'; import 'rules_list/provide_correct_intl_args/provide_correct_intl_args_rule.dart'; +import 'rules_list/tag_name/tag_name_rule.dart'; final _implementedRules = )>{ AlwaysRemoveListenerRule.ruleId: (config) => AlwaysRemoveListenerRule(config), @@ -121,6 +122,7 @@ final _implementedRules = )>{ PreferTrailingCommaRule.ruleId: (config) => PreferTrailingCommaRule(config), ProvideCorrectIntlArgsRule.ruleId: (config) => ProvideCorrectIntlArgsRule(config), + TagNameRule.ruleId: (config) => TagNameRule(config), }; Iterable get allRules => From 21f7d98db72385ff738f4aa5a2c2b0149de7f244 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Tue, 1 Mar 2022 13:59:35 +0800 Subject: [PATCH 25/50] add stripPrefix, stripPostfix --- .../rules_list/tag_name/utils/config_parser.dart | 12 +++++++++++- .../rules/rules_list/tag_name/visitor.dart | 14 +++++++++++++- .../rules_list/tag_name/examples/example.dart | 12 ++++++++---- .../rules_list/tag_name/tag_name_rule_test.dart | 2 ++ website/docs/rules/common/tag-name.md | 10 ++++++++++ 5 files changed, 44 insertions(+), 6 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/utils/config_parser.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/utils/config_parser.dart index 8df2782023..fc731fc848 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/utils/config_parser.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/utils/config_parser.dart @@ -1,6 +1,8 @@ part of '../tag_name_rule.dart'; const _varNamesLabel = 'var-names'; +const _stripPrefixLabel = 'strip-prefix'; +const _stripPostfixLabel = 'strip-postfix'; /// Parser for rule configuration. class _ConfigParser { @@ -8,11 +10,19 @@ class _ConfigParser { varNames: (value[_varNamesLabel] as List? ?? []) .map((item) => item as String) .toList(), + stripPrefix: value[_stripPrefixLabel] as String? ?? '', + stripPostfix: value[_stripPostfixLabel] as String? ?? '', ); } class _ParsedConfig { final List varNames; + final String stripPrefix; + final String stripPostfix; - _ParsedConfig({required this.varNames}); + _ParsedConfig({ + required this.varNames, + required this.stripPrefix, + required this.stripPostfix, + }); } diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart index e299b41767..5ef68913e4 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/tag_name/visitor.dart @@ -39,7 +39,7 @@ class _Visitor extends RecursiveAstVisitor { if (className == null) { return; } - final expectFieldValue = className; + final expectFieldValue = _calculateExpectFieldValue(className); if (fieldInitValue != expectFieldValue) { _nodes.add(_NodeWithMessage( @@ -51,6 +51,18 @@ class _Visitor extends RecursiveAstVisitor { )); } } + + String _calculateExpectFieldValue(String className) { + var ans = className; + if (className.startsWith(_parsedConfig.stripPrefix)) { + ans = ans.substring(_parsedConfig.stripPrefix.length); + } + if (className.endsWith(_parsedConfig.stripPostfix)) { + ans = ans.substring(0, ans.length - _parsedConfig.stripPostfix.length); + } + + return ans; + } } class _NodeWithMessage { diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/examples/example.dart index 7bb3c18f41..f3fc333fe4 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/examples/example.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/examples/example.dart @@ -1,13 +1,17 @@ -class _Fruit { - static const _kTag = '_Fruit'; +class Fruit { + static const _kTag = 'Fruit'; } -class Apple extends _Fruit { +class Apple extends Fruit { static const _kTag = 'Orange'; // LINT } -class Orange extends _Fruit { +class Orange extends Fruit { static const TAG = 'Or' + 'an' + 'ge'; } +class _PlantState { + static const _kTag = 'Plant'; // should not lint +} + const TAG = 'FileLevelTagIsIgnored'; diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule_test.dart index 926150d846..f8937b96f5 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule_test.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/tag_name/tag_name_rule_test.dart @@ -23,6 +23,8 @@ void main() { final unit = await RuleTestHelper.resolveFromFile(_examplePath); final issues = TagNameRule({ 'var-names': ['_kTag', 'tag', 'TAG'], + 'strip-prefix': '_', + 'strip-postfix': 'State', }).check(unit); RuleTestHelper.verifyIssues( diff --git a/website/docs/rules/common/tag-name.md b/website/docs/rules/common/tag-name.md index 1bd613a5f3..db8542de25 100644 --- a/website/docs/rules/common/tag-name.md +++ b/website/docs/rules/common/tag-name.md @@ -23,6 +23,8 @@ dart_code_metrics: ... - tag-name: var-names: [_kTag] + strip-prefix: _ + strip-postfix: State ... ``` @@ -34,6 +36,10 @@ Bad: class Apple { static const _kTag = 'Orange'; // LINT } + +class _OrangeState { + static const _kTag = 'Apple'; // LINT +} ``` Good: @@ -42,4 +48,8 @@ Good: class Apple { static const _kTag = 'Apple'; } + +class _OrangeState { + static const _kTag = 'Orange'; +} ``` From 314d8104fafab5ecf118e1d31dc46a4cafcd1328 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Tue, 1 Mar 2022 14:49:12 +0800 Subject: [PATCH 26/50] explain limitation --- website/docs/rules/common/ban-name.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/website/docs/rules/common/ban-name.md b/website/docs/rules/common/ban-name.md index af4955c6fe..a66a71c48c 100644 --- a/website/docs/rules/common/ban-name.md +++ b/website/docs/rules/common/ban-name.md @@ -14,6 +14,8 @@ Configure some names that you want to ban. Example: When you add some extra functionalities to built-in Flutter functions (such as logging for `showDialog`), you may want to ban the original Flutter function and use your own version. +Limitation: When trying to ban some methods in your package, it also triggers on imported from an external package code. + ### Example {#example} Bad: From eeca3cea4dd309d5144a1f347f7baef7d35eb233 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 26 May 2022 08:39:04 +0800 Subject: [PATCH 27/50] fix wrong merge --- .../rules_list/ban_name/ban_name_rule.dart | 56 ------------------- 1 file changed, 56 deletions(-) delete mode 100644 test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart deleted file mode 100644 index 292d92fae9..0000000000 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart +++ /dev/null @@ -1,56 +0,0 @@ -import 'package:dart_code_metrics/src/analyzers/lint_analyzer/models/severity.dart'; -import 'package:dart_code_metrics/src/analyzers/lint_analyzer/rules/rules_list/ban_name/ban_name_rule.dart'; -import 'package:test/test.dart'; - -import '../../../../../helpers/rule_test_helper.dart'; - -const _examplePath = 'ban_name/examples/example.dart'; - -void main() { - group('BanNameRule', () { - test('initialization', () async { - final unit = await RuleTestHelper.resolveFromFile(_examplePath); - final issues = BanNameRule().check(unit); - - RuleTestHelper.verifyInitialization( - issues: issues, - ruleId: 'ban-name', - severity: Severity.style, - ); - }); - - test('reports about all found issues in example.dart', () async { - final unit = await RuleTestHelper.resolveFromFile(_examplePath); - - final issues = BanNameRule({ - 'entries': [ - {'ident': 'showDialog', 'description': 'Please use myShowDialog'}, - {'ident': 'strangeName', 'description': 'The name is too strange'}, - {'ident': 'AnotherStrangeName', 'description': 'Oops'}, - ], - }).check(unit); - - RuleTestHelper.verifyIssues( - issues: issues, - startLines: [6, 7, 9, 12, 15, 16], - startColumns: [3, 12, 7, 6, 7, 12], - locationTexts: [ - 'showDialog', - 'showDialog', - 'strangeName', - 'strangeName', - 'AnotherStrangeName', - 'strangeName', - ], - messages: [ - 'Please use myShowDialog (showDialog is banned)', - 'Please use myShowDialog (showDialog is banned)', - 'The name is too strange (strangeName is banned)', - 'The name is too strange (strangeName is banned)', - 'Oops (AnotherStrangeName is banned)', - 'The name is too strange (strangeName is banned)', - ], - ); - }); - }); -} From b165d7230bcfa104b4c228aff54c77e4bffa104e Mon Sep 17 00:00:00 2001 From: fzyzcjy <5236035+fzyzcjy@users.noreply.github.com> Date: Thu, 26 May 2022 10:39:30 +0800 Subject: [PATCH 28/50] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3e01b2b78..09aace3240 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased * chore: restrict `analyzer` version to `>=2.4.0 <4.2.0`. +* feat: add [`avoid-banned-imports`](https://dartcodemetrics.dev/docs/rules/common/avoid-banned-imports) rule ## 4.15.2 From 66c12beec448deb341184ee4d39bc1ccd414bad0 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 26 May 2022 10:44:04 +0800 Subject: [PATCH 29/50] add doc --- .../docs/rules/common/avoid-banned-imports.md | 46 +++++++++++++++++++ website/docs/rules/overview.md | 4 ++ 2 files changed, 50 insertions(+) create mode 100644 website/docs/rules/common/avoid-banned-imports.md diff --git a/website/docs/rules/common/avoid-banned-imports.md b/website/docs/rules/common/avoid-banned-imports.md new file mode 100644 index 0000000000..4ac8fd36f7 --- /dev/null +++ b/website/docs/rules/common/avoid-banned-imports.md @@ -0,0 +1,46 @@ +# Ban name + +## Rule id {#rule-id} + +avoid-banned-imports + +## Severity {#severity} + +Style + +## Description {#description} + +Configure some imports that you want to ban. + +### Example {#example} + +With the configuration in the example below, here are some bad/good examples. + +Bad: + +```dart +import "package:flutter/material.dart"; +import "package:flutter_bloc/flutter_bloc.dart"; +``` + +Good: + +```dart +// No restricted imports in listed folders. +``` + +### Config example {#config-example} + +```yaml +dart_code_metrics: + ... + rules: + ... + - avoid_restricted_imports + - paths: ["some/**/folder/*.dart", "another/folder/**.dart"] + deny: ["package:flutter/material.dart"] + message: "Do not import Flutter Material Design library, we should not depend on it!" + - paths: ["core/*.dart"] + deny: ["package:flutter_bloc/flutter_bloc.dart"] + message: 'State management should be not used inside "core" folder.' +``` diff --git a/website/docs/rules/overview.md b/website/docs/rules/overview.md index a1aac31206..729fd3e377 100644 --- a/website/docs/rules/overview.md +++ b/website/docs/rules/overview.md @@ -11,6 +11,10 @@ Rules configuration is [described here](../getting-started/configuration#configu ## Common {#common} +- [avoid-banned-imports](./common/avoid-banned-imports.md)   [![Configurable](https://img.shields.io/badge/-configurable-informational)](./common/avoid-banned-imports.md#config-example) + + Configure some imports that you want to ban. + - [avoid-collection-methods-with-unrelated-types](./common/avoid-collection-methods-with-unrelated-types.md) Avoid using collection methods with unrelated types, such as accessing a map of integers using a string key. From cc95ad4fa07cf2952fb7d4bf60de0fdc25957790 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 26 May 2022 10:46:10 +0800 Subject: [PATCH 30/50] add empty AvoidBannedImportsRule --- .../avoid_banned_imports_rule.dart | 46 +++++++++++++++++ .../utils/config_parser.dart | 27 ++++++++++ .../avoid_banned_imports/visitor.dart | 50 +++++++++++++++++++ 3 files changed, 123 insertions(+) create mode 100644 lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart create mode 100644 lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/utils/config_parser.dart create mode 100644 lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart new file mode 100644 index 0000000000..2eb9be71f9 --- /dev/null +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart @@ -0,0 +1,46 @@ +// ignore_for_file: public_member_api_docs + +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; + +import '../../../../../utils/node_utils.dart'; +import '../../../lint_utils.dart'; +import '../../../models/internal_resolved_unit_result.dart'; +import '../../../models/issue.dart'; +import '../../../models/severity.dart'; +import '../../models/common_rule.dart'; +import '../../rule_utils.dart'; + +part 'visitor.dart'; +part 'utils/config_parser.dart'; + +class AvoidBannedImportsRule extends CommonRule { + static const String ruleId = 'avoid-banned-imports'; + + final List<_BanNameConfigEntry> _entries; + + AvoidBannedImportsRule([Map config = const {}]) + : _entries = _ConfigParser._parseEntryConfig(config), + super( + id: ruleId, + severity: readSeverity(config, Severity.style), + excludes: readExcludes(config), + ); + + @override + Iterable check(InternalResolvedUnitResult source) { + final visitor = _Visitor(_entries); + + source.unit.visitChildren(visitor); + + return visitor.nodes + .map( + (node) => createIssue( + rule: this, + location: nodeLocation(node: node.node, source: source), + message: node.message, + ), + ) + .toList(growable: false); + } +} diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/utils/config_parser.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/utils/config_parser.dart new file mode 100644 index 0000000000..98d019e018 --- /dev/null +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/utils/config_parser.dart @@ -0,0 +1,27 @@ +part of '../avoid_banned_imports_rule.dart'; + +const _entriesLabel = 'entries'; +const _identLabel = 'ident'; +const _descriptionLabel = 'description'; + +/// Parser for rule configuration. +class _ConfigParser { + static List<_BanNameConfigEntry> _parseEntryConfig( + Map config, + ) => + (config[_entriesLabel] as Iterable? ?? []).map((entry) { + final entryMap = entry as Map; + + return _BanNameConfigEntry( + ident: entryMap[_identLabel] as String, + description: entryMap[_descriptionLabel] as String, + ); + }).toList(); +} + +class _BanNameConfigEntry { + final String ident; + final String description; + + _BanNameConfigEntry({required this.ident, required this.description}); +} diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart new file mode 100644 index 0000000000..ff5e8da104 --- /dev/null +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart @@ -0,0 +1,50 @@ +part of 'avoid_banned_imports_rule.dart'; + +class _Visitor extends RecursiveAstVisitor { + final Map _entryMap; + + final _nodes = <_NodeWithMessage>[]; + + Iterable<_NodeWithMessage> get nodes => _nodes; + + _Visitor(List<_BanNameConfigEntry> entries) + : _entryMap = Map.fromEntries(entries.map((e) => MapEntry(e.ident, e))); + + @override + void visitSimpleIdentifier(SimpleIdentifier node) { + super.visitSimpleIdentifier(node); + _visitIdent(node, node); + } + + @override + void visitPrefixedIdentifier(PrefixedIdentifier node) { + super.visitPrefixedIdentifier(node); + _visitIdent(node, node.identifier); + _visitIdent(node, node.prefix); + } + + @override + void visitLibraryIdentifier(LibraryIdentifier node) { + super.visitLibraryIdentifier(node); + for (final component in node.components) { + _visitIdent(node, component); + } + } + + void _visitIdent(Expression node, SimpleIdentifier ident) { + final name = ident.name; + if (_entryMap.containsKey(name)) { + _nodes.add(_NodeWithMessage( + node, + '${_entryMap[name]!.description} ($name is banned)', + )); + } + } +} + +class _NodeWithMessage { + final Expression node; + final String message; + + _NodeWithMessage(this.node, this.message); +} From b1a0614686a6d1cd292d7c69e32dff76a84d88ee Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 26 May 2022 10:46:43 +0800 Subject: [PATCH 31/50] add to factory --- lib/src/analyzers/lint_analyzer/rules/rules_factory.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart b/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart index 364f283b8a..3333ee6d11 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart @@ -1,5 +1,6 @@ import 'models/rule.dart'; import 'rules_list/always_remove_listener/always_remove_listener_rule.dart'; +import 'rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart'; import 'rules_list/avoid_border_all/avoid_border_all_rule.dart'; import 'rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart'; import 'rules_list/avoid_dynamic/avoid_dynamic_rule.dart'; @@ -54,6 +55,7 @@ import 'rules_list/tag_name/tag_name_rule.dart'; final _implementedRules = )>{ AlwaysRemoveListenerRule.ruleId: (config) => AlwaysRemoveListenerRule(config), + AvoidBannedImportsRule.ruleId: (config) => AvoidBannedImportsRule(config), AvoidBorderAllRule.ruleId: (config) => AvoidBorderAllRule(config), AvoidCollectionMethodsWithUnrelatedTypesRule.ruleId: (config) => AvoidCollectionMethodsWithUnrelatedTypesRule(config), From bbe6a8e5862db83412e2c2e2b29d107d4ea9b85c Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 26 May 2022 10:50:31 +0800 Subject: [PATCH 32/50] parse config --- .../avoid_banned_imports_rule.dart | 2 +- .../utils/config_parser.dart | 29 ++++++++++++------- .../avoid_banned_imports/visitor.dart | 4 +-- .../docs/rules/common/avoid-banned-imports.md | 15 +++++----- 4 files changed, 30 insertions(+), 20 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart index 2eb9be71f9..a919929b08 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart @@ -17,7 +17,7 @@ part 'utils/config_parser.dart'; class AvoidBannedImportsRule extends CommonRule { static const String ruleId = 'avoid-banned-imports'; - final List<_BanNameConfigEntry> _entries; + final List<_AvoidBannedImportsConfigEntry> _entries; AvoidBannedImportsRule([Map config = const {}]) : _entries = _ConfigParser._parseEntryConfig(config), diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/utils/config_parser.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/utils/config_parser.dart index 98d019e018..e4994d6e94 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/utils/config_parser.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/utils/config_parser.dart @@ -1,27 +1,36 @@ part of '../avoid_banned_imports_rule.dart'; const _entriesLabel = 'entries'; -const _identLabel = 'ident'; -const _descriptionLabel = 'description'; +const _pathsLabel = 'paths'; +const _denyLabel = 'deny'; +const _messageLabel = 'message'; /// Parser for rule configuration. class _ConfigParser { - static List<_BanNameConfigEntry> _parseEntryConfig( + static List<_AvoidBannedImportsConfigEntry> _parseEntryConfig( Map config, ) => (config[_entriesLabel] as Iterable? ?? []).map((entry) { final entryMap = entry as Map; - return _BanNameConfigEntry( - ident: entryMap[_identLabel] as String, - description: entryMap[_descriptionLabel] as String, + return _AvoidBannedImportsConfigEntry( + paths: _parseListString(entryMap[_pathsLabel]), + deny: _parseListString(entryMap[_denyLabel]), + message: entryMap[_messageLabel] as String, ); }).toList(); + + static List _parseListString(Object? object) => (object! as List).map((e) => e! as String).toList(); } -class _BanNameConfigEntry { - final String ident; - final String description; +class _AvoidBannedImportsConfigEntry { + final List paths; + final List deny; + final String message; - _BanNameConfigEntry({required this.ident, required this.description}); + _AvoidBannedImportsConfigEntry({ + required this.paths, + required this.deny, + required this.message, + }); } diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart index ff5e8da104..f35fd6f27d 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart @@ -1,13 +1,13 @@ part of 'avoid_banned_imports_rule.dart'; class _Visitor extends RecursiveAstVisitor { - final Map _entryMap; + final Map _entryMap; final _nodes = <_NodeWithMessage>[]; Iterable<_NodeWithMessage> get nodes => _nodes; - _Visitor(List<_BanNameConfigEntry> entries) + _Visitor(List<_AvoidBannedImportsConfigEntry> entries) : _entryMap = Map.fromEntries(entries.map((e) => MapEntry(e.ident, e))); @override diff --git a/website/docs/rules/common/avoid-banned-imports.md b/website/docs/rules/common/avoid-banned-imports.md index 4ac8fd36f7..4180d92701 100644 --- a/website/docs/rules/common/avoid-banned-imports.md +++ b/website/docs/rules/common/avoid-banned-imports.md @@ -36,11 +36,12 @@ dart_code_metrics: ... rules: ... - - avoid_restricted_imports - - paths: ["some/**/folder/*.dart", "another/folder/**.dart"] - deny: ["package:flutter/material.dart"] - message: "Do not import Flutter Material Design library, we should not depend on it!" - - paths: ["core/*.dart"] - deny: ["package:flutter_bloc/flutter_bloc.dart"] - message: 'State management should be not used inside "core" folder.' + - avoid_restricted_imports: + entries: + - paths: ["some/**/folder/*.dart", "another/folder/**.dart"] + deny: ["package:flutter/material.dart"] + message: "Do not import Flutter Material Design library, we should not depend on it!" + - paths: ["core/*.dart"] + deny: ["package:flutter_bloc/flutter_bloc.dart"] + message: 'State management should be not used inside "core" folder.' ``` From 2f09f5f59f84e3c9fbe46c2a3bdf9b186e1c42d7 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 26 May 2022 11:08:25 +0800 Subject: [PATCH 33/50] add empty visitor --- .../avoid_banned_imports/visitor.dart | 35 +++++-------------- 1 file changed, 8 insertions(+), 27 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart index f35fd6f27d..e13b9a1260 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart @@ -1,49 +1,30 @@ part of 'avoid_banned_imports_rule.dart'; class _Visitor extends RecursiveAstVisitor { - final Map _entryMap; + final List<_AvoidBannedImportsConfigEntry> entries; final _nodes = <_NodeWithMessage>[]; Iterable<_NodeWithMessage> get nodes => _nodes; - _Visitor(List<_AvoidBannedImportsConfigEntry> entries) - : _entryMap = Map.fromEntries(entries.map((e) => MapEntry(e.ident, e))); + _Visitor(this.entries); @override - void visitSimpleIdentifier(SimpleIdentifier node) { - super.visitSimpleIdentifier(node); - _visitIdent(node, node); - } - - @override - void visitPrefixedIdentifier(PrefixedIdentifier node) { - super.visitPrefixedIdentifier(node); - _visitIdent(node, node.identifier); - _visitIdent(node, node.prefix); - } - - @override - void visitLibraryIdentifier(LibraryIdentifier node) { - super.visitLibraryIdentifier(node); - for (final component in node.components) { - _visitIdent(node, component); - } - } + void visitImportDirective(ImportDirective node) { + print('hi node=$node'); - void _visitIdent(Expression node, SimpleIdentifier ident) { - final name = ident.name; - if (_entryMap.containsKey(name)) { + // TODO + if (false) { _nodes.add(_NodeWithMessage( node, - '${_entryMap[name]!.description} ($name is banned)', + 'TODO', )); } } } class _NodeWithMessage { - final Expression node; + final AstNode node; final String message; _NodeWithMessage(this.node, this.message); From 8eb1426321baa18482582a27dcc6dc4b1b0edc62 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 26 May 2022 11:10:01 +0800 Subject: [PATCH 34/50] add empty test --- .../avoid_banned_imports_rule_test.dart | 46 +++++++++++++++++++ .../examples/example.dart | 17 +++++++ 2 files changed, 63 insertions(+) create mode 100644 test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule_test.dart create mode 100644 test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/examples/example.dart diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule_test.dart new file mode 100644 index 0000000000..0d0d725056 --- /dev/null +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule_test.dart @@ -0,0 +1,46 @@ +import 'package:dart_code_metrics/src/analyzers/lint_analyzer/models/severity.dart'; +import 'package:dart_code_metrics/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart'; +import 'package:test/test.dart'; + +import '../../../../../helpers/rule_test_helper.dart'; + +const _examplePath = 'avoid_banned_imports/examples/example.dart'; + +void main() { + group('AvoidBannedImportsRule', () { + test('initialization', () async { + final unit = await RuleTestHelper.resolveFromFile(_examplePath); + final issues = AvoidBannedImportsRule().check(unit); + + RuleTestHelper.verifyInitialization( + issues: issues, + ruleId: 'ban-name', + severity: Severity.style, + ); + }); + + test('reports about all found issues in example.dart', () async { + final unit = await RuleTestHelper.resolveFromFile(_examplePath); + + final issues = AvoidBannedImportsRule({ + 'entries': [ + // TODO + // { + // 'paths': [TODO], + // 'deny': [TODO], + // 'message': TODO, + // }, + ], + }).check(unit); + + // TODO + RuleTestHelper.verifyIssues( + issues: issues, + startLines: [], + startColumns: [], + locationTexts: [], + messages: [], + ); + }); + }); +} diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/examples/example.dart new file mode 100644 index 0000000000..6b1b853b95 --- /dev/null +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/examples/example.dart @@ -0,0 +1,17 @@ +import 'dialog.dart'; +import 'dialog.dart' as material; + +void func() { + myShowDialog('some_arguments', 'another_argument'); + showDialog('some_arguments', 'another_argument'); // LINT + material.showDialog('some_arguments', 'another_argument'); // LINT + + var strangeName = 42; // LINT +} + +void strangeName() {} // LINT + +// LINT +class AnotherStrangeName { + late var strangeName; // LINT +} From 57d079f3be5a2e72f16d9c81abe8d6fe76f96e39 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 26 May 2022 11:12:59 +0800 Subject: [PATCH 35/50] add test data --- .../examples/example.dart | 20 ++++--------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/examples/example.dart index 6b1b853b95..c47df3e181 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/examples/example.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/examples/example.dart @@ -1,17 +1,5 @@ -import 'dialog.dart'; -import 'dialog.dart' as material; +import 'package:flutter/material.dart'; // LINT +import 'package:good_package/good_file.dart'; -void func() { - myShowDialog('some_arguments', 'another_argument'); - showDialog('some_arguments', 'another_argument'); // LINT - material.showDialog('some_arguments', 'another_argument'); // LINT - - var strangeName = 42; // LINT -} - -void strangeName() {} // LINT - -// LINT -class AnotherStrangeName { - late var strangeName; // LINT -} +import 'package:my_app/ban_folder/something.dart'; // LINT +import 'package:my_app/good_folder/something.dart'; From 1b6be7c8d33abf9da8a1de0073b55a764c33f47f Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 26 May 2022 11:13:15 +0800 Subject: [PATCH 36/50] fix wrong name --- .../avoid_banned_imports/avoid_banned_imports_rule_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule_test.dart index 0d0d725056..0035b6b544 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule_test.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule_test.dart @@ -14,7 +14,7 @@ void main() { RuleTestHelper.verifyInitialization( issues: issues, - ruleId: 'ban-name', + ruleId: 'avoid-banned-imports', severity: Severity.style, ); }); From d0323d0b8b7b112c018be10c3092691b913be052 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 26 May 2022 11:18:08 +0800 Subject: [PATCH 37/50] try to implement logic --- .../avoid_banned_imports_rule.dart | 6 +++- .../avoid_banned_imports/visitor.dart | 28 ++++++++++++------- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart index a919929b08..7ec2bd7d04 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart @@ -12,6 +12,7 @@ import '../../models/common_rule.dart'; import '../../rule_utils.dart'; part 'visitor.dart'; + part 'utils/config_parser.dart'; class AvoidBannedImportsRule extends CommonRule { @@ -29,7 +30,10 @@ class AvoidBannedImportsRule extends CommonRule { @override Iterable check(InternalResolvedUnitResult source) { - final visitor = _Visitor(_entries); + final activeEntries = + _entries.where((entry) => entry.paths.any((path) => _globMatch(pattern: path, data: source.path))).toList(); + + final visitor = _Visitor(activeEntries); source.unit.visitChildren(visitor); diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart index e13b9a1260..925c4d8c0a 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart @@ -1,24 +1,30 @@ part of 'avoid_banned_imports_rule.dart'; class _Visitor extends RecursiveAstVisitor { - final List<_AvoidBannedImportsConfigEntry> entries; + final List<_AvoidBannedImportsConfigEntry> _activeEntries; final _nodes = <_NodeWithMessage>[]; Iterable<_NodeWithMessage> get nodes => _nodes; - _Visitor(this.entries); + _Visitor(this._activeEntries); @override void visitImportDirective(ImportDirective node) { - print('hi node=$node'); - - // TODO - if (false) { - _nodes.add(_NodeWithMessage( - node, - 'TODO', - )); + // print('hi node=$node uri=${node.uri} uri.stringValue=${node.uri.stringValue}'); + + final uri = node.uri.stringValue; + if (uri == null) { + return; + } + + for (final entry in _activeEntries) { + if (entry.deny.any((deny) => _globMatch(pattern: deny, data: uri))) { + _nodes.add(_NodeWithMessage( + node, + 'Avoid banned imports (${entry.message})', + )); + } } } } @@ -29,3 +35,5 @@ class _NodeWithMessage { _NodeWithMessage(this.node, this.message); } + +bool _globMatch({required String pattern, required String data}) => TODO; From 1c1fb12f98f451d9233073257ab1b50449595152 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 26 May 2022 11:20:52 +0800 Subject: [PATCH 38/50] implement regexp --- .../avoid_banned_imports_rule.dart | 2 +- .../avoid_banned_imports/utils/config_parser.dart | 11 ++++++----- .../rules_list/avoid_banned_imports/visitor.dart | 4 +--- website/docs/rules/common/avoid-banned-imports.md | 6 ++++-- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart index 7ec2bd7d04..f63d5a0b01 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart @@ -31,7 +31,7 @@ class AvoidBannedImportsRule extends CommonRule { @override Iterable check(InternalResolvedUnitResult source) { final activeEntries = - _entries.where((entry) => entry.paths.any((path) => _globMatch(pattern: path, data: source.path))).toList(); + _entries.where((entry) => entry.paths.any((path) => path.hasMatch(source.path))).toList(); final visitor = _Visitor(activeEntries); diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/utils/config_parser.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/utils/config_parser.dart index e4994d6e94..c8a8a7a2a5 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/utils/config_parser.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/utils/config_parser.dart @@ -14,18 +14,19 @@ class _ConfigParser { final entryMap = entry as Map; return _AvoidBannedImportsConfigEntry( - paths: _parseListString(entryMap[_pathsLabel]), - deny: _parseListString(entryMap[_denyLabel]), + paths: _parseListRegExp(entryMap[_pathsLabel]), + deny: _parseListRegExp(entryMap[_denyLabel]), message: entryMap[_messageLabel] as String, ); }).toList(); - static List _parseListString(Object? object) => (object! as List).map((e) => e! as String).toList(); + static List _parseListRegExp(Object? object) => + (object! as List).map((e) => e! as String).map((e) => RegExp(e)).toList(); } class _AvoidBannedImportsConfigEntry { - final List paths; - final List deny; + final List paths; + final List deny; final String message; _AvoidBannedImportsConfigEntry({ diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart index 925c4d8c0a..6be11fe47e 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart @@ -19,7 +19,7 @@ class _Visitor extends RecursiveAstVisitor { } for (final entry in _activeEntries) { - if (entry.deny.any((deny) => _globMatch(pattern: deny, data: uri))) { + if (entry.deny.any((deny) => deny.hasMatch(uri))) { _nodes.add(_NodeWithMessage( node, 'Avoid banned imports (${entry.message})', @@ -35,5 +35,3 @@ class _NodeWithMessage { _NodeWithMessage(this.node, this.message); } - -bool _globMatch({required String pattern, required String data}) => TODO; diff --git a/website/docs/rules/common/avoid-banned-imports.md b/website/docs/rules/common/avoid-banned-imports.md index 4180d92701..48cf96b755 100644 --- a/website/docs/rules/common/avoid-banned-imports.md +++ b/website/docs/rules/common/avoid-banned-imports.md @@ -31,6 +31,8 @@ Good: ### Config example {#config-example} +The `paths` and `deny` both support regular expressions. + ```yaml dart_code_metrics: ... @@ -38,10 +40,10 @@ dart_code_metrics: ... - avoid_restricted_imports: entries: - - paths: ["some/**/folder/*.dart", "another/folder/**.dart"] + - paths: ["some/folder/.*\.dart", "another/folder/.*\.dart"] deny: ["package:flutter/material.dart"] message: "Do not import Flutter Material Design library, we should not depend on it!" - - paths: ["core/*.dart"] + - paths: ["core/.*\.dart"] deny: ["package:flutter_bloc/flutter_bloc.dart"] message: 'State management should be not used inside "core" folder.' ``` From 3a4fb0e33304ec8265a34774689a30fc2b809c56 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 26 May 2022 11:22:06 +0800 Subject: [PATCH 39/50] add tests --- .../avoid_banned_imports_rule_test.dart | 31 ++++++++++++------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule_test.dart index 0035b6b544..7061c621ca 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule_test.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule_test.dart @@ -24,22 +24,31 @@ void main() { final issues = AvoidBannedImportsRule({ 'entries': [ - // TODO - // { - // 'paths': [TODO], - // 'deny': [TODO], - // 'message': TODO, - // }, + { + 'paths': [ + r'.*examples.*\.dart', + ], + 'deny': [ + 'package:flutter/.*', + 'package:my_app/ban_folder/.*', + ], + 'message': 'sample message', + }, ], }).check(unit); - // TODO RuleTestHelper.verifyIssues( issues: issues, - startLines: [], - startColumns: [], - locationTexts: [], - messages: [], + startLines: [1, 4], + startColumns: [1, 1], + locationTexts: [ + "import 'package:flutter/material.dart';", + "import 'package:my_app/ban_folder/something.dart';", + ], + messages: [ + 'Avoid banned imports (sample message)', + 'Avoid banned imports (sample message)', + ], ); }); }); From cc2acfe82f0ce88801dd2d9e26168eed2c1b5524 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 26 May 2022 11:24:01 +0800 Subject: [PATCH 40/50] format code --- .../avoid_banned_imports/avoid_banned_imports_rule.dart | 5 +++-- .../rules_list/avoid_banned_imports/utils/config_parser.dart | 5 ++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart index f63d5a0b01..36b3a8781b 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart @@ -30,8 +30,9 @@ class AvoidBannedImportsRule extends CommonRule { @override Iterable check(InternalResolvedUnitResult source) { - final activeEntries = - _entries.where((entry) => entry.paths.any((path) => path.hasMatch(source.path))).toList(); + final activeEntries = _entries + .where((entry) => entry.paths.any((path) => path.hasMatch(source.path))) + .toList(); final visitor = _Visitor(activeEntries); diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/utils/config_parser.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/utils/config_parser.dart index c8a8a7a2a5..00f4f4dd4b 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/utils/config_parser.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/utils/config_parser.dart @@ -21,7 +21,10 @@ class _ConfigParser { }).toList(); static List _parseListRegExp(Object? object) => - (object! as List).map((e) => e! as String).map((e) => RegExp(e)).toList(); + (object! as List) + .map((e) => e! as String) + .map((e) => RegExp(e)) + .toList(); } class _AvoidBannedImportsConfigEntry { From 090b44b62ca536c37441f6cd3aa0d8251e3a4fbc Mon Sep 17 00:00:00 2001 From: fzyzcjy <5236035+fzyzcjy@users.noreply.github.com> Date: Sat, 28 May 2022 21:26:28 +0800 Subject: [PATCH 41/50] Update visitor.dart --- .../rules/rules_list/avoid_banned_imports/visitor.dart | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart index 6be11fe47e..12c2aa8d0c 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart @@ -7,12 +7,10 @@ class _Visitor extends RecursiveAstVisitor { Iterable<_NodeWithMessage> get nodes => _nodes; - _Visitor(this._activeEntries); + _Visitor(this._activeEntries);f @override void visitImportDirective(ImportDirective node) { - // print('hi node=$node uri=${node.uri} uri.stringValue=${node.uri.stringValue}'); - final uri = node.uri.stringValue; if (uri == null) { return; From be0b5cf7fe51e5a3b9cc2be411c421cf4b957044 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Wed, 8 Jun 2022 18:13:31 +0800 Subject: [PATCH 42/50] remove strange char --- .../rules/rules_list/avoid_banned_imports/visitor.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart index 12c2aa8d0c..8913bbe74a 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart @@ -7,7 +7,7 @@ class _Visitor extends RecursiveAstVisitor { Iterable<_NodeWithMessage> get nodes => _nodes; - _Visitor(this._activeEntries);f + _Visitor(this._activeEntries); @override void visitImportDirective(ImportDirective node) { From 80908dd2d05a4752bbeb64f78aea77d6358127a7 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Wed, 8 Jun 2022 18:26:45 +0800 Subject: [PATCH 43/50] add tests --- .../examples/example_max_line_count.dart | 31 +++++++++++++++++++ ...prefer_extracting_callbacks_rule_test.dart | 23 +++++++++++++- 2 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/examples/example_max_line_count.dart diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/examples/example_max_line_count.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/examples/example_max_line_count.dart new file mode 100644 index 0000000000..dc1b44913e --- /dev/null +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/examples/example_max_line_count.dart @@ -0,0 +1,31 @@ +class MyWidget extends StatelessWidget { + @override + Widget build(BuildContext context) { + const a = TextButton( + onPressed: () => null, + child: Container(), + ); + + const b = TextButton( + onPressed: () { + firstLine(); + secondLine(); + thirdLine(); + }, + child: Container(), + ); + + const c = TextButton( + // LINT + onPressed: () { + firstLine(); + secondLine(); + thirdLine(); + fourthLine(); + }, + child: Container(), + ); + + return Container(); + } +} diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule_test.dart index d3d7042db0..81012becb0 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule_test.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule_test.dart @@ -5,6 +5,7 @@ import 'package:test/test.dart'; import '../../../../../helpers/rule_test_helper.dart'; const _examplePath = 'prefer_extracting_callbacks/examples/example.dart'; +const _exampleMaxLineCountPath = 'prefer_extracting_callbacks/examples/example_max_line_count.dart'; void main() { group('PreferExtractingCallbacksRule', () { @@ -46,7 +47,7 @@ void main() { ); }); - test('with custom config reports no issues', () async { + test('with ignored-named-arguments config reports no issues', () async { final unit = await RuleTestHelper.resolveFromFile(_examplePath); final config = { 'ignored-named-arguments': [ @@ -58,5 +59,25 @@ void main() { RuleTestHelper.verifyNoIssues(issues); }); + + test('with max-line-count config', () async { + final unit = await RuleTestHelper.resolveFromFile(_examplePath); + final config = { + 'max-line-count': 3, + }; + + final issues = PreferExtractingCallbacksRule(config).check(unit); + + // TODO + RuleTestHelper.verifyIssues( + issues: issues, + startLines: [], + startColumns: [], + locationTexts: [], + messages: [ + 'Prefer extracting the callback to a separate widget method.', + ], + ); + }); }); } From af80c4bfd9d4fa300d80ec2e16ce0099450a6688 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Wed, 8 Jun 2022 18:28:54 +0800 Subject: [PATCH 44/50] parse config --- .../prefer_extracting_callbacks/config_parser.dart | 6 ++++++ .../prefer_extracting_callbacks_rule.dart | 2 ++ 2 files changed, 8 insertions(+) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/config_parser.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/config_parser.dart index 9c05652ccd..e878a57962 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/config_parser.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/config_parser.dart @@ -2,10 +2,16 @@ part of 'prefer_extracting_callbacks_rule.dart'; class _ConfigParser { static const _ignoredArgumentsConfig = 'ignored-named-arguments'; + static const _maxLineCountConfig = 'max-line-count'; static Iterable parseIgnoredArguments(Map config) => config.containsKey(_ignoredArgumentsConfig) && config[_ignoredArgumentsConfig] is Iterable ? List.from(config[_ignoredArgumentsConfig] as Iterable) : []; + + static int? parseMaxLineCount(Map config) { + final raw = config[_maxLineCountConfig]; + return raw is int? ? raw : null; + } } diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule.dart index c432083266..27e6d71316 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule.dart @@ -22,9 +22,11 @@ class PreferExtractingCallbacksRule extends FlutterRule { 'Prefer extracting the callback to a separate widget method.'; final Iterable _ignoredArguments; + final int? _maxLineCount; PreferExtractingCallbacksRule([Map config = const {}]) : _ignoredArguments = _ConfigParser.parseIgnoredArguments(config), + _maxLineCount = _ConfigParser.parseMaxLineCount(config), super( id: ruleId, severity: readSeverity(config, Severity.style), From f4e4625427c906abe40f7c34925fee4316372bb2 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Wed, 8 Jun 2022 18:29:35 +0800 Subject: [PATCH 45/50] pass down config --- .../prefer_extracting_callbacks_rule.dart | 2 +- .../rules_list/prefer_extracting_callbacks/visitor.dart | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule.dart index 27e6d71316..45e3ebea06 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule.dart @@ -35,7 +35,7 @@ class PreferExtractingCallbacksRule extends FlutterRule { @override Iterable check(InternalResolvedUnitResult source) { - final visitor = _Visitor(_ignoredArguments); + final visitor = _Visitor(_ignoredArguments, _maxLineCount); source.unit.visitChildren(visitor); diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/visitor.dart index ed09849a2f..7454f9b624 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/visitor.dart @@ -4,10 +4,11 @@ class _Visitor extends SimpleAstVisitor { final _expressions = []; final Iterable _ignoredArguments; + final int? _maxLineCount; Iterable get expressions => _expressions; - _Visitor(this._ignoredArguments); + _Visitor(this._ignoredArguments, this._maxLineCount); @override void visitClassDeclaration(ClassDeclaration node) { @@ -17,7 +18,7 @@ class _Visitor extends SimpleAstVisitor { return; } - final visitor = _InstanceCreationVisitor(_ignoredArguments); + final visitor = _InstanceCreationVisitor(_ignoredArguments, _maxLineCount); node.visitChildren(visitor); _expressions.addAll(visitor.expressions); @@ -28,10 +29,11 @@ class _InstanceCreationVisitor extends RecursiveAstVisitor { final _expressions = []; final Iterable _ignoredArguments; + final int? _maxLineCount; Iterable get expressions => _expressions; - _InstanceCreationVisitor(this._ignoredArguments); + _InstanceCreationVisitor(this._ignoredArguments, this._maxLineCount); @override void visitInstanceCreationExpression(InstanceCreationExpression node) { From 0ef1776fb24dc86288cfefeb835ffd7ab7a22e04 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Wed, 8 Jun 2022 18:31:07 +0800 Subject: [PATCH 46/50] rename --- .../config_parser.dart | 6 +++--- .../prefer_extracting_callbacks_rule.dart | 6 +++--- .../prefer_extracting_callbacks/visitor.dart | 18 ++++++++++++------ .../prefer_extracting_callbacks_rule_test.dart | 4 ++-- 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/config_parser.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/config_parser.dart index e878a57962..01be76659d 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/config_parser.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/config_parser.dart @@ -2,7 +2,7 @@ part of 'prefer_extracting_callbacks_rule.dart'; class _ConfigParser { static const _ignoredArgumentsConfig = 'ignored-named-arguments'; - static const _maxLineCountConfig = 'max-line-count'; + static const _allowedLineCountConfig = 'allowed-line-count'; static Iterable parseIgnoredArguments(Map config) => config.containsKey(_ignoredArgumentsConfig) && @@ -10,8 +10,8 @@ class _ConfigParser { ? List.from(config[_ignoredArgumentsConfig] as Iterable) : []; - static int? parseMaxLineCount(Map config) { - final raw = config[_maxLineCountConfig]; + static int? parseAllowedLineCount(Map config) { + final raw = config[_allowedLineCountConfig]; return raw is int? ? raw : null; } } diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule.dart index 45e3ebea06..32f0cefd6c 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule.dart @@ -22,11 +22,11 @@ class PreferExtractingCallbacksRule extends FlutterRule { 'Prefer extracting the callback to a separate widget method.'; final Iterable _ignoredArguments; - final int? _maxLineCount; + final int? _allowedLineCount; PreferExtractingCallbacksRule([Map config = const {}]) : _ignoredArguments = _ConfigParser.parseIgnoredArguments(config), - _maxLineCount = _ConfigParser.parseMaxLineCount(config), + _allowedLineCount = _ConfigParser.parseAllowedLineCount(config), super( id: ruleId, severity: readSeverity(config, Severity.style), @@ -35,7 +35,7 @@ class PreferExtractingCallbacksRule extends FlutterRule { @override Iterable check(InternalResolvedUnitResult source) { - final visitor = _Visitor(_ignoredArguments, _maxLineCount); + final visitor = _Visitor(_ignoredArguments, _allowedLineCount); source.unit.visitChildren(visitor); diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/visitor.dart index 7454f9b624..f7c9204c89 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/visitor.dart @@ -4,11 +4,11 @@ class _Visitor extends SimpleAstVisitor { final _expressions = []; final Iterable _ignoredArguments; - final int? _maxLineCount; + final int? _allowedLineCount; Iterable get expressions => _expressions; - _Visitor(this._ignoredArguments, this._maxLineCount); + _Visitor(this._ignoredArguments, this._allowedLineCount); @override void visitClassDeclaration(ClassDeclaration node) { @@ -18,7 +18,7 @@ class _Visitor extends SimpleAstVisitor { return; } - final visitor = _InstanceCreationVisitor(_ignoredArguments, _maxLineCount); + final visitor = _InstanceCreationVisitor(_ignoredArguments, _allowedLineCount); node.visitChildren(visitor); _expressions.addAll(visitor.expressions); @@ -29,11 +29,11 @@ class _InstanceCreationVisitor extends RecursiveAstVisitor { final _expressions = []; final Iterable _ignoredArguments; - final int? _maxLineCount; + final int? _allowedLineCount; Iterable get expressions => _expressions; - _InstanceCreationVisitor(this._ignoredArguments, this._maxLineCount); + _InstanceCreationVisitor(this._ignoredArguments, this._allowedLineCount); @override void visitInstanceCreationExpression(InstanceCreationExpression node) { @@ -46,7 +46,8 @@ class _InstanceCreationVisitor extends RecursiveAstVisitor { if (_isNotIgnored(argument) && expression is FunctionExpression && _hasNotEmptyBlockBody(expression) && - !_isFlutterBuilder(expression)) { + !_isFlutterBuilder(expression) && + _isLongEnough(expression)) { _expressions.add(argument); } } @@ -76,4 +77,9 @@ class _InstanceCreationVisitor extends RecursiveAstVisitor { bool _isNotIgnored(Expression argument) => argument is! NamedExpression || !_ignoredArguments.contains(argument.name.label.name); + + bool _isLongEnough(Expression expression) { + final lineCount = TODO; + return lineCount >= _allowedLineCount; + } } diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule_test.dart index 81012becb0..5505759a20 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule_test.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule_test.dart @@ -60,10 +60,10 @@ void main() { RuleTestHelper.verifyNoIssues(issues); }); - test('with max-line-count config', () async { + test('with allowed-line-count config', () async { final unit = await RuleTestHelper.resolveFromFile(_examplePath); final config = { - 'max-line-count': 3, + 'allowed-line-count': 3, }; final issues = PreferExtractingCallbacksRule(config).check(unit); From 5edbc4204c49c278d03a616e23fcc6c5e1e41485 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Wed, 8 Jun 2022 18:32:11 +0800 Subject: [PATCH 47/50] update doc --- website/docs/rules/flutter/prefer-extracting-callbacks.md | 1 + 1 file changed, 1 insertion(+) diff --git a/website/docs/rules/flutter/prefer-extracting-callbacks.md b/website/docs/rules/flutter/prefer-extracting-callbacks.md index 6cf979ce2a..0ee4a29af6 100644 --- a/website/docs/rules/flutter/prefer-extracting-callbacks.md +++ b/website/docs/rules/flutter/prefer-extracting-callbacks.md @@ -31,6 +31,7 @@ dart_code_metrics: - prefer-extracting-callbacks: ignored-named-arguments: - onPressed + allowed-line-count: 3 ``` ### Example {#example} From aa4d315f97bf39b46df4bc88c5f5c1f6c698836c Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Wed, 8 Jun 2022 18:37:27 +0800 Subject: [PATCH 48/50] try to implement it --- .../prefer_extracting_callbacks_rule.dart | 3 ++- .../prefer_extracting_callbacks/visitor.dart | 19 ++++++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule.dart index 32f0cefd6c..b5994d0966 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule.dart @@ -6,6 +6,7 @@ import 'package:analyzer/dart/ast/visitor.dart'; import '../../../../../utils/flutter_types_utils.dart'; import '../../../../../utils/node_utils.dart'; import '../../../lint_utils.dart'; +import '../../../metrics/metrics_list/source_lines_of_code/source_code_visitor.dart'; import '../../../models/internal_resolved_unit_result.dart'; import '../../../models/issue.dart'; import '../../../models/severity.dart'; @@ -35,7 +36,7 @@ class PreferExtractingCallbacksRule extends FlutterRule { @override Iterable check(InternalResolvedUnitResult source) { - final visitor = _Visitor(_ignoredArguments, _allowedLineCount); + final visitor = _Visitor(source, _ignoredArguments, _allowedLineCount); source.unit.visitChildren(visitor); diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/visitor.dart index f7c9204c89..dd674b6e0d 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/visitor.dart @@ -3,12 +3,13 @@ part of 'prefer_extracting_callbacks_rule.dart'; class _Visitor extends SimpleAstVisitor { final _expressions = []; + final InternalResolvedUnitResult _source; final Iterable _ignoredArguments; final int? _allowedLineCount; Iterable get expressions => _expressions; - _Visitor(this._ignoredArguments, this._allowedLineCount); + _Visitor(this._source, this._ignoredArguments, this._allowedLineCount); @override void visitClassDeclaration(ClassDeclaration node) { @@ -18,7 +19,7 @@ class _Visitor extends SimpleAstVisitor { return; } - final visitor = _InstanceCreationVisitor(_ignoredArguments, _allowedLineCount); + final visitor = _InstanceCreationVisitor(_source, _ignoredArguments, _allowedLineCount); node.visitChildren(visitor); _expressions.addAll(visitor.expressions); @@ -28,12 +29,13 @@ class _Visitor extends SimpleAstVisitor { class _InstanceCreationVisitor extends RecursiveAstVisitor { final _expressions = []; + final InternalResolvedUnitResult _source; final Iterable _ignoredArguments; final int? _allowedLineCount; Iterable get expressions => _expressions; - _InstanceCreationVisitor(this._ignoredArguments, this._allowedLineCount); + _InstanceCreationVisitor(this._source, this._ignoredArguments, this._allowedLineCount); @override void visitInstanceCreationExpression(InstanceCreationExpression node) { @@ -79,7 +81,14 @@ class _InstanceCreationVisitor extends RecursiveAstVisitor { !_ignoredArguments.contains(argument.name.label.name); bool _isLongEnough(Expression expression) { - final lineCount = TODO; - return lineCount >= _allowedLineCount; + final allowedLineCount = _allowedLineCount; + if (allowedLineCount == null) { + return true; + } + + final visitor = SourceCodeVisitor(_source.lineInfo); + expression.visitChildren(visitor); + + return visitor.linesWithCode.length > allowedLineCount; } } From 17695b4f161d8a5f85ed912fadf5f17e24511bfd Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Wed, 8 Jun 2022 18:38:15 +0800 Subject: [PATCH 49/50] fix tests --- .../examples/example_max_line_count.dart | 28 +++++++++++++++++++ ...prefer_extracting_callbacks_rule_test.dart | 16 +++++++---- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/examples/example_max_line_count.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/examples/example_max_line_count.dart index dc1b44913e..a55edd7732 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/examples/example_max_line_count.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/examples/example_max_line_count.dart @@ -29,3 +29,31 @@ class MyWidget extends StatelessWidget { return Container(); } } + +class Widget {} + +class StatelessWidget extends Widget {} + +class BuildContext {} + +class Container extends Widget { + final Widget? child; + + const Container({this.child}); +} + +class TextButton { + final Widget child; + final void Function()? onPressed; + final Widget Function(BuildContext)? builder; + final Widget Function(BuildContext, int)? anotherBuilder; + final MyOtherWidget Function(BuildContext, int)? myOtherWidgetBuilder; + + const TextButton({ + required this.child, + required this.onPressed, + this.builder, + this.anotherBuilder, + this.myOtherWidgetBuilder, + }); +} diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule_test.dart index 5505759a20..0c3491cd41 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule_test.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule_test.dart @@ -61,19 +61,25 @@ void main() { }); test('with allowed-line-count config', () async { - final unit = await RuleTestHelper.resolveFromFile(_examplePath); + final unit = await RuleTestHelper.resolveFromFile(_exampleMaxLineCountPath); final config = { 'allowed-line-count': 3, }; final issues = PreferExtractingCallbacksRule(config).check(unit); - // TODO RuleTestHelper.verifyIssues( issues: issues, - startLines: [], - startColumns: [], - locationTexts: [], + startLines: [20], + startColumns: [7], + locationTexts: [ + 'onPressed: () {\n' + ' firstLine();\n' + ' secondLine();\n' + ' thirdLine();\n' + ' fourthLine();\n' + ' }', + ], messages: [ 'Prefer extracting the callback to a separate widget method.', ], From 66acc4d7fea4a68ca6437aa0a04fe58962b15b45 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Wed, 8 Jun 2022 18:41:18 +0800 Subject: [PATCH 50/50] format code --- .../rules_list/prefer_extracting_callbacks/visitor.dart | 6 ++++-- .../prefer_extracting_callbacks_rule_test.dart | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/visitor.dart index dd674b6e0d..6902f2a602 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/visitor.dart @@ -19,7 +19,8 @@ class _Visitor extends SimpleAstVisitor { return; } - final visitor = _InstanceCreationVisitor(_source, _ignoredArguments, _allowedLineCount); + final visitor = + _InstanceCreationVisitor(_source, _ignoredArguments, _allowedLineCount); node.visitChildren(visitor); _expressions.addAll(visitor.expressions); @@ -35,7 +36,8 @@ class _InstanceCreationVisitor extends RecursiveAstVisitor { Iterable get expressions => _expressions; - _InstanceCreationVisitor(this._source, this._ignoredArguments, this._allowedLineCount); + _InstanceCreationVisitor( + this._source, this._ignoredArguments, this._allowedLineCount); @override void visitInstanceCreationExpression(InstanceCreationExpression node) { diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule_test.dart index 0c3491cd41..94ccfb0c2b 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule_test.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/prefer_extracting_callbacks_rule_test.dart @@ -5,7 +5,8 @@ import 'package:test/test.dart'; import '../../../../../helpers/rule_test_helper.dart'; const _examplePath = 'prefer_extracting_callbacks/examples/example.dart'; -const _exampleMaxLineCountPath = 'prefer_extracting_callbacks/examples/example_max_line_count.dart'; +const _exampleMaxLineCountPath = + 'prefer_extracting_callbacks/examples/example_max_line_count.dart'; void main() { group('PreferExtractingCallbacksRule', () { @@ -61,7 +62,8 @@ void main() { }); test('with allowed-line-count config', () async { - final unit = await RuleTestHelper.resolveFromFile(_exampleMaxLineCountPath); + final unit = + await RuleTestHelper.resolveFromFile(_exampleMaxLineCountPath); final config = { 'allowed-line-count': 3, };