Skip to content
This repository was archived by the owner on Mar 25, 2021. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
"compile:scripts": "tsc -p scripts",
"compile:test": "tsc -p test",
"lint": "npm-run-all -p lint:global lint:from-bin",
"lint:global": "tslint --project test/tsconfig.json --format stylish # test includes 'src' too",
"lint:from-bin": "node bin/tslint --project test/tsconfig.json --format stylish",
"lint:global": "tslint --project test/tsconfig.json --format stylish --type-check # test includes 'src' too",
"lint:from-bin": "node bin/tslint --project test/tsconfig.json --format stylish --type-check",
"test": "npm-run-all test:pre -p test:mocha test:rules",
"test:pre": "cd ./test/config && npm install",
"test:mocha": "mocha --reporter spec --colors \"build/test/**/*Tests.js\"",
Expand Down Expand Up @@ -72,7 +72,7 @@
"rimraf": "^2.5.4",
"tslint": "latest",
"tslint-test-config-non-relative": "file:test/external/tslint-test-config-non-relative",
"typescript": "^2.2.2"
"typescript": "^2.3.0"
},
"license": "Apache-2.0",
"engines": {
Expand Down
2 changes: 1 addition & 1 deletion src/configs/all.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ export const RULES_EXCLUDED_FROM_ALL_CONFIG =
// Exclude typescript-only rules from jsRules, otherwise it's identical.
export const jsRules: { [key: string]: any } = {};
for (const key in rules) {
if (!Object.prototype.hasOwnProperty.call(rules, key)) {
if (!(Object.prototype.hasOwnProperty.call(rules, key) as boolean)) {
continue;
}

Expand Down
10 changes: 7 additions & 3 deletions src/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
* limitations under the License.
*/

// tslint:disable no-unsafe-any (TODO)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this needed?


import findup = require("findup-sync");
import * as fs from "fs";
import * as path from "path";
Expand Down Expand Up @@ -157,8 +159,8 @@ export function loadConfigurationFromPath(configFilePath?: string): IConfigurati
let rawConfigFile: any;
if (path.extname(resolvedConfigFilePath) === ".json") {
const fileContent = stripComments(fs.readFileSync(resolvedConfigFilePath)
.toString()
.replace(/^\uFEFF/, ""));
.toString()
.replace(/^\uFEFF/, ""));
rawConfigFile = JSON.parse(fileContent);
} else {
rawConfigFile = require(resolvedConfigFilePath);
Expand Down Expand Up @@ -252,7 +254,7 @@ export function extendConfigurationFile(targetConfig: IConfigurationFile,
};
}

function getHomeDir() {
function getHomeDir(): string | undefined {
const environment = global.process.env;
const paths = [

@nchen63 nchen63 Apr 16, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer type annotation on paths instead of the function return value. or both

environment.USERPROFILE,
Expand All @@ -266,6 +268,8 @@ function getHomeDir() {
return homePath;
}
}

return undefined;
}

// returns the absolute path (contrary to what the name implies)
Expand Down
2 changes: 1 addition & 1 deletion src/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export class FatalError extends Error {
}

export function isError(possibleError: any): possibleError is Error {
return possibleError != null && possibleError.message !== undefined;
return possibleError != null && (possibleError as Error).message !== undefined;
}

export function showWarningOnce(message: string) {
Expand Down
12 changes: 6 additions & 6 deletions src/formatterLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@

import * as fs from "fs";
import * as path from "path";
import {FormatterFunction} from "./index";
import {FormatterConstructor} from "./index";
import {camelize} from "./utils";

const moduleDirectory = path.dirname(module.filename);
const CORE_FORMATTERS_DIRECTORY = path.resolve(moduleDirectory, ".", "formatters");

export function findFormatter(name: string | FormatterFunction, formattersDirectory?: string) {
export function findFormatter(name: string | FormatterConstructor, formattersDirectory?: string): FormatterConstructor | undefined {
if (typeof name === "function") {
return name;
} else if (typeof name === "string") {
Expand Down Expand Up @@ -52,24 +52,24 @@ export function findFormatter(name: string | FormatterFunction, formattersDirect
}
}

function loadFormatter(...paths: string[]) {
function loadFormatter(...paths: string[]): FormatterConstructor | undefined {
const formatterPath = paths.reduce((p, c) => path.join(p, c), "");
const fullPath = path.resolve(moduleDirectory, formatterPath);

if (fs.existsSync(`${fullPath}.js`)) {
const formatterModule = require(fullPath);
return formatterModule.Formatter;
return formatterModule.Formatter; // tslint:disable-line no-unsafe-any

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of disabling rule, cast the require to an interface like { Formatter: FormatterConstructor }

}

return undefined;
}

function loadFormatterModule(name: string) {
function loadFormatterModule(name: string): FormatterConstructor | undefined {
let src: string;
try {
src = require.resolve(name);
} catch (e) {
return undefined;
}
return require(src).Formatter;
return require(src).Formatter; // tslint:disable-line no-unsafe-any

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above comment

}
7 changes: 3 additions & 4 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import * as Configuration from "./configuration";
import * as Formatters from "./formatters";
import {FormatterConstructor} from "./language/formatter/formatter";
import {RuleFailure} from "./language/rule/rule";
import * as Linter from "./linter";
import * as Rules from "./rules";
Expand All @@ -38,15 +39,13 @@ export interface LintResult {
warningCount: number;
failures: RuleFailure[];
fixes?: RuleFailure[];
format: string | FormatterFunction;
format: string | FormatterConstructor;
output: string;
}

export type FormatterFunction = (failures: RuleFailure[]) => string;

export interface ILinterOptions {
fix: boolean;
formatter?: string | FormatterFunction;
formatter?: string | FormatterConstructor;
formattersDirectory?: string;
rulesDirectory?: string | string[];
}
4 changes: 4 additions & 0 deletions src/language/formatter/formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ export interface IFormatterMetadata {

export type ConsumerType = "human" | "machine";

export interface FormatterConstructor {
new(): IFormatter;
}

export interface IFormatter {
/**
* Formats linter results
Expand Down
5 changes: 5 additions & 0 deletions src/language/rule/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ import * as ts from "typescript";
import {arrayify, flatMap} from "../../utils";
import {IWalker} from "../walker";

export interface RuleConstructor {
metadata: IRuleMetadata;
new(options: IOptions): IRule;
}

export interface IRuleMetadata {
/**
* The kebab-case name of the rule.
Expand Down
3 changes: 2 additions & 1 deletion src/language/walker/blockScopeAwareRuleWalker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import * as ts from "typescript";

import {IOptions} from "../rule/rule";
import {isBlockScopeBoundary} from "../utils";
import {ScopeAwareRuleWalker} from "./scopeAwareRuleWalker";

Expand All @@ -27,7 +28,7 @@ import {ScopeAwareRuleWalker} from "./scopeAwareRuleWalker";
export abstract class BlockScopeAwareRuleWalker<T, U> extends ScopeAwareRuleWalker<T> {
private blockScopeStack: U[];

constructor(sourceFile: ts.SourceFile, options?: any) {
constructor(sourceFile: ts.SourceFile, options: IOptions) {
super(sourceFile, options);

// initialize with global scope if file is not a module
Expand Down
3 changes: 2 additions & 1 deletion src/language/walker/scopeAwareRuleWalker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@

import * as ts from "typescript";

import {IOptions} from "../rule/rule";
import {isScopeBoundary} from "../utils";
import {RuleWalker} from "./ruleWalker";

export abstract class ScopeAwareRuleWalker<T> extends RuleWalker {
private scopeStack: T[];

constructor(sourceFile: ts.SourceFile, options?: any) {
constructor(sourceFile: ts.SourceFile, options: IOptions) {
super(sourceFile, options);

// initialize with global scope if file is not a module
Expand Down
31 changes: 15 additions & 16 deletions src/ruleLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,12 @@ import * as path from "path";

import { getRelativePath } from "./configuration";
import { showWarningOnce } from "./error";
import { AbstractRule } from "./language/rule/abstractRule";
import { IDisabledInterval, IOptions, IRule } from "./language/rule/rule";
import { IDisabledInterval, IOptions, IRule, RuleConstructor } from "./language/rule/rule";
import { arrayify, camelize, dedent } from "./utils";

const moduleDirectory = path.dirname(module.filename);
const CORE_RULES_DIRECTORY = path.resolve(moduleDirectory, ".", "rules");
const cachedRules = new Map<string, typeof AbstractRule | null>(); // null indicates that the rule was not found

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually I found this to be clearer

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But rules aren't actually guaranteed to be implemented by AbstractRule subclasses, are they?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that it's renamed to RuleConstructor I'm fine with it.

const cachedRules = new Map<string, RuleConstructor | null>(); // null indicates that the rule was not found

export interface IEnableDisablePosition {
isEnabled: boolean;
Expand All @@ -45,7 +44,7 @@ export function loadRules(ruleOptionsList: IOptions[],
const ruleName = ruleOptions.ruleName;
const enableDisableRules = enableDisableRuleMap.get(ruleName);
if (ruleOptions.ruleSeverity !== "off" || enableDisableRuleMap) {
const Rule: (typeof AbstractRule) | null = findRule(ruleName, rulesDirectories);
const Rule = findRule(ruleName, rulesDirectories);
if (Rule == null) {
notFoundRules.push(ruleName);
} else {
Expand All @@ -54,7 +53,7 @@ export function loadRules(ruleOptionsList: IOptions[],
} else {
const ruleSpecificList = enableDisableRules || [];
ruleOptions.disabledIntervals = buildDisabledIntervalsFromSwitches(ruleSpecificList);
rules.push(new (Rule as any)(ruleOptions));
rules.push(new (Rule as any)(ruleOptions) as IRule);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need either cast


if (Rule.metadata && Rule.metadata.deprecationMessage) {
showWarningOnce(`${Rule.metadata.ruleName} is deprecated. ${Rule.metadata.deprecationMessage}`);
Expand Down Expand Up @@ -89,9 +88,9 @@ export function loadRules(ruleOptionsList: IOptions[],
return rules;
}

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

// first check for core rules
Rule = loadCachedRule(CORE_RULES_DIRECTORY, camelizedName);
Expand All @@ -109,7 +108,7 @@ export function findRule(name: string, rulesDirectories?: string | string[]) {
return Rule;
}

function transformName(name: string) {
function transformName(name: string): string {
// camelize strips out leading and trailing underscores and dashes, so make sure they aren't passed to camelize
// the regex matches the groups (leading underscores and dashes)(other characters)(trailing underscores and dashes)
const nameMatch = name.match(/^([-_]*)(.*?)([-_]*)$/);
Expand All @@ -123,18 +122,18 @@ function transformName(name: string) {
* @param directory - An absolute path to a directory of rules
* @param ruleName - A name of a rule in filename format. ex) "someLintRule"
*/
function loadRule(directory: string, ruleName: string) {
function loadRule(directory: string, ruleName: string): RuleConstructor | null {
const fullPath = path.join(directory, ruleName);
if (fs.existsSync(fullPath + ".js")) {
const ruleModule = require(fullPath);
if (ruleModule && ruleModule.Rule) {
const ruleModule = require(fullPath) as { Rule: RuleConstructor } | undefined;
if (ruleModule !== undefined) {
return ruleModule.Rule;
}
}
return undefined;
return null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the switch to nulls? can't we use undefined?

@andy-hanson andy-hanson Apr 13, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use cachedRules.set(fullPath, Rule);, so if Rule is undefined, it will look like we didn't cache a result at all. I actually already had to do this in #2369.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should use a string literal like "not-found" or "load-error" instead of null

}

function loadCachedRule(directory: string, ruleName: string, isCustomPath = false) {
function loadCachedRule(directory: string, ruleName: string, isCustomPath = false): RuleConstructor | null {
// use cached value if available
const fullPath = path.join(directory, ruleName);
const cachedRule = cachedRules.get(fullPath);
Expand All @@ -153,9 +152,9 @@ function loadCachedRule(directory: string, ruleName: string, isCustomPath = fals
}
}

let Rule: typeof AbstractRule | null = null;
if (absolutePath != null) {
Rule = loadRule(absolutePath, ruleName);
let Rule: RuleConstructor | null = null;
if (absolutePath !== undefined) {
Rule = loadRule(absolutePath, ruleName); // tslint:disable-line no-unsafe-any

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need to disable the rule here

}
cachedRules.set(fullPath, Rule);
return Rule;
Expand Down
2 changes: 1 addition & 1 deletion src/rules/alignRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class AlignWalker extends Lint.AbstractWalker<Options> {
} else if (this.options.arguments &&
(node.kind === ts.SyntaxKind.CallExpression ||
node.kind === ts.SyntaxKind.NewExpression && (node as ts.NewExpression).arguments !== undefined)) {
this.checkAlignment((node as ts.CallExpression | ts.NewExpression).arguments, Rule.ARGUMENTS_OPTION);
this.checkAlignment((node as ts.CallExpression | ts.NewExpression).arguments!, Rule.ARGUMENTS_OPTION);
}
return ts.forEachChild(node, cb);
};
Expand Down
2 changes: 1 addition & 1 deletion src/rules/banRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export class Rule extends Lint.Rules.AbstractRule {
public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
const options = this.getOptions();
const banFunctionWalker = new BanFunctionWalker(sourceFile, options);
const functionsToBan = options.ruleArguments;
const functionsToBan = options.ruleArguments as string[][];
if (functionsToBan !== undefined) {
functionsToBan.forEach((f) => banFunctionWalker.addBannedFunction(f));
}
Expand Down
2 changes: 1 addition & 1 deletion src/rules/curlyRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class CurlyWalker extends Lint.RuleWalker {
constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) {
super(sourceFile, options);

const args = this.getOptions();
const args = this.getOptions() as any[];

this.optionIgnoreSameLine = args.indexOf(OPTION_IGNORE_SAME_LINE) > -1;
}
Expand Down
2 changes: 1 addition & 1 deletion src/rules/cyclomaticComplexityRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export class Rule extends Lint.Rules.AbstractRule {

private get threshold(): number {
if (this.ruleArguments[0] !== undefined) {
return this.ruleArguments[0];
return this.ruleArguments[0] as number;
}
return Rule.DEFAULT_THRESHOLD;
}
Expand Down
2 changes: 1 addition & 1 deletion src/rules/fileHeaderRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export class Rule extends Lint.Rules.AbstractRule {
const { text } = sourceFile;
// ignore shebang if it exists
const offset = text.startsWith("#!") ? text.indexOf("\n") + 1 : 0;
if (!textHasComment(text, offset, new RegExp(this.ruleArguments[0]))) {
if (!textHasComment(text, offset, new RegExp(this.ruleArguments[0] as string))) {
return [new Lint.RuleFailure(sourceFile, offset, offset, Rule.FAILURE_STRING, this.ruleName)];
}
return [];
Expand Down
8 changes: 3 additions & 5 deletions src/rules/maxClassesPerFileRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,11 @@ class MaxClassesPerFileWalker extends Lint.RuleWalker {
constructor(sourceFile: ts.SourceFile, options: Lint.IOptions) {
super(sourceFile, options);

if (options.ruleArguments[0] === undefined
|| isNaN(options.ruleArguments[0])
|| options.ruleArguments[0] < 1) {

const option = options.ruleArguments[0] as number | undefined;
if (option === undefined || isNaN(option) || option < 1) {
this.maxClassCount = 1;
} else {
this.maxClassCount = options.ruleArguments[0];
this.maxClassCount = option;
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/rules/maxFileLineCountRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@ export class Rule extends Lint.Rules.AbstractRule {
}

public isEnabled(): boolean {
return super.isEnabled() && this.ruleArguments[0] > 0;
return super.isEnabled() && this.ruleArguments[0] as number > 0;
}

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
const ruleFailures: Lint.RuleFailure[] = [];
const ruleArguments = this.getOptions().ruleArguments;
const lineLimit: number = ruleArguments[0];
const lineCount: number = sourceFile.getLineStarts().length;
const lineLimit = ruleArguments[0] as number;
const lineCount = sourceFile.getLineStarts().length;
const disabledIntervals = this.getOptions().disabledIntervals;

if (lineCount > lineLimit && disabledIntervals.length === 0) {
Expand Down
2 changes: 1 addition & 1 deletion src/rules/maxLineLengthRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export class Rule extends Lint.Rules.AbstractRule {
}

public isEnabled(): boolean {
return super.isEnabled() && this.ruleArguments[0] > 0;
return super.isEnabled() && this.ruleArguments[0] as number > 0;
}

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
Expand Down
Loading