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

fix: fix moving to variable false positives #1186

Merged
merged 6 commits into from
Feb 12, 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 @@
* feat: add `allow-nullable` config option for [`avoid-returning-widgets`](https://dcm.dev/docs/individuals/rules/common/avoid-returning-widgets).
* fix: support `assert(mounted)` for [`use-setstate-synchronously`](https://dcm.dev/docs/individuals/rules/flutter/use-setstate-synchronously).
* fix: correctly support dartdoc tags for [`format-comment`](https://dcm.dev/docs/individuals/rules/common/format-comment).
* fix: resolve several false-positives with while loops, setters and implicit type parameters for [`prefer-moving-to-variable`](https://dcm.dev/docs/individuals/rules/common/prefer-moving-to-variable).

## 5.6.0-dev.1

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,14 @@ class _BlockVisitor extends RecursiveAstVisitor<void> {
}

bool _isDuplicate(AstNode visitedInvocation, AstNode node) {
if (_haveDifferentParameterTypes(visitedInvocation, node) ||
_isSetter(node) ||
_isSetter(visitedInvocation) ||
_hasMutableArguments(visitedInvocation, node) ||
_bothInsideWhile(visitedInvocation, node)) {
return false;
}

final visitedSwitch =
visitedInvocation.thisOrAncestorOfType<SwitchStatement>();

Expand Down Expand Up @@ -153,4 +161,46 @@ class _BlockVisitor extends RecursiveAstVisitor<void> {
}
}
}

bool _hasMutableArguments(AstNode visitedInvocation, AstNode node) =>
visitedInvocation is MethodInvocation &&
node is MethodInvocation &&
visitedInvocation.argumentList.arguments.any((argument) {
final expression =
argument is NamedExpression ? argument.expression : argument;

if (expression is SimpleIdentifier) {
final element = expression.staticElement;

return element is VariableElement &&
!element.isConst &&
!element.isFinal;
}

return false;
});

bool _isSetter(AstNode node) {
if (node is! PropertyAccess) {
return false;
}

final parent = node.parent;

return parent is AssignmentExpression && parent.leftHandSide == node;
}

bool _bothInsideWhile(AstNode visitedInvocation, AstNode node) {
final visitedWhile =
visitedInvocation.thisOrAncestorOfType<WhileStatement>();
final parentWhile = node.thisOrAncestorOfType<WhileStatement>();

return visitedWhile != null && visitedWhile == parentWhile;
}

bool _haveDifferentParameterTypes(AstNode visitedInvocation, AstNode node) =>
visitedInvocation is Expression &&
node is Expression &&
visitedInvocation.staticParameterElement?.type !=
node.staticParameterElement?.type;
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,19 @@ void main() {
String someFunction() {
return 'qwe';
}

class SomeClass {
final SomeClass inner;

String _value = '123';

String get value => _value;
set(String value) {
_value = value;
}

void method() {
final first = inner.inner.value;
inner.inner.value = 'hello';
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
class MyApp extends StatefulWidget {
const MyApp();
}

class _MyAppState extends State<MyApp> {
late final A a;
late final B b;
late final C c;

@override
void initState() {
super.initState();

a = context.read();
b = context.read();
c = context.read();
}

@override
Widget build(BuildContext context) {
return Container();
}
}

class A {}

class B {}

class C {}

class Widget {}

class StatefulWidget extends Widget {}

class BuildContext {}

extension ReadContext on BuildContext {
T read<T>() {
return 'value' as T;
}
}

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

void setState(VoidCallback callback) => callback();

BuildContext get context => BuildContext();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
class SomeClass {
final _addedComments = <Token>{};

Token? _latestCommentToken(Token token) {
Token? latestCommentToken = token.precedingComments;
while (latestCommentToken?.next != null) {
latestCommentToken = latestCommentToken?.next;
}

return latestCommentToken;
}

void method(Token token) {
Token? commentToken = token.precedingComments;

if (commentToken != null && !_addedComments.contains(commentToken)) {
if (!fromEnd) {
sink.write('\n');
}
}

while (commentToken != null) {
if (_addedComments.contains(commentToken)) {
commentToken = commentToken.next;
continue;
}
_addedComments.add(commentToken);
}
}
}

class Token {
final Token precedingComments;

const Token(this.precedingComments);
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ const _assignmentExamplePath =
'prefer_moving_to_variable/examples/assignment_example.dart';
const _prefixExamplePath =
'prefer_moving_to_variable/examples/prefix_example.dart';
const _providerExamplePath =
'prefer_moving_to_variable/examples/provider_example.dart';
const _whileExamplePath =
'prefer_moving_to_variable/examples/while_example.dart';

void main() {
group('PreferMovingToVariableRule', () {
Expand Down Expand Up @@ -240,6 +244,20 @@ void main() {
RuleTestHelper.verifyNoIssues(issues);
});

test('reports no issues for provider read', () async {
final unit = await RuleTestHelper.resolveFromFile(_providerExamplePath);
final issues = PreferMovingToVariableRule().check(unit);

RuleTestHelper.verifyNoIssues(issues);
});

test('reports no issues for while', () async {
final unit = await RuleTestHelper.resolveFromFile(_whileExamplePath);
final issues = PreferMovingToVariableRule().check(unit);

RuleTestHelper.verifyNoIssues(issues);
});

test('reports issues with custom config', () async {
final unit = await RuleTestHelper.resolveFromFile(_examplePath);
final config = {
Expand Down