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

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 cfb6b8990b4d311c1b85e87220b9e6e9462fa186..7bf3b35e176d510267a22b52012c18aaaebb7569 100644
--- a/pkg/analyzer/lib/src/generated/resolver.dart
+++ b/pkg/analyzer/lib/src/generated/resolver.dart
@@ -91,6 +91,20 @@ class BestPracticesVerifier extends RecursiveAstVisitor<Object> {
         _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;
@@ -674,6 +688,43 @@ class BestPracticesVerifier extends RecursiveAstVisitor<Object> {
     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.