Add kindModifier to mark if a completion item is for an optional field (microsoft#20839)

mjbvz · mhegazy · commit 12d5063e4ce3 · 2018-01-04T16:36:31.000-08:00
* Add kindModifier to mark if a completion item is for an optional field For microsoft#12458 Adds a new KindModifier for completion items to mark when a property is optional. This can be used by editors to either change the item icon or add a `?` to the item text * Add method test * Baseline-accept
diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts
@@ -872,7 +872,7 @@ namespace FourSlash {
             });
         }
 
-        public verifyCompletionListContains(entryId: ts.Completions.CompletionEntryIdentifier, text?: string, documentation?: string, kind?: string, spanIndex?: number, hasAction?: boolean, options?: FourSlashInterface.VerifyCompletionListContainsOptions) {
+        public verifyCompletionListContains(entryId: ts.Completions.CompletionEntryIdentifier, text?: string, documentation?: string, kind?: string | { kind?: string, kindModifiers?: string }, spanIndex?: number, hasAction?: boolean, options?: FourSlashInterface.VerifyCompletionListContainsOptions) {
             const completions = this.getCompletionListAtCaret(options);
             if (completions) {
                 this.assertItemInCompletionList(completions.entries, entryId, text, documentation, kind, spanIndex, hasAction, options);
@@ -893,7 +893,7 @@ namespace FourSlash {
          * @param expectedKind the kind of symbol (see ScriptElementKind)
          * @param spanIndex the index of the range that the completion item's replacement text span should match
          */
-        public verifyCompletionListDoesNotContain(entryId: ts.Completions.CompletionEntryIdentifier, expectedText?: string, expectedDocumentation?: string, expectedKind?: string, spanIndex?: number, options?: FourSlashInterface.CompletionsAtOptions) {
+        public verifyCompletionListDoesNotContain(entryId: ts.Completions.CompletionEntryIdentifier, expectedText?: string, expectedDocumentation?: string, expectedKind?: string | { kind?: string, kindModifiers?: string }, spanIndex?: number, options?: FourSlashInterface.CompletionsAtOptions) {
             let replacementSpan: ts.TextSpan;
             if (spanIndex !== undefined) {
                 replacementSpan = this.getTextSpanForRangeAtIndex(spanIndex);
@@ -902,7 +902,7 @@ namespace FourSlash {
             const completions = this.getCompletionListAtCaret(options);
             if (completions) {
                 let filterCompletions = completions.entries.filter(e => e.name === entryId.name && e.source === entryId.source);
-                filterCompletions = expectedKind ? filterCompletions.filter(e => e.kind === expectedKind) : filterCompletions;
+                filterCompletions = expectedKind ? filterCompletions.filter(e => e.kind === expectedKind || (typeof expectedKind === "object" && e.kind === expectedKind.kind)) : filterCompletions;
                 filterCompletions = filterCompletions.filter(entry => {
                     const details = this.getCompletionEntryDetails(entry.name);
                     const documentation = details && ts.displayPartsToString(details.documentation);
@@ -3089,7 +3089,7 @@ Actual: ${stringify(fullActual)}`);
             entryId: ts.Completions.CompletionEntryIdentifier,
             text: string | undefined,
             documentation: string | undefined,
-            kind: string | undefined,
+            kind: string | undefined | { kind?: string, kindModifiers?: string },
             spanIndex: number | undefined,
             hasAction: boolean | undefined,
             options: FourSlashInterface.VerifyCompletionListContainsOptions | undefined,
@@ -3123,9 +3123,21 @@ Actual: ${stringify(fullActual)}`);
             }
 
             if (kind !== undefined) {
-                assert.equal(item.kind, kind, this.assertionMessageAtLastKnownMarker("completion item kind for " + entryId));
+                if (typeof kind === "string") {
+                    assert.equal(item.kind, kind, this.assertionMessageAtLastKnownMarker("completion item kind for " + entryId));
+                }
+                else {
+                    if (kind.kind) {
+                        assert.equal(item.kind, kind.kind, this.assertionMessageAtLastKnownMarker("completion item kind for " + entryId));
+                    }
+                    if (kind.kindModifiers !== undefined) {
+                        assert.equal(item.kindModifiers, kind.kindModifiers, this.assertionMessageAtLastKnownMarker("completion item kindModifiers for " + entryId));
+                    }
+                }
             }
 
+
+
             if (spanIndex !== undefined) {
                 const span = this.getTextSpanForRangeAtIndex(spanIndex);
                 assert.isTrue(TestState.textSpansEqual(span, item.replacementSpan), this.assertionMessageAtLastKnownMarker(stringify(span) + " does not equal " + stringify(item.replacementSpan) + " replacement span for " + entryId));
@@ -3842,7 +3854,7 @@ namespace FourSlashInterface {
 
         // Verifies the completion list contains the specified symbol. The
         // completion list is brought up if necessary
-        public completionListContains(entryId: string | ts.Completions.CompletionEntryIdentifier, text?: string, documentation?: string, kind?: string, spanIndex?: number, hasAction?: boolean, options?: VerifyCompletionListContainsOptions) {
+        public completionListContains(entryId: string | ts.Completions.CompletionEntryIdentifier, text?: string, documentation?: string, kind?: string | { kind?: string, kindModifiers?: string }, spanIndex?: number, hasAction?: boolean, options?: VerifyCompletionListContainsOptions) {
             if (typeof entryId === "string") {
                 entryId = { name: entryId, source: undefined };
             }
diff --git a/src/services/symbolDisplay.ts b/src/services/symbolDisplay.ts
@@ -91,9 +91,14 @@ namespace ts.SymbolDisplay {
     }
 
     export function getSymbolModifiers(symbol: Symbol): string {
-        return symbol && symbol.declarations && symbol.declarations.length > 0
+        const nodeModifiers = symbol && symbol.declarations && symbol.declarations.length > 0
             ? getNodeModifiers(symbol.declarations[0])
             : ScriptElementKindModifier.none;
+
+        const symbolModifiers = symbol && symbol.flags & SymbolFlags.Optional ?
+            ScriptElementKindModifier.optionalModifier
+            : ScriptElementKindModifier.none;
+        return nodeModifiers && symbolModifiers ? nodeModifiers + "," + symbolModifiers : nodeModifiers || symbolModifiers;
     }
 
     interface SymbolDisplayPartsDocumentationAndSymbolKind {
diff --git a/src/services/types.ts b/src/services/types.ts
@@ -954,6 +954,7 @@ namespace ts {
         ambientModifier = "declare",
         staticModifier = "static",
         abstractModifier = "abstract",
+        optionalModifier = "optional"
     }
 
     export const enum ClassificationTypeNames {
diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts
@@ -4505,6 +4505,7 @@ declare namespace ts {
         ambientModifier = "declare",
         staticModifier = "static",
         abstractModifier = "abstract",
+        optionalModifier = "optional",
     }
     enum ClassificationTypeNames {
         comment = "comment",
diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts
@@ -4505,6 +4505,7 @@ declare namespace ts {
         ambientModifier = "declare",
         staticModifier = "static",
         abstractModifier = "abstract",
+        optionalModifier = "optional",
     }
     enum ClassificationTypeNames {
         comment = "comment",
diff --git a/tests/cases/fourslash/completionsOptionalKindModifier.ts b/tests/cases/fourslash/completionsOptionalKindModifier.ts
@@ -0,0 +1,10 @@
+/// <reference path="fourslash.ts" />
+
+////interface A { a?: number; method?(): number; };
+////function f(x: A) {
+////x./*a*/;
+////}
+
+goTo.marker("a");
+verify.completionListContains("a", /* text */ undefined, /* documentation */ undefined, { kindModifiers: "optional" });
+verify.completionListContains("method", /* text */ undefined, /* documentation */ undefined, { kindModifiers: "optional" });
diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts
@@ -146,7 +146,7 @@ declare namespace FourSlashInterface {
             entryId: string | { name: string, source?: string },
             text?: string,
             documentation?: string,
-            kind?: string,
+            kind?: string | { kind?: string, kindModifiers?: string },
             spanIndex?: number,
             hasAction?: boolean,
             options?: { includeExternalModuleExports?: boolean, sourceDisplay?: string, isRecommended?: true },

Original file line number	Diff line number	Diff line change
`@@ -91,9 +91,14 @@ namespace ts.SymbolDisplay {`
`91`	`91`	`}`
`92`	`92`
`93`	`93`	`export function getSymbolModifiers(symbol: Symbol): string {`
`94`		`- return symbol && symbol.declarations && symbol.declarations.length > 0`
	`94`	`+ const nodeModifiers = symbol && symbol.declarations && symbol.declarations.length > 0`
`95`	`95`	`? getNodeModifiers(symbol.declarations[0])`
`96`	`96`	`: ScriptElementKindModifier.none;`
	`97`	`+`
	`98`	`+ const symbolModifiers = symbol && symbol.flags & SymbolFlags.Optional ?`
	`99`	`+ ScriptElementKindModifier.optionalModifier`
	`100`	`+ : ScriptElementKindModifier.none;`
	`101`	`+ return nodeModifiers && symbolModifiers ? nodeModifiers + "," + symbolModifiers : nodeModifiers \|\| symbolModifiers;`
`97`	`102`	`}`
`98`	`103`
`99`	`104`	`interface SymbolDisplayPartsDocumentationAndSymbolKind {`
Original file line number	Diff line number	Diff line change
`@@ -954,6 +954,7 @@ namespace ts {`
`954`	`954`	`ambientModifier = "declare",`
`955`	`955`	`staticModifier = "static",`
`956`	`956`	`abstractModifier = "abstract",`
	`957`	`+ optionalModifier = "optional"`
`957`	`958`	`}`
`958`	`959`
`959`	`960`	`export const enum ClassificationTypeNames {`
Original file line number	Diff line number	Diff line change
`@@ -4505,6 +4505,7 @@ declare namespace ts {`
`4505`	`4505`	`ambientModifier = "declare",`
`4506`	`4506`	`staticModifier = "static",`
`4507`	`4507`	`abstractModifier = "abstract",`
	`4508`	`+ optionalModifier = "optional",`
`4508`	`4509`	`}`
`4509`	`4510`	`enum ClassificationTypeNames {`
`4510`	`4511`	`comment = "comment",`