Skip to content

Commit

Permalink
Made inline functions compatible with default parameters
Browse files Browse the repository at this point in the history
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=159463552
  • Loading branch information
bellashim authored and blickly committed Jun 19, 2017
1 parent 4970a2d commit 8909f88
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 5 deletions.
3 changes: 1 addition & 2 deletions src/com/google/javascript/jscomp/DefinitionsRemover.java
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ static Definition getDefinition(Node n, boolean isExtern) {
return new AssignmentDefinition(parent, isExtern);
} else if (NodeUtil.isObjectLitKey(n)) {
return new ObjectLiteralPropertyDefinition(parent, n, n.getFirstChild(), isExtern);
} else if (NodeUtil.getEnclosingType(n, Token.PARAM_LIST) != null
&& !n.isParamList() && !n.isRest()) {
} else if (NodeUtil.getEnclosingType(n, Token.PARAM_LIST) != null && n.isName()) {
Node paramList = NodeUtil.getEnclosingType(n, Token.PARAM_LIST);
Node function = paramList.getParent();
return new FunctionArgumentDefinition(function, n, isExtern);
Expand Down
16 changes: 13 additions & 3 deletions src/com/google/javascript/jscomp/FunctionArgumentInjector.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ class FunctionArgumentInjector {
// identifier can be used, so we use "this".
static final String THIS_MARKER = "this";

static final String REST_MARKER = "REST";
static final String REST_MARKER = "rest param";

static final String DEFAULT_MARKER = "Default Value";

private FunctionArgumentInjector() {
// A private constructor to prevent instantiation.
Expand Down Expand Up @@ -138,15 +140,21 @@ static LinkedHashMap<String, Node> getFunctionCallParameterMap(
cArg = cArg.getNext();
}
argMap.put(fnParam.getFirstChild().getString(), array);
return argMap;
} else if (fnParam.isDefaultValue()) {
argMap.put(fnParam.getFirstChild().getString(), cArg);
} else {
argMap.put(fnParam.getString(), cArg);
cArg = cArg.getNext();
}
cArg = cArg.getNext();
} else {
if (fnParam.isRest()) {
//No arguments for REST parameters
Node array = IR.arraylit();
argMap.put(fnParam.getFirstChild().getString(), array);
} else if (fnParam.isDefaultValue()) {
Node defaultValue = fnParam.getSecondChild().cloneTree();
argMap.put(fnParam.getFirstChild().getString(), defaultValue);
} else {
Node srcLocation = callNode;
argMap.put(fnParam.getString(), NodeUtil.newUndefinedNode(srcLocation));
Expand All @@ -158,7 +166,7 @@ static LinkedHashMap<String, Node> getFunctionCallParameterMap(
// called function.
while (cArg != null) {
String uniquePlaceholder =
getUniqueAnonymousParameterName(safeNameIdSupplier);
getUniqueAnonymousParameterName(safeNameIdSupplier);
argMap.put(uniquePlaceholder, cArg);
cArg = cArg.getNext();
}
Expand Down Expand Up @@ -571,6 +579,8 @@ private static Set<String> getFunctionParameterSet(Node fnNode) {
for (Node n : NodeUtil.getFunctionParameters(fnNode).children()) {
if (n.isRest()){
set.add(REST_MARKER);
} else if (n.isDefaultValue()){
set.add(DEFAULT_MARKER);
} else {
set.add(n.getString());
}
Expand Down
92 changes: 92 additions & 0 deletions test/com/google/javascript/jscomp/InlineFunctionsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2599,4 +2599,96 @@ public void disabled_testRestParam1() {
"countArgs(1, 1, 1, 1, 1);"),
"[1, 1, 1, 1, 1].length;");
}

public void testDefaultParam1() {
test(
LINE_JOINER.join(
"function foo(a, b = 1) {",
" return a + b;",
"}",
"foo(1);"),
"1+1");
}

public void testDefaultParam2() {
test(
LINE_JOINER.join(
"function foo(a, b = 1) {",
" return a + b;",
"}",
"foo(1, 2);"),
"1+2");
}

public void testDefaultParam3() {
test(
LINE_JOINER.join(
"function foo(a = 1, b = 2) {",
" return a + b;",
"}",
"foo(3, 4);"),
"3+4");
}

public void testDefaultParam4() {
test(
LINE_JOINER.join(
"function foo(a, b = {foo: 5}) {",
" return a + b.foo;",
"}",
"foo(3, {foo: 9});"),
"{ var b$jscomp$inline_1={foo:9}; 3 + b$jscomp$inline_1.foo; }");
}

public void testDefaultParam5() {
test(
LINE_JOINER.join(
"function foo(a, b = {foo: 5, bar: 6}) {",
" return a + b.foo + b.bar;",
"}",
"foo(3, {foo: 1, bar: 2});"),
"{ var b$jscomp$inline_1={foo:1,bar:2};3+b$jscomp$inline_1.foo"
+ "+b$jscomp$inline_1.bar }");
}

public void testDefaultParam6() {
test(
LINE_JOINER.join(
"function foo(a, b = {foo: 5}) {",
" return a + b.foo;",
"}",
"foo(3);"),
"{ var b$jscomp$inline_1={foo:5};3+b$jscomp$inline_1.foo }");
}

public void testDefaultParam7() {
test(
LINE_JOINER.join(
"function foo(a, b = {foo: 5, bar: 6}) {",
" return a + b.foo + b.bar;",
"}",
"foo(3, {foo: 1});"),
"{ var b$jscomp$inline_1={foo:1};3+b$jscomp$inline_1.foo"
+ "+b$jscomp$inline_1.bar }");
}

public void testDefaultParam8() {
test(
LINE_JOINER.join(
"function foo(a, b = [1, 2]) {",
" return a + b[1];",
"}",
"foo(3, [7, 8]);"),
"{ var b$jscomp$inline_1=[7,8];3+b$jscomp$inline_1[1] }");
}

public void testDefaultParam9() {
test(
LINE_JOINER.join(
"function foo(a, b = []) {",
" return a + b[1];",
"}",
"foo(3, [7, 8]);"),
"{ var b$jscomp$inline_1=[7,8];3+b$jscomp$inline_1[1] }");
}
}
17 changes: 17 additions & 0 deletions test/com/google/javascript/jscomp/IntegrationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4348,6 +4348,23 @@ public void testInlineRestParam() {
+ "=arguments[a];return b[0]}(8))");
}

// TODO(bellashim): Add a non-transpiling version of this test when InlineFunctions
// can run in that mode.
public void testDefaultParameters() {
CompilerOptions options = createCompilerOptions();
CompilationLevel.ADVANCED_OPTIMIZATIONS.setOptionsForCompilationLevel(options);
options.setLanguageIn(LanguageMode.ECMASCRIPT_2017);
options.setLanguageOut(LanguageMode.ECMASCRIPT5);

test(options,
LINE_JOINER.join(
"function foo(a, b = {foo: 5}) {",
" return a + b.foo;",
"}",
"alert(foo(3, {foo: 9}));"),
"var a={a:9},a=void 0===a?{a:5}:a;alert(3+a.a)");
}

/** Creates a CompilerOptions object with google coding conventions. */
@Override
protected CompilerOptions createCompilerOptions() {
Expand Down

0 comments on commit 8909f88

Please sign in to comment.