Merge pull request #24329 from Microsoft/MergeMaster5-22-2

Merge master 5/22

Author: mhegazy

src/compiler/binder.ts

@@ -158,7 +158,7 @@ namespace ts {
          * If so, the node _must_ be in the current file (as that's the only way anything could have traversed to it to yield it as the error node)
          * This version of `createDiagnosticForNode` uses the binder's context to account for this, and always yields correct diagnostics even in these situations.
          */
-        function createDiagnosticForNode(node: Node, message: DiagnosticMessage, arg0?: string | number, arg1?: string | number, arg2?: string | number): Diagnostic {
+        function createDiagnosticForNode(node: Node, message: DiagnosticMessage, arg0?: string | number, arg1?: string | number, arg2?: string | number): DiagnosticWithLocation {
             return createDiagnosticForNodeInSourceFile(getSourceFileOfNode(node) || file, node, message, arg0, arg1, arg2);
         }
 
@@ -1208,7 +1208,7 @@ namespace ts {
             bind(node.statement);
             popActiveLabel();
             if (!activeLabel.referenced && !options.allowUnusedLabels) {
-                file.bindDiagnostics.push(createDiagnosticForNode(node.label, Diagnostics.Unused_label));
+                errorOrSuggestionOnFirstToken(unusedLabelIsError(options), node, Diagnostics.Unused_label);
             }
             if (!node.statement || node.statement.kind !== SyntaxKind.DoStatement) {
                 // do statement sets current flow inside bindDoStatement
@@ -1914,6 +1914,17 @@ namespace ts {
             file.bindDiagnostics.push(createFileDiagnostic(file, span.start, span.length, message, arg0, arg1, arg2));
         }
 
+        function errorOrSuggestionOnFirstToken(isError: boolean, node: Node, message: DiagnosticMessage, arg0?: any, arg1?: any, arg2?: any) {
+            const span = getSpanOfTokenAtPosition(file, node.pos);
+            const diag = createFileDiagnostic(file, span.start, span.length, message, arg0, arg1, arg2);
+            if (isError) {
+                file.bindDiagnostics.push(diag);
+            }
+            else {
+                file.bindSuggestionDiagnostics = append(file.bindSuggestionDiagnostics, { ...diag, category: DiagnosticCategory.Suggestion });
+            }
+        }
+
         function bind(node: Node): void {
             if (!node) {
                 return;
@@ -2730,26 +2741,26 @@ namespace ts {
                 if (reportError) {
                     currentFlow = reportedUnreachableFlow;
 
-                    // unreachable code is reported if
-                    // - user has explicitly asked about it AND
-                    // - statement is in not ambient context (statements in ambient context is already an error
-                    //   so we should not report extras) AND
-                    //   - node is not variable statement OR
-                    //   - node is block scoped variable statement OR
-                    //   - node is not block scoped variable statement and at least one variable declaration has initializer
-                    //   Rationale: we don't want to report errors on non-initialized var's since they are hoisted
-                    //   On the other side we do want to report errors on non-initialized 'lets' because of TDZ
-                    const reportUnreachableCode =
-                        !options.allowUnreachableCode &&
-                        !(node.flags & NodeFlags.Ambient) &&
-                        (
-                            node.kind !== SyntaxKind.VariableStatement ||
-                            getCombinedNodeFlags((<VariableStatement>node).declarationList) & NodeFlags.BlockScoped ||
-                            forEach((<VariableStatement>node).declarationList.declarations, d => d.initializer)
-                        );
-
-                    if (reportUnreachableCode) {
-                        errorOnFirstToken(node, Diagnostics.Unreachable_code_detected);
+                    if (!options.allowUnreachableCode) {
+                        // unreachable code is reported if
+                        // - user has explicitly asked about it AND
+                        // - statement is in not ambient context (statements in ambient context is already an error
+                        //   so we should not report extras) AND
+                        //   - node is not variable statement OR
+                        //   - node is block scoped variable statement OR
+                        //   - node is not block scoped variable statement and at least one variable declaration has initializer
+                        //   Rationale: we don't want to report errors on non-initialized var's since they are hoisted
+                        //   On the other side we do want to report errors on non-initialized 'lets' because of TDZ
+                        const isError =
+                            unreachableCodeIsError(options) &&
+                            !(node.flags & NodeFlags.Ambient) &&
+                            (
+                                !isVariableStatement(node) ||
+                                !!(getCombinedNodeFlags(node.declarationList) & NodeFlags.BlockScoped) ||
+                                node.declarationList.declarations.some(d => !!d.initializer)
+                            );
+
+                        errorOrSuggestionOnFirstToken(isError, node, Diagnostics.Unreachable_code_detected);
                     }
                 }
             }

src/compiler/checker.ts

@@ -313,11 +313,11 @@ namespace ts {
 
             getSuggestionDiagnostics: file => {
                 return (suggestionDiagnostics.get(file.fileName) || emptyArray).concat(getUnusedDiagnostics());
-                function getUnusedDiagnostics(): ReadonlyArray<Diagnostic> {
+                function getUnusedDiagnostics(): ReadonlyArray<DiagnosticWithLocation> {
                     if (file.isDeclarationFile) return emptyArray;
 
                     checkSourceFile(file);
-                    const diagnostics: Diagnostic[] = [];
+                    const diagnostics: DiagnosticWithLocation[] = [];
                     Debug.assert(!!(getNodeLinks(file).flags & NodeCheckFlags.TypeChecked));
                     checkUnusedIdentifiers(getPotentiallyUnusedIdentifiers(file), (kind, diag) => {
                         if (!unusedIsError(kind)) {
@@ -481,7 +481,7 @@ namespace ts {
 
         const diagnostics = createDiagnosticCollection();
         // Suggestion diagnostics must have a file. Keyed by source file name.
-        const suggestionDiagnostics = createMultiMap<Diagnostic>();
+        const suggestionDiagnostics = createMultiMap<DiagnosticWithLocation>();
 
         const enum TypeFacts {
             None = 0,
@@ -628,7 +628,7 @@ namespace ts {
             Local,
             Parameter,
         }
-        type AddUnusedDiagnostic = (type: UnusedKind, diagnostic: Diagnostic) => void;
+        type AddUnusedDiagnostic = (type: UnusedKind, diagnostic: DiagnosticWithLocation) => void;
 
         const builtinGlobals = createSymbolTable();
         builtinGlobals.set(undefinedSymbol.escapedName, undefinedSymbol);
@@ -824,7 +824,7 @@ namespace ts {
             diagnostics.add(diagnostic);
         }
 
-        function addErrorOrSuggestion(isError: boolean, diagnostic: Diagnostic) {
+        function addErrorOrSuggestion(isError: boolean, diagnostic: DiagnosticWithLocation) {
             if (isError) {
                 diagnostics.add(diagnostic);
             }
@@ -9286,8 +9286,10 @@ namespace ts {
             return type;
         }
 
-        function getRegularTypeOfLiteralType(type: Type) {
-            return type.flags & TypeFlags.StringOrNumberLiteral && type.flags & TypeFlags.FreshLiteral ? (<LiteralType>type).regularType : type;
+        function getRegularTypeOfLiteralType(type: Type): Type {
+            return type.flags & TypeFlags.StringOrNumberLiteral && type.flags & TypeFlags.FreshLiteral ? (<LiteralType>type).regularType :
+                type.flags & TypeFlags.Union ? getUnionType(sameMap((<UnionType>type).types, getRegularTypeOfLiteralType)) :
+                type;
         }
 
         function getLiteralType(value: string | number, enumId?: number, symbol?: Symbol) {
@@ -12774,10 +12776,12 @@ namespace ts {
             // all inferences were made to top-level occurrences of the type parameter, and
             // the type parameter has no constraint or its constraint includes no primitive or literal types, and
             // the type parameter was fixed during inference or does not occur at top-level in the return type.
-            const widenLiteralTypes = inference.topLevel &&
-                !hasPrimitiveConstraint(inference.typeParameter) &&
+            const primitiveConstraint = hasPrimitiveConstraint(inference.typeParameter);
+            const widenLiteralTypes = !primitiveConstraint && inference.topLevel &&
                 (inference.isFixed || !isTypeParameterAtTopLevel(getReturnTypeOfSignature(signature), inference.typeParameter));
-            const baseCandidates = widenLiteralTypes ? sameMap(candidates, getWidenedLiteralType) : candidates;
+            const baseCandidates = primitiveConstraint ? sameMap(candidates, getRegularTypeOfLiteralType) :
+                widenLiteralTypes ? sameMap(candidates, getWidenedLiteralType) :
+                candidates;
             // If all inferences were made from contravariant positions, infer a common subtype. Otherwise, if
             // union types were requested or if all inferences were made from the return type position, infer a
             // union type. Otherwise, infer a common supertype.
@@ -15303,6 +15307,26 @@ namespace ts {
             }
         }
 
+        // Return true if the given expression is possibly a discriminant value. We limit the kinds of
+        // expressions we check to those that don't depend on their contextual type in order not to cause
+        // recursive (and possibly infinite) invocations of getContextualType.
+        function isPossiblyDiscriminantValue(node: Expression): boolean {
+            switch (node.kind) {
+                case SyntaxKind.StringLiteral:
+                case SyntaxKind.NumericLiteral:
+                case SyntaxKind.NoSubstitutionTemplateLiteral:
+                case SyntaxKind.TrueKeyword:
+                case SyntaxKind.FalseKeyword:
+                case SyntaxKind.NullKeyword:
+                case SyntaxKind.Identifier:
+                    return true;
+                case SyntaxKind.PropertyAccessExpression:
+                case SyntaxKind.ParenthesizedExpression:
+                    return isPossiblyDiscriminantValue((<PropertyAccessExpression | ParenthesizedExpression>node).expression);
+            }
+            return false;
+        }
+
         // Return the contextual type for a given expression node. During overload resolution, a contextual type may temporarily
         // be "pushed" onto a node using the contextualType property.
         function getApparentTypeOfContextualType(node: Expression): Type {
@@ -15316,8 +15340,8 @@ namespace ts {
             propLoop: for (const prop of node.properties) {
                 if (!prop.symbol) continue;
                 if (prop.kind !== SyntaxKind.PropertyAssignment) continue;
-                if (isDiscriminantProperty(contextualType, prop.symbol.escapedName)) {
-                    const discriminatingType = getTypeOfNode(prop.initializer);
+                if (isPossiblyDiscriminantValue(prop.initializer) && isDiscriminantProperty(contextualType, prop.symbol.escapedName)) {
+                    const discriminatingType = checkExpression(prop.initializer);
                     for (const type of (contextualType as UnionType).types) {
                         const targetType = getTypeOfPropertyOfType(type, prop.symbol.escapedName);
                         if (targetType && checkTypeAssignableTo(discriminatingType, targetType, /*errorNode*/ undefined)) {

src/compiler/core.ts

@@ -14,8 +14,8 @@ namespace ts {
         return pathIsRelative(moduleName) || isRootedDiskPath(moduleName);
     }
 
-    export function sortAndDeduplicateDiagnostics(diagnostics: ReadonlyArray<Diagnostic>): Diagnostic[] {
-        return sortAndDeduplicate(diagnostics, compareDiagnostics);
+    export function sortAndDeduplicateDiagnostics<T extends Diagnostic>(diagnostics: ReadonlyArray<T>): T[] {
+        return sortAndDeduplicate<T>(diagnostics, compareDiagnostics);
     }
 }
 
@@ -1619,8 +1619,8 @@ namespace ts {
         return localizedDiagnosticMessages && localizedDiagnosticMessages[message.key] || message.message;
     }
 
-    export function createFileDiagnostic(file: SourceFile, start: number, length: number, message: DiagnosticMessage, ...args: (string | number)[]): Diagnostic;
-    export function createFileDiagnostic(file: SourceFile, start: number, length: number, message: DiagnosticMessage): Diagnostic {
+    export function createFileDiagnostic(file: SourceFile, start: number, length: number, message: DiagnosticMessage, ...args: (string | number)[]): DiagnosticWithLocation;
+    export function createFileDiagnostic(file: SourceFile, start: number, length: number, message: DiagnosticMessage): DiagnosticWithLocation {
         Debug.assertGreaterThanOrEqual(start, 0);
         Debug.assertGreaterThanOrEqual(length, 0);
 
@@ -1991,6 +1991,14 @@ namespace ts {
         return moduleResolution;
     }
 
+    export function unreachableCodeIsError(options: CompilerOptions): boolean {
+        return options.allowUnreachableCode === false;
+    }
+
+    export function unusedLabelIsError(options: CompilerOptions): boolean {
+        return options.allowUnusedLabels === false;
+    }
+
     export function getAreDeclarationMapsEnabled(options: CompilerOptions) {
         return !!(options.declaration && options.declarationMap);
     }

src/compiler/diagnosticMessages.json

@@ -2238,7 +2238,8 @@
     },
     "Left side of comma operator is unused and has no side effects.": {
         "category": "Error",
-        "code": 2695
+        "code": 2695,
+        "reportsUnnecessary": true
     },
     "The 'Object' type is assignable to very few other types. Did you mean to use the 'any' type instead?": {
         "category": "Error",
@@ -3673,7 +3674,8 @@
     },
     "Unreachable code detected.": {
         "category": "Error",
-        "code": 7027
+        "code": 7027,
+        "reportsUnnecessary": true
     },
     "Unused label.": {
         "category": "Error",

src/compiler/factory.ts

@@ -2463,6 +2463,7 @@ namespace ts {
             if (node.symbolCount !== undefined) updated.symbolCount = node.symbolCount;
             if (node.parseDiagnostics !== undefined) updated.parseDiagnostics = node.parseDiagnostics;
             if (node.bindDiagnostics !== undefined) updated.bindDiagnostics = node.bindDiagnostics;
+            if (node.bindSuggestionDiagnostics !== undefined) updated.bindSuggestionDiagnostics = node.bindSuggestionDiagnostics;
             if (node.lineMap !== undefined) updated.lineMap = node.lineMap;
             if (node.classifiableNames !== undefined) updated.classifiableNames = node.classifiableNames;
             if (node.resolvedModules !== undefined) updated.resolvedModules = node.resolvedModules;

src/compiler/parser.ts

@@ -595,7 +595,7 @@ namespace ts {
         // tslint:enable variable-name
 
         let sourceFile: SourceFile;
-        let parseDiagnostics: Diagnostic[];
+        let parseDiagnostics: DiagnosticWithLocation[];
         let syntaxCursor: IncrementalParser.SyntaxCursor;
 
         let currentToken: SyntaxKind;
@@ -912,6 +912,7 @@ namespace ts {
 
             sourceFile.text = sourceText;
             sourceFile.bindDiagnostics = [];
+            sourceFile.bindSuggestionDiagnostics = undefined;
             sourceFile.languageVersion = languageVersion;
             sourceFile.fileName = normalizePath(fileName);
             sourceFile.languageVariant = getLanguageVariant(scriptKind);