fix(typescript-estree): correct parenthesized optional chain AST (#1141)

- Also fixes the package not working in the browser: - ts.sys undefined in the browser - process is undefined in the browser - Whitelist 3.7.1-rc
typescript-eslint · Oct 25, 2019 · 5ae286e · 5ae286e
1 parent 1508670
commit 5ae286e
Show file tree

Hide file tree

Showing 11 changed files with 9,915 additions and 874 deletions.
diff --git a/packages/parser/tests/lib/__snapshots__/typescript.ts.snap b/packages/parser/tests/lib/__snapshots__/typescript.ts.snap
diff --git a/packages/shared-fixtures/fixtures/typescript/basics/optional-chain-call-with-parens.src.ts b/packages/shared-fixtures/fixtures/typescript/basics/optional-chain-call-with-parens.src.ts
@@ -0,0 +1,13 @@
+function processOptionalCallParens(one?: any) {
+  (one?.fn());
+  (one?.two).fn();
+  (one.two?.fn());
+  (one.two?.three).fn();
+  (one.two?.three?.fn());
+
+  (one?.());
+  (one?.())();
+  (one?.())?.();
+
+  (one?.()).two;
+}
diff --git a/packages/shared-fixtures/fixtures/typescript/basics/optional-chain-call.src.ts b/packages/shared-fixtures/fixtures/typescript/basics/optional-chain-call.src.ts
@@ -6,6 +6,7 @@ function processOptionalCall(one?: any) {
   one.two?.three?.fn();
 
   one?.();
+  one?.()();
   one?.()?.();
 
   one?.().two;

diff --git a/...ared-fixtures/fixtures/typescript/basics/optional-chain-element-access-with-parens.src.ts b/...ared-fixtures/fixtures/typescript/basics/optional-chain-element-access-with-parens.src.ts
@@ -0,0 +1,8 @@
+function processOptionalElementParens(one?: any) {
+  (one?.[2]);
+  (one?.[2])[3];
+  (one[2]?.[3]);
+  (one[2]?.[3])[4];
+  (one[2]?.[3]?.[4]);
+  (one[2]?.[3]?.[4])[5];
+}
diff --git a/packages/shared-fixtures/fixtures/typescript/basics/optional-chain-with-parens.src.ts b/packages/shared-fixtures/fixtures/typescript/basics/optional-chain-with-parens.src.ts
@@ -0,0 +1,8 @@
+function processOptionalParens(one?: any) {
+  (one?.two);
+  (one?.two).three;
+  (one.two?.three);
+  (one.two?.three).four;
+  (one.two?.three?.four);
+  (one.two?.three?.four).five;
+}
diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts
@@ -1691,16 +1691,19 @@ export class Converter {
       }
 
       case SyntaxKind.PropertyAccessExpression: {
-        const isLocallyOptional = node.questionDotToken !== undefined;
         const object = this.convertChild(node.expression);
         const property = this.convertChild(node.name);
         const computed = false;
-        if (
-          isLocallyOptional ||
-          // the optional expression should propogate up the member expression tree
-          object.type === AST_NODE_TYPES.OptionalMemberExpression ||
-          object.type === AST_NODE_TYPES.OptionalCallExpression
-        ) {
+
+        const isLocallyOptional = node.questionDotToken !== undefined;
+        // the optional expression should propogate up the member expression tree
+        const isChildOptional =
+          (object.type === AST_NODE_TYPES.OptionalMemberExpression ||
+            object.type === AST_NODE_TYPES.OptionalCallExpression) &&
+          // (x?.y).z is semantically different, and as such .z is no longer optional
+          node.expression.kind !== ts.SyntaxKind.ParenthesizedExpression;
+
+        if (isLocallyOptional || isChildOptional) {
           return this.createNode<TSESTree.OptionalMemberExpression>(node, {
             type: AST_NODE_TYPES.OptionalMemberExpression,
             object,
@@ -1720,16 +1723,19 @@ export class Converter {
       }
 
       case SyntaxKind.ElementAccessExpression: {
-        const isLocallyOptional = node.questionDotToken !== undefined;
         const object = this.convertChild(node.expression);
         const property = this.convertChild(node.argumentExpression);
         const computed = true;
-        if (
-          isLocallyOptional ||
-          // the optional expression should propogate up the member expression tree
-          object.type === AST_NODE_TYPES.OptionalMemberExpression ||
-          object.type === AST_NODE_TYPES.OptionalCallExpression
-        ) {
+
+        const isLocallyOptional = node.questionDotToken !== undefined;
+        // the optional expression should propogate up the member expression tree
+        const isChildOptional =
+          (object.type === AST_NODE_TYPES.OptionalMemberExpression ||
+            object.type === AST_NODE_TYPES.OptionalCallExpression) &&
+          // (x?.y).z is semantically different, and as such .z is no longer optional
+          node.expression.kind !== ts.SyntaxKind.ParenthesizedExpression;
+
+        if (isLocallyOptional || isChildOptional) {
           return this.createNode<TSESTree.OptionalMemberExpression>(node, {
             type: AST_NODE_TYPES.OptionalMemberExpression,
             object,
@@ -1749,16 +1755,19 @@ export class Converter {
       }
 
       case SyntaxKind.CallExpression: {
-        const isLocallyOptional = node.questionDotToken !== undefined;
         const callee = this.convertChild(node.expression);
         const args = node.arguments.map(el => this.convertChild(el));
         let result;
-        if (
-          isLocallyOptional ||
-          // the optional expression should propogate up the member expression tree
-          callee.type === AST_NODE_TYPES.OptionalMemberExpression ||
-          callee.type === AST_NODE_TYPES.OptionalCallExpression
-        ) {
+
+        const isLocallyOptional = node.questionDotToken !== undefined;
+        // the optional expression should propogate up the member expression tree
+        const isChildOptional =
+          (callee.type === AST_NODE_TYPES.OptionalMemberExpression ||
+            callee.type === AST_NODE_TYPES.OptionalCallExpression) &&
+          // (x?.y).z() is semantically different, and as such .z() is no longer optional
+          node.expression.kind !== ts.SyntaxKind.ParenthesizedExpression;
+
+        if (isLocallyOptional || isChildOptional) {
           result = this.createNode<TSESTree.OptionalCallExpression>(node, {
             type: AST_NODE_TYPES.OptionalCallExpression,
             callee,

diff --git a/packages/typescript-estree/src/create-program/shared.ts b/packages/typescript-estree/src/create-program/shared.ts
@@ -20,7 +20,10 @@ const DEFAULT_COMPILER_OPTIONS: ts.CompilerOptions = {
 
 // This narrows the type so we can be sure we're passing canonical names in the correct places
 type CanonicalPath = string & { __brand: unknown };
-const getCanonicalFileName = ts.sys.useCaseSensitiveFileNames
+// typescript doesn't provide a ts.sys implementation for browser environments
+const useCaseSensitiveFileNames =
+  ts.sys !== undefined ? ts.sys.useCaseSensitiveFileNames : true;
+const getCanonicalFileName = useCaseSensitiveFileNames
   ? (filePath: string): CanonicalPath =>
       path.normalize(filePath) as CanonicalPath
   : (filePath: string): CanonicalPath =>

diff --git a/packages/typescript-estree/src/parser.ts b/packages/typescript-estree/src/parser.ts
@@ -16,11 +16,18 @@ import { TSESTree } from './ts-estree';
  * This needs to be kept in sync with the top-level README.md in the
  * typescript-eslint monorepo
  */
-const SUPPORTED_TYPESCRIPT_VERSIONS = '>=3.2.1 <3.8.0 || >3.7.0-dev.0';
+const SUPPORTED_TYPESCRIPT_VERSIONS = '>=3.2.1 <3.8.0';
+/*
+ * The semver package will ignore prerelease ranges, and we don't want to explicitly document every one
+ * List them all separately here, so we can automatically create the full string
+ */
+const SUPPORTED_PRERELEASE_RANGES = ['>3.7.0-dev.0', '3.7.1-rc'];
 const ACTIVE_TYPESCRIPT_VERSION = ts.version;
 const isRunningSupportedTypeScriptVersion = semver.satisfies(
   ACTIVE_TYPESCRIPT_VERSION,
-  SUPPORTED_TYPESCRIPT_VERSIONS,
+  [SUPPORTED_TYPESCRIPT_VERSIONS]
+    .concat(SUPPORTED_PRERELEASE_RANGES)
+    .join(' || '),
 );
 
 let extra: Extra;
@@ -224,22 +231,21 @@ function applyParserOptionsToExtra(options: TSESTreeOptions): void {
 }
 
 function warnAboutTSVersion(): void {
-  if (
-    !isRunningSupportedTypeScriptVersion &&
-    !warnedAboutTSVersion &&
-    process.stdout.isTTY
-  ) {
-    const border = '=============';
-    const versionWarning = [
-      border,
-      'WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree.',
-      'You may find that it works just fine, or you may not.',
-      `SUPPORTED TYPESCRIPT VERSIONS: ${SUPPORTED_TYPESCRIPT_VERSIONS}`,
-      `YOUR TYPESCRIPT VERSION: ${ACTIVE_TYPESCRIPT_VERSION}`,
-      'Please only submit bug reports when using the officially supported version.',
-      border,
-    ];
-    extra.log(versionWarning.join('\n\n'));
+  if (!isRunningSupportedTypeScriptVersion && !warnedAboutTSVersion) {
+    const isTTY = typeof process === undefined ? false : process.stdout.isTTY;
+    if (isTTY) {
+      const border = '=============';
+      const versionWarning = [
+        border,
+        'WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree.',
+        'You may find that it works just fine, or you may not.',
+        `SUPPORTED TYPESCRIPT VERSIONS: ${SUPPORTED_TYPESCRIPT_VERSIONS}`,
+        `YOUR TYPESCRIPT VERSION: ${ACTIVE_TYPESCRIPT_VERSION}`,
+        'Please only submit bug reports when using the officially supported version.',
+        border,
+      ];
+      extra.log(versionWarning.join('\n\n'));
+    }
     warnedAboutTSVersion = true;
   }
 }

diff --git a/packages/typescript-estree/tests/ast-alignment/fixtures-to-test.ts b/packages/typescript-estree/tests/ast-alignment/fixtures-to-test.ts
@@ -583,8 +583,11 @@ tester.addFixturePatternConfig('typescript/basics', {
      */
     // optional chaining
     'optional-chain',
+    'optional-chain-with-parens',
     'optional-chain-call',
+    'optional-chain-call-with-parens',
     'optional-chain-element-access',
+    'optional-chain-element-access-with-parens',
     'async-function-expression',
     'class-with-accessibility-modifiers',
     'class-with-mixin',

diff --git a/packages/typescript-estree/tests/lib/__snapshots__/semantic-diagnostics-enabled.ts.snap b/packages/typescript-estree/tests/lib/__snapshots__/semantic-diagnostics-enabled.ts.snap
@@ -1922,8 +1922,14 @@ exports[`Parse all fixtures with "errorOnTypeScriptSyntacticAndSemanticIssues" e
 
 exports[`Parse all fixtures with "errorOnTypeScriptSyntacticAndSemanticIssues" enabled fixtures/typescript/basics/optional-chain-call.src 1`] = `"TEST OUTPUT: No semantic or syntactic issues found"`;
 
+exports[`Parse all fixtures with "errorOnTypeScriptSyntacticAndSemanticIssues" enabled fixtures/typescript/basics/optional-chain-call-with-parens.src 1`] = `"TEST OUTPUT: No semantic or syntactic issues found"`;
+
 exports[`Parse all fixtures with "errorOnTypeScriptSyntacticAndSemanticIssues" enabled fixtures/typescript/basics/optional-chain-element-access.src 1`] = `"TEST OUTPUT: No semantic or syntactic issues found"`;
 
+exports[`Parse all fixtures with "errorOnTypeScriptSyntacticAndSemanticIssues" enabled fixtures/typescript/basics/optional-chain-element-access-with-parens.src 1`] = `"TEST OUTPUT: No semantic or syntactic issues found"`;
+
+exports[`Parse all fixtures with "errorOnTypeScriptSyntacticAndSemanticIssues" enabled fixtures/typescript/basics/optional-chain-with-parens.src 1`] = `"TEST OUTPUT: No semantic or syntactic issues found"`;
+
 exports[`Parse all fixtures with "errorOnTypeScriptSyntacticAndSemanticIssues" enabled fixtures/typescript/basics/parenthesized-use-strict.src 1`] = `"TEST OUTPUT: No semantic or syntactic issues found"`;
 
 exports[`Parse all fixtures with "errorOnTypeScriptSyntacticAndSemanticIssues" enabled fixtures/typescript/basics/readonly-arrays.src 1`] = `"TEST OUTPUT: No semantic or syntactic issues found"`;