Merge branch 'master' into getJSDocParameterTags_defined

andy-hanson · andy-hanson · commit e5e488810746 · 2018-03-13T17:11:48.000-07:00
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -422,6 +422,7 @@ namespace ts {
 
         let deferredNodes: Node[];
         let deferredUnusedIdentifierNodes: Node[];
+        const seenDeferredUnusedIdentifiers = createMap<true>(); // For assertion that we don't defer the same identifier twice
 
         let flowLoopStart = 0;
         let flowLoopCount = 0;
@@ -4166,7 +4167,17 @@ namespace ts {
                 return getTypeForBindingElement(<BindingElement>declaration);
             }
 
-            const isOptional = !isBindingElement(declaration) && !isVariableDeclaration(declaration) && !!declaration.questionToken && includeOptionality;
+            let isOptional = false;
+            if (includeOptionality) {
+                if (isInJavaScriptFile(declaration) && isParameter(declaration)) {
+                    const parameterTags = getJSDocParameterTags(declaration);
+                    isOptional = !!(parameterTags && parameterTags.length > 0 && find(parameterTags, tag => tag.isBracketed));
+                }
+                if (!isBindingElement(declaration) && !isVariableDeclaration(declaration) && !!declaration.questionToken) {
+                    isOptional = true;
+                }
+            }
+
             // Use type from type annotation if one is present
             const declaredType = tryGetTypeFromEffectiveTypeNode(declaration);
             if (declaredType) {
@@ -8656,9 +8667,10 @@ namespace ts {
                     return getTypeFromIntersectionTypeNode(<IntersectionTypeNode>node);
                 case SyntaxKind.JSDocNullableType:
                     return getTypeFromJSDocNullableTypeNode(<JSDocNullableType>node);
+                case SyntaxKind.JSDocOptionalType:
+                    return addOptionality(getTypeFromTypeNode((node as JSDocOptionalType).type));
                 case SyntaxKind.ParenthesizedType:
                 case SyntaxKind.JSDocNonNullableType:
-                case SyntaxKind.JSDocOptionalType:
                 case SyntaxKind.JSDocTypeExpression:
                     return getTypeFromTypeNode((<ParenthesizedTypeNode | JSDocTypeReferencingNode | JSDocTypeExpression>node).type);
                 case SyntaxKind.JSDocVariadicType:
@@ -18661,7 +18673,6 @@ namespace ts {
 
             // The identityMapper object is used to indicate that function expressions are wildcards
             if (checkMode === CheckMode.SkipContextSensitive && isContextSensitive(node)) {
-                checkNodeDeferred(node);
                 return anyFunctionType;
             }
 
@@ -21551,6 +21562,7 @@ namespace ts {
 
         function registerForUnusedIdentifiersCheck(node: Node) {
             if (deferredUnusedIdentifierNodes) {
+                Debug.assert(addToSeen(seenDeferredUnusedIdentifiers, getNodeId(node)), "Deferring unused identifier check twice");
                 deferredUnusedIdentifierNodes.push(node);
             }
         }
@@ -24560,6 +24572,7 @@ namespace ts {
                 }
 
                 deferredNodes = undefined;
+                seenDeferredUnusedIdentifiers.clear();
                 deferredUnusedIdentifierNodes = undefined;
 
                 if (isExternalOrCommonJsModule(node)) {
diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -3839,6 +3839,16 @@ namespace ts {
     export function showModuleSpecifier({ moduleSpecifier }: ImportDeclaration): string {
         return isStringLiteral(moduleSpecifier) ? moduleSpecifier.text : getTextOfNode(moduleSpecifier);
     }
+
+    /** Add a value to a set, and return true if it wasn't already present. */
+    export function addToSeen(seen: Map<true>, key: string | number): boolean {
+        key = String(key);
+        if (seen.has(key)) {
+            return false;
+        }
+        seen.set(key, true);
+        return true;
+    }
 }
 
 namespace ts {
diff --git a/src/harness/virtualFileSystemWithWatch.ts b/src/harness/virtualFileSystemWithWatch.ts
@@ -276,12 +276,14 @@ interface Array<T> {}`
         DynamicPolling = "RecursiveDirectoryUsingDynamicPriorityPolling"
     }
 
+    const timeIncrements = 1000;
     export class TestServerHost implements server.ServerHost, FormatDiagnosticsHost, ModuleResolutionHost {
         args: string[] = [];
 
         private readonly output: string[] = [];
 
         private fs: Map<FSEntry> = createMap<FSEntry>();
+        private time = timeIncrements;
         getCanonicalFileName: (s: string) => string;
         private toPath: (f: string) => Path;
         private timeoutCallbacks = new Callbacks();
@@ -355,6 +357,11 @@ interface Array<T> {}`
             return s;
         }
 
+        private now() {
+            this.time += timeIncrements;
+            return new Date(this.time);
+        }
+
         reloadFS(fileOrFolderList: ReadonlyArray<FileOrFolder>, options?: Partial<ReloadWatchInvokeOptions>) {
             const mapNewLeaves = createMap<true>();
             const isNewFs = this.fs.size === 0;
@@ -381,8 +388,8 @@ interface Array<T> {}`
                                 }
                                 else {
                                     currentEntry.content = fileOrDirectory.content;
-                                    currentEntry.modifiedTime = new Date();
-                                    this.fs.get(getDirectoryPath(currentEntry.path)).modifiedTime = new Date();
+                                    currentEntry.modifiedTime = this.now();
+                                    this.fs.get(getDirectoryPath(currentEntry.path)).modifiedTime = this.now();
                                     if (options && options.invokeDirectoryWatcherInsteadOfFileChanged) {
                                         this.invokeDirectoryWatcher(getDirectoryPath(currentEntry.fullPath), currentEntry.fullPath);
                                     }
@@ -406,7 +413,7 @@ interface Array<T> {}`
                         }
                         else {
                             // Folder update: Nothing to do.
-                            currentEntry.modifiedTime = new Date();
+                            currentEntry.modifiedTime = this.now();
                         }
                     }
                 }
@@ -505,7 +512,7 @@ interface Array<T> {}`
 
         private addFileOrFolderInFolder(folder: Folder, fileOrDirectory: File | Folder | SymLink, ignoreWatch?: boolean) {
             insertSorted(folder.entries, fileOrDirectory, (a, b) => compareStringsCaseSensitive(getBaseFileName(a.path), getBaseFileName(b.path)));
-            folder.modifiedTime = new Date();
+            folder.modifiedTime = this.now();
             this.fs.set(fileOrDirectory.path, fileOrDirectory);
 
             if (ignoreWatch) {
@@ -520,7 +527,7 @@ interface Array<T> {}`
             const baseFolder = this.fs.get(basePath) as Folder;
             if (basePath !== fileOrDirectory.path) {
                 Debug.assert(!!baseFolder);
-                baseFolder.modifiedTime = new Date();
+                baseFolder.modifiedTime = this.now();
                 filterMutate(baseFolder.entries, entry => entry !== fileOrDirectory);
             }
             this.fs.delete(fileOrDirectory.path);
@@ -587,7 +594,7 @@ interface Array<T> {}`
             return {
                 path: this.toPath(fullPath),
                 fullPath,
-                modifiedTime: new Date()
+                modifiedTime: this.now()
             };
         }
 
diff --git a/src/services/utilities.ts b/src/services/utilities.ts
@@ -1205,16 +1205,6 @@ namespace ts {
         };
     }
 
-    /** Add a value to a set, and return true if it wasn't already present. */
-    export function addToSeen(seen: Map<true>, key: string | number): boolean {
-        key = String(key);
-        if (seen.has(key)) {
-            return false;
-        }
-        seen.set(key, true);
-        return true;
-    }
-
     export function getSnapshotText(snap: IScriptSnapshot): string {
         return snap.getText(0, snap.getLength());
     }
diff --git a/tests/baselines/reference/jsdocParamTagTypeLiteral.types b/tests/baselines/reference/jsdocParamTagTypeLiteral.types
@@ -23,18 +23,18 @@ normal(12);
  * @param {string} [opts1.w="hi"] doc5
  */
 function foo1(opts1) {
->foo1 : (opts1: { x: string; y?: string; z?: string; w?: string; }) => void
->opts1 : { x: string; y?: string; z?: string; w?: string; }
+>foo1 : (opts1: { x: string; y?: string | undefined; z?: string; w?: string; }) => void
+>opts1 : { x: string; y?: string | undefined; z?: string; w?: string; }
 
     opts1.x;
 >opts1.x : string
->opts1 : { x: string; y?: string; z?: string; w?: string; }
+>opts1 : { x: string; y?: string | undefined; z?: string; w?: string; }
 >x : string
 }
 
 foo1({x: 'abc'});
 >foo1({x: 'abc'}) : void
->foo1 : (opts1: { x: string; y?: string; z?: string; w?: string; }) => void
+>foo1 : (opts1: { x: string; y?: string | undefined; z?: string; w?: string; }) => void
 >{x: 'abc'} : { x: string; }
 >x : string
 >'abc' : "abc"
@@ -45,20 +45,20 @@ foo1({x: 'abc'});
  * @param {string=} opts2[].anotherY
  */
 function foo2(/** @param opts2 bad idea theatre! */opts2) {
->foo2 : (opts2: { anotherX: string; anotherY?: string; }[]) => void
->opts2 : { anotherX: string; anotherY?: string; }[]
+>foo2 : (opts2: { anotherX: string; anotherY?: string | undefined; }[]) => void
+>opts2 : { anotherX: string; anotherY?: string | undefined; }[]
 
     opts2[0].anotherX;
 >opts2[0].anotherX : string
->opts2[0] : { anotherX: string; anotherY?: string; }
->opts2 : { anotherX: string; anotherY?: string; }[]
+>opts2[0] : { anotherX: string; anotherY?: string | undefined; }
+>opts2 : { anotherX: string; anotherY?: string | undefined; }[]
 >0 : 0
 >anotherX : string
 }
 
 foo2([{anotherX: "world"}]);
 >foo2([{anotherX: "world"}]) : void
->foo2 : (opts2: { anotherX: string; anotherY?: string; }[]) => void
+>foo2 : (opts2: { anotherX: string; anotherY?: string | undefined; }[]) => void
 >[{anotherX: "world"}] : { anotherX: string; }[]
 >{anotherX: "world"} : { anotherX: string; }
 >anotherX : string
@@ -92,20 +92,20 @@ foo3({x: 'abc'});
  * @param {string} [opts4[].w="hi"]
  */
 function foo4(opts4) {
->foo4 : (opts4: { x: string; y?: string; z?: string; w?: string; }[]) => void
->opts4 : { x: string; y?: string; z?: string; w?: string; }[]
+>foo4 : (opts4: { x: string; y?: string | undefined; z?: string; w?: string; }[]) => void
+>opts4 : { x: string; y?: string | undefined; z?: string; w?: string; }[]
 
     opts4[0].x;
 >opts4[0].x : string
->opts4[0] : { x: string; y?: string; z?: string; w?: string; }
->opts4 : { x: string; y?: string; z?: string; w?: string; }[]
+>opts4[0] : { x: string; y?: string | undefined; z?: string; w?: string; }
+>opts4 : { x: string; y?: string | undefined; z?: string; w?: string; }[]
 >0 : 0
 >x : string
 }
 
 foo4([{ x: 'hi' }]);
 >foo4([{ x: 'hi' }]) : void
->foo4 : (opts4: { x: string; y?: string; z?: string; w?: string; }[]) => void
+>foo4 : (opts4: { x: string; y?: string | undefined; z?: string; w?: string; }[]) => void
 >[{ x: 'hi' }] : { x: string; }[]
 >{ x: 'hi' } : { x: string; }
 >x : string
diff --git a/tests/baselines/reference/paramTagBracketsAddOptionalUndefined.symbols b/tests/baselines/reference/paramTagBracketsAddOptionalUndefined.symbols
@@ -0,0 +1,38 @@
+=== tests/cases/conformance/jsdoc/a.js ===
+/**
+ * @param {number} [p]
+ * @param {number=} q
+ * @param {number} [r=101]
+ */
+function f(p, q, r) {
+>f : Symbol(f, Decl(a.js, 0, 0))
+>p : Symbol(p, Decl(a.js, 5, 11))
+>q : Symbol(q, Decl(a.js, 5, 13))
+>r : Symbol(r, Decl(a.js, 5, 16))
+
+    p = undefined
+>p : Symbol(p, Decl(a.js, 5, 11))
+>undefined : Symbol(undefined)
+
+    q = undefined
+>q : Symbol(q, Decl(a.js, 5, 13))
+>undefined : Symbol(undefined)
+
+    // note that, unlike TS, JSDOC [r=101] retains | undefined because
+    // there's no code emitted to get rid of it.
+    r = undefined
+>r : Symbol(r, Decl(a.js, 5, 16))
+>undefined : Symbol(undefined)
+}
+f()
+>f : Symbol(f, Decl(a.js, 0, 0))
+
+f(undefined, undefined, undefined)
+>f : Symbol(f, Decl(a.js, 0, 0))
+>undefined : Symbol(undefined)
+>undefined : Symbol(undefined)
+>undefined : Symbol(undefined)
+
+f(1, 2, 3)
+>f : Symbol(f, Decl(a.js, 0, 0))
+
diff --git a/tests/baselines/reference/paramTagBracketsAddOptionalUndefined.types b/tests/baselines/reference/paramTagBracketsAddOptionalUndefined.types
@@ -0,0 +1,47 @@
+=== tests/cases/conformance/jsdoc/a.js ===
+/**
+ * @param {number} [p]
+ * @param {number=} q
+ * @param {number} [r=101]
+ */
+function f(p, q, r) {
+>f : (p?: number | undefined, q?: number | undefined, r?: number | undefined) => void
+>p : number | undefined
+>q : number | undefined
+>r : number | undefined
+
+    p = undefined
+>p = undefined : undefined
+>p : number | undefined
+>undefined : undefined
+
+    q = undefined
+>q = undefined : undefined
+>q : number | undefined
+>undefined : undefined
+
+    // note that, unlike TS, JSDOC [r=101] retains | undefined because
+    // there's no code emitted to get rid of it.
+    r = undefined
+>r = undefined : undefined
+>r : number | undefined
+>undefined : undefined
+}
+f()
+>f() : void
+>f : (p?: number | undefined, q?: number | undefined, r?: number | undefined) => void
+
+f(undefined, undefined, undefined)
+>f(undefined, undefined, undefined) : void
+>f : (p?: number | undefined, q?: number | undefined, r?: number | undefined) => void
+>undefined : undefined
+>undefined : undefined
+>undefined : undefined
+
+f(1, 2, 3)
+>f(1, 2, 3) : void
+>f : (p?: number | undefined, q?: number | undefined, r?: number | undefined) => void
+>1 : 1
+>2 : 2
+>3 : 3
+
diff --git a/tests/cases/conformance/jsdoc/paramTagBracketsAddOptionalUndefined.ts b/tests/cases/conformance/jsdoc/paramTagBracketsAddOptionalUndefined.ts
@@ -0,0 +1,21 @@
+// @noEmit: true
+// @allowJs: true
+// @checkJs: true
+// @strict: true
+// @Filename: a.js
+
+/**
+ * @param {number} [p]
+ * @param {number=} q
+ * @param {number} [r=101]
+ */
+function f(p, q, r) {
+    p = undefined
+    q = undefined
+    // note that, unlike TS, JSDOC [r=101] retains | undefined because
+    // there's no code emitted to get rid of it.
+    r = undefined
+}
+f()
+f(undefined, undefined, undefined)
+f(1, 2, 3)
