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

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

 // Copyright (c) 2014, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library engine.resolver;
 
 import 'dart:collection';
 
+import 'package:analyzer/src/generated/scanner.dart';
+
 import 'ast.dart';
 import 'constant.dart';
 import 'element.dart';
 import 'element_resolver.dart';
 import 'engine.dart';
 import 'error.dart';
 import 'error_verifier.dart';
 import 'html.dart' as ht;
 import 'java_core.dart';
 import 'java_engine.dart';
@@ skipping 110 matching lines @@
       _enclosingClass = node.element;
       // Commented out until we decide that we want this hint in the analyzer
       //    checkForOverrideEqualsButNotHashCode(node);
       return super.visitClassDeclaration(node);
     } finally {
       _enclosingClass = outerClass;
     }
   }
 
   @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;
     if (importElement != null) {
       if (importElement.isDeferred) {
         _checkForLoadLibraryFunction(node, importElement);
       }
     }
     return super.visitImportDirective(node);
   }
@@ skipping 55 matching lines @@
     return super.visitSuperConstructorInvocation(node);
   }
 
   @override
   Object visitVariableDeclaration(VariableDeclaration node) {
     _checkForUseOfVoidResult(node.initializer);
     _checkForInvalidAssignment(node.name, node.initializer);
     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.
    *
    * @param node the is expression to check
    * @return `true` if and only if a hint code is generated on the passed node
    * See [HintCode.TYPE_CHECK_IS_NOT_NULL], [HintCode.TYPE_CHECK_IS_NULL],
    * [HintCode.UNNECESSARY_TYPE_CHECK_TRUE], and
    * [HintCode.UNNECESSARY_TYPE_CHECK_FALSE].
    */
@@ skipping 376 matching lines @@
     BlockFunctionBody blockFunctionBody = body as BlockFunctionBody;
     if (!ExitDetector.exits(blockFunctionBody)) {
       _errorReporter.reportErrorForNode(
           HintCode.MISSING_RETURN, returnType, [returnTypeType.displayName]);
       return true;
     }
     return false;
   }
 
   /**
+   * 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.
    *
    * @param node the class declaration to check
    * @return `true` if and only if a hint code is generated on the passed node
    * See [HintCode.OVERRIDE_EQUALS_BUT_NOT_HASH_CODE].
    */
 //  bool _checkForOverrideEqualsButNotHashCode(ClassDeclaration node) {
 //    ClassElement classElement = node.element;
 //    if (classElement == null) {
@@ skipping 4185 matching lines @@
    */
   InheritanceManager _manager;
 
   GatherUsedLocalElementsVisitor _usedLocalElementsVisitor;
 
   HintGenerator(this._compilationUnits, this._context, this._errorListener) {
     _library = _compilationUnits[0].element.library;
     _usedImportedElementsVisitor =
         new GatherUsedImportedElementsVisitor(_library);
     _enableDart2JSHints = _context.analysisOptions.dart2jsHint;
-    _manager = new InheritanceManager(_compilationUnits[0].element.library);
+    _manager = new InheritanceManager(_library);
     _usedLocalElementsVisitor = new GatherUsedLocalElementsVisitor(_library);
   }
 
   void generateForLibrary() {
     PerformanceStatistics.hints.makeCurrentWhile(() {
       for (CompilationUnit unit in _compilationUnits) {
         CompilationUnitElement element = unit.element;
         if (element != null) {
           _generateForCompilationUnit(unit, element.source);
         }
@@ skipping 11127 matching lines @@
     nonFields.add(node);
     return null;
   }
 
   @override
   Object visitNode(AstNode node) => node.accept(TypeResolverVisitor_this);
 
   @override
   Object visitWithClause(WithClause node) => null;
 }