Analyzer support for `@factory` methods (linter#253).

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 analyzer.src.generated.resolver;
 
 import 'dart:collection';
 
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/token.dart';
@@ skipping 73 matching lines @@
    *
    * @param errorReporter the error reporter
    */
   BestPracticesVerifier(
       this._errorReporter, TypeProvider typeProvider, this._currentLibrary,
       {TypeSystem typeSystem})
       : _futureNullType = typeProvider.futureNullType,
         _typeSystem = typeSystem ?? new TypeSystemImpl();
 
   @override
+  Object visitAnnotation(Annotation node) {
+    if (node.elementAnnotation?.isFactory == true) {
+      AstNode parent = node.parent;
+      if (parent is MethodDeclaration) {
+        _checkForInvalidFactory(parent);
+      } else {
+        _errorReporter
+            .reportErrorForNode(HintCode.INVALID_FACTORY_ANNOTATION, node, []);
+      }
+    }
+    return super.visitAnnotation(node);
+  }
+
+  @override
   Object visitArgumentList(ArgumentList node) {
     for (Expression argument in node.arguments) {
       ParameterElement parameter = argument.bestParameterElement;
       if (parameter?.parameterKind == ParameterKind.POSITIONAL) {
         _checkForDeprecatedMemberUse(parameter, argument);
       }
     }
     _checkForArgumentTypesNotAssignableInList(node);
     return super.visitArgumentList(node);
   }
@@ skipping 563 matching lines @@
     if (leftType != null && bestRightType != null) {
       if (!_typeSystem.isAssignableTo(bestRightType, leftType)) {
         _errorReporter.reportTypeErrorForNode(
             HintCode.INVALID_ASSIGNMENT, rhs, [bestRightType, leftType]);
         return true;
       }
     }
     return false;
   }
 
+  void _checkForInvalidFactory(MethodDeclaration decl) {
+    // Check declaration.
+    TypeName returnType = decl.returnType;
+    if (returnType == null || returnType.type is VoidType) {

[Brian Wilkerson, 2016/06/24 21:04:00]

Checking for `void` makes sense; not sure about checking for `null`. Won't the
latter produce other lints (always use type annotations)?

[pquitslund, 2016/06/24 21:40:35]

Good point.   There could be a pile up, assuming that lint is enabled.  (Of
course, in this case, I think we can rest assured that it will be.)  I'll remove
the check for now.

+      _errorReporter.reportErrorForNode(HintCode.INVALID_FACTORY_METHOD_DECL,
+          decl.name, [decl.name.toString()]);
+      return;
+    }
+
+    // Check implementation.
+
+    FunctionBody body = decl.body;
+    if (body is EmptyFunctionBody) {
+      // Abstract methods are OK.
+      return;
+    }
+
+    // `new Foo()` or `null`.
+    bool factoryExpression(Expression expression) =>
+        expression is InstanceCreationExpression || expression is NullLiteral;
+
+    if (body is ExpressionFunctionBody && factoryExpression(body.expression)) {
+      return;
+    } else if (body is BlockFunctionBody) {
+      NodeList<Statement> statements = body.block.statements;
+      if (statements.isNotEmpty) {
+        Statement last = statements.last;
+        if (last is ReturnStatement && factoryExpression(last.expression)) {
+          return;
+        }
+      }
+    }
+
+    _errorReporter.reportErrorForNode(HintCode.INVALID_FACTORY_METHOD_IMPL,
+        decl.name, [decl.name.toString()]);
+  }
+
   /**
    * Produces a hint if the given identifier is a protected closure, field or
    * getter/setter, method closure or invocation accessed outside a subclass.
    */
   void _checkForInvalidProtectedMemberAccess(SimpleIdentifier identifier) {
     if (identifier.inDeclarationContext()) {
       return;
     }
 
     bool isProtected(Element element) {
@@ skipping 10279 matching lines @@
       return null;
     }
     if (identical(node.staticElement, variable)) {
       if (node.inSetterContext()) {
         result = true;
       }
     }
     return null;
   }
 }