Fix parsing of non-primitive Expressions (#725)

* Fix parsing of non-primitive Expressions

* Add templateLiteral to test

* Update changelog

CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 * Scan for module imports in inline and external JavaScript, analyzing the entire import graph.
 * Changed the way HTML script tag containing document features are made available to the JavaScript document, by creating a ScriptTagBackReferenceImport instead of appending the HTML document features directly to JavaScript document.
 * [minor] Add an `astNode` property on `Slot`.
+* Improve handling of types of properties defined in a behavior or legacy polymer element.
 
 ## [2.3.0] - 2017-09-25
 

src/javascript/esutil.ts

@@ -21,7 +21,7 @@ import * as estree from 'estree';
 import {ScannedMethod} from '../index';
 import {ImmutableSet} from '../model/immutable';
 import {Privacy} from '../model/model';
-import {ScannedEvent, Severity, SourceRange, Warning, WarningCarryingException} from '../model/model';
+import {ScannedEvent, Severity, SourceRange, Warning} from '../model/model';
 import {ParsedDocument} from '../parser/document';
 import * as docs from '../polymer/docs';
 import {annotateEvent} from '../polymer/docs';
@@ -88,6 +88,13 @@ export function objectKeyToString(key: estree.Node): string|undefined {
 export const CLOSURE_CONSTRUCTOR_MAP = new Map(
     [['Boolean', 'boolean'], ['Number', 'number'], ['String', 'string']]);
 
+const VALID_EXPRESSION_TYPES = new Map([
+  ['FunctionExpression', 'Function'],
+  ['ObjectExpression', 'Object'],
+  ['ArrayExpression', 'Array'],
+  ['TemplateLiteral', 'string']
+]);
+
 /**
  * AST expression -> Closure type.
  *
@@ -98,22 +105,23 @@ export const CLOSURE_CONSTRUCTOR_MAP = new Map(
  */
 export function closureType(
     node: estree.Node, sourceRange: SourceRange, document: ParsedDocument):
-    string {
-  if (node.type.match(/Expression$/)) {
-    return node.type.substr(0, node.type.length - 10);
+    string|Warning {
+  const type = VALID_EXPRESSION_TYPES.get(node.type);
+  if (type) {
+    return type;
   } else if (node.type === 'Literal') {
     return typeof node.value;
   } else if (node.type === 'Identifier') {
     return CLOSURE_CONSTRUCTOR_MAP.get(node.name) || node.name;
-  } else {
-    throw new WarningCarryingException(new Warning({
-      code: 'no-closure-type',
-      message: `Unable to determine closure type for expression of type ` +
-          `${node.type}`,
-      severity: Severity.WARNING, sourceRange,
-      parsedDocument: document,
-    }));
   }
+  return new Warning({
+    code: 'no-closure-type',
+    message: `Unable to determine closure type for expression of type ` +
+        `${node.type}`,
+    severity: Severity.WARNING,
+    sourceRange,
+    parsedDocument: document,
+  });
 }
 
 export function getAttachedComment(node: estree.Node): string|undefined {
@@ -150,10 +158,10 @@ function getLeadingComments(node: estree.Node): string[]|undefined {
   }
   const comments = [];
   for (const comment of node.leadingComments || []) {
-    // Espree says any comment that immediately precedes a node is "leading",
-    // but we want to be stricter and require them to be touching. If we don't
-    // have locations for some reason, err on the side of including the
-    // comment.
+    // Espree says any comment that immediately precedes a node is
+    // "leading", but we want to be stricter and require them to be
+    // touching. If we don't have locations for some reason, err on the
+    // side of including the comment.
     if (!node.loc || !comment.loc ||
         node.loc.start.line - comment.loc.end.line < 2) {
       comments.push(comment.value);
@@ -205,6 +213,10 @@ export function toScannedMethod(
   if (typeTag) {
     type = doctrine.type.stringify(typeTag.type!) || type;
   }
+  if (type instanceof Warning) {
+    warnings.push(type);
+    type = 'Function';
+  }
   const name = maybeName || '';
   const scannedMethod: ScannedMethod = {
     name,
@@ -284,8 +296,8 @@ export function getOrInferPrivacy(
 }
 
 /**
- * Properties on element prototypes that are part of the custom elment lifecycle
- * or Polymer configuration syntax.
+ * Properties on element prototypes that are part of the custom elment
+ * lifecycle or Polymer configuration syntax.
  *
  * TODO(rictic): only treat the Polymer ones as private when dealing with
  *   Polymer.

src/polymer/js-utils.ts

@@ -50,6 +50,10 @@ export function toScannedPolymerProperty(
   if (typeTag) {
     type = doctrine.type.stringify(typeTag.type!) || type;
   }
+  if (type instanceof Warning) {
+    warnings.push(type);
+    type = 'Object';
+  }
   const name = maybeName || '';
   const result: ScannedPolymerProperty = {
     name,

src/test/polymer/behavior-scanner_test.ts

@@ -30,7 +30,7 @@ suite('BehaviorScanner', () => {
   let behaviors: Map<string, ScannedBehavior>;
   let behaviorsList: ScannedBehavior[];
 
-  suiteSetup(async() => {
+  suiteSetup(async () => {
     const parser = new JavaScriptParser();
     const file = fs.readFileSync(
         path.resolve(__dirname, '../static/js-behaviors.js'), 'utf8');
@@ -74,8 +74,8 @@ suite('BehaviorScanner', () => {
   test('Supports behaviors On.Property.Paths', () => {
     assert(behaviors.has('Really.Really.Deep.Behavior'));
     assert.equal(
-        behaviors.get('Really.Really.Deep.Behavior')!.properties
-            .get('deep')!.name,
+        behaviors.get('Really.Really.Deep.Behavior')!.properties.get('deep')!
+            .name,
         'deep');
   });
 
@@ -109,6 +109,25 @@ suite('BehaviorScanner', () => {
       throw new Error('Could not find Polymer.SimpleNamespacedBehavior');
     }
     assert.deepEqual([...behavior.methods.keys()], ['method']);
-    assert.deepEqual([...behavior.properties.keys()], ['simple']);
+    assert.deepEqual(
+        [...behavior.properties.keys()],
+        ['simple', 'object', 'array', 'attached', 'templateLiteral']);
+  });
+
+  test('Correctly transforms property types', function() {
+    const behavior = behaviors.get('Polymer.SimpleNamespacedBehavior');
+    if (!behavior) {
+      throw new Error('Could not find Polymer.SimpleNamespacedBehavior');
+    }
+    assert.deepEqual(
+        [...behavior.properties.values()].map(
+            (p) => ({name: p.name, type: p.type})),
+        [
+          {name: 'simple', type: 'boolean'},
+          {name: 'object', type: 'Object'},
+          {name: 'array', type: 'Array'},
+          {name: 'attached', type: 'Object'},
+          {name: 'templateLiteral', type: 'string'}
+        ]);
   });
 });

src/test/static/js-behaviors.js

@@ -9,9 +9,13 @@ var SimpleBehavior = {
  * */
 var SimpleNamespacedBehavior = {
   simple: true,
-  method: function (paramA, paramB) {
+  method: function(paramA, paramB) {
 
   },
+  object: {},
+  array: [],
+  attached: true ? null : function() {},
+  templateLiteral: `foo`
 };