Skip to content

Commit

Permalink
Enable more rules in codebase (palantir#1978)
Browse files Browse the repository at this point in the history
* recommended.ts code style cleanup

* Enable file-header rule

* Small code style cleanup

* Enable prefer-const rule and auto-fix failures

* Enable interface-over-type-literal, no-default-export, no-empty-interface

* Enable callable-types rule

* Fix various minor lint errors

* No need to disable no-unused-variable in tslint.json

* Enable no-string-throw rule; auto-fix failures

* Remove BOM from maxClassesPerFile.ts
  • Loading branch information
adidahiya authored and nchen63 committed Jan 4, 2017
1 parent 7c841c0 commit 6dcf3f0
Show file tree
Hide file tree
Showing 42 changed files with 271 additions and 123 deletions.
54 changes: 24 additions & 30 deletions src/configs/recommended.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ export const rules = {
"max-classes-per-file": [true, 1],
"max-line-length": [true, 120],
"member-access": true,
"member-ordering": [true,
{ "order": "statics-first" },
],
"member-ordering": [true, {
"order": "statics-first",
}],
"new-parens": true,
"no-any": false,
"no-arg": true,
Expand Down Expand Up @@ -98,29 +98,25 @@ export const rules = {
"radix": true,
"semicolon": [true, "always"],
"switch-default": true,
"trailing-comma": [true,
{
"multiline": "always",
"singleline": "never",
},
],
"trailing-comma": [true, {
"multiline": "always",
"singleline": "never",
}],
"triple-equals": [true, "allow-null-check"],
"typedef": false,
"typedef-whitespace": [true,
{
"call-signature": "nospace",
"index-signature": "nospace",
"parameter": "nospace",
"property-declaration": "nospace",
"variable-declaration": "nospace",
}, {
"call-signature": "onespace",
"index-signature": "onespace",
"parameter": "onespace",
"property-declaration": "onespace",
"variable-declaration": "onespace",
},
],
"typedef-whitespace": [true, {
"call-signature": "nospace",
"index-signature": "nospace",
"parameter": "nospace",
"property-declaration": "nospace",
"variable-declaration": "nospace",
}, {
"call-signature": "onespace",
"index-signature": "onespace",
"parameter": "onespace",
"property-declaration": "onespace",
"variable-declaration": "onespace",
}],
"use-isnan": true,
"variable-name": [true,
"ban-keywords",
Expand Down Expand Up @@ -191,12 +187,10 @@ export const jsRules = {
"radix": true,
"semicolon": [true, "always"],
"switch-default": true,
"trailing-comma": [true,
{
"multiline": "always",
"singleline": "never",
},
],
"trailing-comma": [true, {
"multiline": "always",
"singleline": "never",
}],
"triple-equals": [true, "allow-null-check"],
"use-isnan": true,
"variable-name": [true,
Expand Down
4 changes: 2 additions & 2 deletions src/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,14 +225,14 @@ function resolveConfigurationPath(filePath: string, relativeTo?: string) {

export function extendConfigurationFile(targetConfig: IConfigurationFile,
nextConfigSource: IConfigurationFile): IConfigurationFile {
let combinedConfig: IConfigurationFile = {};
const combinedConfig: IConfigurationFile = {};

const configRulesDirectory = arrayify(targetConfig.rulesDirectory);
const nextConfigRulesDirectory = arrayify(nextConfigSource.rulesDirectory);
combinedConfig.rulesDirectory = configRulesDirectory.concat(nextConfigRulesDirectory);

const combineProperties = (targetProperty: any, nextProperty: any) => {
let combinedProperty: any = {};
const combinedProperty: any = {};
for (const name of Object.keys(objectify(targetProperty))) {
combinedProperty[name] = targetProperty[name];
}
Expand Down
21 changes: 19 additions & 2 deletions src/formatters/checkstyleFormatter.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,20 @@
/**
* @license
* Copyright 2016 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.
*/

import {AbstractFormatter} from "../language/formatter/abstractFormatter";
import {IFormatterMetadata} from "../language/formatter/formatter";
import {RuleFailure} from "../language/rule/rule";
Expand Down Expand Up @@ -26,11 +43,11 @@ export class Formatter extends AbstractFormatter {
let output = '<?xml version="1.0" encoding="utf-8"?><checkstyle version="4.3">';

if (failures.length) {
let failuresSorted = failures.sort((a, b) => {
const failuresSorted = failures.sort((a, b) => {
return a.getFileName().localeCompare(b.getFileName());
});
let previousFilename: string | null = null;
for (let failure of failuresSorted) {
for (const failure of failuresSorted) {
if (failure.getFileName() !== previousFilename) {
if (previousFilename) {
output += "</file>";
Expand Down
1 change: 1 addition & 0 deletions src/formatters/msbuildFormatter.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/**
* @license
* Copyright 2016 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.
Expand Down
2 changes: 1 addition & 1 deletion src/formatters/pmdFormatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export class Formatter extends AbstractFormatter {
public format(failures: RuleFailure[]): string {
let output = "<pmd version=\"tslint\">";

for (let failure of failures) {
for (const failure of failures) {
const failureString = failure.getFailure()
.replace(/&/g, "&amp;")
.replace(/</g, "&lt;")
Expand Down
6 changes: 3 additions & 3 deletions src/formatters/proseFormatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ export class Formatter extends AbstractFormatter {
return "";
}

let fixLines: string[] = [];
const fixLines: string[] = [];
if (fixes) {
let perFileFixes: { [fileName: string]: number } = {};
const perFileFixes: { [fileName: string]: number } = {};
for (const fix of fixes) {
if (perFileFixes[fix.getFileName()] == null) {
perFileFixes[fix.getFileName()] = 1;
Expand All @@ -52,7 +52,7 @@ export class Formatter extends AbstractFormatter {
fixLines.push(""); // add a blank line between fixes and failures
}

let errorLines = failures.map((failure: RuleFailure) => {
const errorLines = failures.map((failure: RuleFailure) => {
const fileName = failure.getFileName();
const failureString = failure.getFailure();

Expand Down
1 change: 1 addition & 0 deletions src/formatters/vsoFormatter.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/**
* @license
* Copyright 2016 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.
Expand Down
12 changes: 6 additions & 6 deletions src/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ class Linter {
let fileFailures: RuleFailure[] = [];

if (this.options.fix) {
for (let rule of enabledRules) {
let ruleFailures = this.applyRule(rule, sourceFile);
for (const rule of enabledRules) {
const ruleFailures = this.applyRule(rule, sourceFile);
const fixes = ruleFailures.map((f) => f.getFix()).filter((f): f is Fix => !!f) as Fix[];
source = fs.readFileSync(fileName, { encoding: "utf-8" });
if (fixes.length > 0) {
Expand All @@ -129,7 +129,7 @@ class Linter {
// make a 1st pass or make a 2nd pass if there were any fixes because the positions may be off
if (!hasLinterRun || this.fixes.length > 0) {
fileFailures = [];
for (let rule of enabledRules) {
for (const rule of enabledRules) {
const ruleFailures = this.applyRule(rule, sourceFile);
if (ruleFailures.length > 0) {
fileFailures = fileFailures.concat(ruleFailures);
Expand Down Expand Up @@ -171,8 +171,8 @@ class Linter {
} else {
ruleFailures = rule.apply(sourceFile, this.languageService);
}
let fileFailures: RuleFailure[] = [];
for (let ruleFailure of ruleFailures) {
const fileFailures: RuleFailure[] = [];
for (const ruleFailure of ruleFailures) {
if (!this.containsRule(this.failures, ruleFailure)) {
fileFailures.push(ruleFailure);
}
Expand All @@ -196,7 +196,7 @@ class Linter {

const rulesDirectories = arrayify(this.options.rulesDirectory)
.concat(arrayify(configuration.rulesDirectory));
let configuredRules = loadRules(configurationRules, enableDisableRuleMap, rulesDirectories, isJs);
const configuredRules = loadRules(configurationRules, enableDisableRuleMap, rulesDirectories, isJs);

return configuredRules.filter((r) => r.isEnabled());
}
Expand Down
6 changes: 3 additions & 3 deletions src/ruleLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,17 +89,17 @@ export function loadRules(ruleConfiguration: {[name: string]: any},
}

export function findRule(name: string, rulesDirectories?: string | string[]) {
let camelizedName = transformName(name);
const camelizedName = transformName(name);

// first check for core rules
let Rule = loadRule(CORE_RULES_DIRECTORY, camelizedName);
if (Rule != null) {
return Rule;
}

let directories = getRulesDirectories(rulesDirectories);
const directories = getRulesDirectories(rulesDirectories);

for (let rulesDirectory of directories) {
for (const rulesDirectory of directories) {
// then check for rules within the first level of rulesDirectory
if (rulesDirectory != null) {
Rule = loadRule(rulesDirectory, camelizedName);
Expand Down
2 changes: 1 addition & 1 deletion src/rules/alignRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class AlignWalker extends Lint.RuleWalker {
const alignToColumn = prevPos.character;

// skip first node in list
for (let node of nodes.slice(1)) {
for (const node of nodes.slice(1)) {
const curPos = this.getLineAndCharacterOfPosition(node.getStart());
if (curPos.line !== prevPos.line && curPos.character !== alignToColumn) {
this.addFailureAtNode(node, kind + Rule.FAILURE_STRING_SUFFIX);
Expand Down
17 changes: 17 additions & 0 deletions src/rules/arrayTypeRule.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,20 @@
/**
* @license
* Copyright 2016 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.
*/

import * as ts from "typescript";

import * as Lint from "../index";
Expand Down
18 changes: 17 additions & 1 deletion src/rules/fileHeaderRule.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,21 @@
import * as ts from "typescript";
/**
* @license
* Copyright 2016 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.
*/

import * as ts from "typescript";
import * as Lint from "../index";

export class Rule extends Lint.Rules.AbstractRule {
Expand Down
3 changes: 2 additions & 1 deletion src/rules/importBlacklistRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ class NoRequireFullLibraryWalker extends Lint.RuleWalker {

private reportFailure (node: ts.Expression): void {
this.addFailureAt(
node.getStart() + 1, // take quotes into account
// take quotes into account
node.getStart() + 1,
node.getWidth() - 2,
Rule.FAILURE_STRING,
);
Expand Down
2 changes: 1 addition & 1 deletion src/rules/indentRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class IndentWalker extends Lint.RuleWalker {
let endOfComment = -1;
let endOfTemplateString = -1;
const scanner = ts.createScanner(ts.ScriptTarget.ES5, false, ts.LanguageVariant.Standard, node.text);
for (let lineStart of node.getLineStarts()) {
for (const lineStart of node.getLineStarts()) {
if (lineStart < endOfComment || lineStart < endOfTemplateString) {
// skip checking lines inside multi-line comments or template strings
continue;
Expand Down
5 changes: 2 additions & 3 deletions src/rules/interfaceOverTypeLiteralRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
*/

import * as ts from "typescript";

import * as Lint from "../index";

export class Rule extends Lint.Rules.AbstractRule {
Expand All @@ -35,11 +34,11 @@ export class Rule extends Lint.Rules.AbstractRule {
public static FAILURE_STRING = "Use an interface instead of a type literal.";

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithWalker(new Walker(sourceFile, this.getOptions()));
return this.applyWithWalker(new InterfaceOverTypeLiteralWalker(sourceFile, this.getOptions()));
}
}

class Walker extends Lint.RuleWalker {
class InterfaceOverTypeLiteralWalker extends Lint.RuleWalker {
public visitTypeAliasDeclaration(node: ts.TypeAliasDeclaration) {
if (node.type.kind === ts.SyntaxKind.TypeLiteral) {
this.addFailureAtNode(node.name, Rule.FAILURE_STRING);
Expand Down
2 changes: 1 addition & 1 deletion src/rules/jsdocFormatRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class JsdocWalker extends Lint.SkippableTokenAwareRuleWalker {
// all lines but the first and last
const otherLines = lines.splice(1, lines.length - 2);
jsdocPosition += firstLine.length + 1; // + 1 for the splitted-out newline
for (let line of otherLines) {
for (const line of otherLines) {
// regex is: start of string, followed by any amount of whitespace, followed by *,
// followed by either a space or the end of the string
const asteriskMatch = line.match(/^\s*\*( |$)/);
Expand Down
19 changes: 18 additions & 1 deletion src/rules/maxClassesPerFileRule.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,21 @@
import * as ts from "typescript";
/**
* @license
* Copyright 2016 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.
*/

import * as ts from "typescript";
import * as Lint from "../index";

export class Rule extends Lint.Rules.AbstractRule {
Expand Down
1 change: 1 addition & 0 deletions src/rules/maxFileLineCountRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as ts from "typescript";
import * as Lint from "../index";

Expand Down
2 changes: 1 addition & 1 deletion src/rules/noConsecutiveBlankLinesRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class NoConsecutiveBlankLinesWalker extends Lint.RuleWalker {
const soureFileLines = sourceFileText.split(/\n/);

// find all the lines that are blank or only contain whitespace
let blankLineIndexes: number[] = [];
const blankLineIndexes: number[] = [];
soureFileLines.forEach((txt, i) => {
if (txt.trim() === "") {
blankLineIndexes.push(i);
Expand Down
Loading

0 comments on commit 6dcf3f0

Please sign in to comment.