Add eslint rule for binding scheduleRender render argument
Bug: NO_BUG
Change-Id: I64d643a65d0cf4bc3546ed4f75adde7a53c83c4e
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3624641
Reviewed-by: Jack Franklin <[email protected]>
Commit-Queue: Ergün Erdoğmuş <[email protected]>
diff --git a/front_end/.eslintrc.js b/front_end/.eslintrc.js
index d5136d2..deadff1 100644
--- a/front_end/.eslintrc.js
+++ b/front_end/.eslintrc.js
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+// clang-format off
const path = require('path');
const rulesDirPlugin = require('eslint-plugin-rulesdir');
rulesDirPlugin.RULES_DIR = path.join(__dirname, '..', 'scripts', 'eslint_rules', 'lib');
@@ -28,6 +29,7 @@
'rules': {
'@typescript-eslint/explicit-function-return-type': 2,
'rulesdir/no_importing_images_from_src': 2,
+ 'rulesdir/enforce_bound_render_for_schedule_render': 2,
'rulesdir/enforce_custom_event_names': 2,
'rulesdir/set_data_type_reference': 2,
'rulesdir/no_bound_component_methods': 2,
@@ -120,10 +122,7 @@
},
{
// Ignore type properties that require quotes
- 'selector': [
- 'typeProperty',
- 'enumMember'
- ],
+ 'selector': ['typeProperty', 'enumMember'],
'format': null,
'modifiers': ['requiresQuotes']
}
@@ -150,3 +149,4 @@
}
]
};
+// clang-format on
diff --git a/scripts/eslint_rules/lib/enforce_bound_render_for_schedule_render.js b/scripts/eslint_rules/lib/enforce_bound_render_for_schedule_render.js
new file mode 100644
index 0000000..be5c456
--- /dev/null
+++ b/scripts/eslint_rules/lib/enforce_bound_render_for_schedule_render.js
@@ -0,0 +1,110 @@
+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+'use strict';
+
+function goToClassDeclaration(node) {
+ if (!node) {
+ return null;
+ }
+
+ if (node.type === 'ClassDeclaration') {
+ return node;
+ }
+
+ return goToClassDeclaration(node.parent);
+}
+
+function isMemberExpressionOnThis(memberExpression) {
+ if (!memberExpression) {
+ return false;
+ }
+
+ if (memberExpression.object.type === 'ThisExpression') {
+ // Take into `a.this.bind()` case into account
+ // `this` must be the last object in the `MemberExpression` chain
+ return !memberExpression.object.object;
+ }
+
+ return isMemberExpressionOnThis(memberExpression.object);
+}
+
+// Whether the right hand side of property definition is `this.xxx.yyy.bind(this);`
+function isPropertyDefinitionViaBindCallToThis(propertyDefinition) {
+ if (propertyDefinition.value.type !== 'CallExpression' ||
+ propertyDefinition.value.callee.type !== 'MemberExpression') {
+ return false;
+ }
+
+ const isCalleeObjectThis = isMemberExpressionOnThis(propertyDefinition.value.callee);
+ // Whether the CallExpression is on a property of `this` (this.xxx.yyy.bind)
+ if (!isCalleeObjectThis) {
+ return false;
+ }
+
+ const isItBindCall = propertyDefinition.value.callee.property.name === 'bind';
+ // Whether the CallExpression is a `bind` call on a property of `this`
+ if (!isItBindCall) {
+ return false;
+ }
+
+ const callArgument = propertyDefinition.value.arguments[0];
+ // Call argument to `bind` is not `this`
+ if (!callArgument || callArgument.type !== 'ThisExpression') {
+ return false;
+ }
+
+ return true;
+}
+
+// Whether the property definition is arrow function like `#render = () => {}`
+function isPropertyDefinitionViaArrowFunction(propertyDefinition) {
+ return propertyDefinition.value.type === 'ArrowFunctionExpression';
+}
+
+module.exports = {
+ meta: {
+ type: 'problem',
+ docs: {
+ description: 'Enforce render method to be bound while calling scheduleRender',
+ category: 'Possible Errors',
+ },
+ fixable: 'code',
+ schema: [] // no options
+ },
+ create: function(context) {
+ return {
+ CallExpression(node) {
+ // Calls in the form of `ScheduledRender.scheduleRender`
+ const isScheduleRenderCall = node.callee.type === 'MemberExpression' &&
+ node.callee.object?.property?.name === 'ScheduledRender' && node.callee.property?.name === 'scheduleRender';
+ if (!isScheduleRenderCall) {
+ return;
+ }
+
+ const callbackArgument = node.arguments[1];
+ // Whether the second argument points to a property of `this`
+ // like `ScheduledRender.scheduleRender(<any>, this.<any>)
+ if (callbackArgument.type !== 'MemberExpression' || callbackArgument.object.type !== 'ThisExpression') {
+ return;
+ }
+
+ const containingClassForTheCall = goToClassDeclaration(node);
+ // Only care about the calls in custom components
+ if (!containingClassForTheCall.superClass || containingClassForTheCall.superClass.name !== 'HTMLElement') {
+ return;
+ }
+
+ const calledMethod = callbackArgument.property;
+ // Check whether the called method is bound (it should be 'PropertyDefinition')
+ const propertyDefinition = containingClassForTheCall.body.body.find(
+ bodyNode => bodyNode.type === 'PropertyDefinition' && bodyNode.key.name === calledMethod.name);
+ if (!propertyDefinition ||
+ (!isPropertyDefinitionViaArrowFunction(propertyDefinition) &&
+ !isPropertyDefinitionViaBindCallToThis(propertyDefinition))) {
+ context.report({node, message: 'Bind `render` method of `scheduleRender` to `this` in components'});
+ }
+ }
+ };
+ }
+};
diff --git a/scripts/eslint_rules/tests/enforce_bound_render_for_schedule_render_test.js b/scripts/eslint_rules/tests/enforce_bound_render_for_schedule_render_test.js
new file mode 100644
index 0000000..4e40d89
--- /dev/null
+++ b/scripts/eslint_rules/tests/enforce_bound_render_for_schedule_render_test.js
@@ -0,0 +1,74 @@
+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+'use strict';
+
+const rule = require('../lib/enforce_bound_render_for_schedule_render.js');
+const ruleTester = new (require('eslint').RuleTester)({
+ parserOptions: {ecmaVersion: 9, sourceType: 'module'},
+ parser: require.resolve('@typescript-eslint/parser'),
+});
+
+ruleTester.run('enforce_bound_render_for_schedule_render', rule, {
+ valid: [
+ {
+ code: `
+ class Component extends HTMLElement {
+ #boundRender = this.#render.bind(this);
+ get data(data) {
+ this.data = data;
+ void ComponentHelpers.ScheduledRender.scheduleRender(this, this.#boundRender);
+ }
+
+ #render() {}
+ }
+ `,
+ filename: 'front_end/ui/components/foo/Foo.ts',
+ },
+ {
+ code: `
+ class Component extends HTMLElement {
+ get data(data) {
+ this.data = data;
+ void ComponentHelpers.ScheduledRender.scheduleRender(this, this.#render);
+ }
+
+ #render = () => {};
+ }
+ `,
+ filename: 'front_end/ui/components/foo/Foo.ts',
+ },
+ {
+ code: `
+ class Renderer {
+ render() {
+ }
+ }
+
+ class Component extends HTMLElement {
+ #renderer = new Renderer();
+ #boundRender = this.#renderer.render.bind(this);
+ get data(data) {
+ this.data = data;
+ void ComponentHelpers.ScheduledRender.scheduleRender(this, this.#boundRender);
+ }
+ }
+ `,
+ filename: 'front_end/ui/components/foo/Foo.ts',
+ },
+ ],
+ invalid: [
+ {
+ code: `class Component extends HTMLElement {
+ get data(data) {
+ this.data = data;
+ void ComponentHelpers.ScheduledRender.scheduleRender(this, this.#render);
+ }
+
+ #render() {}
+ }`,
+ filename: 'front_end/components/test.ts',
+ errors: [{message: 'Bind `render` method of `scheduleRender` to `this` in components'}],
+ },
+ ]
+});
