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

feat: add static code diagnostic avoid-initializing-in-on-mount #1165

Merged
merged 5 commits into from
Jan 30, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
## Unreleased

* fix: export missing parts of public API.
* feat: add static code diagnostic [`correct-game-instantiating`](https://dcm.dev/docs/individuals/rules/flame/correct-game-instantiating).
* feat: add static code diagnostic [`avoid-initializing-in-on-mount`](https://dcm.dev/docs/individuals/rules/flame/avoid-initializing-in-on-mount).

## 5.5.0

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.
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');
}
4 changes: 4 additions & 0 deletions lib/src/analyzers/lint_analyzer/rules/rules_factory.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import 'rules_list/avoid_dynamic/avoid_dynamic_rule.dart';
import 'rules_list/avoid_expanded_as_spacer/avoid_expanded_as_spacer_rule.dart';
import 'rules_list/avoid_global_state/avoid_global_state_rule.dart';
import 'rules_list/avoid_ignoring_return_values/avoid_ignoring_return_values_rule.dart';
import 'rules_list/avoid_initializing_in_on_mount/avoid_initializing_in_on_mount_rule.dart';
import 'rules_list/avoid_late_keyword/avoid_late_keyword_rule.dart';
import 'rules_list/avoid_missing_enum_constant_in_map/avoid_missing_enum_constant_in_map_rule.dart';
import 'rules_list/avoid_nested_conditional_expressions/avoid_nested_conditional_expressions_rule.dart';
Expand All @@ -35,6 +36,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 @@ -87,6 +89,7 @@ final _implementedRules = <String, Rule Function(Map<String, Object>)>{
AvoidDynamicRule.ruleId: AvoidDynamicRule.new,
AvoidGlobalStateRule.ruleId: AvoidGlobalStateRule.new,
AvoidIgnoringReturnValuesRule.ruleId: AvoidIgnoringReturnValuesRule.new,
AvoidInitializingInOnMountRule.ruleId: AvoidInitializingInOnMountRule.new,
AvoidLateKeywordRule.ruleId: AvoidLateKeywordRule.new,
AvoidMissingEnumConstantInMapRule.ruleId:
AvoidMissingEnumConstantInMapRule.new,
Expand Down Expand Up @@ -119,6 +122,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
Expand Up @@ -37,10 +37,7 @@ class AvoidGlobalStateRule extends CommonRule {
return visitor.declarations
.map((declaration) => createIssue(
rule: this,
location: nodeLocation(
node: declaration,
source: source,
),
location: nodeLocation(node: declaration, source: source),
message: _warning,
))
.toList(growable: false);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// ignore_for_file: public_member_api_docs

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

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

part 'visitor.dart';

class AvoidInitializingInOnMountRule extends FlameRule {
static const String ruleId = 'avoid-initializing-in-on-mount';

static const _warningMessage =
'Avoid initializing final late variables in onMount.';

AvoidInitializingInOnMountRule([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.expressions
.map((expression) => createIssue(
rule: this,
location: nodeLocation(node: expression, source: source),
message: _warningMessage,
))
.toList(growable: false);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
part of 'avoid_initializing_in_on_mount_rule.dart';

class _Visitor extends SimpleAstVisitor<void> {
final _expressions = <AssignmentExpression>[];

Iterable<AssignmentExpression> get expressions => _expressions;

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

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

final onMountMethod = node.members.firstWhereOrNull((member) =>
member is MethodDeclaration &&
member.name.lexeme == 'onMount' &&
isOverride(member.metadata));

if (onMountMethod is MethodDeclaration) {
final visitor = _AssignmentExpressionVisitor();
onMountMethod.visitChildren(visitor);

_expressions.addAll(visitor.wrongAssignments);
}
}
}

class _AssignmentExpressionVisitor extends RecursiveAstVisitor<void> {
final wrongAssignments = <AssignmentExpression>{};

@override
void visitAssignmentExpression(AssignmentExpression node) {
super.visitAssignmentExpression(node);

final element = node.writeElement;
if (element is PropertyAccessorElement &&
element.variable.isFinal &&
element.variable.isLate) {
wrongAssignments.add(node);
}
}
}
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});
}
10 changes: 10 additions & 0 deletions lib/src/utils/flame_type_utils.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import 'package:analyzer/dart/element/type.dart';

bool isComponentOrSubclass(DartType? type) =>
_isComponent(type) || _isSubclassOfComponent(type);

bool _isComponent(DartType? type) =>
type?.getDisplayString(withNullability: false) == 'Component';

bool _isSubclassOfComponent(DartType? type) =>
type is InterfaceType && type.allSupertypes.any(_isComponent);
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,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/avoid_initializing_in_on_mount/avoid_initializing_in_on_mount_rule.dart';
import 'package:test/test.dart';

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

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

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

RuleTestHelper.verifyInitialization(
issues: issues,
ruleId: 'avoid-initializing-in-on-mount',
severity: Severity.warning,
);
});

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

RuleTestHelper.verifyIssues(
issues: issues,
startLines: [8],
startColumns: [5],
locationTexts: ['x = 1'],
messages: [
'Avoid initializing final late variables in onMount.',
],
);
});
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
class MyComponent extends Component {
late final int x;

int y;

@override
void onMount() {
x = 1; // LINT
y = 2;
}
}

class Component {
void onMount() {}
}
Loading