Skip to content

Update linting to avoid typechecking for booleanTriviaRule #15021

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 5, 2017
Merged
Show file tree
Hide file tree
Changes from all 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
46 changes: 23 additions & 23 deletions Gulpfile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ for (const i in libraryTargets) {
const sources = [copyright].concat(entry.sources.map(function(s) {
return path.join(libraryDirectory, s);
}));
gulp.task(target, false, [], function() {
gulp.task(target, /*help*/ false, [], function() {
return gulp.src(sources)
.pipe(newer(target))
.pipe(concat(target, { newLine: "\n\n" }))
Expand Down Expand Up @@ -275,7 +275,7 @@ function getCompilerSettings(base: tsc.Settings, useBuiltCompiler?: boolean): ts
return copy;
}

gulp.task(configureNightlyJs, false, [], () => {
gulp.task(configureNightlyJs, /*help*/ false, [], () => {
const settings: tsc.Settings = {
declaration: false,
removeComments: true,
Expand Down Expand Up @@ -304,7 +304,7 @@ const importDefinitelyTypedTestsDirectory = path.join(scriptsDirectory, "importD
const importDefinitelyTypedTestsJs = path.join(importDefinitelyTypedTestsDirectory, "importDefinitelyTypedTests.js");
const importDefinitelyTypedTestsTs = path.join(importDefinitelyTypedTestsDirectory, "importDefinitelyTypedTests.ts");

gulp.task(importDefinitelyTypedTestsJs, false, [], () => {
gulp.task(importDefinitelyTypedTestsJs, /*help*/ false, [], () => {
const settings: tsc.Settings = getCompilerSettings({
declaration: false,
removeComments: true,
Expand Down Expand Up @@ -335,7 +335,7 @@ const generatedDiagnosticMessagesJSON = path.join(compilerDirectory, "diagnostic
const builtGeneratedDiagnosticMessagesJSON = path.join(builtLocalDirectory, "diagnosticMessages.generated.json");

// processDiagnosticMessages script
gulp.task(processDiagnosticMessagesJs, false, [], () => {
gulp.task(processDiagnosticMessagesJs, /*help*/ false, [], () => {
const settings: tsc.Settings = getCompilerSettings({
target: "es5",
declaration: false,
Expand Down Expand Up @@ -383,7 +383,7 @@ function prependCopyright(outputCopyright: boolean = !useDebugMode) {
return insert.prepend(outputCopyright ? (copyrightContent || (copyrightContent = fs.readFileSync(copyright).toString())) : "");
}

gulp.task(builtLocalCompiler, false, [servicesFile], () => {
gulp.task(builtLocalCompiler, /*help*/ false, [servicesFile], () => {
const localCompilerProject = tsc.createProject("src/compiler/tsconfig.json", getCompilerSettings({}, /*useBuiltCompiler*/true));
return localCompilerProject.src()
.pipe(newer(builtLocalCompiler))
Expand All @@ -394,7 +394,7 @@ gulp.task(builtLocalCompiler, false, [servicesFile], () => {
.pipe(gulp.dest("src/compiler"));
});

gulp.task(servicesFile, false, ["lib", "generate-diagnostics"], () => {
gulp.task(servicesFile, /*help*/ false, ["lib", "generate-diagnostics"], () => {
const servicesProject = tsc.createProject("src/services/tsconfig.json", getCompilerSettings({ removeComments: false }, /*useBuiltCompiler*/false));
const {js, dts} = servicesProject.src()
.pipe(newer(servicesFile))
Expand Down Expand Up @@ -428,7 +428,7 @@ gulp.task(servicesFile, false, ["lib", "generate-diagnostics"], () => {

// cancellationToken.js
const cancellationTokenJs = path.join(builtLocalDirectory, "cancellationToken.js");
gulp.task(cancellationTokenJs, false, [servicesFile], () => {
gulp.task(cancellationTokenJs, /*help*/ false, [servicesFile], () => {
const cancellationTokenProject = tsc.createProject("src/server/cancellationToken/tsconfig.json", getCompilerSettings({}, /*useBuiltCompiler*/true));
return cancellationTokenProject.src()
.pipe(newer(cancellationTokenJs))
Expand All @@ -441,7 +441,7 @@ gulp.task(cancellationTokenJs, false, [servicesFile], () => {

// typingsInstallerFile.js
const typingsInstallerJs = path.join(builtLocalDirectory, "typingsInstaller.js");
gulp.task(typingsInstallerJs, false, [servicesFile], () => {
gulp.task(typingsInstallerJs, /*help*/ false, [servicesFile], () => {
const cancellationTokenProject = tsc.createProject("src/server/typingsInstaller/tsconfig.json", getCompilerSettings({}, /*useBuiltCompiler*/true));
return cancellationTokenProject.src()
.pipe(newer(typingsInstallerJs))
Expand All @@ -454,7 +454,7 @@ gulp.task(typingsInstallerJs, false, [servicesFile], () => {

const serverFile = path.join(builtLocalDirectory, "tsserver.js");

gulp.task(serverFile, false, [servicesFile, typingsInstallerJs, cancellationTokenJs], () => {
gulp.task(serverFile, /*help*/ false, [servicesFile, typingsInstallerJs, cancellationTokenJs], () => {
const serverProject = tsc.createProject("src/server/tsconfig.json", getCompilerSettings({}, /*useBuiltCompiler*/true));
return serverProject.src()
.pipe(newer(serverFile))
Expand All @@ -468,7 +468,7 @@ gulp.task(serverFile, false, [servicesFile, typingsInstallerJs, cancellationToke
const tsserverLibraryFile = path.join(builtLocalDirectory, "tsserverlibrary.js");
const tsserverLibraryDefinitionFile = path.join(builtLocalDirectory, "tsserverlibrary.d.ts");

gulp.task(tsserverLibraryFile, false, [servicesFile], (done) => {
gulp.task(tsserverLibraryFile, /*help*/ false, [servicesFile], (done) => {
const serverLibraryProject = tsc.createProject("src/server/tsconfig.library.json", getCompilerSettings({}, /*useBuiltCompiler*/ true));
const {js, dts}: { js: NodeJS.ReadableStream, dts: NodeJS.ReadableStream } = serverLibraryProject.src()
.pipe(sourcemaps.init())
Expand Down Expand Up @@ -497,7 +497,7 @@ const word2mdTs = path.join(scriptsDirectory, "word2md.ts");
const specWord = path.join(docDirectory, "TypeScript Language Specification.docx");
const specMd = path.join(docDirectory, "spec.md");

gulp.task(word2mdJs, false, [], () => {
gulp.task(word2mdJs, /*help*/ false, [], () => {
const settings: tsc.Settings = getCompilerSettings({
outFile: word2mdJs
}, /*useBuiltCompiler*/ false);
Expand All @@ -509,7 +509,7 @@ gulp.task(word2mdJs, false, [], () => {
.pipe(gulp.dest("."));
});

gulp.task(specMd, false, [word2mdJs], (done) => {
gulp.task(specMd, /*help*/ false, [word2mdJs], (done) => {
const specWordFullPath = path.resolve(specWord);
const specMDFullPath = path.resolve(specMd);
const cmd = "cscript //nologo " + word2mdJs + " \"" + specWordFullPath + "\" " + "\"" + specMDFullPath + "\"";
Expand All @@ -525,10 +525,10 @@ gulp.task("clean", "Cleans the compiler output, declare files, and tests", [], (
return del([builtDirectory]);
});

gulp.task("useDebugMode", false, [], (done) => { useDebugMode = true; done(); });
gulp.task("dontUseDebugMode", false, [], (done) => { useDebugMode = false; done(); });
gulp.task("useDebugMode", /*help*/ false, [], (done) => { useDebugMode = true; done(); });
gulp.task("dontUseDebugMode", /*help*/ false, [], (done) => { useDebugMode = false; done(); });

gulp.task("VerifyLKG", false, [], () => {
gulp.task("VerifyLKG", /*help*/ false, [], () => {
const expectedFiles = [builtLocalCompiler, servicesFile, serverFile, nodePackageFile, nodeDefinitionsFile, standaloneDefinitionsFile, tsserverLibraryFile, tsserverLibraryDefinitionFile, typingsInstallerJs, cancellationTokenJs].concat(libraryTargets);
const missingFiles = expectedFiles.filter(function(f) {
return !fs.existsSync(f);
Expand All @@ -541,7 +541,7 @@ gulp.task("VerifyLKG", false, [], () => {
return gulp.src(expectedFiles).pipe(gulp.dest(LKGDirectory));
});

gulp.task("LKGInternal", false, ["lib", "local"]);
gulp.task("LKGInternal", /*help*/ false, ["lib", "local"]);

gulp.task("LKG", "Makes a new LKG out of the built js files", ["clean", "dontUseDebugMode"], () => {
return runSequence("LKGInternal", "VerifyLKG");
Expand All @@ -550,7 +550,7 @@ gulp.task("LKG", "Makes a new LKG out of the built js files", ["clean", "dontUse

// Task to build the tests infrastructure using the built compiler
const run = path.join(builtLocalDirectory, "run.js");
gulp.task(run, false, [servicesFile], () => {
gulp.task(run, /*help*/ false, [servicesFile], () => {
const testProject = tsc.createProject("src/harness/tsconfig.json", getCompilerSettings({}, /*useBuiltCompiler*/true));
return testProject.src()
.pipe(newer(run))
Expand Down Expand Up @@ -724,7 +724,7 @@ gulp.task("runtests",

const nodeServerOutFile = "tests/webTestServer.js";
const nodeServerInFile = "tests/webTestServer.ts";
gulp.task(nodeServerOutFile, false, [servicesFile], () => {
gulp.task(nodeServerOutFile, /*help*/ false, [servicesFile], () => {
const settings: tsc.Settings = getCompilerSettings({ module: "commonjs" }, /*useBuiltCompiler*/ true);
return gulp.src(nodeServerInFile)
.pipe(newer(nodeServerOutFile))
Expand Down Expand Up @@ -889,7 +889,7 @@ gulp.task("baseline-accept-test262", "Makes the most recent test262 test results
// Webhost
const webhostPath = "tests/webhost/webtsc.ts";
const webhostJsPath = "tests/webhost/webtsc.js";
gulp.task(webhostJsPath, false, [servicesFile], () => {
gulp.task(webhostJsPath, /*help*/ false, [servicesFile], () => {
const settings: tsc.Settings = getCompilerSettings({
outFile: webhostJsPath
}, /*useBuiltCompiler*/ true);
Expand All @@ -909,7 +909,7 @@ gulp.task("webhost", "Builds the tsc web host", [webhostJsPath], () => {
// Perf compiler
const perftscPath = "tests/perftsc.ts";
const perftscJsPath = "built/local/perftsc.js";
gulp.task(perftscJsPath, false, [servicesFile], () => {
gulp.task(perftscJsPath, /*help*/ false, [servicesFile], () => {
const settings: tsc.Settings = getCompilerSettings({
outFile: perftscJsPath
}, /*useBuiltCompiler*/ true);
Expand All @@ -927,10 +927,10 @@ gulp.task("perftsc", "Builds augmented version of the compiler for perf tests",
// Instrumented compiler
const loggedIOpath = path.join(harnessDirectory, "loggedIO.ts");
const loggedIOJsPath = path.join(builtLocalDirectory, "loggedIO.js");
gulp.task(loggedIOJsPath, false, [], (done) => {
gulp.task(loggedIOJsPath, /*help*/ false, [], (done) => {
const temp = path.join(builtLocalDirectory, "temp");
mkdirP(temp, (err) => {
if (err) { console.error(err); done(err); process.exit(1); };
if (err) { console.error(err); done(err); process.exit(1); }
exec(host, [LKGCompiler, "--types --outdir", temp, loggedIOpath], () => {
fs.renameSync(path.join(temp, "/harness/loggedIO.js"), loggedIOJsPath);
del(temp).then(() => done(), done);
Expand All @@ -940,7 +940,7 @@ gulp.task(loggedIOJsPath, false, [], (done) => {

const instrumenterPath = path.join(harnessDirectory, "instrumenter.ts");
const instrumenterJsPath = path.join(builtLocalDirectory, "instrumenter.js");
gulp.task(instrumenterJsPath, false, [servicesFile], () => {
gulp.task(instrumenterJsPath, /*help*/ false, [servicesFile], () => {
const settings: tsc.Settings = getCompilerSettings({
outFile: instrumenterJsPath
}, /*useBuiltCompiler*/ true);
Expand Down
59 changes: 23 additions & 36 deletions scripts/tslint/booleanTriviaRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,12 @@ import * as Lint from "tslint/lib";
import * as ts from "typescript";

export class Rule extends Lint.Rules.AbstractRule {
public static FAILURE_STRING_FACTORY(name: string, currently?: string): string {
const current = currently ? ` (currently '${currently}')` : "";
return `Tag boolean argument as '${name}'${current}`;
}

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
// Cheat to get type checker
const program = ts.createProgram([sourceFile.fileName], Lint.createCompilerOptions());
const checker = program.getTypeChecker();
return this.applyWithFunction(program.getSourceFile(sourceFile.fileName), ctx => walk(ctx, checker));
return this.applyWithFunction(sourceFile, ctx => walk(ctx));
}
}

function walk(ctx: Lint.WalkContext<void>, checker: ts.TypeChecker): void {
function walk(ctx: Lint.WalkContext<void>): void {
ts.forEachChild(ctx.sourceFile, recur);
function recur(node: ts.Node): void {
if (node.kind === ts.SyntaxKind.CallExpression) {
Expand All @@ -25,39 +17,34 @@ function walk(ctx: Lint.WalkContext<void>, checker: ts.TypeChecker): void {
}

function checkCall(node: ts.CallExpression): void {
if (!node.arguments || !node.arguments.some(arg => arg.kind === ts.SyntaxKind.TrueKeyword || arg.kind === ts.SyntaxKind.FalseKeyword)) {
return;
}

const targetCallSignature = checker.getResolvedSignature(node);
if (!targetCallSignature) {
return;
}

const targetParameters = targetCallSignature.getParameters();
for (let index = 0; index < targetParameters.length; index++) {
const param = targetParameters[index];
const arg = node.arguments[index];
if (!(arg && param)) {
for (const arg of node.arguments) {
if (arg.kind !== ts.SyntaxKind.TrueKeyword && arg.kind !== ts.SyntaxKind.FalseKeyword) {
continue;
}

const argType = checker.getContextualType(arg);
if (argType && (argType.getFlags() & ts.TypeFlags.Boolean)) {
if (arg.kind !== ts.SyntaxKind.TrueKeyword && arg.kind !== ts.SyntaxKind.FalseKeyword) {
if (node.expression.kind === ts.SyntaxKind.PropertyAccessExpression) {
const methodName = (node.expression as ts.PropertyAccessExpression).name.text
// Skip certain method names whose parameter names are not informative
if (methodName === 'set' ||
methodName === 'equal' ||
methodName === 'fail' ||
methodName === 'isTrue' ||
methodName === 'assert') {
continue;
}
let triviaContent: string | undefined;
const ranges = ts.getLeadingCommentRanges(arg.getFullText(), 0);
if (ranges && ranges.length === 1 && ranges[0].kind === ts.SyntaxKind.MultiLineCommentTrivia) {
triviaContent = arg.getFullText().slice(ranges[0].pos + 2, ranges[0].end - 2); // +/-2 to remove /**/
}
else if (node.expression.kind === ts.SyntaxKind.Identifier) {
const functionName = (node.expression as ts.Identifier).text;
// Skip certain function names whose parameter names are not informative
if (functionName === 'assert') {
continue;
}
}

const paramName = param.getName();
if (triviaContent !== paramName && triviaContent !== paramName + ":") {
ctx.addFailureAtNode(arg, Rule.FAILURE_STRING_FACTORY(param.getName(), triviaContent));
}
const ranges = ts.getLeadingCommentRanges(arg.getFullText(), 0);
if (!(ranges && ranges.length === 1 && ranges[0].kind === ts.SyntaxKind.MultiLineCommentTrivia)) {
ctx.addFailureAtNode(arg, 'Tag boolean argument with parameter name');
}
}
}
}
}
8 changes: 4 additions & 4 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9680,7 +9680,7 @@ namespace ts {
const original = getTypeOfSymbol(property);
const updated = f(original);
members.set(property.name, updated === original ? property : createSymbolWithType(property, updated));
};
}
return members;
}

Expand Down Expand Up @@ -9724,7 +9724,7 @@ namespace ts {
// Since get accessors already widen their return value there is no need to
// widen accessor based properties here.
members.set(prop.name, prop.flags & SymbolFlags.Property ? getWidenedProperty(prop) : prop);
};
}
const stringIndexInfo = getIndexInfoOfType(type, IndexKind.String);
const numberIndexInfo = getIndexInfoOfType(type, IndexKind.Number);
return createAnonymousType(type.symbol, members, emptyArray, emptyArray,
Expand Down Expand Up @@ -15100,7 +15100,7 @@ namespace ts {
if (!checkApplicableSignature(node, args, candidate, relation, excludeArgument, /*reportErrors*/ false)) {
break;
}
const index = excludeArgument ? indexOf(excludeArgument, true) : -1;
const index = excludeArgument ? indexOf(excludeArgument, /*value*/ true) : -1;
if (index < 0) {
return candidate;
}
Expand Down Expand Up @@ -18728,7 +18728,7 @@ namespace ts {
case SyntaxKind.ConstructorType:
checkUnusedTypeParameters(<FunctionLikeDeclaration>node);
break;
};
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/commandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1580,7 +1580,7 @@ namespace ts {
delete wildcardDirectories[key];
}
}
};
}
}

return wildcardDirectories;
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/declarationEmitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ namespace ts {
currentIdentifiers = node.identifiers;
isCurrentFileExternalModule = isExternalModule(node);
enclosingDeclaration = node;
emitDetachedComments(currentText, currentLineMap, writer, writeCommentRange, node, newLine, true /* remove comments */);
emitDetachedComments(currentText, currentLineMap, writer, writeCommentRange, node, newLine, /*removeComents*/ true);
emitLines(node.statements);
}

Expand Down
2 changes: 1 addition & 1 deletion src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1845,7 +1845,7 @@ namespace ts {
case ParsingContext.JSDocTupleTypes: return Diagnostics.Type_expected;
case ParsingContext.JSDocRecordMembers: return Diagnostics.Property_assignment_expected;
}
};
}

// Parses a comma-delimited list of elements
function parseDelimitedList<T extends Node>(kind: ParsingContext, parseElement: () => T, considerSemicolonAsDelimiter?: boolean): NodeArray<T> {
Expand Down
12 changes: 6 additions & 6 deletions src/compiler/sys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ namespace ts {
referenceCount: number;
}

declare var require: any;
declare var process: any;
declare var global: any;
declare var __filename: string;
declare const require: any;
declare const process: any;
declare const global: any;
declare const __filename: string;

export function getNodeMajorVersion() {
if (typeof process === "undefined") {
Expand All @@ -74,7 +74,7 @@ namespace ts {
return parseInt(version.substring(1, dot));
}

declare var ChakraHost: {
declare const ChakraHost: {
args: string[];
currentDirectory: string;
executingFile: string;
Expand Down Expand Up @@ -368,7 +368,7 @@ namespace ts {
if (eventName === "rename") {
// When deleting a file, the passed baseFileName is null
callback(!relativeFileName ? relativeFileName : normalizePath(combinePaths(directoryName, relativeFileName)));
};
}
}
);
},
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/transformers/module/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1162,7 +1162,7 @@ namespace ts {
statement = createStatement(
createExportExpression(
createIdentifier("__esModule"),
createLiteral(true)
createLiteral(/*value*/ true)
)
);
}
Expand All @@ -1175,7 +1175,7 @@ namespace ts {
createIdentifier("exports"),
createLiteral("__esModule"),
createObjectLiteral([
createPropertyAssignment("value", createLiteral(true))
createPropertyAssignment("value", createLiteral(/*value*/ true))
])
]
)
Expand Down
Loading