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

feat: add avoid-creating-vector-in-update #1166

Merged
merged 4 commits into from
Feb 1, 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 @@ -5,6 +5,7 @@
* docs: remove old website
* 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).
* feat: add static code diagnostic [`avoid-creating-vector-in-update`](https://dcm.dev/docs/individuals/rules/flame/avoid-creating-vector-in-update).

## 5.5.1

Expand Down
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 @@ -5,6 +5,7 @@ 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_cascade_after_if_null/avoid_cascade_after_if_null_rule.dart';
import 'rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart';
import 'rules_list/avoid_creating_vector_in_update/avoid_creating_vector_in_update_rule.dart';
import 'rules_list/avoid_double_slash_imports/avoid_double_slash_imports_rule.dart';
import 'rules_list/avoid_duplicate_exports/avoid_duplicate_exports_rule.dart';
import 'rules_list/avoid_dynamic/avoid_dynamic_rule.dart';
Expand Down Expand Up @@ -84,6 +85,7 @@ final _implementedRules = <String, Rule Function(Map<String, Object>)>{
AvoidCascadeAfterIfNullRule.ruleId: AvoidCascadeAfterIfNullRule.new,
AvoidCollectionMethodsWithUnrelatedTypesRule.ruleId:
AvoidCollectionMethodsWithUnrelatedTypesRule.new,
AvoidCreatingVectorInUpdateRule.ruleId: AvoidCreatingVectorInUpdateRule.new,
AvoidDoubleSlashImportsRule.ruleId: AvoidDoubleSlashImportsRule.new,
AvoidDuplicateExportsRule.ruleId: AvoidDuplicateExportsRule.new,
AvoidDynamicRule.ruleId: AvoidDynamicRule.new,
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
// ignore: implementation_imports
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 AvoidCreatingVectorInUpdateRule extends FlameRule {
static const String ruleId = 'avoid-creating-vector-in-update';

static const _warningMessage = "Avoid creating Vector2 in 'update' method.";
incendial marked this conversation as resolved.
Show resolved Hide resolved

AvoidCreatingVectorInUpdateRule([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,52 @@
part of 'avoid_creating_vector_in_update_rule.dart';

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

Iterable<Expression> get expressions => _expressions;

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

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

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

if (updateMethod is MethodDeclaration) {
final visitor = _VectorVisitor();
updateMethod.visitChildren(visitor);

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

class _VectorVisitor extends RecursiveAstVisitor<void> {
final wrongExpressions = <Expression>{};

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

if (isVector(node.staticType)) {
wrongExpressions.add(node);
}
}

@override
void visitBinaryExpression(BinaryExpression node) {
super.visitBinaryExpression(node);

if (isVector(node.leftOperand.staticType) &&
isVector(node.rightOperand.staticType)) {
wrongExpressions.add(node);
}
}
}
3 changes: 3 additions & 0 deletions lib/src/utils/flame_type_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ import 'package:analyzer/dart/element/type.dart';
bool isComponentOrSubclass(DartType? type) =>
_isComponent(type) || _isSubclassOfComponent(type);

bool isVector(DartType? type) =>
type?.getDisplayString(withNullability: false) == 'Vector2';

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

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
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_creating_vector_in_update/avoid_creating_vector_in_update_rule.dart';
import 'package:test/test.dart';

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

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

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

RuleTestHelper.verifyInitialization(
issues: issues,
ruleId: 'avoid-creating-vector-in-update',
severity: Severity.warning,
);
});

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

RuleTestHelper.verifyIssues(
issues: issues,
startLines: [4, 23, 24],
startColumns: [23, 23, 23],
locationTexts: [
'Vector2(10, 10)',
'vector1 + vector2',
'vector1 - vector2',
],
messages: [
"Avoid creating Vector2 in 'update' method.",
"Avoid creating Vector2 in 'update' method.",
"Avoid creating Vector2 in 'update' method.",
],
);
});
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
class MyComponent extends Component {
@override
void update(double dt) {
final newVector = Vector2(10, 10); // LINT
}
}

class MyComponent extends Component {
final _temporaryVector = Vector2.zero();

@override
void update(double dt) {
_temporaryVector.setValues(10, 10);
}
}

class MyComponent extends Component {
final vector1 = Vector2(10, 10);
final vector2 = Vector2(20, 20);

@override
void update(double dt) {
final addVector = vector1 + vector2; // LINT
final subVector = vector1 - vector2; // LINT
}
}

class Component {
void onMount() {}
}

class Vector2 {
final double x;
final double y;

const Vector2(this.x, this.y);

const Vector2.zero()
: x = 0,
y = 0;

void setValues(double x, double y) {
this.x = x;
this.y = y;
}

@override
Vector2 operator /(double scale) => this;

@override
Vector2 operator +(double scale) => this;

@override
Vector2 operator -(double scale) => this;

@override
Vector2 operator *(double scale) => this;
}