typescript-eslint · bradzacher · May 9, 2019 · Apr 26, 2019 · May 9, 2019 · May 9, 2019
diff --git a/.eslintrc.json b/.eslintrc.json
@@ -8,6 +8,7 @@
   "extends": ["eslint:recommended", "plugin:@typescript-eslint/recommended"],
   "rules": {
     "comma-dangle": ["error", "always-multiline"],
+    "curly": ["error", "all"],
     "no-mixed-operators": "error",
     "no-console": "off",
     "no-undef": "off",

diff --git a/packages/eslint-plugin/src/rules/member-naming.ts b/packages/eslint-plugin/src/rules/member-naming.ts
@@ -76,9 +76,13 @@ export default util.createRule<Options, MessageIds>({
       const convention = conventions[accessibility];
 
       const method = node as TSESTree.MethodDefinition;
-      if (method.kind === 'constructor') return;
+      if (method.kind === 'constructor') {
+        return;
+      }
 
-      if (!convention || convention.test(name)) return;
+      if (!convention || convention.test(name)) {
+        return;
+      }
 
       context.report({
         node: node.key,

diff --git a/packages/eslint-plugin/src/rules/no-extraneous-class.ts b/packages/eslint-plugin/src/rules/no-extraneous-class.ts
@@ -96,7 +96,9 @@ export default util.createRule<Options, MessageIds>({
               onlyStatic = false;
             }
           }
-          if (!(onlyStatic || onlyConstructor)) break;
+          if (!(onlyStatic || onlyConstructor)) {
+            break;
+          }
         }
 
         if (onlyConstructor) {

diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts
@@ -1,5 +1,15 @@
 import { TSESTree } from '@typescript-eslint/typescript-estree';
-import * as tsutils from 'tsutils';
+import {
+  isCallExpression,
+  isNewExpression,
+  isObjectType,
+  isObjectFlagSet,
+  isParameterDeclaration,
+  isPropertyDeclaration,
+  isStrictCompilerOptionEnabled,
+  isTypeFlagSet,
+  isVariableDeclaration,
+} from 'tsutils';
 import ts from 'typescript';
 import * as util from '../util';
 
@@ -8,7 +18,7 @@ type Options = [
     typesToIgnore?: string[];
   }
 ];
-type MessageIds = 'unnecessaryAssertion';
+type MessageIds = 'contextuallyUnnecessary' | 'unnecessaryAssertion';
 
 export default util.createRule<Options, MessageIds>({
   name: 'no-unnecessary-type-assertion',
@@ -24,6 +34,8 @@ export default util.createRule<Options, MessageIds>({
     messages: {
       unnecessaryAssertion:
         'This assertion is unnecessary since it does not change the type of the expression.',
+      contextuallyUnnecessary:
+        'This assertion is unnecessary since the receiver accepts the original type of the expression.',
     },
     schema: [
       {
@@ -44,6 +56,8 @@ export default util.createRule<Options, MessageIds>({
   create(context, [options]) {
     const sourceCode = context.getSourceCode();
     const parserServices = util.getParserServices(context);
+    const checker = parserServices.program.getTypeChecker();
+    const compilerOptions = parserServices.program.getCompilerOptions();
 
     /**
      * Sometimes tuple types don't have ObjectFlags.Tuple set, like when they're being matched against an inferred type.
@@ -76,91 +90,170 @@ export default util.createRule<Options, MessageIds>({
       return true;
     }
 
-    function checkNonNullAssertion(
-      node: TSESTree.Node,
+    /**
+     * Returns the contextual type of a given node.
+     * Contextual type is the type of the target the node is going into.
+     * i.e. the type of a called function's parameter, or the defined type of a variable declaration
+     */
+    function getContextualType(
       checker: ts.TypeChecker,
-    ): void {
-      const originalNode = parserServices.esTreeNodeToTSNodeMap.get<
-        ts.NonNullExpression
-      >(node);
-      const type = checker.getTypeAtLocation(originalNode.expression);
-
-      if (type === checker.getNonNullableType(type)) {
-        context.report({
-          node,
-          messageId: 'unnecessaryAssertion',
-          fix(fixer) {
-            return fixer.removeRange([
-              originalNode.expression.end,
-              originalNode.end,
-            ]);
-          },
-        });
+      node: ts.Expression,
+    ): ts.Type | undefined {
+      const parent = node.parent;
+      if (!parent) {
+        return;
       }
-    }
 
-    function verifyCast(
-      node: TSESTree.TSTypeAssertion | TSESTree.TSAsExpression,
-      checker: ts.TypeChecker,
-    ): void {
-      if (
-        options &&
-        options.typesToIgnore &&
-        options.typesToIgnore.indexOf(
-          sourceCode.getText(node.typeAnnotation),
-        ) !== -1
+      if (isCallExpression(parent) || isNewExpression(parent)) {
+        if (node === parent.expression) {
+          // is the callee, so has no contextual type
+          return;
+        }
+      } else if (
+        isVariableDeclaration(parent) ||
+        isPropertyDeclaration(parent) ||
+        isParameterDeclaration(parent)
+      ) {
+        return parent.type
+          ? checker.getTypeFromTypeNode(parent.type)
+          : undefined;
+      } else if (
+        ![ts.SyntaxKind.TemplateSpan, ts.SyntaxKind.JsxExpression].includes(
+          parent.kind,
+        )
       ) {
+        // parent is not something we know we can get the contextual type of
         return;
       }
+      // TODO - support return statement checking
 
-      const originalNode = parserServices.esTreeNodeToTSNodeMap.get<
-        ts.AssertionExpression
-      >(node);
-      const castType = checker.getTypeAtLocation(originalNode);
+      return checker.getContextualType(node);
+    }
 
+    /**
+     * Returns true if there's a chance the variable has been used before a value has been assigned to it
+     */
+    function isPossiblyUsedBeforeAssigned(node: ts.Expression): boolean {
+      const declaration = util.getDeclaration(checker, node);
       if (
-        tsutils.isTypeFlagSet(castType, ts.TypeFlags.Literal) ||
-        (tsutils.isObjectType(castType) &&
-          (tsutils.isObjectFlagSet(castType, ts.ObjectFlags.Tuple) ||
-            couldBeTupleType(castType)))
+        // non-strict mode doesn't care about used before assigned errors
+        isStrictCompilerOptionEnabled(compilerOptions, 'strictNullChecks') &&
+        // ignore class properties as they are compile time guarded
+        // also ignore function arguments as they can't be used before defined
+        isVariableDeclaration(declaration) &&
+        // is it `const x!: number`
+        declaration.initializer === undefined &&
+        declaration.exclamationToken === undefined &&
+        declaration.type !== undefined
       ) {
-        // It's not always safe to remove a cast to a literal type or tuple
-        // type, as those types are sometimes widened without the cast.
-        return;
+        // check if the defined variable type has changed since assignment
+        const declarationType = checker.getTypeFromTypeNode(declaration.type);
+        const type = util.getConstrainedTypeAtLocation(checker, node);
+        if (declarationType === type) {
+          // possibly used before assigned, so just skip it
+          // better to false negative and skip it, than false postiive and fix to compile erroring code
+          //
+          // no better way to figure this out right now
+          // https://github.com/Microsoft/TypeScript/issues/31124
+          return true;
+        }
       }
+      return false;
+    }
+
+    return {
+      TSNonNullExpression(node) {
+        const originalNode = parserServices.esTreeNodeToTSNodeMap.get<
+          ts.NonNullExpression
+        >(node);
+        const type = util.getConstrainedTypeAtLocation(
+          checker,
+          originalNode.expression,
+        );
+
+        if (!util.isNullableType(type)) {
+          if (isPossiblyUsedBeforeAssigned(originalNode.expression)) {
+            return;
+          }
 
-      const uncastType = checker.getTypeAtLocation(originalNode.expression);
-
-      if (uncastType === castType) {
-        context.report({
-          node,
-          messageId: 'unnecessaryAssertion',
-          fix(fixer) {
-            return originalNode.kind === ts.SyntaxKind.TypeAssertionExpression
-              ? fixer.removeRange([
-                  originalNode.getStart(),
-                  originalNode.expression.getStart(),
-                ])
-              : fixer.removeRange([
+          context.report({
+            node,
+            messageId: 'unnecessaryAssertion',
+            fix(fixer) {
+              return fixer.removeRange([
+                originalNode.expression.end,
+                originalNode.end,
+              ]);
+            },
+          });
+        } else {
+          // we know it's a nullable type
+          // so figure out if the variable is used in a place that accepts nullable types
+          const contextualType = getContextualType(checker, originalNode);
+          if (contextualType && util.isNullableType(contextualType)) {
+            context.report({
+              node,
+              messageId: 'contextuallyUnnecessary',
+              fix(fixer) {
+                return fixer.removeRange([
                   originalNode.expression.end,
                   originalNode.end,
                 ]);
-          },
-        });
-      }
-    }
+              },
+            });
+          }
+        }
+      },
+      'TSAsExpression, TSTypeAssertion'(
+        node: TSESTree.TSTypeAssertion | TSESTree.TSAsExpression,
+      ): void {
+        if (
+          options &&
+          options.typesToIgnore &&
+          options.typesToIgnore.indexOf(
+            sourceCode.getText(node.typeAnnotation),
+          ) !== -1
+        ) {
+          return;
+        }
 
-    const checker = parserServices.program.getTypeChecker();
+        const originalNode = parserServices.esTreeNodeToTSNodeMap.get<
+          ts.AssertionExpression
+        >(node);
+        const castType = checker.getTypeAtLocation(originalNode);
 
-    return {
-      TSNonNullExpression(node) {
-        checkNonNullAssertion(node, checker);
-      },
-      TSTypeAssertion(node) {
-        verifyCast(node, checker);
-      },
-      TSAsExpression(node) {
-        verifyCast(node, checker);
+        if (
+          isTypeFlagSet(castType, ts.TypeFlags.Literal) ||
+          (isObjectType(castType) &&
+            (isObjectFlagSet(castType, ts.ObjectFlags.Tuple) ||
+              couldBeTupleType(castType)))
+        ) {
+          // It's not always safe to remove a cast to a literal type or tuple
+          // type, as those types are sometimes widened without the cast.
+          return;
+        }
+
+        const uncastType = checker.getTypeAtLocation(originalNode.expression);
+
+        if (uncastType === castType) {
+          context.report({
+            node,
+            messageId: 'unnecessaryAssertion',
+            fix(fixer) {
+              return originalNode.kind === ts.SyntaxKind.TypeAssertionExpression
+                ? fixer.removeRange([
+                    originalNode.getStart(),
+                    originalNode.expression.getStart(),
+                  ])
+                : fixer.removeRange([
+                    originalNode.expression.end,
+                    originalNode.end,
+                  ]);
+            },
+          });
+        }
+
+        // TODO - add contextually unnecessary check for this
       },
     };
   },

diff --git a/packages/eslint-plugin/src/util/types.ts b/packages/eslint-plugin/src/util/types.ts
@@ -1,4 +1,9 @@
-import * as tsutils from 'tsutils';
+import {
+  isTypeFlagSet,
+  isTypeReference,
+  isUnionOrIntersectionType,
+  unionTypeParts,
+} from 'tsutils';
 import ts from 'typescript';
 
 /**
@@ -10,11 +15,11 @@ export function containsTypeByName(
   type: ts.Type,
   allowedNames: Set<string>,
 ): boolean {
-  if (tsutils.isTypeFlagSet(type, ts.TypeFlags.Any | ts.TypeFlags.Unknown)) {
+  if (isTypeFlagSet(type, ts.TypeFlags.Any | ts.TypeFlags.Unknown)) {
     return true;
   }
 
-  if (tsutils.isTypeReference(type)) {
+  if (isTypeReference(type)) {
     type = type.target;
   }
 
@@ -25,7 +30,7 @@ export function containsTypeByName(
     return true;
   }
 
-  if (tsutils.isUnionOrIntersectionType(type)) {
+  if (isUnionOrIntersectionType(type)) {
     return type.types.some(t => containsTypeByName(t, allowedNames));
   }
 
@@ -35,3 +40,44 @@ export function containsTypeByName(
     bases.some(t => containsTypeByName(t, allowedNames))
   );
 }
+
+/**
+ * Resolves the given node's type. Will resolve to the type's generic constraint, if it has one.
+ */
+export function getConstrainedTypeAtLocation(
+  checker: ts.TypeChecker,
+  node: ts.Node,
+): ts.Type {
+  const nodeType = checker.getTypeAtLocation(node);
+  const constrained = checker.getBaseConstraintOfType(nodeType);
+
+  return constrained || nodeType;
+}
+
+/**
+ * Checks if the given type is (or accepts) nullable
+ * @param isReceiver true if the type is a receiving type (i.e. the type of a called function's parameter)
+ */
+export function isNullableType(type: ts.Type, isReceiver?: boolean): boolean {
+  let flags: ts.TypeFlags = 0;
+  for (const t of unionTypeParts(type)) {
+    flags |= t.flags;
+  }
+
+  flags =
+    isReceiver && flags & (ts.TypeFlags.Any | ts.TypeFlags.Unknown)
+      ? -1
+      : flags;
+
+  return (flags & (ts.TypeFlags.Null | ts.TypeFlags.Undefined)) !== 0;
+}
+
+/**
+ * Gets the declaration for the given variable
+ */
+export function getDeclaration(
+  checker: ts.TypeChecker,
+  node: ts.Expression,
+): ts.Declaration {
+  return checker.getSymbolAtLocation(node)!.declarations![0];
+}