From 75b19bee914da27d601ced83296c00f83352d7c5 Mon Sep 17 00:00:00 2001 From: rishipal Date: Thu, 23 May 2024 09:52:29 -0700 Subject: [PATCH] This CL does: - fork the method so that implementation is private, - the current entry point is deprecated, - the new entry point asserts that the node is either a lhs "let, const, var" name node, or a RHS class/function, or a class/function declaration. - adds unit tests for the new entry point PiperOrigin-RevId: 636583160 --- .../google/javascript/jscomp/NodeUtil.java | 70 +++++++- .../javascript/jscomp/NodeUtilTest.java | 158 ++++++++++++++++++ 2 files changed, 225 insertions(+), 3 deletions(-) diff --git a/src/com/google/javascript/jscomp/NodeUtil.java b/src/com/google/javascript/jscomp/NodeUtil.java index df5a6bcabce..2a0d8fb022e 100644 --- a/src/com/google/javascript/jscomp/NodeUtil.java +++ b/src/com/google/javascript/jscomp/NodeUtil.java @@ -3068,9 +3068,7 @@ static boolean isClassExpression(Node n) { return n.isClass() && (!isNamedClass(n) || !isDeclarationParent(n.getParent())); } - /** - * @return Whether the node is both a function expression and the function is named. - */ + /** Returns whether the node is both a class expression and the class is named. */ static boolean isNamedClassExpression(Node n) { return NodeUtil.isClassExpression(n) && n.getFirstChild().isName(); } @@ -5179,7 +5177,73 @@ private static boolean isToStringMethodCall(Node call) { return jsdocNode == null ? null : jsdocNode.getJSDocInfo(); } + /** + * Find the best JSDocInfo node for the given node. This version is stricter than {@code + * getBestJSDocInfoNode} in that it only accepts a node that is either a lhs "let, const, var" + * name node, or a RHS class/function, or a class/function declaration, or an export. + * + * @throws IllegalStateException if any other input node token is provided. + */ + public static @Nullable Node getBestJsDocInfoNodeStrict(Node n) { + boolean isDeclaredName = n.isName() && NodeUtil.isNameDeclaration(n.getParent()); + if (isDeclaredName) { + return getBestJsDocInfoNodeInternal(n); + } + boolean isDeclaredClass = NodeUtil.isClassDeclaration(n); + if (isDeclaredClass) { + return getBestJsDocInfoNodeInternal(n); + } + boolean isDeclaredFunction = NodeUtil.isFunctionDeclaration(n); + if (isDeclaredFunction) { + return getBestJsDocInfoNodeInternal(n); + } + boolean isRhsClass = isRhsClass(n); + if (isRhsClass) { + return getBestJsDocInfoNodeInternal(n); + } + boolean isRhsFunction = isRhsFunction(n); + if (isRhsFunction) { + return getBestJsDocInfoNodeInternal(n); + } + boolean isRhsClassName = n.isName() && isRhsClass(n.getParent()); + if (isRhsClassName) { + return getBestJsDocInfoNodeInternal(n); + } + boolean isRhsFunctionName = n.isName() && isRhsFunction(n.getParent()); + if (isRhsFunctionName) { + return getBestJsDocInfoNodeInternal(n); + } + boolean isExport = n.isExport(); + if (isExport) { + return getBestJsDocInfoNodeInternal(n); + } + throw new IllegalStateException("Not allowed to get JSDocInfo node for node: " + n); + } + + // e.g. `class A` in `/** some */ var x = class A {};` + private static boolean isRhsClass(Node n) { + return NodeUtil.isNamedClassExpression(n) + && (n.getParent().isName() || n.getParent().isAssign()); + } + + // e.g. `function A` in `/** some */ var x = function A() {};` + private static boolean isRhsFunction(Node n) { + return NodeUtil.isNamedFunctionExpression(n) + && (n.getParent().isName() || n.getParent().isAssign()); + } + + /** + * Find the best JSDocInfo node for the given node. + * + * @deprecated because we want to control the input node tokens accepted by this function. Use + * {@code getBestJSDocInfoNodeStrict} instead. + */ + @Deprecated public static @Nullable Node getBestJSDocInfoNode(Node n) { + return getBestJsDocInfoNodeInternal(n); + } + + private static @Nullable Node getBestJsDocInfoNodeInternal(Node n) { if (n.isExprResult()) { return getBestJSDocInfoNode(n.getFirstChild()); } diff --git a/test/com/google/javascript/jscomp/NodeUtilTest.java b/test/com/google/javascript/jscomp/NodeUtilTest.java index 35397b84dbe..6ebbde66cb4 100644 --- a/test/com/google/javascript/jscomp/NodeUtilTest.java +++ b/test/com/google/javascript/jscomp/NodeUtilTest.java @@ -2690,6 +2690,164 @@ public void testIsNotLValue() { assertNotLValueNamedX(x); } + @Test + public void testGetBestJsDocInfoNodeStrict_declaredName_doesNotThrow() { + Node aName = parse("/** some */ let A;").getFirstFirstChild(); + assertThat(aName.isName()).isTrue(); + + Node bestJsDocInfoNode = NodeUtil.getBestJSDocInfoNode(aName); + assertThat(bestJsDocInfoNode).isNotNull(); + assertThat(bestJsDocInfoNode.equals(aName.getParent())).isTrue(); + + // check that the strict version returns the same node + Node bestJsDocInfoNodeStrict = NodeUtil.getBestJsDocInfoNodeStrict(aName); + assertThat(bestJsDocInfoNodeStrict).isNotNull(); + assertThat(bestJsDocInfoNodeStrict.equals(aName.getParent())).isTrue(); + } + + @Test + public void testGetBestJsDocInfoNodeStrict_classDeclaration_doesNotThrow() { + Node classNode = parse("/** some */ class A {}").getFirstChild(); + assertThat(classNode.isClass()).isTrue(); + + Node bestJsDocInfoNode = NodeUtil.getBestJSDocInfoNode(classNode); + assertThat(bestJsDocInfoNode).isNotNull(); + assertThat(bestJsDocInfoNode.equals(classNode)).isTrue(); + + // check that the strict version returns the same node + Node bestJsDocInfoNodeStrict = NodeUtil.getBestJsDocInfoNodeStrict(classNode); + assertThat(bestJsDocInfoNodeStrict).isNotNull(); + assertThat(bestJsDocInfoNodeStrict.equals(classNode)).isTrue(); + } + + @Test + public void testGetBestJsDocInfoNodeStrict_rhsNamedClassExpression_doesNotThrow() { + Node constNode = parse("/** some */ const x = class A {}").getFirstChild(); + Node xName = constNode.getFirstChild(); + Node classNode = xName.getFirstChild(); + assertThat(classNode.isClass()).isTrue(); + + Node bestJsDocInfoNode = NodeUtil.getBestJSDocInfoNode(classNode); + assertThat(bestJsDocInfoNode).isNotNull(); + assertThat(bestJsDocInfoNode.equals(constNode)).isTrue(); + + // check that the strict version returns the same node + Node bestJsDocInfoNodeStrict = NodeUtil.getBestJsDocInfoNodeStrict(classNode); + assertThat(bestJsDocInfoNodeStrict).isNotNull(); + assertThat(bestJsDocInfoNodeStrict.equals(constNode)).isTrue(); + } + + @Test + public void testGetBestJsDocInfoNodeStrict_rhsUnnamedClassExpression_throws() { + Node constNode = parse("/** some */ const x = class {}").getFirstChild(); + Node xName = constNode.getFirstChild(); + Node classNode = xName.getFirstChild(); + assertThat(classNode.isClass()).isTrue(); + + Node bestJsDocInfoNode = NodeUtil.getBestJSDocInfoNode(classNode); + assertThat(bestJsDocInfoNode).isNotNull(); + assertThat(bestJsDocInfoNode.equals(constNode)).isTrue(); + + // check that the strict version rejects an unnamed class expression + IllegalStateException e = + assertThrows( + IllegalStateException.class, () -> NodeUtil.getBestJsDocInfoNodeStrict(classNode)); + assertThat(e.getMessage().contains("Not allowed to get JSDocInfo node for node")).isTrue(); + } + + @Test + public void testGetBestJsDocInfoNodeStrict_rhsClassName_doesNotThrow() { + Node constNode = parse("/** some */ const x = class A {}").getFirstChild(); + Node xName = constNode.getFirstChild(); + Node classNode = xName.getFirstChild(); + assertThat(classNode.isClass()).isTrue(); + Node className = classNode.getFirstChild(); + assertThat(className.isName()).isTrue(); + + Node bestJsDocInfoNode = NodeUtil.getBestJSDocInfoNode(className); + assertThat(bestJsDocInfoNode).isNotNull(); + assertThat(bestJsDocInfoNode.equals(constNode)).isTrue(); + + // check that the strict version returns the same node + Node bestJsDocInfoNodeStrict = NodeUtil.getBestJsDocInfoNodeStrict(className); + assertThat(bestJsDocInfoNodeStrict).isNotNull(); + assertThat(bestJsDocInfoNodeStrict.equals(constNode)).isTrue(); + } + + @Test + public void testGetBestJsDocInfoNodeStrict_rhsNamedFunctionExpression_doesNotThrow() { + Node constNode = parse("/** some */ const x = function A() {}").getFirstChild(); + Node xName = constNode.getFirstChild(); + Node functionNode = xName.getFirstChild(); + assertThat(functionNode.isFunction()).isTrue(); + + Node bestJsDocInfoNode = NodeUtil.getBestJSDocInfoNode(functionNode); + assertThat(bestJsDocInfoNode).isNotNull(); + assertThat(bestJsDocInfoNode.equals(constNode)).isTrue(); + + // check that the strict version returns the same node + Node bestJsDocInfoNodeStrict = NodeUtil.getBestJsDocInfoNodeStrict(functionNode); + assertThat(bestJsDocInfoNodeStrict).isNotNull(); + assertThat(bestJsDocInfoNodeStrict.equals(constNode)).isTrue(); + } + + @Test + public void testGetBestJsDocInfoNodeStrict_exportedFunction_doesNotThrow() { + Node exportNode = parse("/** some */ export function A() {}").getFirstFirstChild(); + assertThat(exportNode.isExport()).isTrue(); + Node functionNode = exportNode.getFirstChild(); + assertThat(functionNode.isFunction()).isTrue(); + + Node bestJsDocInfoNode = NodeUtil.getBestJSDocInfoNode(functionNode); + assertThat(bestJsDocInfoNode).isNotNull(); + // JSDoc gets attached to the children of export nodes, and there are no warnings. + // See https://github.com/google/closure-compiler/issues/781 + // Hence, the best JSDocInfo node is the function node itself when the search is starting from + // it. + assertThat(bestJsDocInfoNode.equals(functionNode)).isTrue(); + + // check that the strict version returns the same node + Node bestJsDocInfoNodeStrict = NodeUtil.getBestJsDocInfoNodeStrict(functionNode); + assertThat(bestJsDocInfoNodeStrict).isNotNull(); + assertThat(bestJsDocInfoNodeStrict.equals(functionNode)).isTrue(); + } + + @Test + public void testGetBestJsDocInfoNodeStrict_exportNode_doesNotThrow() { + Node exportNode = parse("/** some */ export function A() {}").getFirstFirstChild(); + assertThat(exportNode.isExport()).isTrue(); + Node functionNode = exportNode.getFirstChild(); + assertThat(functionNode.isFunction()).isTrue(); + + Node bestJsDocInfoNode = NodeUtil.getBestJSDocInfoNode(exportNode); + assertThat(bestJsDocInfoNode).isNotNull(); + assertThat(bestJsDocInfoNode.equals(exportNode)).isTrue(); + + // check that the strict version returns the same node + Node bestJsDocInfoNodeStrict = NodeUtil.getBestJsDocInfoNodeStrict(exportNode); + assertThat(bestJsDocInfoNodeStrict).isNotNull(); + assertThat(bestJsDocInfoNodeStrict.equals(exportNode)).isTrue(); + } + + @Test + public void testGetBestJsDocInfoNodeStrict_rhsFunctionName_doesNotThrow() { + Node constNode = parse("/** some */ const x = function A() {}").getFirstChild(); + Node xName = constNode.getFirstChild(); + Node functionNode = xName.getFirstChild(); + assertThat(functionNode.isFunction()).isTrue(); + Node functionName = functionNode.getFirstChild(); + assertThat(functionName.isName()).isTrue(); + + Node bestJsDocInfoNode = NodeUtil.getBestJSDocInfoNode(functionName); + assertThat(bestJsDocInfoNode).isNotNull(); + assertThat(bestJsDocInfoNode.equals(constNode)).isTrue(); + + // check that the strict version returns the same node + Node bestJsDocInfoNodeStrict = NodeUtil.getBestJsDocInfoNodeStrict(functionName); + assertThat(bestJsDocInfoNodeStrict).isNotNull(); + assertThat(bestJsDocInfoNodeStrict.equals(constNode)).isTrue(); + } + @Test public void testIsConstantDeclaration() { assertIsConstantDeclaration(false, parse("var x = 1;").getFirstFirstChild());