From f2422fb9e09b965042399d929143057d55edf0a0 Mon Sep 17 00:00:00 2001 From: Kristofer Baxter Date: Thu, 16 Apr 2020 18:12:46 -0700 Subject: [PATCH 1/2] Transformer for removing AMP devAsserts, working on existing known cases --- .../index.js | 165 ++++++++++++++++++ .../transform-assertions/invoked/input.js | 22 +++ .../transform-assertions/invoked/options.json | 3 + .../transform-assertions/invoked/output.js | 17 ++ .../transform-assertions/thenable/input.js | 33 ++++ .../thenable/options.json | 3 + .../transform-assertions/thenable/output.js | 32 ++++ .../test/index.js | 19 ++ 8 files changed, 294 insertions(+) create mode 100644 build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/index.js create mode 100644 build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/invoked/input.js create mode 100644 build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/invoked/options.json create mode 100644 build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/invoked/output.js create mode 100644 build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/thenable/input.js create mode 100644 build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/thenable/options.json create mode 100644 build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/thenable/output.js create mode 100644 build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/index.js diff --git a/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/index.js b/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/index.js new file mode 100644 index 000000000000..3232f144632c --- /dev/null +++ b/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/index.js @@ -0,0 +1,165 @@ +/** + * Copyright 2020 The AMP HTML Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS-IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Is a MemberExpression "thenable", `foo().then(() => {});` + * + * @param {babel.MemberExpression} memberExpression + * @return {boolean} + */ +function isThenable(memberExpression) { + return memberExpression.get('property').isIdentifier({name: 'then'}); +} + +/** + * Does a MemberExpression have a inner CallExpression? Return it. + * + * @param {babel.MemberExpression} memberExpression + * @return {undefined | babel.CallExpression} + */ +function getMatchingInnerCallExpression(memberExpression) { + const memberObject = memberExpression.get('object'); + + if ( + memberObject.isCallExpression() && + memberObject.get('callee').isIdentifier({name: 'devAssert'}) + ) { + return memberObject; + } + + return undefined; +} + +/** + * Extend babel's path.evaluate with extra confidence for very specific scenarios in this transform. + * + * @param {babel.CallExpression} callExpression + * @return {babel.Evaluation} + */ +function evaluateDevAssert(callExpression) { + const argument = callExpression.get('arguments.0'); + if (argument.isMemberExpression()) { + return {confident: true}; + } + if (argument.isStringLiteral()) { + return {confident: true, value: argument.node.value}; + } + + return argument.evaluate(); +} + +module.exports = function ({types: t}) { + /** + * Given a potentially literal value, attempt to create a babel AST Literal Node for it. + * + * @param {*} value to try and create a babel literal for. + * @return {undefined | babel.NumericLiteral | babel.StringLiteral | babel.BooleanLiteral} value + */ + function usableEvaluateValue(value) { + switch (typeof value) { + case 'number': + return t.numericLiteral(value); + case 'string': + return t.stringLiteral(value); + case 'boolean': + return t.booleanLiteral(value); + case 'object': + if (String(value) === 'null') { + return t.nullLiteral(); + } + return undefined; + default: + return undefined; + } + } + + /** + * Given a wrapping CallExpression and inner MemberExpression. + * Attempt to find and replace the contents of a CallExpression inside the MemberExpression. + * + * @param {babel.CallExpression} callExpression + * @param {babel.MemberExpression} memberExpression + * @return {boolean} + */ + function handleInnerMemberExpression(callExpression, memberExpression) { + const innerCallExpression = getMatchingInnerCallExpression( + memberExpression + ); + if (innerCallExpression) { + const evalutedDevAssert = evaluateDevAssert(innerCallExpression); + if (!evalutedDevAssert.confident) { + return; + } + + // MemberExpression has an inner CallExpression with a removable `devAssert`. + if (isThenable(memberExpression)) { + // MemberExpression is thenable, so removing the `devAssert` means thenabling the first argument of `devAssert`. + const newCallee = innerCallExpression.get('arguments.0'); + const newArguments = callExpression.get('arguments.0'); + const newCallExpression = t.callExpression( + t.cloneDeep(newCallee.node), + [t.cloneDeep(newArguments.node)] + ); + + callExpression.replaceWith(newCallExpression); + return true; + } + + return false; + } + + return true; + } + + return { + visitor: { + CallExpression(path) { + const callee = path.get('callee'); + + if (callee.isMemberExpression()) { + handleInnerMemberExpression(path, callee); + return; + } + + if (callee.isIdentifier({name: 'devAssert'})) { + const evaluatedDevAssert = evaluateDevAssert(path); + + if ( + evaluatedDevAssert.confident && + evaluatedDevAssert.value !== undefined + ) { + const replacement = usableEvaluateValue(evaluatedDevAssert.value); + if (replacement !== undefined) { + if (t.isExpressionStatement(path.parent)) { + // When the direct parent of a `devAssert` is a ExpressionStatement, it can safely be removed. + // This is only true when the replacement was statically analyseable. + path.remove(); + return; + } + + // Given the replacement can be statically analysed, replace the value with the static replacement. + path.replaceWith(replacement); + return; + } + } + + // In other conditions, simply remove the `devAssert()` wrapper and allow the first argument to exist. + path.replaceWith(path.get('arguments.0')); + } + }, + }, + }; +}; diff --git a/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/invoked/input.js b/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/invoked/input.js new file mode 100644 index 000000000000..dc417e789c13 --- /dev/null +++ b/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/invoked/input.js @@ -0,0 +1,22 @@ +/** + * Copyright 2019 The AMP HTML Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS-IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +devAssert(1 + 1); +devAssert(''); +devAssert(false); +devAssert(null); +devAssert(0); +devAssert(dev().assert(2 + 2)); +let result = devAssert(foo, 'hello', 'world'); diff --git a/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/invoked/options.json b/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/invoked/options.json new file mode 100644 index 000000000000..a47282a6be9a --- /dev/null +++ b/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/invoked/options.json @@ -0,0 +1,3 @@ +{ + "plugins": ["../../../.."] +} diff --git a/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/invoked/output.js b/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/invoked/output.js new file mode 100644 index 000000000000..8cad86cbb4cb --- /dev/null +++ b/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/invoked/output.js @@ -0,0 +1,17 @@ +/** + * Copyright 2019 The AMP HTML Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS-IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +dev().assert(2 + 2); +let result = foo; diff --git a/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/thenable/input.js b/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/thenable/input.js new file mode 100644 index 000000000000..3e2364a2b10e --- /dev/null +++ b/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/thenable/input.js @@ -0,0 +1,33 @@ +/** + * Copyright 2018 The AMP HTML Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS-IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +class ThenableSuccess { + refresh() { + return devAssert(this.promise_).then(() => { + console.log('success'); + }); + } +} + +class ThenableSuccessFailure { + refresh() { + return devAssert(this.promise_).then(() => { + console.log('success'); + }, () => { + console.log('failure'); + }); + } +} \ No newline at end of file diff --git a/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/thenable/options.json b/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/thenable/options.json new file mode 100644 index 000000000000..a47282a6be9a --- /dev/null +++ b/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/thenable/options.json @@ -0,0 +1,3 @@ +{ + "plugins": ["../../../.."] +} diff --git a/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/thenable/output.js b/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/thenable/output.js new file mode 100644 index 000000000000..5a81289fe103 --- /dev/null +++ b/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/thenable/output.js @@ -0,0 +1,32 @@ +/** + * Copyright 2018 The AMP HTML Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS-IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +class ThenableSuccess { + refresh() { + return this.promise_(() => { + console.log('success'); + }); + } + +} + +class ThenableSuccessFailure { + refresh() { + return this.promise_(() => { + console.log('success'); + }); + } + +} diff --git a/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/index.js b/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/index.js new file mode 100644 index 000000000000..bb2e50c56b6a --- /dev/null +++ b/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/index.js @@ -0,0 +1,19 @@ +/** + * Copyright 2020 The AMP HTML Authors. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS-IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +const runner = require('@babel/helper-plugin-test-runner').default; + +runner(__dirname); From 565930bd7f1e28097d4645cae02ba65846714db0 Mon Sep 17 00:00:00 2001 From: Kristofer Baxter Date: Thu, 16 Apr 2020 18:19:10 -0700 Subject: [PATCH 2/2] Fix for thenable rejections --- .../babel-plugin-amp-dev-assert-transformer/index.js | 4 ++-- .../test/fixtures/transform-assertions/thenable/output.js | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/index.js b/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/index.js index 3232f144632c..a6fed34dc4cb 100644 --- a/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/index.js +++ b/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/index.js @@ -108,10 +108,10 @@ module.exports = function ({types: t}) { if (isThenable(memberExpression)) { // MemberExpression is thenable, so removing the `devAssert` means thenabling the first argument of `devAssert`. const newCallee = innerCallExpression.get('arguments.0'); - const newArguments = callExpression.get('arguments.0'); + const newArguments = callExpression.get('arguments'); const newCallExpression = t.callExpression( t.cloneDeep(newCallee.node), - [t.cloneDeep(newArguments.node)] + [...newArguments.map((arg) => t.cloneDeep(arg.node))] ); callExpression.replaceWith(newCallExpression); diff --git a/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/thenable/output.js b/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/thenable/output.js index 5a81289fe103..d277758f0fe3 100644 --- a/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/thenable/output.js +++ b/build-system/babel-plugins/babel-plugin-amp-dev-assert-transformer/test/fixtures/transform-assertions/thenable/output.js @@ -26,6 +26,8 @@ class ThenableSuccessFailure { refresh() { return this.promise_(() => { console.log('success'); + }, () => { + console.log('failure'); }); }