Skip to content

Commit 90dd327

Browse files
DanielRosenwasserDanielRosenwasser
committed
Changed logic for break/continue search in switch statements and loops.
Now if a labeled break in a switch refers to its original switch statement, we also highlight the 'switch' keyword. Also added tests for loop/break/continue.
1 parent 69803d4 commit 90dd327

9 files changed

+560
-36
lines changed

‎src/services/services.ts

Lines changed: 69 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1359,11 +1359,19 @@ module ts {
13591359
}
13601360

13611361
enum SearchMeaning {
1362+
None = 0x0,
13621363
Value = 0x1,
13631364
Type = 0x2,
13641365
Namespace = 0x4
13651366
}
13661367

1368+
enum BreakContinueSearchType {
1369+
None = 0x0,
1370+
Unlabeled = 0x1,
1371+
Labeled = 0x2,
1372+
All = Unlabeled | Labeled
1373+
}
1374+
13671375
// A cache of completion entries for keywords, these do not change between sessions
13681376
var keywordCompletions:CompletionEntry[] = [];
13691377
for (var i = SyntaxKind.FirstKeyword; i <= SyntaxKind.LastKeyword; i++) {
@@ -2318,88 +2326,96 @@ module ts {
23182326
}
23192327
}
23202328
}
2329+
2330+
// These track whether we can own unlabeled break/continues.
2331+
var breakSearchType = BreakContinueSearchType.All;
2332+
var continueSearchType = BreakContinueSearchType.All;
23212333

2322-
// This switch tracks whether or not we're traversing into a construct that takes
2323-
// ownership over unlabelled 'break'/'continue' statements.
2324-
var onlyCheckLabelled = false;
2325-
2326-
forEachChild(loopNode.statement, function aggregateBreakContinues(node: Node) {
2327-
// This tracks the status of the flag before diving into the next node.
2328-
var lastOnlyCheckLabelled = onlyCheckLabelled;
2334+
(function aggregateBreakContinues(node: Node) {
2335+
// Remember the statuses of the flags before diving into the next node.
2336+
var prevBreakSearchType = breakSearchType;
2337+
var prevContinueSearchType = continueSearchType;
23292338

23302339
switch (node.kind) {
23312340
case SyntaxKind.BreakStatement:
23322341
case SyntaxKind.ContinueStatement:
2333-
// If the 'break'/'continue' statement has a label, it must be one of our tracked labels.
2334-
if ((<BreakOrContinueStatement>node).label) {
2335-
var labelName = (<BreakOrContinueStatement>node).label.text;
2336-
if (isLabelledBy(loopNode, labelName)) {
2337-
pushKeywordIf(keywords, node.getFirstToken(), SyntaxKind.BreakKeyword, SyntaxKind.ContinueKeyword);
2338-
}
2339-
}
2340-
// If not, we are free to add it if we haven't lost ownership of unlabeled break/continue statements.
2341-
else if (!onlyCheckLabelled) {
2342+
if (ownsBreakOrContinue(loopNode, <BreakOrContinueStatement>node, breakSearchType, continueSearchType)) {
23422343
pushKeywordIf(keywords, node.getFirstToken(), SyntaxKind.BreakKeyword, SyntaxKind.ContinueKeyword);
23432344
}
23442345
break;
2345-
2346+
23462347
case SyntaxKind.ForStatement:
23472348
case SyntaxKind.ForInStatement:
23482349
case SyntaxKind.DoStatement:
23492350
case SyntaxKind.WhileStatement:
2350-
case SyntaxKind.SwitchStatement:
2351-
onlyCheckLabelled = true;
2351+
continueSearchType = BreakContinueSearchType.Labeled;
23522352
// Fall through
2353-
default:
2354-
// Do not cross function boundaries.
2355-
if (!isAnyFunction(node)) {
2356-
forEachChild(node, aggregateBreakContinues);
2357-
}
2353+
case SyntaxKind.SwitchStatement:
2354+
breakSearchType = BreakContinueSearchType.Labeled;
2355+
}
2356+
2357+
// Do not cross function boundaries.
2358+
if (!isAnyFunction(node)) {
2359+
forEachChild(node, aggregateBreakContinues);
23582360
}
2361+
23592362
// Restore the last state.
2360-
onlyCheckLabelled = lastOnlyCheckLabelled;
2361-
});
2363+
breakSearchType = prevBreakSearchType;
2364+
continueSearchType = prevContinueSearchType;
2365+
})(loopNode.statement);
23622366

2363-
return map(keywords, keywordToReferenceEntry);
2367+
return map(keywords, getReferenceEntryFromNode);
23642368
}
23652369

23662370
function getSwitchCaseDefaultOccurrences(switchStatement: SwitchStatement) {
23672371
var keywords: Node[] = [];
23682372

23692373
pushKeywordIf(keywords, switchStatement.getFirstToken(), SyntaxKind.SwitchKeyword);
23702374

2371-
// Go through each clause in the switch statement, collecting the clause keywords.
2375+
// Types of break statements we can grab on to.
2376+
var breakSearchType = BreakContinueSearchType.All;
2377+
2378+
// Go through each clause in the switch statement, collecting the case/default keywords.
23722379
forEach(switchStatement.clauses, clause => {
23732380
pushKeywordIf(keywords, clause.getFirstToken(), SyntaxKind.CaseKeyword, SyntaxKind.DefaultKeyword);
23742381

23752382
// For each clause, also recursively traverse the statements where we can find analogous breaks.
23762383
forEachChild(clause, function aggregateBreakKeywords(node: Node): void {
2384+
// Back the old search value up.
2385+
var oldBreakSearchType = breakSearchType;
2386+
23772387
switch (node.kind) {
23782388
case SyntaxKind.BreakStatement:
23792389
// If the break statement has a label, it cannot be part of a switch block.
2380-
if (!(<BreakOrContinueStatement>node).label) {
2390+
if (ownsBreakOrContinue(switchStatement,
2391+
<BreakOrContinueStatement>node,
2392+
breakSearchType,
2393+
/*continuesSearchType*/ BreakContinueSearchType.None)) {
23812394
pushKeywordIf(keywords, node.getFirstToken(), SyntaxKind.BreakKeyword);
23822395
}
2383-
// Fall through
2396+
break;
23842397
case SyntaxKind.ForStatement:
23852398
case SyntaxKind.ForInStatement:
23862399
case SyntaxKind.DoStatement:
23872400
case SyntaxKind.WhileStatement:
23882401
case SyntaxKind.SwitchStatement:
2389-
return;
2402+
breakSearchType = BreakContinueSearchType.Labeled;
23902403
}
23912404

23922405
// Do not cross function boundaries.
23932406
if (!isAnyFunction(node)) {
23942407
forEachChild(node, aggregateBreakKeywords);
23952408
}
2409+
2410+
// Restore the last state.
2411+
breakSearchType = oldBreakSearchType;
23962412
});
23972413
});
23982414

23992415
return map(keywords, getReferenceEntryFromNode);
24002416
}
24012417

2402-
function getBreakOrContinueStatementOccurences(breakOrContinueStatement: BreakOrContinueStatement): ReferenceEntry[]{
2418+
function getBreakOrContinueStatementOccurences(breakOrContinueStatement: BreakOrContinueStatement): ReferenceEntry[] {
24032419
for (var owner = node.parent; owner; owner = owner.parent) {
24042420
switch (owner.kind) {
24052421
case SyntaxKind.ForStatement:
@@ -2413,8 +2429,8 @@ module ts {
24132429
}
24142430
break;
24152431
case SyntaxKind.SwitchStatement:
2416-
// A switch statement can only be the owner of an unlabeled break statement.
2417-
if (breakOrContinueStatement.kind === SyntaxKind.BreakStatement && !breakOrContinueStatement.label) {
2432+
// A switch statement can only be the owner of an break statement.
2433+
if (breakOrContinueStatement.kind === SyntaxKind.BreakStatement && (!breakOrContinueStatement.label || isLabelledBy(owner, breakOrContinueStatement.label.text))) {
24182434
return getSwitchCaseDefaultOccurrences(<SwitchStatement>owner);
24192435
}
24202436
break;
@@ -2429,6 +2445,25 @@ module ts {
24292445
return undefined;
24302446
}
24312447

2448+
// Note: 'statement' must be a descendant of 'root'.
2449+
// Reasonable logic for restricting traversal prior to arriving at the
2450+
// 'statement' node is beyond the scope of this function.
2451+
function ownsBreakOrContinue(root: Node,
2452+
statement: BreakOrContinueStatement,
2453+
breakSearchType: BreakContinueSearchType,
2454+
continueSearchType: BreakContinueSearchType): boolean {
2455+
var searchType = statement.kind === SyntaxKind.BreakStatement ?
2456+
breakSearchType :
2457+
continueSearchType;
2458+
2459+
if (statement.label) {
2460+
return isLabelledBy(root, statement.label.text);
2461+
}
2462+
else {
2463+
return !!(searchType & BreakContinueSearchType.Unlabeled);
2464+
}
2465+
}
2466+
24322467
// returns true if 'node' is defined and has a matching 'kind'.
24332468
function hasKind(node: Node, kind: SyntaxKind) {
24342469
return node !== undefined && node.kind === kind;

‎tests/cases/fourslash/getOccurrencesLoopBreakContinue.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////var arr = [1, 2, 3, 4];
4+
////label1: [|for|] (var n in arr) {
5+
//// [|break|];
6+
//// [|continue|];
7+
//// [|br/**/eak|] label1;
8+
//// [|continue|] label1;
9+
////
10+
//// label2: for (var i = 0; i < arr[n]; i++) {
11+
//// [|break|] label1;
12+
//// [|continue|] label1;
13+
////
14+
//// break;
15+
//// continue;
16+
//// break label2;
17+
//// continue label2;
18+
////
19+
//// function foo() {
20+
//// label3: while (true) {
21+
//// break;
22+
//// continue;
23+
//// break label3;
24+
//// continue label3;
25+
////
26+
//// // these cross function boundaries
27+
//// break label1;
28+
//// continue label1;
29+
//// break label2;
30+
//// continue label2;
31+
////
32+
//// label4: do {
33+
//// break;
34+
//// continue;
35+
//// break label4;
36+
//// continue label4;
37+
////
38+
//// break label3;
39+
//// continue label3;
40+
////
41+
//// switch (10) {
42+
//// case 1:
43+
//// case 2:
44+
//// break;
45+
//// break label4;
46+
//// default:
47+
//// continue;
48+
//// }
49+
////
50+
//// // these cross function boundaries
51+
//// break label1;
52+
//// continue label1;
53+
//// break label2;
54+
//// continue label2;
55+
//// () => { break; }
56+
//// } while (true)
57+
//// }
58+
//// }
59+
//// }
60+
////}
61+
////
62+
////label5: while (true) break label5;
63+
////
64+
////label7: while (true) continue label5;
65+
66+
test.ranges().forEach(r => {
67+
goTo.position(r.start);
68+
69+
test.ranges().forEach(range => {
70+
verify.occurrencesAtPositionContains(range, false);
71+
});
72+
});
73+
74+
goTo.marker();
75+
test.ranges().forEach(range => {
76+
verify.occurrencesAtPositionContains(range, false);
77+
});

‎tests/cases/fourslash/getOccurrencesLoopBreakContinue2.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////var arr = [1, 2, 3, 4];
4+
////label1: for (var n in arr) {
5+
//// break;
6+
//// continue;
7+
//// break label1;
8+
//// continue label1;
9+
////
10+
//// label2: [|f/**/or|] (var i = 0; i < arr[n]; i++) {
11+
//// break label1;
12+
//// continue label1;
13+
////
14+
//// [|break|];
15+
//// [|continue|];
16+
//// [|break|] label2;
17+
//// [|continue|] label2;
18+
////
19+
//// function foo() {
20+
//// label3: while (true) {
21+
//// break;
22+
//// continue;
23+
//// break label3;
24+
//// continue label3;
25+
////
26+
//// // these cross function boundaries
27+
//// break label1;
28+
//// continue label1;
29+
//// break label2;
30+
//// continue label2;
31+
////
32+
//// label4: do {
33+
//// break;
34+
//// continue;
35+
//// break label4;
36+
//// continue label4;
37+
////
38+
//// break label3;
39+
//// continue label3;
40+
////
41+
//// switch (10) {
42+
//// case 1:
43+
//// case 2:
44+
//// break;
45+
//// break label4;
46+
//// default:
47+
//// continue;
48+
//// }
49+
////
50+
//// // these cross function boundaries
51+
//// break label1;
52+
//// continue label1;
53+
//// break label2;
54+
//// continue label2;
55+
//// () => { break;
56+
//// } while (true)
57+
//// }
58+
//// }
59+
//// }
60+
////}
61+
////
62+
////label5: while (true) break label5;
63+
////
64+
////label7: while (true) continue label5;
65+
66+
test.ranges().forEach(r => {
67+
goTo.position(r.start);
68+
69+
test.ranges().forEach(range => {
70+
verify.occurrencesAtPositionContains(range, false);
71+
});
72+
});
73+
74+
goTo.marker();
75+
test.ranges().forEach(range => {
76+
verify.occurrencesAtPositionContains(range, false);
77+
});

0 commit comments

Comments
 (0)