From 58e612c4cd6a7d39ce5c3b1dd54d72af361e4943 Mon Sep 17 00:00:00 2001
From: moz <moz@google.com>
Date: Wed, 2 Nov 2016 15:07:08 -0700
Subject: [PATCH] Move RemoveSuperMethodsPass before OptimizeCalls

OptimizeCalls can change the number of parameters, which breaks the assumption
of RemoveSuperMethodsPass in some cases.

Fixes #2122 on GitHub.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=137999659
---
 .../javascript/jscomp/DefaultPassConfig.java  | 27 ++++++++++----
 .../javascript/jscomp/IntegrationTest.java    | 37 +++++++++++++++++++
 2 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/src/com/google/javascript/jscomp/DefaultPassConfig.java b/src/com/google/javascript/jscomp/DefaultPassConfig.java
index 9de47c6b480..54eadef9e22 100644
--- a/src/com/google/javascript/jscomp/DefaultPassConfig.java
+++ b/src/com/google/javascript/jscomp/DefaultPassConfig.java
@@ -222,7 +222,7 @@ protected List<PassFactory> getTranspileOnlyPasses() {
     }
 
     assertAllOneTimePasses(passes);
-    assertValidOrder(passes);
+    assertValidOrderForChecks(passes);
     return passes;
   }
 
@@ -422,7 +422,7 @@ protected List<PassFactory> getChecks() {
     checks.add(createEmptyPass("afterStandardChecks"));
 
     assertAllOneTimePasses(checks);
-    assertValidOrder(checks);
+    assertValidOrderForChecks(checks);
 
     return checks;
   }
@@ -731,14 +731,15 @@ protected List<PassFactory> getOptimizations() {
       passes.add(flowSensitiveInlineVariables);
     }
 
-    passes.addAll(getMainOptimizationLoop());
-    passes.add(createEmptyPass("afterMainOptimizations"));
-
-    // Must run after ProcessClosurePrimitives, Es6ConvertSuper, and assertion removals.
+    // Must run after ProcessClosurePrimitives, Es6ConvertSuper, and assertion removals, but
+    // before OptimizeCalls (specifically, OptimizeParameters).
     if (options.removeSuperMethods) {
       passes.add(removeSuperMethodsPass);
     }
 
+    passes.addAll(getMainOptimizationLoop());
+    passes.add(createEmptyPass("afterMainOptimizations"));
+
     passes.add(createEmptyPass("beforeModuleMotion"));
 
     if (options.crossModuleCodeMotion) {
@@ -912,6 +913,7 @@ protected List<PassFactory> getOptimizations() {
       passes.add(rewriteBindThis);
     }
 
+    assertValidOrderForOptimizations(passes);
     return passes;
   }
 
@@ -1086,7 +1088,7 @@ private void assertPassOrder(
    * This enforces those constraints.
    * @param checks The list of check passes
    */
-  private void assertValidOrder(List<PassFactory> checks) {
+  private void assertValidOrderForChecks(List<PassFactory> checks) {
     assertPassOrder(
         checks,
         closureRewriteModule,
@@ -1147,6 +1149,17 @@ private void assertValidOrder(List<PassFactory> checks) {
             + "removing assertions may make more super calls eligible to be stripped.");
   }
 
+  /**
+   * Certain optimizations need to run in a particular order. For example, OptimizeCalls must run
+   * before RemoveSuperMethodsPass, because the former can invalidate assumptions in the latter.
+   * This enforces those constraints.
+   * @param optimizations The list of optimization passes
+   */
+  private void assertValidOrderForOptimizations(List<PassFactory> optimizations) {
+    assertPassOrder(optimizations, removeSuperMethodsPass, optimizeCalls,
+        "RemoveSuperMethodsPass must run before OptimizeCalls");
+  }
+
   /** Checks that all constructed classes are goog.require()d. */
   private final HotSwapPassFactory checkRequires =
       new HotSwapPassFactory("checkRequires", true) {
diff --git a/test/com/google/javascript/jscomp/IntegrationTest.java b/test/com/google/javascript/jscomp/IntegrationTest.java
index 41612100335..bcc32137d2f 100644
--- a/test/com/google/javascript/jscomp/IntegrationTest.java
+++ b/test/com/google/javascript/jscomp/IntegrationTest.java
@@ -3734,6 +3734,43 @@ public void testStrictEqualityComparisonAgainstUndefined() {
             "null===window['c']&&(window['d']=12)");
   }
 
+  // GitHub issue #2122: https://github.com/google/closure-compiler/issues/2122
+  public void testNoRemoveSuperCallWithWrongArgumentCount() {
+    CompilerOptions options = createCompilerOptions();
+    options.setCheckTypes(true);
+    CompilationLevel.ADVANCED_OPTIMIZATIONS.setOptionsForCompilationLevel(options);
+    test(options,
+        LINE_JOINER.join(
+            "var goog = {}",
+            "goog.inherits = function(childCtor, parentCtor) {",
+            "  childCtor.superClass_ = parentCtor.prototype;",
+            "};",
+            "/** @constructor */",
+            "var Foo = function() {}",
+            "/**",
+            " * @param {number=} width",
+            " * @param {number=} height",
+            " */",
+            "Foo.prototype.resize = function(width, height) {",
+            "  window.size = width * height;",
+            "}",
+            "/**",
+            " * @constructor",
+            " * @extends {Foo}",
+            " */",
+            "var Bar = function() {}",
+            "goog.inherits(Bar, Foo);",
+            "/** @override */",
+            "Bar.prototype.resize = function(width, height) {",
+            "  goog.base(this, 'resize', width);",
+            "};",
+            "(new Bar).resize(100, 200);"),
+        LINE_JOINER.join(
+            "function a(){}a.prototype.a=function(b,e){window.c=b*e};",
+            "function d(){}d.b=a.prototype;d.prototype.a=function(b){d.b.a.call(this,b)};",
+            "(new d).a(100, 200);"));
+  }
+
   /** Creates a CompilerOptions object with google coding conventions. */
   @Override
   protected CompilerOptions createCompilerOptions() {