diff --git a/src/language-js/reprinter/test_assets/typescript.tslint_nested_switches.input.ts.txt b/src/language-js/reprinter/test_assets/typescript.nested_switches.input.ts.txt similarity index 100% rename from src/language-js/reprinter/test_assets/typescript.tslint_nested_switches.input.ts.txt rename to src/language-js/reprinter/test_assets/typescript.nested_switches.input.ts.txt diff --git a/src/language-js/reprinter/test_assets/typescript.tslint_nested_switches.output.ts.txt b/src/language-js/reprinter/test_assets/typescript.nested_switches.output.ts.txt similarity index 100% rename from src/language-js/reprinter/test_assets/typescript.tslint_nested_switches.output.ts.txt rename to src/language-js/reprinter/test_assets/typescript.nested_switches.output.ts.txt index 7f90c1f5d..ca3253290 100644 --- a/src/language-js/reprinter/test_assets/typescript.tslint_nested_switches.output.ts.txt +++ b/src/language-js/reprinter/test_assets/typescript.nested_switches.output.ts.txt @@ -8,9 +8,6 @@ function canHaveTrailingTrivia(tokenKind: ts.SyntaxKind, parent: ts.Node): boole parent.kind !== ts.SyntaxKind.JsxExpression || parent.parent.kind !== ts.SyntaxKind.JsxElement ); - case ts.SyntaxKind.JsxText: - // there is no trivia after JsxText - return false; case ts.SyntaxKind.GreaterThanToken: switch (parent.kind) { case ts.SyntaxKind.JsxClosingElement: @@ -23,6 +20,9 @@ function canHaveTrailingTrivia(tokenKind: ts.SyntaxKind, parent: ts.Node): boole default: return true; } + case ts.SyntaxKind.JsxText: + // there is no trivia after JsxText + return false; default: return true; } diff --git a/src/language-js/sortSwitchCases/index.ts b/src/language-js/sortSwitchCases/index.ts index ce6053ea0..0824ce562 100644 --- a/src/language-js/sortSwitchCases/index.ts +++ b/src/language-js/sortSwitchCases/index.ts @@ -11,6 +11,12 @@ import { reorderValues } from "../../utilities/sort-utils"; +enum HasImmediateExitOption { + Indeterminate, + True, + False +} + export interface SortSwitchCaseOptions {} export function sortSwitchCases( @@ -37,6 +43,7 @@ export function sortSwitchCases( return fileContents; } + let doesBreakOut = HasImmediateExitOption.Indeterminate; let newFileContents = fileContents.slice(); let contextGroups = getContextGroups(cases, comments, fileContents); @@ -47,6 +54,11 @@ export function sortSwitchCases( continue; } + doesBreakOut = doesCaseBreakOutOfSwitch(cases[cases.length - 1]); + if (doesBreakOut !== HasImmediateExitOption.True) { + cases.pop(); + } + // Determine where the "break" statements are so we dont' sort through them // which would change the logic of the code let switchGroupsWithBreaks: Array = []; @@ -54,7 +66,11 @@ export function sortSwitchCases( let switchCaseEnd = 0; for (switchCaseEnd = 0; switchCaseEnd < cases.length; switchCaseEnd++) { // Do not rearrange items that are in a non-break statement - if (!doesCaseBreakOutOfSwitch(cases[switchCaseEnd])) { + doesBreakOut = doesCaseBreakOutOfSwitch(cases[switchCaseEnd]); + if (doesBreakOut === HasImmediateExitOption.Indeterminate) { + return fileContents; + } + if (doesBreakOut === HasImmediateExitOption.False) { continue; } @@ -109,11 +125,9 @@ export function sortSwitchCases( } // If the last switch group is a fall through, don't include it in the swap - if (!doesCaseBreakOutOfSwitch(cases[cases.length - 1])) { - switchGroupsWithBreaks = switchGroupsWithBreaks.slice( - 0, - switchGroupsWithBreaks.length - 1 - ); + doesBreakOut = doesCaseBreakOutOfSwitch(cases[cases.length - 1]); + if (doesBreakOut !== HasImmediateExitOption.True) { + switchGroupsWithBreaks.pop(); } // Now sort the actual switch groups @@ -165,20 +179,48 @@ export function sortSwitchCases( return newFileContents; } -function doesCaseBreakOutOfSwitch(caseStatement: any) { - let breakStatement = caseStatement.consequent.filter((value: any) => { - if (value.type === "BlockStatement") { - return ( - value.body.filter((value: any) => { - return ( - value.type === "BreakStatement" || value.type === "ReturnStatement" - ); - }).length !== 0 - ); +function doesCaseBreakOutOfSwitch(caseStatement: any): HasImmediateExitOption { + return doesHaveImmediateExit(caseStatement.consequent); +} + +function doesHaveImmediateExit(values: any): HasImmediateExitOption { + for (let value of values) { + switch (value.type) { + case "BlockStatement": { + return doesHaveImmediateExit(value.body); + } + case "BreakStatement": + case "ReturnStatement": + case "ThrowStatement": + return HasImmediateExitOption.True; + case "SwitchStatement": { + // If the last option in the switch statement has an exit, then either + // all previosu consequents have an exit (e.g. not getting to the last one) + // or they all fall through to the last one which means we exit + if ( + doesHaveImmediateExit( + value.cases[value.cases.length - 1].consequent + ) === HasImmediateExitOption.True + ) { + return HasImmediateExitOption.True; + } + } + default: + // There are several types which are a bit more complicated which + // leaves us in an undeterminate state if we will exit or not + if ( + // Value is some sort of loop + value.body != null || + // Value is some sort of conditional + value.consequent != null || + // Value is a try catch + value.block != null + ) { + return HasImmediateExitOption.Indeterminate; + } } - return value.type === "BreakStatement" || value.type === "ReturnStatement"; - }); - return breakStatement.length !== 0; + } + return HasImmediateExitOption.False; } function getSortableText(a: any, fileContents: string) { diff --git a/src/language-js/sortSwitchCases/test_assets/typescript.fallthrough_as_last_doesnt_sort.input.txt b/src/language-js/sortSwitchCases/test_assets/typescript.fallthrough_as_last_doesnt_sort.input.txt index 7c422e25e..f366949fe 100644 --- a/src/language-js/sortSwitchCases/test_assets/typescript.fallthrough_as_last_doesnt_sort.input.txt +++ b/src/language-js/sortSwitchCases/test_assets/typescript.fallthrough_as_last_doesnt_sort.input.txt @@ -1,8 +1,9 @@ // Since the last case statement falls through, if we sort without ignoring the last break, we will log "a" and "b", not just "a" -switch ("a") { - case "b": - log("b"); +switch (unify.kind) { + case "single-parameter-difference": { break; - case "a": - log("a"); + } + case "extra-parameter": { + const { extraParameter, otherSignature } = unify; + } } \ No newline at end of file diff --git a/src/language-js/sortSwitchCases/test_assets/typescript.fallthrough_as_last_doesnt_sort.output.txt b/src/language-js/sortSwitchCases/test_assets/typescript.fallthrough_as_last_doesnt_sort.output.txt index 7c422e25e..f366949fe 100644 --- a/src/language-js/sortSwitchCases/test_assets/typescript.fallthrough_as_last_doesnt_sort.output.txt +++ b/src/language-js/sortSwitchCases/test_assets/typescript.fallthrough_as_last_doesnt_sort.output.txt @@ -1,8 +1,9 @@ // Since the last case statement falls through, if we sort without ignoring the last break, we will log "a" and "b", not just "a" -switch ("a") { - case "b": - log("b"); +switch (unify.kind) { + case "single-parameter-difference": { break; - case "a": - log("a"); + } + case "extra-parameter": { + const { extraParameter, otherSignature } = unify; + } } \ No newline at end of file diff --git a/src/language-js/sortSwitchCases/test_assets/typescript.indeterminate_break_in_case.input.txt b/src/language-js/sortSwitchCases/test_assets/typescript.indeterminate_break_in_case.input.txt new file mode 100644 index 000000000..a164f7b42 --- /dev/null +++ b/src/language-js/sortSwitchCases/test_assets/typescript.indeterminate_break_in_case.input.txt @@ -0,0 +1,29 @@ +// Currently we declare this "indeterminate" because "ts.SyntaxKind.PropertyAccessExpression2" has a conditional that sometimes breaks and sometimes doesn't. +// In these situations we should return the original switch statement as to not break the execution code +switch (node.kind) { + case ts.SyntaxKind.PropertyAccessExpression2: + if (false) { + break; + } // Fallthrough into next line + case ts.SyntaxKind.PropertyAccessExpression: + return isSimple((node as ts.PropertyAccessExpression).expression); + case ts.SyntaxKind.PrefixUnaryExpression: + switch ((node as ts.PrefixUnaryExpression).operator) { + case ts.SyntaxKind.PlusPlusToken: + case ts.SyntaxKind.MinusMinusToken: + return false; + default: + return isSimple((node as ts.PrefixUnaryExpression).operand); + } + case ts.SyntaxKind.Identifier: + case ts.SyntaxKind.NumericLiteral: + case ts.SyntaxKind.StringLiteral: + case ts.SyntaxKind.ThisKeyword: + case ts.SyntaxKind.NoSubstitutionTemplateLiteral: + case ts.SyntaxKind.TrueKeyword: + case ts.SyntaxKind.FalseKeyword: + case ts.SyntaxKind.NullKeyword: + return true; + default: + return false; +} \ No newline at end of file diff --git a/src/language-js/sortSwitchCases/test_assets/typescript.indeterminate_break_in_case.output.txt b/src/language-js/sortSwitchCases/test_assets/typescript.indeterminate_break_in_case.output.txt new file mode 100644 index 000000000..a164f7b42 --- /dev/null +++ b/src/language-js/sortSwitchCases/test_assets/typescript.indeterminate_break_in_case.output.txt @@ -0,0 +1,29 @@ +// Currently we declare this "indeterminate" because "ts.SyntaxKind.PropertyAccessExpression2" has a conditional that sometimes breaks and sometimes doesn't. +// In these situations we should return the original switch statement as to not break the execution code +switch (node.kind) { + case ts.SyntaxKind.PropertyAccessExpression2: + if (false) { + break; + } // Fallthrough into next line + case ts.SyntaxKind.PropertyAccessExpression: + return isSimple((node as ts.PropertyAccessExpression).expression); + case ts.SyntaxKind.PrefixUnaryExpression: + switch ((node as ts.PrefixUnaryExpression).operator) { + case ts.SyntaxKind.PlusPlusToken: + case ts.SyntaxKind.MinusMinusToken: + return false; + default: + return isSimple((node as ts.PrefixUnaryExpression).operand); + } + case ts.SyntaxKind.Identifier: + case ts.SyntaxKind.NumericLiteral: + case ts.SyntaxKind.StringLiteral: + case ts.SyntaxKind.ThisKeyword: + case ts.SyntaxKind.NoSubstitutionTemplateLiteral: + case ts.SyntaxKind.TrueKeyword: + case ts.SyntaxKind.FalseKeyword: + case ts.SyntaxKind.NullKeyword: + return true; + default: + return false; +} \ No newline at end of file