Skip to content

Forbid unused locals/parameters in compiler#11715

Merged
2 commits merged into
masterfrom
unused
Oct 19, 2016
Merged

Forbid unused locals/parameters in compiler#11715
2 commits merged into
masterfrom
unused

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Oct 18, 2016

Mostly I just removed any parameter that wasn't used, and then removed it from callers. If a function is used as a callback, and other callbacks use that parameter, I use an _underscored parameter name.

TODO: The same thing for harness/server/services, and probably use tsconfig inheritance.

Copy link
Copy Markdown
Contributor

@mhegazy mhegazy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

can you also build LKG and make sure non of these changes change typesript.d.ts, we do not want to break API users

Comment thread src/compiler/comments.ts Outdated
}

function emitTripleSlashLeadingComment(commentPos: number, commentEnd: number, kind: SyntaxKind, hasTrailingNewLine: boolean, rangePos: number) {
function emitTripleSlashLeadingComment(commentPos: number, commentEnd: number, _kind: SyntaxKind, hasTrailingNewLine: boolean, rangePos: number) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why _kind? seems it is used two lines below.

Comment thread src/compiler/core.ts Outdated
}
else {
return t => u => u;
return _t => u => u;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider making it jsut _

Comment thread src/compiler/emitter.ts Outdated

const id = (s: SourceFile) => s;
const nullTransformers: Transformer[] = [ctx => id];
const nullTransformers: Transformer[] = [_ctx => id];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_ instead of _ctx

Comment thread src/compiler/moduleNameResolver.ts Outdated
/* @internal */
export function trace(host: ModuleResolutionHost, message: DiagnosticMessage, ...args: any[]): void;
export function trace(host: ModuleResolutionHost, message: DiagnosticMessage): void {
export function trace(host: ModuleResolutionHost, _message: DiagnosticMessage): void {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider removing _message all together from the implementation signature, export function trace(host: ModuleResolutionHost): void { .. }

Comment thread src/compiler/parser.ts Outdated
@@ -575,7 +575,7 @@ namespace ts {
export function parseSourceFile(fileName: string, _sourceText: string, languageVersion: ScriptTarget, _syntaxCursor: IncrementalParser.SyntaxCursor, setParentNodes?: boolean, scriptKind?: ScriptKind): SourceFile {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do these start with _?

Comment thread src/compiler/sys.ts Outdated
getCurrentDirectory: () => ChakraHost.currentDirectory,
getDirectories: ChakraHost.getDirectories,
getEnvironmentVariable: ChakraHost.getEnvironmentVariable || ((name: string) => ""),
getEnvironmentVariable: ChakraHost.getEnvironmentVariable || ((_name: string) => ""),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not think you need ht type annotation here.

@ghost
Copy link
Copy Markdown
Author

ghost commented Oct 19, 2016

typescript.d.ts LKG is identical whether built on master or in this branch. Assuming you didn't mean for me to check in a new LKG.

@ghost ghost merged commit db0ee4f into master Oct 19, 2016
@ghost ghost deleted the unused branch October 19, 2016 13:43
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants