Skip to content

Conditionally emit rest parameters #518

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
156 changes: 89 additions & 67 deletions src/compiler/checker.ts

Large diffs are not rendered by default.

37 changes: 21 additions & 16 deletions src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,11 @@ module ts {
// }
// module m {
// /* this is line 1 -- Assume current writer indent 8
// * line --3 = 8 - 4 + 5
// * line --3 = 8 - 4 + 5
// More right indented comment */ --4 = 8 - 4 + 11
// class c { }
// }
var spacesToEmit = currentWriterIndentSpacing - firstCommentLineIndent + calculateIndent(pos, nextLineStart);
var spacesToEmit = currentWriterIndentSpacing - firstCommentLineIndent + calculateIndent(pos, nextLineStart);
if (spacesToEmit > 0) {
var numberOfSingleSpacesToEmit = spacesToEmit % getIndentSize();
var indentSizeSpaceString = getIndentString((spacesToEmit - numberOfSingleSpacesToEmit) / getIndentSize());
Expand Down Expand Up @@ -348,7 +348,7 @@ module ts {
var emitEnd = function (node: Node) { };

/** Emit the text for the given token that comes after startPos
* This by default writes the text provided with the given tokenKind
* This by default writes the text provided with the given tokenKind
* but if optional emitFn callback is provided the text is emitted using the callback instead of default text
* @param tokenKind the kind of the token to search and emit
* @param startPos the position in the source to start searching for the token
Expand Down Expand Up @@ -417,13 +417,13 @@ module ts {
// 1. Relative Column 0 based
sourceMapData.sourceMapMappings += base64VLQFormatEncode(lastRecordedSourceMapSpan.emittedColumn - prevEncodedEmittedColumn);

// 2. Relative sourceIndex
// 2. Relative sourceIndex
sourceMapData.sourceMapMappings += base64VLQFormatEncode(lastRecordedSourceMapSpan.sourceIndex - lastEncodedSourceMapSpan.sourceIndex);

// 3. Relative sourceLine 0 based
sourceMapData.sourceMapMappings += base64VLQFormatEncode(lastRecordedSourceMapSpan.sourceLine - lastEncodedSourceMapSpan.sourceLine);

// 4. Relative sourceColumn 0 based
// 4. Relative sourceColumn 0 based
sourceMapData.sourceMapMappings += base64VLQFormatEncode(lastRecordedSourceMapSpan.sourceColumn - lastEncodedSourceMapSpan.sourceColumn);

// 5. Relative namePosition 0 based
Expand Down Expand Up @@ -523,7 +523,7 @@ module ts {

function recordNewSourceFileStart(node: SourceFile) {
// Add the the file to tsFilePaths
// If sourceroot option: Use the relative path corresponding to the common directory path
// If sourceroot option: Use the relative path corresponding to the common directory path
// otherwise source locations relative to map file location
var sourcesDirectoryPath = compilerOptions.sourceRoot ? program.getCommonSourceDirectory() : sourceMapDir;

Expand Down Expand Up @@ -625,7 +625,7 @@ module ts {
sourceMapDecodedMappings: []
};

// Normalize source root and make sure it has trailing "/" so that it can be used to combine paths with the
// Normalize source root and make sure it has trailing "/" so that it can be used to combine paths with the
// relative paths of the sources list in the sourcemap
sourceMapData.sourceMapSourceRoot = ts.normalizeSlashes(sourceMapData.sourceMapSourceRoot);
if (sourceMapData.sourceMapSourceRoot.length && sourceMapData.sourceMapSourceRoot.charCodeAt(sourceMapData.sourceMapSourceRoot.length - 1) !== CharacterCodes.slash) {
Expand Down Expand Up @@ -929,15 +929,15 @@ module ts {
var operand = (<TypeAssertion>node.expression).operand;

// Make sure we consider all nested cast expressions, e.g.:
// (<any><number><any>-A).x;
// (<any><number><any>-A).x;
while (operand.kind == SyntaxKind.TypeAssertion) {
operand = (<TypeAssertion>operand).operand;
}

// We have an expression of the form: (<Type>SubExpr)
// Emitting this as (SubExpr) is really not desirable. We would like to emit the subexpr as is.
// Omitting the parentheses, however, could cause change in the semantics of the generated
// code if the casted expression has a lower precedence than the rest of the expression, e.g.:
// code if the casted expression has a lower precedence than the rest of the expression, e.g.:
// (<any>new A).foo should be emitted as (new A).foo and not new A.foo
// (<any>typeof A).toString() should be emitted as (typeof A).toString() and not typeof A.toString()
// new (<any>A()) should be emitted as new (A()) and not new A()
Expand Down Expand Up @@ -1275,6 +1275,11 @@ module ts {
if (hasRestParameters(node)) {
var restIndex = node.parameters.length - 1;
var restParam = node.parameters[restIndex];

if (!resolver.isParameterUsedInFunction(node, restParam)) {
return;
}

writeLine();
emitLeadingComments(restParam);
emitStart(restParam);
Expand Down Expand Up @@ -2156,7 +2161,7 @@ module ts {

if (commentLine >= lastCommentLine + 2) {
// There was a blank line between the last comment and this comment. This
// comment is not part of the copyright comments. Return what we have so
// comment is not part of the copyright comments. Return what we have so
// far.
return detachedComments;
}
Expand Down Expand Up @@ -2335,7 +2340,7 @@ module ts {
}
// If the node is parented in the current source file we need to emit export declare or just export
else if (node.parent === currentSourceFile) {
// If the node is exported
// If the node is exported
if (node.flags & NodeFlags.Export) {
write("export ");
}
Expand All @@ -2361,7 +2366,7 @@ module ts {
}

function writeImportDeclaration(node: ImportDeclaration) {
// note usage of writer. methods instead of aliases created, just to make sure we are using
// note usage of writer. methods instead of aliases created, just to make sure we are using
// correct writer especially to handle asynchronous alias writing
emitJsDocComments(node);
if (node.flags & NodeFlags.Export) {
Expand Down Expand Up @@ -2845,7 +2850,7 @@ module ts {
switch (node.kind) {
case SyntaxKind.ConstructSignature:
// Interfaces cannot have return types that cannot be named
diagnosticMessage = symbolAccesibilityResult.errorModuleName ?
diagnosticMessage = symbolAccesibilityResult.errorModuleName ?
Diagnostics.Return_type_of_constructor_signature_from_exported_interface_has_or_is_using_name_0_from_private_module_1 :
Diagnostics.Return_type_of_constructor_signature_from_exported_interface_has_or_is_using_private_name_0;
break;
Expand Down Expand Up @@ -3036,8 +3041,8 @@ module ts {
return program.getSourceFile(referenceFileName);
}

// Contains the reference paths that needs to go in the declaration file.
// Collecting this separately because reference paths need to be first thing in the declaration file
// Contains the reference paths that needs to go in the declaration file.
// Collecting this separately because reference paths need to be first thing in the declaration file
// and we could be collecting these paths from multiple files into single one with --out option
var referencePathsOutput = "";
function writeReferencePath(referencedFile: SourceFile) {
Expand Down Expand Up @@ -3099,7 +3104,7 @@ module ts {
});
}

// TODO(shkamat): Should we not write any declaration file if any of them can produce error,
// TODO(shkamat): Should we not write any declaration file if any of them can produce error,
// or should we just not write this file like we are doing now
if (!reportedDeclarationError) {
var declarationOutput = referencePathsOutput;
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,7 @@ module ts {
shouldEmitDeclarations(): boolean;
isDeclarationVisible(node: Declaration): boolean;
isImplementationOfOverload(node: FunctionDeclaration): boolean;
isParameterUsedInFunction(func: FunctionDeclaration, param: ParameterDeclaration): boolean;
writeTypeAtLocation(location: Node, enclosingDeclaration: Node, flags: TypeFormatFlags, writer: TextWriter): void;
writeReturnTypeOfSignatureDeclaration(signatureDeclaration: SignatureDeclaration, enclosingDeclaration: Node, flags: TypeFormatFlags, writer: TextWriter): void;
writeSymbol(symbol: Symbol, enclosingDeclaration: Node, meaning: SymbolFlags, writer: TextWriter): void;
Expand Down
4 changes: 0 additions & 4 deletions tests/baselines/reference/arrayLiteralInNonVarArgParameter.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,5 @@ panic([], 'one', 'two');

//// [arrayLiteralInNonVarArgParameter.js]
function panic(val) {
var opt = [];
for (var _i = 1; _i < arguments.length; _i++) {
opt[_i - 1] = arguments[_i];
}
}
panic([], 'one', 'two');
Original file line number Diff line number Diff line change
Expand Up @@ -49,49 +49,19 @@ var a4: (x?: number, y?: string, ...z: number[]) => number;
// call signatures in derived types must have the same or fewer optional parameters as the target for assignment
var a; // ok, same number of required params
a = function () { return 1; }; // ok, same number of required params
a = function () {
var args = [];
for (var _i = 0; _i < arguments.length; _i++) {
args[_i - 0] = arguments[_i];
}
return 1;
}; // ok, same number of required params
a = function () {
var args = [];
for (var _i = 0; _i < arguments.length; _i++) {
args[_i - 0] = arguments[_i];
}
return 1;
}; // error, type mismatch
a = function () { return 1; }; // ok, same number of required params
a = function () { return 1; }; // error, type mismatch
a = function (x) { return 1; }; // ok, same number of required params
a = function (x, y, z) { return 1; }; // ok, same number of required params
a = function (x) { return 1; }; // ok, rest param corresponds to infinite number of params
a = function (x) { return 1; }; // error, incompatible type
var a2;
a2 = function () { return 1; }; // ok, fewer required params
a2 = function () {
var args = [];
for (var _i = 0; _i < arguments.length; _i++) {
args[_i - 0] = arguments[_i];
}
return 1;
}; // ok, fewer required params
a2 = function () { return 1; }; // ok, fewer required params
a2 = function (x) { return 1; }; // ok, fewer required params
a2 = function (x) { return 1; }; // ok, same number of required params
a2 = function (x) {
var args = [];
for (var _i = 1; _i < arguments.length; _i++) {
args[_i - 1] = arguments[_i];
}
return 1;
}; // ok, same number of required params
a2 = function (x) {
var args = [];
for (var _i = 1; _i < arguments.length; _i++) {
args[_i - 1] = arguments[_i];
}
return 1;
}; // should be type mismatch error
a2 = function (x) { return 1; }; // ok, same number of required params
a2 = function (x) { return 1; }; // should be type mismatch error
a2 = function (x, y) { return 1; }; // ok, rest param corresponds to infinite number of params
a2 = function (x, y) { return 1; }; // ok, same number of required params
var a3;
Expand All @@ -100,24 +70,12 @@ a3 = function (x) { return 1; }; // ok, fewer required params
a3 = function (x) { return 1; }; // ok, same number of required params
a3 = function (x, y) { return 1; }; // ok, all present params match
a3 = function (x, y, z) { return 1; }; // error
a3 = function (x) {
var z = [];
for (var _i = 1; _i < arguments.length; _i++) {
z[_i - 1] = arguments[_i];
}
return 1;
}; // error
a3 = function (x) { return 1; }; // error
a3 = function (x, y, z) { return 1; }; // error
var a4;
a4 = function () { return 1; }; // ok, fewer required params
a4 = function (x, y) { return 1; }; // error, type mismatch
a4 = function (x) { return 1; }; // ok, all present params match
a4 = function (x, y) { return 1; }; // error, second param has type mismatch
a4 = function (x, y) { return 1; }; // ok, same number of required params with matching types
a4 = function (x) {
var args = [];
for (var _i = 1; _i < arguments.length; _i++) {
args[_i - 1] = arguments[_i];
}
return 1;
}; // error, rest params have type mismatch
a4 = function (x) { return 1; }; // error, rest params have type mismatch
4 changes: 0 additions & 4 deletions tests/baselines/reference/baseTypeAfterDerivedType.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ var Derived2 = (function () {
function Derived2() {
}
Derived2.prototype.method = function () {
var args = [];
for (var _i = 0; _i < arguments.length; _i++) {
args[_i - 0] = arguments[_i];
}
};
return Derived2;
})();
8 changes: 0 additions & 8 deletions tests/baselines/reference/collisionArgumentsArrowFunctions.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,12 @@ var f1 = function (i) {
var arguments; // no error
};
var f12 = function (arguments) {
var rest = [];
for (var _i = 1; _i < arguments.length; _i++) {
rest[_i - 1] = arguments[_i];
}
var arguments = 10; // no error
};
var f1NoError = function (arguments) {
var arguments = 10; // no error
};
var f2 = function () {
var restParameters = [];
for (var _i = 0; _i < arguments.length; _i++) {
restParameters[_i - 0] = arguments[_i];
}
var arguments = 10; // No Error
};
var f2NoError = function () {
Expand Down
16 changes: 0 additions & 16 deletions tests/baselines/reference/collisionArgumentsClassConstructor.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,6 @@ var c1 = (function () {
})();
var c12 = (function () {
function c12(arguments) {
var rest = [];
for (var _i = 1; _i < arguments.length; _i++) {
rest[_i - 1] = arguments[_i];
}
var arguments = 10; // no error
}
return c12;
Expand All @@ -116,10 +112,6 @@ var c1NoError = (function () {
})();
var c2 = (function () {
function c2() {
var restParameters = [];
for (var _i = 0; _i < arguments.length; _i++) {
restParameters[_i - 0] = arguments[_i];
}
var arguments = 10; // no error
}
return c2;
Expand All @@ -132,10 +124,6 @@ var c2NoError = (function () {
})();
var c3 = (function () {
function c3(arguments) {
var restParameters = [];
for (var _i = 1; _i < arguments.length; _i++) {
restParameters[_i - 1] = arguments[_i];
}
this.arguments = arguments;
var arguments = 10; // no error
}
Expand All @@ -160,10 +148,6 @@ var c5 = (function () {
})();
var c52 = (function () {
function c52(arguments) {
var rest = [];
for (var _i = 1; _i < arguments.length; _i++) {
rest[_i - 1] = arguments[_i];
}
var arguments; // no error
}
return c52;
Expand Down
12 changes: 0 additions & 12 deletions tests/baselines/reference/collisionArgumentsClassMethod.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,6 @@ var c1 = (function () {
var arguments; // no error
};
c1.prototype.foo1 = function (arguments) {
var rest = [];
for (var _i = 1; _i < arguments.length; _i++) {
rest[_i - 1] = arguments[_i];
}
var arguments = 10; // no error
};
c1.prototype.fooNoError = function (arguments) {
Expand All @@ -77,10 +73,6 @@ var c1 = (function () {
var arguments; // no error
};
c1.prototype.f41 = function (arguments) {
var rest = [];
for (var _i = 1; _i < arguments.length; _i++) {
rest[_i - 1] = arguments[_i];
}
var arguments; // no error
};
c1.prototype.f4NoError = function (arguments) {
Expand All @@ -92,10 +84,6 @@ var c3 = (function () {
function c3() {
}
c3.prototype.foo = function () {
var restParameters = [];
for (var _i = 0; _i < arguments.length; _i++) {
restParameters[_i - 0] = arguments[_i];
}
var arguments = 10; // no error
};
c3.prototype.fooNoError = function () {
Expand Down
12 changes: 0 additions & 12 deletions tests/baselines/reference/collisionArgumentsFunction.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,6 @@ declare function f6(arguments: string); // no codegen no error
//// [collisionArgumentsFunction.js]
// Functions
function f1(arguments) {
var restParameters = [];
for (var _i = 1; _i < arguments.length; _i++) {
restParameters[_i - 1] = arguments[_i];
}
var arguments = 10; // no error
}
function f12(i) {
Expand All @@ -64,20 +60,12 @@ function f1NoError(arguments) {
var arguments = 10; // no error
}
function f3() {
var restParameters = [];
for (var _i = 0; _i < arguments.length; _i++) {
restParameters[_i - 0] = arguments[_i];
}
var arguments = 10; // no error
}
function f3NoError() {
var arguments = 10; // no error
}
function f4(arguments) {
var rest = [];
for (var _i = 1; _i < arguments.length; _i++) {
rest[_i - 1] = arguments[_i];
}
var arguments; // No error
}
function f42(i) {
Expand Down
Loading