From b99e83167ebc9ca97ce9c932a007e5066b4f5616 Mon Sep 17 00:00:00 2001 From: James Henry Date: Thu, 12 Sep 2019 10:16:26 +0200 Subject: [PATCH 01/10] docs: financial contributors and TSLint migration updates (#970) --- README.md | 50 ++++++++++++++++++++++++++++++++++++++++++++++++-- package.json | 7 ++++++- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 216f89cdc37e..b57368515235 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,7 @@

Azure Pipelines + GitHub license NPM Downloads Codecov @@ -60,6 +61,14 @@ Palantir, the backers behind TSLint announced earlier this year that **they woul The TypeScript Team themselves also announced their plans to move the TypeScript codebase from TSLint to `typescript-eslint`, and they have been big supporters of this project. More details at https://github.com/microsoft/TypeScript/issues/30553 +### Migrating from TSLint to ESLint + +If you are looking for help in migrating from TSLint to ESLint, you can check out this project: https://github.com/typescript-eslint/tslint-to-eslint-config + +You can look at [`the plugin ROADMAP.md`](./packages/eslint-plugin/ROADMAP.md) for an up to date overview of how TSLint rules compare to the ones in this package. + +There is also the ultimate fallback option of using both linters together for a while during your transition if you absolutely have to by using TSLint _within_ ESLint. For this option, check out [`@typescript-eslint/eslint-plugin-tslint`](./packages/eslint-plugin-tslint/). +
## How does `typescript-eslint` work and why do you have multiple packages? @@ -161,6 +170,8 @@ See an issue? Report it in as much detail as possible, ideally with a clear and If you have the time and the inclination, you can even take it a step further and submit a PR to improve the project. +There are also financial ways to contribute, please see [Financial Contributors](#Financial-Contributors) for more details. + All positive contributions are welcome here!
@@ -223,9 +234,44 @@ TypeScript ESLint inherits from the the original TypeScript ESLint Parser licens
-## Contributors +## Code Contributors + +This project exists thanks to every one of the awesome people who contribute code and documentation: + +
+ + + +
+ +🙏 An extra special thanks goes out to the wonderful people listed in [`CONTRIBUTORS.md`](./CONTRIBUTORS.md) + +
+ +## Financial Contributors + +In addition to submitting code and documentation updates, you can help us sustain our community by becoming a financial contributor [[Click here to contribute - every little helps!](https://opencollective.com/typescript-eslint/contribute)] + +
+ +### Individuals + + + +### Organizations + +Support this project with your organization. Your logo will show up here with a link to your website. [[Click here to contribute - every little helps!](https://opencollective.com/typescript-eslint/contribute)] -Thanks goes to the wonderful people listed in [`CONTRIBUTORS.md`](./CONTRIBUTORS.md). + + + + + + + + + +
diff --git a/package.json b/package.json index 2cd6dc2b9e34..2dce8eb93355 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ "lint-fix": "eslint . --ext .js,.ts --fix", "pre-commit": "yarn lint-staged", "pre-push": "yarn format-check", - "postinstall": "lerna bootstrap && yarn build && lerna link && npm run check-clean-workspace-after-install", + "postinstall": "lerna bootstrap && yarn build && lerna link && npm run check-clean-workspace-after-install && opencollective-postinstall", "check-clean-workspace-after-install": "git diff --quiet --exit-code", "test": "lerna run test --parallel", "typecheck": "lerna run typecheck" @@ -68,11 +68,16 @@ "jest": "^24.9.0", "lerna": "^3.16.4", "lint-staged": "^9.2.5", + "opencollective-postinstall": "^2.0.2", "prettier": "^1.18.2", "rimraf": "^3.0.0", "ts-jest": "^24.0.0", "ts-node": "^8.3.0", "tslint": "^5.19.0", "typescript": ">=3.2.1 <3.7.0" + }, + "collective": { + "type": "opencollective", + "url": "https://opencollective.com/typescript-eslint" } } From b49dbfd6b47b3b5f31ce1ba3db33d0d93ce5d6b3 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Thu, 12 Sep 2019 01:30:33 -0700 Subject: [PATCH 02/10] docs(typescript-estree): document that duplicate filenames are unsupported (#957) --- .vscode/launch.json | 14 +++++++ packages/parser/README.md | 4 +- .../src/WatchCompilerHostOfConfigFile.ts | 37 +++++++++++++++++++ .../typescript-estree/src/tsconfig-parser.ts | 35 ++++++++---------- 4 files changed, 69 insertions(+), 21 deletions(-) create mode 100644 packages/typescript-estree/src/WatchCompilerHostOfConfigFile.ts diff --git a/.vscode/launch.json b/.vscode/launch.json index 35c5c3194ec3..1c2b4b3dd803 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -18,6 +18,20 @@ "sourceMaps": true, "console": "integratedTerminal", "internalConsoleOptions": "neverOpen" + }, + { + "type": "node", + "request": "launch", + "name": "Run currently opened typescript-estree test", + "cwd": "${workspaceFolder}/packages/typescript-estree/", + "program": "${workspaceFolder}/node_modules/jest/bin/jest.js", + "args": [ + "--runInBand", + "${relativeFile}" + ], + "sourceMaps": true, + "console": "integratedTerminal", + "internalConsoleOptions": "neverOpen" } ] } diff --git a/packages/parser/README.md b/packages/parser/README.md index 042fb4f5f540..9fcb9d043633 100644 --- a/packages/parser/README.md +++ b/packages/parser/README.md @@ -66,7 +66,9 @@ The following additional configuration options are available by specifying them ]; ``` - - Note that if you use project references, TypeScript will not automatically use project references to resolve files. This means that you will have to add each referenced tsconfig to the `project` field either separately, or via a glob. + - If you use project references, TypeScript will not automatically use project references to resolve files. This means that you will have to add each referenced tsconfig to the `project` field either separately, or via a glob. + + - TypeScript will ignore files with duplicate filenames in the same folder (for example, `src/file.ts` and `src/file.js`). TypeScript purposely ignore all but one of the files, only keeping the one file with the highest priority extension (the extension priority order (from highest to lowest) is `.ts`, `.tsx`, `.d.ts`, `.js`, `.jsx`). For more info see #955. - Note that if this setting is specified and `createDefaultProgram` is not, you must only lint files that are included in the projects as defined by the provided `tsconfig.json` files. If your existing configuration does not include all of the files you would like to lint, you can create a separate `tsconfig.eslint.json` as follows: diff --git a/packages/typescript-estree/src/WatchCompilerHostOfConfigFile.ts b/packages/typescript-estree/src/WatchCompilerHostOfConfigFile.ts new file mode 100644 index 000000000000..7fb4663b985c --- /dev/null +++ b/packages/typescript-estree/src/WatchCompilerHostOfConfigFile.ts @@ -0,0 +1,37 @@ +// These types are internal to TS. +// They have been trimmed down to only include the relevant bits +// We use some special internal TS apis to help us do our parsing flexibly + +import * as ts from 'typescript'; // leave this as * as ts so people using util package don't need syntheticDefaultImports + +// https://github.com/microsoft/TypeScript/blob/b84e65db4ea5c39dbaa2ccd6594efe4653318251/src/compiler/watchUtilities.ts#L6-L18 +interface DirectoryStructureHost { + readDirectory?( + path: string, + extensions?: ReadonlyArray, + exclude?: ReadonlyArray, + include?: ReadonlyArray, + depth?: number, + ): string[]; +} + +// https://github.com/microsoft/TypeScript/blob/b84e65db4ea5c39dbaa2ccd6594efe4653318251/src/compiler/watchUtilities.ts#L25-L35 +interface CachedDirectoryStructureHost extends DirectoryStructureHost { + readDirectory( + path: string, + extensions?: ReadonlyArray, + exclude?: ReadonlyArray, + include?: ReadonlyArray, + depth?: number, + ): string[]; +} + +// https://github.com/microsoft/TypeScript/blob/5d36aab06f12b0a3ba6197eb14e98204ec0195fb/src/compiler/watch.ts#L548-L554 +interface WatchCompilerHostOfConfigFile + extends ts.WatchCompilerHostOfConfigFile { + onCachedDirectoryStructureHostCreate( + host: CachedDirectoryStructureHost, + ): void; +} + +export { WatchCompilerHostOfConfigFile }; diff --git a/packages/typescript-estree/src/tsconfig-parser.ts b/packages/typescript-estree/src/tsconfig-parser.ts index 364dd0543c8e..e6ef356b6f30 100644 --- a/packages/typescript-estree/src/tsconfig-parser.ts +++ b/packages/typescript-estree/src/tsconfig-parser.ts @@ -1,6 +1,7 @@ import path from 'path'; import * as ts from 'typescript'; // leave this as * as ts so people using util package don't need syntheticDefaultImports import { Extra } from './parser-options'; +import { WatchCompilerHostOfConfigFile } from './WatchCompilerHostOfConfigFile'; //------------------------------------------------------------------------------ // Environment calculation @@ -13,6 +14,8 @@ export const defaultCompilerOptions: ts.CompilerOptions = { allowNonTsExtensions: true, allowJs: true, checkJs: true, + noEmit: true, + // extendedDiagnostics: true, }; /** @@ -115,7 +118,7 @@ export function calculateProjectParserOptions( ts.createSemanticDiagnosticsBuilderProgram, diagnosticReporter, /*reportWatchStatus*/ () => {}, - ); + ) as WatchCompilerHostOfConfigFile; // ensure readFile reads the code being linted instead of the copy on disk const oldReadFile = watchCompilerHost.readFile; @@ -144,8 +147,7 @@ export function calculateProjectParserOptions( }; // register callbacks to trigger program updates without using fileWatchers - // eslint-disable-next-line @typescript-eslint/explicit-function-return-type - watchCompilerHost.watchFile = (fileName, callback) => { + watchCompilerHost.watchFile = (fileName, callback): ts.FileWatcher => { const normalizedFileName = path.normalize(fileName); watchCallbackTrackingMap.set(normalizedFileName, callback); return { @@ -156,26 +158,20 @@ export function calculateProjectParserOptions( }; // ensure fileWatchers aren't created for directories - watchCompilerHost.watchDirectory = (): typeof noopFileWatcher => - noopFileWatcher; + watchCompilerHost.watchDirectory = (): ts.FileWatcher => noopFileWatcher; - // we're using internal typescript APIs which aren't on the types - /* eslint-disable @typescript-eslint/no-explicit-any */ // allow files with custom extensions to be included in program (uses internal ts api) - const oldOnDirectoryStructureHostCreate = (watchCompilerHost as any) - .onCachedDirectoryStructureHostCreate; - (watchCompilerHost as any).onCachedDirectoryStructureHostCreate = ( - host: any, - ): void => { + const oldOnDirectoryStructureHostCreate = + watchCompilerHost.onCachedDirectoryStructureHostCreate; + watchCompilerHost.onCachedDirectoryStructureHostCreate = (host): void => { const oldReadDirectory = host.readDirectory; - // eslint-disable-next-line @typescript-eslint/explicit-function-return-type host.readDirectory = ( - path: string, - extensions?: readonly string[], - exclude?: readonly string[], - include?: readonly string[], - depth?: number, - ) => + path, + extensions, + exclude, + include, + depth, + ): string[] => oldReadDirectory( path, !extensions @@ -187,7 +183,6 @@ export function calculateProjectParserOptions( ); oldOnDirectoryStructureHostCreate(host); }; - /* eslint-enable @typescript-eslint/no-explicit-any */ // create program const programWatch = ts.createWatchProgram(watchCompilerHost); From 571548243a7ce2cc3c42134ae18c03c065b2d560 Mon Sep 17 00:00:00 2001 From: Retsam Date: Thu, 12 Sep 2019 04:42:09 -0400 Subject: [PATCH 03/10] feat(eslint-plugin): add no-unnecessary-condition rule (#699) --- packages/eslint-plugin/README.md | 1 + .../docs/rules/no-unnecessary-condition.md | 64 ++++++ .../docs/rules/strict-boolean-expressions.md | 2 + packages/eslint-plugin/src/configs/all.json | 1 + packages/eslint-plugin/src/rules/index.ts | 2 + .../src/rules/no-unnecessary-condition.ts | 192 +++++++++++++++++ .../rules/no-unnecessary-condition.test.ts | 195 ++++++++++++++++++ 7 files changed, 457 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/no-unnecessary-condition.md create mode 100644 packages/eslint-plugin/src/rules/no-unnecessary-condition.ts create mode 100644 packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 178ca903a9fc..baec8f2426e6 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -177,6 +177,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/no-require-imports`](./docs/rules/no-require-imports.md) | Disallows invocation of `require()` | | | | | [`@typescript-eslint/no-this-alias`](./docs/rules/no-this-alias.md) | Disallow aliasing `this` | :heavy_check_mark: | | | | [`@typescript-eslint/no-type-alias`](./docs/rules/no-type-alias.md) | Disallow the use of type aliases | | | | +| [`@typescript-eslint/no-unnecessary-condition`](./docs/rules/no-unnecessary-condition.md) | Prevents conditionals where the type is always truthy or always falsy | | | :thought_balloon: | | [`@typescript-eslint/no-unnecessary-qualifier`](./docs/rules/no-unnecessary-qualifier.md) | Warns when a namespace qualifier is unnecessary | | :wrench: | :thought_balloon: | | [`@typescript-eslint/no-unnecessary-type-arguments`](./docs/rules/no-unnecessary-type-arguments.md) | Warns if an explicitly specified type argument is the default for that type parameter | | :wrench: | :thought_balloon: | | [`@typescript-eslint/no-unnecessary-type-assertion`](./docs/rules/no-unnecessary-type-assertion.md) | Warns if a type assertion does not change the type of an expression | :heavy_check_mark: | :wrench: | :thought_balloon: | diff --git a/packages/eslint-plugin/docs/rules/no-unnecessary-condition.md b/packages/eslint-plugin/docs/rules/no-unnecessary-condition.md new file mode 100644 index 000000000000..ac54dba64ca3 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-unnecessary-condition.md @@ -0,0 +1,64 @@ +# Condition expressions must be necessary + +Any expression being used as a condition must be able to evaluate as truthy or falsy in order to be considered "necessary". Conversely, any expression that always evaluates to truthy or always evaluates to falsy, as determined by the type of the expression, is considered unnecessary and will be flagged by this rule. + +The following expressions are checked: + +- Arguments to the `&&`, `||` and `?:` (ternary) operators +- Conditions for `if`, `for`, `while`, and `do-while` statements. + +Examples of **incorrect** code for this rule: + +```ts +function head(items: T[]) { + // items can never be nullable, so this is unnecessary + if (items) { + return items[0].toUpperCase(); + } +} + +function foo(arg: 'bar' | 'baz') { + // arg is never nullable or empty string, so this is unnecessary + if (arg) { + } +} +``` + +Examples of **correct** code for this rule: + +```ts +function head(items: T[]) { + // Necessary, since items.length might be 0 + if (items.length) { + return items[0].toUpperCase(); + } +} + +function foo(arg: string) { + // Necessary, since foo might be ''. + if (arg) { + } +} +``` + +## Options + +Accepts an object with the following options: + +- `ignoreRhs` (default `false`) - doesn't check if the right-hand side of `&&` and `||` is a necessary condition. For example, the following code is valid with this option on: + +```ts +function head(items: T[]) { + return items.length && items[0].toUpperCase(); +} +``` + +## When Not To Use It + +The main downside to using this rule is the need for type information. + +## Related To + +- ESLint: [no-constant-condition](https://eslint.org/docs/rules/no-constant-condition) - this rule is essentially a stronger versison + +- [strict-boolean-expression](./strict-boolean-expressions.md) - a stricter alternative to this rule. diff --git a/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md b/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md index de27437a47c2..6bed5b03fe3a 100644 --- a/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md +++ b/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md @@ -61,3 +61,5 @@ Options may be provided as an object with: ## Related To - TSLint: [strict-boolean-expressions](https://palantir.github.io/tslint/rules/strict-boolean-expressions) + +- [no-unnecessary-condition](./no-unnecessary-condition.md) - a looser alternative to this rule. diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index 0974b7a72638..377f4b58f0e4 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -46,6 +46,7 @@ "@typescript-eslint/no-require-imports": "error", "@typescript-eslint/no-this-alias": "error", "@typescript-eslint/no-type-alias": "error", + "@typescript-eslint/no-unnecessary-condition": "error", "@typescript-eslint/no-unnecessary-qualifier": "error", "@typescript-eslint/no-unnecessary-type-arguments": "error", "@typescript-eslint/no-unnecessary-type-assertion": "error", diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 8ae9d698ccab..5302abd05de0 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -35,6 +35,7 @@ import noParameterProperties from './no-parameter-properties'; import noRequireImports from './no-require-imports'; import noThisAlias from './no-this-alias'; import noTypeAlias from './no-type-alias'; +import noUnnecessaryCondition from './no-unnecessary-condition'; import noUnnecessaryQualifier from './no-unnecessary-qualifier'; import noUnnecessaryTypeAssertion from './no-unnecessary-type-assertion'; import noUnusedVars from './no-unused-vars'; @@ -100,6 +101,7 @@ export default { 'no-require-imports': noRequireImports, 'no-this-alias': noThisAlias, 'no-type-alias': noTypeAlias, + 'no-unnecessary-condition': noUnnecessaryCondition, 'no-unnecessary-qualifier': noUnnecessaryQualifier, 'no-unnecessary-type-arguments': useDefaultTypeParameter, 'no-unnecessary-type-assertion': noUnnecessaryTypeAssertion, diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts new file mode 100644 index 000000000000..49c8f13401c8 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -0,0 +1,192 @@ +import { + TSESTree, + AST_NODE_TYPES, +} from '@typescript-eslint/experimental-utils'; +import ts, { TypeFlags } from 'typescript'; +import { + isTypeFlagSet, + unionTypeParts, + isFalsyType, + isBooleanLiteralType, + isLiteralType, +} from 'tsutils'; +import { + createRule, + getParserServices, + getConstrainedTypeAtLocation, +} from '../util'; + +// Truthiness utilities +// #region +const isTruthyLiteral = (type: ts.Type): boolean => + isBooleanLiteralType(type, true) || (isLiteralType(type) && !!type.value); + +const isPossiblyFalsy = (type: ts.Type): boolean => + unionTypeParts(type) + // PossiblyFalsy flag includes literal values, so exclude ones that + // are definitely truthy + .filter(t => !isTruthyLiteral(t)) + .some(type => isTypeFlagSet(type, ts.TypeFlags.PossiblyFalsy)); + +const isPossiblyTruthy = (type: ts.Type): boolean => + unionTypeParts(type).some(type => !isFalsyType(type)); + +// isLiteralType only covers numbers and strings, this is a more exhaustive check. +const isLiteral = (type: ts.Type): boolean => + isBooleanLiteralType(type, true) || + isBooleanLiteralType(type, false) || + type.flags === ts.TypeFlags.Undefined || + type.flags === ts.TypeFlags.Null || + type.flags === ts.TypeFlags.Void || + isLiteralType(type); +// #endregion + +type ExpressionWithTest = + | TSESTree.ConditionalExpression + | TSESTree.DoWhileStatement + | TSESTree.ForStatement + | TSESTree.IfStatement + | TSESTree.WhileStatement; + +export type Options = [ + { + ignoreRhs?: boolean; + }, +]; + +export type MessageId = + | 'alwaysTruthy' + | 'alwaysFalsy' + | 'literalBooleanExpression' + | 'never'; +export default createRule({ + name: 'no-unnecessary-conditionals', + meta: { + type: 'suggestion', + docs: { + description: + 'Prevents conditionals where the type is always truthy or always falsy', + category: 'Best Practices', + recommended: false, + requiresTypeChecking: true, + }, + schema: [ + { + type: 'object', + properties: { + ignoreRhs: { + type: 'boolean', + }, + }, + additionalProperties: false, + }, + ], + messages: { + alwaysTruthy: 'Unnecessary conditional, value is always truthy.', + alwaysFalsy: 'Unnecessary conditional, value is always falsy.', + literalBooleanExpression: + 'Unnecessary conditional, both sides of the expression are literal values', + never: 'Unnecessary conditional, value is `never`', + }, + }, + defaultOptions: [ + { + ignoreRhs: false, + }, + ], + create(context, [{ ignoreRhs }]) { + const service = getParserServices(context); + const checker = service.program.getTypeChecker(); + + function getNodeType(node: TSESTree.Node): ts.Type { + const tsNode = service.esTreeNodeToTSNodeMap.get(node); + return getConstrainedTypeAtLocation(checker, tsNode); + } + + /** + * Checks if a conditional node is necessary: + * if the type of the node is always true or always false, it's not necessary. + */ + function checkNode(node: TSESTree.Node): void { + const type = getNodeType(node); + + // Conditional is always necessary if it involves `any` or `unknown` + if (isTypeFlagSet(type, TypeFlags.Any | TypeFlags.Unknown)) { + return; + } + if (isTypeFlagSet(type, TypeFlags.Never)) { + context.report({ node, messageId: 'never' }); + } else if (!isPossiblyTruthy(type)) { + context.report({ node, messageId: 'alwaysFalsy' }); + } else if (!isPossiblyFalsy(type)) { + context.report({ node, messageId: 'alwaysTruthy' }); + } + } + + /** + * Checks that a binary expression is necessarily conditional, reports otherwise. + * If both sides of the binary expression are literal values, it's not a necessary condition. + * + * NOTE: It's also unnecessary if the types that don't overlap at all + * but that case is handled by the Typescript compiler itself. + */ + const BOOL_OPERATORS = new Set([ + '<', + '>', + '<=', + '>=', + '==', + '===', + '!=', + '!==', + ]); + function checkIfBinaryExpressionIsNecessaryConditional( + node: TSESTree.BinaryExpression, + ): void { + if ( + BOOL_OPERATORS.has(node.operator) && + isLiteral(getNodeType(node.left)) && + isLiteral(getNodeType(node.right)) + ) { + context.report({ node, messageId: 'literalBooleanExpression' }); + } + } + + /** + * Checks that a testable expression is necessarily conditional, reports otherwise. + * Filters all LogicalExpressions to prevent some duplicate reports. + */ + function checkIfTestExpressionIsNecessaryConditional( + node: ExpressionWithTest, + ): void { + if ( + node.test !== null && + node.test.type !== AST_NODE_TYPES.LogicalExpression + ) { + checkNode(node.test); + } + } + + /** + * Checks that a logical expression contains a boolean, reports otherwise. + */ + function checkLogicalExpressionForUnnecessaryConditionals( + node: TSESTree.LogicalExpression, + ): void { + checkNode(node.left); + if (!ignoreRhs) { + checkNode(node.right); + } + } + + return { + BinaryExpression: checkIfBinaryExpressionIsNecessaryConditional, + ConditionalExpression: checkIfTestExpressionIsNecessaryConditional, + DoWhileStatement: checkIfTestExpressionIsNecessaryConditional, + ForStatement: checkIfTestExpressionIsNecessaryConditional, + IfStatement: checkIfTestExpressionIsNecessaryConditional, + WhileStatement: checkIfTestExpressionIsNecessaryConditional, + LogicalExpression: checkLogicalExpressionForUnnecessaryConditionals, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts new file mode 100644 index 000000000000..f298495a26f8 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts @@ -0,0 +1,195 @@ +import path from 'path'; +import rule, { + Options, + MessageId, +} from '../../src/rules/no-unnecessary-condition'; +import { RuleTester } from '../RuleTester'; +import { + TestCaseError, + InvalidTestCase, +} from '@typescript-eslint/experimental-utils/dist/ts-eslint'; + +const rootPath = path.join(process.cwd(), 'tests/fixtures/'); + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + tsconfigRootDir: rootPath, + project: './tsconfig.json', + }, +}); + +const ruleError = ( + line: number, + column: number, + messageId: MessageId, +): TestCaseError => ({ + messageId, + line, + column, +}); + +const necessaryConditionTest = (condition: string): string => ` +declare const b1: ${condition}; +declare const b2: boolean; +const t1 = b1 && b2; +`; + +const unnecessaryConditionTest = ( + condition: string, + messageId: MessageId, +): InvalidTestCase => ({ + code: necessaryConditionTest(condition), + errors: [ruleError(4, 12, messageId)], +}); + +ruleTester.run('no-unnecessary-conditionals', rule, { + valid: [ + ` +declare const b1: boolean; +declare const b2: boolean; +const t1 = b1 && b2; +const t2 = b1 || b2; +if(b1 && b2) {} +while(b1 && b2) {} +for (let i = 0; (b1 && b2); i++) { break; } +const t1 = (b1 && b2) ? 'yes' : 'no'`, + necessaryConditionTest('false | 5'), // Truthy literal and falsy literal + necessaryConditionTest('boolean | "foo"'), // boolean and truthy literal + necessaryConditionTest('0 | boolean'), // boolean and falsy literal + necessaryConditionTest('boolean | object'), // boolean and always-truthy type + necessaryConditionTest('false | object'), // always truthy type and falsy literal + // always falsy type and always truthy type + necessaryConditionTest('null | object'), + necessaryConditionTest('undefined | true'), + necessaryConditionTest('void | true'), + + necessaryConditionTest('any'), // any + necessaryConditionTest('unknown'), // unknown + + // Generic type params + ` +function test(t: T) { + return t ? 'yes' : 'no' +}`, + + // Boolean expressions + ` +function test(a: string) { + return a === "a" +}`, + + // Supports ignoring the RHS + { + code: ` +declare const b1: boolean; +declare const b2: true; +if(b1 && b2) {}`, + options: [{ ignoreRhs: true }], + }, + ], + invalid: [ + // Ensure that it's checking in all the right places + { + code: ` +const b1 = true; +declare const b2: boolean; +const t1 = b1 && b2; +const t2 = b1 || b2; +if(b1 && b2) {} +while(b1 && b2) {} +for (let i = 0; (b1 && b2); i++) { break; } +const t1 = (b1 && b2) ? 'yes' : 'no'`, + errors: [ + ruleError(4, 12, 'alwaysTruthy'), + ruleError(5, 12, 'alwaysTruthy'), + ruleError(6, 4, 'alwaysTruthy'), + ruleError(7, 7, 'alwaysTruthy'), + ruleError(8, 18, 'alwaysTruthy'), + ruleError(9, 13, 'alwaysTruthy'), + ], + }, + // Ensure that it's complaining about the right things + unnecessaryConditionTest('object', 'alwaysTruthy'), + unnecessaryConditionTest('object | true', 'alwaysTruthy'), + unnecessaryConditionTest('"" | false', 'alwaysFalsy'), // Two falsy literals + unnecessaryConditionTest('"always truthy"', 'alwaysTruthy'), + unnecessaryConditionTest(`undefined`, 'alwaysFalsy'), + unnecessaryConditionTest('null', 'alwaysFalsy'), + unnecessaryConditionTest('void', 'alwaysFalsy'), + unnecessaryConditionTest('never', 'never'), + + // Generic type params + { + code: ` +function test(t: T) { + return t ? 'yes' : 'no' +}`, + errors: [ruleError(3, 10, 'alwaysTruthy')], + }, + { + code: ` +function test(t: T) { + return t ? 'yes' : 'no' +}`, + errors: [ruleError(3, 10, 'alwaysFalsy')], + }, + { + code: ` +function test(t: T) { + return t ? 'yes' : 'no' +}`, + errors: [ruleError(3, 10, 'alwaysTruthy')], + }, + + // Boolean expressions + { + code: ` +function test(a: "a") { + return a === "a" +}`, + errors: [ruleError(3, 10, 'literalBooleanExpression')], + }, + { + code: ` +const y = 1; +if (y === 0) {} +`, + errors: [ruleError(3, 5, 'literalBooleanExpression')], + }, + { + code: ` +enum Foo { + a = 1, + b = 2 +} + +const x = Foo.a; +if (x === Foo.a) {} +`, + errors: [ruleError(8, 5, 'literalBooleanExpression')], + }, + + // Still errors on in the expected locations when ignoring RHS + { + options: [{ ignoreRhs: true }], + code: ` +const b1 = true; +const b2 = false; +const t1 = b1 && b2; +const t2 = b1 || b2; +if(b1 && b2) {} +while(b1 && b2) {} +for (let i = 0; (b1 && b2); i++) { break; } +const t1 = (b1 && b2) ? 'yes' : 'no'`, + errors: [ + ruleError(4, 12, 'alwaysTruthy'), + ruleError(5, 12, 'alwaysTruthy'), + ruleError(6, 4, 'alwaysTruthy'), + ruleError(7, 7, 'alwaysTruthy'), + ruleError(8, 18, 'alwaysTruthy'), + ruleError(9, 13, 'alwaysTruthy'), + ], + }, + ], +}); From c713ca43adb1527e6ed997a549ea327d3f8b29c0 Mon Sep 17 00:00:00 2001 From: Nikita Date: Thu, 12 Sep 2019 10:44:10 +0200 Subject: [PATCH 04/10] feat(eslint-plugin): [strict-boolean-expressions] Add allowNullable option (#794) --- .../docs/rules/strict-boolean-expressions.md | 1 + .../src/rules/strict-boolean-expressions.ts | 38 +++++++++++++++++-- .../rules/strict-boolean-expressions.test.ts | 28 ++++++++++++++ 3 files changed, 64 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md b/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md index 6bed5b03fe3a..72d38f1ed66c 100644 --- a/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md +++ b/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md @@ -56,6 +56,7 @@ while (typeof str !== 'undefined') { Options may be provided as an object with: +- `allowNullable` to allow `undefined` and `null` in addition to `boolean` as a type of all boolean expressions. (`false` by default). - `ignoreRhs` to skip the check on the right hand side of expressions like `a && b` or `a || b` - allows these operators to be used for their short-circuiting behavior. (`false` by default). ## Related To diff --git a/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts index 9c3c5b6ea98e..e67e4d1f687f 100644 --- a/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts +++ b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts @@ -16,6 +16,7 @@ type ExpressionWithTest = type Options = [ { ignoreRhs?: boolean; + allowNullable?: boolean; }, ]; @@ -36,6 +37,9 @@ export default util.createRule({ ignoreRhs: { type: 'boolean', }, + allowNullable: { + type: 'boolean', + }, }, additionalProperties: false, }, @@ -47,9 +51,10 @@ export default util.createRule({ defaultOptions: [ { ignoreRhs: false, + allowNullable: false, }, ], - create(context, [{ ignoreRhs }]) { + create(context, [options]) { const service = util.getParserServices(context); const checker = service.program.getTypeChecker(); @@ -61,7 +66,34 @@ export default util.createRule({ node, ); const type = util.getConstrainedTypeAtLocation(checker, tsNode); - return tsutils.isTypeFlagSet(type, ts.TypeFlags.BooleanLike); + + if (tsutils.isTypeFlagSet(type, ts.TypeFlags.BooleanLike)) { + return true; + } + + // Check variants of union + if (tsutils.isTypeFlagSet(type, ts.TypeFlags.Union)) { + let hasBoolean = false; + for (const ty of (type as ts.UnionType).types) { + if (tsutils.isTypeFlagSet(ty, ts.TypeFlags.BooleanLike)) { + hasBoolean = true; + continue; + } + if (options.allowNullable) { + if (tsutils.isTypeFlagSet(ty, ts.TypeFlags.Null)) { + continue; + } + if (tsutils.isTypeFlagSet(ty, ts.TypeFlags.Undefined)) { + continue; + } + } + // Union variant is something else + return false; + } + return hasBoolean; + } + + return false; } /** @@ -88,7 +120,7 @@ export default util.createRule({ ): void { if ( !isBooleanType(node.left) || - (!ignoreRhs && !isBooleanType(node.right)) + (!options.ignoreRhs && !isBooleanType(node.right)) ) { reportNode(node); } diff --git a/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts b/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts index 1067b5fa4c68..c95228b67430 100644 --- a/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts +++ b/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts @@ -165,6 +165,15 @@ const boolOrObj = bool || obj; const boolAndObj = bool && obj; `, }, + { + options: [{ allowNullable: true }], + code: ` + const f1 = (x?: boolean) => x ? 1 : 0; + const f2 = (x: boolean | null) => x ? 1 : 0; + const f3 = (x?: true | null) => x ? 1 : 0; + const f4 = (x?: false) => x ? 1 : 0; + `, + }, ], invalid: [ @@ -933,5 +942,24 @@ const objOrBool = obj || bool; const objAndBool = obj && bool; `, }, + { + options: [{ allowNullable: true }], + errors: [ + { + messageId: 'strictBooleanExpression', + line: 2, + column: 44, + }, + { + messageId: 'strictBooleanExpression', + line: 3, + column: 35, + }, + ], + code: ` + const f = (x: null | undefined) => x ? 1 : 0; + const f = (x?: number) => x ? 1 : 0; + `, + }, ], }); From 6a55921e2b9245102c4c373759dde4c36c3b4c08 Mon Sep 17 00:00:00 2001 From: Nikita Date: Thu, 12 Sep 2019 10:56:08 +0200 Subject: [PATCH 05/10] feat(eslint-plugin): [no-floating-promises] Add ignoreVoid option (#796) --- .../docs/rules/no-floating-promises.md | 30 ++++ .../src/rules/no-floating-promises.ts | 141 ++++++++++-------- .../tests/rules/no-floating-promises.test.ts | 8 + 3 files changed, 120 insertions(+), 59 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-floating-promises.md b/packages/eslint-plugin/docs/rules/no-floating-promises.md index 75bc49efa66d..f52a510b5463 100644 --- a/packages/eslint-plugin/docs/rules/no-floating-promises.md +++ b/packages/eslint-plugin/docs/rules/no-floating-promises.md @@ -36,6 +36,36 @@ returnsPromise().then(() => {}, () => {}); Promise.reject('value').catch(() => {}); ``` +## Options + +The rule accepts an options object with the following properties: + +```ts +type Options = { + // if true, checking void expresions will be skipped + ignoreVoid?: boolean; +}; + +const defaults = { + ignoreVoid: false, +}; +``` + +### ignoreVoid + +This allows to easily suppress false-positives with void operator. + +Examples of **correct** code for this rule with `{ ignoreVoid: true }`: + +```ts +async function returnsPromise() { + return 'value'; +} +void returnsPromise(); + +void Promise.reject('value'); +``` + ## When Not To Use It If you do not use Promise-like values in your codebase or want to allow them to diff --git a/packages/eslint-plugin/src/rules/no-floating-promises.ts b/packages/eslint-plugin/src/rules/no-floating-promises.ts index 816cc0846706..32538271b948 100644 --- a/packages/eslint-plugin/src/rules/no-floating-promises.ts +++ b/packages/eslint-plugin/src/rules/no-floating-promises.ts @@ -3,7 +3,13 @@ import * as ts from 'typescript'; import * as util from '../util'; -export default util.createRule({ +type Options = [ + { + ignoreVoid?: boolean; + }, +]; + +export default util.createRule({ name: 'no-floating-promises', meta: { docs: { @@ -15,12 +21,24 @@ export default util.createRule({ messages: { floating: 'Promises must be handled appropriately', }, - schema: [], + schema: [ + { + type: 'object', + properties: { + ignoreVoid: { type: 'boolean' }, + }, + additionalProperties: false, + }, + ], type: 'problem', }, - defaultOptions: [], + defaultOptions: [ + { + ignoreVoid: false, + }, + ], - create(context) { + create(context, [options]) { const parserServices = util.getParserServices(context); const checker = parserServices.program.getTypeChecker(); @@ -38,64 +56,69 @@ export default util.createRule({ } }, }; + + function isUnhandledPromise( + checker: ts.TypeChecker, + node: ts.Node, + ): boolean { + // First, check expressions whose resulting types may not be promise-like + if ( + ts.isBinaryExpression(node) && + node.operatorToken.kind === ts.SyntaxKind.CommaToken + ) { + // Any child in a comma expression could return a potentially unhandled + // promise, so we check them all regardless of whether the final returned + // value is promise-like. + return ( + isUnhandledPromise(checker, node.left) || + isUnhandledPromise(checker, node.right) + ); + } + + if (ts.isVoidExpression(node) && !options.ignoreVoid) { + // Similarly, a `void` expression always returns undefined, so we need to + // see what's inside it without checking the type of the overall expression. + return isUnhandledPromise(checker, node.expression); + } + + // Check the type. At this point it can't be unhandled if it isn't a promise + if (!isPromiseLike(checker, node)) { + return false; + } + + if (ts.isCallExpression(node)) { + // If the outer expression is a call, it must be either a `.then()` or + // `.catch()` that handles the promise. + return ( + !isPromiseCatchCallWithHandler(node) && + !isPromiseThenCallWithRejectionHandler(node) + ); + } else if (ts.isConditionalExpression(node)) { + // We must be getting the promise-like value from one of the branches of the + // ternary. Check them directly. + return ( + isUnhandledPromise(checker, node.whenFalse) || + isUnhandledPromise(checker, node.whenTrue) + ); + } else if ( + ts.isPropertyAccessExpression(node) || + ts.isIdentifier(node) || + ts.isNewExpression(node) + ) { + // If it is just a property access chain or a `new` call (e.g. `foo.bar` or + // `new Promise()`), the promise is not handled because it doesn't have the + // necessary then/catch call at the end of the chain. + return true; + } + + // We conservatively return false for all other types of expressions because + // we don't want to accidentally fail if the promise is handled internally but + // we just can't tell. + return false; + } }, }); -function isUnhandledPromise(checker: ts.TypeChecker, node: ts.Node): boolean { - // First, check expressions whose resulting types may not be promise-like - if ( - ts.isBinaryExpression(node) && - node.operatorToken.kind === ts.SyntaxKind.CommaToken - ) { - // Any child in a comma expression could return a potentially unhandled - // promise, so we check them all regardless of whether the final returned - // value is promise-like. - return ( - isUnhandledPromise(checker, node.left) || - isUnhandledPromise(checker, node.right) - ); - } else if (ts.isVoidExpression(node)) { - // Similarly, a `void` expression always returns undefined, so we need to - // see what's inside it without checking the type of the overall expression. - return isUnhandledPromise(checker, node.expression); - } - - // Check the type. At this point it can't be unhandled if it isn't a promise - if (!isPromiseLike(checker, node)) { - return false; - } - - if (ts.isCallExpression(node)) { - // If the outer expression is a call, it must be either a `.then()` or - // `.catch()` that handles the promise. - return ( - !isPromiseCatchCallWithHandler(node) && - !isPromiseThenCallWithRejectionHandler(node) - ); - } else if (ts.isConditionalExpression(node)) { - // We must be getting the promise-like value from one of the branches of the - // ternary. Check them directly. - return ( - isUnhandledPromise(checker, node.whenFalse) || - isUnhandledPromise(checker, node.whenTrue) - ); - } else if ( - ts.isPropertyAccessExpression(node) || - ts.isIdentifier(node) || - ts.isNewExpression(node) - ) { - // If it is just a property access chain or a `new` call (e.g. `foo.bar` or - // `new Promise()`), the promise is not handled because it doesn't have the - // necessary then/catch call at the end of the chain. - return true; - } - - // We conservatively return false for all other types of expressions because - // we don't want to accidentally fail if the promise is handled internally but - // we just can't tell. - return false; -} - // Modified from tsutils.isThenable() to only consider thenables which can be // rejected/caught via a second parameter. Original source (MIT licensed): // diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index 63a360715d91..013edb41bc0e 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -24,6 +24,14 @@ async function test() { return Promise.resolve("value"); } `, + { + options: [{ ignoreVoid: true }], + code: ` +async function test() { + void Promise.resolve("value"); +} +`, + }, ` async function test() { await Promise.reject(new Error("message")); From 46ee4c9d6d83bf55d9003a78c18b5df5f2f629fe Mon Sep 17 00:00:00 2001 From: Taeheon Kim Date: Thu, 12 Sep 2019 18:29:25 +0900 Subject: [PATCH 06/10] feat(eslint-plugin): [explicit-member-accessibility] add support of "ignoredMethodNames" (#895) --- .../rules/explicit-member-accessibility.md | 20 +++++++ .../rules/explicit-member-accessibility.ts | 18 ++++-- .../explicit-member-accessibility.test.ts | 60 +++++++++++++++++++ 3 files changed, 94 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/explicit-member-accessibility.md b/packages/eslint-plugin/docs/rules/explicit-member-accessibility.md index 96295809138c..a86257783918 100644 --- a/packages/eslint-plugin/docs/rules/explicit-member-accessibility.md +++ b/packages/eslint-plugin/docs/rules/explicit-member-accessibility.md @@ -20,6 +20,7 @@ type AccessibilityLevel = type Options = { accessibility?: AccessibilityLevel; + ignoredMethodNames?: string[]; overrides?: { accessors?: AccessibilityLevel; constructors?: AccessibilityLevel; @@ -310,6 +311,25 @@ class Animal { } ``` +### Except specific methods + +If you want to ignore some specific methods, you can do it by specifing method names. Note that this option does not care for the context, and will ignore every method with these names, which could lead to it missing some cases. You should use this sparingly. +e.g. `[ { ignoredMethodNames: ['specificMethod', 'whateverMethod'] } ]` + +```ts +class Animal { + get specificMethod() { + console.log('No error because you specified this method on option'); + } + get whateverMethod() { + console.log('No error because you specified this method on option'); + } + public get otherMethod() { + console.log('This method comply with this rule'); + } +} +``` + ## When Not To Use It If you think defaulting to public is a good default, then you should consider using the `no-public` setting. If you want to mix implicit and explicit public members then disable this rule. diff --git a/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts b/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts index a1d61d56ded2..864c630da178 100644 --- a/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts +++ b/packages/eslint-plugin/src/rules/explicit-member-accessibility.ts @@ -11,6 +11,7 @@ type AccessibilityLevel = interface Config { accessibility?: AccessibilityLevel; + ignoredMethodNames?: string[]; overrides?: { accessors?: AccessibilityLevel; constructors?: AccessibilityLevel; @@ -57,8 +58,15 @@ export default util.createRule({ properties: accessibilityLevel, parameterProperties: accessibilityLevel, }, + additionalProperties: false, }, + ignoredMethodNames: { + type: 'array', + items: { + type: 'string', + }, + }, }, additionalProperties: false, }, @@ -74,7 +82,7 @@ export default util.createRule({ const methodCheck = overrides.methods || baseCheck; const propCheck = overrides.properties || baseCheck; const paramPropCheck = overrides.parameterProperties || baseCheck; - + const ignoredMethodNames = new Set(option.ignoredMethodNames || []); /** * Generates the report for rule violations */ @@ -116,14 +124,16 @@ export default util.createRule({ nodeType = `${methodDefinition.kind} property accessor`; break; } - if (check === 'off') { - return; - } const methodName = util.getNameFromClassMember( methodDefinition, sourceCode, ); + + if (check === 'off' || ignoredMethodNames.has(methodName)) { + return; + } + if ( check === 'no-public' && methodDefinition.accessibility === 'public' diff --git a/packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts b/packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts index 051cfb30642e..7eb6bcd076b2 100644 --- a/packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts +++ b/packages/eslint-plugin/tests/rules/explicit-member-accessibility.test.ts @@ -258,6 +258,66 @@ class Test { }, ], }, + { + filename: 'test.ts', + code: ` +class Test { + public getX () { + return this.x + } +} + `, + options: [ + { + ignoredMethodNames: ['getX'], + }, + ], + }, + { + filename: 'test.ts', + code: ` +class Test { + public static getX () { + return this.x + } +} + `, + options: [ + { + ignoredMethodNames: ['getX'], + }, + ], + }, + { + filename: 'test.ts', + code: ` +class Test { + get getX () { + return this.x + } +} + `, + options: [ + { + ignoredMethodNames: ['getX'], + }, + ], + }, + { + filename: 'test.ts', + code: ` +class Test { + getX () { + return this.x + } +} + `, + options: [ + { + ignoredMethodNames: ['getX'], + }, + ], + }, ], invalid: [ { From aeea4cd40e2ac66ff596a205ba7bfc95aa376128 Mon Sep 17 00:00:00 2001 From: Alexander T Date: Thu, 12 Sep 2019 13:05:04 +0300 Subject: [PATCH 07/10] feat(eslint-plugin): [no-magic-numbers] add ignoreReadonlyClassProperties option (#938) --- .../docs/rules/no-magic-numbers.md | 28 ++++++++++++ .../src/rules/no-magic-numbers.ts | 25 +++++++++++ .../tests/rules/no-magic-numbers.test.ts | 44 +++++++++++++++++++ .../eslint-plugin/typings/eslint-rules.d.ts | 1 + 4 files changed, 98 insertions(+) diff --git a/packages/eslint-plugin/docs/rules/no-magic-numbers.md b/packages/eslint-plugin/docs/rules/no-magic-numbers.md index cf647833887d..215890a2e832 100644 --- a/packages/eslint-plugin/docs/rules/no-magic-numbers.md +++ b/packages/eslint-plugin/docs/rules/no-magic-numbers.md @@ -41,6 +41,34 @@ Examples of **correct** code for the `{ "ignoreNumericLiteralTypes": true }` opt type SmallPrimes = 2 | 3 | 5 | 7 | 11; ``` +### ignoreReadonlyClassProperties + +Examples of **incorrect** code for the `{ "ignoreReadonlyClassProperties": false }` option: + +```ts +/*eslint @typescript-eslint/no-magic-numbers: ["error", { "ignoreReadonlyClassProperties": false }]*/ + +class Foo { + readonly A = 1; + readonly B = 2; + public static readonly C = 1; + static readonly D = 1; +} +``` + +Examples of **correct** code for the `{ "ignoreReadonlyClassProperties": true }` option: + +```ts +/*eslint @typescript-eslint/no-magic-numbers: ["error", { "ignoreReadonlyClassProperties": true }]*/ + +class Foo { + readonly A = 1; + readonly B = 2; + public static readonly C = 1; + static readonly D = 1; +} +``` + ### ignoreEnums A boolean to specify if enums used in Typescript are considered okay. `false` by default. diff --git a/packages/eslint-plugin/src/rules/no-magic-numbers.ts b/packages/eslint-plugin/src/rules/no-magic-numbers.ts index 005bae9eb0ae..01a2505498f6 100644 --- a/packages/eslint-plugin/src/rules/no-magic-numbers.ts +++ b/packages/eslint-plugin/src/rules/no-magic-numbers.ts @@ -33,6 +33,9 @@ export default util.createRule({ ignoreEnums: { type: 'boolean', }, + ignoreReadonlyClassProperties: { + type: 'boolean', + }, }, }, ], @@ -46,6 +49,7 @@ export default util.createRule({ detectObjects: false, ignoreNumericLiteralTypes: false, ignoreEnums: false, + ignoreReadonlyClassProperties: false, }, ], create(context, [options]) { @@ -149,6 +153,20 @@ export default util.createRule({ return false; } + /** + * Checks if the node parent is a readonly class property + * @param node the node to be validated. + * @returns true if the node parent is a readonly class property + * @private + */ + function isParentTSReadonlyClassProperty(node: TSESTree.Node): boolean { + return ( + !!node.parent && + node.parent.type === AST_NODE_TYPES.ClassProperty && + !!node.parent.readonly + ); + } + return { Literal(node): void { // Check if the node is a TypeScript enum declaration @@ -156,6 +174,13 @@ export default util.createRule({ return; } + if ( + options.ignoreReadonlyClassProperties && + isParentTSReadonlyClassProperty(node) + ) { + return; + } + // Check TypeScript specific nodes for Numeric Literal if ( options.ignoreNumericLiteralTypes && diff --git a/packages/eslint-plugin/tests/rules/no-magic-numbers.test.ts b/packages/eslint-plugin/tests/rules/no-magic-numbers.test.ts index 09b6c35d0b47..1ab26cfa704d 100644 --- a/packages/eslint-plugin/tests/rules/no-magic-numbers.test.ts +++ b/packages/eslint-plugin/tests/rules/no-magic-numbers.test.ts @@ -41,6 +41,17 @@ ruleTester.run('no-magic-numbers', rule, { code: 'enum foo { SECOND = 1000, NUM = "0123456789" }', options: [{ ignoreEnums: true }], }, + { + code: ` +class Foo { + readonly A = 1; + readonly B = 2; + public static readonly C = 1; + static readonly D = 1; +} + `, + options: [{ ignoreReadonlyClassProperties: true }], + }, ], invalid: [ @@ -166,5 +177,38 @@ ruleTester.run('no-magic-numbers', rule, { }, ], }, + { + code: ` +class Foo { + readonly A = 1; + readonly B = 2; + public static readonly C = 1; + static readonly D = 1; +} + `, + options: [{ ignoreReadonlyClassProperties: false }], + errors: [ + { + messageId: 'noMagic', + line: 3, + column: 16, + }, + { + messageId: 'noMagic', + line: 4, + column: 16, + }, + { + messageId: 'noMagic', + line: 5, + column: 30, + }, + { + messageId: 'noMagic', + line: 6, + column: 23, + }, + ], + }, ], }); diff --git a/packages/eslint-plugin/typings/eslint-rules.d.ts b/packages/eslint-plugin/typings/eslint-rules.d.ts index de6545f744d8..120b11433cb9 100644 --- a/packages/eslint-plugin/typings/eslint-rules.d.ts +++ b/packages/eslint-plugin/typings/eslint-rules.d.ts @@ -199,6 +199,7 @@ declare module 'eslint/lib/rules/no-magic-numbers' { detectObjects?: boolean; ignoreNumericLiteralTypes?: boolean; ignoreEnums?: boolean; + ignoreReadonlyClassProperties?: boolean; }, ], { From 2bf823176927278a08df1c9df255b39c6515e948 Mon Sep 17 00:00:00 2001 From: Pavel Birukov Date: Fri, 13 Sep 2019 20:04:20 +0300 Subject: [PATCH 08/10] fix(typescript-estree): ImportDeclaration.specifier to Literal (#974) --- packages/eslint-plugin/src/rules/triple-slash-reference.ts | 3 +-- packages/typescript-estree/src/ts-estree/ts-estree.ts | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/triple-slash-reference.ts b/packages/eslint-plugin/src/rules/triple-slash-reference.ts index c5ab5772523a..8ce64e5748af 100644 --- a/packages/eslint-plugin/src/rules/triple-slash-reference.ts +++ b/packages/eslint-plugin/src/rules/triple-slash-reference.ts @@ -73,8 +73,7 @@ export default util.createRule({ return { ImportDeclaration(node): void { if (programNode) { - const source = node.source as TSESTree.Literal; - hasMatchingReference(source); + hasMatchingReference(node.source); } }, TSImportEqualsDeclaration(node): void { diff --git a/packages/typescript-estree/src/ts-estree/ts-estree.ts b/packages/typescript-estree/src/ts-estree/ts-estree.ts index 51529a470cba..296e62b0bfb2 100644 --- a/packages/typescript-estree/src/ts-estree/ts-estree.ts +++ b/packages/typescript-estree/src/ts-estree/ts-estree.ts @@ -732,7 +732,7 @@ export interface Import extends BaseNode { export interface ImportDeclaration extends BaseNode { type: AST_NODE_TYPES.ImportDeclaration; - source: Expression; + source: Literal; specifiers: ImportClause[]; } From 752fb31ff044fb3db206e86a977385250a925228 Mon Sep 17 00:00:00 2001 From: gorff5 Date: Sun, 15 Sep 2019 20:41:11 +0300 Subject: [PATCH 09/10] docs(eslint-plugin): fix typo in typedef docs (#976) --- packages/eslint-plugin/docs/rules/typedef.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/docs/rules/typedef.md b/packages/eslint-plugin/docs/rules/typedef.md index 4498ab956100..89bc1d41e6f7 100644 --- a/packages/eslint-plugin/docs/rules/typedef.md +++ b/packages/eslint-plugin/docs/rules/typedef.md @@ -44,7 +44,7 @@ For example, with the following configuration: ```json { "rules": { - "typedef": [ + "@typescript-eslint/typedef": [ "error", { "arrowParameter": false, From fa1cf71af4af75f3fcd3105af45f512cb2705117 Mon Sep 17 00:00:00 2001 From: James Henry Date: Mon, 16 Sep 2019 17:02:11 +0000 Subject: [PATCH 10/10] chore: publish v2.3.0 --- CHANGELOG.md | 20 ++++++++++++++++++++ lerna.json | 2 +- packages/eslint-plugin-tslint/CHANGELOG.md | 8 ++++++++ packages/eslint-plugin-tslint/package.json | 6 +++--- packages/eslint-plugin/CHANGELOG.md | 20 ++++++++++++++++++++ packages/eslint-plugin/package.json | 4 ++-- packages/experimental-utils/CHANGELOG.md | 8 ++++++++ packages/experimental-utils/package.json | 4 ++-- packages/parser/CHANGELOG.md | 8 ++++++++ packages/parser/package.json | 8 ++++---- packages/shared-fixtures/CHANGELOG.md | 8 ++++++++ packages/shared-fixtures/package.json | 2 +- packages/typescript-estree/CHANGELOG.md | 11 +++++++++++ packages/typescript-estree/package.json | 4 ++-- 14 files changed, 98 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 870ae588ac71..a45c6f82df07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,26 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +# [2.3.0](https://github.com/typescript-eslint/typescript-eslint/compare/v2.2.0...v2.3.0) (2019-09-16) + + +### Bug Fixes + +* **typescript-estree:** ImportDeclaration.specifier to Literal ([#974](https://github.com/typescript-eslint/typescript-eslint/issues/974)) ([2bf8231](https://github.com/typescript-eslint/typescript-eslint/commit/2bf8231)) + + +### Features + +* **eslint-plugin:** [explicit-member-accessibility] add support of "ignoredMethodNames" ([#895](https://github.com/typescript-eslint/typescript-eslint/issues/895)) ([46ee4c9](https://github.com/typescript-eslint/typescript-eslint/commit/46ee4c9)) +* **eslint-plugin:** [no-floating-promises] Add ignoreVoid option ([#796](https://github.com/typescript-eslint/typescript-eslint/issues/796)) ([6a55921](https://github.com/typescript-eslint/typescript-eslint/commit/6a55921)) +* **eslint-plugin:** [no-magic-numbers] add ignoreReadonlyClassProperties option ([#938](https://github.com/typescript-eslint/typescript-eslint/issues/938)) ([aeea4cd](https://github.com/typescript-eslint/typescript-eslint/commit/aeea4cd)) +* **eslint-plugin:** [strict-boolean-expressions] Add allowNullable option ([#794](https://github.com/typescript-eslint/typescript-eslint/issues/794)) ([c713ca4](https://github.com/typescript-eslint/typescript-eslint/commit/c713ca4)) +* **eslint-plugin:** add no-unnecessary-condition rule ([#699](https://github.com/typescript-eslint/typescript-eslint/issues/699)) ([5715482](https://github.com/typescript-eslint/typescript-eslint/commit/5715482)) + + + + + # [2.2.0](https://github.com/typescript-eslint/typescript-eslint/compare/v2.1.0...v2.2.0) (2019-09-09) diff --git a/lerna.json b/lerna.json index d605df702a98..ea1d9387a740 100644 --- a/lerna.json +++ b/lerna.json @@ -1,5 +1,5 @@ { - "version": "2.2.0", + "version": "2.3.0", "npmClient": "yarn", "useWorkspaces": true, "stream": true diff --git a/packages/eslint-plugin-tslint/CHANGELOG.md b/packages/eslint-plugin-tslint/CHANGELOG.md index ce13360be83f..04abe45358bd 100644 --- a/packages/eslint-plugin-tslint/CHANGELOG.md +++ b/packages/eslint-plugin-tslint/CHANGELOG.md @@ -3,6 +3,14 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +# [2.3.0](https://github.com/typescript-eslint/typescript-eslint/compare/v2.2.0...v2.3.0) (2019-09-16) + +**Note:** Version bump only for package @typescript-eslint/eslint-plugin-tslint + + + + + # [2.2.0](https://github.com/typescript-eslint/typescript-eslint/compare/v2.1.0...v2.2.0) (2019-09-09) **Note:** Version bump only for package @typescript-eslint/eslint-plugin-tslint diff --git a/packages/eslint-plugin-tslint/package.json b/packages/eslint-plugin-tslint/package.json index bcf7455d69de..b824c1d3808e 100644 --- a/packages/eslint-plugin-tslint/package.json +++ b/packages/eslint-plugin-tslint/package.json @@ -1,6 +1,6 @@ { "name": "@typescript-eslint/eslint-plugin-tslint", - "version": "2.2.0", + "version": "2.3.0", "main": "dist/index.js", "typings": "src/index.ts", "description": "TSLint wrapper plugin for ESLint", @@ -31,7 +31,7 @@ "typecheck": "tsc --noEmit" }, "dependencies": { - "@typescript-eslint/experimental-utils": "2.2.0", + "@typescript-eslint/experimental-utils": "2.3.0", "lodash.memoize": "^4.1.2" }, "peerDependencies": { @@ -41,6 +41,6 @@ }, "devDependencies": { "@types/lodash.memoize": "^4.1.4", - "@typescript-eslint/parser": "2.2.0" + "@typescript-eslint/parser": "2.3.0" } } diff --git a/packages/eslint-plugin/CHANGELOG.md b/packages/eslint-plugin/CHANGELOG.md index 7e0867066c8e..92ad694e32a6 100644 --- a/packages/eslint-plugin/CHANGELOG.md +++ b/packages/eslint-plugin/CHANGELOG.md @@ -3,6 +3,26 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +# [2.3.0](https://github.com/typescript-eslint/typescript-eslint/compare/v2.2.0...v2.3.0) (2019-09-16) + + +### Bug Fixes + +* **typescript-estree:** ImportDeclaration.specifier to Literal ([#974](https://github.com/typescript-eslint/typescript-eslint/issues/974)) ([2bf8231](https://github.com/typescript-eslint/typescript-eslint/commit/2bf8231)) + + +### Features + +* **eslint-plugin:** [explicit-member-accessibility] add support of "ignoredMethodNames" ([#895](https://github.com/typescript-eslint/typescript-eslint/issues/895)) ([46ee4c9](https://github.com/typescript-eslint/typescript-eslint/commit/46ee4c9)) +* **eslint-plugin:** [no-floating-promises] Add ignoreVoid option ([#796](https://github.com/typescript-eslint/typescript-eslint/issues/796)) ([6a55921](https://github.com/typescript-eslint/typescript-eslint/commit/6a55921)) +* **eslint-plugin:** [no-magic-numbers] add ignoreReadonlyClassProperties option ([#938](https://github.com/typescript-eslint/typescript-eslint/issues/938)) ([aeea4cd](https://github.com/typescript-eslint/typescript-eslint/commit/aeea4cd)) +* **eslint-plugin:** [strict-boolean-expressions] Add allowNullable option ([#794](https://github.com/typescript-eslint/typescript-eslint/issues/794)) ([c713ca4](https://github.com/typescript-eslint/typescript-eslint/commit/c713ca4)) +* **eslint-plugin:** add no-unnecessary-condition rule ([#699](https://github.com/typescript-eslint/typescript-eslint/issues/699)) ([5715482](https://github.com/typescript-eslint/typescript-eslint/commit/5715482)) + + + + + # [2.2.0](https://github.com/typescript-eslint/typescript-eslint/compare/v2.1.0...v2.2.0) (2019-09-09) diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index 7a8a84a0e2d5..ee8d22ae84ef 100644 --- a/packages/eslint-plugin/package.json +++ b/packages/eslint-plugin/package.json @@ -1,6 +1,6 @@ { "name": "@typescript-eslint/eslint-plugin", - "version": "2.2.0", + "version": "2.3.0", "description": "TypeScript plugin for ESLint", "keywords": [ "eslint", @@ -40,7 +40,7 @@ "typecheck": "tsc --noEmit" }, "dependencies": { - "@typescript-eslint/experimental-utils": "2.2.0", + "@typescript-eslint/experimental-utils": "2.3.0", "eslint-utils": "^1.4.2", "functional-red-black-tree": "^1.0.1", "regexpp": "^2.0.1", diff --git a/packages/experimental-utils/CHANGELOG.md b/packages/experimental-utils/CHANGELOG.md index 9352d0a4c46d..9b3abf341a2a 100644 --- a/packages/experimental-utils/CHANGELOG.md +++ b/packages/experimental-utils/CHANGELOG.md @@ -3,6 +3,14 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +# [2.3.0](https://github.com/typescript-eslint/typescript-eslint/compare/v2.2.0...v2.3.0) (2019-09-16) + +**Note:** Version bump only for package @typescript-eslint/experimental-utils + + + + + # [2.2.0](https://github.com/typescript-eslint/typescript-eslint/compare/v2.1.0...v2.2.0) (2019-09-09) **Note:** Version bump only for package @typescript-eslint/experimental-utils diff --git a/packages/experimental-utils/package.json b/packages/experimental-utils/package.json index a28cfb2c89ed..86f65797bf2e 100644 --- a/packages/experimental-utils/package.json +++ b/packages/experimental-utils/package.json @@ -1,6 +1,6 @@ { "name": "@typescript-eslint/experimental-utils", - "version": "2.2.0", + "version": "2.3.0", "description": "(Experimental) Utilities for working with TypeScript + ESLint together", "keywords": [ "eslint", @@ -37,7 +37,7 @@ }, "dependencies": { "@types/json-schema": "^7.0.3", - "@typescript-eslint/typescript-estree": "2.2.0", + "@typescript-eslint/typescript-estree": "2.3.0", "eslint-scope": "^5.0.0" }, "peerDependencies": { diff --git a/packages/parser/CHANGELOG.md b/packages/parser/CHANGELOG.md index 507eac315f78..f9120a7acde3 100644 --- a/packages/parser/CHANGELOG.md +++ b/packages/parser/CHANGELOG.md @@ -3,6 +3,14 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +# [2.3.0](https://github.com/typescript-eslint/typescript-eslint/compare/v2.2.0...v2.3.0) (2019-09-16) + +**Note:** Version bump only for package @typescript-eslint/parser + + + + + # [2.2.0](https://github.com/typescript-eslint/typescript-eslint/compare/v2.1.0...v2.2.0) (2019-09-09) **Note:** Version bump only for package @typescript-eslint/parser diff --git a/packages/parser/package.json b/packages/parser/package.json index 9deee00e0bb7..a0b6a09ceadf 100644 --- a/packages/parser/package.json +++ b/packages/parser/package.json @@ -1,6 +1,6 @@ { "name": "@typescript-eslint/parser", - "version": "2.2.0", + "version": "2.3.0", "description": "An ESLint custom parser which leverages TypeScript ESTree", "main": "dist/parser.js", "types": "dist/parser.d.ts", @@ -43,13 +43,13 @@ }, "dependencies": { "@types/eslint-visitor-keys": "^1.0.0", - "@typescript-eslint/experimental-utils": "2.2.0", - "@typescript-eslint/typescript-estree": "2.2.0", + "@typescript-eslint/experimental-utils": "2.3.0", + "@typescript-eslint/typescript-estree": "2.3.0", "eslint-visitor-keys": "^1.1.0" }, "devDependencies": { "@types/glob": "^7.1.1", - "@typescript-eslint/shared-fixtures": "2.2.0", + "@typescript-eslint/shared-fixtures": "2.3.0", "glob": "^7.1.4" } } diff --git a/packages/shared-fixtures/CHANGELOG.md b/packages/shared-fixtures/CHANGELOG.md index 4dfa4cd5e51a..7d8e9d76f5d2 100644 --- a/packages/shared-fixtures/CHANGELOG.md +++ b/packages/shared-fixtures/CHANGELOG.md @@ -3,6 +3,14 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +# [2.3.0](https://github.com/typescript-eslint/typescript-eslint/compare/v2.2.0...v2.3.0) (2019-09-16) + +**Note:** Version bump only for package @typescript-eslint/shared-fixtures + + + + + # [2.2.0](https://github.com/typescript-eslint/typescript-eslint/compare/v2.1.0...v2.2.0) (2019-09-09) **Note:** Version bump only for package @typescript-eslint/shared-fixtures diff --git a/packages/shared-fixtures/package.json b/packages/shared-fixtures/package.json index 4081dcf5f031..0c48c0177f7b 100644 --- a/packages/shared-fixtures/package.json +++ b/packages/shared-fixtures/package.json @@ -1,5 +1,5 @@ { "name": "@typescript-eslint/shared-fixtures", - "version": "2.2.0", + "version": "2.3.0", "private": true } diff --git a/packages/typescript-estree/CHANGELOG.md b/packages/typescript-estree/CHANGELOG.md index 4fff19407f16..4d3c460a3897 100644 --- a/packages/typescript-estree/CHANGELOG.md +++ b/packages/typescript-estree/CHANGELOG.md @@ -3,6 +3,17 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +# [2.3.0](https://github.com/typescript-eslint/typescript-eslint/compare/v2.2.0...v2.3.0) (2019-09-16) + + +### Bug Fixes + +* **typescript-estree:** ImportDeclaration.specifier to Literal ([#974](https://github.com/typescript-eslint/typescript-eslint/issues/974)) ([2bf8231](https://github.com/typescript-eslint/typescript-eslint/commit/2bf8231)) + + + + + # [2.2.0](https://github.com/typescript-eslint/typescript-eslint/compare/v2.1.0...v2.2.0) (2019-09-09) **Note:** Version bump only for package @typescript-eslint/typescript-estree diff --git a/packages/typescript-estree/package.json b/packages/typescript-estree/package.json index b1edc190b3ea..6f1217c43523 100644 --- a/packages/typescript-estree/package.json +++ b/packages/typescript-estree/package.json @@ -1,6 +1,6 @@ { "name": "@typescript-eslint/typescript-estree", - "version": "2.2.0", + "version": "2.3.0", "description": "A parser that converts TypeScript source code into an ESTree compatible form", "main": "dist/parser.js", "types": "dist/parser.d.ts", @@ -56,7 +56,7 @@ "@types/lodash.isplainobject": "^4.0.4", "@types/lodash.unescape": "^4.0.4", "@types/semver": "^6.0.1", - "@typescript-eslint/shared-fixtures": "2.2.0", + "@typescript-eslint/shared-fixtures": "2.3.0", "babel-code-frame": "^6.26.0", "glob": "^7.1.4", "lodash.isplainobject": "4.0.6",