From 1cf765d664ad78bd0a2b2daa89311b23ec58161e Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 27 Mar 2017 15:09:00 -0700 Subject: [PATCH 1/2] Lint for fallthrough in switch --- src/compiler/binder.ts | 6 ++++-- src/compiler/checker.ts | 17 +++++++++++------ src/compiler/parser.ts | 9 ++++++--- src/compiler/program.ts | 8 ++++---- src/compiler/scanner.ts | 6 +++++- src/compiler/transformers/module/system.ts | 2 +- src/compiler/utilities.ts | 9 ++++++--- src/compiler/visitor.ts | 6 ++++-- src/services/breakpoints.ts | 7 +++++-- src/services/classifier.ts | 2 +- src/services/codefixes/importFixes.ts | 3 ++- src/services/codefixes/unusedIdentifierFixes.ts | 2 ++ src/services/completions.ts | 2 +- src/services/documentHighlights.ts | 2 +- src/services/findAllReferences.ts | 4 ++-- src/services/formatting/formatting.ts | 5 ++--- src/services/outliningElementsCollector.ts | 2 +- src/services/services.ts | 5 +++-- src/services/utilities.ts | 2 +- tslint.json | 3 ++- 20 files changed, 64 insertions(+), 38 deletions(-) diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index 650797c6dba2a..eb215ce158cd8 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -1413,6 +1413,7 @@ namespace ts { if (isObjectLiteralOrClassExpressionMethod(node)) { return ContainerFlags.IsContainer | ContainerFlags.IsControlFlowContainer | ContainerFlags.HasLocals | ContainerFlags.IsFunctionLike | ContainerFlags.IsObjectLiteralOrClassExpressionMethod; } + // falls through case SyntaxKind.Constructor: case SyntaxKind.FunctionDeclaration: case SyntaxKind.MethodSignature: @@ -1714,7 +1715,7 @@ namespace ts { declareModuleMember(node, symbolFlags, symbolExcludes); break; } - // fall through. + // falls through default: if (!blockScopeContainer.locals) { blockScopeContainer.locals = createMap(); @@ -2006,6 +2007,7 @@ namespace ts { bindBlockScopedDeclaration(parentNode, SymbolFlags.TypeAlias, SymbolFlags.TypeAliasExcludes); break; } + // falls through case SyntaxKind.ThisKeyword: if (currentFlow && (isExpression(node) || parent.kind === SyntaxKind.ShorthandPropertyAssignment)) { node.flowNode = currentFlow; @@ -2183,7 +2185,7 @@ namespace ts { if (!isFunctionLike(node.parent)) { return; } - // Fall through + // falls through case SyntaxKind.ModuleBlock: return updateStrictModeStatementList((node).statements); } diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 4e55a1c46b564..5dc8f83ff5bda 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -872,6 +872,7 @@ namespace ts { case SyntaxKind.SourceFile: if (!isExternalOrCommonJsModule(location)) break; isInExternalModule = true; + // falls through case SyntaxKind.ModuleDeclaration: const moduleExports = getSymbolOfNode(location).exports; if (location.kind === SyntaxKind.SourceFile || isAmbientModule(location)) { @@ -1872,6 +1873,7 @@ namespace ts { if (!isExternalOrCommonJsModule(location)) { break; } + // falls through case SyntaxKind.ModuleDeclaration: if (result = callback(getSymbolOfNode(location).exports)) { return result; @@ -3592,7 +3594,7 @@ namespace ts { // If the binding pattern is empty, this variable declaration is not visible return false; } - // Otherwise fall through + // falls through case SyntaxKind.ModuleDeclaration: case SyntaxKind.ClassDeclaration: case SyntaxKind.InterfaceDeclaration: @@ -3623,7 +3625,8 @@ namespace ts { // Private/protected properties/methods are not visible return false; } - // Public properties/methods are visible if its parents are visible, so const it fall into next case statement + // Public properties/methods are visible if its parents are visible, so + // falls through case SyntaxKind.Constructor: case SyntaxKind.ConstructSignature: @@ -20863,7 +20866,7 @@ namespace ts { } break; } - // fallthrough + // falls through case SyntaxKind.ClassDeclaration: case SyntaxKind.EnumDeclaration: case SyntaxKind.FunctionDeclaration: @@ -21498,6 +21501,7 @@ namespace ts { if (!isExternalOrCommonJsModule(location)) { break; } + // falls through case SyntaxKind.ModuleDeclaration: copySymbols(getSymbolOfNode(location).exports, meaning & SymbolFlags.ModuleMember); break; @@ -21509,7 +21513,8 @@ namespace ts { if (className) { copySymbol(location.symbol, meaning); } - // fall through; this fall-through is necessary because we would like to handle + // falls through + // this fall-through is necessary because we would like to handle // type parameter inside class expression similar to how we handle it in classDeclaration and interface Declaration case SyntaxKind.ClassDeclaration: case SyntaxKind.InterfaceDeclaration: @@ -21795,7 +21800,7 @@ namespace ts { return sig.thisParameter; } } - // fallthrough + // falls through case SyntaxKind.SuperKeyword: const type = isPartOfExpression(node) ? getTypeOfExpression(node) : getTypeFromTypeNode(node); @@ -21823,7 +21828,7 @@ namespace ts { if (isInJavaScriptFile(node) && isRequireCall(node.parent, /*checkArgumentIsStringLiteral*/ false)) { return resolveExternalModuleName(node, node); } - // Fall through + // falls through case SyntaxKind.NumericLiteral: // index access diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index ee74ce9c9a596..ffdfe67af87f9 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -3576,6 +3576,7 @@ namespace ts { if (isAwaitExpression()) { return parseAwaitExpression(); } + // falls through default: return parseIncrementExpression(); } @@ -3609,8 +3610,8 @@ namespace ts { if (sourceFile.languageVariant !== LanguageVariant.JSX) { return false; } - // We are in JSX context and the token is part of JSXElement. - // Fall through + // We are in JSX context and the token is part of JSXElement. + // falls through default: return true; } @@ -6557,7 +6558,8 @@ namespace ts { indent += scanner.getTokenText().length; break; } - // FALLTHROUGH otherwise to record the * as a comment + // record the * as a comment + // falls through default: state = JSDocState.SavingComments; // leading identifiers start recording as well pushComment(scanner.getTokenText()); @@ -6783,6 +6785,7 @@ namespace ts { break; case SyntaxKind.Identifier: canParseTag = false; + break; case SyntaxKind.EndOfFileToken: break; } diff --git a/src/compiler/program.ts b/src/compiler/program.ts index cffb6df1e3d7d..73d3c0ddb3a33 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -941,8 +941,7 @@ namespace ts { diagnostics.push(createDiagnosticForNode(node, Diagnostics._0_can_only_be_used_in_a_ts_file, "?")); return; } - - // Pass through + // falls through case SyntaxKind.MethodDeclaration: case SyntaxKind.MethodSignature: case SyntaxKind.Constructor: @@ -1022,7 +1021,7 @@ namespace ts { diagnostics.push(createDiagnosticForNodeArray(nodes, Diagnostics.type_parameter_declarations_can_only_be_used_in_a_ts_file)); return; } - // pass through + // falls through case SyntaxKind.VariableStatement: // Check modifiers if (nodes === (parent).modifiers) { @@ -1070,7 +1069,8 @@ namespace ts { if (isConstValid) { continue; } - // Fallthrough to report error + // to report error, + // falls through case SyntaxKind.PublicKeyword: case SyntaxKind.PrivateKeyword: case SyntaxKind.ProtectedKeyword: diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts index 466074bdbcc6f..fe79a7bfab3a7 100644 --- a/src/compiler/scanner.ts +++ b/src/compiler/scanner.ts @@ -306,6 +306,7 @@ namespace ts { if (text.charCodeAt(pos) === CharacterCodes.lineFeed) { pos++; } + // falls through case CharacterCodes.lineFeed: result.push(lineStart); lineStart = pos; @@ -452,6 +453,7 @@ namespace ts { if (text.charCodeAt(pos + 1) === CharacterCodes.lineFeed) { pos++; } + // falls through case CharacterCodes.lineFeed: pos++; if (stopAfterLineBreak) { @@ -623,6 +625,7 @@ namespace ts { if (text.charCodeAt(pos + 1) === CharacterCodes.lineFeed) { pos++; } + // falls through case CharacterCodes.lineFeed: pos++; if (trailing) { @@ -1067,7 +1070,7 @@ namespace ts { if (pos < end && text.charCodeAt(pos) === CharacterCodes.lineFeed) { pos++; } - // fall through + // falls through case CharacterCodes.lineFeed: case CharacterCodes.lineSeparator: case CharacterCodes.paragraphSeparator: @@ -1449,6 +1452,7 @@ namespace ts { // This fall-through is a deviation from the EcmaScript grammar. The grammar says that a leading zero // can only be followed by an octal digit, a dot, or the end of the number literal. However, we are being // permissive and allowing decimal digits of the form 08* and 09* (which many browsers also do). + // falls through case CharacterCodes._1: case CharacterCodes._2: case CharacterCodes._3: diff --git a/src/compiler/transformers/module/system.ts b/src/compiler/transformers/module/system.ts index 642ac1ce6fbaf..e8c438531916f 100644 --- a/src/compiler/transformers/module/system.ts +++ b/src/compiler/transformers/module/system.ts @@ -477,8 +477,8 @@ namespace ts { // module is imported only for side-effects, no emit required break; } + // falls through - // fall-through case SyntaxKind.ImportEqualsDeclaration: Debug.assert(importVariableName !== undefined); // save import into the local diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 33cb0d8f4898d..fb2a9c34aba99 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -733,6 +733,7 @@ namespace ts { // At this point, node is either a qualified name or an identifier Debug.assert(node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.QualifiedName || node.kind === SyntaxKind.PropertyAccessExpression, "'node' was expected to be a qualified name, identifier or property access in 'isPartOfTypeNode'."); + // falls through case SyntaxKind.QualifiedName: case SyntaxKind.PropertyAccessExpression: case SyntaxKind.ThisKeyword: @@ -842,6 +843,7 @@ namespace ts { if (operand) { traverse(operand); } + return; case SyntaxKind.EnumDeclaration: case SyntaxKind.InterfaceDeclaration: case SyntaxKind.ModuleDeclaration: @@ -1059,7 +1061,7 @@ namespace ts { if (!includeArrowFunctions) { continue; } - // Fall through + // falls through case SyntaxKind.FunctionDeclaration: case SyntaxKind.FunctionExpression: case SyntaxKind.ModuleDeclaration: @@ -1118,6 +1120,7 @@ namespace ts { if (!stopOnFunctions) { continue; } + // falls through case SyntaxKind.PropertyDeclaration: case SyntaxKind.PropertySignature: case SyntaxKind.MethodDeclaration: @@ -1317,7 +1320,7 @@ namespace ts { if (node.parent.kind === SyntaxKind.TypeQuery || isJSXTagName(node)) { return true; } - // fall through + // falls through case SyntaxKind.NumericLiteral: case SyntaxKind.StringLiteral: case SyntaxKind.ThisKeyword: @@ -1995,7 +1998,7 @@ namespace ts { if (node.asteriskToken) { flags |= FunctionFlags.Generator; } - // fall through + // falls through case SyntaxKind.ArrowFunction: if (hasModifier(node, ModifierFlags.Async)) { flags |= FunctionFlags.Async; diff --git a/src/compiler/visitor.ts b/src/compiler/visitor.ts index 297d2cc0dde3c..b4b46a52fee67 100644 --- a/src/compiler/visitor.ts +++ b/src/compiler/visitor.ts @@ -322,7 +322,7 @@ namespace ts { nodesVisitor((node).types, visitor, isTypeNode)); case SyntaxKind.ParenthesizedType: - Debug.fail("not implemented."); + throw Debug.fail("not implemented."); case SyntaxKind.TypeOperator: return updateTypeOperatorNode(node, visitNode((node).type, visitor, isTypeNode)); @@ -1289,6 +1289,7 @@ namespace ts { case SyntaxKind.JsxAttributes: result = reduceNodes((node).properties, cbNodes, result); + break; case SyntaxKind.JsxClosingElement: result = reduceNode((node).tagName, cbNode, result); @@ -1310,7 +1311,7 @@ namespace ts { // Clauses case SyntaxKind.CaseClause: result = reduceNode((node).expression, cbNode, result); - // fall-through + // falls through case SyntaxKind.DefaultClause: result = reduceNodes((node).statements, cbNodes, result); @@ -1344,6 +1345,7 @@ namespace ts { case SyntaxKind.EnumMember: result = reduceNode((node).name, cbNode, result); result = reduceNode((node).initializer, cbNode, result); + break; // Top-level nodes case SyntaxKind.SourceFile: diff --git a/src/services/breakpoints.ts b/src/services/breakpoints.ts index 1f6bec4110902..7a49fd2ead7d6 100644 --- a/src/services/breakpoints.ts +++ b/src/services/breakpoints.ts @@ -97,7 +97,7 @@ namespace ts.BreakpointResolver { if (isFunctionBlock(node)) { return spanInFunctionBlock(node); } - // Fall through + // falls through case SyntaxKind.ModuleBlock: return spanInBlock(node); @@ -186,6 +186,7 @@ namespace ts.BreakpointResolver { if (getModuleInstanceState(node) !== ModuleInstanceState.Instantiated) { return undefined; } + // falls through case SyntaxKind.ClassDeclaration: case SyntaxKind.EnumDeclaration: @@ -473,6 +474,7 @@ namespace ts.BreakpointResolver { if (getModuleInstanceState(block.parent) !== ModuleInstanceState.Instantiated) { return undefined; } + // falls through // Set on parent if on same line otherwise on first statement case SyntaxKind.WhileStatement: @@ -582,6 +584,7 @@ namespace ts.BreakpointResolver { if (getModuleInstanceState(node.parent.parent) !== ModuleInstanceState.Instantiated) { return undefined; } + // falls through case SyntaxKind.EnumDeclaration: case SyntaxKind.ClassDeclaration: @@ -593,7 +596,7 @@ namespace ts.BreakpointResolver { // Span on close brace token return textSpan(node); } - // fall through + // falls through case SyntaxKind.CatchClause: return spanInNode(lastOrUndefined((node.parent).statements)); diff --git a/src/services/classifier.ts b/src/services/classifier.ts index c15b65b0c3900..0deff25321206 100644 --- a/src/services/classifier.ts +++ b/src/services/classifier.ts @@ -160,7 +160,7 @@ namespace ts { case EndOfLineState.InTemplateMiddleOrTail: text = "}\n" + text; offset = 2; - // fallthrough + // falls through case EndOfLineState.InTemplateSubstitutionPosition: templateStack.push(SyntaxKind.TemplateHead); break; diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 0c1511ff1ebdb..804a41cda089d 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -42,12 +42,13 @@ namespace ts.codefix { switch (this.compareModuleSpecifiers(existingAction.moduleSpecifier, newAction.moduleSpecifier)) { case ModuleSpecifierComparison.Better: - // the new one is not worth considering if it is a new improt. + // the new one is not worth considering if it is a new import. // However if it is instead a insertion into existing import, the user might want to use // the module specifier even it is worse by our standards. So keep it. if (newAction.kind === "NewImport") { return; } + // falls through case ModuleSpecifierComparison.Equal: // the current one is safe. But it is still possible that the new one is worse // than another existing one. For example, you may have new imports from "./foo/bar" diff --git a/src/services/codefixes/unusedIdentifierFixes.ts b/src/services/codefixes/unusedIdentifierFixes.ts index f77f9723d0f1e..c16de71c9c6f2 100644 --- a/src/services/codefixes/unusedIdentifierFixes.ts +++ b/src/services/codefixes/unusedIdentifierFixes.ts @@ -58,6 +58,8 @@ namespace ts.codefix { return deleteNodeInList(token.parent); } } + // TODO: #14885 + // falls through case SyntaxKind.TypeParameter: const typeParameters = (token.parent.parent).typeParameters; diff --git a/src/services/completions.ts b/src/services/completions.ts index 738c8b3b8bdff..40e1c19411187 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -485,7 +485,7 @@ namespace ts.Completions { // It has a left-hand side, so we're not in an opening JSX tag. break; } - // fall through + // falls through case SyntaxKind.JsxSelfClosingElement: case SyntaxKind.JsxElement: diff --git a/src/services/documentHighlights.ts b/src/services/documentHighlights.ts index 7308ad235fce9..9108b3785ab55 100644 --- a/src/services/documentHighlights.ts +++ b/src/services/documentHighlights.ts @@ -245,7 +245,7 @@ namespace ts.DocumentHighlights { if (statement.kind === SyntaxKind.ContinueStatement) { continue; } - // Fall through. + // falls through case SyntaxKind.ForStatement: case SyntaxKind.ForInStatement: case SyntaxKind.ForOfStatement: diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts index 5a471fb6f356c..b91eddebb3e01 100644 --- a/src/services/findAllReferences.ts +++ b/src/services/findAllReferences.ts @@ -956,7 +956,7 @@ namespace ts.FindAllReferences { if (isObjectLiteralMethod(searchSpaceNode)) { break; } - // fall through + // falls through case SyntaxKind.PropertyDeclaration: case SyntaxKind.PropertySignature: case SyntaxKind.Constructor: @@ -969,7 +969,7 @@ namespace ts.FindAllReferences { if (isExternalModule(searchSpaceNode)) { return undefined; } - // Fall through + // falls through case SyntaxKind.FunctionDeclaration: case SyntaxKind.FunctionExpression: break; diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 4fa3b71a3a200..c0c5e1793957e 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -483,9 +483,8 @@ namespace ts.formatting { case SyntaxKind.MethodDeclaration: if ((node).asteriskToken) { return SyntaxKind.AsteriskToken; - }/* - fall-through - */ + } + // falls through case SyntaxKind.PropertyDeclaration: case SyntaxKind.Parameter: return (node).name.kind; diff --git a/src/services/outliningElementsCollector.ts b/src/services/outliningElementsCollector.ts index 43054a10f5d39..5f17129db278e 100644 --- a/src/services/outliningElementsCollector.ts +++ b/src/services/outliningElementsCollector.ts @@ -144,7 +144,7 @@ namespace ts.OutliningElementsCollector { }); break; } - // Fallthrough. + // falls through case SyntaxKind.ModuleBlock: { const openBrace = findChildOfKind(n, SyntaxKind.OpenBraceToken, sourceFile); diff --git a/src/services/services.ts b/src/services/services.ts index d8a4cbeae675a..ae87b800afd87 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -635,7 +635,7 @@ namespace ts { if (!hasModifier(node, ModifierFlags.ParameterPropertyModifier)) { break; } - // fall through + // falls through case SyntaxKind.VariableDeclaration: case SyntaxKind.BindingElement: { const decl = node; @@ -646,6 +646,7 @@ namespace ts { if (decl.initializer) visit(decl.initializer); } + // falls through case SyntaxKind.EnumMember: case SyntaxKind.PropertyDeclaration: case SyntaxKind.PropertySignature: @@ -2028,7 +2029,7 @@ namespace ts { if (node.parent.kind === SyntaxKind.ComputedPropertyName) { return isObjectLiteralElement(node.parent.parent) ? node.parent.parent : undefined; } - // intentionally fall through + // falls through case SyntaxKind.Identifier: return isObjectLiteralElement(node.parent) && (node.parent.parent.kind === SyntaxKind.ObjectLiteralExpression || node.parent.parent.kind === SyntaxKind.JsxAttributes) && diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 92b63656573f9..0450c8c24eb52 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -450,7 +450,7 @@ namespace ts { if (!(n).arguments) { return true; } - // fall through + // falls through case SyntaxKind.CallExpression: case SyntaxKind.ParenthesizedExpression: case SyntaxKind.ParenthesizedType: diff --git a/tslint.json b/tslint.json index 920f60879098d..6d1b5f29ca726 100644 --- a/tslint.json +++ b/tslint.json @@ -49,6 +49,7 @@ "no-increment-decrement": true, "object-literal-surrounding-space": true, "no-type-assertion-whitespace": true, - "no-in-operator": true + "no-in-operator": true, + "no-switch-case-fall-through": true } } From 84a316bce26ff97512973fb4aef2af4738b7f49b Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 28 Mar 2017 09:53:49 -0700 Subject: [PATCH 2/2] Add colon --- src/compiler/checker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 5dc8f83ff5bda..9ee44d59ef39e 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -3625,7 +3625,7 @@ namespace ts { // Private/protected properties/methods are not visible return false; } - // Public properties/methods are visible if its parents are visible, so + // Public properties/methods are visible if its parents are visible, so: // falls through case SyntaxKind.Constructor: