Skip to content

Commit ed941c2

Browse files
author
Andy
authored
Don't check modifier legality, highlight anyway (microsoft#21335)
1 parent 01d160e commit ed941c2

6 files changed

Lines changed: 44 additions & 72 deletions

File tree

‎src/services/documentHighlights.ts‎

Lines changed: 2 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -189,11 +189,6 @@ namespace ts.DocumentHighlights {
189189
}
190190

191191
function getModifierOccurrences(modifier: SyntaxKind, declaration: Node): Node[] {
192-
// Make sure we only highlight the keyword when it makes sense to do so.
193-
if (!isLegalModifier(modifier, declaration)) {
194-
return undefined;
195-
}
196-
197192
const modifierFlag = modifierToFlag(modifier);
198193
return mapDefined(getNodesToSearchForModifier(declaration, modifierFlag), node => {
199194
if (getModifierFlags(node) & modifierFlag) {
@@ -213,8 +208,8 @@ namespace ts.DocumentHighlights {
213208
case SyntaxKind.CaseClause:
214209
case SyntaxKind.DefaultClause:
215210
// Container is either a class declaration or the declaration is a classDeclaration
216-
if (modifierFlag & ModifierFlags.Abstract) {
217-
return [...(<ClassDeclaration>declaration).members, declaration];
211+
if (modifierFlag & ModifierFlags.Abstract && isClassDeclaration(declaration)) {
212+
return [...declaration.members, declaration];
218213
}
219214
else {
220215
return (<ModuleBlock | SourceFile | Block | CaseClause | DefaultClause>container).statements;
@@ -242,33 +237,6 @@ namespace ts.DocumentHighlights {
242237
}
243238
}
244239

245-
function isLegalModifier(modifier: SyntaxKind, declaration: Node): boolean {
246-
const container = declaration.parent;
247-
switch (modifier) {
248-
case SyntaxKind.PrivateKeyword:
249-
case SyntaxKind.ProtectedKeyword:
250-
case SyntaxKind.PublicKeyword:
251-
switch (container.kind) {
252-
case SyntaxKind.ClassDeclaration:
253-
case SyntaxKind.ClassExpression:
254-
return true;
255-
case SyntaxKind.Constructor:
256-
return declaration.kind === SyntaxKind.Parameter;
257-
default:
258-
return false;
259-
}
260-
case SyntaxKind.StaticKeyword:
261-
return container.kind === SyntaxKind.ClassDeclaration || container.kind === SyntaxKind.ClassExpression;
262-
case SyntaxKind.ExportKeyword:
263-
case SyntaxKind.DeclareKeyword:
264-
return container.kind === SyntaxKind.ModuleBlock || container.kind === SyntaxKind.SourceFile;
265-
case SyntaxKind.AbstractKeyword:
266-
return container.kind === SyntaxKind.ClassDeclaration || declaration.kind === SyntaxKind.ClassDeclaration;
267-
default:
268-
return false;
269-
}
270-
}
271-
272240
function pushKeywordIf(keywordList: Push<Node>, token: Node, ...expected: SyntaxKind[]): boolean {
273241
if (token && contains(expected, token.kind)) {
274242
keywordList.push(token);

‎tests/cases/fourslash/getOccurrencesConst01.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
/// <reference path='fourslash.ts' />
22

3-
/////*1*/const enum E1 {
3+
////[|const|] enum E1 {
44
//// v1,
55
//// v2
66
////}
77
////
88
/////*2*/const c = 0;
99

10-
goTo.marker("1");
11-
verify.occurrencesAtPositionCount(0);
10+
verify.rangesAreOccurrences();
1211

1312
goTo.marker("2");
1413
verify.occurrencesAtPositionCount(0);

‎tests/cases/fourslash/getOccurrencesConst02.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
/// <reference path='fourslash.ts' />
22

33
////module m {
4-
//// declare [|const|] x;
4+
//// declare /*1*/const x;
55
//// declare [|const|] enum E {
66
//// }
77
////}
88
////
9-
////declare [|const|] x;
9+
////declare /*2*/const x;
1010
////declare [|const|] enum E {
1111
////}
1212

13-
goTo.eachRange(() => verify.occurrencesAtPositionCount(0));
13+
goTo.eachRange(() => verify.occurrencesAtPositionCount(1)); // They are in different scopes, so not counted together.
14+
goTo.eachMarker(() => verify.occurrencesAtPositionCount(0));

‎tests/cases/fourslash/getOccurrencesConst03.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
/// <reference path='fourslash.ts' />
22

33
////module m {
4-
//// export [|const|] x;
4+
//// export /*1*/const x;
55
//// export [|const|] enum E {
66
//// }
77
////}
88
////
9-
////export [|const|] x;
9+
////export /*2*/const x;
1010
////export [|const|] enum E {
1111
////}
1212

13-
goTo.eachRange(() => verify.occurrencesAtPositionCount(0));
13+
goTo.eachRange(() => verify.occurrencesAtPositionCount(1)); // They are in different scopes, so not counted together.
14+
goTo.eachMarker(() => verify.occurrencesAtPositionCount(0));

‎tests/cases/fourslash/getOccurrencesConst04.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
////}
88

99
goTo.marker("1");
10-
verify.occurrencesAtPositionCount(0);
10+
verify.occurrencesAtPositionCount(1);
1111
goTo.marker("2");
1212
verify.occurrencesAtPositionCount(1);
1313
goTo.marker("3");
14-
verify.occurrencesAtPositionCount(0);
14+
verify.occurrencesAtPositionCount(1);

‎tests/cases/fourslash/getOccurrencesModifiersNegatives1.ts‎

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,40 @@
11
/// <reference path='fourslash.ts' />
22

33
////class C {
4-
//// [|export|] foo;
5-
//// [|declare|] bar;
6-
//// [|export|] [|declare|] foobar;
7-
//// [|declare|] [|export|] barfoo;
4+
//// [|{| "count": 3 |}export|] foo;
5+
//// [|{| "count": 3 |}declare|] bar;
6+
//// [|{| "count": 3 |}export|] [|{| "count": 3 |}declare|] foobar;
7+
//// [|{| "count": 3 |}declare|] [|{| "count": 3 |}export|] barfoo;
88
////
9-
//// constructor([|export|] conFoo,
10-
//// [|declare|] conBar,
11-
//// [|export|] [|declare|] conFooBar,
12-
//// [|declare|] [|export|] conBarFoo,
13-
//// [|static|] sue,
14-
//// [|static|] [|export|] [|declare|] sueFooBar,
15-
//// [|static|] [|declare|] [|export|] sueBarFoo,
16-
//// [|declare|] [|static|] [|export|] barSueFoo) {
9+
//// constructor([|{| "count": 9 |}export|] conFoo,
10+
//// [|{| "count": 9 |}declare|] conBar,
11+
//// [|{| "count": 9 |}export|] [|{| "count": 9 |}declare|] conFooBar,
12+
//// [|{| "count": 9 |}declare|] [|{| "count": 9 |}export|] conBarFoo,
13+
//// [|{| "count": 4 |}static|] sue,
14+
//// [|{| "count": 4 |}static|] [|{| "count": 9 |}export|] [|{| "count": 9 |}declare|] sueFooBar,
15+
//// [|{| "count": 4 |}static|] [|{| "count": 9 |}declare|] [|{| "count": 9 |}export|] sueBarFoo,
16+
//// [|{| "count": 9 |}declare|] [|{| "count": 4 |}static|] [|{| "count": 9 |}export|] barSueFoo) {
1717
//// }
1818
////}
1919
////
2020
////module m {
21-
//// [|static|] a;
22-
//// [|public|] b;
23-
//// [|private|] c;
24-
//// [|protected|] d;
25-
//// [|static|] [|public|] [|private|] [|protected|] e;
26-
//// [|public|] [|static|] [|protected|] [|private|] f;
27-
//// [|protected|] [|static|] [|public|] g;
21+
//// [|{| "count": 0 |}static|] a;
22+
//// [|{| "count": 0 |}public|] b;
23+
//// [|{| "count": 0 |}private|] c;
24+
//// [|{| "count": 0 |}protected|] d;
25+
//// [|{| "count": 0 |}static|] [|{| "count": 0 |}public|] [|{| "count": 0 |}private|] [|{| "count": 0 |}protected|] e;
26+
//// [|{| "count": 0 |}public|] [|{| "count": 0 |}static|] [|{| "count": 0 |}protected|] [|{| "count": 0 |}private|] f;
27+
//// [|{| "count": 0 |}protected|] [|{| "count": 0 |}static|] [|{| "count": 0 |}public|] g;
2828
////}
29-
////[|static|] a;
30-
////[|public|] b;
31-
////[|private|] c;
32-
////[|protected|] d;
33-
////[|static|] [|public|] [|private|] [|protected|] e;
34-
////[|public|] [|static|] [|protected|] [|private|] f;
35-
////[|protected|] [|static|] [|public|] g;
29+
////[|{| "count": 0 |}static|] a;
30+
////[|{| "count": 0 |}public|] b;
31+
////[|{| "count": 0 |}private|] c;
32+
////[|{| "count": 0 |}protected|] d;
33+
////[|{| "count": 0 |}static|] [|{| "count": 0 |}public|] [|{| "count": 0 |}private|] [|{| "count": 0 |}protected|] e;
34+
////[|{| "count": 0 |}public|] [|{| "count": 0 |}static|] [|{| "count": 0 |}protected|] [|{| "count": 0 |}private|] f;
35+
////[|{| "count": 0 |}protected|] [|{| "count": 0 |}static|] [|{| "count": 0 |}public|] g;
3636

37-
goTo.eachRange(() => verify.occurrencesAtPositionCount(0));
37+
for (const range of test.ranges()) {
38+
goTo.rangeStart(range);
39+
verify.occurrencesAtPositionCount(range.marker.data.count);
40+
}

0 commit comments

Comments
 (0)