Skip to content
This repository has been 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

Conversation

andy-hanson
Copy link
Contributor

PR checklist

Overview of change:

Applies the strict-boolean-expressions rule to the code.

Copy link
Contributor

@ajafff ajafff left a comment

Choose a reason for hiding this comment

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

It's a little verbose, especially for fast null checks or types like boolean | undefined, but that's not too bad.

@andy-hanson andy-hanson force-pushed the strict-boolean-expressions branch from d982932 to bccbe4e Compare May 7, 2017 18:22
Copy link
Contributor

@adidahiya adidahiya left a comment

Choose a reason for hiding this comment

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

some minor nits. seems fine as long as vscode-tslint continues working with type checking rules enabled (do we have any other such rules enabled in this code base?)

@@ -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


return CompletedDocsWalker.modifierAliases[description] || description;
const alias = CompletedDocsWalker.modifierAliases[description];
if (alias !== undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: would've preferred a ternary expression here

}

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
const limit = this.ruleArguments[0] as number | undefined || Rule.DEFAULT_ALLOWED_BLANKS;
return this.applyWithFunction(sourceFile, walk, limit);
const limit = this.ruleArguments[0] as number | undefined ;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: extra space before ;

TRANSFORMS[optionSet["named-imports-order"] || "case-insensitive"];
interface Options { "import-sources-order"?: string; "named-imports-order"?: string; }
const optionSet = (this.getOptions() as [Options])[0];
const { "import-sources-order": sources = "case-insensitive", "named-imports-order": named = "case-insensitive" } =
Copy link
Contributor

Choose a reason for hiding this comment

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

this is kind of a crazy line. might be nice to spread it out:

const {
    "import-sources-order": sources = "case-insensitive",
    "named-imports-order": named = "case-insensitive",
} = optionSet === undefined ? {} : optionSet;

@@ -207,7 +208,7 @@ function isFunction(t: ts.Type): boolean {
return true;
}
const symbol = t.getSymbol();
return (symbol && symbol.getName()) === "Function";
return symbol === undefined ? false : symbol.getName() === "Function";
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: a simple boolean expression would be slightly easier to read, symbol !== undefined && symbol.getName() === "Function"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#2534 will catch this. ⚡

@andy-hanson
Copy link
Contributor Author

andy-hanson commented May 9, 2017

As for running this rule in vscode, see Microsoft/vscode-tslint#70. The instructions at https://github.com/angelozerr/tslint-language-service/blob/master/README.md work for me.

@andy-hanson andy-hanson force-pushed the strict-boolean-expressions branch 2 times, most recently from 4c9c62a to 365914d Compare May 9, 2017 03:23
@andy-hanson andy-hanson force-pushed the strict-boolean-expressions branch from 365914d to 520f8b9 Compare May 9, 2017 03:30
@nchen63 nchen63 merged commit 2dd17e2 into palantir:master May 9, 2017
@andy-hanson andy-hanson deleted the strict-boolean-expressions branch May 9, 2017 05:13
@egamma
Copy link

egamma commented May 24, 2017

@andy-hanson @adidahiya there is a preview of the vscode-tslint extension that uses the typescript server plugin which supports type checked rules. All you need to do is to install this extension.

One thing to keep in mind when using the tslint typescript server plugin is that the no-unused-variable rule is disabled because of this tslint issue #2649.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants