- 
                Notifications
    You must be signed in to change notification settings 
- Fork 13.1k
Unused identifiers compiler code #9200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
617b819
              a8bc8b4
              01966ba
              1e5ca92
              890d178
              69dbeea
              c78d0e2
              107b369
              481baa3
              cd1ce0f
              a38149d
              0571c19
              e17ed58
              2b7f3a7
              5a34352
              93b7490
              7aba626
              ed5052d
              f5cdc9c
              8c3c7b1
              dfad7cc
              c325625
              6a711bc
              d62a43f
              043a625
              8f9d4ae
              f15448a
              c82453f
              bcd6fc4
              49385f4
              5993015
              f464f92
              3b5f8d2
              ed282d7
              e502ba0
              0e2e43d
              d6c2bcd
              f93c6c8
              4521058
              5eb7153
              1401506
              5361e5f
              7fc4616
              9753d09
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -8310,8 +8310,21 @@ namespace ts { | |
| return container === declarationContainer; | ||
| } | ||
|  | ||
| function updateReferencesForInterfaceHeritiageClauseTargets(node: InterfaceDeclaration): void { | ||
| const extendedTypeNode = getClassExtendsHeritageClauseElement(node); | ||
| if (extendedTypeNode) { | ||
| const t = getTypeFromTypeNode(extendedTypeNode); | ||
| if (t !== unknownType && t.symbol && (compilerOptions.noUnusedLocals || compilerOptions.noUnusedParameters) && !isInAmbientContext(node)) { | ||
| t.symbol.hasReference = true; | ||
| } | ||
| } | ||
| } | ||
|  | ||
| function checkIdentifier(node: Identifier): Type { | ||
| const symbol = getResolvedSymbol(node); | ||
| if (symbol && (compilerOptions.noUnusedLocals || compilerOptions.noUnusedParameters) && !isInAmbientContext(node)) { | ||
| symbol.hasReference = true; | ||
| } | ||
|  | ||
| // As noted in ECMAScript 6 language spec, arrow functions never have an arguments objects. | ||
| // Although in down-level emit of arrow function, we emit it using function expression which means that | ||
|  | @@ -10202,6 +10215,10 @@ namespace ts { | |
| return unknownType; | ||
| } | ||
|  | ||
| if ((compilerOptions.noUnusedLocals || compilerOptions.noUnusedParameters) && !isInAmbientContext(node)) { | ||
| prop.hasReference = true; | ||
| } | ||
|  | ||
| getNodeLinks(node).resolvedSymbol = prop; | ||
|  | ||
| if (prop.parent && prop.parent.flags & SymbolFlags.Class) { | ||
|  | @@ -12144,6 +12161,8 @@ namespace ts { | |
| } | ||
| } | ||
| } | ||
| checkUnusedIdentifiers(node); | ||
| checkUnusedTypeParameters(node); | ||
| } | ||
| } | ||
|  | ||
|  | @@ -13214,6 +13233,9 @@ namespace ts { | |
| checkAsyncFunctionReturnType(<FunctionLikeDeclaration>node); | ||
| } | ||
| } | ||
| if (!(<FunctionDeclaration>node).body) { | ||
| checkUnusedTypeParameters(node); | ||
| } | ||
| } | ||
| } | ||
|  | ||
|  | @@ -13366,6 +13388,8 @@ namespace ts { | |
| checkGrammarConstructorTypeParameters(node) || checkGrammarConstructorTypeAnnotation(node); | ||
|  | ||
| checkSourceElement(node.body); | ||
| checkUnusedIdentifiers(node); | ||
| checkUnusedTypeParameters(node); | ||
|  | ||
| const symbol = getSymbolOfNode(node); | ||
| const firstDeclaration = getDeclarationOfKind(symbol, node.kind); | ||
|  | @@ -13558,13 +13582,18 @@ namespace ts { | |
| function checkTypeReferenceNode(node: TypeReferenceNode | ExpressionWithTypeArguments) { | ||
| checkGrammarTypeArguments(node, node.typeArguments); | ||
| const type = getTypeFromTypeReference(node); | ||
| if (type !== unknownType && node.typeArguments) { | ||
| // Do type argument local checks only if referenced type is successfully resolved | ||
| forEach(node.typeArguments, checkSourceElement); | ||
| if (produceDiagnostics) { | ||
| const symbol = getNodeLinks(node).resolvedSymbol; | ||
| const typeParameters = symbol.flags & SymbolFlags.TypeAlias ? getSymbolLinks(symbol).typeParameters : (<TypeReference>type).target.localTypeParameters; | ||
| checkTypeArgumentConstraints(typeParameters, node.typeArguments); | ||
| if (type !== unknownType) { | ||
| if (type.symbol && (compilerOptions.noUnusedLocals || compilerOptions.noUnusedParameters) && !isInAmbientContext(node)) { | ||
| type.symbol.hasReference = true; | ||
| } | ||
| if (node.typeArguments) { | ||
| // Do type argument local checks only if referenced type is successfully resolved | ||
| forEach(node.typeArguments, checkSourceElement); | ||
| if (produceDiagnostics) { | ||
| const symbol = getNodeLinks(node).resolvedSymbol; | ||
| const typeParameters = symbol.flags & SymbolFlags.TypeAlias ? getSymbolLinks(symbol).typeParameters : (<TypeReference>type).target.localTypeParameters; | ||
| checkTypeArgumentConstraints(typeParameters, node.typeArguments); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|  | @@ -14407,6 +14436,8 @@ namespace ts { | |
| } | ||
|  | ||
| checkSourceElement(node.body); | ||
| checkUnusedIdentifiers(node); | ||
| checkUnusedTypeParameters(node); | ||
| if (!node.asteriskToken) { | ||
| const returnOrPromisedType = node.type && (isAsync ? checkAsyncFunctionReturnType(node) : getTypeFromTypeNode(node.type)); | ||
| checkAllCodePathsInNonVoidFunctionReturnOrThrow(node, returnOrPromisedType); | ||
|  | @@ -14428,12 +14459,83 @@ namespace ts { | |
| } | ||
| } | ||
|  | ||
| function checkUnusedIdentifiers(node: FunctionDeclaration | MethodDeclaration | ConstructorDeclaration | FunctionExpression | ArrowFunction | ForInStatement | Block | CatchClause): void { | ||
| if (node.parent.kind !== SyntaxKind.InterfaceDeclaration && (compilerOptions.noUnusedLocals || compilerOptions.noUnusedParameters) && !isInAmbientContext(node)) { | ||
| for (const key in node.locals) { | ||
| if (hasProperty(node.locals, key)) { | ||
| const local = node.locals[key]; | ||
| if (!local.hasReference && local.valueDeclaration) { | ||
| if (local.valueDeclaration.kind !== SyntaxKind.Parameter && compilerOptions.noUnusedLocals) { | ||
| error(local.valueDeclaration.name, Diagnostics._0_is_declared_but_never_used, local.name); | ||
| } | ||
| else if (local.valueDeclaration.kind === SyntaxKind.Parameter && compilerOptions.noUnusedParameters) { | ||
|          | ||
| if (local.valueDeclaration.flags === 0) { | ||
| error(local.valueDeclaration.name, Diagnostics._0_is_declared_but_never_used, local.name); | ||
| } | ||
|          | ||
| } | ||
| } | ||
| } | ||
|          | ||
| } | ||
| } | ||
| } | ||
|  | ||
| function checkUnusedClassLocals(node: ClassDeclaration): void { | ||
| if (compilerOptions.noUnusedLocals && !isInAmbientContext(node)) { | ||
| if (node.members) { | ||
| for (const member of node.members) { | ||
| if (member.kind === SyntaxKind.MethodDeclaration || member.kind === SyntaxKind.PropertyDeclaration) { | ||
| if (isPrivateNode(member) && !member.symbol.hasReference) { | ||
| error(member.name, Diagnostics._0_is_declared_but_never_used, member.symbol.name); | ||
| } | ||
| } | ||
| else if (member.kind === SyntaxKind.Constructor) { | ||
| for (const parameter of (<ConstructorDeclaration>member).parameters) { | ||
| if (isPrivateNode(parameter) && !parameter.symbol.hasReference) { | ||
| error(parameter.name, Diagnostics._0_is_declared_but_never_used, parameter.symbol.name); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|  | ||
| function checkUnusedTypeParameters(node: ClassDeclaration | FunctionDeclaration | MethodDeclaration | FunctionExpression | ArrowFunction | ConstructorDeclaration | SignatureDeclaration | InterfaceDeclaration) { | ||
| if (compilerOptions.noUnusedLocals && !isInAmbientContext(node)) { | ||
| if (node.typeParameters) { | ||
| for (const typeParameter of node.typeParameters) { | ||
| if (!typeParameter.symbol.hasReference) { | ||
| error(typeParameter.name, Diagnostics._0_is_declared_but_never_used, typeParameter.symbol.name); | ||
| } | ||
| } | ||
| } | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i would extract this to  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done | ||
| } | ||
| } | ||
|  | ||
| function isPrivateNode(node: Node): boolean { | ||
| return (node.flags & NodeFlags.Private) !== 0; | ||
| } | ||
|  | ||
| function checkUnusedModuleLocals(node: ModuleDeclaration | SourceFile): void { | ||
| if (compilerOptions.noUnusedLocals && !isInAmbientContext(node)) { | ||
| for (const key in node.locals) { | ||
| if (hasProperty(node.locals, key)) { | ||
|          | ||
| const local = node.locals[key]; | ||
| if (!local.hasReference && !local.exportSymbol) { | ||
|          | ||
| forEach(local.declarations, d => error(d.name, Diagnostics._0_is_declared_but_never_used, local.name)); | ||
| } | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no need to check the  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we merge this function with the checkUnusedModulePrivates ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed the ambient context check. I prefer to keep them separate for sake of clarity, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i am not sure i see the point here. just merge them togather, and get rid of all the checks on the kinds. | ||
| } | ||
| } | ||
| } | ||
| } | ||
|  | ||
| function checkBlock(node: Block) { | ||
| // Grammar checking for SyntaxKind.Block | ||
| if (node.kind === SyntaxKind.Block) { | ||
| checkGrammarStatementInAmbientContext(node); | ||
| } | ||
| forEach(node.statements, checkSourceElement); | ||
| checkUnusedIdentifiers(node); | ||
| } | ||
|  | ||
| function checkCollisionWithArgumentsInGeneratedCode(node: SignatureDeclaration) { | ||
|  | @@ -14938,6 +15040,7 @@ namespace ts { | |
| } | ||
|  | ||
| checkSourceElement(node.statement); | ||
| checkUnusedIdentifiers(node); | ||
| } | ||
|  | ||
| function checkForInStatement(node: ForInStatement) { | ||
|  | @@ -14985,6 +15088,7 @@ namespace ts { | |
| } | ||
|  | ||
| checkSourceElement(node.statement); | ||
| checkUnusedIdentifiers(node); | ||
| } | ||
|  | ||
| function checkForInOrForOfVariableDeclaration(iterationStatement: ForInStatement | ForOfStatement): void { | ||
|  | @@ -15424,6 +15528,7 @@ namespace ts { | |
| } | ||
|  | ||
| checkBlock(catchClause.block); | ||
| checkUnusedIdentifiers(catchClause); | ||
| } | ||
|  | ||
| if (node.finallyBlock) { | ||
|  | @@ -15585,6 +15690,8 @@ namespace ts { | |
| } | ||
| checkClassLikeDeclaration(node); | ||
| forEach(node.members, checkSourceElement); | ||
| checkUnusedClassLocals(node); | ||
| checkUnusedTypeParameters(node); | ||
| } | ||
|  | ||
| function checkClassLikeDeclaration(node: ClassLikeDeclaration) { | ||
|  | @@ -15894,6 +16001,8 @@ namespace ts { | |
|  | ||
| if (produceDiagnostics) { | ||
| checkTypeForDuplicateIndexSignatures(node); | ||
| updateReferencesForInterfaceHeritiageClauseTargets(node); | ||
| checkUnusedTypeParameters(node); | ||
| } | ||
| } | ||
|  | ||
|  | @@ -16290,6 +16399,7 @@ namespace ts { | |
|  | ||
| if (node.body) { | ||
| checkSourceElement(node.body); | ||
| checkUnusedModuleLocals(node); | ||
| } | ||
| } | ||
|  | ||
|  | @@ -16470,6 +16580,9 @@ namespace ts { | |
| if (target.flags & SymbolFlags.Type) { | ||
| checkTypeNameIsReserved(node.name, Diagnostics.Import_name_cannot_be_0); | ||
| } | ||
| if ((compilerOptions.noUnusedLocals || compilerOptions.noUnusedParameters) && !isInAmbientContext(node)) { | ||
| target.hasReference = true; | ||
| } | ||
| } | ||
| } | ||
| else { | ||
|  | @@ -16812,6 +16925,9 @@ namespace ts { | |
|  | ||
| deferredNodes = []; | ||
| forEach(node.statements, checkSourceElement); | ||
| if (isExternalModule(node)) { | ||
| checkUnusedModuleLocals(node); | ||
| } | ||
| checkDeferredNodes(); | ||
| deferredNodes = undefined; | ||
|  | ||
|  | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -2792,6 +2792,18 @@ | |
| "category": "Message", | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be an "Error" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done | ||
| "code": 6132 | ||
| }, | ||
| "'{0}' is declared but never used.": { | ||
| "category": "Error", | ||
| "code": 6133 | ||
| }, | ||
| "Report Errors on Unused Locals.": { | ||
| "category": "Message", | ||
| "code": 6134 | ||
| }, | ||
| "Report Errors on Unused Parameters.": { | ||
| "category": "Message", | ||
| "code": 6135 | ||
| }, | ||
|  | ||
| "Variable '{0}' implicitly has an '{1}' type.": { | ||
| "category": "Error", | ||
|  | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| tests/cases/compiler/unusedClassesinModule1.ts(3,11): error TS6133: 'Calculator' is declared but never used. | ||
|  | ||
|  | ||
| ==== tests/cases/compiler/unusedClassesinModule1.ts (1 errors) ==== | ||
|  | ||
| module A { | ||
| class Calculator { | ||
| ~~~~~~~~~~ | ||
| !!! error TS6133: 'Calculator' is declared but never used. | ||
| public handelChar() { | ||
| } | ||
| } | ||
| } | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| //// [unusedClassesinModule1.ts] | ||
|  | ||
| module A { | ||
| class Calculator { | ||
| public handelChar() { | ||
| } | ||
| } | ||
| } | ||
|  | ||
| //// [unusedClassesinModule1.js] | ||
| var A; | ||
| (function (A) { | ||
| var Calculator = (function () { | ||
| function Calculator() { | ||
| } | ||
| Calculator.prototype.handelChar = function () { | ||
| }; | ||
| return Calculator; | ||
| }()); | ||
| })(A || (A = {})); | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| tests/cases/compiler/unusedClassesinNamespace1.ts(3,11): error TS6133: 'c1' is declared but never used. | ||
|  | ||
|  | ||
| ==== tests/cases/compiler/unusedClassesinNamespace1.ts (1 errors) ==== | ||
|  | ||
| namespace Validation { | ||
| class c1 { | ||
| ~~ | ||
| !!! error TS6133: 'c1' is declared but never used. | ||
|  | ||
| } | ||
| } | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| //// [unusedClassesinNamespace1.ts] | ||
|  | ||
| namespace Validation { | ||
| class c1 { | ||
|  | ||
| } | ||
| } | ||
|  | ||
| //// [unusedClassesinNamespace1.js] | ||
| var Validation; | ||
| (function (Validation) { | ||
| var c1 = (function () { | ||
| function c1() { | ||
| } | ||
| return c1; | ||
| }()); | ||
| })(Validation || (Validation = {})); | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| tests/cases/compiler/unusedClassesinNamespace2.ts(3,11): error TS6133: 'c1' is declared but never used. | ||
|  | ||
|  | ||
| ==== tests/cases/compiler/unusedClassesinNamespace2.ts (1 errors) ==== | ||
|  | ||
| namespace Validation { | ||
| class c1 { | ||
| ~~ | ||
| !!! error TS6133: 'c1' is declared but never used. | ||
|  | ||
| } | ||
|  | ||
| export class c2 { | ||
|  | ||
| } | ||
| } | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should check unsued type parameters in functions as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done (And added test cases also)