Merge branch 'master' into prefer-type-alias

packages/eslint-plugin-tslint/package.json

@@ -19,10 +19,11 @@
   },
   "license": "MIT",
   "scripts": {
-    "test": "jest --coverage",
-    "prebuild": "npm run clean",
     "build": "tsc -p tsconfig.build.json",
     "clean": "rimraf dist/",
+    "format": "prettier --write \"./**/*.{ts,js,json,md}\" --ignore-path ../../.prettierignore",
+    "prebuild": "npm run clean",
+    "test": "jest --coverage",
     "typecheck": "tsc --noEmit"
   },
   "dependencies": {

packages/eslint-plugin/README.md

@@ -135,6 +135,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e
 | [`@typescript-eslint/no-extraneous-class`](./docs/rules/no-extraneous-class.md)                           | Forbids the use of classes as namespaces (`no-unnecessary-class` from TSLint)                                                                       |                    |          |                   |
 | [`@typescript-eslint/no-for-in-array`](./docs/rules/no-for-in-array.md)                                   | Disallow iterating over an array with a for-in loop (`no-for-in-array` from TSLint)                                                                 |                    |          | :thought_balloon: |
 | [`@typescript-eslint/no-inferrable-types`](./docs/rules/no-inferrable-types.md)                           | Disallows explicit type declarations for variables or parameters initialized to a number, string, or boolean. (`no-inferrable-types` from TSLint)   | :heavy_check_mark: | :wrench: |                   |
+| [`@typescript-eslint/no-magic-numbers`](./docs/rules/no-magic-numbers.md)                                 | Disallows magic numbers.                                                                                                                            | :heavy_check_mark: |          |
 | [`@typescript-eslint/no-misused-new`](./docs/rules/no-misused-new.md)                                     | Enforce valid definition of `new` and `constructor`. (`no-misused-new` from TSLint)                                                                 | :heavy_check_mark: |          |                   |
 | [`@typescript-eslint/no-namespace`](./docs/rules/no-namespace.md)                                         | Disallow the use of custom TypeScript modules and namespaces (`no-namespace` from TSLint)                                                           | :heavy_check_mark: |          |                   |
 | [`@typescript-eslint/no-non-null-assertion`](./docs/rules/no-non-null-assertion.md)                       | Disallows non-null assertions using the `!` postfix operator (`no-non-null-assertion` from TSLint)                                                  | :heavy_check_mark: |          |                   |

packages/eslint-plugin/docs/rules/explicit-function-return-type.md

@@ -61,12 +61,19 @@ class Test {
 
 The rule accepts an options object with the following properties:
 
-- `allowExpressions` if true, only functions which are part of a declaration will be checked
-- `allowTypedFunctionExpressions` if true, type annotations are also allowed on the variable
-  of a function expression rather than on the function directly.
+```ts
+type Options = {
+  // if true, only functions which are part of a declaration will be checked
+  allowExpressions?: boolean;
+  // if true, type annotations are also allowed on the variable of a function expression rather than on the function directly.
+  allowTypedFunctionExpressions?: boolean;
+};
 
-By default, `allowExpressions: false` and `allowTypedFunctionExpressions: false` are used,
-meaning all declarations and expressions _must_ have a return type.
+const defaults = {
+  allowExpressions: false,
+  allowTypedFunctionExpressions: false,
+};
+```
 
 ### allowExpressions
 
@@ -88,6 +95,20 @@ const foo = arr.map(i => i * i);
 
 ### allowTypedFunctionExpressions
 
+Examples of **incorrect** code for this rule with `{ allowTypedFunctionExpressions: true }`:
+
+```ts
+let arrowFn = () => 'test';
+
+let funcExpr = function() {
+  return 'test';
+};
+
+let objectProp = {
+  foo: () => 1,
+};
+```
+
 Examples of additional **correct** code for this rule with `{ allowTypedFunctionExpressions: true }`:
 
 ```ts
@@ -100,21 +121,20 @@ let funcExpr: FuncType = function() {
 };
 
 let asTyped = (() => '') as () => string;
+let caasTyped = <() => string>(() => '');
 
 interface ObjectType {
   foo(): number;
 }
 let objectProp: ObjectType = {
   foo: () => 1,
 };
-
-interface ObjectType {
-  foo(): number;
-}
-
-let asObjectProp = {
+let objectPropAs = {
   foo: () => 1,
 } as ObjectType;
+let objectPropCast = <ObjectType>{
+  foo: () => 1,
+};
 ```
 
 ## When Not To Use It

packages/eslint-plugin/docs/rules/no-magic-numbers.md

@@ -0,0 +1,44 @@
+# Disallow Magic Numbers (@typescript-eslint/no-magic-numbers)
+
+'Magic numbers' are numbers that occur multiple times in code without an explicit meaning.
+They should preferably be replaced by named constants.
+
+## Rule Details
+
+The `@typescript-eslint/no-magic-numbers` rule extends the `no-magic-numbers` rule from ESLint core, and adds support for handling Typescript specific code that would otherwise trigger the rule.
+
+See the [ESLint documentation](https://eslint.org/docs/rules/no-magic-numbers) for more details on the `no-magic-numbers` rule.
+
+## Rule Changes
+
+```cjson
+{
+    // note you must disable the base rule as it can report incorrect errors
+    "no-magic-numbers": "off",
+    "@typescript-eslint/no-magic-numbers": ["error", { "ignoreNumericLiteralTypes": true }]
+}
+```
+
+In addition to the options supported by the `no-magic-numbers` rule in ESLint core, the rule adds the following options:
+
+### ignoreNumericLiteralTypes
+
+A boolean to specify if numbers used in Typescript numeric literal types are considered okay. `false` by default.
+
+Examples of **incorrect** code for the `{ "ignoreNumericLiteralTypes": false }` option:
+
+```ts
+/*eslint @typescript-eslint/no-magic-numbers: ["error", { "ignoreNumericLiteralTypes": false }]*/
+
+type SmallPrimes = 2 | 3 | 5 | 7 | 11;
+```
+
+Examples of **correct** code for the `{ "ignoreNumericLiteralTypes": true }` option:
+
+```ts
+/*eslint @typescript-eslint/no-magic-numbers: ["error", { "ignoreNumericLiteralTypes": true }]*/
+
+type SmallPrimes = 2 | 3 | 5 | 7 | 11;
+```
+
+<sup>Taken with ❤️ [from ESLint core](https://github.com/eslint/eslint/blob/master/docs/rules/no-magic-numbers.md)</sup>

packages/eslint-plugin/package.json

@@ -25,13 +25,14 @@
   "license": "MIT",
   "main": "dist/index.js",
   "scripts": {
+    "build": "tsc -p tsconfig.build.json",
+    "clean": "rimraf dist/",
     "docs": "eslint-docs",
     "docs:check": "eslint-docs check",
-    "test": "jest --coverage",
-    "recommended:update": "ts-node tools/update-recommended.ts",
+    "format": "prettier --write \"./**/*.{ts,js,json,md}\" --ignore-path ../../.prettierignore",
     "prebuild": "npm run clean",
-    "build": "tsc -p tsconfig.build.json",
-    "clean": "rimraf dist/",
+    "recommended:update": "ts-node tools/update-recommended.ts",
+    "test": "jest --coverage",
     "typecheck": "tsc --noEmit"
   },
   "dependencies": {

packages/eslint-plugin/src/rules/explicit-function-return-type.ts

@@ -5,6 +5,7 @@ type Options = [
   {
     allowExpressions?: boolean;
     allowTypedFunctionExpressions?: boolean;
+    allowUntypedSetters?: boolean;
   }
 ];
 type MessageIds = 'missingReturnType';
@@ -41,15 +42,17 @@ export default util.createRule<Options, MessageIds>({
     {
       allowExpressions: false,
       allowTypedFunctionExpressions: false,
+      allowUntypedSetters: true,
     },
   ],
   create(context, [options]) {
     /**
      * Checks if a node is a constructor.
      * @param node The node to check
      */
-    function isConstructor(node: TSESTree.Node): boolean {
+    function isConstructor(node: TSESTree.Node | undefined): boolean {
       return (
+        !!node &&
         node.type === AST_NODE_TYPES.MethodDefinition &&
         node.kind === 'constructor'
       );
@@ -58,14 +61,17 @@ export default util.createRule<Options, MessageIds>({
     /**
      * Checks if a node is a setter.
      */
-    function isSetter(node: TSESTree.Node): boolean {
+    function isSetter(node: TSESTree.Node | undefined): boolean {
       return (
-        node.type === AST_NODE_TYPES.MethodDefinition && node.kind === 'set'
+        !!node &&
+        node.type === AST_NODE_TYPES.MethodDefinition &&
+        node.kind === 'set'
       );
     }
 
     /**
      * Checks if a node is a variable declarator with a type annotation.
+     * `const x: Foo = ...`
      */
     function isVariableDeclaratorWithTypeAnnotation(
       node: TSESTree.Node,
@@ -76,41 +82,62 @@ export default util.createRule<Options, MessageIds>({
       );
     }
 
+    /**
+     * Checks if a node is a class property with a type annotation.
+     * `public x: Foo = ...`
+     */
+    function isClassPropertyWithTypeAnnotation(node: TSESTree.Node): boolean {
+      return (
+        node.type === AST_NODE_TYPES.ClassProperty && !!node.typeAnnotation
+      );
+    }
+
+    /**
+     * Checks if a node is a type cast
+     * `(() => {}) as Foo`
+     * `<Foo>(() => {})`
+     */
+    function isTypeCast(node: TSESTree.Node): boolean {
+      return (
+        node.type === AST_NODE_TYPES.TSAsExpression ||
+        node.type === AST_NODE_TYPES.TSTypeAssertion
+      );
+    }
+
     /**
      * Checks if a node belongs to:
-     * const x: Foo = { prop: () => {} }
+     * `const x: Foo = { prop: () => {} }`
+     * `const x = { prop: () => {} } as Foo`
+     * `const x = <Foo>{ prop: () => {} }`
      */
-    function isPropertyOfObjectVariableDeclaratorWithTypeAnnotation(
-      node: TSESTree.Node,
+    function isPropertyOfObjectWithType(
+      parent: TSESTree.Node | undefined,
     ): boolean {
-      let parent = node.parent;
       if (!parent || parent.type !== AST_NODE_TYPES.Property) {
         return false;
       }
-      parent = parent.parent;
-      if (!parent || parent.type !== AST_NODE_TYPES.ObjectExpression) {
+      parent = parent.parent; // this shouldn't happen, checking just in case
+      /* istanbul ignore if */ if (
+        !parent ||
+        parent.type !== AST_NODE_TYPES.ObjectExpression
+      ) {
         return false;
       }
-      parent = parent.parent;
-      return !!parent && isVariableDeclaratorWithTypeAnnotation(parent);
-    }
 
-    function isPropertyOfObjectInAsExpression(node: TSESTree.Node): boolean {
-      let parent = node.parent;
-      if (!parent || parent.type !== AST_NODE_TYPES.Property) {
+      parent = parent.parent; // this shouldn't happen, checking just in case
+      /* istanbul ignore if */ if (!parent) {
         return false;
       }
-      parent = parent.parent;
-      if (!parent || parent.type !== AST_NODE_TYPES.ObjectExpression) {
-        return false;
-      }
-      parent = parent.parent;
-      return !!parent && parent.type === AST_NODE_TYPES.TSAsExpression;
+
+      return (
+        isTypeCast(parent) ||
+        isClassPropertyWithTypeAnnotation(parent) ||
+        isVariableDeclaratorWithTypeAnnotation(parent)
+      );
     }
 
     /**
      * Checks if a function declaration/expression has a return type.
-     * @param node The node representing a function.
      */
     function checkFunctionReturnType(
       node:
@@ -119,22 +146,14 @@ export default util.createRule<Options, MessageIds>({
         | TSESTree.FunctionExpression,
     ): void {
       if (
-        options.allowExpressions &&
-        node.type !== AST_NODE_TYPES.FunctionDeclaration &&
-        node.parent &&
-        node.parent.type !== AST_NODE_TYPES.VariableDeclarator &&
-        node.parent.type !== AST_NODE_TYPES.MethodDefinition
+        node.returnType ||
+        isConstructor(node.parent) ||
+        isSetter(node.parent)
       ) {
         return;
       }
 
-      if (
-        !node.returnType &&
-        node.parent &&
-        !isConstructor(node.parent) &&
-        !isSetter(node.parent) &&
-        util.isTypeScriptFile(context.getFilename())
-      ) {
+      if (util.isTypeScriptFile(context.getFilename())) {
         context.report({
           node,
           messageId: 'missingReturnType',
@@ -144,20 +163,29 @@ export default util.createRule<Options, MessageIds>({
 
     /**
      * Checks if a function declaration/expression has a return type.
-     * @param {ASTNode} node The node representing a function.
      */
     function checkFunctionExpressionReturnType(
       node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression,
     ): void {
-      if (
-        options.allowTypedFunctionExpressions &&
-        node.parent &&
-        (isVariableDeclaratorWithTypeAnnotation(node.parent) ||
-          isPropertyOfObjectVariableDeclaratorWithTypeAnnotation(node) ||
-          node.parent.type === AST_NODE_TYPES.TSAsExpression ||
-          isPropertyOfObjectInAsExpression(node))
-      ) {
-        return;
+      if (node.parent) {
+        if (options.allowTypedFunctionExpressions) {
+          if (
+            isTypeCast(node.parent) ||
+            isVariableDeclaratorWithTypeAnnotation(node.parent) ||
+            isClassPropertyWithTypeAnnotation(node.parent) ||
+            isPropertyOfObjectWithType(node.parent)
+          ) {
+            return;
+          }
+        }
+
+        if (
+          options.allowExpressions &&
+          node.parent.type !== AST_NODE_TYPES.VariableDeclarator &&
+          node.parent.type !== AST_NODE_TYPES.MethodDefinition
+        ) {
+          return;
+        }
       }
 
       checkFunctionReturnType(node);