Skip to content
This repository was archived by the owner on Mar 25, 2021. It is now read-only.

Lint for strict-boolean-expressions #2408

Merged
merged 4 commits into from
May 9, 2017
Merged

Lint for strict-boolean-expressions #2408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c22f406
Lint for strict-boolean-expressions
andy-hanson Mar 25, 2017
bccbe4e
Merge branch 'master' into strict-boolean-expressions
andy-hanson May 7, 2017
c333e46
Respond to PR comments
andy-hanson May 9, 2017
520f8b9
Merge branch 'master' into strict-boolean-expressions
andy-hanson May 9, 2017
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
24 changes: 12 additions & 12 deletions src/configuration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,9 @@ export function findConfigurationPath(suppliedConfigFilePath: string | null, inp
* This is case-insensitive, so it can find 'TsLiNt.JsOn' when searching for 'tslint.json'.
*/
function findup(filename: string, directory: string): string | undefined {
while (true) {
while (true) { // tslint:disable-line strict-boolean-expressions
const res = findFile(directory);
if (res) {
if (res !== undefined) {
return res;
}

Expand Down Expand Up @@ -236,7 +236,7 @@ function resolveConfigurationPath(filePath: string, relativeTo?: string) {
}
}

const basedir = relativeTo || process.cwd();
const basedir = relativeTo !== undefined ? relativeTo : process.cwd();
try {
return resolve.sync(filePath, { basedir });
} catch (err) {
Expand All @@ -261,7 +261,7 @@ export function extendConfigurationFile(targetConfig: IConfigurationFile,
return combinedProperty as T;

function add(property: T | undefined): void {
if (property) {
if (property !== undefined) {
for (const name in property) {
if (hasOwnProperty(property, name)) {
combinedProperty[name] = property[name];
Expand Down Expand Up @@ -321,7 +321,7 @@ function getHomeDir(): string | undefined {
// returns the absolute path (contrary to what the name implies)
export function getRelativePath(directory?: string | null, relativeTo?: string) {
if (directory != null) {
const basePath = relativeTo || process.cwd();
const basePath = relativeTo !== undefined ? relativeTo : process.cwd();
return path.resolve(basePath, directory);
}
return undefined;
Expand Down Expand Up @@ -371,7 +371,7 @@ function parseRuleOptions(ruleConfigValue: RawRuleConfig, rawDefaultRuleSeverity
let ruleArguments: any[] | undefined;
let defaultRuleSeverity: RuleSeverity = "error";

if (rawDefaultRuleSeverity) {
if (rawDefaultRuleSeverity !== undefined) {
switch (rawDefaultRuleSeverity.toLowerCase()) {
case "warn":
case "warning":
Expand Down Expand Up @@ -402,7 +402,7 @@ function parseRuleOptions(ruleConfigValue: RawRuleConfig, rawDefaultRuleSeverity
ruleArguments = [];
ruleSeverity = ruleConfigValue === true ? defaultRuleSeverity : "off";
} else if (typeof ruleConfigValue === "object") {
if (ruleConfigValue.severity) {
if (ruleConfigValue.severity !== undefined) {
switch (ruleConfigValue.severity.toLowerCase()) {
case "default":
ruleSeverity = defaultRuleSeverity;
Expand Down Expand Up @@ -460,14 +460,14 @@ export function parseConfigFile(configFile: RawConfigFile, configFileDir?: strin
return {
extends: arrayify(configFile.extends),
jsRules: parseRules(configFile.jsRules),
linterOptions: configFile.linterOptions || {},
linterOptions: configFile.linterOptions !== undefined ? configFile.linterOptions : {},
rules: parseRules(configFile.rules),
rulesDirectory: getRulesDirectories(configFile.rulesDirectory, configFileDir),
};

function parseRules(config: RawRulesConfig | undefined): Map<string, Partial<IOptions>> {
const map = new Map<string, Partial<IOptions>>();
if (config) {
if (config !== undefined) {
for (const ruleName in config) {
if (hasOwnProperty(config, ruleName)) {
map.set(ruleName, parseRuleOptions(config[ruleName], configFile.defaultSeverity));
Expand All @@ -483,12 +483,12 @@ export function parseConfigFile(configFile: RawConfigFile, configFileDir?: strin
*/
export function convertRuleOptions(ruleConfiguration: Map<string, Partial<IOptions>>): IOptions[] {
const output: IOptions[] = [];
ruleConfiguration.forEach((partialOptions, ruleName) => {
ruleConfiguration.forEach(({ ruleArguments, ruleSeverity }, ruleName) => {
const options: IOptions = {
disabledIntervals: [], // deprecated, so just provide an empty array.
ruleArguments: partialOptions.ruleArguments || [],
ruleArguments: ruleArguments != null ? ruleArguments : [],
ruleName,
ruleSeverity: partialOptions.ruleSeverity || "error",
ruleSeverity: ruleSeverity != null ? ruleSeverity : "error",
};
output.push(options);
});
Expand Down
20 changes: 11 additions & 9 deletions src/enableDisableRules.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import * as ts from "typescript";
import { RuleFailure } from "./language/rule/rule";

export function removeDisabledFailures(sourceFile: ts.SourceFile, failures: RuleFailure[]): RuleFailure[] {
if (!failures.length) {
if (failures.length === 0) {
// Usually there won't be failures anyway, so no need to look for "tslint:disable".
return failures;
}
Expand All @@ -31,7 +31,7 @@ export function removeDisabledFailures(sourceFile: ts.SourceFile, failures: Rule
const map = getDisableMap(sourceFile, failingRules);
return failures.filter((failure) => {
const disabledIntervals = map.get(failure.getRuleName());
return !disabledIntervals || !disabledIntervals.some(({ pos, end }) => {
return disabledIntervals === undefined || !disabledIntervals.some(({ pos, end }) => {
const failPos = failure.getStartPosition().getPosition();
const failEnd = failure.getEndPosition().getPosition();
return failEnd >= pos && (end === -1 || failPos <= end);
Expand All @@ -51,10 +51,10 @@ function getDisableMap(sourceFile: ts.SourceFile, failingRules: Set<string>): Re
? fullText.substring(comment.pos + 2, comment.end)
: fullText.substring(comment.pos + 2, comment.end - 2);
const parsed = parseComment(commentText);
if (parsed) {
if (parsed !== undefined) {
const { rulesList, isEnabled, modifier } = parsed;
const switchRange = getSwitchRange(modifier, comment, sourceFile);
if (switchRange) {
if (switchRange !== undefined) {
const rulesToSwitch = rulesList === "all" ? Array.from(failingRules) : rulesList.filter((r) => failingRules.has(r));
for (const ruleToSwitch of rulesToSwitch) {
switchRuleState(ruleToSwitch, isEnabled, switchRange.pos, switchRange.end);
Expand All @@ -69,7 +69,7 @@ function getDisableMap(sourceFile: ts.SourceFile, failingRules: Set<string>): Re
const disableRanges = map.get(ruleName);

if (isEnable) {
if (disableRanges) {
if (disableRanges !== undefined) {
const lastDisable = disableRanges[disableRanges.length - 1];
if (lastDisable.end === -1) {
lastDisable.end = start;
Expand All @@ -80,7 +80,7 @@ function getDisableMap(sourceFile: ts.SourceFile, failingRules: Set<string>): Re
}
}
} else { // disable
if (!disableRanges) {
if (disableRanges === undefined) {
map.set(ruleName, [{ pos: start, end }]);
} else if (disableRanges[disableRanges.length - 1].end !== -1) {
disableRanges.push({ pos: start, end });
Expand Down Expand Up @@ -138,9 +138,7 @@ function parseComment(commentText: string): { rulesList: string[] | "all", isEna
// remove everything matched by the previous regex to get only the specified rules
// split at whitespaces
// filter empty items coming from whitespaces at start, at end or empty list
let rulesList: string[] | "all" = commentText.substr(match[0].length)
.split(/\s+/)
.filter((rule) => !!rule);
let rulesList: string[] | "all" = splitOnSpaces(commentText.substr(match[0].length));
if (rulesList.length === 0 && match[3] === ":") {
// nothing to do here: an explicit separator was specified but no rules to switch
return undefined;
Expand All @@ -154,3 +152,7 @@ function parseComment(commentText: string): { rulesList: string[] | "all", isEna

return { rulesList, isEnabled: match[1] === "enable", modifier: match[2] as Modifier };
}

function splitOnSpaces(str: string): string[] {
return str.split(/\s+/).filter((s) => s !== "");
}
4 changes: 2 additions & 2 deletions src/formatterLoader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ export function findFormatter(name: string | FormatterConstructor, formattersDir
}

// then check for rules within the first level of rulesDirectory
if (formattersDirectory) {
if (formattersDirectory !== undefined) {
Formatter = loadFormatter(formattersDirectory, camelizedName);
if (Formatter) {
if (Formatter !== undefined) {
return Formatter;
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/formatters/checkstyleFormatter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,15 @@ export class Formatter extends AbstractFormatter {
public format(failures: RuleFailure[]): string {
let output = '<?xml version="1.0" encoding="utf-8"?><checkstyle version="4.3">';

if (failures.length) {
if (failures.length !== 0) {
const failuresSorted = failures.sort((a, b) => {
return a.getFileName().localeCompare(b.getFileName());
});
let previousFilename: string | null = null;
for (const failure of failuresSorted) {
const severity = failure.getRuleSeverity();
if (failure.getFileName() !== previousFilename) {
if (previousFilename) {
if (previousFilename !== null) {
output += "</file>";
}
previousFilename = failure.getFileName();
Expand All @@ -63,7 +63,7 @@ export class Formatter extends AbstractFormatter {
// checkstyle parser wants "source" to have structure like <anything>dot<category>dot<type>
output += "source=\"failure.tslint." + this.escapeXml(failure.getRuleName()) + "\" />";
}
if (previousFilename) {
if (previousFilename !== null) {
output += "</file>";
}
}
Expand Down
7 changes: 4 additions & 3 deletions src/formatters/proseFormatter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,16 @@ export class Formatter extends AbstractFormatter {
/* tslint:enable:object-literal-sort-keys */

public format(failures: RuleFailure[], fixes?: RuleFailure[]): string {
if (failures.length === 0 && (!fixes || fixes.length === 0)) {
if (failures.length === 0 && (fixes === undefined || fixes.length === 0)) {
return "\n";
}

const fixLines: string[] = [];
if (fixes) {
if (fixes !== undefined) {
const perFileFixes = new Map<string, number>();
for (const fix of fixes) {
perFileFixes.set(fix.getFileName(), (perFileFixes.get(fix.getFileName()) || 0) + 1);
const prevFixes = perFileFixes.get(fix.getFileName());
perFileFixes.set(fix.getFileName(), (prevFixes !== undefined ? prevFixes : 0) + 1);
}

perFileFixes.forEach((fixCount, fixedFile) => {
Expand Down
2 changes: 1 addition & 1 deletion src/formatters/stylishFormatter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export class Formatter extends AbstractFormatter {
}

private mapToMessages(failures: RuleFailure[]): string[] {
if (!failures) {
if (failures.length === 0) {
return [];
}
const outputLines: string[] = [];
Expand Down
2 changes: 1 addition & 1 deletion src/formatters/vsoFormatter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export class Formatter extends AbstractFormatter {
const lineAndCharacter = failure.getStartPosition().getLineAndCharacter();
const line = lineAndCharacter.line + 1;
const character = lineAndCharacter.character + 1;
const code = (failure.getRuleName ? failure.getRuleName() : "");
const code = failure.getRuleName();
Copy link
Contributor

Choose a reason for hiding this comment

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

nice

const properties = `sourcepath=${fileName};linenumber=${line};columnnumber=${character};code=${code};`;

return `##vso[task.logissue type=warning;${properties}]${failureString}`;
Expand Down
4 changes: 2 additions & 2 deletions src/language/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export function doesIntersect(failure: RuleFailure, disabledIntervals: IDisabled
/**
* @returns true if any modifier kinds passed along exist in the given modifiers array
*/
export function hasModifier(modifiers: ts.ModifiersArray | undefined, ...modifierKinds: ts.SyntaxKind[]) {
export function hasModifier(modifiers: ts.ModifiersArray | undefined, ...modifierKinds: ts.SyntaxKind[]): boolean {
if (modifiers === undefined || modifierKinds.length === 0) {
return false;
}
Expand Down Expand Up @@ -101,7 +101,7 @@ export function ancestorWhere<T extends ts.Node>(node: ts.Node, predicate: (n: t
return cur as T;
}
cur = cur.parent;
} while (cur);
} while (cur !== undefined);
return undefined;
}

Expand Down
2 changes: 1 addition & 1 deletion src/language/walker/ruleWalker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class RuleWalker extends SyntaxWalker implements IWalker {
}

public hasOption(option: string): boolean {
if (this.options) {
if (this.options !== undefined) {
return this.options.indexOf(option) !== -1;
} else {
return false;
Expand Down
10 changes: 5 additions & 5 deletions src/linter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ class Linter {
const enabledRules = this.getEnabledRules(configuration, isJs);

let fileFailures = this.getAllFailures(sourceFile, enabledRules);
if (!fileFailures.length) {
if (fileFailures.length === 0) {
// Usual case: no errors.
return;
}
Expand Down Expand Up @@ -124,9 +124,9 @@ class Linter {
let formatter: IFormatter;
const formattersDirectory = getRelativePath(this.options.formattersDirectory);

const formatterName = this.options.formatter || "prose";
const formatterName = this.options.formatter !== undefined ? this.options.formatter : "prose";
const Formatter = findFormatter(formatterName, formattersDirectory);
if (Formatter) {
if (Formatter !== undefined) {
formatter = new Formatter();
} else {
throw new Error(`formatter '${formatterName}' not found`);
Expand Down Expand Up @@ -193,7 +193,7 @@ class Linter {

private applyRule(rule: IRule, sourceFile: ts.SourceFile): RuleFailure[] {
try {
if (this.program && isTypedRule(rule)) {
if (this.program !== undefined && isTypedRule(rule)) {
return rule.applyWithProgram(sourceFile, this.program);
} else {
return rule.apply(sourceFile);
Expand All @@ -216,7 +216,7 @@ class Linter {
}

private getSourceFile(fileName: string, source: string) {
if (this.program) {
if (this.program !== undefined) {
const sourceFile = this.program.getSourceFile(fileName);
if (sourceFile === undefined) {
const INVALID_SOURCE_ERROR = dedent`
Expand Down
6 changes: 3 additions & 3 deletions src/ruleLoader.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const cachedRules = new Map<string, RuleConstructor | "not-found">();

export function loadRules(ruleOptionsList: IOptions[],
rulesDirectories?: string | string[],
isJs?: boolean): IRule[] {
isJs = false): IRule[] {
const rules: IRule[] = [];
const notFoundRules: string[] = [];
const notAllowedInJsRules: string[] = [];
Expand All @@ -44,15 +44,15 @@ export function loadRules(ruleOptionsList: IOptions[],
const Rule = findRule(ruleName, rulesDirectories);
if (Rule === undefined) {
notFoundRules.push(ruleName);
} else if (isJs && Rule.metadata && Rule.metadata.typescriptOnly) {
} else if (isJs && Rule.metadata !== undefined && Rule.metadata.typescriptOnly) {
notAllowedInJsRules.push(ruleName);
} else {
const rule = new Rule(ruleOptions);
if (rule.isEnabled()) {
rules.push(rule);
}

if (Rule.metadata && Rule.metadata.deprecationMessage) {
if (Rule.metadata !== undefined && Rule.metadata.deprecationMessage !== undefined) {
showWarningOnce(`${Rule.metadata.ruleName} is deprecated. ${Rule.metadata.deprecationMessage}`);
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/rules/arrayTypeRule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ function walk(ctx: Lint.WalkContext<Option>): void {
const failureString = option === "generic" ? Rule.FAILURE_STRING_GENERIC : Rule.FAILURE_STRING_GENERIC_SIMPLE;
const parens = elementType.kind === ts.SyntaxKind.ParenthesizedType ? 1 : 0;
// Add a space if the type is preceded by 'as' and the node has no leading whitespace
const space = !parens && parent!.kind === ts.SyntaxKind.AsExpression && node.getStart() === node.getFullStart();
const space = parens === 0 && parent!.kind === ts.SyntaxKind.AsExpression && node.getStart() === node.getFullStart();
const fix = [
new Lint.Replacement(elementType.getStart(), parens, (space ? " " : "") + "Array<"),
// Delete the square brackets and replace with an angle bracket
Expand All @@ -96,7 +96,7 @@ function walk(ctx: Lint.WalkContext<Option>): void {
}

const failureString = option === "array" ? Rule.FAILURE_STRING_ARRAY : Rule.FAILURE_STRING_ARRAY_SIMPLE;
if (!typeArguments || typeArguments.length === 0) {
if (typeArguments === undefined || typeArguments.length === 0) {
// Create an 'any' array
const fix = Lint.Replacement.replaceFromTo(node.getStart(), node.getEnd(), "any[]");
ctx.addFailureAtNode(node, failureString, fix);
Expand Down Expand Up @@ -152,7 +152,7 @@ function isSimpleType(nodeType: ts.TypeNode): boolean {
case ts.SyntaxKind.TypeReference:
// TypeReferences must be non-generic or be another Array with a simple type
const { typeName, typeArguments } = nodeType as ts.TypeReferenceNode;
if (!typeArguments) {
if (typeArguments === undefined) {
return true;
}
switch (typeArguments.length) {
Expand Down
2 changes: 1 addition & 1 deletion src/rules/arrowReturnShorthandRule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ function createFix(arrowFunction: ts.FunctionLikeDeclaration, body: ts.Block, ex
const semicolon = Lint.childOfKind(statement, ts.SyntaxKind.SemicolonToken);

const anyComments = hasComments(arrow) || hasComments(openBrace) || hasComments(statement) || hasComments(returnKeyword) ||
hasComments(expr) || (semicolon && hasComments(semicolon)) || hasComments(closeBrace);
hasComments(expr) || (semicolon !== undefined && hasComments(semicolon)) || hasComments(closeBrace);
return anyComments ? undefined : [
// Object literal must be wrapped in `()`
...(expr.kind === ts.SyntaxKind.ObjectLiteralExpression ? [
Expand Down
3 changes: 1 addition & 2 deletions src/rules/awaitPromiseRule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ function couldBePromise(type: ts.Type): boolean {

function isPromiseType(type: ts.Type): boolean {
const { target } = type as ts.TypeReference;
const symbol = target && target.symbol;
return !!symbol && symbol.name === "Promise";
return target !== undefined && target.symbol !== undefined && target.symbol.name === "Promise";
}

function isUnionType(type: ts.Type): type is ts.UnionType {
Expand Down
2 changes: 1 addition & 1 deletion src/rules/banRule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export class Rule extends Lint.Rules.AbstractRule {
/* tslint:enable:object-literal-sort-keys */

public static FAILURE_STRING_FACTORY(expression: string, messageAddition?: string) {
return `Calls to '${expression}' are not allowed.${messageAddition ? " " + messageAddition : ""}`;
return `Calls to '${expression}' are not allowed.${messageAddition !== undefined ? " " + messageAddition : ""}`;
}

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
Expand Down
2 changes: 1 addition & 1 deletion src/rules/banTypesRule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export class Rule extends Lint.Rules.AbstractRule {

public static FAILURE_STRING_FACTORY(typeName: string, messageAddition?: string) {
return `Don't use '${typeName}' as a type.` +
(messageAddition ? " " + messageAddition : "");
(messageAddition !== undefined ? " " + messageAddition : "");
}

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