JS: when both clauses of if become empty during optimization, remove if entirely. Make condition and then clause of JsIf non-nullable. Fix #KT-13912

Author: Alexey Andreev

js/js.dart-ast/src/com/google/dart/compiler/backend/js/ast/JsIf.java

@@ -6,50 +6,55 @@
 
 import com.google.dart.compiler.util.AstUtil;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 /**
  * Represents a JavaScript if statement.
  */
 public final class JsIf extends SourceInfoAwareJsNode implements JsStatement {
+    @NotNull
     private JsExpression ifExpression;
+
+    @NotNull
     private JsStatement thenStatement;
-    private JsStatement elseStatement;
 
-    public JsIf() {
-    }
+    @Nullable
+    private JsStatement elseStatement;
 
-    public JsIf(JsExpression ifExpression, JsStatement thenStatement, JsStatement elseStatement) {
+    public JsIf(@NotNull JsExpression ifExpression, @NotNull JsStatement thenStatement, @Nullable JsStatement elseStatement) {
         this.ifExpression = ifExpression;
         this.thenStatement = thenStatement;
         this.elseStatement = elseStatement;
     }
 
-    public JsIf(JsExpression ifExpression, JsStatement thenStatement) {
-        this.ifExpression = ifExpression;
-        this.thenStatement = thenStatement;
+    public JsIf(@NotNull JsExpression ifExpression, @NotNull JsStatement thenStatement) {
+        this(ifExpression, thenStatement, null);
     }
 
+    @Nullable
     public JsStatement getElseStatement() {
         return elseStatement;
     }
 
+    @NotNull
     public JsExpression getIfExpression() {
         return ifExpression;
     }
 
+    @NotNull
     public JsStatement getThenStatement() {
         return thenStatement;
     }
 
-    public void setElseStatement(JsStatement elseStatement) {
+    public void setElseStatement(@Nullable JsStatement elseStatement) {
         this.elseStatement = elseStatement;
     }
 
-    public void setIfExpression(JsExpression ifExpression) {
+    public void setIfExpression(@NotNull JsExpression ifExpression) {
         this.ifExpression = ifExpression;
     }
 
-    public void setThenStatement(JsStatement thenStatement) {
+    public void setThenStatement(@NotNull JsStatement thenStatement) {
         this.thenStatement = thenStatement;
     }
 

js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/RedundantLabelRemoval.kt

@@ -100,17 +100,21 @@ internal class RedundantLabelRemoval(private val root: JsStatement) {
             }
         is JsIf -> {
             val thenRemoved = perform(statement.thenStatement, name) == null
-            val elseRemoved = statement.elseStatement?.let { perform(it, name) == null } ?: false
+            val elseStatement = statement.elseStatement
+            val elseRemoved = elseStatement?.let { perform(it, name) == null } ?: false
             when {
-                thenRemoved && elseRemoved -> null
+                thenRemoved && (elseRemoved || elseStatement == null) -> {
+                    hasChanges = true
+                    JsAstUtils.asSyntheticStatement(statement.ifExpression)
+                }
                 elseRemoved -> {
                     hasChanges = true
                     statement.elseStatement = null
                     statement
                 }
                 thenRemoved -> {
                     hasChanges = true
-                    statement.thenStatement = statement.elseStatement
+                    statement.thenStatement = elseStatement ?: JsEmpty
                     statement.elseStatement = null
                     statement.ifExpression = JsAstUtils.not(statement.ifExpression)
                     statement

js/js.parser/src/com/google/gwt/dev/js/JsAstMapper.java

@@ -630,16 +630,7 @@ private JsIf mapIfStatement(Node ifNode) throws JsParserException {
 
         // Create the "if" statement we're mapping to.
         //
-        JsIf toIf = new JsIf();
-
-        // Map the test expression.
-        //
-        JsExpression toTestExpr = mapExpression(fromTestExpr);
-        toIf.setIfExpression(toTestExpr);
-
-        // Map the "then" block.
-        //
-        toIf.setThenStatement(mapStatement(fromThenBlock));
+        JsIf toIf = new JsIf(mapExpression(fromTestExpr), mapStatement(fromThenBlock));
 
         // Map the "else" block.
         //

js/js.tests/test/org/jetbrains/kotlin/js/test/optimizer/BasicOptimizerTest.kt

@@ -108,6 +108,11 @@ abstract class BasicOptimizerTest(private var basePath: String) {
                     }
                     super.visitIf(x)
                 }
+
+                override fun visitLabel(x: JsLabel) {
+                    x.synthetic = isSyntheticId(x.name.ident)
+                    super.visitLabel(x)
+                }
             }.accept(stmt)
         }
     }
@@ -119,7 +124,6 @@ abstract class BasicOptimizerTest(private var basePath: String) {
 
     private fun isSyntheticId(id: String) = id.startsWith("$")
 
-
     private fun astToString(ast: List<JsStatement>): String {
         val output = TextOutputImpl()
         val visitor = JsSourceGenerationVisitor(output, null)

js/js.tests/test/org/jetbrains/kotlin/js/test/optimizer/RedundantLabelRemovalTest.kt

@@ -0,0 +1,25 @@
+/*
+ * Copyright 2010-2016 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.jetbrains.kotlin.js.test.optimizer
+
+import org.junit.Test
+
+class RedundantLabelRemovalTest : BasicOptimizerTest("redundant-label-removal") {
+    @Test fun emptyIfConditionPreserved() = box()
+
+    @Test fun ifWithEmptyThenAndNoElse() = box()
+}

js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/ExpressionVisitor.java

@@ -285,6 +285,9 @@ public JsNode visitIfExpression(@NotNull KtIfExpression expression, @NotNull Tra
                 return new JsConditional(testExpression, jsThenExpression, jsElseExpression).source(expression);
             }
         }
+        if (thenStatement == null) {
+            thenStatement = JsEmpty.INSTANCE;
+        }
         JsIf ifStatement = new JsIf(testExpression, thenStatement, elseStatement);
         return ifStatement.source(expression);
     }

js/js.translator/testData/js-optimizer/redundant-label-removal/emptyIfConditionPreserved.optimized.js

@@ -0,0 +1,17 @@
+var global = "";
+
+function id(value) {
+    global += value + ";";
+    return value;
+}
+
+function test(x) {
+    id(x);
+    id(x + 1);
+}
+
+function box() {
+    test(23);
+    if (global != "23;24;") return "fail";
+    return "OK";
+}

js/js.translator/testData/js-optimizer/redundant-label-removal/emptyIfConditionPreserved.original.js

@@ -0,0 +1,23 @@
+var global = "";
+
+function id(value) {
+    global += value + ";";
+    return value;
+}
+
+function test(x) {
+    $outer: {
+        if (id(x) + id(x + 1) > 0) {
+            break $outer;
+        }
+        else {
+            break $outer;
+        }
+    }
+}
+
+function box() {
+    test(23);
+    if (global != "23;24;") return "fail";
+    return "OK";
+}

js/js.translator/testData/js-optimizer/redundant-label-removal/ifWithEmptyThenAndNoElse.optimized.js

@@ -0,0 +1,7 @@
+function test(x) {
+    return "OK";
+}
+
+function box() {
+    return test(23);
+}

js/js.translator/testData/js-optimizer/redundant-label-removal/ifWithEmptyThenAndNoElse.original.js

@@ -0,0 +1,12 @@
+function test(x) {
+    $outer: {
+        if (x > 10) {
+            break $outer;
+        }
+    }
+    return "OK";
+}
+
+function box() {
+    return test(23);
+}

js/js.translator/testData/sourcemap/cases/emptyIfInsideInlineLambda.kt

@@ -0,0 +1,19 @@
+package foo
+
+fun test1(): String {
+    run {
+        if (false) {
+        }
+    }
+    return "O"
+}
+
+fun test2(): String {
+    1.let {
+        if (false) {
+        }
+    }
+    return "K"
+}
+
+fun box() = test1() + test2()