Skip to content
This repository has been archived by the owner on Jul 16, 2023. It is now read-only.

feat: add new rule avoid-banned-imports & modify prefer-extracting-callbacks allow short callbacks to exist #868

Merged
merged 58 commits into from Jun 16, 2022
Merged
Show file tree
Hide file tree
Changes from 57 commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
e225a30
add doc
fzyzcjy Feb 25, 2022
dd561c1
add empty ban_name_rule
fzyzcjy Feb 25, 2022
a1db691
add tests
fzyzcjy Feb 25, 2022
059c803
config parser
fzyzcjy Feb 25, 2022
27e338c
implement rule (without visitor)
fzyzcjy Feb 25, 2022
d81fc29
try to implement visitor
fzyzcjy Feb 25, 2022
ed5edb3
fix bugs
fzyzcjy Feb 25, 2022
6c30d46
enhance tests
fzyzcjy Feb 25, 2022
b41cfa9
fix wrong ruleid
fzyzcjy Feb 26, 2022
eafaaa5
change severity
fzyzcjy Feb 26, 2022
9113a63
rephrase
fzyzcjy Feb 26, 2022
3aa5b1c
changelog
fzyzcjy Feb 26, 2022
ff8105b
Merge branch 'master' into ban-name
incendial Feb 26, 2022
0139ef2
add more tests
fzyzcjy Feb 26, 2022
6fd7c99
scaffold for tag-name
fzyzcjy Feb 26, 2022
0250bf5
add tests
fzyzcjy Feb 26, 2022
e9fbd2c
partial impl
fzyzcjy Feb 26, 2022
c124eff
more implementations
fzyzcjy Feb 26, 2022
0eeb812
pass tests
fzyzcjy Feb 26, 2022
712b0f7
enhance tests and still pass tests
fzyzcjy Feb 26, 2022
8e21f5c
format code
fzyzcjy Feb 26, 2022
80f9637
Merge branch 'tag-name'
fzyzcjy Mar 1, 2022
6a6a2cc
Merge branch 'ban-name'
fzyzcjy Mar 1, 2022
f53b303
update changelog
fzyzcjy Mar 1, 2022
a6a65f5
revert bumping analyzer, otherwise flutter stable version cannot work
fzyzcjy Mar 1, 2022
85c48f4
fix linter issue
fzyzcjy Mar 1, 2022
a643e7e
regsiter TagNameRule
fzyzcjy Mar 1, 2022
21f7d98
add stripPrefix, stripPostfix
fzyzcjy Mar 1, 2022
314d810
explain limitation
fzyzcjy Mar 1, 2022
54fec9e
Merge branch 'master' into master
dkrutskikh Mar 6, 2022
2f12169
Merge branch 'master' of https://github.com/dart-code-checker/dart-co…
fzyzcjy May 26, 2022
eeca3ce
fix wrong merge
fzyzcjy May 26, 2022
b165d72
Update CHANGELOG.md
fzyzcjy May 26, 2022
66c12be
add doc
fzyzcjy May 26, 2022
cc95ad4
add empty AvoidBannedImportsRule
fzyzcjy May 26, 2022
b1a0614
add to factory
fzyzcjy May 26, 2022
bbe6a8e
parse config
fzyzcjy May 26, 2022
2f09f5f
add empty visitor
fzyzcjy May 26, 2022
8eb1426
add empty test
fzyzcjy May 26, 2022
57d079f
add test data
fzyzcjy May 26, 2022
1b6be7c
fix wrong name
fzyzcjy May 26, 2022
d0323d0
try to implement logic
fzyzcjy May 26, 2022
1c1fb12
implement regexp
fzyzcjy May 26, 2022
3a4fb0e
add tests
fzyzcjy May 26, 2022
cc2acfe
format code
fzyzcjy May 26, 2022
090b44b
Update visitor.dart
fzyzcjy May 28, 2022
b1fcae4
Merge remote-tracking branch 'upstream/master'
fzyzcjy Jun 8, 2022
ae3ac94
Merge remote-tracking branch 'origin/master'
fzyzcjy Jun 8, 2022
be0b5cf
remove strange char
fzyzcjy Jun 8, 2022
80908dd
add tests
fzyzcjy Jun 8, 2022
af80c4b
parse config
fzyzcjy Jun 8, 2022
f4e4625
pass down config
fzyzcjy Jun 8, 2022
0ef1776
rename
fzyzcjy Jun 8, 2022
5edbc42
update doc
fzyzcjy Jun 8, 2022
aa4d315
try to implement it
fzyzcjy Jun 8, 2022
17695b4
fix tests
fzyzcjy Jun 8, 2022
66acc4d
format code
fzyzcjy Jun 8, 2022
75a8fb4
Merge branch 'master' into master
incendial Jun 16, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,7 @@
* test: added test case in [`prefer-const-border-radius`](https://dartcodemetrics.dev/docs/rules/flutter/prefer-const-border-radius) rule.
* chore: restrict `analyzer` version to `>=2.4.0 <4.2.0`.
* fix: improve context root included files calculation.
* feat: add [`avoid-banned-imports`](https://dartcodemetrics.dev/docs/rules/common/avoid-banned-imports) rule
incendial marked this conversation as resolved.
Show resolved Hide resolved

## 4.15.2

Expand Down
2 changes: 2 additions & 0 deletions 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';
Expand Down Expand Up @@ -54,6 +55,7 @@ import 'rules_list/tag_name/tag_name_rule.dart';

final _implementedRules = <String, Rule Function(Map<String, Object>)>{
AlwaysRemoveListenerRule.ruleId: (config) => AlwaysRemoveListenerRule(config),
AvoidBannedImportsRule.ruleId: (config) => AvoidBannedImportsRule(config),
AvoidBorderAllRule.ruleId: (config) => AvoidBorderAllRule(config),
AvoidCollectionMethodsWithUnrelatedTypesRule.ruleId: (config) =>
AvoidCollectionMethodsWithUnrelatedTypesRule(config),
Expand Down
@@ -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<String, Object> config = const {}])
: _entries = _ConfigParser._parseEntryConfig(config),
super(
id: ruleId,
severity: readSeverity(config, Severity.style),
excludes: readExcludes(config),
);

@override
Iterable<Issue> 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);
}
}
@@ -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<String, Object> config,
) =>
(config[_entriesLabel] as Iterable<Object?>? ?? []).map((entry) {
final entryMap = entry as Map<Object?, Object?>;

return _AvoidBannedImportsConfigEntry(
paths: _parseListRegExp(entryMap[_pathsLabel]),
deny: _parseListRegExp(entryMap[_denyLabel]),
message: entryMap[_messageLabel] as String,
);
}).toList();

static List<RegExp> _parseListRegExp(Object? object) =>
(object! as List<Object?>)
incendial marked this conversation as resolved.
Show resolved Hide resolved
.map((e) => e! as String)
.map((e) => RegExp(e))
.toList();
}

class _AvoidBannedImportsConfigEntry {
final List<RegExp> paths;
final List<RegExp> deny;
final String message;

_AvoidBannedImportsConfigEntry({
required this.paths,
required this.deny,
required this.message,
});
}
@@ -0,0 +1,35 @@
part of 'avoid_banned_imports_rule.dart';

class _Visitor extends RecursiveAstVisitor<void> {
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);
}
Expand Up @@ -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<String> parseIgnoredArguments(Map<String, Object> config) =>
config.containsKey(_ignoredArgumentsConfig) &&
config[_ignoredArgumentsConfig] is Iterable
? List<String>.from(config[_ignoredArgumentsConfig] as Iterable)
: <String>[];

static int? parseAllowedLineCount(Map<String, Object> config) {
final raw = config[_allowedLineCountConfig];
return raw is int? ? raw : null;
}
}
Expand Up @@ -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';
incendial marked this conversation as resolved.
Show resolved Hide resolved
import '../../../models/internal_resolved_unit_result.dart';
import '../../../models/issue.dart';
import '../../../models/severity.dart';
Expand All @@ -22,9 +23,11 @@ class PreferExtractingCallbacksRule extends FlutterRule {
'Prefer extracting the callback to a separate widget method.';

final Iterable<String> _ignoredArguments;
final int? _allowedLineCount;

PreferExtractingCallbacksRule([Map<String, Object> config = const {}])
: _ignoredArguments = _ConfigParser.parseIgnoredArguments(config),
_allowedLineCount = _ConfigParser.parseAllowedLineCount(config),
super(
id: ruleId,
severity: readSeverity(config, Severity.style),
Expand All @@ -33,7 +36,7 @@ class PreferExtractingCallbacksRule extends FlutterRule {

@override
Iterable<Issue> check(InternalResolvedUnitResult source) {
final visitor = _Visitor(_ignoredArguments);
final visitor = _Visitor(source, _ignoredArguments, _allowedLineCount);

source.unit.visitChildren(visitor);

Expand Down
Expand Up @@ -3,11 +3,13 @@ part of 'prefer_extracting_callbacks_rule.dart';
class _Visitor extends SimpleAstVisitor<void> {
final _expressions = <Expression>[];

final InternalResolvedUnitResult _source;
final Iterable<String> _ignoredArguments;
final int? _allowedLineCount;

Iterable<Expression> get expressions => _expressions;

_Visitor(this._ignoredArguments);
_Visitor(this._source, this._ignoredArguments, this._allowedLineCount);

@override
void visitClassDeclaration(ClassDeclaration node) {
Expand All @@ -17,7 +19,8 @@ class _Visitor extends SimpleAstVisitor<void> {
return;
}

final visitor = _InstanceCreationVisitor(_ignoredArguments);
final visitor =
_InstanceCreationVisitor(_source, _ignoredArguments, _allowedLineCount);
node.visitChildren(visitor);

_expressions.addAll(visitor.expressions);
Expand All @@ -27,11 +30,14 @@ class _Visitor extends SimpleAstVisitor<void> {
class _InstanceCreationVisitor extends RecursiveAstVisitor<void> {
final _expressions = <Expression>[];

final InternalResolvedUnitResult _source;
final Iterable<String> _ignoredArguments;
final int? _allowedLineCount;

Iterable<Expression> get expressions => _expressions;

_InstanceCreationVisitor(this._ignoredArguments);
_InstanceCreationVisitor(
this._source, this._ignoredArguments, this._allowedLineCount);

@override
void visitInstanceCreationExpression(InstanceCreationExpression node) {
Expand All @@ -44,7 +50,8 @@ class _InstanceCreationVisitor extends RecursiveAstVisitor<void> {
if (_isNotIgnored(argument) &&
expression is FunctionExpression &&
_hasNotEmptyBlockBody(expression) &&
!_isFlutterBuilder(expression)) {
!_isFlutterBuilder(expression) &&
_isLongEnough(expression)) {
_expressions.add(argument);
}
}
Expand Down Expand Up @@ -74,4 +81,16 @@ class _InstanceCreationVisitor extends RecursiveAstVisitor<void> {
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);
incendial marked this conversation as resolved.
Show resolved Hide resolved
expression.visitChildren(visitor);

return visitor.linesWithCode.length > allowedLineCount;
}
}
@@ -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': <Object>[
<String, Object>{
'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)',
],
);
});
});
}
@@ -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';
@@ -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,
});
}