From 5754b46f5e599a093ecf387d8cb6fde045a77bce Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Tue, 5 Sep 2017 17:05:52 -0700 Subject: [PATCH 1/3] Fix parsing of non-primitive Expressions --- src/javascript/esutil.ts | 48 ++++++++++++++--------- src/polymer/js-utils.ts | 4 ++ src/test/polymer/behavior-scanner_test.ts | 26 ++++++++++-- src/test/static/js-behaviors.js | 5 ++- 4 files changed, 60 insertions(+), 23 deletions(-) diff --git a/src/javascript/esutil.ts b/src/javascript/esutil.ts index b293fbd9..8a7d8ac1 100644 --- a/src/javascript/esutil.ts +++ b/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. diff --git a/src/polymer/js-utils.ts b/src/polymer/js-utils.ts index 4c06f4c5..2f32f836 100644 --- a/src/polymer/js-utils.ts +++ b/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, diff --git a/src/test/polymer/behavior-scanner_test.ts b/src/test/polymer/behavior-scanner_test.ts index 293c2dff..78640cc4 100644 --- a/src/test/polymer/behavior-scanner_test.ts +++ b/src/test/polymer/behavior-scanner_test.ts @@ -30,7 +30,7 @@ suite('BehaviorScanner', () => { let behaviors: Map; 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,24 @@ 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']); + }); + + 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'} + ]); }); }); diff --git a/src/test/static/js-behaviors.js b/src/test/static/js-behaviors.js index 85d5c409..c32f0725 100644 --- a/src/test/static/js-behaviors.js +++ b/src/test/static/js-behaviors.js @@ -9,9 +9,12 @@ var SimpleBehavior = { * */ var SimpleNamespacedBehavior = { simple: true, - method: function (paramA, paramB) { + method: function(paramA, paramB) { }, + object: {}, + array: [], + attached: true ? null : function() {} }; From 920548f338e7b5ebaf7297c8d10e62ebb4a5cd46 Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Tue, 5 Sep 2017 20:13:01 -0700 Subject: [PATCH 2/3] Add templateLiteral to test --- src/test/polymer/behavior-scanner_test.ts | 5 +++-- src/test/static/js-behaviors.js | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/test/polymer/behavior-scanner_test.ts b/src/test/polymer/behavior-scanner_test.ts index 78640cc4..2c71afeb 100644 --- a/src/test/polymer/behavior-scanner_test.ts +++ b/src/test/polymer/behavior-scanner_test.ts @@ -111,7 +111,7 @@ suite('BehaviorScanner', () => { assert.deepEqual([...behavior.methods.keys()], ['method']); assert.deepEqual( [...behavior.properties.keys()], - ['simple', 'object', 'array', 'attached']); + ['simple', 'object', 'array', 'attached', 'templateLiteral']); }); test('Correctly transforms property types', function() { @@ -126,7 +126,8 @@ suite('BehaviorScanner', () => { {name: 'simple', type: 'boolean'}, {name: 'object', type: 'Object'}, {name: 'array', type: 'Array'}, - {name: 'attached', type: 'Object'} + {name: 'attached', type: 'Object'}, + {name: 'templateLiteral', type: 'string'} ]); }); }); diff --git a/src/test/static/js-behaviors.js b/src/test/static/js-behaviors.js index c32f0725..e04f78c2 100644 --- a/src/test/static/js-behaviors.js +++ b/src/test/static/js-behaviors.js @@ -14,7 +14,8 @@ var SimpleNamespacedBehavior = { }, object: {}, array: [], - attached: true ? null : function() {} + attached: true ? null : function() {}, + templateLiteral: `foo` }; From fc77d69d184ce49f5fc483918fefdc95e060548d Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Tue, 5 Sep 2017 20:15:09 -0700 Subject: [PATCH 3/3] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14efd0fc..86078011 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). * Scan for CSS custom variable uses and assignments. * Fix value of reflectToAttribute for polymer properties * Remove scriptElement from PolymerElement +* Fix handling of types of properties defined in a behavior ## [2.2.2] - 2017-07-20