Have Symbol#isReferenced check the SymbolFlags of the reference (#21996)

Andy · web-flow · commit 9ee51fadd9d1 · 2018-02-16T16:47:13.000-08:00
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -1394,7 +1394,7 @@ namespace ts {
             // If `result === lastSelfReferenceLocation.symbol`, that means that we are somewhere inside `lastSelfReferenceLocation` looking up a name, and resolving to `lastLocation` itself.
             // That means that this is a self-reference of `lastLocation`, and shouldn't count this when considering whether `lastLocation` is used.
             if (isUse && result && nameNotFoundMessage && noUnusedIdentifiers && (!lastSelfReferenceLocation || result !== lastSelfReferenceLocation.symbol)) {
-                result.isReferenced = true;
+                result.isReferenced |= meaning;
             }
 
             if (!result) {
@@ -15697,7 +15697,7 @@ namespace ts {
             if (reactSym) {
                 // Mark local symbol as referenced here because it might not have been marked
                 // if jsx emit was not react as there wont be error being emitted
-                reactSym.isReferenced = true;
+                reactSym.isReferenced = SymbolFlags.All;
 
                 // If react symbol is alias, mark it as refereced
                 if (reactSym.flags & SymbolFlags.Alias && !isConstEnumOrConstEnumOnlyModule(resolveAlias(reactSym))) {
@@ -16267,12 +16267,7 @@ namespace ts {
                 }
             }
 
-            if (getCheckFlags(prop) & CheckFlags.Instantiated) {
-                getSymbolLinks(prop).target.isReferenced = true;
-            }
-            else {
-                prop.isReferenced = true;
-            }
+            (getCheckFlags(prop) & CheckFlags.Instantiated ? getSymbolLinks(prop).target : prop).isReferenced = SymbolFlags.All;
         }
 
         function isValidPropertyAccess(node: PropertyAccessExpression | QualifiedName, propertyName: __String): boolean {
@@ -21442,7 +21437,9 @@ namespace ts {
         function checkUnusedLocalsAndParameters(node: Node): void {
             if (noUnusedIdentifiers && !(node.flags & NodeFlags.Ambient)) {
                 node.locals.forEach(local => {
-                    if (!local.isReferenced) {
+                    // If it's purely a type parameter, ignore, will be checked in `checkUnusedTypeParameters`.
+                    // If it's a type parameter merged with a parameter, check if the parameter-side is used.
+                    if (local.flags & SymbolFlags.TypeParameter ? (local.flags & SymbolFlags.Variable && !(local.isReferenced & SymbolFlags.Variable)) : !local.isReferenced) {
                         if (local.valueDeclaration && getRootDeclaration(local.valueDeclaration).kind === SyntaxKind.Parameter) {
                             const parameter = <ParameterDeclaration>getRootDeclaration(local.valueDeclaration);
                             const name = getNameOfDeclaration(local.valueDeclaration);
@@ -21453,7 +21450,7 @@ namespace ts {
                                 error(name, Diagnostics._0_is_declared_but_its_value_is_never_read, symbolName(local));
                             }
                         }
-                        else if (local.flags & SymbolFlags.TypeParameter ? compilerOptions.noUnusedParameters : compilerOptions.noUnusedLocals) {
+                        else if (compilerOptions.noUnusedLocals) {
                             forEach(local.declarations, d => errorUnusedLocal(d, symbolName(local)));
                         }
                     }
@@ -21538,7 +21535,7 @@ namespace ts {
                         return;
                     }
                     for (const typeParameter of node.typeParameters) {
-                        if (!getMergedSymbol(typeParameter.symbol).isReferenced && !isIdentifierThatStartsWithUnderScore(typeParameter.name)) {
+                        if (!(getMergedSymbol(typeParameter.symbol).isReferenced & SymbolFlags.TypeParameter) && !isIdentifierThatStartsWithUnderScore(typeParameter.name)) {
                             error(typeParameter.name, Diagnostics._0_is_declared_but_its_value_is_never_read, symbolName(typeParameter.symbol));
                         }
                     }
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -3307,7 +3307,7 @@ namespace ts {
         /* @internal */ parent?: Symbol;        // Parent symbol
         /* @internal */ exportSymbol?: Symbol;  // Exported symbol associated with this symbol
         /* @internal */ constEnumOnlyModule?: boolean; // True if module contains only const enums or other modules with only const enums
-        /* @internal */ isReferenced?: boolean; // True if the symbol is referenced elsewhere
+        /* @internal */ isReferenced?: SymbolFlags; // True if the symbol is referenced elsewhere. Keeps track of the meaning of a reference in case a symbol is both a type parameter and parameter.
         /* @internal */ isReplaceableByMethod?: boolean; // Can this Javascript class property be replaced by a method symbol?
         /* @internal */ isAssigned?: boolean;   // True if the symbol is a parameter with assignments
     }
diff --git a/tests/baselines/reference/noUnusedLocals_typeParameterMergedWithParameter.errors.txt b/tests/baselines/reference/noUnusedLocals_typeParameterMergedWithParameter.errors.txt
@@ -0,0 +1,27 @@
+tests/cases/compiler/noUnusedLocals_typeParameterMergedWithParameter.ts(1,18): error TS6133: 'T' is declared but its value is never read.
+tests/cases/compiler/noUnusedLocals_typeParameterMergedWithParameter.ts(1,21): error TS6133: 'T' is declared but its value is never read.
+tests/cases/compiler/noUnusedLocals_typeParameterMergedWithParameter.ts(3,19): error TS6133: 'T' is declared but its value is never read.
+tests/cases/compiler/noUnusedLocals_typeParameterMergedWithParameter.ts(7,26): error TS6133: 'T' is declared but its value is never read.
+
+
+==== tests/cases/compiler/noUnusedLocals_typeParameterMergedWithParameter.ts (4 errors) ====
+    function useNone<T>(T: number) {}
+                     ~
+!!! error TS6133: 'T' is declared but its value is never read.
+                        ~
+!!! error TS6133: 'T' is declared but its value is never read.
+    
+    function useParam<T>(T: number) {
+                      ~
+!!! error TS6133: 'T' is declared but its value is never read.
+        return T;
+    }
+    
+    function useTypeParam<T>(T: T) {}
+                             ~
+!!! error TS6133: 'T' is declared but its value is never read.
+    
+    function useBoth<T>(T: T) {
+        return T;
+    }
+    
diff --git a/tests/baselines/reference/noUnusedLocals_typeParameterMergedWithParameter.js b/tests/baselines/reference/noUnusedLocals_typeParameterMergedWithParameter.js
@@ -0,0 +1,23 @@
+//// [noUnusedLocals_typeParameterMergedWithParameter.ts]
+function useNone<T>(T: number) {}
+
+function useParam<T>(T: number) {
+    return T;
+}
+
+function useTypeParam<T>(T: T) {}
+
+function useBoth<T>(T: T) {
+    return T;
+}
+
+
+//// [noUnusedLocals_typeParameterMergedWithParameter.js]
+function useNone(T) { }
+function useParam(T) {
+    return T;
+}
+function useTypeParam(T) { }
+function useBoth(T) {
+    return T;
+}
diff --git a/tests/baselines/reference/noUnusedLocals_typeParameterMergedWithParameter.symbols b/tests/baselines/reference/noUnusedLocals_typeParameterMergedWithParameter.symbols
@@ -0,0 +1,31 @@
+=== tests/cases/compiler/noUnusedLocals_typeParameterMergedWithParameter.ts ===
+function useNone<T>(T: number) {}
+>useNone : Symbol(useNone, Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 0, 0))
+>T : Symbol(T, Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 0, 17), Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 0, 20))
+>T : Symbol(T, Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 0, 17), Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 0, 20))
+
+function useParam<T>(T: number) {
+>useParam : Symbol(useParam, Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 0, 33))
+>T : Symbol(T, Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 2, 18), Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 2, 21))
+>T : Symbol(T, Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 2, 18), Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 2, 21))
+
+    return T;
+>T : Symbol(T, Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 2, 18), Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 2, 21))
+}
+
+function useTypeParam<T>(T: T) {}
+>useTypeParam : Symbol(useTypeParam, Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 4, 1))
+>T : Symbol(T, Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 6, 22), Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 6, 25))
+>T : Symbol(T, Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 6, 22), Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 6, 25))
+>T : Symbol(T, Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 6, 22), Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 6, 25))
+
+function useBoth<T>(T: T) {
+>useBoth : Symbol(useBoth, Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 6, 33))
+>T : Symbol(T, Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 8, 17), Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 8, 20))
+>T : Symbol(T, Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 8, 17), Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 8, 20))
+>T : Symbol(T, Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 8, 17), Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 8, 20))
+
+    return T;
+>T : Symbol(T, Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 8, 17), Decl(noUnusedLocals_typeParameterMergedWithParameter.ts, 8, 20))
+}
+
diff --git a/tests/baselines/reference/noUnusedLocals_typeParameterMergedWithParameter.types b/tests/baselines/reference/noUnusedLocals_typeParameterMergedWithParameter.types
@@ -0,0 +1,31 @@
+=== tests/cases/compiler/noUnusedLocals_typeParameterMergedWithParameter.ts ===
+function useNone<T>(T: number) {}
+>useNone : <T>(T: number) => void
+>T : T
+>T : number
+
+function useParam<T>(T: number) {
+>useParam : <T>(T: number) => number
+>T : T
+>T : number
+
+    return T;
+>T : number
+}
+
+function useTypeParam<T>(T: T) {}
+>useTypeParam : <T>(T: T) => void
+>T : T
+>T : T
+>T : T
+
+function useBoth<T>(T: T) {
+>useBoth : <T>(T: T) => T
+>T : T
+>T : T
+>T : T
+
+    return T;
+>T : T
+}
+
diff --git a/tests/cases/compiler/noUnusedLocals_typeParameterMergedWithParameter.ts b/tests/cases/compiler/noUnusedLocals_typeParameterMergedWithParameter.ts
@@ -0,0 +1,14 @@
+// @noUnusedLocals: true
+// @noUnusedParameters: true
+
+function useNone<T>(T: number) {}
+
+function useParam<T>(T: number) {
+    return T;
+}
+
+function useTypeParam<T>(T: T) {}
+
+function useBoth<T>(T: T) {
+    return T;
+}

Original file line number	Diff line number	Diff line change
`@@ -1394,7 +1394,7 @@ namespace ts {`
`1394`	`1394`	// If `result === lastSelfReferenceLocation.symbol`, that means that we are somewhere inside `lastSelfReferenceLocation` looking up a name, and resolving to `lastLocation` itself.
`1395`	`1395`	// That means that this is a self-reference of `lastLocation`, and shouldn't count this when considering whether `lastLocation` is used.
`1396`	`1396`	`if (isUse && result && nameNotFoundMessage && noUnusedIdentifiers && (!lastSelfReferenceLocation \|\| result !== lastSelfReferenceLocation.symbol)) {`
`1397`		`- result.isReferenced = true;`
	`1397`	`+ result.isReferenced \|= meaning;`
`1398`	`1398`	`}`
`1399`	`1399`
`1400`	`1400`	`if (!result) {`
`@@ -15697,7 +15697,7 @@ namespace ts {`
`15697`	`15697`	`if (reactSym) {`
`15698`	`15698`	`// Mark local symbol as referenced here because it might not have been marked`
`15699`	`15699`	`// if jsx emit was not react as there wont be error being emitted`
`15700`		`- reactSym.isReferenced = true;`
	`15700`	`+ reactSym.isReferenced = SymbolFlags.All;`
`15701`	`15701`
`15702`	`15702`	`// If react symbol is alias, mark it as refereced`
`15703`	`15703`	`if (reactSym.flags & SymbolFlags.Alias && !isConstEnumOrConstEnumOnlyModule(resolveAlias(reactSym))) {`
`@@ -16267,12 +16267,7 @@ namespace ts {`
`16267`	`16267`	`}`
`16268`	`16268`	`}`
`16269`	`16269`
`16270`		`- if (getCheckFlags(prop) & CheckFlags.Instantiated) {`
`16271`		`- getSymbolLinks(prop).target.isReferenced = true;`
`16272`		`- }`
`16273`		`- else {`
`16274`		`- prop.isReferenced = true;`
`16275`		`- }`
	`16270`	`+ (getCheckFlags(prop) & CheckFlags.Instantiated ? getSymbolLinks(prop).target : prop).isReferenced = SymbolFlags.All;`
`16276`	`16271`	`}`
`16277`	`16272`
`16278`	`16273`	`function isValidPropertyAccess(node: PropertyAccessExpression \| QualifiedName, propertyName: __String): boolean {`
`@@ -21442,7 +21437,9 @@ namespace ts {`
`21442`	`21437`	`function checkUnusedLocalsAndParameters(node: Node): void {`
`21443`	`21438`	`if (noUnusedIdentifiers && !(node.flags & NodeFlags.Ambient)) {`
`21444`	`21439`	`node.locals.forEach(local => {`
`21445`		`- if (!local.isReferenced) {`
	`21440`	+ // If it's purely a type parameter, ignore, will be checked in `checkUnusedTypeParameters`.
	`21441`	`+ // If it's a type parameter merged with a parameter, check if the parameter-side is used.`
	`21442`	`+ if (local.flags & SymbolFlags.TypeParameter ? (local.flags & SymbolFlags.Variable && !(local.isReferenced & SymbolFlags.Variable)) : !local.isReferenced) {`
`21446`	`21443`	`if (local.valueDeclaration && getRootDeclaration(local.valueDeclaration).kind === SyntaxKind.Parameter) {`
`21447`	`21444`	`const parameter = <ParameterDeclaration>getRootDeclaration(local.valueDeclaration);`
`21448`	`21445`	`const name = getNameOfDeclaration(local.valueDeclaration);`
`@@ -21453,7 +21450,7 @@ namespace ts {`
`21453`	`21450`	`error(name, Diagnostics._0_is_declared_but_its_value_is_never_read, symbolName(local));`
`21454`	`21451`	`}`
`21455`	`21452`	`}`
`21456`		`- else if (local.flags & SymbolFlags.TypeParameter ? compilerOptions.noUnusedParameters : compilerOptions.noUnusedLocals) {`
	`21453`	`+ else if (compilerOptions.noUnusedLocals) {`
`21457`	`21454`	`forEach(local.declarations, d => errorUnusedLocal(d, symbolName(local)));`
`21458`	`21455`	`}`
`21459`	`21456`	`}`
`@@ -21538,7 +21535,7 @@ namespace ts {`
`21538`	`21535`	`return;`
`21539`	`21536`	`}`
`21540`	`21537`	`for (const typeParameter of node.typeParameters) {`
`21541`		`- if (!getMergedSymbol(typeParameter.symbol).isReferenced && !isIdentifierThatStartsWithUnderScore(typeParameter.name)) {`
	`21538`	`+ if (!(getMergedSymbol(typeParameter.symbol).isReferenced & SymbolFlags.TypeParameter) && !isIdentifierThatStartsWithUnderScore(typeParameter.name)) {`
`21542`	`21539`	`error(typeParameter.name, Diagnostics._0_is_declared_but_its_value_is_never_read, symbolName(typeParameter.symbol));`
`21543`	`21540`	`}`
`21544`	`21541`	`}`
Original file line number	Diff line number	Diff line change
`@@ -3307,7 +3307,7 @@ namespace ts {`
`3307`	`3307`	`/* @internal */ parent?: Symbol; // Parent symbol`
`3308`	`3308`	`/* @internal */ exportSymbol?: Symbol; // Exported symbol associated with this symbol`
`3309`	`3309`	`/* @internal */ constEnumOnlyModule?: boolean; // True if module contains only const enums or other modules with only const enums`
`3310`		`- /* @internal */ isReferenced?: boolean; // True if the symbol is referenced elsewhere`
	`3310`	`+ /* @internal */ isReferenced?: SymbolFlags; // True if the symbol is referenced elsewhere. Keeps track of the meaning of a reference in case a symbol is both a type parameter and parameter.`
`3311`	`3311`	`/* @internal */ isReplaceableByMethod?: boolean; // Can this Javascript class property be replaced by a method symbol?`
`3312`	`3312`	`/* @internal */ isAssigned?: boolean; // True if the symbol is a parameter with assignments`
`3313`	`3313`	`}`