remove SemanticNode

we didn't really need it after Siggi's reporting changes and introducing Conversion
(and it's trivial to check the "dynamic" bit in codegen)

[email protected]

Review URL: https://chromereviews.googleplex.com/131277013

Author: John Messerly

lib/src/checker/checker.dart

@@ -19,8 +19,8 @@ CheckerResults checkProgram(Uri fileUri, TypeResolver resolver,
   // Invoke the checker on the entry point.
   TypeProvider provider = resolver.context.typeProvider;
   var rules = new RestrictedRules(provider, reporter);
-  final visitor = new ProgramChecker(
-      resolver, rules, fileUri, checkSdk, reporter);
+  final visitor =
+      new ProgramChecker(resolver, rules, fileUri, checkSdk, reporter);
   visitor.check();
   return new CheckerResults(visitor.libraries, rules, visitor.failure);
 }
@@ -209,14 +209,12 @@ class ProgramChecker extends RecursiveAstVisitor {
 
   void checkFunctionApplication(
       Expression node, Expression f, ArgumentList list) {
-    DartType type = _rules.getStaticType(f);
-    if (type.isDynamic || type.isDartCoreFunction || type is! FunctionType) {
+    if (_rules.isDynamicCall(f)) {
       // TODO(vsm): For a function object, we should still be able to derive a
       // function type from it.
       _recordDynamicInvoke(node);
     } else {
-      assert(type is FunctionType);
-      checkArgumentList(list, type);
+      checkArgumentList(list, _rules.getStaticType(f));
     }
   }
 
@@ -286,10 +284,7 @@ class ProgramChecker extends RecursiveAstVisitor {
   }
 
   visitPropertyAccess(PropertyAccess node) {
-    final target = node.realTarget;
-    DartType receiverType = _rules.getStaticType(target);
-    assert(receiverType != null);
-    if (receiverType.isDynamic || receiverType.isDartCoreFunction) {
+    if (_rules.isDynamicGet(node.realTarget)) {
       _recordDynamicInvoke(node);
     }
     node.visitChildren(this);
@@ -299,12 +294,8 @@ class ProgramChecker extends RecursiveAstVisitor {
     final target = node.prefix;
     // Check if the prefix is a library - PrefixElement denotes a library
     // access.
-    if (target.staticElement is! PrefixElement) {
-      DartType receiverType = _rules.getStaticType(target);
-      assert(receiverType != null);
-      if (receiverType.isDynamic || receiverType.isDartCoreFunction) {
-        _recordDynamicInvoke(node);
-      }
+    if (target.staticElement is! PrefixElement && _rules.isDynamicGet(target)) {
+      _recordDynamicInvoke(node);
     }
     node.visitChildren(this);
   }
@@ -354,22 +345,8 @@ class ProgramChecker extends RecursiveAstVisitor {
     return expr;
   }
 
-  SemanticNode _getSemanticNode(AstNode astNode) {
-    if (astNode == null) return null;
-
-    final semanticNode = _current.nodeInfo[astNode];
-    if (semanticNode == null) {
-      return _current.nodeInfo[astNode] = new SemanticNode(astNode);
-    }
-    return semanticNode;
-  }
-
   void _recordDynamicInvoke(AstNode node) {
-    final info = new DynamicInvoke(_rules, node);
-    final semanticNode = _getSemanticNode(node);
-    assert(semanticNode.dynamicInvoke == null);
-    semanticNode.dynamicInvoke = info;
-    _reporter.log(info);
+    _reporter.log(new DynamicInvoke(_rules, node));
   }
 
   void _recordMessage(StaticInfo info) {

lib/src/checker/rules.dart

@@ -29,6 +29,23 @@ abstract class TypeRules {
   DartType getStaticType(Expression expr) => expr.staticType;
 
   DartType elementType(Element e);
+
+  /// Returns `true` if the expression is a dynamic property access or prefixed
+  /// identifier.
+  bool isDynamicGet(Expression expr) {
+    var t = getStaticType(expr);
+    // TODO(jmesserly): we should not allow all property gets on `Function`
+    return t.isDynamic || t.isDartCoreFunction;
+  }
+
+  /// Returns `true` if the expression is a dynamic function call or method
+  /// invocation.
+  bool isDynamicCall(Expression call) {
+    var t = getStaticType(call);
+    // TODO(jmesserly): fix handling of types with `call` methods. These are not
+    // FunctionType, but they also aren't dynamic calls.
+    return t.isDynamic || t.isDartCoreFunction || t is! FunctionType;
+  }
 }
 
 class DartRules extends TypeRules {
@@ -63,9 +80,6 @@ class RestrictedRules extends TypeRules {
   DartType getStaticType(Expression expr) {
     var type = expr.staticType;
     if (type != null) return type;
-
-    var node = currentLibraryInfo.nodeInfo
-        .putIfAbsent(expr, () => new SemanticNode(expr));
     _reporter.log(new MissingTypeError(expr));
     return provider.dynamicType;
   }

lib/src/codegen/js_codegen.dart

@@ -425,7 +425,7 @@ var $name = (function (_super) {
 
   @override
   void visitMethodInvocation(MethodInvocation node) {
-    if (_isDynamic(node)) {
+    if (rules.isDynamicCall(node.methodName)) {
       _unimplemented(node);
       return;
     }
@@ -693,7 +693,7 @@ var $name = (function (_super) {
 
   /// Shared code for [PrefixedIdentifier] and [PropertyAccess].
   void _visitGet(Expression target, SimpleIdentifier name) {
-    if (_isDynamic(target.parent)) {
+    if (rules.isDynamicGet(target)) {
       out.write('dart_runtime.dload(');
       target.accept(this);
       out.write(', "');
@@ -813,13 +813,6 @@ var $name = (function (_super) {
     id.accept(this);
   }
 
-  // TODO(jmesserly): is there a cleaner way to track this dynamic bit?
-  // Ideally we could just look at the resolved information on the AstNode.
-  bool _isDynamic(AstNode node) {
-    final info = libraryInfo.nodeInfo[node];
-    return info != null && info.dynamicInvoke != null;
-  }
-
   /// Safely visit the given node, with an optional prefix or suffix.
   void _visitNode(AstNode node, {String prefix: '', String suffix: ''}) {
     if (node == null) return;

lib/src/info.dart

@@ -30,10 +30,6 @@ class LibraryInfo {
   /// Corresponding analyzer element.
   final LibraryElement library;
 
-  /// Information on each node that belongs to this library (includes nodes in
-  /// the declaring unit and in parts).
-  Map<AstNode, SemanticNode> nodeInfo = <AstNode, SemanticNode>{};
-
   LibraryInfo(this.library) {
     name = library.name;
     if (name != null && library.name != '') return;
@@ -45,18 +41,6 @@ class LibraryInfo {
   }
 }
 
-/// Semantic information about a node.
-// TODO(jmesserly): this structure is very incomplete.
-class SemanticNode {
-  /// The syntax tree node this info is attached to.
-  final AstNode node;
-
-  /// If this operation is dynamically dispatched, this will be set.
-  DynamicInvoke dynamicInvoke;
-
-  SemanticNode(this.node);
-}
-
 // The abstract type of coercions mapping one type to another.
 // This class also exposes static builder functions which
 // check for errors and reduce redundant coercions to the identity.