From a06d36881805fd37dd835e240cd87a01e964522e Mon Sep 17 00:00:00 2001 From: Vitor Buzinaro Date: Fri, 13 Nov 2015 22:58:19 -0300 Subject: [PATCH 01/17] Fixes #757 - links to community custom rules Added links to `buzinas/tslint-eslint-rules` and `Microsoft/tslint-microsoft-contrib`. --- README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/README.md b/README.md index ad6bb662588..0d8922353dd 100644 --- a/README.md +++ b/README.md @@ -247,6 +247,14 @@ Rules flags enable or disable rules as they are parsed. A rule is enabled or dis For example, imagine the directive `/* tslint:disable */` on the first line of a file, `/* tslint:enable:ban class-name */` on the 10th line and `/* tslint:enable */` on the 20th. No rules will be checked between the 1st and 10th lines, only the `ban` and `class-name` rules will be checked between the 10th and 20th, and all rules will be checked for the remainder of the file. +Community Rules +--------------- + +In case we don't have the rules you're looking for, you can take advantage of some rules created by the community: + +- [ESLint rules for TSLint - Improve your TSLint with the missing ESLint Rules](https://github.com/buzinas/tslint-eslint-rules) +- [tslint-microsoft-contrib - A set of TSLint rules used on some Microsoft projects](https://github.com/Microsoft/tslint-microsoft-contrib) + Custom Rules ------------ TSLint ships with a set of core rules that can be configured. However, users are also allowed to write their own rules, which allows them to enforce specific behavior not covered by the core of TSLint. TSLint's internal rules are itself written to be pluggable, so adding a new rule is as simple as creating a new rule file named by convention. New rules can be written in either TypeScript or Javascript; if written in TypeScript, the code must be compiled to Javascript before invoking TSLint. From 46f8df75d82cd20e5b3642b15484b3c93e09697e Mon Sep 17 00:00:00 2001 From: Jason Killian Date: Fri, 13 Nov 2015 21:22:42 -0500 Subject: [PATCH 02/17] tweak wording of README --- README.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 0d8922353dd..aace18897e0 100644 --- a/README.md +++ b/README.md @@ -247,15 +247,15 @@ Rules flags enable or disable rules as they are parsed. A rule is enabled or dis For example, imagine the directive `/* tslint:disable */` on the first line of a file, `/* tslint:enable:ban class-name */` on the 10th line and `/* tslint:enable */` on the 20th. No rules will be checked between the 1st and 10th lines, only the `ban` and `class-name` rules will be checked between the 10th and 20th, and all rules will be checked for the remainder of the file. -Community Rules +Custom Rules (from the TypeScript community) --------------- -In case we don't have the rules you're looking for, you can take advantage of some rules created by the community: +If we don't have all the rules you're looking for, you can either write your own custom rules or use custom rules that others have developed. The repos below are a good source of custom rules: -- [ESLint rules for TSLint - Improve your TSLint with the missing ESLint Rules](https://github.com/buzinas/tslint-eslint-rules) -- [tslint-microsoft-contrib - A set of TSLint rules used on some Microsoft projects](https://github.com/Microsoft/tslint-microsoft-contrib) +- [ESLint rules for TSLint](https://github.com/buzinas/tslint-eslint-rules) - Improve your TSLint with the missing ESLint Rules +- [tslint-microsoft-contrib](https://github.com/Microsoft/tslint-microsoft-contrib) - A set of TSLint rules used on some Microsoft projects -Custom Rules +Writing Custom Rules ------------ TSLint ships with a set of core rules that can be configured. However, users are also allowed to write their own rules, which allows them to enforce specific behavior not covered by the core of TSLint. TSLint's internal rules are itself written to be pluggable, so adding a new rule is as simple as creating a new rule file named by convention. New rules can be written in either TypeScript or Javascript; if written in TypeScript, the code must be compiled to Javascript before invoking TSLint. @@ -326,7 +326,7 @@ As you can see, it's a pretty straightforward translation from the equivalent Ty Finally, core rules cannot be overwritten with a custom implementation, and rules can also take in options (retrieved via `this.getOptions()`). -Custom Formatters +Writing Custom Formatters ----------------- Just like rules, additional formatters can also be supplied to TSLint via `--formatters-dir` on the CLI or `formattersDirectory` option on the library or `grunt-tslint`. Writing a new formatter is simpler than writing a new rule, as shown in the JSON formatter's code. From 4cb3bdf1c9f64bad42c6dbb43a1a49d11de5617f Mon Sep 17 00:00:00 2001 From: nikklassen Date: Sun, 15 Nov 2015 17:20:08 -0500 Subject: [PATCH 03/17] Fix member-access checking for object literals and nested objects --- README.md | 2 + src/rules/memberAccessRule.ts | 33 ++++++++++++ .../files/rules/memberaccess-accessor.test.ts | 17 ++++++ .../rules/memberaccess-constructor.test.ts | 9 ++++ test/files/rules/memberaccess.test.ts | 42 ++++++++++----- test/rules/memberAccessTests.ts | 52 +++++++++++++++---- 6 files changed, 132 insertions(+), 23 deletions(-) create mode 100644 test/files/rules/memberaccess-accessor.test.ts create mode 100644 test/files/rules/memberaccess-constructor.test.ts diff --git a/README.md b/README.md index aace18897e0..48663e2a98b 100644 --- a/README.md +++ b/README.md @@ -152,6 +152,8 @@ A sample configuration file with all options is available [here](https://github. * `label-undefined` checks that labels are defined before usage. * `max-line-length` sets the maximum length of a line. * `member-access` enforces using explicit visibility on class members + * `"check-accessor"` enforces explicit visibility on accessors (can only be public) + * `"check-constructor"` enforces explicit visibility on constructors (can only be public) * `member-ordering` enforces member ordering. Rule options: * `public-before-private` All public members must be declared before private members * `static-before-instance` All static members must be declared before instance members diff --git a/src/rules/memberAccessRule.ts b/src/rules/memberAccessRule.ts index ce4288fafec..8bc038e9c81 100644 --- a/src/rules/memberAccessRule.ts +++ b/src/rules/memberAccessRule.ts @@ -26,19 +26,52 @@ export class Rule extends Lint.Rules.AbstractRule { } export class MemberAccessWalker extends Lint.RuleWalker { + private validateConstructors: boolean; + private validateAccessors: boolean; + constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { super(sourceFile, options); + this.validateConstructors = this.hasOption("check-constructor"); + this.validateAccessors = this.hasOption("check-accessor"); + } + + public visitConstructorDeclaration(node: ts.ConstructorDeclaration) { + if (this.validateConstructors) { + // constructor is only allowed to have public or nothing, but the compiler will catch this + this.validateVisibilityModifiers(node); + } + + super.visitConstructorDeclaration(node); } public visitMethodDeclaration(node: ts.MethodDeclaration) { this.validateVisibilityModifiers(node); + super.visitMethodDeclaration(node); } public visitPropertyDeclaration(node: ts.PropertyDeclaration) { this.validateVisibilityModifiers(node); } + public visitGetAccessor(node: ts.AccessorDeclaration) { + if (this.validateAccessors) { + this.validateVisibilityModifiers(node); + } + super.visitGetAccessor(node); + } + + public visitSetAccessor(node: ts.AccessorDeclaration) { + if (this.validateAccessors) { + this.validateVisibilityModifiers(node); + } + super.visitSetAccessor(node); + } + private validateVisibilityModifiers(node: ts.Node) { + if (node.parent.kind === ts.SyntaxKind.ObjectLiteralExpression) { + return; + } + const hasAnyVisibilityModifiers = Lint.hasModifier( node.modifiers, ts.SyntaxKind.PublicKeyword, diff --git a/test/files/rules/memberaccess-accessor.test.ts b/test/files/rules/memberaccess-accessor.test.ts new file mode 100644 index 00000000000..f664fa7f324 --- /dev/null +++ b/test/files/rules/memberaccess-accessor.test.ts @@ -0,0 +1,17 @@ +class Members { + get g() { + return 1; + } + set s(o: any) {} + + public get publicG() { + return 1; + } + public set publicS(o: any) {} +} + +const obj = { + get g() { + return 1; + } +}; diff --git a/test/files/rules/memberaccess-constructor.test.ts b/test/files/rules/memberaccess-constructor.test.ts new file mode 100644 index 00000000000..4f20fd4fc2b --- /dev/null +++ b/test/files/rules/memberaccess-constructor.test.ts @@ -0,0 +1,9 @@ +class ContructorsNoAccess { + constructor(i: number); + constructor(o: any) {} +} + +class ContructorsAccess { + public constructor(i: number); + public constructor(o: any) {} +} diff --git a/test/files/rules/memberaccess.test.ts b/test/files/rules/memberaccess.test.ts index 0800b0c4d0a..cff562c6672 100644 --- a/test/files/rules/memberaccess.test.ts +++ b/test/files/rules/memberaccess.test.ts @@ -1,18 +1,32 @@ -class Foo { - constructor() { - } +declare class AmbientNoAccess { + a(): number; +} - public w: number; - private x: number; - protected y: number; - z: number; +declare class AmbientAccess { + public a(): number; +} - public barW(): any { - } - private barX(): any { - } - protected barY(): any { - } - barZ(): any { +class Members { + i: number; + + public nPublic: number; + protected nProtected: number; + private nPrivate: number; + + noAccess(x: number): number; + noAccess(o: any): any {} + + public access(x: number): number; + public access(o: any): any {} +} + +const obj = { + func() {} +}; + +function main() { + class A { + i: number; + public n: number; } } diff --git a/test/rules/memberAccessTests.ts b/test/rules/memberAccessTests.ts index fc2920813c9..4b0c17843eb 100644 --- a/test/rules/memberAccessTests.ts +++ b/test/rules/memberAccessTests.ts @@ -16,14 +16,48 @@ import * as Lint from "../lint"; describe("", () => { - it("enforces using explicit visibility on class members", () => { - let fileName = "rules/memberaccess.test.ts"; - let MemberAccessRule = Lint.Test.getRule("member-access"); - let actualFailures = Lint.Test.applyRuleOnFile(fileName, MemberAccessRule); - - Lint.Test.assertFailuresEqual(actualFailures, [ - Lint.Test.createFailure(fileName, [8, 5], [8, 15], MemberAccessRule.FAILURE_STRING), - Lint.Test.createFailure(fileName, [16, 5], [17, 6], MemberAccessRule.FAILURE_STRING) - ]); + + it("ensures that class properties have access modifiers", () => { + const fileName = "rules/memberaccess.test.ts"; + const expectedFailures = [ + [[2, 5], [2, 17]], + [[10, 5], [10, 15]], + [[16, 5], [16, 33]], + [[17, 5], [17, 29]], + [[29, 9], [29, 19]] + ]; + + checkFile(fileName, expectedFailures); + }); + + it("ensures that constructors have access modifiers", () => { + const fileName = "rules/memberaccess-constructor.test.ts"; + const expectedFailures = [ + [[2, 5], [2, 28]], + [[3, 5], [3, 27]] + ]; + const options = [true, "check-constructor"]; + + checkFile(fileName, expectedFailures, options); }); + + it("ensures that accessors have access modifiers", () => { + const fileName = "rules/memberaccess-accessor.test.ts"; + const expectedFailures = [ + [[2, 5], [4, 6]], + [[5, 5], [5, 21]] + ]; + const options = [true, "check-accessor"]; + + checkFile(fileName, expectedFailures, options); + }); + + function checkFile(fileName: string, expectedFailures: number[][][], options: any[] = [true]) { + const MemberAccessRule = Lint.Test.getRule("member-access"); + const createFailure = Lint.Test.createFailuresOnFile(fileName, MemberAccessRule.FAILURE_STRING); + const expectedFileFailures = expectedFailures.map(failure => createFailure(failure[0], failure[1])); + + const actualFailures = Lint.Test.applyRuleOnFile(fileName, MemberAccessRule, options); + Lint.Test.assertFailuresEqual(actualFailures, expectedFileFailures); + } }); From 1a1b6357f3c20f8a09dddac2b1a4365b266cf677 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Mon, 16 Nov 2015 13:24:48 -0500 Subject: [PATCH 04/17] Update tsconfig to prevent atom-typescript from rewriting files --- src/tsconfig.json | 5 ++++- test/tsconfig.json | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/tsconfig.json b/src/tsconfig.json index d37238b8b3c..3155a7f22ae 100644 --- a/src/tsconfig.json +++ b/src/tsconfig.json @@ -8,6 +8,9 @@ "target": "es5", "outDir": "../lib" }, + "atom": { + "rewriteTsconfig": false + }, "filesGlob": [ "../typings/tsd.d.ts", "./*.ts", @@ -98,4 +101,4 @@ "rules/variableNameRule.ts", "rules/whitespaceRule.ts" ] -} \ No newline at end of file +} diff --git a/test/tsconfig.json b/test/tsconfig.json index ee210e73559..cb7128c529e 100644 --- a/test/tsconfig.json +++ b/test/tsconfig.json @@ -7,6 +7,9 @@ "target": "es5", "outDir": "../build" }, + "atom": { + "rewriteTsconfig": false + }, "filesGlob": [ "../typings/tsd.d.ts", "./typings/**/*.d.ts", @@ -166,4 +169,4 @@ "formatters/proseFormatterTests.ts", "formatters/verboseFormatterTests.ts" ] -} \ No newline at end of file +} From 9bd325aed280971f981ed85b6486312a0b30c20d Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Mon, 16 Nov 2015 13:33:58 -0500 Subject: [PATCH 05/17] Prepare release 3.0.0 - Set typescript peer dependency to >= 1.6.2 - Set typescript dev dependency to latest (1.6.2) - Some whitespace cleanup in CHANGELOG.md --- CHANGELOG.md | 59 +++++++++++++++++++++++++++++---------------------- package.json | 6 +++--- src/tslint.ts | 2 +- 3 files changed, 38 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 858a39930bf..ef78bd7722e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,15 @@ Change Log === +v3.0.0 +--- +* All the changes from the following releases, including some **breaking change**: + * `3.0.0-dev.3` + * `3.0.0-dev.2` + * `3.0.0-dev.1` + * `2.6.0-dev.2` + * `2.6.0-dev.1` + v3.0.0-dev.3 --- * Typescript is now a peerDependency (#791) @@ -16,12 +25,12 @@ v3.0.0-dev.2 v3.0.0-dev.1 --- * **BREAKING CHANGES** - * Rearchitect tslint to use external modules instead of merged namespaces (#726) - * Dependencies need to be handled differently now by custom rules and formatters - * See the [PR](https://github.com/palantir/tslint/pull/726) for full details about this change - * `no-trailing-comma` rule removed, it is replaced by the `trailing-comma` rule (#687) - * Rename `sort-object-literal-keys` rule to `object-literal-sort-keys` (#304, #537) - * `Lint.abstract()` has been removed (#700) + * Rearchitect TSLint to use external modules instead of merged namespaces (#726) + * Dependencies need to be handled differently now by custom rules and formatters + * See the [PR](https://github.com/palantir/tslint/pull/726) for full details about this change + * `no-trailing-comma` rule removed, it is replaced by the `trailing-comma` rule (#687) + * Rename `sort-object-literal-keys` rule to `object-literal-sort-keys` (#304, #537) + * `Lint.abstract()` has been removed (#700) * [new-rule] `trailing-comma` rule (#557, #687) * [new-rule-option] "ban-keywords" option for `variable-name` rule (#735, #748) * [bugfix] `typedef` rule now handles `for-of` loops correctly (#743) @@ -42,7 +51,7 @@ v2.5.1 * [new-rule-option] "avoid-escape" option for quotemark rule (#543) * [bugfix] type declaration for tslint external module #686 * [enhancement] `AbstractRule` and `AbstractFormatter` are now abstract classes (#631) - * Note: `Lint.abstract()` is now deprecated + * Note: `Lint.abstract()` is now deprecated v2.5.0 --- @@ -112,7 +121,7 @@ v2.4.0 * [bug] fix error message in `no-var-keyword` rule * [enhancement] CI tests are now run on node v0.12 in addition to v0.10 * **BREAKING** - * `-f` option removed from CLI + * `-f` option removed from CLI v2.3.1-beta --- @@ -120,7 +129,7 @@ v2.3.1-beta * [new-rule] `no-require-imports` disallows `require()` style imports * [new-rule] `no-shadowed-variable` moves over shadowed variable checking from `no-duplicate-variable` into its own rule * **BREAKING** - * `no-duplicate-variable` now only checks for duplicates within the same block scope; enable `no-shadowed-variable` to get duplicate-variable checking across block scopes + * `no-duplicate-variable` now only checks for duplicates within the same block scope; enable `no-shadowed-variable` to get duplicate-variable checking across block scopes * [enhancement] `no-duplicate-variable`, `no-shadowed-variable`, and `no-use-before-declare` now support ES6 destructuring * [enhancement] tslint CLI now uses a default configuration if no config file is found @@ -137,8 +146,8 @@ v2.2.0-beta --- * Upgraded Typescript compiler to 1.5.0-beta * **BREAKING CHANGES** - * due to changes to the typescript compiler API, old custom rules may no longer work and may need to be rewritten - * the JSON formatter's line and character positions are now back to being 0-indexed instead of 1-indexed + * due to changes to the typescript compiler API, old custom rules may no longer work and may need to be rewritten + * the JSON formatter's line and character positions are now back to being 0-indexed instead of 1-indexed * [bugs] #328 #334 #319 #351 #365 #254 * [bug] fixes for tslint behavior around template strings (fixes #357, #349, #332, and more) * [new-rule] `align` rule now enforces vertical alignment on parameters, arguments, and statements @@ -160,16 +169,16 @@ v2.0.1 --- * Upgraded Typescript compiler to 1.4 * **BREAKING CHANGES** - * typedef rule options were modified: - * index-signature removed as no longer necessary - * property-signature renamed to property-declaration - * variable-declarator renamed to variable-declaration - * member-variable-declarator renamed to member-variable-declaration - * typedef-whitespace rule options were modified: - * catch-clause was removed as invalid - * further options were added, see readme for more details - * due to changes to the typescript compiler API, old custom rules may no longer work and may need to be rewritten - * the JSON formatter's line and character positions are now 1-indexed instead of 0-indexed + * typedef rule options were modified: + * index-signature removed as no longer necessary + * property-signature renamed to property-declaration + * variable-declarator renamed to variable-declaration + * member-variable-declarator renamed to member-variable-declaration + * typedef-whitespace rule options were modified: + * catch-clause was removed as invalid + * further options were added, see readme for more details + * due to changes to the typescript compiler API, old custom rules may no longer work and may need to be rewritten + * the JSON formatter's line and character positions are now 1-indexed instead of 0-indexed v1.2.0 --- @@ -183,10 +192,10 @@ v1.0.0 --- * upgrade TypeScript compiler to 1.3 * **BREAKING CHANGES** - * all error messages now start with a lower-case character and do not end with a period - * all rule options are consistent in nomenclature. The `typedef` and `typedef-whitespace` rules now take in hyphenated options - * `unused-variables` rule cannot find unused private variables defined in the constructor due to a bug in 1.3 compiler - * `indent` rule has changed to only check for tabs or spaces and not enforce indentation levels + * all error messages now start with a lower-case character and do not end with a period + * all rule options are consistent in nomenclature. The `typedef` and `typedef-whitespace` rules now take in hyphenated options + * `unused-variables` rule cannot find unused private variables defined in the constructor due to a bug in 1.3 compiler + * `indent` rule has changed to only check for tabs or spaces and not enforce indentation levels v0.4.12 --- diff --git a/package.json b/package.json index fc54bf01e1c..7823a16c397 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "tslint", - "version": "3.0.0-dev.3", + "version": "3.0.0", "description": "a static analysis linter for TypeScript", "bin": { "tslint": "./bin/tslint" @@ -36,10 +36,10 @@ "grunt-tslint": "latest", "mocha": "^2.2.5", "tslint": "latest", - "typescript": "next" + "typescript": "latest" }, "peerDependencies": { - "typescript": ">=1.7.0 || >=1.7.0-dev.20151003 || >=1.8.0-dev" + "typescript": ">=1.6.2" }, "license": "Apache-2.0" } diff --git a/src/tslint.ts b/src/tslint.ts index 2d60b201271..5910df2f33e 100644 --- a/src/tslint.ts +++ b/src/tslint.ts @@ -20,7 +20,7 @@ import {findConfiguration as config} from "./configuration"; const moduleDirectory = path.dirname(module.filename); class Linter { - public static VERSION = "3.0.0-dev.3"; + public static VERSION = "3.0.0"; public static findConfiguration = config; private fileName: string; From 30cc400eab0164ceab35868bcfbd5902f63a1de5 Mon Sep 17 00:00:00 2001 From: nikklassen Date: Mon, 16 Nov 2015 14:12:35 -0500 Subject: [PATCH 06/17] Style fixes and better clarity --- README.md | 2 +- src/rules/memberAccessRule.ts | 10 +++------- test/rules/memberAccessTests.ts | 9 ++++----- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 48663e2a98b..3ac571bbf7e 100644 --- a/README.md +++ b/README.md @@ -152,7 +152,7 @@ A sample configuration file with all options is available [here](https://github. * `label-undefined` checks that labels are defined before usage. * `max-line-length` sets the maximum length of a line. * `member-access` enforces using explicit visibility on class members - * `"check-accessor"` enforces explicit visibility on accessors (can only be public) + * `"check-accessor"` enforces explicit visibility on get/set accessors (can only be public) * `"check-constructor"` enforces explicit visibility on constructors (can only be public) * `member-ordering` enforces member ordering. Rule options: * `public-before-private` All public members must be declared before private members diff --git a/src/rules/memberAccessRule.ts b/src/rules/memberAccessRule.ts index 8bc038e9c81..869509dbf11 100644 --- a/src/rules/memberAccessRule.ts +++ b/src/rules/memberAccessRule.ts @@ -26,17 +26,12 @@ export class Rule extends Lint.Rules.AbstractRule { } export class MemberAccessWalker extends Lint.RuleWalker { - private validateConstructors: boolean; - private validateAccessors: boolean; - constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { super(sourceFile, options); - this.validateConstructors = this.hasOption("check-constructor"); - this.validateAccessors = this.hasOption("check-accessor"); } public visitConstructorDeclaration(node: ts.ConstructorDeclaration) { - if (this.validateConstructors) { + if (this.hasOption("check-constructor")) { // constructor is only allowed to have public or nothing, but the compiler will catch this this.validateVisibilityModifiers(node); } @@ -51,10 +46,11 @@ export class MemberAccessWalker extends Lint.RuleWalker { public visitPropertyDeclaration(node: ts.PropertyDeclaration) { this.validateVisibilityModifiers(node); + super.visitMethodDeclaration(node); } public visitGetAccessor(node: ts.AccessorDeclaration) { - if (this.validateAccessors) { + if (this.hasOption("check-accessor")) { this.validateVisibilityModifiers(node); } super.visitGetAccessor(node); diff --git a/test/rules/memberAccessTests.ts b/test/rules/memberAccessTests.ts index 4b0c17843eb..dc6d6b651d1 100644 --- a/test/rules/memberAccessTests.ts +++ b/test/rules/memberAccessTests.ts @@ -16,7 +16,6 @@ import * as Lint from "../lint"; describe("", () => { - it("ensures that class properties have access modifiers", () => { const fileName = "rules/memberaccess.test.ts"; const expectedFailures = [ @@ -27,7 +26,7 @@ describe("", () => { [[29, 9], [29, 19]] ]; - checkFile(fileName, expectedFailures); + assertFailuresInFile(fileName, expectedFailures); }); it("ensures that constructors have access modifiers", () => { @@ -38,7 +37,7 @@ describe("", () => { ]; const options = [true, "check-constructor"]; - checkFile(fileName, expectedFailures, options); + assertFailuresInFile(fileName, expectedFailures, options); }); it("ensures that accessors have access modifiers", () => { @@ -49,10 +48,10 @@ describe("", () => { ]; const options = [true, "check-accessor"]; - checkFile(fileName, expectedFailures, options); + assertFailuresInFile(fileName, expectedFailures, options); }); - function checkFile(fileName: string, expectedFailures: number[][][], options: any[] = [true]) { + function assertFailuresInFile(fileName: string, expectedFailures: number[][][], options: any[] = [true]) { const MemberAccessRule = Lint.Test.getRule("member-access"); const createFailure = Lint.Test.createFailuresOnFile(fileName, MemberAccessRule.FAILURE_STRING); const expectedFileFailures = expectedFailures.map(failure => createFailure(failure[0], failure[1])); From 42c2b38a442dd9c953a93fe2d11442fe72ac1467 Mon Sep 17 00:00:00 2001 From: nikklassen Date: Mon, 16 Nov 2015 14:17:49 -0500 Subject: [PATCH 07/17] Fix super call and option check --- src/rules/memberAccessRule.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rules/memberAccessRule.ts b/src/rules/memberAccessRule.ts index 869509dbf11..532fd916602 100644 --- a/src/rules/memberAccessRule.ts +++ b/src/rules/memberAccessRule.ts @@ -46,7 +46,7 @@ export class MemberAccessWalker extends Lint.RuleWalker { public visitPropertyDeclaration(node: ts.PropertyDeclaration) { this.validateVisibilityModifiers(node); - super.visitMethodDeclaration(node); + super.visitPropertyDeclaration(node); } public visitGetAccessor(node: ts.AccessorDeclaration) { @@ -57,7 +57,7 @@ export class MemberAccessWalker extends Lint.RuleWalker { } public visitSetAccessor(node: ts.AccessorDeclaration) { - if (this.validateAccessors) { + if (this.hasOption("check-accessor")) { this.validateVisibilityModifiers(node); } super.visitSetAccessor(node); From 67b86f86ac7274dc238eff20c942382ae8d9ce34 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Wed, 18 Nov 2015 13:29:46 -0500 Subject: [PATCH 08/17] Fix failing test, accept no-unused-variable false positive for JSX spreads --- test/tsxTests.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/tsxTests.ts b/test/tsxTests.ts index 3a7ac546cb4..6a3f732e1b7 100644 --- a/test/tsxTests.ts +++ b/test/tsxTests.ts @@ -83,7 +83,8 @@ describe("TSX syntax", () => { }); it("with no false positives", () => { - assert.lengthOf(actualFailures, 3); + // TODO: one false positive exists because of a compiler bug (see #806) + assert.lengthOf(actualFailures, 4); }); }); From 8621a3f05c89104013107bf18708ea0e34883ddb Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Wed, 18 Nov 2015 13:29:59 -0500 Subject: [PATCH 09/17] Turn off comment-format check-lowercase rule in our codebase --- tslint.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tslint.json b/tslint.json index f2a56af03ee..5678fb131c7 100644 --- a/tslint.json +++ b/tslint.json @@ -1,7 +1,7 @@ { "rules": { "class-name": true, - "comment-format": [true, "check-lowercase", "check-space"], + "comment-format": [true, "check-space"], "curly": true, "eofline": true, "forin": true, From d9022cfc619b4806bc2c973318c9f4460ba4b2dd Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Wed, 18 Nov 2015 13:56:38 -0500 Subject: [PATCH 10/17] Update readme with 3.0 changes - explain peer dependency - update custom rules documentation --- README.md | 169 +++++++++++++++++++++++++++++++----------------------- 1 file changed, 97 insertions(+), 72 deletions(-) diff --git a/README.md b/README.md index ad6bb662588..5cefe81cbc1 100644 --- a/README.md +++ b/README.md @@ -14,84 +14,100 @@ Installation ##### CLI -```npm install tslint -g``` +``` +npm install tslint -g +npm install typescript -g +``` ##### Library -```npm install tslint``` +``` +npm install tslint +npm install typescript +``` -##### `next` distribution +##### Peer dependencies -The [`next` branch of the TSLint repo](https://github.com/palantir/tslint/tree/next) tracks the latest TypeScript -compiler and allows you to lint TS code that uses the latest features of the language. Releases from this branch -are published to npm with the `next` dist-tag, so you can get the latest dev version of TSLint via -`npm install tslint@next`. +The `typescript` module is a peer dependency of TSLint, which allows you to update the compiler independently from the +linter. + +Keep in mind that NPM v3 tries to flatten dependency trees so `tslint` will likely have to use the same version of `tsc` +used to actaully compile your sources. This means that breaking changes in the latest dev release of `typescript@next` +might break something in the linter if we haven't built against that release yet. If this happens to you, you can try: +1. picking up `tslint@next`, which may have some bugfixes not released in `tslint@latest` + (see [release notes here](https://github.com/palantir/tslint/releases)). +2. rolling back `typescript` to a known working version (NPM v3 tries to flatten dependency trees so `tslint` will + likely have to use the same version of `tsc` used to actaully compile your sources). Usage ----- -Please first ensure that the TypeScript source files compile correctly. +Please ensure that the TypeScript source files compile correctly _before_ running the linter. ##### CLI - usage: `tslint [options] [file ...]` +usage: `tslint [options] [file ...]` - Options: +Options: - -c, --config configuration file - -o, --out output file - -r, --rules-dir rules directory - -s, --formatters-dir formatters directory - -t, --format output format (prose, json) [default: "prose"] +``` +-c, --config configuration file +-o, --out output file +-r, --rules-dir rules directory +-s, --formatters-dir formatters directory +-t, --format output format (prose, json) [default: "prose"] +``` By default, configuration is loaded from `tslint.json`, if it exists in the current path, or the user's home directory, in that order. tslint accepts the following command-line options: - -c, --config: - The location of the configuration file that tslint will use to - determine which rules are activated and what options to provide - to the rules. If no option is specified, the config file named - tslint.json is used, so long as it exists in the path. - The format of the file is { rules: { /* rules list */ } }, - where /* rules list */ is a key: value comma-seperated list of - rulename: rule-options pairs. Rule-options can be either a - boolean true/false value denoting whether the rule is used or not, - or a list [boolean, ...] where the boolean provides the same role - as in the non-list case, and the rest of the list are options passed - to the rule that will determine what it checks for (such as number - of characters for the max-line-length rule, or what functions to ban - for the ban rule). - - -o, --out: - A filename to output the results to. By default, tslint outputs to - stdout, which is usually the console where you're running it from. - - -r, --rules-dir: - An additional rules directory, for user-created rules. - tslint will always check its default rules directory, in - node_modules/tslint/build/rules, before checking the user-provided - rules directory, so rules in the user-provided rules directory - with the same name as the base rules will not be loaded. - - -s, --formatters-dir: - An additional formatters directory, for user-created formatters. - Formatters are files that will format the tslint output, before - writing it to stdout or the file passed in --out. The default - directory, node_modules/tslint/build/formatters, will always be - checked first, so user-created formatters with the same names - as the base formatters will not be loaded. - - -t, --format: - The formatter to use to format the results of the linter before - outputting it to stdout or the file passed in --out. The core - formatters are prose (human readable) and json (machine readable), - and prose is the default if this option is not used. Additional - formatters can be added and used if the --formatters-dir option - is set. - - --help: - Prints this help message. +``` +-c, --config: + The location of the configuration file that tslint will use to + determine which rules are activated and what options to provide + to the rules. If no option is specified, the config file named + tslint.json is used, so long as it exists in the path. + The format of the file is { rules: { /* rules list */ } }, + where /* rules list */ is a key: value comma-seperated list of + rulename: rule-options pairs. Rule-options can be either a + boolean true/false value denoting whether the rule is used or not, + or a list [boolean, ...] where the boolean provides the same role + as in the non-list case, and the rest of the list are options passed + to the rule that will determine what it checks for (such as number + of characters for the max-line-length rule, or what functions to ban + for the ban rule). + +-o, --out: + A filename to output the results to. By default, tslint outputs to + stdout, which is usually the console where you're running it from. + +-r, --rules-dir: + An additional rules directory, for user-created rules. + tslint will always check its default rules directory, in + node_modules/tslint/build/rules, before checking the user-provided + rules directory, so rules in the user-provided rules directory + with the same name as the base rules will not be loaded. + +-s, --formatters-dir: + An additional formatters directory, for user-created formatters. + Formatters are files that will format the tslint output, before + writing it to stdout or the file passed in --out. The default + directory, node_modules/tslint/build/formatters, will always be + checked first, so user-created formatters with the same names + as the base formatters will not be loaded. + +-t, --format: + The formatter to use to format the results of the linter before + outputting it to stdout or the file passed in --out. The core + formatters are prose (human readable) and json (machine readable), + and prose is the default if this option is not used. Additional + formatters can be added and used if the --formatters-dir option + is set. + +--help: + Prints this help message. +``` ##### Library @@ -251,13 +267,15 @@ Custom Rules ------------ TSLint ships with a set of core rules that can be configured. However, users are also allowed to write their own rules, which allows them to enforce specific behavior not covered by the core of TSLint. TSLint's internal rules are itself written to be pluggable, so adding a new rule is as simple as creating a new rule file named by convention. New rules can be written in either TypeScript or Javascript; if written in TypeScript, the code must be compiled to Javascript before invoking TSLint. -Rule names are always camel-cased and *must* contain the suffix `Rule`. Let us take the example of how to write a new rule to forbid all import statements (you know, *for science*). Let us name the rule file `noImportsRule.ts`. Rules can be referenced in `tslint.json` in their dasherized forms, so `"no-imports": true` would turn on the rule. +Rule names are always camel-cased and *must* contain the suffix `Rule`. Let us take the example of how to write a new rule to forbid all import statements (you know, *for science*). Let us name the rule file `noImportsRule.ts`. Rules can be referenced in `tslint.json` in their kebab-case forms, so `"no-imports": true` would turn on the rule. -Now, let us first write the rule in TypeScript. At the top, we reference TSLint's [definition file](https://github.com/palantir/tslint/blob/master/lib/tslint.d.ts) and the [definition file](https://github.com/palantir/tslint/blob/master/typings/typescriptServices.d.ts) for TypeScript's language services. The exported class must always be named `Rule` and extend from `Lint.Rules.AbstractRule`. +Now, let us first write the rule in TypeScript. A few things to note: +- We import `tslint/lib/lint` to get the whole `Lint` namespace instead of just the `Linter` class. +- The exported class must always be named `Rule` and extend from `Lint.Rules.AbstractRule`. ```typescript -/// -/// +import * as ts from "typescript"; +import * as Lint from "tslint/lib/lint"; export class Rule extends Lint.Rules.AbstractRule { public static FAILURE_STRING = "import statement forbidden"; @@ -266,11 +284,8 @@ export class Rule extends Lint.Rules.AbstractRule { return this.applyWithWalker(new NoImportsWalker(sourceFile, this.getOptions())); } } -``` -The walker takes care of all the work. - -```typescript +// The walker takes care of all the work. class NoImportsWalker extends Lint.RuleWalker { public visitImportDeclaration(node: ts.ImportDeclaration) { // create a failure at the current position @@ -286,9 +301,12 @@ Given a walker, TypeScript's parser visits the AST using the visitor pattern. So We still need to hook up this new rule to TSLint. First make sure to compile `noImportsRule.ts`: `tsc -m commonjs --noImplicitAny noImportsRule.ts tslint.d.ts`. Then, if using the CLI, provide the directory that contains this rule as an option to `--rules-dir`. If using TSLint as a library or via `grunt-tslint`, the `options` hash must contain `"rulesDirectory": "..."`. If you run the linter, you'll see that we have now successfully banned all import statements via TSLint! -Now, let us rewrite the same rule in Javascript. +Now, let us rewrite the same rule in JavaScript. ```javascript +import "typescript"; +import * as Lint from "tslint/lib/lint"; + function Rule() { Lint.Rules.AbstractRule.apply(this, arguments); } @@ -323,8 +341,8 @@ Custom Formatters Just like rules, additional formatters can also be supplied to TSLint via `--formatters-dir` on the CLI or `formattersDirectory` option on the library or `grunt-tslint`. Writing a new formatter is simpler than writing a new rule, as shown in the JSON formatter's code. ```typescript -/// -/// +import * as ts from "typescript"; +import * as Lint from "tslint/lib/lint"; export class Formatter extends Lint.Formatters.AbstractFormatter { public format(failures: Lint.RuleFailure[]): string { @@ -347,11 +365,18 @@ npm install grunt ``` +#### `next` branch + +The [`next` branch of the TSLint repo](https://github.com/palantir/tslint/tree/next) tracks the latest TypeScript +compiler as a `devDependency`. This allows you to develop the linter and its rules against the latest features of the +language. Releases from this branch are published to npm with the `next` dist-tag, so you can get the latest dev +version of TSLint via `npm install tslint@next`. + Creating a new Release ---------------------- -1. Bump up the version number in package.json and tslint.ts -2. Add a section for the new release in CHANGELOG.md +1. Bump up the version number in `package.json` and `tslint.ts` +2. Add a section for the new release in `CHANGELOG.md` 3. Run `grunt` to build the latest sources 4. Commit 5. Run `npm publish` From 4f61a7fcb7246f986ec0f63383ddf63ab7e7fc81 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Wed, 18 Nov 2015 14:17:47 -0500 Subject: [PATCH 11/17] Minor readme formatting edits --- README.md | 89 +++++++++++++++++++++++++++---------------------------- 1 file changed, 44 insertions(+), 45 deletions(-) diff --git a/README.md b/README.md index 5cefe81cbc1..8b71227918e 100644 --- a/README.md +++ b/README.md @@ -145,19 +145,19 @@ A sample configuration file with all options is available [here](https://github. * `"parameters"` checks alignment of function parameters. * `"arguments"` checks alignment of function call arguments. * `"statements"` checks alignment of statements. -* `ban` bans the use of specific functions. Options are ["object", "function"] pairs that ban the use of object.function() +* `ban` bans the use of specific functions. Options are ["object", "function"] pairs that ban the use of object.function(). * `class-name` enforces PascalCased class and interface names. * `comment-format` enforces rules for single-line comments. Rule options: * `"check-space"` enforces the rule that all single-line comments must begin with a space, as in `// comment` * note that comments starting with `///` are also allowed, for things such as `///` - * `"check-lowercase"` enforces the rule that the first non-whitespace character of a comment must be lowercase, if applicable - * `"check-uppercase"` enforces the rule that the first non-whitespace character of a comment must be uppercase, if applicable + * `"check-lowercase"` enforces the rule that the first non-whitespace character of a comment must be lowercase, if applicable. + * `"check-uppercase"` enforces the rule that the first non-whitespace character of a comment must be uppercase, if applicable. * `curly` enforces braces for `if`/`for`/`do`/`while` statements. * `eofline` enforces the file to end with a newline. * `forin` enforces a `for ... in` statement to be filtered with an `if` statement.* * `indent` enforces indentation with tabs or spaces. Rule options (one is required): - * `"tabs"` enforces consistent tabs - * `"spaces"` enforces consistent spaces + * `"tabs"` enforces consistent tabs. + * `"spaces"` enforces consistent spaces. * `interface-name` enforces the rule that interface names must begin with a capital 'I' * `jsdoc-format` enforces basic format rules for jsdoc comments -- comments starting with `/**` * each line contains an asterisk and asterisks must be aligned @@ -169,9 +169,9 @@ A sample configuration file with all options is available [here](https://github. * `max-line-length` sets the maximum length of a line. * `member-access` enforces using explicit visibility on class members * `member-ordering` enforces member ordering. Rule options: - * `public-before-private` All public members must be declared before private members - * `static-before-instance` All static members must be declared before instance members - * `variables-before-functions` All variables needs to be declared before functions + * `public-before-private` All public members must be declared before private members. + * `static-before-instance` All static members must be declared before instance members. + * `variables-before-functions` All variables needs to be declared before functions. * `no-any` diallows usages of `any` as a type decoration. * `no-arg` disallows access to `arguments.callee`. * `no-bitwise` disallows bitwise operators. @@ -185,9 +185,9 @@ A sample configuration file with all options is available [here](https://github. * `no-duplicate-variable` disallows duplicate variable declarations in the same block scope. * `no-empty` disallows empty blocks. * `no-eval` disallows `eval` function invocations. -* `no-inferrable-types` disallows explicit type declarations for variables or parameters initialized to a number, string, or boolean -* `no-internal-module` disallows internal `module`, use `namespace` instead. -* `no-require-imports` disallows require() style imports. +* `no-inferrable-types` disallows explicit type declarations for variables or parameters initialized to a number, string, or boolean. +* `no-internal-module` disallows internal `module` (use `namespace` instead). +* `no-require-imports` disallows `require()` style imports (use ES6-style imports instead). * `no-shadowed-variable` disallows shadowed variable declarations. * `no-string-literal` disallows object access via string literals. * `no-switch-case-fall-through` disallows falling through case statements. @@ -197,58 +197,57 @@ A sample configuration file with all options is available [here](https://github. * `no-unused-variable` disallows unused imports, variables, functions and private class members. * `"check-parameters"` disallows unused function and constructor parameters. * NOTE: this option is experimental and does not work with classes that use abstract method declarations, among other things. Use at your own risk. - * `"react"` relaxes the rule for a namespace import named `React` (from either the module `"react"` or `"react/addons"`) to also consider JSX expressions - uses of the import + * `"react"` relaxes the rule for a namespace import named `React` (from either the module `"react"` or `"react/addons"`) to also consider JSX expressions uses of the import. * `no-use-before-declare` disallows usage of variables before their declaration. * `no-var-keyword` disallows usage of the `var` keyword, use `let` or `const` instead. -* `no-var-requires` disallows the use of require statements except in import statements, banning the use of forms such as `var module = require("module")` -* `object-literal-sort-keys` checks that keys in object literals are declared in alphabetical order +* `no-var-requires` disallows the use of require statements except in import statements, banning the use of forms such as `var module = require("module")`. +* `object-literal-sort-keys` checks that keys in object literals are declared in alphabetical order (useful to prevent merge conflicts). * `one-line` enforces the specified tokens to be on the same line as the expression preceding it. Rule options: - * `"check-catch"` checks that `catch` is on the same line as the closing brace for `try` - * `"check-else"` checks that `else` is on the same line as the closing brace for `if` + * `"check-catch"` checks that `catch` is on the same line as the closing brace for `try`. + * `"check-else"` checks that `else` is on the same line as the closing brace for `if`. * `"check-open-brace"` checks that an open brace falls on the same line as its preceding expression. * `"check-whitespace"` checks preceding whitespace for the specified tokens. * `quotemark` enforces consistent single or double quoted string literals. Rule options (at least one of `"double"` or `"single"` is required): - * `"single"` enforces single quotes - * `"double"` enforces double quotes + * `"single"` enforces single quotes. + * `"double"` enforces double quotes. * `"avoid-escape"` allows you to use the "other" quotemark in cases where escaping would normally be required. For example, `[true, "double", "avoid-escape"]` would not report a failure on the string literal `'Hello "World"'`. -* `radix` enforces the radix parameter of `parseInt` +* `radix` enforces the radix parameter of `parseInt`. * `semicolon` enforces semicolons at the end of every statement. * `switch-default` enforces a `default` case in `switch` statements. * `trailing-comma` enforces or disallows trailing comma within array and object literals, destructuring assignment and named imports. Each rule option requires a value of `"always"` or `"never"`. Rule options: - * `"multiline"` checks multi-line object literals - * `"singleline"` checks single-line object literals + * `"multiline"` checks multi-line object literals. + * `"singleline"` checks single-line object literals. * `triple-equals` enforces === and !== in favor of == and !=. * `typedef` enforces type definitions to exist. Rule options: - * `"call-signature"` checks return type of functions - * `"parameter"` checks type specifier of function parameters - * `"property-declaration"` checks return types of interface properties - * `"variable-declaration"` checks variable declarations - * `"member-variable-declaration"` checks member variable declarations + * `"call-signature"` checks return type of functions. + * `"parameter"` checks type specifier of function parameters. + * `"property-declaration"` checks return types of interface properties. + * `"variable-declaration"` checks variable declarations. + * `"member-variable-declaration"` checks member variable declarations. * `typedef-whitespace` enforces spacing whitespace for type definitions. Each rule option requires a value of `"space"` or `"nospace"` to require a space or no space before the type specifier's colon. Rule options: - * `"call-signature"` checks return type of functions - * `"index-signature"` checks index type specifier of indexers - * `"parameter"` checks function parameters - * `"property-declaration"` checks object property declarations - * `"variable-declaration"` checks variable declaration -* `use-strict` enforces ECMAScript 5's strict mode - * `check-module` checks that all top-level modules are using strict mode - * `check-function` checks that all top-level functions are using strict mode + * `"call-signature"` checks return type of functions. + * `"index-signature"` checks index type specifier of indexers. + * `"parameter"` checks function parameters. + * `"property-declaration"` checks object property declarations. + * `"variable-declaration"` checks variable declaration. +* `use-strict` enforces ECMAScript 5's strict mode. + * `check-module` checks that all top-level modules are using strict mode. + * `check-function` checks that all top-level functions are using strict mode. * `variable-name` checks variables names for various errors. Rule options: * `"check-format"`: allows only camelCased or UPPER_CASED variable names - * `"allow-leading-underscore"` allows underscores at the beginnning - * `"allow-trailing-underscore"` allows underscores at the end - * `"ban-keywords"`: disallows the use of certain TypeScript keywords (`any`, `Number`, `number`, `String`, `string`, `Boolean`, `boolean`, `undefined`) as variable or parameter names + * `"allow-leading-underscore"` allows underscores at the beginning. + * `"allow-trailing-underscore"` allows underscores at the end. + * `"ban-keywords"`: disallows the use of certain TypeScript keywords (`any`, `Number`, `number`, `String`, `string`, `Boolean`, `boolean`, `undefined`) as variable or parameter names. * `whitespace` enforces spacing whitespace. Rule options: - * `"check-branch"` checks branching statements (`if`/`else`/`for`/`while`) are followed by whitespace - * `"check-decl"`checks that variable declarations have whitespace around the equals token - * `"check-operator"` checks for whitespace around operator tokens - * `"check-module"` checks for whitespace in import & export statements - * `"check-separator"` checks for whitespace after separator tokens (`,`/`;`) - * `"check-type"` checks for whitespace before a variable type specification - * `"check-typecast"` checks for whitespace between a typecast and its target + * `"check-branch"` checks branching statements (`if`/`else`/`for`/`while`) are followed by whitespace. + * `"check-decl"`checks that variable declarations have whitespace around the equals token. + * `"check-operator"` checks for whitespace around operator tokens. + * `"check-module"` checks for whitespace in import & export statements. + * `"check-separator"` checks for whitespace after separator tokens (`,`/`;`). + * `"check-type"` checks for whitespace before a variable type specification. + * `"check-typecast"` checks for whitespace between a typecast and its target. TSLint Rule Flags ----- From 8376a6a04b7fbd86f6e6aa7aef1fe2f819ce3c24 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Thu, 19 Nov 2015 19:05:40 -0500 Subject: [PATCH 12/17] Readme fixes --- README.md | 49 ++++++++----------------------------------------- 1 file changed, 8 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index 8b71227918e..b26190fb675 100644 --- a/README.md +++ b/README.md @@ -29,15 +29,12 @@ npm install typescript ##### Peer dependencies The `typescript` module is a peer dependency of TSLint, which allows you to update the compiler independently from the -linter. +linter. This also means that `tslint` will have to use the same version of `tsc` used to actually compile your sources. -Keep in mind that NPM v3 tries to flatten dependency trees so `tslint` will likely have to use the same version of `tsc` -used to actaully compile your sources. This means that breaking changes in the latest dev release of `typescript@next` -might break something in the linter if we haven't built against that release yet. If this happens to you, you can try: +Breaking changes in the latest dev release of `typescript@next` might break something in the linter if we haven't built against that release yet. If this happens to you, you can try: 1. picking up `tslint@next`, which may have some bugfixes not released in `tslint@latest` (see [release notes here](https://github.com/palantir/tslint/releases)). -2. rolling back `typescript` to a known working version (NPM v3 tries to flatten dependency trees so `tslint` will - likely have to use the same version of `tsc` used to actaully compile your sources). +2. rolling back `typescript` to a known working version. Usage ----- @@ -194,10 +191,10 @@ A sample configuration file with all options is available [here](https://github. * `no-trailing-whitespace` disallows trailing whitespace at the end of a line. * `no-unreachable` disallows unreachable code after `break`, `catch`, `throw`, and `return` statements. * `no-unused-expression` disallows unused expression statements, that is, expression statements that are not assignments or function invocations (and thus no-ops). -* `no-unused-variable` disallows unused imports, variables, functions and private class members. +* `no-unused-variable` disallows unused imports, variables, functions and private class members. Rule options: * `"check-parameters"` disallows unused function and constructor parameters. * NOTE: this option is experimental and does not work with classes that use abstract method declarations, among other things. Use at your own risk. - * `"react"` relaxes the rule for a namespace import named `React` (from either the module `"react"` or `"react/addons"`) to also consider JSX expressions uses of the import. + * `"react"` relaxes the rule for a namespace import named `React` (from either the module `"react"` or `"react/addons"`). Any JSX expression in the file will be treated as a usage of `React` (because it expands to `React.createElement`). * `no-use-before-declare` disallows usage of variables before their declaration. * `no-var-keyword` disallows usage of the `var` keyword, use `let` or `const` instead. * `no-var-requires` disallows the use of require statements except in import statements, banning the use of forms such as `var module = require("module")`. @@ -300,40 +297,10 @@ Given a walker, TypeScript's parser visits the AST using the visitor pattern. So We still need to hook up this new rule to TSLint. First make sure to compile `noImportsRule.ts`: `tsc -m commonjs --noImplicitAny noImportsRule.ts tslint.d.ts`. Then, if using the CLI, provide the directory that contains this rule as an option to `--rules-dir`. If using TSLint as a library or via `grunt-tslint`, the `options` hash must contain `"rulesDirectory": "..."`. If you run the linter, you'll see that we have now successfully banned all import statements via TSLint! -Now, let us rewrite the same rule in JavaScript. +Final notes: -```javascript -import "typescript"; -import * as Lint from "tslint/lib/lint"; - -function Rule() { - Lint.Rules.AbstractRule.apply(this, arguments); -} - -Rule.prototype = Object.create(Lint.Rules.AbstractRule.prototype); -Rule.prototype.apply = function(sourceFile) { - return this.applyWithWalker(new NoImportsWalker(sourceFile, this.getOptions())); -}; - -function NoImportsWalker() { - Lint.RuleWalker.apply(this, arguments); -} - -NoImportsWalker.prototype = Object.create(Lint.RuleWalker.prototype); -NoImportsWalker.prototype.visitImportDeclaration = function (node) { - // create a failure at the current position - this.addFailure(this.createFailure(node.getStart(), node.getWidth(), "import statement forbidden")); - - // call the base version of this visitor to actually parse this node - Lint.RuleWalker.prototype.visitImportDeclaration.call(this, node); -}; - -exports.Rule = Rule; -``` - -As you can see, it's a pretty straightforward translation from the equivalent TypeScript code. - -Finally, core rules cannot be overwritten with a custom implementation, and rules can also take in options (retrieved via `this.getOptions()`). +- Core rules cannot be overwritten with a custom implementation. +- Custom rules can also take in options just like core rules (retrieved via `this.getOptions()`). Custom Formatters ----------------- From 116ada4d30fe841ca22dfecbca63889aaed72e2b Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Thu, 19 Nov 2015 19:08:07 -0500 Subject: [PATCH 13/17] Update changelog to include #801 --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ef78bd7722e..992d43f2bde 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,9 @@ Change Log v3.0.0 --- -* All the changes from the following releases, including some **breaking change**: +* [bugfix] `member-access` rule now handles object literals and get/set accessors properly (#801) + * New rule options: `check-accessor` and `check-constructor` +* All the changes from the following releases, including some **breaking changes**: * `3.0.0-dev.3` * `3.0.0-dev.2` * `3.0.0-dev.1` From 4a750f744aa39dcc80d3cb407e1b266eec3128f9 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Thu, 19 Nov 2015 20:22:50 -0500 Subject: [PATCH 14/17] Fix a minor markdown formatting issue in readme --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index b26190fb675..813ac736f70 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,7 @@ The `typescript` module is a peer dependency of TSLint, which allows you to upda linter. This also means that `tslint` will have to use the same version of `tsc` used to actually compile your sources. Breaking changes in the latest dev release of `typescript@next` might break something in the linter if we haven't built against that release yet. If this happens to you, you can try: + 1. picking up `tslint@next`, which may have some bugfixes not released in `tslint@latest` (see [release notes here](https://github.com/palantir/tslint/releases)). 2. rolling back `typescript` to a known working version. From 569c6238c01bf61eb81968fb7cb1ae6b54b26af0 Mon Sep 17 00:00:00 2001 From: Jason Killian Date: Fri, 20 Nov 2015 11:03:21 -0500 Subject: [PATCH 15/17] Use correct rule name in rule disable --- test/formatters/jsonFormatterTests.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/formatters/jsonFormatterTests.ts b/test/formatters/jsonFormatterTests.ts index 246b86c6f29..04fc3d40694 100644 --- a/test/formatters/jsonFormatterTests.ts +++ b/test/formatters/jsonFormatterTests.ts @@ -36,7 +36,7 @@ describe("JSON Formatter", () => { new Lint.RuleFailure(sourceFile, 0, maxPosition, "full failure", "full-name") ]; - /* tslint:disable:sort-object-literal-keys */ + /* tslint:disable:object-literal-sort-keys */ const expectedResult = [{ name: TEST_FILE, failure: "first failure", @@ -82,7 +82,7 @@ describe("JSON Formatter", () => { }, ruleName: "full-name" }]; - /* tslint:enable:sort-object-literal-keys */ + /* tslint:enable:object-literal-sort-keys */ const actualResult = JSON.parse(formatter.format(failures)); assert.deepEqual(actualResult, expectedResult); From 026634fb95e0cb9ecdceb56e87105ec15f0e932c Mon Sep 17 00:00:00 2001 From: Lionel Besson Date: Tue, 13 Oct 2015 16:52:48 +0200 Subject: [PATCH 16/17] Fixes #727 --- src/language/utils.ts | 11 +++++-- src/rules/noShadowedVariableRule.ts | 30 +++++++++++++------ test/files/rules/no-shadowed-variable.test.ts | 14 +++++++++ test/rules/noShadowedVariableRuleTests.ts | 5 +++- 4 files changed, 47 insertions(+), 13 deletions(-) diff --git a/src/language/utils.ts b/src/language/utils.ts index 50dbac8a7c3..9fdba79c8c9 100644 --- a/src/language/utils.ts +++ b/src/language/utils.ts @@ -97,16 +97,21 @@ export function isBlockScopedVariable(node: ts.VariableDeclaration | ts.Variable } export function isBlockScopedBindingElement(node: ts.BindingElement): boolean { + const variableDeclaration = getBindingElementVariableDeclaration(node); + // if no variable declaration, it must be a function param, which is block scoped + return (variableDeclaration == null) || isBlockScopedVariable(variableDeclaration); +} + +export function getBindingElementVariableDeclaration(node: ts.BindingElement): ts.VariableDeclaration { let currentParent = node.parent; while (currentParent.kind !== ts.SyntaxKind.VariableDeclaration) { if (currentParent.parent == null) { - // if we didn't encounter a VariableDeclaration, this must be a function parameter, which is block scoped - return true; + return null; // function parameter, no variable declaration } else { currentParent = currentParent.parent; } } - return isBlockScopedVariable( currentParent); + return currentParent; } /** diff --git a/src/rules/noShadowedVariableRule.ts b/src/rules/noShadowedVariableRule.ts index f105595a863..46b51a40975 100644 --- a/src/rules/noShadowedVariableRule.ts +++ b/src/rules/noShadowedVariableRule.ts @@ -36,10 +36,15 @@ class NoShadowedVariableWalker extends Lint.BlockScopeAwareRuleWalker node.name, isBlockScoped); + if (variableDeclaration) { + const isBlockScopedVariable = Lint.isBlockScopedVariable(variableDeclaration); + this.handleSingleVariableIdentifier( node.name, isBlockScopedVariable); + } else { + this.handleSingleParameterIdentifier( node.name); + } } super.visitBindingElement(node); @@ -64,15 +69,11 @@ class NoShadowedVariableWalker extends Lint.BlockScopeAwareRuleWalker node.name; - const variableName = variableIdentifier.text; - const currentScope = this.getCurrentScope(); + const isSingleParameter = node.name.kind === ts.SyntaxKind.Identifier; - if (this.isVarInAnyScope(variableName)) { - this.addFailureOnIdentifier(variableIdentifier); + if (isSingleParameter) { + this.handleSingleParameterIdentifier( node.name); } - currentScope.varNames.push(variableName); super.visitParameterDeclaration(node); } @@ -112,6 +113,17 @@ class NoShadowedVariableWalker extends Lint.BlockScopeAwareRuleWalker scopeInfo.varNames.indexOf(varName) >= 0); } diff --git a/test/files/rules/no-shadowed-variable.test.ts b/test/files/rules/no-shadowed-variable.test.ts index 126727bcb61..36b63434301 100644 --- a/test/files/rules/no-shadowed-variable.test.ts +++ b/test/files/rules/no-shadowed-variable.test.ts @@ -114,3 +114,17 @@ interface FalsePositive4 { (parameters: T, runSynchonous: boolean): TResult; (parameters: T, callback: (error: Error, result: TResult) => void): void; } + +let p = 1; +function testParameterDestructuring( + { pos: p }: FalsePositive3, // failure + { pos: q, specular: r }: FalsePositive3, + { pos }: FalsePositive3 +) { + p = 2; + var q = 1; // failure + + function someInnerFunc() { + let pos = 3; // failure + } +} diff --git a/test/rules/noShadowedVariableRuleTests.ts b/test/rules/noShadowedVariableRuleTests.ts index ba24cf86131..3d8911fca60 100644 --- a/test/rules/noShadowedVariableRuleTests.ts +++ b/test/rules/noShadowedVariableRuleTests.ts @@ -41,7 +41,10 @@ describe("", () => { Lint.Test.createFailure(fileName, [90, 16], [90, 17], NoShadowedVariableRule.FAILURE_STRING + "z'"), Lint.Test.createFailure(fileName, [94, 15], [94, 16], NoShadowedVariableRule.FAILURE_STRING + "x'"), Lint.Test.createFailure(fileName, [95, 15], [95, 16], NoShadowedVariableRule.FAILURE_STRING + "y'"), - Lint.Test.createFailure(fileName, [97, 17], [97, 18], NoShadowedVariableRule.FAILURE_STRING + "z'") + Lint.Test.createFailure(fileName, [97, 17], [97, 18], NoShadowedVariableRule.FAILURE_STRING + "z'"), + Lint.Test.createFailure(fileName, [120, 12], [120, 13], NoShadowedVariableRule.FAILURE_STRING + "p'"), + Lint.Test.createFailure(fileName, [125, 9], [125, 10], NoShadowedVariableRule.FAILURE_STRING + "q'"), + Lint.Test.createFailure(fileName, [128, 13], [128, 16], NoShadowedVariableRule.FAILURE_STRING + "pos'") ]; const actualFailures = Lint.Test.applyRuleOnFile(fileName, NoShadowedVariableRule); From cea5910bd8fa892f8bd8f62d99211ac14530a708 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Wed, 25 Nov 2015 12:29:57 -0500 Subject: [PATCH 17/17] Prepare release v3.1.0-dev.1 --- CHANGELOG.md | 5 +++++ package.json | 8 ++++---- src/tslint.ts | 2 +- test/tsxTests.ts | 4 ++-- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 992d43f2bde..4c31e320107 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,11 @@ Change Log === +v3.1.0-dev.1 +--- +* [bugfix] fixed `no-shadowed-variable` false positives when handling destructuring in function params (#727) +- [enhancement] `rulesDirectory` in `tslint.json` now supports multiple file paths (#795) + v3.0.0 --- * [bugfix] `member-access` rule now handles object literals and get/set accessors properly (#801) diff --git a/package.json b/package.json index 7823a16c397..6e8be109f1f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "tslint", - "version": "3.0.0", + "version": "3.1.0-dev.1", "description": "a static analysis linter for TypeScript", "bin": { "tslint": "./bin/tslint" @@ -35,11 +35,11 @@ "grunt-ts": "^5.1.0", "grunt-tslint": "latest", "mocha": "^2.2.5", - "tslint": "latest", - "typescript": "latest" + "tslint": "next", + "typescript": "next" }, "peerDependencies": { - "typescript": ">=1.6.2" + "typescript": ">=1.7.0 || >=1.7.0-dev.20151003 || >=1.8.0-dev" }, "license": "Apache-2.0" } diff --git a/src/tslint.ts b/src/tslint.ts index 5910df2f33e..266cf59fc60 100644 --- a/src/tslint.ts +++ b/src/tslint.ts @@ -20,7 +20,7 @@ import {findConfiguration as config} from "./configuration"; const moduleDirectory = path.dirname(module.filename); class Linter { - public static VERSION = "3.0.0"; + public static VERSION = "3.1.0-dev.1"; public static findConfiguration = config; private fileName: string; diff --git a/test/tsxTests.ts b/test/tsxTests.ts index 6a3f732e1b7..a2b32f6c8b6 100644 --- a/test/tsxTests.ts +++ b/test/tsxTests.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import * as Lint from "./lint"; describe("TSX syntax", () => { @@ -83,8 +84,7 @@ describe("TSX syntax", () => { }); it("with no false positives", () => { - // TODO: one false positive exists because of a compiler bug (see #806) - assert.lengthOf(actualFailures, 4); + assert.lengthOf(actualFailures, 3); }); });