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

feat: add new rule correct-game-instantiating #1163

Merged
merged 3 commits into from
Jan 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Unreleased

* docs: remove old website
* feat: add static code diagnostic [`correct-game-instantiating`](https://dcm.dev/docs/individuals/rules/flame/correct-game-instantiating).

## 5.5.1

Expand Down
16 changes: 16 additions & 0 deletions lib/src/analyzers/lint_analyzer/rules/models/flame_rule.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import 'rule.dart';
import 'rule_type.dart';

/// Represents a base class for Flutter-specific rules.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@incendial maybe for Flutter Flame Engine ?

abstract class FlameRule extends Rule {
static const link = 'https://pub.dev/packages/flame';

const FlameRule({
required super.id,
required super.severity,
required super.excludes,
required super.includes,
}) : super(
type: RuleType.flame,
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ class RuleType {
static const flutter = RuleType._('flutter');
static const intl = RuleType._('intl');
static const angular = RuleType._('angular');
static const flame = RuleType._('flame');
}
2 changes: 2 additions & 0 deletions lib/src/analyzers/lint_analyzer/rules/rules_factory.dart
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import 'rules_list/binary_expression_operand_order/binary_expression_operand_ord
import 'rules_list/check_for_equals_in_render_object_setters/check_for_equals_in_render_object_setters_rule.dart';
import 'rules_list/component_annotation_arguments_ordering/component_annotation_arguments_ordering_rule.dart';
import 'rules_list/consistent_update_render_object/consistent_update_render_object_rule.dart';
import 'rules_list/correct_game_instantiating/correct_game_instantiating_rule.dart';
import 'rules_list/double_literal_format/double_literal_format_rule.dart';
import 'rules_list/format_comment/format_comment_rule.dart';
import 'rules_list/list_all_equatable_fields/list_all_equatable_fields_rule.dart';
Expand Down Expand Up @@ -119,6 +120,7 @@ final _implementedRules = <String, Rule Function(Map<String, Object>)>{
ComponentAnnotationArgumentsOrderingRule.ruleId:
ComponentAnnotationArgumentsOrderingRule.new,
ConsistentUpdateRenderObjectRule.ruleId: ConsistentUpdateRenderObjectRule.new,
CorrectGameInstantiatingRule.ruleId: CorrectGameInstantiatingRule.new,
DoubleLiteralFormatRule.ruleId: DoubleLiteralFormatRule.new,
FormatCommentRule.ruleId: FormatCommentRule.new,
ListAllEquatableFieldsRule.ruleId: ListAllEquatableFieldsRule.new,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
// ignore_for_file: public_member_api_docs

import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:collection/collection.dart';

import '../../../../../utils/flutter_types_utils.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/flame_rule.dart';
import '../../rule_utils.dart';

part 'visitor.dart';

class CorrectGameInstantiatingRule extends FlameRule {
static const String ruleId = 'correct-game-instantiating';

static const _warningMessage =
'Incorrect game instantiation. The game will reset on each rebuild.';
static const _correctionMessage = "Replace with 'controlled'.";

CorrectGameInstantiatingRule([Map<String, Object> config = const {}])
: super(
id: ruleId,
severity: readSeverity(config, Severity.warning),
excludes: readExcludes(config),
includes: readIncludes(config),
);

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

source.unit.visitChildren(visitor);

return visitor.info
.map((info) => createIssue(
rule: this,
location: nodeLocation(node: info.node, source: source),
message: _warningMessage,
replacement: _createReplacement(info),
))
.toList(growable: false);
}

Replacement? _createReplacement(_InstantiationInfo info) {
if (info.isStateless) {
final arguments = info.node.argumentList.arguments.map((arg) {
if (arg is NamedExpression && arg.name.label.name == 'game') {
final expression = arg.expression;
if (expression is InstanceCreationExpression) {
final name =
expression.staticType?.getDisplayString(withNullability: false);
if (name != null) {
return 'gameFactory: $name.new,';
}
}
}

return arg.toSource();
});

return Replacement(
replacement: 'GameWidget.controlled$arguments;',
comment: _correctionMessage,
);
}

return null;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
part of 'correct_game_instantiating_rule.dart';

class _Visitor extends RecursiveAstVisitor<void> {
final _info = <_InstantiationInfo>[];

Iterable<_InstantiationInfo> get info => _info;

@override
void visitClassDeclaration(ClassDeclaration node) {
super.visitClassDeclaration(node);

final type = node.extendsClause?.superclass.type;
if (type == null) {
return;
}

final isWidget = isWidgetOrSubclass(type);
final isWidgetState = isWidgetStateOrSubclass(type);
if (!isWidget && !isWidgetState) {
return;
}

final declaration = node.members.firstWhereOrNull((declaration) =>
declaration is MethodDeclaration && declaration.name.lexeme == 'build');

if (declaration is MethodDeclaration) {
final visitor = _GameWidgetInstantiatingVisitor();
declaration.visitChildren(visitor);

if (visitor.wrongInstantiation != null) {
_info.add(_InstantiationInfo(
visitor.wrongInstantiation!,
isStateless: !isWidgetState,
));
}
}
}
}

class _GameWidgetInstantiatingVisitor extends RecursiveAstVisitor<void> {
InstanceCreationExpression? wrongInstantiation;

_GameWidgetInstantiatingVisitor();

@override
void visitInstanceCreationExpression(InstanceCreationExpression node) {
super.visitInstanceCreationExpression(node);

if (isGameWidget(node.staticType) &&
node.constructorName.name?.name != 'controlled') {
final gameArgument = node.argumentList.arguments.firstWhereOrNull(
(argument) =>
argument is NamedExpression && argument.name.label.name == 'game',
);

if (gameArgument is NamedExpression &&
gameArgument.expression is InstanceCreationExpression) {
wrongInstantiation = node;
}
}
}
}

class _InstantiationInfo {
final bool isStateless;
final InstanceCreationExpression node;

const _InstantiationInfo(this.node, {required this.isStateless});
}
3 changes: 3 additions & 0 deletions lib/src/utils/flutter_types_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ bool isPaddingWidget(DartType? type) =>
bool isBuildContext(DartType? type) =>
type?.getDisplayString(withNullability: false) == 'BuildContext';

bool isGameWidget(DartType? type) =>
type?.getDisplayString(withNullability: false) == 'GameWidget';

bool _isWidget(DartType? type) =>
type?.getDisplayString(withNullability: false) == 'Widget';

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import 'package:dart_code_metrics/src/analyzers/lint_analyzer/models/severity.dart';
import 'package:dart_code_metrics/src/analyzers/lint_analyzer/rules/rules_list/correct_game_instantiating/correct_game_instantiating_rule.dart';
import 'package:test/test.dart';

import '../../../../../helpers/rule_test_helper.dart';

const _examplePath = 'correct_game_instantiating/examples/example.dart';

void main() {
group('CorrectGameInstantiatingRule', () {
test('initialization', () async {
final unit = await RuleTestHelper.resolveFromFile(_examplePath);
final issues = CorrectGameInstantiatingRule().check(unit);

RuleTestHelper.verifyInitialization(
issues: issues,
ruleId: 'correct-game-instantiating',
severity: Severity.warning,
);
});

test('reports about found issues', () async {
final unit = await RuleTestHelper.resolveFromFile(_examplePath);
final issues = CorrectGameInstantiatingRule().check(unit);

RuleTestHelper.verifyIssues(
issues: issues,
startLines: [4, 25],
startColumns: [12, 12],
locationTexts: [
'GameWidget(game: MyFlameGame())',
'GameWidget(game: MyFlameGame())',
],
messages: [
'Incorrect game instantiation. The game will reset on each rebuild.',
'Incorrect game instantiation. The game will reset on each rebuild.',
],
replacements: [
'GameWidget.controlled(gameFactory: MyFlameGame.new,);',
null,
],
replacementComments: [
"Replace with 'controlled'.",
null,
],
);
});
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
class MyGamePage extends StatelessWidget {
@override
Widget build(BuildContext context) {
return GameWidget(game: MyFlameGame()); // LINT
}
}

class MyGamePage extends StatefulWidget {
@override
State<MyGamePage> createState() => _MyGamePageState();
}

class _MyGamePageState extends State<MyGamePage> {
late final _game = MyFlameGame();

@override
Widget build(BuildContext context) {
return GameWidget(game: _game);
}
}

class _MyGamePageState extends State<MyGamePage> {
@override
Widget build(BuildContext context) {
return GameWidget(game: MyFlameGame()); // LINT
}
}

class MyGamePage extends StatelessWidget {
@override
Widget build(BuildContext context) {
return GameWidget.controlled(gameFactory: MyFlameGame.new);
}
}

class GameWidget extends Widget {
final Widget game;

GameWidget({required this.game});

GameWidget.controlled({required GameFactory<Widget> gameFactory}) {
this.game = gameFactory();
}
}

class MyFlameGame extends Widget {}

typedef GameFactory<T> = T Function();

class StatefulWidget extends Widget {}

class StatelessWidget extends Widget {}

class Widget {
const Widget();
}

class BuildContext {}

abstract class State<T> {
void initState();

void setState(VoidCallback callback) => callback();
}
8 changes: 4 additions & 4 deletions test/src/helpers/rule_test_helper.dart
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ class RuleTestHelper {
Iterable<int>? startColumns,
Iterable<String>? locationTexts,
Iterable<String>? messages,
Iterable<String>? replacements,
Iterable<String>? replacementComments,
Iterable<String?>? replacements,
Iterable<String?>? replacementComments,
}) {
if (startLines != null) {
expect(
Expand Down Expand Up @@ -83,15 +83,15 @@ class RuleTestHelper {

if (replacements != null) {
expect(
issues.map((issue) => issue.suggestion!.replacement),
issues.map((issue) => issue.suggestion?.replacement),
equals(replacements),
reason: 'incorrect replacement',
);
}

if (replacementComments != null) {
expect(
issues.map((issue) => issue.suggestion!.comment),
issues.map((issue) => issue.suggestion?.comment),
equals(replacementComments),
reason: 'incorrect replacement comment',
);
Expand Down