Merge pull request dart-archive#412 from alexeieleusis/337_postfixExpression

invariant_boolean: When searching for references reassignment include PostfixExpression and similar, e.g. variable++.

Author: alexeieleusis

lib/src/rules/invariant_booleans.dart

@@ -216,15 +216,29 @@ AstNodePredicate _isConditionalStatementWithReturn(
 AstNodePredicate _noFurtherAssignmentInvalidatingCondition(
     Expression node, Iterable<AstNode> nodesInDFS) {
   Set<Identifier> identifiers = _findStatementIdentifiers(node.parent);
-  return (AstNode statement) =>
-      nodesInDFS
-          .skipWhile((n) => n != statement)
-          .takeWhile((n) => n != node)
-          .where((n) =>
-              n is AssignmentExpression &&
-              !identifiers.contains(n.leftHandSide))
-          .length ==
-      0;
+  return (AstNode statement) {
+    bool isMutation(AstNode n) {
+      if (n is AssignmentExpression) {
+        return !identifiers.contains(n.leftHandSide);
+      } else if (n is PostfixExpression) {
+        TokenType type = n.operator.type;
+        return (type == TokenType.PLUS_PLUS || type == TokenType.MINUS_MINUS) &&
+            !identifiers.contains(n.operand);
+      } else if (n is PrefixExpression) {
+        TokenType type = n.operator.type;
+        return (type == TokenType.PLUS_PLUS || type == TokenType.MINUS_MINUS) &&
+            !identifiers.contains(n.operand);
+      }
+
+      return false;
+    }
+
+    return nodesInDFS
+            .skipWhile((n) => n != statement)
+            .takeWhile((n) => n != node)
+            .where(isMutation)
+            .isEmpty;
+  };
 }
 
 List<Expression> _splitConjunctions(Expression expression) {

lib/src/rules/unnecessary_lambdas.dart

@@ -5,6 +5,7 @@
 library linter.src.rules.unnecessary_lambdas;
 
 import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/ast/token.dart';
 import 'package:analyzer/dart/ast/visitor.dart';
 import 'package:analyzer/dart/element/element.dart';
 import 'package:linter/src/analyzer.dart';
@@ -36,6 +37,13 @@ Iterable<Element> _extractElementsOfSimpleIdentifiers(AstNode node) =>
         .where((e) => e is SimpleIdentifier)
         .map((e) => (e as SimpleIdentifier).staticElement);
 
+bool _hasQuestionPeriod(AstNode node) {
+  return (node is PropertyAccess &&
+          node.operator?.type == TokenType.QUESTION_PERIOD) ||
+      (node is MethodInvocation &&
+          node.operator?.type == TokenType.QUESTION_PERIOD);
+}
+
 class UnnecessaryLambdas extends LintRule {
   _Visitor _visitor;
 
@@ -56,6 +64,11 @@ class _Visitor extends SimpleAstVisitor {
   final LintRule rule;
   _Visitor(this.rule);
 
+  @override
+  visitCompilationUnit(CompilationUnit node) {
+    var a = DartTypeUtilities.traverseNodesInDFS(node);
+  }
+
   @override
   void visitFunctionExpression(FunctionExpression node) {
     if (node.element.name != '') {
@@ -80,16 +93,17 @@ class _Visitor extends SimpleAstVisitor {
 
   void _visitInvocationExpression(
       InvocationExpression node, FunctionExpression nodeToLint) {
-    if (nodeToLint.parameters.length != node.argumentList.length) {
+    if (nodeToLint.parameters.length !=
+        node.argumentList.length) {
       return;
     }
     if (node.argumentList.arguments.any((e) => e is! SimpleIdentifier)) {
       return;
     }
     final parameters =
-        nodeToLint.parameters.parameters.map((e) => e.identifier.staticElement);
+        nodeToLint.parameters.parameters.map((e) => e.identifier.bestElement);
     final arguments = node.argumentList.arguments
-        .map((e) => (e as SimpleIdentifier).staticElement);
+        .map((e) => (e as SimpleIdentifier).bestElement);
     for (int i = 0; i < parameters.length; i++) {
       if (parameters.elementAt(i) != arguments.elementAt(i)) {
         return;
@@ -99,6 +113,13 @@ class _Visitor extends SimpleAstVisitor {
     if (node is FunctionExpressionInvocation) {
       restOfElements = _extractElementsOfSimpleIdentifiers(node.function);
     } else if (node is MethodInvocation && node.target != null) {
+      if (_hasQuestionPeriod(node) ||
+          _hasQuestionPeriod(node.target) ||
+          DartTypeUtilities
+              .traverseNodesInDFS(node.target)
+              .any(_hasQuestionPeriod)) {
+        return;
+      }
       restOfElements = node.target is SimpleIdentifier
           ? [(node.target as SimpleIdentifier).staticElement]
           : _extractElementsOfSimpleIdentifiers(node.target);

test/rules/invariant_booleans.dart

@@ -237,3 +237,56 @@ void bug372(bool foo) {
     // doSomethingElse();
   }
 }
+
+void bug337_1(int offset, int length) {
+  if (offset >= length) {
+    return;
+  }
+
+  offset++;
+  if (offset >= length) { // OK
+  }
+}
+
+void bug337_2(int offset, int length) {
+  if (offset >= length) {
+    return;
+  }
+
+  offset--;
+  if (offset >= length) { // OK
+  }
+}
+
+void bug337_3(int offset, int length) {
+  if (offset >= length) {
+    return;
+  }
+
+  ++offset;
+  if (offset >= length) { // OK
+  }
+}
+
+void bug337_4(int offset, int length) {
+  if (offset >= length) {
+    return;
+  }
+
+  --offset;
+  if (offset >= length) { // OK
+  }
+}
+
+void test337_5() {
+  int b = 2;
+  if (b > 0) {
+    b--;
+    if (b == 0) {
+      return;
+    }
+    if (b > 0) {
+      return;
+    }
+  }
+}

test/rules/unnecessary_lambdas.dart

@@ -4,6 +4,14 @@
 
 // test w/ `pub run test -N unnecessary_lambdas`
 
+Function stateList<T>(String finder) {
+  return (String element) => _stateOf<T>(element, finder);
+}
+
+Iterable<T> _stateOf<T>(String element, String finder) {
+  return <T>[];
+}
+
 void main() {
   List<String> names = [];
 
@@ -38,6 +46,16 @@ void main() {
   var deeplyNestedVariable = (a, b) { // OK
     foo(foo(b)).foo(a, b);
   };
+
+
+  // ignore: unused_local_variable
+  var caseWithNullCheck1 = () => names?.first.toLowerCase(); // OK
+
+  // ignore: unused_local_variable
+  var caseWithNullCheck2 = () => names.first?.toLowerCase(); // OK
+
+  // ignore: unused_local_variable
+  var caseWithTwoNullChecks = () => names?.first?.toLowerCase(); // OK
 }
 
 foo(a) {