From fe8da93fe1746c948e0ce74dc232371c6f315b2c Mon Sep 17 00:00:00 2001 From: Alexander Schrab Date: Fri, 22 May 2015 09:55:28 +0200 Subject: [PATCH 01/21] New rule: memberNoDefaultAccess. Disallows public access modifiers that are not explicit in the class --- README.md | 1 + docs/sample.tslint.json | 1 + src/rules/memberNoDefaultAccessRule.ts | 70 +++++++++++++++++++ src/rules/tsconfig.json | 1 + .../files/rules/membernodefaultaccess.test.ts | 18 +++++ test/rules/memberNoDefaultAccessTests.ts | 30 ++++++++ test/tsconfig.json | 1 + 7 files changed, 122 insertions(+) create mode 100644 src/rules/memberNoDefaultAccessRule.ts create mode 100644 test/files/rules/membernodefaultaccess.test.ts create mode 100644 test/rules/memberNoDefaultAccessTests.ts diff --git a/README.md b/README.md index 1ddecd3d9a2..f45cf5192f5 100644 --- a/README.md +++ b/README.md @@ -144,6 +144,7 @@ A sample configuration file with all options is available [here](https://github. * `label-position` enforces labels only on sensible statements. * `label-undefined` checks that labels are defined before usage. * `max-line-length` sets the maximum length of a line. +* `member-no-default-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 diff --git a/docs/sample.tslint.json b/docs/sample.tslint.json index 685bb4260ec..fd92e63e5b1 100644 --- a/docs/sample.tslint.json +++ b/docs/sample.tslint.json @@ -5,6 +5,7 @@ "arguments", "statements"], "ban": false, + "member-no-default-access": true, "class-name": true, "comment-format": [true, "check-space", diff --git a/src/rules/memberNoDefaultAccessRule.ts b/src/rules/memberNoDefaultAccessRule.ts new file mode 100644 index 00000000000..008b4ba1a64 --- /dev/null +++ b/src/rules/memberNoDefaultAccessRule.ts @@ -0,0 +1,70 @@ +/* + * Copyright 2013 Palantir Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. +*/ + +export class Rule extends Lint.Rules.AbstractRule { + static FAILURE_STRING = "default access modifier on member/method not allowed"; + + public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + return this.applyWithWalker(new MemberAccessWalker(sourceFile, this.getOptions())); + } +} + +interface IModifiers { + hasAccessModifier: boolean; +} + +function getModifiers(modifiers?: ts.ModifiersArray): IModifiers { + let modifierStrings: string[] = []; + if (modifiers != null) { + modifierStrings = modifiers.map((x) => { + return x.getText(); + }); + } + + let hasAccessModifier = modifierStrings.indexOf("public") !== -1; + hasAccessModifier = hasAccessModifier || modifierStrings.indexOf("private") !== -1; + hasAccessModifier = hasAccessModifier || modifierStrings.indexOf("protected") !== -1; + + return { + hasAccessModifier: hasAccessModifier + }; +} + +export class MemberAccessWalker extends Lint.RuleWalker { + constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { + super(sourceFile, options); + } + + public visitMethodDeclaration(node: ts.MethodDeclaration): void { + this.checkModifiers(node, getModifiers(node.modifiers)); + super.visitMethodDeclaration(node); + } + + public visitPropertyDeclaration(node: ts.PropertyDeclaration): void { + this.checkModifiers(node, getModifiers(node.modifiers)); + super.visitPropertyDeclaration(node); + } + + private checkModifiers(node: ts.Node, current: IModifiers): void { + if (!this.followsRules(current)) { + this.addFailure(this.createFailure(node.getStart(), node.getWidth(), Rule.FAILURE_STRING)); + } + } + + private followsRules(current: IModifiers): boolean { + return current.hasAccessModifier; + } +} diff --git a/src/rules/tsconfig.json b/src/rules/tsconfig.json index 41803dab9b1..af4e25a2713 100644 --- a/src/rules/tsconfig.json +++ b/src/rules/tsconfig.json @@ -30,6 +30,7 @@ "./labelPositionRule.ts", "./labelUndefinedRule.ts", "./maxLineLengthRule.ts", + "./memberNoDefaultAccessRule.ts", "./memberOrderingRule.ts", "./noAnyRule.ts", "./noArgRule.ts", diff --git a/test/files/rules/membernodefaultaccess.test.ts b/test/files/rules/membernodefaultaccess.test.ts new file mode 100644 index 00000000000..ec05e491b31 --- /dev/null +++ b/test/files/rules/membernodefaultaccess.test.ts @@ -0,0 +1,18 @@ +class Foo { + constructor() { + } + + public w: number; + private x: number; + protected y: number; + z: number; + + public barW(): any { + } + private barX(): any { + } + protected barY(): any { + } + barZ(): any { + } +} diff --git a/test/rules/memberNoDefaultAccessTests.ts b/test/rules/memberNoDefaultAccessTests.ts new file mode 100644 index 00000000000..6beb70f6950 --- /dev/null +++ b/test/rules/memberNoDefaultAccessTests.ts @@ -0,0 +1,30 @@ +/* + * Copyright 2013 Palantir Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +describe("", () => { + it("disallow default access on member", () => { + let fileName = "rules/membernodefaultaccess.test.ts"; + let MemberNoDefaultAccessRule = Lint.Test.getRule("member-no-default-access"); + let actualFailures = Lint.Test.applyRuleOnFile(fileName, MemberNoDefaultAccessRule); + + Lint.Test.assertFailuresEqual(actualFailures, [ + Lint.Test.createFailure(fileName, [8, 5], [8, 15], + MemberNoDefaultAccessRule.FAILURE_STRING), + Lint.Test.createFailure(fileName, [16, 5], [17, 6], + MemberNoDefaultAccessRule.FAILURE_STRING) + ]); + }); +}); diff --git a/test/tsconfig.json b/test/tsconfig.json index a6c47d077a4..8ebe11a30dc 100644 --- a/test/tsconfig.json +++ b/test/tsconfig.json @@ -47,6 +47,7 @@ "./rules/labelPositionRuleTests.ts", "./rules/labelUndefinedRuleTests.ts", "./rules/maxLineLengthRuleTests.ts", + "./rules/memberNoDefaultAccessTests.ts", "./rules/memberOrderingRuleTests.ts", "./rules/noAnyRuleTests.ts", "./rules/noArgRuleTests.ts", From 99d25da6f21accf09f9b0e443a9833e692319c61 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Thu, 6 Aug 2015 15:58:45 -0400 Subject: [PATCH 02/21] Add docs about "next" distribution to readme --- README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/README.md b/README.md index 1ddecd3d9a2..107bd4c433e 100644 --- a/README.md +++ b/README.md @@ -9,6 +9,14 @@ TSLint A linter for the TypeScript language. +"next" distribution +------------------- + +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`. + Installation ------------ From 4e574ea2175ecac19b15aa6e8e309e6ba29a2c43 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Thu, 6 Aug 2015 16:42:15 -0400 Subject: [PATCH 03/21] Move "next" distribution docs into Installation section --- README.md | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 107bd4c433e..acc79947c24 100644 --- a/README.md +++ b/README.md @@ -9,14 +9,6 @@ TSLint A linter for the TypeScript language. -"next" distribution -------------------- - -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`. - Installation ------------ @@ -28,6 +20,13 @@ Installation ```npm install tslint``` +##### `next` distribution + +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`. + Usage ----- From c510503fcdf87efe60bfba0e0b63e422b4a86f66 Mon Sep 17 00:00:00 2001 From: Phips Peter Date: Fri, 7 Aug 2015 15:40:11 -0700 Subject: [PATCH 04/21] Add TypeScript definition This will allow `tsd` 0.6 to discover the file --- package.json | 3 +++ 1 file changed, 3 insertions(+) diff --git a/package.json b/package.json index 573041fc8d6..acf6c07697c 100644 --- a/package.json +++ b/package.json @@ -6,6 +6,9 @@ "tslint": "./bin/tslint" }, "main": "./lib/tslint", + "typescript": { + "definition": "lib/tslint.d.ts" + }, "repository": { "type": "git", "url": "https://github.com/palantir/tslint.git" From 578ebbcb369082ca872085b3399ecc7000d018eb Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Thu, 13 Aug 2015 13:47:56 -0400 Subject: [PATCH 05/21] Use latest tslint, typescript, and grunt-tslint Fixes #503 --- package.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index acf6c07697c..06d094bd5c0 100644 --- a/package.json +++ b/package.json @@ -36,10 +36,10 @@ "grunt-mocha-test": "^0.12.7", "grunt-run": "^0.3.0", "grunt-ts": "^4.1.0", - "grunt-tslint": "^2.3.1-beta", + "grunt-tslint": "latest", "mocha": "^2.2.5", - "tslint": "^2.3.1-beta", - "typescript": "1.5.3" + "tslint": "latest", + "typescript": "latest" }, "license": "Apache-2.0" } From 9e2cbcdfdec69d2034fd2afdd2b62a3f1cea3705 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Thu, 13 Aug 2015 15:31:46 -0400 Subject: [PATCH 06/21] Turn on no-unused-variable rule - Fixes #505 - Also rename some variables to better adhere to variable-name rule --- lib/tslint.d.ts | 2 +- src/formatterLoader.ts | 4 +- src/ruleLoader.ts | 8 +- src/rules/labelPositionRule.ts | 6 +- src/rules/noShadowedVariableRule.ts | 1 - src/rules/noSwitchCaseFallThroughRule.ts | 1 - src/rules/noVarKeywordRule.ts | 2 - src/tslint-cli.ts | 292 ++++++++++++----------- test/ruleDisableEnableTests.ts | 6 +- tslint.json | 2 +- 10 files changed, 160 insertions(+), 164 deletions(-) diff --git a/lib/tslint.d.ts b/lib/tslint.d.ts index 7d847befbef..19ddcc6f296 100644 --- a/lib/tslint.d.ts +++ b/lib/tslint.d.ts @@ -239,9 +239,9 @@ declare module Lint { declare module Lint { interface LintResult { failureCount: number; + failures: RuleFailure[]; format: string; output: string; - failures: RuleFailure[]; } interface ILinterOptions { configuration: any; diff --git a/src/formatterLoader.ts b/src/formatterLoader.ts index e4249875398..96fc0ae79b9 100644 --- a/src/formatterLoader.ts +++ b/src/formatterLoader.ts @@ -17,7 +17,7 @@ module Lint { const fs = require("fs"); const path = require("path"); - const _s = require("underscore.string"); + const {camelize} = require("underscore.string"); const moduleDirectory = path.dirname(module.filename); const CORE_FORMATTERS_DIRECTORY = path.resolve(moduleDirectory, "..", "build", "formatters"); @@ -27,7 +27,7 @@ module Lint { return name; } - const camelizedName = _s.camelize(name + "Formatter"); + const camelizedName = camelize(name + "Formatter"); // first check for core formatters let Formatter = loadFormatter(CORE_FORMATTERS_DIRECTORY, camelizedName); diff --git a/src/ruleLoader.ts b/src/ruleLoader.ts index 2772e546e17..6e4a787173d 100644 --- a/src/ruleLoader.ts +++ b/src/ruleLoader.ts @@ -17,7 +17,7 @@ module Lint { const fs = require("fs"); const path = require("path"); - const _s = require("underscore.string"); + const {camelize, strLeft, strRight} = require("underscore.string"); const moduleDirectory = path.dirname(module.filename); const CORE_RULES_DIRECTORY = path.resolve(moduleDirectory, "..", "build", "rules"); @@ -68,8 +68,8 @@ module Lint { // finally check for rules within the first level of directories, // using dash prefixes as the sub-directory names if (rulesDirectory) { - const subDirectory = _s.strLeft(rulesDirectory, "-"); - const ruleName = _s.strRight(rulesDirectory, "-"); + const subDirectory = strLeft(rulesDirectory, "-"); + const ruleName = strRight(rulesDirectory, "-"); if (subDirectory !== rulesDirectory && ruleName !== rulesDirectory) { camelizedName = transformName(ruleName); Rule = loadRule(rulesDirectory, subDirectory, camelizedName); @@ -89,7 +89,7 @@ module Lint { if (nameMatch == null) { return name + "Rule"; } - return nameMatch[1] + _s.camelize(nameMatch[2]) + nameMatch[3] + "Rule"; + return nameMatch[1] + camelize(nameMatch[2]) + nameMatch[3] + "Rule"; } function loadRule(...paths: string[]) { diff --git a/src/rules/labelPositionRule.ts b/src/rules/labelPositionRule.ts index 20491730100..d31f5f01b2a 100644 --- a/src/rules/labelPositionRule.ts +++ b/src/rules/labelPositionRule.ts @@ -18,13 +18,11 @@ export class Rule extends Lint.Rules.AbstractRule { public static FAILURE_STRING = "unexpected label on statement"; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithWalker(new LabelPosWalker(sourceFile, this.getOptions())); + return this.applyWithWalker(new LabelPositionWalker(sourceFile, this.getOptions())); } } -class LabelPosWalker extends Lint.RuleWalker { - private isValidLabel: boolean; - +class LabelPositionWalker extends Lint.RuleWalker { public visitLabeledStatement(node: ts.LabeledStatement) { const statement = node.statement; if (statement.kind !== ts.SyntaxKind.DoStatement diff --git a/src/rules/noShadowedVariableRule.ts b/src/rules/noShadowedVariableRule.ts index 052b43b9230..a2adc7c9128 100644 --- a/src/rules/noShadowedVariableRule.ts +++ b/src/rules/noShadowedVariableRule.ts @@ -57,7 +57,6 @@ class NoShadowedVariableWalker extends Lint.BlockScopeAwareRuleWalker node.name; const variableName = variableIdentifier.text; const currentScope = this.getCurrentScope(); - const currentBlockScope = this.getCurrentBlockScope(); if (this.isVarInAnyScope(variableName)) { this.addFailureOnIdentifier(variableIdentifier); diff --git a/src/rules/noSwitchCaseFallThroughRule.ts b/src/rules/noSwitchCaseFallThroughRule.ts index 3089965b8fc..d2a0785359f 100644 --- a/src/rules/noSwitchCaseFallThroughRule.ts +++ b/src/rules/noSwitchCaseFallThroughRule.ts @@ -54,7 +54,6 @@ export class NoSwitchCaseFallThroughWalker extends Lint.RuleWalker { private fallThroughAllowed(nextCaseOrDefaultStatement: ts.Node) { const sourceFileText = nextCaseOrDefaultStatement.getSourceFile().text; - const childCount = nextCaseOrDefaultStatement.getChildCount(); const firstChild = nextCaseOrDefaultStatement.getChildAt(0); const commentRanges = ts.getLeadingCommentRanges(sourceFileText, firstChild.getFullStart()); if (commentRanges != null) { diff --git a/src/rules/noVarKeywordRule.ts b/src/rules/noVarKeywordRule.ts index f1646781256..5292da04b0f 100644 --- a/src/rules/noVarKeywordRule.ts +++ b/src/rules/noVarKeywordRule.ts @@ -14,8 +14,6 @@ * limitations under the License. */ -const OPTION_LEADING_UNDERSCORE = "no-var-keyword"; - export class Rule extends Lint.Rules.AbstractRule { public static FAILURE_STRING = "forbidden var keyword"; diff --git a/src/tslint-cli.ts b/src/tslint-cli.ts index d4cc217ae5b..24d56939a29 100644 --- a/src/tslint-cli.ts +++ b/src/tslint-cli.ts @@ -18,161 +18,163 @@ /// /// -const fs = require("fs"); -const optimist = require("optimist") - .usage("usage: $0") - .check((argv: any) => { - // at least one of file, help, version or unqualified argument must be present - if (!(argv.h || argv.v || argv._.length > 0)) { - throw "Missing files"; - } - - if (argv.f) { - throw "-f option is no longer available. Supply files directly to the tslint command instead."; - } - }) - .options({ - "c": { - alias: "config", - describe: "configuration file" - }, - "h": { - alias: "help", - describe: "display detailed help" - }, - "o": { - alias: "out", - describe: "output file" - }, - "r": { - alias: "rules-dir", - describe: "rules directory" - }, - "s": { - alias: "formatters-dir", - describe: "formatters directory" - }, - "t": { - alias: "format", - default: "prose", - describe: "output format (prose, json, verbose)" - }, - "v": { - alias: "version", - describe: "current version" - } - }); -const argv = optimist.argv; - -let outputStream: any; -if (argv.o !== undefined) { - outputStream = fs.createWriteStream(argv.o, { - end: false, - flags: "w+", - mode: 420 - }); -} else { - outputStream = process.stdout; -} - -if (argv.v !== undefined) { - outputStream.write(Lint.Linter.VERSION + "\n"); - process.exit(0); -} - -if ("help" in argv) { - outputStream.write(optimist.help()); - const outputString = ` -tslint accepts the following commandline 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 additional 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), json (machine readable) - and verbose. prose is the default if this option is not used. Additonal - formatters can be added and used if the --formatters-dir option is set. - - -v, --version: - The current version of tslint. - - -h, --help: - Prints this help message.`; - outputStream.write(outputString); - process.exit(0); -} - -// when provided, it should point to an existing location -if (argv.c && !fs.existsSync(argv.c)) { - console.error("Invalid option for configuration: " + argv.c); - process.exit(1); -} +module Lint { + const fs = require("fs"); + const optimist = require("optimist") + .usage("usage: $0") + .check((argv: any) => { + // at least one of file, help, version or unqualified argument must be present + if (!(argv.h || argv.v || argv._.length > 0)) { + throw "Missing files"; + } + + if (argv.f) { + throw "-f option is no longer available. Supply files directly to the tslint command instead."; + } + }) + .options({ + "c": { + alias: "config", + describe: "configuration file" + }, + "h": { + alias: "help", + describe: "display detailed help" + }, + "o": { + alias: "out", + describe: "output file" + }, + "r": { + alias: "rules-dir", + describe: "rules directory" + }, + "s": { + alias: "formatters-dir", + describe: "formatters directory" + }, + "t": { + alias: "format", + default: "prose", + describe: "output format (prose, json, verbose)" + }, + "v": { + alias: "version", + describe: "current version" + } + }); + const argv = optimist.argv; + + let outputStream: any; + if (argv.o !== undefined) { + outputStream = fs.createWriteStream(argv.o, { + end: false, + flags: "w+", + mode: 420 + }); + } else { + outputStream = process.stdout; + } -const processFile = (file: string) => { - if (!fs.existsSync(file)) { - console.error("Unable to open file: " + file); - process.exit(1); + if (argv.v !== undefined) { + outputStream.write(Linter.VERSION + "\n"); + process.exit(0); } - const contents = fs.readFileSync(file, "utf8"); - const configuration = Lint.Configuration.findConfiguration(argv.c, file); + if ("help" in argv) { + outputStream.write(optimist.help()); + const outputString = ` + tslint accepts the following commandline 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 additional 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), json (machine readable) + and verbose. prose is the default if this option is not used. Additonal + formatters can be added and used if the --formatters-dir option is set. + + -v, --version: + The current version of tslint. + + -h, --help: + Prints this help message.`; + outputStream.write(outputString); + process.exit(0); + } - if (configuration === undefined) { - console.error("unable to find tslint configuration"); + // when provided, it should point to an existing location + if (argv.c && !fs.existsSync(argv.c)) { + console.error("Invalid option for configuration: " + argv.c); process.exit(1); } - const linter = new Lint.Linter(file, contents, { - configuration: configuration, - formatter: argv.t, - formattersDirectory: argv.s, - rulesDirectory: argv.r - }); + const processFile = (file: string) => { + if (!fs.existsSync(file)) { + console.error("Unable to open file: " + file); + process.exit(1); + } + + const contents = fs.readFileSync(file, "utf8"); + const configuration = Configuration.findConfiguration(argv.c, file); - const lintResult = linter.lint(); + if (configuration === undefined) { + console.error("unable to find tslint configuration"); + process.exit(1); + } - if (lintResult.failureCount > 0) { - outputStream.write(lintResult.output, () => { - process.exit(2); + const linter = new Linter(file, contents, { + configuration: configuration, + formatter: argv.t, + formattersDirectory: argv.s, + rulesDirectory: argv.r }); - } -}; -const files = argv._; + const lintResult = linter.lint(); + + if (lintResult.failureCount > 0) { + outputStream.write(lintResult.output, () => { + process.exit(2); + }); + } + }; + + const files = argv._; -for (const file of files) { - processFile(file); + for (const file of files) { + processFile(file); + } } diff --git a/test/ruleDisableEnableTests.ts b/test/ruleDisableEnableTests.ts index f9d7d6b04e4..4ac372a2b5c 100644 --- a/test/ruleDisableEnableTests.ts +++ b/test/ruleDisableEnableTests.ts @@ -14,10 +14,10 @@ * limitations under the License. */ -const fs = require("fs"); -const path = require("path"); - describe("Enable and Disable Rules", () => { + const fs = require("fs"); + const path = require("path"); + it("is enabled and disabled in all the right places", () => { const validConfiguration = {rules: { "variable-name": true, diff --git a/tslint.json b/tslint.json index 54a64289b5b..0fa36d08c1d 100644 --- a/tslint.json +++ b/tslint.json @@ -30,7 +30,7 @@ "no-trailing-comma": true, "no-trailing-whitespace": true, "no-unused-expression": true, - "no-unused-variable": false, + "no-unused-variable": true, "no-unreachable": true, "no-use-before-declare": true, "no-var-keyword": true, From 0b4c3cc7cf152126eaceaa18fb3cac5137989179 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Fri, 14 Aug 2015 16:12:51 -0400 Subject: [PATCH 07/21] Small refactoring of member-ordering rule Fixes #569 --- src/rules/memberOrderingRule.ts | 54 +++++++++++++++------------------ 1 file changed, 25 insertions(+), 29 deletions(-) diff --git a/src/rules/memberOrderingRule.ts b/src/rules/memberOrderingRule.ts index f42245015ac..4d0f824f741 100644 --- a/src/rules/memberOrderingRule.ts +++ b/src/rules/memberOrderingRule.ts @@ -31,17 +31,10 @@ interface IModifiers { } function getModifiers(isMethod: boolean, modifiers?: ts.ModifiersArray): IModifiers { - let modifierStrings: string[] = []; - if (modifiers != null) { - modifierStrings = modifiers.map((x) => { - return x.getText(); - }); - } - return { - isInstance: modifierStrings.indexOf("static") === -1, + isInstance: !Lint.hasModifier(modifiers, ts.SyntaxKind.StaticKeyword), isMethod: isMethod, - isPrivate: modifierStrings.indexOf("private") !== -1 + isPrivate: Lint.hasModifier(modifiers, ts.SyntaxKind.PrivateKeyword) }; } @@ -55,7 +48,7 @@ function toString(modifiers: IModifiers): string { } export class MemberOrderingWalker extends Lint.RuleWalker { - private previous: IModifiers; + private previousMember: IModifiers; constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { super(sourceFile, options); @@ -72,22 +65,22 @@ export class MemberOrderingWalker extends Lint.RuleWalker { } public visitMethodDeclaration(node: ts.MethodDeclaration) { - this.checkAndSetModifiers(node, getModifiers(true, node.modifiers)); + this.checkModifiersAndSetPrevious(node, getModifiers(true, node.modifiers)); super.visitMethodDeclaration(node); } public visitMethodSignature(node: ts.SignatureDeclaration) { - this.checkAndSetModifiers(node, getModifiers(true, node.modifiers)); + this.checkModifiersAndSetPrevious(node, getModifiers(true, node.modifiers)); super.visitMethodSignature(node); } public visitPropertyDeclaration(node: ts.PropertyDeclaration) { - this.checkAndSetModifiers(node, getModifiers(false, node.modifiers)); + this.checkModifiersAndSetPrevious(node, getModifiers(false, node.modifiers)); super.visitPropertyDeclaration(node); } public visitPropertySignature(node: ts.Node) { - this.checkAndSetModifiers(node, getModifiers(false, node.modifiers)); + this.checkModifiersAndSetPrevious(node, getModifiers(false, node.modifiers)); super.visitPropertySignature(node); } @@ -96,37 +89,40 @@ export class MemberOrderingWalker extends Lint.RuleWalker { } private resetPreviousModifiers() { - this.previous = { + this.previousMember = { isInstance: false, isMethod: false, isPrivate: false }; } - private checkAndSetModifiers(node: ts.Node, current: IModifiers) { - if (!this.canAppearAfter(this.previous, current)) { - const message = "Declaration of " + toString(current) + - " not allowed to appear after declaration of " + toString(this.previous); - this.addFailure(this.createFailure(node.getStart(), node.getWidth(), message)); + private checkModifiersAndSetPrevious(node: ts.Node, currentMember: IModifiers) { + if (!this.canAppearAfter(this.previousMember, currentMember)) { + const failure = this.createFailure( + node.getStart(), + node.getWidth(), + `Declaration of ${toString(currentMember)} not allowed to appear after declaration of ${toString(this.previousMember)}` + ); + this.addFailure(failure); } - this.previous = current; + this.previousMember = currentMember; } - private canAppearAfter(previous: IModifiers, current: IModifiers) { - if (previous == null || current == null) { + private canAppearAfter(previousMember: IModifiers, currentMember: IModifiers) { + if (previousMember == null || currentMember == null) { return true; } - if (this.hasOption(OPTION_VARIABLES_BEFORE_FUNCTIONS) && previous.isMethod !== current.isMethod) { - return Number(previous.isMethod) < Number(current.isMethod); + if (this.hasOption(OPTION_VARIABLES_BEFORE_FUNCTIONS) && previousMember.isMethod !== currentMember.isMethod) { + return Number(previousMember.isMethod) < Number(currentMember.isMethod); } - if (this.hasOption(OPTION_STATIC_BEFORE_INSTANCE) && previous.isInstance !== current.isInstance) { - return Number(previous.isInstance) < Number(current.isInstance); + if (this.hasOption(OPTION_STATIC_BEFORE_INSTANCE) && previousMember.isInstance !== currentMember.isInstance) { + return Number(previousMember.isInstance) < Number(currentMember.isInstance); } - if (this.hasOption(OPTION_PUBLIC_BEFORE_PRIVATE) && previous.isPrivate !== current.isPrivate) { - return Number(previous.isPrivate) < Number(current.isPrivate); + if (this.hasOption(OPTION_PUBLIC_BEFORE_PRIVATE) && previousMember.isPrivate !== currentMember.isPrivate) { + return Number(previousMember.isPrivate) < Number(currentMember.isPrivate); } return true; From f9355862ae127378c6bd53886987193dba02c4d4 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Fri, 14 Aug 2015 16:20:57 -0400 Subject: [PATCH 08/21] Small tweaks to retrigger build --- test/ruleDisableEnableTests.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/ruleDisableEnableTests.ts b/test/ruleDisableEnableTests.ts index 4ac372a2b5c..100c5a40067 100644 --- a/test/ruleDisableEnableTests.ts +++ b/test/ruleDisableEnableTests.ts @@ -15,8 +15,8 @@ */ describe("Enable and Disable Rules", () => { - const fs = require("fs"); - const path = require("path"); + const {readFileSync} = require("fs"); + const {join} = require("path"); it("is enabled and disabled in all the right places", () => { const validConfiguration = {rules: { @@ -24,8 +24,8 @@ describe("Enable and Disable Rules", () => { "quotemark": [true, "double"] }}; - const relativePath = path.join("test", "files", "rules/enabledisable.test.ts"); - const source = fs.readFileSync(relativePath, "utf8"); + const relativePath = join("test", "files", "rules/enabledisable.test.ts"); + const source = readFileSync(relativePath, "utf8"); const options: Lint.ILinterOptions = { configuration: validConfiguration, From 5300b20fa83c4a9b3cee5bf7cabf3ce6d74e1898 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Fri, 14 Aug 2015 16:29:33 -0400 Subject: [PATCH 09/21] Bump findup-sync and underscore.string dependencies --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 06d094bd5c0..71c8d5cafc8 100644 --- a/package.json +++ b/package.json @@ -22,9 +22,9 @@ "test": "grunt" }, "dependencies": { - "findup-sync": "~0.1.2", + "findup-sync": "~0.2.1", "optimist": "~0.6.0", - "underscore.string": "~2.3.3" + "underscore.string": "~3.1.1" }, "devDependencies": { "chai": "^3.0.0", From 2fd802acba4a3b42f1e7efef418026625e0cd11f Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Fri, 14 Aug 2015 16:36:35 -0400 Subject: [PATCH 10/21] Turn on member-ordering rule Fixes #573 --- src/rules/forinRule.ts | 26 +++++++++++++------------- src/rules/oneLineRule.ts | 18 +++++++++--------- src/tslint.ts | 4 ++-- tslint.json | 5 +++++ 4 files changed, 29 insertions(+), 24 deletions(-) diff --git a/src/rules/forinRule.ts b/src/rules/forinRule.ts index e13518cb8a0..017a8f44f81 100644 --- a/src/rules/forinRule.ts +++ b/src/rules/forinRule.ts @@ -51,7 +51,7 @@ class ForInWalker extends Lint.RuleWalker { // if this "if" statement has a single continue block const ifStatement = ( firstBlockStatement).thenStatement; - if (ForInWalker.nodeIsContinue(ifStatement)) { + if (nodeIsContinue(ifStatement)) { return; } } @@ -61,21 +61,21 @@ class ForInWalker extends Lint.RuleWalker { const failure = this.createFailure(node.getStart(), node.getWidth(), Rule.FAILURE_STRING); this.addFailure(failure); } +} - private static nodeIsContinue(node: ts.Node) { - const kind = node.kind; +function nodeIsContinue(node: ts.Node) { + const kind = node.kind; - if (kind === ts.SyntaxKind.ContinueStatement) { - return true; - } + if (kind === ts.SyntaxKind.ContinueStatement) { + return true; + } - if (kind === ts.SyntaxKind.Block) { - const blockStatements = ( node).statements; - if (blockStatements.length === 1 && blockStatements[0].kind === ts.SyntaxKind.ContinueStatement) { - return true; - } + if (kind === ts.SyntaxKind.Block) { + const blockStatements = ( node).statements; + if (blockStatements.length === 1 && blockStatements[0].kind === ts.SyntaxKind.ContinueStatement) { + return true; } - - return false; } + + return false; } diff --git a/src/rules/oneLineRule.ts b/src/rules/oneLineRule.ts index 3f9874b74f8..28be132a989 100644 --- a/src/rules/oneLineRule.ts +++ b/src/rules/oneLineRule.ts @@ -44,7 +44,7 @@ class OneLineWalker extends Lint.RuleWalker { const elseStatement = node.elseStatement; if (elseStatement != null) { // find the else keyword - const elseKeyword = OneLineWalker.getFirstChildOfKind(node, ts.SyntaxKind.ElseKeyword); + const elseKeyword = getFirstChildOfKind(node, ts.SyntaxKind.ElseKeyword); if (elseStatement.kind === ts.SyntaxKind.Block) { const elseOpeningBrace = elseStatement.getChildAt(0); this.handleOpeningBrace(elseKeyword, elseOpeningBrace); @@ -152,7 +152,7 @@ class OneLineWalker extends Lint.RuleWalker { public visitEnumDeclaration(node: ts.EnumDeclaration) { const nameNode = node.name; - const openBraceToken = OneLineWalker.getFirstChildOfKind(node, ts.SyntaxKind.OpenBraceToken); + const openBraceToken = getFirstChildOfKind(node, ts.SyntaxKind.OpenBraceToken); this.handleOpeningBrace(nameNode, openBraceToken); super.visitEnumDeclaration(node); } @@ -166,14 +166,14 @@ class OneLineWalker extends Lint.RuleWalker { public visitInterfaceDeclaration(node: ts.InterfaceDeclaration) { const nameNode = node.name; - const openBraceToken = OneLineWalker.getFirstChildOfKind(node, ts.SyntaxKind.OpenBraceToken); + const openBraceToken = getFirstChildOfKind(node, ts.SyntaxKind.OpenBraceToken); this.handleOpeningBrace(nameNode, openBraceToken); super.visitInterfaceDeclaration(node); } public visitClassDeclaration(node: ts.ClassDeclaration) { const nameNode = node.name; - const openBraceToken = OneLineWalker.getFirstChildOfKind(node, ts.SyntaxKind.OpenBraceToken); + const openBraceToken = getFirstChildOfKind(node, ts.SyntaxKind.OpenBraceToken); this.handleOpeningBrace(nameNode, openBraceToken); super.visitClassDeclaration(node); } @@ -196,7 +196,7 @@ class OneLineWalker extends Lint.RuleWalker { public visitArrowFunction(node: ts.FunctionLikeDeclaration) { const body = node.body; if (body != null && body.kind === ts.SyntaxKind.Block) { - const arrowToken = OneLineWalker.getFirstChildOfKind(node, ts.SyntaxKind.EqualsGreaterThanToken); + const arrowToken = getFirstChildOfKind(node, ts.SyntaxKind.EqualsGreaterThanToken); const openBraceToken = node.body.getChildAt(0); this.handleOpeningBrace(arrowToken, openBraceToken); } @@ -210,7 +210,7 @@ class OneLineWalker extends Lint.RuleWalker { if (node.type != null) { this.handleOpeningBrace(node.type, openBraceToken); } else { - const closeParenToken = OneLineWalker.getFirstChildOfKind(node, ts.SyntaxKind.CloseParenToken); + const closeParenToken = getFirstChildOfKind(node, ts.SyntaxKind.CloseParenToken); this.handleOpeningBrace(closeParenToken, openBraceToken); } } @@ -246,8 +246,8 @@ class OneLineWalker extends Lint.RuleWalker { this.addFailure(failure); } } +} - private static getFirstChildOfKind(node: ts.Node, kind: ts.SyntaxKind) { - return node.getChildren().filter((child) => child.kind === kind)[0]; - } +function getFirstChildOfKind(node: ts.Node, kind: ts.SyntaxKind) { + return node.getChildren().filter((child) => child.kind === kind)[0]; } diff --git a/src/tslint.ts b/src/tslint.ts index a888241498f..4eb4c097d25 100644 --- a/src/tslint.ts +++ b/src/tslint.ts @@ -33,12 +33,12 @@ module Lint { } export class Linter { + public static VERSION = "2.4.2"; + private fileName: string; private source: string; private options: ILinterOptions; - public static VERSION = "2.4.2"; - constructor(fileName: string, source: string, options: ILinterOptions) { this.fileName = fileName; this.source = source; diff --git a/tslint.json b/tslint.json index 54a64289b5b..0e6754d364c 100644 --- a/tslint.json +++ b/tslint.json @@ -9,6 +9,11 @@ "label-position": true, "label-undefined": true, "max-line-length": [true, 140], + "member-ordering": [true, + "public-before-private", + "static-before-instance", + "variables-before-functions" + ], "no-arg": true, "no-bitwise": true, "no-console": [true, From 4adfe3d321cbe3f2998694593d1e6a5a787af41c Mon Sep 17 00:00:00 2001 From: Jason Killian Date: Tue, 18 Aug 2015 18:50:01 -0400 Subject: [PATCH 11/21] Ensure that extra failues during classNameRuleTests will be detected --- test/rules/classNameRuleTests.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/rules/classNameRuleTests.ts b/test/rules/classNameRuleTests.ts index 51591516bc7..074a8566024 100644 --- a/test/rules/classNameRuleTests.ts +++ b/test/rules/classNameRuleTests.ts @@ -20,11 +20,12 @@ describe("", () => { it("ensures class names are always pascal-cased", () => { const createFailure = Lint.Test.createFailuresOnFile(fileName, ClassNameRule.FAILURE_STRING); - const expectedFailure1 = createFailure([5, 7], [5, 23]); - const expectedFailure2 = createFailure([9, 7], [9, 33]); + const expectedFailures = [ + createFailure([5, 7], [5, 23]), + createFailure([9, 7], [9, 33]) + ]; const actualFailures = Lint.Test.applyRuleOnFile(fileName, ClassNameRule); - Lint.Test.assertContainsFailure(actualFailures, expectedFailure1); - Lint.Test.assertContainsFailure(actualFailures, expectedFailure2); + Lint.Test.assertFailuresEqual(actualFailures, expectedFailures); }); }); From 7a17cdc11d47db418b9a31123e7de00695e8e1cb Mon Sep 17 00:00:00 2001 From: Jason Killian Date: Wed, 19 Aug 2015 09:55:20 -0400 Subject: [PATCH 12/21] Fix indentation/typos --- test/check-bin.sh | 8 ++++---- test/rules/classNameRuleTests.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/check-bin.sh b/test/check-bin.sh index e887e5d8988..e0cba273d54 100755 --- a/test/check-bin.sh +++ b/test/check-bin.sh @@ -29,7 +29,7 @@ expectOut () { echo "Checking tslint binary" # make sure calling tslint with no args exits correctly. ./bin/tslint -expectOut $? 1 "tslint with not args did not exit correctly" +expectOut $? 1 "tslint with no args did not exit correctly" # make sure calling tslint with a good file exits correctly. ./bin/tslint src/configuration.ts @@ -37,11 +37,11 @@ expectOut $? 0 "tslint with a good file did not exit correctly" # make sure calling tslint without the -f flag exits correctly ./bin/tslint src/configuration.ts src/formatterLoader.ts -expectOut $? 0 "tslint with no did not exit correctly" +expectOut $? 0 "tslint without -f flag did not exit correctly" -# make sure calling tslint the -f flag exits correctly +# make sure calling tslint with the -f flag exits correctly ./bin/tslint src/configuration.ts -f src/formatterLoader.ts -expectOut $? 1 "tslint with no did not exit correctly" +expectOut $? 1 "tslint with -f flag did not exit correctly" if [ $num_failures != 0 ] then diff --git a/test/rules/classNameRuleTests.ts b/test/rules/classNameRuleTests.ts index 074a8566024..9e2d22234ad 100644 --- a/test/rules/classNameRuleTests.ts +++ b/test/rules/classNameRuleTests.ts @@ -21,8 +21,8 @@ describe("", () => { it("ensures class names are always pascal-cased", () => { const createFailure = Lint.Test.createFailuresOnFile(fileName, ClassNameRule.FAILURE_STRING); const expectedFailures = [ - createFailure([5, 7], [5, 23]), - createFailure([9, 7], [9, 33]) + createFailure([5, 7], [5, 23]), + createFailure([9, 7], [9, 33]) ]; const actualFailures = Lint.Test.applyRuleOnFile(fileName, ClassNameRule); From 3357d0153ad119bf393c9d55edc6f3655bf06f89 Mon Sep 17 00:00:00 2001 From: Jason Killian Date: Wed, 19 Aug 2015 10:56:31 -0400 Subject: [PATCH 13/21] Clarify error message --- test/check-bin.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/check-bin.sh b/test/check-bin.sh index e0cba273d54..d1fe71ac0fd 100755 --- a/test/check-bin.sh +++ b/test/check-bin.sh @@ -37,7 +37,7 @@ expectOut $? 0 "tslint with a good file did not exit correctly" # make sure calling tslint without the -f flag exits correctly ./bin/tslint src/configuration.ts src/formatterLoader.ts -expectOut $? 0 "tslint without -f flag did not exit correctly" +expectOut $? 0 "tslint with valid arguments did not exit correctly" # make sure calling tslint with the -f flag exits correctly ./bin/tslint src/configuration.ts -f src/formatterLoader.ts From 7fbc55610a288780e13cb6d0baa82ffc1291416b Mon Sep 17 00:00:00 2001 From: Alexander Schrab Date: Wed, 19 Aug 2015 19:12:26 +0200 Subject: [PATCH 14/21] Review fixes --- docs/sample.tslint.json | 2 +- src/rules/memberNoDefaultAccessRule.ts | 44 ++++++++---------------- test/rules/memberNoDefaultAccessTests.ts | 6 ++-- 3 files changed, 17 insertions(+), 35 deletions(-) diff --git a/docs/sample.tslint.json b/docs/sample.tslint.json index fd92e63e5b1..db0df27f252 100644 --- a/docs/sample.tslint.json +++ b/docs/sample.tslint.json @@ -5,7 +5,6 @@ "arguments", "statements"], "ban": false, - "member-no-default-access": true, "class-name": true, "comment-format": [true, "check-space", @@ -20,6 +19,7 @@ "label-position": true, "label-undefined": true, "max-line-length": [true, 140], + "member-no-default-access": true, "member-ordering": [true, "public-before-private", "static-before-instance", diff --git a/src/rules/memberNoDefaultAccessRule.ts b/src/rules/memberNoDefaultAccessRule.ts index 008b4ba1a64..cf7a39d2e21 100644 --- a/src/rules/memberNoDefaultAccessRule.ts +++ b/src/rules/memberNoDefaultAccessRule.ts @@ -12,7 +12,7 @@ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. -*/ + */ export class Rule extends Lint.Rules.AbstractRule { static FAILURE_STRING = "default access modifier on member/method not allowed"; @@ -26,45 +26,29 @@ interface IModifiers { hasAccessModifier: boolean; } -function getModifiers(modifiers?: ts.ModifiersArray): IModifiers { - let modifierStrings: string[] = []; - if (modifiers != null) { - modifierStrings = modifiers.map((x) => { - return x.getText(); - }); - } - - let hasAccessModifier = modifierStrings.indexOf("public") !== -1; - hasAccessModifier = hasAccessModifier || modifierStrings.indexOf("private") !== -1; - hasAccessModifier = hasAccessModifier || modifierStrings.indexOf("protected") !== -1; - - return { - hasAccessModifier: hasAccessModifier - }; -} - export class MemberAccessWalker extends Lint.RuleWalker { constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { super(sourceFile, options); } - public visitMethodDeclaration(node: ts.MethodDeclaration): void { - this.checkModifiers(node, getModifiers(node.modifiers)); - super.visitMethodDeclaration(node); + public visitMethodDeclaration(node: ts.MethodDeclaration) { + this.validateVisibilityModifiers(node); } - public visitPropertyDeclaration(node: ts.PropertyDeclaration): void { - this.checkModifiers(node, getModifiers(node.modifiers)); - super.visitPropertyDeclaration(node); + public visitPropertyDeclaration(node: ts.PropertyDeclaration) { + this.validateVisibilityModifiers(node); } - private checkModifiers(node: ts.Node, current: IModifiers): void { - if (!this.followsRules(current)) { + private validateVisibilityModifiers(node: ts.Node) { + const hasAnyVisibilityModifiers = Lint.hasModifier( + node.modifiers, + ts.SyntaxKind.PublicKeyword, + ts.SyntaxKind.PrivateKeyword, + ts.SyntaxKind.ProtectedKeyword + ); + + if (!hasAnyVisibilityModifiers) { this.addFailure(this.createFailure(node.getStart(), node.getWidth(), Rule.FAILURE_STRING)); } } - - private followsRules(current: IModifiers): boolean { - return current.hasAccessModifier; - } } diff --git a/test/rules/memberNoDefaultAccessTests.ts b/test/rules/memberNoDefaultAccessTests.ts index 6beb70f6950..e1b1676aa63 100644 --- a/test/rules/memberNoDefaultAccessTests.ts +++ b/test/rules/memberNoDefaultAccessTests.ts @@ -21,10 +21,8 @@ describe("", () => { let actualFailures = Lint.Test.applyRuleOnFile(fileName, MemberNoDefaultAccessRule); Lint.Test.assertFailuresEqual(actualFailures, [ - Lint.Test.createFailure(fileName, [8, 5], [8, 15], - MemberNoDefaultAccessRule.FAILURE_STRING), - Lint.Test.createFailure(fileName, [16, 5], [17, 6], - MemberNoDefaultAccessRule.FAILURE_STRING) + Lint.Test.createFailure(fileName, [8, 5], [8, 15], MemberNoDefaultAccessRule.FAILURE_STRING), + Lint.Test.createFailure(fileName, [16, 5], [17, 6], MemberNoDefaultAccessRule.FAILURE_STRING) ]); }); }); From f55ea3650d6180a12879aebe526aa2c7a1387562 Mon Sep 17 00:00:00 2001 From: Jason Killian Date: Wed, 19 Aug 2015 16:30:39 -0400 Subject: [PATCH 15/21] Add skeleton for rule ans tests --- src/rules/noCondAssignRule.ts | 28 ++++++++++++++++++++++++++ src/rules/tsconfig.json | 1 + test/files/rules/nocondassign.test.ts | 29 +++++++++++++++++++++++++++ test/rules/noCondAssignRuleTests.ts | 29 +++++++++++++++++++++++++++ test/tsconfig.json | 1 + 5 files changed, 88 insertions(+) create mode 100644 src/rules/noCondAssignRule.ts create mode 100644 test/files/rules/nocondassign.test.ts create mode 100644 test/rules/noCondAssignRuleTests.ts diff --git a/src/rules/noCondAssignRule.ts b/src/rules/noCondAssignRule.ts new file mode 100644 index 00000000000..b4b78e741b4 --- /dev/null +++ b/src/rules/noCondAssignRule.ts @@ -0,0 +1,28 @@ +/* + * Copyright 2015 Palantir Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export class Rule extends Lint.Rules.AbstractRule { + public static FAILURE_STRING = "assignment in conditional: "; + + public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + const walker = new NoConditionalAssignmentWalker(sourceFile, this.getOptions()); + return this.applyWithWalker(walker); + } +} + +class NoConditionalAssignmentWalker extends Lint.RuleWalker { + +} diff --git a/src/rules/tsconfig.json b/src/rules/tsconfig.json index 41803dab9b1..d511e3ec37a 100644 --- a/src/rules/tsconfig.json +++ b/src/rules/tsconfig.json @@ -34,6 +34,7 @@ "./noAnyRule.ts", "./noArgRule.ts", "./noBitwiseRule.ts", + "./noCondAssignRule.ts", "./noConsecutiveBlankLinesRule.ts", "./noConsoleRule.ts", "./noConstructRule.ts", diff --git a/test/files/rules/nocondassign.test.ts b/test/files/rules/nocondassign.test.ts new file mode 100644 index 00000000000..58fee0066dd --- /dev/null +++ b/test/files/rules/nocondassign.test.ts @@ -0,0 +1,29 @@ +// valid cases +if (x == 5) { } +if (x === 5) { } +else if (y <= 23) { } +else if ((z && a) == 7) { } +else { } + +do { } while (x == 2); +do { } while (x !== 2); + +while (x == 2) { } +while (x !== 2) { } + +for (var x = 8; x == 8; ++x) { } +for (var x = 8; x == 8; x = 12) { } +for (;;) { } + +// invalid cases +if (x = 5) { } +if (a && (b = 5)) { } +else if (x == 2) { } + +do { } while (x = 4); + +while (x = 4); +while ((x = y - 12)); + +for (var x = 4; x = 8; x++) { } +for (; (y == 2) && (x = 3); ) { } diff --git a/test/rules/noCondAssignRuleTests.ts b/test/rules/noCondAssignRuleTests.ts new file mode 100644 index 00000000000..524fb01808c --- /dev/null +++ b/test/rules/noCondAssignRuleTests.ts @@ -0,0 +1,29 @@ +/* + * Copyright 2015 Palantir Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +describe("", () => { + const fileName = "rules/nocondassign.test.ts"; + const NoCondAssignRule = Lint.Test.getRule("no-cond-assign"); + let actualFailures: Lint.RuleFailure[]; + + before(() => { + actualFailures = Lint.Test.applyRuleOnFile(fileName, NoCondAssignRule); + }); + + it("no false positives for rule", () => { + assert.lengthOf(actualFailures, 8); + }); +}); diff --git a/test/tsconfig.json b/test/tsconfig.json index a6c47d077a4..7aef3e40fa1 100644 --- a/test/tsconfig.json +++ b/test/tsconfig.json @@ -51,6 +51,7 @@ "./rules/noAnyRuleTests.ts", "./rules/noArgRuleTests.ts", "./rules/noBitwiseRuleTests.ts", + "./rules/noCondAssignRuleTests.ts", "./rules/noConsecutiveBlankLinesRuleTests.ts", "./rules/noConsoleRuleTests.ts", "./rules/noConstructRuleTests.ts", From 2a3d34083e33ccbac59cc74d5358cd5c85cf0a2c Mon Sep 17 00:00:00 2001 From: Jason Killian Date: Wed, 19 Aug 2015 18:14:15 -0400 Subject: [PATCH 16/21] Implement no-conditional-assignment rule --- lib/tslint.d.ts | 2 +- src/language/walker/syntaxWalker.ts | 2 +- src/rules/noCondAssignRule.ts | 28 ------ src/rules/noConditionalAssignmentRule.ts | 92 +++++++++++++++++++ src/rules/tsconfig.json | 2 +- test/files/rules/nocondassign.test.ts | 7 +- test/rules/noCondAssignRuleTests.ts | 29 ------ .../rules/noConditionalAssignmentRuleTests.ts | 64 +++++++++++++ test/tsconfig.json | 2 +- 9 files changed, 166 insertions(+), 62 deletions(-) delete mode 100644 src/rules/noCondAssignRule.ts create mode 100644 src/rules/noConditionalAssignmentRule.ts delete mode 100644 test/rules/noCondAssignRuleTests.ts create mode 100644 test/rules/noConditionalAssignmentRuleTests.ts diff --git a/lib/tslint.d.ts b/lib/tslint.d.ts index 19ddcc6f296..12e874de393 100644 --- a/lib/tslint.d.ts +++ b/lib/tslint.d.ts @@ -64,7 +64,7 @@ declare module Lint { protected visitVariableStatement(node: ts.VariableStatement): void; protected visitWhileStatement(node: ts.WhileStatement): void; protected visitNode(node: ts.Node): void; - private walkChildren(node); + protected walkChildren(node: ts.Node): void; } } declare module Lint { diff --git a/src/language/walker/syntaxWalker.ts b/src/language/walker/syntaxWalker.ts index a1ab5fe7e49..f4d4b033d02 100644 --- a/src/language/walker/syntaxWalker.ts +++ b/src/language/walker/syntaxWalker.ts @@ -528,7 +528,7 @@ module Lint { } } - private walkChildren(node: ts.Node) { + protected walkChildren(node: ts.Node) { ts.forEachChild(node, (child) => this.visitNode(child)); } } diff --git a/src/rules/noCondAssignRule.ts b/src/rules/noCondAssignRule.ts deleted file mode 100644 index b4b78e741b4..00000000000 --- a/src/rules/noCondAssignRule.ts +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright 2015 Palantir Technologies, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -export class Rule extends Lint.Rules.AbstractRule { - public static FAILURE_STRING = "assignment in conditional: "; - - public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - const walker = new NoConditionalAssignmentWalker(sourceFile, this.getOptions()); - return this.applyWithWalker(walker); - } -} - -class NoConditionalAssignmentWalker extends Lint.RuleWalker { - -} diff --git a/src/rules/noConditionalAssignmentRule.ts b/src/rules/noConditionalAssignmentRule.ts new file mode 100644 index 00000000000..e577c26dfed --- /dev/null +++ b/src/rules/noConditionalAssignmentRule.ts @@ -0,0 +1,92 @@ +/* + * Copyright 2015 Palantir Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export class Rule extends Lint.Rules.AbstractRule { + public static FAILURE_STRING = "assignment in conditional: "; + + public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + const walker = new NoCondAssignWalker(sourceFile, this.getOptions()); + return this.applyWithWalker(walker); + } +} + +class NoCondAssignWalker extends Lint.RuleWalker { + private inConditionalExpression = false; + + protected visitIfStatement(node: ts.IfStatement) { + this.inConditionalExpression = true; + if (node.expression.kind === ts.SyntaxKind.BinaryExpression) { + this.checkBinaryExpForAssignment( node.expression); + } + this.walkChildren(node.expression); + this.inConditionalExpression = false; + + super.visitIfStatement(node); + } + + protected visitWhileStatement(node: ts.WhileStatement) { + this.inConditionalExpression = true; + if (node.expression.kind === ts.SyntaxKind.BinaryExpression) { + this.checkBinaryExpForAssignment( node.expression); + } + this.walkChildren(node.expression); + this.inConditionalExpression = false; + + super.visitWhileStatement(node); + } + + protected visitDoStatement(node: ts.DoStatement) { + this.inConditionalExpression = true; + if (node.expression.kind === ts.SyntaxKind.BinaryExpression) { + this.checkBinaryExpForAssignment( node.expression); + } + this.walkChildren(node.expression); + this.inConditionalExpression = false; + + super.visitWhileStatement(node); + } + + protected visitForStatement(node: ts.ForStatement) { + this.inConditionalExpression = true; + if (node.condition) { + if (node.condition.kind === ts.SyntaxKind.BinaryExpression) { + this.checkBinaryExpForAssignment( node.condition); + } + this.walkChildren(node.condition); + } + this.inConditionalExpression = false; + + super.visitForStatement(node); + } + + protected visitBinaryExpression(node: ts.BinaryExpression) { + if (this.inConditionalExpression) { + this.checkBinaryExpForAssignment(node); + } + super.visitBinaryExpression(node); + } + + private checkBinaryExpForAssignment(expression: ts.BinaryExpression) { + if (this.isAssignmentToken(expression.operatorToken.kind)) { + this.addFailure(this.createFailure(expression.getStart(), expression.getWidth(), Rule.FAILURE_STRING)); + } + } + + private isAssignmentToken(token: ts.SyntaxKind): boolean { + return token >= ts.SyntaxKind.FirstAssignment && token <= ts.SyntaxKind.LastAssignment; + } + +} diff --git a/src/rules/tsconfig.json b/src/rules/tsconfig.json index d511e3ec37a..6d686067e30 100644 --- a/src/rules/tsconfig.json +++ b/src/rules/tsconfig.json @@ -34,7 +34,7 @@ "./noAnyRule.ts", "./noArgRule.ts", "./noBitwiseRule.ts", - "./noCondAssignRule.ts", + "./noConditionalAssignmentRule.ts", "./noConsecutiveBlankLinesRule.ts", "./noConsoleRule.ts", "./noConstructRule.ts", diff --git a/test/files/rules/nocondassign.test.ts b/test/files/rules/nocondassign.test.ts index 58fee0066dd..cbbad767feb 100644 --- a/test/files/rules/nocondassign.test.ts +++ b/test/files/rules/nocondassign.test.ts @@ -18,7 +18,7 @@ for (;;) { } // invalid cases if (x = 5) { } if (a && (b = 5)) { } -else if (x == 2) { } +else if (x = 2) { } do { } while (x = 4); @@ -27,3 +27,8 @@ while ((x = y - 12)); for (var x = 4; x = 8; x++) { } for (; (y == 2) && (x = 3); ) { } + +if (x += 2) { } +else if (h || (x <<= 4)) { } + +do { } while (x ^= 4) { } diff --git a/test/rules/noCondAssignRuleTests.ts b/test/rules/noCondAssignRuleTests.ts deleted file mode 100644 index 524fb01808c..00000000000 --- a/test/rules/noCondAssignRuleTests.ts +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright 2015 Palantir Technologies, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -describe("", () => { - const fileName = "rules/nocondassign.test.ts"; - const NoCondAssignRule = Lint.Test.getRule("no-cond-assign"); - let actualFailures: Lint.RuleFailure[]; - - before(() => { - actualFailures = Lint.Test.applyRuleOnFile(fileName, NoCondAssignRule); - }); - - it("no false positives for rule", () => { - assert.lengthOf(actualFailures, 8); - }); -}); diff --git a/test/rules/noConditionalAssignmentRuleTests.ts b/test/rules/noConditionalAssignmentRuleTests.ts new file mode 100644 index 00000000000..29d5b39f131 --- /dev/null +++ b/test/rules/noConditionalAssignmentRuleTests.ts @@ -0,0 +1,64 @@ +/* + * Copyright 2015 Palantir Technologies, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +describe("", () => { + const fileName = "rules/nocondassign.test.ts"; + const NoConditionalAssignmentRule = Lint.Test.getRule("no-conditional-assignment"); + const createFailure = Lint.Test.createFailuresOnFile(fileName, NoConditionalAssignmentRule.FAILURE_STRING); + let actualFailures: Lint.RuleFailure[]; + + before(() => { + actualFailures = Lint.Test.applyRuleOnFile(fileName, NoConditionalAssignmentRule); + }); + + it("should detect assignments in if conditionals", () => { + const expectedFailure1 = createFailure([19, 5], [19, 10]); + const expectedFailure2 = createFailure([20, 11], [20, 16]); + const expectedFailure3 = createFailure([21, 10], [21, 15]); + const expectedFailure4 = createFailure([31, 5], [31, 11]); + const expectedFailure5 = createFailure([32, 16], [32, 23]); + Lint.Test.assertContainsFailure(actualFailures, expectedFailure1); + Lint.Test.assertContainsFailure(actualFailures, expectedFailure2); + Lint.Test.assertContainsFailure(actualFailures, expectedFailure3); + Lint.Test.assertContainsFailure(actualFailures, expectedFailure4); + Lint.Test.assertContainsFailure(actualFailures, expectedFailure5); + }); + + it("should detect assignments in do-while conditionals", () => { + const expectedFailure1 = createFailure([23, 15], [23, 20]); + const expectedFailure2 = createFailure([34, 15], [34, 21]); + Lint.Test.assertContainsFailure(actualFailures, expectedFailure1); + Lint.Test.assertContainsFailure(actualFailures, expectedFailure2); + }); + + it("should detect assignments in while conditionals", () => { + const expectedFailure1 = createFailure([25, 8], [25, 13]); + const expectedFailure2 = createFailure([26, 9], [26, 19]); + Lint.Test.assertContainsFailure(actualFailures, expectedFailure1); + Lint.Test.assertContainsFailure(actualFailures, expectedFailure2); + }); + + it("should detect assignments in for conditionals", () => { + const expectedFailure1 = createFailure([28, 17], [28, 22]); + const expectedFailure2 = createFailure([29, 21], [29, 26]); + Lint.Test.assertContainsFailure(actualFailures, expectedFailure1); + Lint.Test.assertContainsFailure(actualFailures, expectedFailure2); + }); + + it("no false positives for rule", () => { + assert.lengthOf(actualFailures, 11); + }); +}); diff --git a/test/tsconfig.json b/test/tsconfig.json index 7aef3e40fa1..9eff499abec 100644 --- a/test/tsconfig.json +++ b/test/tsconfig.json @@ -51,7 +51,7 @@ "./rules/noAnyRuleTests.ts", "./rules/noArgRuleTests.ts", "./rules/noBitwiseRuleTests.ts", - "./rules/noCondAssignRuleTests.ts", + "./rules/noConditionalAssignmentRuleTests.ts", "./rules/noConsecutiveBlankLinesRuleTests.ts", "./rules/noConsoleRuleTests.ts", "./rules/noConstructRuleTests.ts", From 72625953dc68c33b65e0b708927793b8bee7dde0 Mon Sep 17 00:00:00 2001 From: Jason Killian Date: Wed, 19 Aug 2015 18:18:01 -0400 Subject: [PATCH 17/21] Update documentation with no-conditional-assignment rule --- README.md | 1 + docs/sample.tslint.json | 1 + 2 files changed, 2 insertions(+) diff --git a/README.md b/README.md index acc79947c24..cbd633469b2 100644 --- a/README.md +++ b/README.md @@ -158,6 +158,7 @@ A sample configuration file with all options is available [here](https://github. * `no-any` diallows usages of `any` as a type decoration. * `no-arg` disallows access to `arguments.callee`. * `no-bitwise` disallows bitwise operators. +* `no-conditional-assignment` disallows any type of assignment in any conditionals. This applies to `do-while`, `for`, `if`, and `while` statements. * `no-console` disallows access to the specified functions on `console`. Rule options are functions to ban on the console variable. * `no-consecutive-blank-lines` disallows having more than one blank line in a row in a file * `no-construct` disallows access to the constructors of `String`, `Number`, and `Boolean`. diff --git a/docs/sample.tslint.json b/docs/sample.tslint.json index 685bb4260ec..0d9ddaa6928 100644 --- a/docs/sample.tslint.json +++ b/docs/sample.tslint.json @@ -27,6 +27,7 @@ "no-any": false, "no-arg": true, "no-bitwise": true, + "no-conditional-assignment": true, "no-console": [true, "debug", "info", From 21799936f53785ffe384be43ed03a6ec7bc83779 Mon Sep 17 00:00:00 2001 From: Jason Killian Date: Wed, 19 Aug 2015 18:29:36 -0400 Subject: [PATCH 18/21] DRY up noConditionalAssignmentRule code --- src/rules/noConditionalAssignmentRule.ts | 52 +++++++++--------------- 1 file changed, 19 insertions(+), 33 deletions(-) diff --git a/src/rules/noConditionalAssignmentRule.ts b/src/rules/noConditionalAssignmentRule.ts index e577c26dfed..a1423d41fd4 100644 --- a/src/rules/noConditionalAssignmentRule.ts +++ b/src/rules/noConditionalAssignmentRule.ts @@ -18,68 +18,54 @@ export class Rule extends Lint.Rules.AbstractRule { public static FAILURE_STRING = "assignment in conditional: "; public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - const walker = new NoCondAssignWalker(sourceFile, this.getOptions()); + const walker = new NoConditionalAssignmentWalker(sourceFile, this.getOptions()); return this.applyWithWalker(walker); } } -class NoCondAssignWalker extends Lint.RuleWalker { +class NoConditionalAssignmentWalker extends Lint.RuleWalker { private inConditionalExpression = false; protected visitIfStatement(node: ts.IfStatement) { - this.inConditionalExpression = true; - if (node.expression.kind === ts.SyntaxKind.BinaryExpression) { - this.checkBinaryExpForAssignment( node.expression); - } - this.walkChildren(node.expression); - this.inConditionalExpression = false; - + this.checkExpressionForBinaryExpressions(node.expression); super.visitIfStatement(node); } protected visitWhileStatement(node: ts.WhileStatement) { - this.inConditionalExpression = true; - if (node.expression.kind === ts.SyntaxKind.BinaryExpression) { - this.checkBinaryExpForAssignment( node.expression); - } - this.walkChildren(node.expression); - this.inConditionalExpression = false; - + this.checkExpressionForBinaryExpressions(node.expression); super.visitWhileStatement(node); } protected visitDoStatement(node: ts.DoStatement) { - this.inConditionalExpression = true; - if (node.expression.kind === ts.SyntaxKind.BinaryExpression) { - this.checkBinaryExpForAssignment( node.expression); - } - this.walkChildren(node.expression); - this.inConditionalExpression = false; - + this.checkExpressionForBinaryExpressions(node.expression); super.visitWhileStatement(node); } protected visitForStatement(node: ts.ForStatement) { - this.inConditionalExpression = true; if (node.condition) { - if (node.condition.kind === ts.SyntaxKind.BinaryExpression) { - this.checkBinaryExpForAssignment( node.condition); - } - this.walkChildren(node.condition); + this.checkExpressionForBinaryExpressions(node.condition); } - this.inConditionalExpression = false; super.visitForStatement(node); } - protected visitBinaryExpression(node: ts.BinaryExpression) { + protected visitBinaryExpression(expression: ts.BinaryExpression) { if (this.inConditionalExpression) { - this.checkBinaryExpForAssignment(node); + this.checkBinaryExpressionForAssignment(expression); } - super.visitBinaryExpression(node); + super.visitBinaryExpression(expression); + } + + private checkExpressionForBinaryExpressions(expression: ts.Expression) { + this.inConditionalExpression = true; + if (expression.kind === ts.SyntaxKind.BinaryExpression) { + this.checkBinaryExpressionForAssignment( expression); + } + this.walkChildren(expression); + this.inConditionalExpression = false; } - private checkBinaryExpForAssignment(expression: ts.BinaryExpression) { + private checkBinaryExpressionForAssignment(expression: ts.BinaryExpression) { if (this.isAssignmentToken(expression.operatorToken.kind)) { this.addFailure(this.createFailure(expression.getStart(), expression.getWidth(), Rule.FAILURE_STRING)); } From e6dcfd890ba04bb90fe7b7eb92ccd66c0f8690d1 Mon Sep 17 00:00:00 2001 From: Alexander Schrab Date: Thu, 20 Aug 2015 14:59:09 +0200 Subject: [PATCH 19/21] Review fixes #2 --- README.md | 2 +- docs/sample.tslint.json | 2 +- ...efaultAccessRule.ts => memberAccessRule.ts} | 6 +----- src/rules/tsconfig.json | 4 ++-- test/files/rules/memberaccess.test.ts | 18 ++++++++++++++++++ test/files/rules/membernodefaultaccess.test.ts | 18 ------------------ ...aultAccessTests.ts => memberAccessTests.ts} | 14 +++++++------- test/tsconfig.json | 4 ++-- 8 files changed, 32 insertions(+), 36 deletions(-) rename src/rules/{memberNoDefaultAccessRule.ts => memberAccessRule.ts} (96%) create mode 100644 test/files/rules/memberaccess.test.ts delete mode 100644 test/files/rules/membernodefaultaccess.test.ts rename test/rules/{memberNoDefaultAccessTests.ts => memberAccessTests.ts} (71%) diff --git a/README.md b/README.md index f45cf5192f5..e74c56fa72b 100644 --- a/README.md +++ b/README.md @@ -144,7 +144,7 @@ A sample configuration file with all options is available [here](https://github. * `label-position` enforces labels only on sensible statements. * `label-undefined` checks that labels are defined before usage. * `max-line-length` sets the maximum length of a line. -* `member-no-default-access` enforces using explicit visibility on class members +* `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 diff --git a/docs/sample.tslint.json b/docs/sample.tslint.json index db0df27f252..0e7c1e8f1ed 100644 --- a/docs/sample.tslint.json +++ b/docs/sample.tslint.json @@ -19,7 +19,7 @@ "label-position": true, "label-undefined": true, "max-line-length": [true, 140], - "member-no-default-access": true, + "member-access": true, "member-ordering": [true, "public-before-private", "static-before-instance", diff --git a/src/rules/memberNoDefaultAccessRule.ts b/src/rules/memberAccessRule.ts similarity index 96% rename from src/rules/memberNoDefaultAccessRule.ts rename to src/rules/memberAccessRule.ts index cf7a39d2e21..2830ea2d463 100644 --- a/src/rules/memberNoDefaultAccessRule.ts +++ b/src/rules/memberAccessRule.ts @@ -22,10 +22,6 @@ export class Rule extends Lint.Rules.AbstractRule { } } -interface IModifiers { - hasAccessModifier: boolean; -} - export class MemberAccessWalker extends Lint.RuleWalker { constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) { super(sourceFile, options); @@ -45,7 +41,7 @@ export class MemberAccessWalker extends Lint.RuleWalker { ts.SyntaxKind.PublicKeyword, ts.SyntaxKind.PrivateKeyword, ts.SyntaxKind.ProtectedKeyword - ); + ); if (!hasAnyVisibilityModifiers) { this.addFailure(this.createFailure(node.getStart(), node.getWidth(), Rule.FAILURE_STRING)); diff --git a/src/rules/tsconfig.json b/src/rules/tsconfig.json index af4e25a2713..8b5f5ea9116 100644 --- a/src/rules/tsconfig.json +++ b/src/rules/tsconfig.json @@ -30,7 +30,7 @@ "./labelPositionRule.ts", "./labelUndefinedRule.ts", "./maxLineLengthRule.ts", - "./memberNoDefaultAccessRule.ts", + "./memberAccessRule.ts", "./memberOrderingRule.ts", "./noAnyRule.ts", "./noArgRule.ts", @@ -70,4 +70,4 @@ "./variableNameRule.ts", "./whitespaceRule.ts" ] -} +} \ No newline at end of file diff --git a/test/files/rules/memberaccess.test.ts b/test/files/rules/memberaccess.test.ts new file mode 100644 index 00000000000..0800b0c4d0a --- /dev/null +++ b/test/files/rules/memberaccess.test.ts @@ -0,0 +1,18 @@ +class Foo { + constructor() { + } + + public w: number; + private x: number; + protected y: number; + z: number; + + public barW(): any { + } + private barX(): any { + } + protected barY(): any { + } + barZ(): any { + } +} diff --git a/test/files/rules/membernodefaultaccess.test.ts b/test/files/rules/membernodefaultaccess.test.ts deleted file mode 100644 index ec05e491b31..00000000000 --- a/test/files/rules/membernodefaultaccess.test.ts +++ /dev/null @@ -1,18 +0,0 @@ -class Foo { - constructor() { - } - - public w: number; - private x: number; - protected y: number; - z: number; - - public barW(): any { - } - private barX(): any { - } - protected barY(): any { - } - barZ(): any { - } -} diff --git a/test/rules/memberNoDefaultAccessTests.ts b/test/rules/memberAccessTests.ts similarity index 71% rename from test/rules/memberNoDefaultAccessTests.ts rename to test/rules/memberAccessTests.ts index e1b1676aa63..dfe8704ec62 100644 --- a/test/rules/memberNoDefaultAccessTests.ts +++ b/test/rules/memberAccessTests.ts @@ -14,15 +14,15 @@ * limitations under the License. */ -describe("", () => { - it("disallow default access on member", () => { - let fileName = "rules/membernodefaultaccess.test.ts"; - let MemberNoDefaultAccessRule = Lint.Test.getRule("member-no-default-access"); - let actualFailures = Lint.Test.applyRuleOnFile(fileName, MemberNoDefaultAccessRule); +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], MemberNoDefaultAccessRule.FAILURE_STRING), - Lint.Test.createFailure(fileName, [16, 5], [17, 6], MemberNoDefaultAccessRule.FAILURE_STRING) + Lint.Test.createFailure(fileName, [8, 5], [8, 15], MemberAccessRule.FAILURE_STRING), + Lint.Test.createFailure(fileName, [16, 5], [17, 6], MemberAccessRule.FAILURE_STRING) ]); }); }); diff --git a/test/tsconfig.json b/test/tsconfig.json index 8ebe11a30dc..4fac2ec3566 100644 --- a/test/tsconfig.json +++ b/test/tsconfig.json @@ -47,7 +47,7 @@ "./rules/labelPositionRuleTests.ts", "./rules/labelUndefinedRuleTests.ts", "./rules/maxLineLengthRuleTests.ts", - "./rules/memberNoDefaultAccessTests.ts", + "./rules/memberAccessTests.ts", "./rules/memberOrderingRuleTests.ts", "./rules/noAnyRuleTests.ts", "./rules/noArgRuleTests.ts", @@ -87,4 +87,4 @@ "./rules/variableNameRuleTests.ts", "./rules/whitespaceRuleTests.ts" ] -} +} \ No newline at end of file From daf3f1f694b3ea5534fb80795f6feed8f4726bcc Mon Sep 17 00:00:00 2001 From: Jason Killian Date: Thu, 20 Aug 2015 14:29:48 -0400 Subject: [PATCH 20/21] Clean up naming in noConditionalAssignmentRule, add a couple tests --- src/rules/noConditionalAssignmentRule.ts | 36 +++++++++---------- test/files/rules/nocondassign.test.ts | 1 + .../rules/noConditionalAssignmentRuleTests.ts | 6 +++- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/rules/noConditionalAssignmentRule.ts b/src/rules/noConditionalAssignmentRule.ts index a1423d41fd4..86f74c4c46e 100644 --- a/src/rules/noConditionalAssignmentRule.ts +++ b/src/rules/noConditionalAssignmentRule.ts @@ -24,55 +24,55 @@ export class Rule extends Lint.Rules.AbstractRule { } class NoConditionalAssignmentWalker extends Lint.RuleWalker { - private inConditionalExpression = false; + private isInConditional = false; protected visitIfStatement(node: ts.IfStatement) { - this.checkExpressionForBinaryExpressions(node.expression); + this.validateConditionalExpression(node.expression); super.visitIfStatement(node); } protected visitWhileStatement(node: ts.WhileStatement) { - this.checkExpressionForBinaryExpressions(node.expression); + this.validateConditionalExpression(node.expression); super.visitWhileStatement(node); } protected visitDoStatement(node: ts.DoStatement) { - this.checkExpressionForBinaryExpressions(node.expression); + this.validateConditionalExpression(node.expression); super.visitWhileStatement(node); } protected visitForStatement(node: ts.ForStatement) { - if (node.condition) { - this.checkExpressionForBinaryExpressions(node.condition); + if (node.condition != null) { + this.validateConditionalExpression(node.condition); } - super.visitForStatement(node); } protected visitBinaryExpression(expression: ts.BinaryExpression) { - if (this.inConditionalExpression) { - this.checkBinaryExpressionForAssignment(expression); + if (this.isInConditional) { + this.checkForAssignment(expression); } super.visitBinaryExpression(expression); } - private checkExpressionForBinaryExpressions(expression: ts.Expression) { - this.inConditionalExpression = true; + private validateConditionalExpression(expression: ts.Expression) { + this.isInConditional = true; if (expression.kind === ts.SyntaxKind.BinaryExpression) { - this.checkBinaryExpressionForAssignment( expression); + // check for simple assignment in a conditional, like `if (a = 1) {` + this.checkForAssignment( expression); } + // walk the children of the conditional expression for nested assignments, like `if ((a = 1) && (b == 1)) {` this.walkChildren(expression); - this.inConditionalExpression = false; + this.isInConditional = false; } - private checkBinaryExpressionForAssignment(expression: ts.BinaryExpression) { - if (this.isAssignmentToken(expression.operatorToken.kind)) { + private checkForAssignment(expression: ts.BinaryExpression) { + if (this.isAssignmentToken(expression.operatorToken)) { this.addFailure(this.createFailure(expression.getStart(), expression.getWidth(), Rule.FAILURE_STRING)); } } - private isAssignmentToken(token: ts.SyntaxKind): boolean { - return token >= ts.SyntaxKind.FirstAssignment && token <= ts.SyntaxKind.LastAssignment; + private isAssignmentToken(token: ts.Node) { + return token.kind >= ts.SyntaxKind.FirstAssignment && token.kind <= ts.SyntaxKind.LastAssignment; } - } diff --git a/test/files/rules/nocondassign.test.ts b/test/files/rules/nocondassign.test.ts index cbbad767feb..c93a4edbf3e 100644 --- a/test/files/rules/nocondassign.test.ts +++ b/test/files/rules/nocondassign.test.ts @@ -32,3 +32,4 @@ if (x += 2) { } else if (h || (x <<= 4)) { } do { } while (x ^= 4) { } +while ((a = 5) && ((b == 4) || (c = 3))) diff --git a/test/rules/noConditionalAssignmentRuleTests.ts b/test/rules/noConditionalAssignmentRuleTests.ts index 29d5b39f131..ac9160cfb89 100644 --- a/test/rules/noConditionalAssignmentRuleTests.ts +++ b/test/rules/noConditionalAssignmentRuleTests.ts @@ -47,8 +47,12 @@ describe("", () => { it("should detect assignments in while conditionals", () => { const expectedFailure1 = createFailure([25, 8], [25, 13]); const expectedFailure2 = createFailure([26, 9], [26, 19]); + const expectedFailure3 = createFailure([35, 9], [35, 14]); + const expectedFailure4 = createFailure([35, 33], [35, 38]); Lint.Test.assertContainsFailure(actualFailures, expectedFailure1); Lint.Test.assertContainsFailure(actualFailures, expectedFailure2); + Lint.Test.assertContainsFailure(actualFailures, expectedFailure3); + Lint.Test.assertContainsFailure(actualFailures, expectedFailure4); }); it("should detect assignments in for conditionals", () => { @@ -59,6 +63,6 @@ describe("", () => { }); it("no false positives for rule", () => { - assert.lengthOf(actualFailures, 11); + assert.lengthOf(actualFailures, 13); }); }); From 799d1092eec3e648f3da195ab9fa65ed7d52e42e Mon Sep 17 00:00:00 2001 From: Jason Killian Date: Fri, 21 Aug 2015 11:55:02 -0400 Subject: [PATCH 21/21] Add usage info for tslint binary --- README.md | 4 ++-- src/tslint-cli.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index acc79947c24..969744690e1 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,7 @@ Please first ensure that the TypeScript source files compile correctly. ##### CLI - usage: `tslint [options...] [file ...]` + usage: `tslint [options] [file ...]` Options: @@ -46,7 +46,7 @@ Please first ensure that the TypeScript source files compile correctly. 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 commandline options: +tslint accepts the following command-line options: -c, --config: The location of the configuration file that tslint will use to diff --git a/src/tslint-cli.ts b/src/tslint-cli.ts index 24d56939a29..69370655149 100644 --- a/src/tslint-cli.ts +++ b/src/tslint-cli.ts @@ -21,7 +21,7 @@ module Lint { const fs = require("fs"); const optimist = require("optimist") - .usage("usage: $0") + .usage("Usage: $0 [options] [file ...]") .check((argv: any) => { // at least one of file, help, version or unqualified argument must be present if (!(argv.h || argv.v || argv._.length > 0)) { @@ -131,7 +131,7 @@ module Lint { The current version of tslint. -h, --help: - Prints this help message.`; + Prints this help message.\n`; outputStream.write(outputString); process.exit(0); }