diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f766af470..5739706608 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * chore: restrict `analyzer` version to `>=2.4.0 <4.2.0`. * fix: normalize file paths after extraction from analyzed folder. * fix: improve context root included files calculation. +* feat: add [`avoid-banned-imports`](https://dartcodemetrics.dev/docs/rules/common/avoid-banned-imports) rule * fix: resolve package with imported analysis options. ## 4.15.2 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), 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..36b3a8781b --- /dev/null +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule.dart @@ -0,0 +1,51 @@ +// 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<_AvoidBannedImportsConfigEntry> _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 activeEntries = _entries + .where((entry) => entry.paths.any((path) => path.hasMatch(source.path))) + .toList(); + + final visitor = _Visitor(activeEntries); + + 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..00f4f4dd4b --- /dev/null +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/utils/config_parser.dart @@ -0,0 +1,40 @@ +part of '../avoid_banned_imports_rule.dart'; + +const _entriesLabel = 'entries'; +const _pathsLabel = 'paths'; +const _denyLabel = 'deny'; +const _messageLabel = 'message'; + +/// Parser for rule configuration. +class _ConfigParser { + static List<_AvoidBannedImportsConfigEntry> _parseEntryConfig( + Map config, + ) => + (config[_entriesLabel] as Iterable? ?? []).map((entry) { + final entryMap = entry as Map; + + return _AvoidBannedImportsConfigEntry( + paths: _parseListRegExp(entryMap[_pathsLabel]), + deny: _parseListRegExp(entryMap[_denyLabel]), + message: entryMap[_messageLabel] 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 String message; + + _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 new file mode 100644 index 0000000000..8913bbe74a --- /dev/null +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/visitor.dart @@ -0,0 +1,35 @@ +part of 'avoid_banned_imports_rule.dart'; + +class _Visitor extends RecursiveAstVisitor { + final List<_AvoidBannedImportsConfigEntry> _activeEntries; + + final _nodes = <_NodeWithMessage>[]; + + Iterable<_NodeWithMessage> get nodes => _nodes; + + _Visitor(this._activeEntries); + + @override + void visitImportDirective(ImportDirective node) { + final uri = node.uri.stringValue; + if (uri == null) { + return; + } + + for (final entry in _activeEntries) { + if (entry.deny.any((deny) => deny.hasMatch(uri))) { + _nodes.add(_NodeWithMessage( + node, + 'Avoid banned imports (${entry.message})', + )); + } + } + } +} + +class _NodeWithMessage { + final AstNode node; + final String message; + + _NodeWithMessage(this.node, this.message); +} 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..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,10 +2,16 @@ part of 'prefer_extracting_callbacks_rule.dart'; class _ConfigParser { static const _ignoredArgumentsConfig = 'ignored-named-arguments'; + static const _allowedLineCountConfig = 'allowed-line-count'; static Iterable parseIgnoredArguments(Map config) => config.containsKey(_ignoredArgumentsConfig) && config[_ignoredArgumentsConfig] is Iterable ? List.from(config[_ignoredArgumentsConfig] as Iterable) : []; + + 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 c432083266..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'; @@ -22,9 +23,11 @@ class PreferExtractingCallbacksRule extends FlutterRule { 'Prefer extracting the callback to a separate widget method.'; final Iterable _ignoredArguments; + final int? _allowedLineCount; PreferExtractingCallbacksRule([Map config = const {}]) : _ignoredArguments = _ConfigParser.parseIgnoredArguments(config), + _allowedLineCount = _ConfigParser.parseAllowedLineCount(config), super( id: ruleId, severity: readSeverity(config, Severity.style), @@ -33,7 +36,7 @@ class PreferExtractingCallbacksRule extends FlutterRule { @override Iterable check(InternalResolvedUnitResult source) { - final visitor = _Visitor(_ignoredArguments); + 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 ed09849a2f..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 @@ -3,11 +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); + _Visitor(this._source, this._ignoredArguments, this._allowedLineCount); @override void visitClassDeclaration(ClassDeclaration node) { @@ -17,7 +19,8 @@ class _Visitor extends SimpleAstVisitor { return; } - final visitor = _InstanceCreationVisitor(_ignoredArguments); + final visitor = + _InstanceCreationVisitor(_source, _ignoredArguments, _allowedLineCount); node.visitChildren(visitor); _expressions.addAll(visitor.expressions); @@ -27,11 +30,14 @@ 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); + _InstanceCreationVisitor( + this._source, this._ignoredArguments, this._allowedLineCount); @override void visitInstanceCreationExpression(InstanceCreationExpression node) { @@ -44,7 +50,8 @@ class _InstanceCreationVisitor extends RecursiveAstVisitor { if (_isNotIgnored(argument) && expression is FunctionExpression && _hasNotEmptyBlockBody(expression) && - !_isFlutterBuilder(expression)) { + !_isFlutterBuilder(expression) && + _isLongEnough(expression)) { _expressions.add(argument); } } @@ -74,4 +81,16 @@ class _InstanceCreationVisitor extends RecursiveAstVisitor { bool _isNotIgnored(Expression argument) => argument is! NamedExpression || !_ignoredArguments.contains(argument.name.label.name); + + bool _isLongEnough(Expression expression) { + final allowedLineCount = _allowedLineCount; + if (allowedLineCount == null) { + return true; + } + + final visitor = SourceCodeVisitor(_source.lineInfo); + expression.visitChildren(visitor); + + return visitor.linesWithCode.length > allowedLineCount; + } } 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..7061c621ca --- /dev/null +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/avoid_banned_imports_rule_test.dart @@ -0,0 +1,55 @@ +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: 'avoid-banned-imports', + severity: Severity.style, + ); + }); + + test('reports about all found issues in example.dart', () async { + final unit = await RuleTestHelper.resolveFromFile(_examplePath); + + final issues = AvoidBannedImportsRule({ + 'entries': [ + { + 'paths': [ + r'.*examples.*\.dart', + ], + 'deny': [ + 'package:flutter/.*', + 'package:my_app/ban_folder/.*', + ], + 'message': 'sample message', + }, + ], + }).check(unit); + + RuleTestHelper.verifyIssues( + issues: issues, + 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)', + ], + ); + }); + }); +} 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..c47df3e181 --- /dev/null +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_banned_imports/examples/example.dart @@ -0,0 +1,5 @@ +import 'package:flutter/material.dart'; // LINT +import 'package:good_package/good_file.dart'; + +import 'package:my_app/ban_folder/something.dart'; // LINT +import 'package:my_app/good_folder/something.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..a55edd7732 --- /dev/null +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/prefer_extracting_callbacks/examples/example_max_line_count.dart @@ -0,0 +1,59 @@ +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(); + } +} + +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 d3d7042db0..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,6 +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'; void main() { group('PreferExtractingCallbacksRule', () { @@ -46,7 +48,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 +60,32 @@ void main() { RuleTestHelper.verifyNoIssues(issues); }); + + test('with allowed-line-count config', () async { + final unit = + await RuleTestHelper.resolveFromFile(_exampleMaxLineCountPath); + final config = { + 'allowed-line-count': 3, + }; + + final issues = PreferExtractingCallbacksRule(config).check(unit); + + RuleTestHelper.verifyIssues( + issues: issues, + 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.', + ], + ); + }); }); } 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..48cf96b755 --- /dev/null +++ b/website/docs/rules/common/avoid-banned-imports.md @@ -0,0 +1,49 @@ +# 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} + +The `paths` and `deny` both support regular expressions. + +```yaml +dart_code_metrics: + ... + rules: + ... + - 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.' +``` 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} 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.