Skip to content

TSLint rules and fixes #3642

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.

Sign up for GitHub

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

Closed
wants to merge 10 commits into from
Closed

TSLint rules and fixes #3642

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
82ecd5d
Basic tslint rules, non-final list
danquirk Jun 25, 2015
ed1ff3d
Fixing up whitespace and semicolons
danquirk Jun 25, 2015
d4403df
var -> let
danquirk Jun 26, 2015
cd48d81
Fix build errors due to naive var->let replacement
danquirk Jun 26, 2015
5fc3099
Rule changes
danquirk Jun 30, 2015
be1371d
Merge master
danquirk Jul 8, 2015
5dfa610
Turn off a few rules and more cleanup post merge
danquirk Jul 9, 2015
27f5bf1
CR feedback
danquirk Jul 9, 2015
a80a4ba
Add jake task for linting the compiler sources
danquirk Jul 9, 2015
c363dec
for...in not for...of
danquirk Jul 9, 2015
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
36 changes: 27 additions & 9 deletions Jakefile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ compileFile(servicesFile, servicesSources,[builtLocalDirectory, copyright].conca
/*keepComments*/ true,
/*noResolve*/ false,
/*stripInternal*/ true,
/*callback*/ function () {
/*callback*/ function () {
jake.cpR(servicesFile, nodePackageFile, {silent: true});

prependFile(copyright, standaloneDefinitionsFile);
Expand All @@ -379,12 +379,12 @@ compileFile(serverFile, serverSources,[builtLocalDirectory, copyright].concat(se

var lsslFile = path.join(builtLocalDirectory, "tslssl.js");
compileFile(
lsslFile,
languageServiceLibrarySources,
lsslFile,
languageServiceLibrarySources,
[builtLocalDirectory, copyright].concat(languageServiceLibrarySources),
/*prefixes*/ [copyright],
/*useBuiltCompiler*/ true,
/*noOutFile*/ false,
/*prefixes*/ [copyright],
/*useBuiltCompiler*/ true,
/*noOutFile*/ false,
/*generateDeclarations*/ true);

// Local target to build the language service server library
Expand Down Expand Up @@ -488,7 +488,7 @@ var refTest262Baseline = path.join(internalTests, "baselines/test262/reference")
desc("Builds the test infrastructure using the built compiler");
task("tests", ["local", run].concat(libraryTargets));

function exec(cmd, completeHandler) {
function exec(cmd, completeHandler, errorHandler) {
var ex = jake.createExec([cmd], {windowsVerbatimArguments: true});
// Add listeners for output and error
ex.addListener("stdout", function(output) {
Expand All @@ -504,8 +504,12 @@ function exec(cmd, completeHandler) {
complete();
});
ex.addListener("error", function(e, status) {
fail("Process exited with code " + status);
})
if(errorHandler) {
errorHandler(e, status);
} else {
fail("Process exited with code " + status);
}
});

ex.run();
}
Expand Down Expand Up @@ -721,3 +725,17 @@ task("update-sublime", [serverFile], function() {
jake.cpR(serverFile, "../TypeScript-Sublime-Plugin/tsserver/");
jake.cpR(serverFile + ".map", "../TypeScript-Sublime-Plugin/tsserver/");
});

// if the codebase were free of linter errors we could make jake runtests
// run this task automatically
desc("Runs tslint on the compiler sources");
task("lint", [], function() {
for(var i in compilerSources) {
var f = compilerSources[i];
var cmd = 'tslint -f ' + f;
exec(cmd,
function() { console.log('SUCCESS: No linter errors'); },
function() { console.log('FAILURE: Please fix linting errors in ' + f + '\n');
});
}
}, { async: true });
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
"mocha": "latest",
"chai": "latest",
"browserify": "latest",
"istanbul": "latest"
"istanbul": "latest",
"tslint": "latest"
},
"scripts": {
"test": "jake runtests"
Expand Down
3 changes: 1 addition & 2 deletions scripts/errorCheck.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ fs.readFile('src/compiler/diagnosticInformationMap.generated.ts', 'utf-8', (err,
console.log('Consumed ' + allSrc.length + ' characters of source');

let count = 0;
console.log('== List of errors not used in source ==')
console.log('== List of errors not used in source ==');
for (let errName of errorNames) {
if (allSrc.indexOf(errName) < 0) {
console.log(errName);
Expand All @@ -84,4 +84,3 @@ fs.readFile('src/compiler/diagnosticInformationMap.generated.ts', 'utf-8', (err,
console.log(count + ' of ' + errorNames.length + ' errors are not used in source');
});
});

70 changes: 35 additions & 35 deletions src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace ts {
}

export function getModuleInstanceState(node: Node): ModuleInstanceState {
// A module is uninstantiated if it contains only
// A module is uninstantiated if it contains only
// 1. interface declarations, type alias declarations
if (node.kind === SyntaxKind.InterfaceDeclaration || node.kind === SyntaxKind.TypeAliasDeclaration) {
return ModuleInstanceState.NonInstantiated;
Expand Down Expand Up @@ -53,7 +53,7 @@ namespace ts {
}

const enum ContainerFlags {
// The current node is not a container, and no container manipulation should happen before
// The current node is not a container, and no container manipulation should happen before
// recursing into it.
None = 0,

Expand Down Expand Up @@ -90,13 +90,13 @@ namespace ts {
let lastContainer: Node;

// If this file is an external module, then it is automatically in strict-mode according to
// ES6. If it is not an external module, then we'll determine if it is in strict mode or
// ES6. If it is not an external module, then we'll determine if it is in strict mode or
// not depending on if we see "use strict" in certain places (or if we hit a class/namespace).
let inStrictMode = !!file.externalModuleIndicator;

let symbolCount = 0;
let Symbol = objectAllocator.getSymbolConstructor();
let classifiableNames: Map<string> = {};
let classifiableNames: Map<string> = {};

if (!file.locals) {
bind(file);
Expand Down Expand Up @@ -179,7 +179,7 @@ namespace ts {
* @param parent - node's parent declaration.
* @param node - The declaration to be added to the symbol table
* @param includes - The SymbolFlags that node has in addition to its declaration type (eg: export, ambient, etc.)
* @param excludes - The flags which node cannot be declared alongside in a symbol table. Used to report forbidden declarations.
* @param excludes - The flags which node cannot be declared alongside in a symbol table. Used to report forbidden declarations.
*/
function declareSymbol(symbolTable: SymbolTable, parent: Symbol, node: Declaration, includes: SymbolFlags, excludes: SymbolFlags): Symbol {
Debug.assert(!hasDynamicName(node));
Expand All @@ -192,13 +192,13 @@ namespace ts {

// Check and see if the symbol table already has a symbol with this name. If not,
// create a new symbol with this name and add it to the table. Note that we don't
// give the new symbol any flags *yet*. This ensures that it will not conflict
// give the new symbol any flags *yet*. This ensures that it will not conflict
// with the 'excludes' flags we pass in.
//
// If we do get an existing symbol, see if it conflicts with the new symbol we're
// creating. For example, a 'var' symbol and a 'class' symbol will conflict within
// the same symbol table. If we have a conflict, report the issue on each
// declaration we have for this symbol, and then create a new symbol for this
// the same symbol table. If we have a conflict, report the issue on each
// declaration we have for this symbol, and then create a new symbol for this
// declaration.
//
// If we created a new symbol, either because we didn't have a symbol with this name
Expand Down Expand Up @@ -259,7 +259,7 @@ namespace ts {
// ExportType, or ExportContainer flag, and an associated export symbol with all the correct flags set
// on it. There are 2 main reasons:
//
// 1. We treat locals and exports of the same name as mutually exclusive within a container.
// 1. We treat locals and exports of the same name as mutually exclusive within a container.
// That means the binder will issue a Duplicate Identifier error if you mix locals and exports
// with the same name in the same container.
// TODO: Make this a more specific error and decouple it from the exclusion logic.
Expand All @@ -282,11 +282,11 @@ namespace ts {
}
}

// All container nodes are kept on a linked list in declaration order. This list is used by
// the getLocalNameOfContainer function in the type checker to validate that the local name
// All container nodes are kept on a linked list in declaration order. This list is used by
// the getLocalNameOfContainer function in the type checker to validate that the local name
// used for a container is unique.
function bindChildren(node: Node) {
// Before we recurse into a node's chilren, we first save the existing parent, container
// Before we recurse into a node's chilren, we first save the existing parent, container
// and block-container. Then after we pop out of processing the children, we restore
// these saved values.
let saveParent = parent;
Expand All @@ -295,16 +295,16 @@ namespace ts {

// This node will now be set as the parent of all of its children as we recurse into them.
parent = node;

// Depending on what kind of node this is, we may have to adjust the current container
// and block-container. If the current node is a container, then it is automatically
// considered the current block-container as well. Also, for containers that we know
// may contain locals, we proactively initialize the .locals field. We do this because
// it's highly likely that the .locals will be needed to place some child in (for example,
// a parameter, or variable declaration).
//
//
// However, we do not proactively create the .locals for block-containers because it's
// totally normal and common for block-containers to never actually have a block-scoped
// totally normal and common for block-containers to never actually have a block-scoped
// variable in them. We don't want to end up allocating an object for every 'block' we
// run into when most of them won't be necessary.
//
Expand Down Expand Up @@ -345,7 +345,7 @@ namespace ts {
case SyntaxKind.TypeLiteral:
case SyntaxKind.ObjectLiteralExpression:
return ContainerFlags.IsContainer;

case SyntaxKind.CallSignature:
case SyntaxKind.ConstructSignature:
case SyntaxKind.IndexSignature:
Expand Down Expand Up @@ -373,7 +373,7 @@ namespace ts {

case SyntaxKind.Block:
// do not treat blocks directly inside a function as a block-scoped-container.
// Locals that reside in this block should go to the function locals. Othewise 'x'
// Locals that reside in this block should go to the function locals. Othewise 'x'
// would not appear to be a redeclaration of a block scoped local in the following
// example:
//
Expand All @@ -386,7 +386,7 @@ namespace ts {
// the block, then there would be no collision.
//
// By not creating a new block-scoped-container here, we ensure that both 'var x'
// and 'let x' go into the Function-container's locals, and we do get a collision
// and 'let x' go into the Function-container's locals, and we do get a collision
// conflict.
return isFunctionLike(node.parent) ? ContainerFlags.None : ContainerFlags.IsBlockScopedContainer;
}
Expand Down Expand Up @@ -484,13 +484,13 @@ namespace ts {
}

function hasExportDeclarations(node: ModuleDeclaration | SourceFile): boolean {
var body = node.kind === SyntaxKind.SourceFile ? node : (<ModuleDeclaration>node).body;
let body = node.kind === SyntaxKind.SourceFile ? node : (<ModuleDeclaration>node).body;
if (body.kind === SyntaxKind.SourceFile || body.kind === SyntaxKind.ModuleBlock) {
for (let stat of (<Block>body).statements) {
if (stat.kind === SyntaxKind.ExportDeclaration || stat.kind === SyntaxKind.ExportAssignment) {
return true;
}
}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

huh.. why?

Copy link
Member Author

Choose a reason for hiding this comment

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

might've been the rule getting for...of line, will double check

Edit: yeah, removing it since the rule still isn't satisfied anyway

}
return false;
}
Expand Down Expand Up @@ -536,8 +536,8 @@ namespace ts {
// For a given function symbol "<...>(...) => T" we want to generate a symbol identical
// to the one we would get for: { <...>(...): T }
//
// We do that by making an anonymous type literal symbol, and then setting the function
// symbol as its sole member. To the rest of the system, this symbol will be indistinguishable
// We do that by making an anonymous type literal symbol, and then setting the function
// symbol as its sole member. To the rest of the system, this symbol will be indistinguishable
// from an actual type literal symbol you would have gotten had you used the long form.
let symbol = createSymbol(SymbolFlags.Signature, getDeclarationName(node));
addDeclarationToSymbol(symbol, node, SymbolFlags.Signature);
Expand Down Expand Up @@ -638,7 +638,7 @@ namespace ts {
}

function getStrictModeIdentifierMessage(node: Node) {
// Provide specialized messages to help the user understand why we think they're in
// Provide specialized messages to help the user understand why we think they're in
// strict mode.
if (getContainingClass(node)) {
return Diagnostics.Identifier_expected_0_is_a_reserved_word_in_strict_mode_Class_definitions_are_automatically_in_strict_mode;
Expand Down Expand Up @@ -696,7 +696,7 @@ namespace ts {
}

function getStrictModeEvalOrArgumentsMessage(node: Node) {
// Provide specialized messages to help the user understand why we think they're in
// Provide specialized messages to help the user understand why we think they're in
// strict mode.
if (getContainingClass(node)) {
return Diagnostics.Invalid_use_of_0_Class_definitions_are_automatically_in_strict_mode;
Expand Down Expand Up @@ -760,24 +760,24 @@ namespace ts {
function bind(node: Node) {
node.parent = parent;

var savedInStrictMode = inStrictMode;
let savedInStrictMode = inStrictMode;
if (!savedInStrictMode) {
updateStrictMode(node);
}

// First we bind declaration nodes to a symbol if possible. We'll both create a symbol
// and then potentially add the symbol to an appropriate symbol table. Possible
// and then potentially add the symbol to an appropriate symbol table. Possible
// destination symbol tables are:
//
//
// 1) The 'exports' table of the current container's symbol.
// 2) The 'members' table of the current container's symbol.
// 3) The 'locals' table of the current container.
//
// However, not all symbols will end up in any of these tables. 'Anonymous' symbols
// However, not all symbols will end up in any of these tables. 'Anonymous' symbols
// (like TypeLiterals for example) will not be put in any table.
bindWorker(node);

// Then we recurse into the children of the node to bind them as well. For certain
// Then we recurse into the children of the node to bind them as well. For certain
// symbols we do specialized work when we recurse. For example, we'll keep track of
// the current 'container' node when it changes. This helps us know which symbol table
// a local should go into for example.
Expand Down Expand Up @@ -817,7 +817,7 @@ namespace ts {
}
}
}

/// Should be called only on prologue directives (isPrologueDirective(node) should be true)
function isUseStrictPrologueDirective(node: ExpressionStatement): boolean {
let nodeText = getTextOfNodeFromSourceText(file.text, node.expression);
Expand Down Expand Up @@ -972,9 +972,9 @@ namespace ts {
let symbol = node.symbol;

// TypeScript 1.0 spec (April 2014): 8.4
// Every class automatically contains a static property member named 'prototype', the
// Every class automatically contains a static property member named 'prototype', the
// type of which is an instantiation of the class type with type Any supplied as a type
// argument for each type parameter. It is an error to explicitly declare a static
// argument for each type parameter. It is an error to explicitly declare a static
// property member with the name 'prototype'.
//
// Note: we check for this here because this class may be merging into a module. The
Expand All @@ -1000,7 +1000,7 @@ namespace ts {

function bindVariableDeclarationOrBindingElement(node: VariableDeclaration | BindingElement) {
if (inStrictMode) {
checkStrictModeEvalOrArguments(node, node.name)
checkStrictModeEvalOrArguments(node, node.name);
}

if (!isBindingPattern(node.name)) {
Expand Down Expand Up @@ -1039,7 +1039,7 @@ namespace ts {
declareSymbolAndAddToSymbolTable(node, SymbolFlags.FunctionScopedVariable, SymbolFlags.ParameterExcludes);
}

// If this is a property-parameter, then also declare the property symbol into the
// If this is a property-parameter, then also declare the property symbol into the
// containing class.
if (node.flags & NodeFlags.AccessibilityModifier &&
node.parent.kind === SyntaxKind.Constructor &&
Expand All @@ -1056,4 +1056,4 @@ namespace ts {
: declareSymbolAndAddToSymbolTable(node, symbolFlags, symbolExcludes);
}
}
}
}
Loading