Skip to content

Commit 84fbfba

Browse files
DanielRosenwasserDanielRosenwasser
committed
Merge pull request #883 from Microsoft/semanticColorizationModules
Fixed semantic colorization for module names on the value side.
2 parents e22500d + d266b68 commit 84fbfba

13 files changed

+222
-52
lines changed

‎src/harness/fourslash.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ module FourSlash {
146146

147147
function convertGlobalOptionsToCompilationSettings(globalOptions: { [idx: string]: string }): ts.CompilationSettings {
148148
var settings: ts.CompilationSettings = {};
149-
// Convert all property in globalOptions into ts.CompilationSettings
149+
// Convert all property in globalOptions into ts.CompilationSettings
150150
for (var prop in globalOptions) {
151151
if (globalOptions.hasOwnProperty(prop)) {
152152
switch (prop) {
@@ -1610,7 +1610,7 @@ module FourSlash {
16101610
Harness.IO.log(this.getNameOrDottedNameSpan(pos));
16111611
}
16121612

1613-
private verifyClassifications(expected: { classificationType: string; text: string }[], actual: ts.ClassifiedSpan[]) {
1613+
private verifyClassifications(expected: { classificationType: string; text: string; textSpan?: TextSpan }[], actual: ts.ClassifiedSpan[]) {
16141614
if (actual.length !== expected.length) {
16151615
this.raiseError('verifyClassifications failed - expected total classifications to be ' + expected.length + ', but was ' + actual.length);
16161616
}
@@ -1626,7 +1626,19 @@ module FourSlash {
16261626
actualClassification.classificationType);
16271627
}
16281628

1629+
var expectedSpan = expectedClassification.textSpan;
16291630
var actualSpan = actualClassification.textSpan;
1631+
1632+
if (expectedSpan) {
1633+
var expectedLength = expectedSpan.end - expectedSpan.start;
1634+
1635+
if (expectedSpan.start !== actualSpan.start() || expectedLength !== actualSpan.length()) {
1636+
this.raiseError("verifyClassifications failed - expected span of text to be " +
1637+
"{start=" + expectedSpan.start + ", length=" + expectedLength + "}, but was " +
1638+
"{start=" + actualSpan.start() + ", length=" + actualSpan.length() + "}");
1639+
}
1640+
}
1641+
16301642
var actualText = this.activeFile.content.substr(actualSpan.start(), actualSpan.length());
16311643
if (expectedClassification.text !== actualText) {
16321644
this.raiseError('verifyClassifications failed - expected classificatied text to be ' +

‎src/services/services.ts‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4734,7 +4734,24 @@ module ts {
47344734
}
47354735
}
47364736
else if (flags & SymbolFlags.Module) {
4737-
return ClassificationTypeNames.moduleName;
4737+
// Only classify a module as such if
4738+
// - It appears in a namespace context.
4739+
// - There exists a module declaration which actually impacts the value side.
4740+
if (meaningAtPosition & SemanticMeaning.Namespace ||
4741+
(meaningAtPosition & SemanticMeaning.Value && hasValueSideModule(symbol))) {
4742+
return ClassificationTypeNames.moduleName;
4743+
}
4744+
}
4745+
4746+
return undefined;
4747+
4748+
/**
4749+
* Returns true if there exists a module that introduces entities on the value side.
4750+
*/
4751+
function hasValueSideModule(symbol: Symbol): boolean {
4752+
return forEach(symbol.declarations, declaration => {
4753+
return declaration.kind === SyntaxKind.ModuleDeclaration && isInstantiated(declaration);
4754+
});
47384755
}
47394756
}
47404757

‎tests/cases/fourslash/fourslash.ts‎

Lines changed: 49 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,10 @@ module FourSlashInterface {
8080
return FourSlash.currentTestState.getMarkers();
8181
}
8282

83+
public marker(name?: string): Marker {
84+
return FourSlash.currentTestState.getMarkerByName(name);
85+
}
86+
8387
public ranges(): Range[] {
8488
return FourSlash.currentTestState.getRanges();
8589
}
@@ -401,11 +405,17 @@ module FourSlashInterface {
401405
FourSlash.currentTestState.verifyCompletionEntryDetails(entryName, text, documentation, kind);
402406
}
403407

408+
/**
409+
* This method *requires* a contiguous, complete, and ordered stream of classifications for a file.
410+
*/
404411
public syntacticClassificationsAre(...classifications: { classificationType: string; text: string }[]) {
405412
FourSlash.currentTestState.verifySyntacticClassifications(classifications);
406413
}
407414

408-
public semanticClassificationsAre(...classifications: { classificationType: string; text: string }[]) {
415+
/**
416+
* This method *requires* an ordered stream of classifications for a file, and spans are highly recommended.
417+
*/
418+
public semanticClassificationsAre(...classifications: { classificationType: string; text: string; textSpan?: TextSpan }[]) {
409419
FourSlash.currentTestState.verifySemanticClassifications(classifications);
410420
}
411421

@@ -563,61 +573,69 @@ module FourSlashInterface {
563573
}
564574
}
565575

566-
export class classification {
567-
public static comment(text: string): { classificationType: string; text: string } {
568-
return { classificationType: "comment", text: text };
576+
export module classification {
577+
export function comment(text: string, position?: number): { classificationType: string; text: string; textSpan?: TextSpan } {
578+
return getClassification("comment", text, position);
569579
}
570580

571-
public static identifier(text: string): { classificationType: string; text: string } {
572-
return { classificationType: "identifier", text: text };
581+
export function identifier(text: string, position?: number): { classificationType: string; text: string; textSpan?: TextSpan } {
582+
return getClassification("identifier", text, position);
573583
}
574584

575-
public static keyword(text: string): { classificationType: string; text: string } {
576-
return { classificationType: "keyword", text: text };
585+
export function keyword(text: string, position?: number): { classificationType: string; text: string; textSpan?: TextSpan } {
586+
return getClassification("keyword", text, position);
577587
}
578588

579-
public static numericLiteral(text: string): { classificationType: string; text: string } {
580-
return { classificationType: "numericLiteral", text: text };
589+
export function numericLiteral(text: string, position?: number): { classificationType: string; text: string; textSpan?: TextSpan } {
590+
return getClassification("numericLiteral", text, position);
581591
}
582592

583-
public static operator(text: string): { classificationType: string; text: string } {
584-
return { classificationType: "operator", text: text };
593+
export function operator(text: string, position?: number): { classificationType: string; text: string; textSpan?: TextSpan } {
594+
return getClassification("operator", text, position);
585595
}
586596

587-
public static stringLiteral(text: string): { classificationType: string; text: string } {
588-
return { classificationType: "stringLiteral", text: text };
597+
export function stringLiteral(text: string, position?: number): { classificationType: string; text: string; textSpan?: TextSpan } {
598+
return getClassification("stringLiteral", text, position);
589599
}
590600

591-
public static whiteSpace(text: string): { classificationType: string; text: string } {
592-
return { classificationType: "whiteSpace", text: text };
601+
export function whiteSpace(text: string, position?: number): { classificationType: string; text: string; textSpan?: TextSpan } {
602+
return getClassification("whiteSpace", text, position);
593603
}
594604

595-
public static text(text: string): { classificationType: string; text: string } {
596-
return { classificationType: "text", text: text };
605+
export function text(text: string, position?: number): { classificationType: string; text: string; textSpan?: TextSpan } {
606+
return getClassification("text", text, position);
597607
}
598608

599-
public static punctuation(text: string): { classificationType: string; text: string } {
600-
return { classificationType: "punctuation", text: text };
609+
export function punctuation(text: string, position?: number): { classificationType: string; text: string; textSpan?: TextSpan } {
610+
return getClassification("punctuation", text, position);
601611
}
602612

603-
public static className(text: string): { classificationType: string; text: string } {
604-
return { classificationType: "className", text: text };
613+
export function className(text: string, position?: number): { classificationType: string; text: string; textSpan?: TextSpan } {
614+
return getClassification("className", text, position);
605615
}
606616

607-
public static enumName(text: string): { classificationType: string; text: string } {
608-
return { classificationType: "enumName", text: text };
617+
export function enumName(text: string, position?: number): { classificationType: string; text: string; textSpan?: TextSpan } {
618+
return getClassification("enumName", text, position);
609619
}
610620

611-
public static interfaceName(text: string): { classificationType: string; text: string } {
612-
return { classificationType: "interfaceName", text: text };
621+
export function interfaceName(text: string, position?: number): { classificationType: string; text: string; textSpan?: TextSpan } {
622+
return getClassification("interfaceName", text, position);
613623
}
614624

615-
public static moduleName(text: string): { classificationType: string; text: string } {
616-
return { classificationType: "moduleName", text: text };
625+
export function moduleName(text: string, position?: number): { classificationType: string; text: string; textSpan?: TextSpan } {
626+
return getClassification("moduleName", text, position);
617627
}
618628

619-
public static typeParameterName(text: string): { classificationType: string; text: string } {
620-
return { classificationType: "typeParameterName", text: text };
629+
export function typeParameterName(text: string, position?: number): { classificationType: string; text: string; textSpan?: TextSpan } {
630+
return getClassification("typeParameterName", text, position);
631+
}
632+
633+
function getClassification(type: string, text: string, position?: number) {
634+
return {
635+
classificationType: type,
636+
text: text,
637+
textSpan: position === undefined ? undefined : { start: position, end: position + text.length }
638+
};
621639
}
622640
}
623641
}
@@ -635,6 +653,7 @@ module fs {
635653
function verifyOperationIsCancelled(f) {
636654
FourSlash.verifyOperationIsCancelled(f);
637655
}
656+
638657
var test = new FourSlashInterface.test_();
639658
var goTo = new FourSlashInterface.goTo();
640659
var verify = new FourSlashInterface.verify();

‎tests/cases/fourslash/semanticClassification1.ts‎

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

3-
//// module M {
4-
//// export interface I {
3+
//// module /*0*/M {
4+
//// export interface /*1*/I {
55
//// }
66
//// }
7-
//// interface X extends M.I { }
7+
//// interface /*2*/X extends /*3*/M./*4*/I { }
88

99
var c = classification;
1010
verify.semanticClassificationsAre(
11-
c.moduleName("M"), c.interfaceName("I"), c.interfaceName("X"), c.moduleName("M"), c.interfaceName("I"));
11+
c.moduleName("M", test.marker("0").position),
12+
c.interfaceName("I", test.marker("1").position),
13+
c.interfaceName("X", test.marker("2").position),
14+
c.moduleName("M", test.marker("3").position),
15+
c.interfaceName("I", test.marker("4").position));

‎tests/cases/fourslash/semanticClassification2.ts‎

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

3-
//// interface Thing {
3+
//// interface /*0*/Thing {
44
//// toExponential(): number;
55
//// }
66
////
77
//// var Thing = 0;
88
//// Thing.toExponential();
99

1010
var c = classification;
11-
verify.semanticClassificationsAre(c.interfaceName("Thing"));
11+
verify.semanticClassificationsAre(c.interfaceName("Thing", test.marker("0").position));

‎tests/cases/fourslash/semanticClassification3.ts‎

Lines changed: 0 additions & 12 deletions
This file was deleted.

‎tests/cases/fourslash/semanticClassificationInstantiatedModuleWithVariableOfSameName1.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/// <reference path="fourslash.ts"/>
2+
3+
////module /*0*/M {
4+
//// export interface /*1*/I {
5+
//// }
6+
//// var x = 10;
7+
////}
8+
////
9+
////var /*2*/M = {
10+
//// foo: 10,
11+
//// bar: 20
12+
////}
13+
////
14+
////var v: /*3*/M./*4*/I;
15+
////
16+
////var x = /*5*/M;
17+
18+
var c = classification;
19+
verify.semanticClassificationsAre(
20+
c.moduleName("M", test.marker("0").position),
21+
c.interfaceName("I", test.marker("1").position),
22+
c.moduleName("M", test.marker("3").position),
23+
c.interfaceName("I", test.marker("4").position),
24+
c.moduleName("M", test.marker("5").position));

‎tests/cases/fourslash/semanticClassificationInstantiatedModuleWithVariableOfSameName2.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/// <reference path="fourslash.ts"/>
2+
3+
////module /*0*/M {
4+
//// export interface /*1*/I {
5+
//// }
6+
////}
7+
////
8+
////module /*2*/M {
9+
//// var x = 10;
10+
////}
11+
////
12+
////var /*3*/M = {
13+
//// foo: 10,
14+
//// bar: 20
15+
////}
16+
////
17+
////var v: /*4*/M./*5*/I;
18+
////
19+
////var x = /*6*/M;
20+
21+
var c = classification;
22+
verify.semanticClassificationsAre(
23+
c.moduleName("M", test.marker("0").position),
24+
c.interfaceName("I", test.marker("1").position),
25+
c.moduleName("M", test.marker("2").position),
26+
c.moduleName("M", test.marker("4").position),
27+
c.interfaceName("I", test.marker("5").position),
28+
c.moduleName("M", test.marker("6").position));

‎tests/cases/fourslash/semanticClassificationModules.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/// <reference path="fourslash.ts"/>
2+
3+
////module /*0*/M {
4+
//// export var v;
5+
//// export interface /*1*/I {
6+
//// }
7+
////}
8+
////
9+
////var x: /*2*/M./*3*/I = /*4*/M.v;
10+
////var y = /*5*/M;
11+
12+
var c = classification;
13+
verify.semanticClassificationsAre(
14+
c.moduleName("M", test.marker("0").position),
15+
c.interfaceName("I", test.marker("1").position),
16+
c.moduleName("M", test.marker("2").position),
17+
c.interfaceName("I", test.marker("3").position),
18+
c.moduleName("M", test.marker("4").position),
19+
c.moduleName("M", test.marker("5").position));

‎tests/cases/fourslash/semanticClassificationUninstantiatedModuleWithVariableOfSameName1.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/// <reference path="fourslash.ts"/>
2+
3+
////declare module /*0*/M {
4+
//// interface /*1*/I {
5+
////
6+
//// }
7+
////}
8+
////
9+
////var M = { I: 10 };
10+
11+
var c = classification;
12+
verify.semanticClassificationsAre(
13+
c.moduleName("M", test.marker("0").position),
14+
c.interfaceName("I", test.marker("1").position));

0 commit comments

Comments
 (0)