fix: do not throw an error if decorator expr is wrapped in a paren expr

Closes #1214
dsherret · Nov 13, 2021 · c77db30 · c77db30
1 parent be87373
commit c77db30
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 17 deletions.
diff --git a/bvm.json b/bvm.json
@@ -1,9 +1,9 @@
 {
   "binaries": [
     {
-      "path": "https://bvm.land/deno/1.15.3.json",
-      "checksum": "adb30172c0741a161ce6b53651e16f832c44e67bf1cabf55a9dd9780505b749a",
-      "version": "1.15.3"
+      "path": "https://bvm.land/deno/1.16.1.json",
+      "checksum": "0294b4e8d9426972cf015ebd3cc0d657eab9b20983790a92c27c96c265cb73d1",
+      "version": "1.16.1"
     },
     {
       "path": "https://github.com/dprint/dprint/releases/download/0.18.1/bvm.json",

diff --git a/packages/ts-morph/src/compiler/ast/decorator/Decorator.ts b/packages/ts-morph/src/compiler/ast/decorator/Decorator.ts
@@ -21,25 +21,24 @@ export class Decorator extends DecoratorBase<ts.Decorator> {
      * Gets the name node of the decorator.
      */
     getNameNode() {
-        if (this.isDecoratorFactory()) {
-            const callExpression = this.getCallExpression()!;
+        const callExpression = this.getCallExpression();
+        if (callExpression)
             return getIdentifierFromName(callExpression.getExpression());
-        }
-
-        return getIdentifierFromName(this.getExpression());
+        else
+            return getIdentifierFromName(this.#getInnerExpression());
 
         function getIdentifierFromName(expression: Expression) {
             const identifier = getNameFromExpression(expression);
             if (!Node.isIdentifier(identifier)) {
                 throw new errors.NotImplementedError(
-                    `Expected the decorator expression '${identifier.getText()}' to be an identifier, `
-                        + `but it wasn't. Please report this as a bug.`,
+                    `Expected the decorator expression '${identifier.getText()}' to be an identifier. `
+                        + `Please deal directly with 'getExpression()' on the decorator to handle more complex scenarios.`,
                 );
             }
             return identifier;
         }
 
-        function getNameFromExpression(expression: Expression) {
+        function getNameFromExpression(expression: Expression): Node {
             if (Node.isPropertyAccessExpression(expression))
                 return expression.getNameNode();
             return expression;
@@ -61,7 +60,7 @@ export class Decorator extends DecoratorBase<ts.Decorator> {
      * Gets if the decorator is a decorator factory.
      */
     isDecoratorFactory() {
-        return this.compilerNode.expression.kind === SyntaxKind.CallExpression;
+        return Node.isCallExpression(this.#getInnerExpression());
     }
 
     /**
@@ -73,7 +72,7 @@ export class Decorator extends DecoratorBase<ts.Decorator> {
             return this;
 
         if (isDecoratorFactory) {
-            const expression = this.getExpression();
+            const expression = this.#getInnerExpression();
             const expressionText = expression.getText();
             insertIntoParentTextRange({
                 parent: this,
@@ -121,10 +120,8 @@ export class Decorator extends DecoratorBase<ts.Decorator> {
      * Gets the call expression if a decorator factory.
      */
     getCallExpression(): CallExpression | undefined {
-        if (!this.isDecoratorFactory())
-            return undefined;
-
-        return this.getExpression() as any as CallExpression;
+        const expression = this.#getInnerExpression();
+        return Node.isCallExpression(expression) ? expression : undefined;
     }
 
     /**
@@ -269,6 +266,13 @@ export class Decorator extends DecoratorBase<ts.Decorator> {
         }
     }
 
+    #getInnerExpression() {
+        let expr: Expression = this.getExpression();
+        while (Node.isParenthesizedExpression(expr))
+            expr = expr.getExpression();
+        return expr;
+    }
+
     /**
      * Sets the node from a structure.
      * @param structure - Structure to set the node with.

diff --git a/packages/ts-morph/src/tests/compiler/ast/decorator/decoratorTests.ts b/packages/ts-morph/src/tests/compiler/ast/decorator/decoratorTests.ts
@@ -26,6 +26,11 @@ describe(nameof(Decorator), () => {
             const { firstDecorator } = getFirstClassDecorator("@decorator('str', 23)\nclass Identifier {}");
             expect(firstDecorator.isDecoratorFactory()).to.equal(true);
         });
+
+        it("should be even when surrounded in parens", () => {
+            const { firstDecorator } = getFirstClassDecorator("@(decorator('str', 23))\nclass Identifier {}");
+            expect(firstDecorator.isDecoratorFactory()).to.equal(true);
+        });
     });
 
     describe(nameof<Decorator>(d => d.setIsDecoratorFactory), () => {
@@ -83,6 +88,10 @@ describe(nameof(Decorator), () => {
         it("should get the name node for a decorator factory decorator with a namespace", () => {
             doTest("@namespaceTest.decorator()\nclass Identifier {}", "decorator");
         });
+
+        it("should get the name node for a decorator wrapped in a paren expr", () => {
+            doTest("@(decorator())\nclass Identifier {}", "decorator");
+        });
     });
 
     describe(nameof<Decorator>(d => d.getName), () => {