Add hint for use of ?. in conditions (issue 24649)

pkg/analyzer/lib/src/generated/resolver.dart

Index: pkg/analyzer/lib/src/generated/resolver.dart
diff --git a/pkg/analyzer/lib/src/generated/resolver.dart b/pkg/analyzer/lib/src/generated/resolver.dart
index 40a216228e023948cb8ea441a93abc05123bd76a..a33605a947b4f38c19b7159b3cf4477ec8b6105d 100644
--- a/pkg/analyzer/lib/src/generated/resolver.dart
+++ b/pkg/analyzer/lib/src/generated/resolver.dart
@@ -6,6 +6,8 @@ library engine.resolver;
 
 import 'dart:collection';
 
+import 'package:analyzer/src/generated/scanner.dart';
+
 import 'ast.dart';
 import 'constant.dart';
 import 'element.dart';
@@ -136,18 +138,36 @@ class BestPracticesVerifier extends RecursiveAstVisitor<Object> {
   }
 
   @override
+  Object visitDoStatement(DoStatement node) {
+    _checkForPossibleNullCondition(node.condition);
+    return super.visitDoStatement(node);
+  }
+
+  @override
   Object visitExportDirective(ExportDirective node) {
     _checkForDeprecatedMemberUse(node.uriElement, node);
     return super.visitExportDirective(node);
   }
 
   @override
+  Object visitForStatement(ForStatement node) {
+    _checkForPossibleNullCondition(node.condition);
+    return super.visitForStatement(node);
+  }
+
+  @override
   Object visitFunctionDeclaration(FunctionDeclaration node) {
     _checkForMissingReturn(node.returnType, node.functionExpression.body);
     return super.visitFunctionDeclaration(node);
   }
 
   @override
+  Object visitIfStatement(IfStatement node) {
+    _checkForPossibleNullCondition(node.condition);
+    return super.visitIfStatement(node);
+  }
+
+  @override
   Object visitImportDirective(ImportDirective node) {
     _checkForDeprecatedMemberUse(node.uriElement, node);
     ImportElement importElement = node.element;
@@ -223,6 +243,12 @@ class BestPracticesVerifier extends RecursiveAstVisitor<Object> {
     return super.visitVariableDeclaration(node);
   }
 
+  @override
+  Object visitWhileStatement(WhileStatement node) {
+    _checkForPossibleNullCondition(node.condition);
+    return super.visitWhileStatement(node);
+  }
+
   /**
    * Check for the passed is expression for the unnecessary type check hint codes as well as null
    * checks expressed using an is expression.
@@ -619,6 +645,28 @@ class BestPracticesVerifier extends RecursiveAstVisitor<Object> {
   }
 
   /**
+   * Produce a hint if the given [condition] could have a value of `null`.
+   */
+  void _checkForPossibleNullCondition(Expression condition) {
+    while (condition is ParenthesizedExpression) {
+      condition = (condition as ParenthesizedExpression).expression;
+    }
+    if (condition is MethodInvocation) {
+      Token operator = condition.operator;
+      if (operator != null && operator.type == TokenType.QUESTION_PERIOD) {

[scheglov, 2015/10/25 17:48:52]

if (operator?.type == TokenType.QUESTION_PERIOD) maybe? :-)

[Brian Wilkerson, 2015/10/25 22:11:40]

I don't happen to find it to be very readable, but...

+        _errorReporter.reportErrorForNode(
+            HintCode.NULL_AWARE_IN_CONDITION, condition);
+      }
+    } else if (condition is PropertyAccess) {
+      Token operator = condition.operator;
+      if (operator != null && operator.type == TokenType.QUESTION_PERIOD) {
+        _errorReporter.reportErrorForNode(
+            HintCode.NULL_AWARE_IN_CONDITION, condition);
+      }
+    }

[scheglov, 2015/10/25 17:48:52]

We could also validate every part of &&, ||, etc expression.

Do we need to validate AssertStatement and ConditionalStatement?

[Brian Wilkerson, 2015/10/25 22:11:40]

Done
Probably a good idea. Done

+  }
+
+  /**
    * Check for the passed class declaration for the
    * [HintCode.OVERRIDE_EQUALS_BUT_NOT_HASH_CODE] hint code.
    *
@@ -4824,7 +4872,7 @@ class HintGenerator {
     _usedImportedElementsVisitor =
         new GatherUsedImportedElementsVisitor(_library);
     _enableDart2JSHints = _context.analysisOptions.dart2jsHint;
-    _manager = new InheritanceManager(_compilationUnits[0].element.library);
+    _manager = new InheritanceManager(_library);
     _usedLocalElementsVisitor = new GatherUsedLocalElementsVisitor(_library);
   }