Reduce the number of casts to Node in preferTemplateLiterals

The problem is that ESLint defines a `parent` property on the estree Node. However specific estree Node subtypes, such as MemberExpression, have properties with estree Node subtypes, such as Identifier. The value in practice does have a `parent` property but this is not reflected in types.

This CL makes subrules unaware of the difference by exposing only the estree type to them and makes "core" libraries do the casting where necessary.

Bug: 400353541
Change-Id: I84db45bb35bbfb09f0d6bbde8f730d0f61f09c71
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6400633
Commit-Queue: Danil Somsikov <[email protected]>
Reviewed-by: Philip Pfaffe <[email protected]>

Author: danilsomsikov

scripts/eslint_rules/lib/no-imperative-dom-api.js

@@ -17,7 +17,8 @@ const {DomFragment} = require('./no-imperative-dom-api/dom-fragment.js');
 const toolbar = require('./no-imperative-dom-api/toolbar.js');
 const widget = require('./no-imperative-dom-api/widget.js');
 
-/** @typedef {import('eslint').Rule.Node} Node */
+/** @typedef {import('estree').Node} Node */
+/** @typedef {import('eslint').Rule.Node} EsLintNode */
 /** @typedef {import('eslint').AST.SourceLocation} SourceLocation */
 /** @typedef {import('eslint').Scope.Variable} Variable */
 /** @typedef {import('eslint').Scope.Reference} Reference*/
@@ -64,31 +65,31 @@ module.exports = {
     }
 
     /**
-     *  @param {Node} reference
+     *  @param {EsLintNode} reference
      *  @param {DomFragment} domFragment
      */
     function processReference(reference, domFragment) {
       const parent = reference.parent;
       const isAccessed = parent.type === 'MemberExpression' && parent.object === reference;
-      const property = isAccessed ? /** @type {Node} */ (parent.property) : null;
+      const property = isAccessed ? parent.property : null;
       const grandParent = parent.parent;
       const isPropertyAssignment =
           isAccessed && grandParent.type === 'AssignmentExpression' && grandParent.left === parent;
-      const propertyValue = isPropertyAssignment ? /** @type {Node} */(grandParent.right) : null;
+      const propertyValue = isPropertyAssignment ? grandParent.right : null;
       const isMethodCall = isAccessed && grandParent.type === 'CallExpression' && grandParent.callee === parent;
-      const firstArg = isMethodCall ?  /** @type {Node} */(grandParent.arguments[0]) : null;
-      const secondArg = isMethodCall ? /** @type {Node} */(grandParent.arguments[1]) : null;
+      const firstArg = isMethodCall ? grandParent.arguments[0] : null;
+      const secondArg = isMethodCall ? grandParent.arguments[1] : null;
       const grandGrandParent = grandParent.parent;
       const isPropertyMethodCall = isAccessed && grandParent.type === 'MemberExpression' &&
           grandParent.object === parent && grandGrandParent.type === 'CallExpression' &&
           grandGrandParent.callee === grandParent;
-      const propertyMethodArgument = isPropertyMethodCall ? /** @type {Node} */ (grandGrandParent.arguments[0]) : null;
+      const propertyMethodArgument = isPropertyMethodCall ? grandGrandParent.arguments[0] : null;
       const isSubpropertyAssignment = isAccessed && grandParent.type === 'MemberExpression' &&
           grandParent.object === parent && grandParent.property.type === 'Identifier' &&
           grandGrandParent.type === 'AssignmentExpression' && grandGrandParent.left === grandParent;
       const subproperty =
           isSubpropertyAssignment && grandParent.property.type === 'Identifier' ? grandParent.property : null;
-      const subpropertyValue = isSubpropertyAssignment ? /** @type {Node} */ (grandGrandParent.right) : null;
+      const subpropertyValue = isSubpropertyAssignment ? grandGrandParent.right : null;
       for (const rule of subrules) {
         if (isPropertyAssignment) {
           rule.propertyAssignment?.(property, propertyValue, domFragment);
@@ -151,7 +152,7 @@ module.exports = {
         node: domFragment.replacementLocation,
         messageId: 'preferTemplateLiterals',
         fix(fixer) {
-          let replacementLocation = /** @type {Node} */(domFragment.replacementLocation);
+          let replacementLocation = domFragment.replacementLocation;
           if (replacementLocation.parent.type === 'ExportNamedDeclaration') {
             replacementLocation = replacementLocation.parent;
           }

scripts/eslint_rules/lib/no-imperative-dom-api/adorner.js

@@ -27,19 +27,18 @@ module.exports = {
             if (property.type !== 'Property') {
               continue;
             }
-            const key = /** @type {Node} */ (property.key);
-            if (isIdentifier(key, 'name')) {
+            if (isIdentifier(property.key, 'name')) {
               domFragment.attributes.push({
                 key: 'aria-label',
-                value: /** @type {Node} */ (property.value),
+                value: property.value,
               });
             }
-            if (isIdentifier(key, 'jslogContext')) {
+            if (isIdentifier(property.key, 'jslogContext')) {
               domFragment.attributes.push(
                   {key: 'jslog', value: '${VisualLogging.adorner(' + sourceCode.getText(property.value) + ')}'});
             }
-            if (isIdentifier(key, 'content')) {
-              const childFragment = DomFragment.getOrCreate(/** @type {Node} */ (property.value), sourceCode);
+            if (isIdentifier(property.key, 'content')) {
+              const childFragment = DomFragment.getOrCreate(property.value, sourceCode);
               childFragment.parent = domFragment;
               domFragment.children.push(childFragment);
             }

scripts/eslint_rules/lib/no-imperative-dom-api/ast.js

@@ -6,7 +6,8 @@
  */
 'use strict';
 
-/** @typedef {import('eslint').Rule.Node} Node */
+/** @typedef {import('estree').Node} Node */
+/** @typedef {import('eslint').Rule.Node} EsLintNode */
 
 /**
  * @param {Node} node
@@ -23,12 +24,12 @@ function isIdentifier(node, name) {
  * @param {function(Node): boolean} propertyPredicate
  */
 function isMemberExpression(node, objectPredicate, propertyPredicate) {
-  return node.type === 'MemberExpression' && objectPredicate(/** @type {Node} */ (node.object)) &&
-      propertyPredicate(/** @type {Node} */ (node.property));
+  return node.type === 'MemberExpression' && objectPredicate(node.object) && propertyPredicate(node.property);
 }
 
-/** @param {Node} node */
-function getEnclosingExpression(node) {
+/** @param {Node} estreeNode */
+function getEnclosingExpression(estreeNode) {
+  let node = /** @type {EsLintNode} */ (estreeNode);
   while (node.parent) {
     if (node.parent.type === 'BlockStatement') {
       return node;
@@ -38,17 +39,19 @@ function getEnclosingExpression(node) {
   return null;
 }
 
-function getEnclosingProperty(node) {
+/** @param {Node} estreeNode */
+function getEnclosingProperty(estreeNode) {
+  const node = /** @type {EsLintNode} */ (estreeNode);
   if (node.parent.type === 'AssignmentExpression' && node.parent.right === node &&
       isMemberExpression(node.parent.left, n => n.type === 'ThisExpression', n => n.type === 'Identifier')) {
-    return /** @type {Node} */ (node.parent.left);
+    return node.parent.left;
   }
   return null;
 }
 
 /** @param {Node} node */
 function getEnclosingClassDeclaration(node) {
-  let parent = node.parent;
+  let parent = /** @type {EsLintNode} */ (node).parent;
   while (parent && parent.type !== 'ClassDeclaration') {
     parent = parent.parent;
   }

scripts/eslint_rules/lib/no-imperative-dom-api/dom-fragment.js

@@ -8,7 +8,8 @@
 
 const {getEnclosingProperty, getEnclosingClassDeclaration} = require('./ast.js');
 
-/** @typedef {import('eslint').Rule.Node} Node */
+/** @typedef {import('estree').Node} Node */
+/** @typedef {import('eslint').Rule.Node} EsLintNode */
 /** @typedef {import('eslint').Scope.Variable} Variable */
 /** @typedef {import('eslint').SourceCode} SourceCode */
 
@@ -26,15 +27,16 @@ class DomFragment {
   /** @type {DomFragment[]} */ children = [];
   /** @type {DomFragment|undefined} */ parent;
   /** @type {string|undefined} */ expression;
-  /** @type {Node|undefined} */ replacementLocation;
-  /** @type {Node[]} */ references = [];
+  /** @type {EsLintNode|undefined} */ replacementLocation;
+  /** @type {EsLintNode[]} */ references = [];
 
   /**
-   * @param {Node} node
+   * @param {Node} estreeNode
    * @param {SourceCode} sourceCode
    * @return {DomFragment}
    */
-  static getOrCreate(node, sourceCode) {
+  static getOrCreate(estreeNode, sourceCode) {
+    const node = /** @type {EsLintNode} */ (estreeNode);
     const variable = getEnclosingVariable(node, sourceCode);
     const key = variable ?? sourceCode.getText(getEnclosingProperty(node) ?? node);
 
@@ -43,8 +45,8 @@ class DomFragment {
       result = new DomFragment();
       domFragments.set(key, result);
       if (variable) {
-        result.references = variable.references.map(r => (/** @type {Node} */ (r.identifier)));
-        result.references.push(/** @type {Node} */ (variable.identifiers[0]));
+        result.references = variable.references.map(r => (/** @type {EsLintNode} */ (r.identifier)));
+        result.references.push(/** @type {EsLintNode} */ (variable.identifiers[0]));
       } else {
         result.expression = sourceCode.getText(node);
         const classDeclaration = getEnclosingClassDeclaration(node);
@@ -146,11 +148,12 @@ class DomFragment {
 }
 
 /**
- * @param {Node} node
+ * @param {Node} estreeNode
  * @param {SourceCode} sourceCode
  * @return {Variable|null}
  */
-function getEnclosingVariable(node, sourceCode) {
+function getEnclosingVariable(estreeNode, sourceCode) {
+  const node = /** @type {EsLintNode} */ (estreeNode);
   if (node.type === 'Identifier') {
     let scope = sourceCode.getScope(node);
     const variableName = node.name;