KT-962: Function is called two times

safecall expression now compute receiver only once.

js/src/com/google/dart/compiler/util/AstUtil.java

@@ -309,6 +309,28 @@ public class AstUtil {
         throw new AssertionError("Set qualifier should be applied only to JsInvocation or JsNameRef instances");
     }
 
+    public static JsExpression getQualifier(JsExpression expression) {
+        if (expression instanceof JsInvocation) {
+            return ((JsInvocation) expression).getQualifier();
+        }
+        if (expression instanceof JsNameRef) {
+            return ((JsNameRef) expression).getQualifier();
+        }
+        throw new AssertionError("Get qualifier should be applied only to JsInvocation or JsNameRef instances");
+    }
+
+    public static void substituteTopLevelQualifier(JsExpression expression, JsExpression substitutor) {
+        assert expression != null;
+        JsExpression previous = expression;
+        JsExpression current = getQualifier(expression);
+        assert current != null : "Cannot be applied to unqualified expression";
+        while (getQualifier(current) != null) {
+            previous = current;
+            current = getQualifier(current);
+        }
+        setQualifier(previous, substitutor);
+    }
+
     //TODO: look for should-be-usages
     public static JsNameRef qualified(JsName selector, JsExpression qualifier) {
         JsNameRef reference = selector.makeRef();

translator/src/org/jetbrains/k2js/translate/context/DynamicContext.java

@@ -5,54 +5,57 @@ import com.google.dart.compiler.util.AstUtil;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor;
 
+//TODO: track that temporary declarations is in the scope of the block where they are declared
+
 public class DynamicContext {
 
     @NotNull
     public static DynamicContext rootContext(@NotNull NamingScope rootScope, @NotNull JsBlock globalBlock) {
-        return new DynamicContext(rootScope, globalBlock);
+        return new DynamicContext(rootScope, rootScope, globalBlock);
     }
 
     @NotNull
     public static DynamicContext contextWithScope(@NotNull NamingScope scope) {
-        return new DynamicContext(scope, new JsBlock());
+        return new DynamicContext(scope, scope, new JsBlock());
     }
 
     @NotNull
-    private NamingScope namingScope;
+    private NamingScope currentScope;
+    @NotNull
+    private NamingScope blockScope;
     @NotNull
     private JsBlock currentBlock;
 
-    private DynamicContext(@NotNull NamingScope scope, @NotNull JsBlock block) {
+    private DynamicContext(@NotNull NamingScope scope, @NotNull NamingScope blockScope, @NotNull JsBlock block) {
-        this.namingScope = scope;
+        this.currentScope = scope;
         this.currentBlock = block;
+        this.blockScope = blockScope;
     }
 
     @NotNull
     public DynamicContext innerScope(@NotNull JsScope scope) {
-        return new DynamicContext(namingScope.innerScope(scope), currentBlock);
+        return new DynamicContext(currentScope.innerScope(scope), blockScope, currentBlock);
     }
 
-    //TODO:
     @NotNull
     public DynamicContext innerBlock(@NotNull JsBlock block) {
-        return new DynamicContext(namingScope, block);
+        return new DynamicContext(currentScope, currentScope, block);
     }
 
     @NotNull
     public JsName getLocalName(@NotNull DeclarationDescriptor descriptor) {
-        JsName name = namingScope.getName(descriptor);
+        JsName name = currentScope.getName(descriptor);
         assert name != null : descriptor.getName() + " is not declared. Use isDeclared to check.";
         return name;
     }
 
     public boolean isDeclared(@NotNull DeclarationDescriptor descriptor) {
-        return namingScope.isDeclared(descriptor);
+        return currentScope.isDeclared(descriptor);
     }
 
     @NotNull
     public TemporaryVariable declareTemporary(@NotNull JsExpression initExpression) {
-
+        JsName temporaryName = blockScope.declareTemporary();
-        JsName temporaryName = namingScope.declareTemporary();
         JsVars temporaryDeclaration = AstUtil.newVar(temporaryName, null);
         jsBlock().addVarDeclaration(temporaryDeclaration);
         return new TemporaryVariable(temporaryName, initExpression);
@@ -60,15 +63,14 @@ public class DynamicContext {
 
     @NotNull
     public JsName declareLocalVariable(@NotNull DeclarationDescriptor descriptor) {
-        return namingScope.declareVariable(descriptor, descriptor.getName());
+        return currentScope.declareVariable(descriptor, descriptor.getName());
     }
 
     @NotNull
     public JsScope jsScope() {
-        return namingScope.jsScope();
+        return currentScope.jsScope();
     }
 
-    //TODO:
     @NotNull
     public JsBlock jsBlock() {
         return currentBlock;

translator/src/org/jetbrains/k2js/translate/declaration/NamespaceTranslator.java

@@ -16,6 +16,7 @@ import java.util.List;
  * @author Pavel.Talanov
  */
 //TODO: rework translator to translate everything in the namespace not only in one file
+// TEST IT
 public final class NamespaceTranslator extends AbstractTranslator {
 
     @NotNull
@@ -66,8 +67,7 @@ public final class NamespaceTranslator extends AbstractTranslator {
         JsInvocation namespaceDeclaration = namespaceCreateMethodInvocation();
         addMemberDeclarations(namespaceDeclaration);
         addClassesDeclarations(namespaceDeclaration);
-        return AstUtil.newAssignmentStatement
+        return AstUtil.newVar(namespaceName, namespaceDeclaration);
-                (namespaceName.makeRef(), namespaceDeclaration);
     }
 
     private void addClassesDeclarations(@NotNull JsInvocation namespaceDeclaration) {

translator/src/org/jetbrains/k2js/translate/expression/ExpressionVisitor.java

@@ -8,6 +8,7 @@ import org.jetbrains.jet.lang.psi.*;
 import org.jetbrains.jet.lang.resolve.BindingContext;
 import org.jetbrains.jet.lang.resolve.constants.CompileTimeConstant;
 import org.jetbrains.jet.lang.resolve.constants.NullValue;
+import org.jetbrains.k2js.translate.context.TemporaryVariable;
 import org.jetbrains.k2js.translate.context.TranslationContext;
 import org.jetbrains.k2js.translate.general.Translation;
 import org.jetbrains.k2js.translate.general.TranslatorVisitor;
@@ -16,7 +17,6 @@ import org.jetbrains.k2js.translate.operation.IncrementTranslator;
 import org.jetbrains.k2js.translate.operation.UnaryOperationTranslator;
 import org.jetbrains.k2js.translate.reference.AccessTranslator;
 import org.jetbrains.k2js.translate.reference.CallTranslator;
-import org.jetbrains.k2js.translate.reference.PropertyAccessTranslator;
 import org.jetbrains.k2js.translate.reference.ReferenceTranslator;
 import org.jetbrains.k2js.translate.utils.BindingUtils;
 
@@ -247,29 +247,23 @@ public final class ExpressionVisitor extends TranslatorVisitor<JsNode> {
         if (expression.getSelectorExpression() instanceof JetCallExpression) {
             return CallTranslator.translate(expression, context);
         }
-        return translateQualifiedExpression(expression, context);
-    }
-
-    @NotNull
-    private JsExpression translateQualifiedExpression(@NotNull JetQualifiedExpression expression,
-                                                      @NotNull TranslationContext context) {
-        if (PropertyAccessTranslator.canBePropertyGetterCall(expression, context)) {
-            return PropertyAccessTranslator.translateAsPropertyGetterCall(expression, context);
-        }
-        return translateAsQualifiedAccess(expression, context);
-    }
-
-    @NotNull
-    private JsExpression translateAsQualifiedAccess(@NotNull JetQualifiedExpression expression,
-                                                    @NotNull TranslationContext context) {
         JsExpression receiver = translateReceiver(expression, context);
         JsExpression selector = translateSelector(expression, context);
-        assert (selector instanceof JsNameRef || selector instanceof JsInvocation)
+        return composeQualifiedExpression(receiver, selector);
+    }
+
+    @NotNull
+    private JsExpression composeQualifiedExpression(@NotNull JsExpression receiver, @NotNull JsExpression selector) {
+        //TODO: make sure that logic would not break for binary operation. check if there is a way to provide clearer logic
+        assert (selector instanceof JsNameRef || selector instanceof JsInvocation || selector instanceof JsBinaryOperation)
                 : "Selector should be a name reference or a method invocation in dot qualified expression.";
         if (selector instanceof JsInvocation) {
             return translateAsQualifiedInvocation(receiver, (JsInvocation) selector);
-        } else {
+        } else if (selector instanceof JsNameRef) {
             return translateAsQualifiedNameReference(receiver, (JsNameRef) selector);
+        } else {
+            ((JsBinaryOperation) selector).setArg1(receiver);
+            return selector;
         }
     }
 
@@ -327,11 +321,13 @@ public final class ExpressionVisitor extends TranslatorVisitor<JsNode> {
     @NotNull
     public JsNode visitSafeQualifiedExpression(@NotNull JetSafeQualifiedExpression expression,
                                                @NotNull TranslationContext context) {
-        JsExpression receiver = translateReceiver(expression, context);
+        TemporaryVariable receiver = context.declareTemporary(translateReceiver(expression, context));
         JsNullLiteral nullLiteral = context.program().getNullLiteral();
-        JsExpression thenExpression = translateQualifiedExpression(expression, context);
+        JsExpression selector = translateSelector(expression, context);
-        return new JsConditional(notNullCheck(context, receiver),
+        JsExpression thenExpression = composeQualifiedExpression(receiver.nameReference(), selector);
-                thenExpression, nullLiteral);
+        JsConditional callMethodIfNotNullConditional
+                = new JsConditional(notNullCheck(context, receiver.nameReference()), thenExpression, nullLiteral);
+        return AstUtil.newSequence(receiver.assignmentExpression(), callMethodIfNotNullConditional);
     }
 
     @Override

translator/src/org/jetbrains/k2js/translate/expression/ForTranslator.java

@@ -37,7 +37,6 @@ public final class ForTranslator extends AbstractTranslator {
         this.expression = forExpression;
     }
 
-
     @NotNull
     private JsBlock translate() {
         JsName parameterName = declareParameter();
@@ -55,9 +54,13 @@ public final class ForTranslator extends AbstractTranslator {
 
     @NotNull
     private JsBlock generateCycleBody(@NotNull JsName parameterName, @NotNull TemporaryVariable iterator) {
-        JsStatement parameterAssignment = AstUtil.newAssignmentStatement(parameterName.makeRef(), nextMethodInvocation(iterator));
+        JsBlock cycleBody = new JsBlock();
-        JsNode originalBody = Translation.translateExpression(getLoopBody(expression), context());
+        JsStatement parameterAssignment =
-        return AstUtil.newBlock(parameterAssignment, AstUtil.convertToBlock(originalBody));
+                AstUtil.newVar(parameterName, nextMethodInvocation(iterator));
+        JsNode originalBody = Translation.translateExpression(getLoopBody(expression), context().innerBlock(cycleBody));
+        cycleBody.addStatement(parameterAssignment);
+        cycleBody.addStatement(AstUtil.convertToBlock(originalBody));
+        return cycleBody;
     }
 
     @NotNull

translator/test/org/jetbrains/k2js/test/MiscTest.java

@@ -26,4 +26,16 @@ public class MiscTest extends AbstractExpressionTest {
         // checkOutput("intRange.kt", " ");
         testFooBoxIsTrue("intRange.kt");
     }
+
+
+    @Test
+    public void safecallComputesExpressionOnlyOnce() throws Exception {
+        testFooBoxIsTrue("safecallComputesExpressionOnlyOnce.kt");
+    }
+
+    @Test
+    public void KT962() throws Exception {
+        checkOutput("KT-962.kt", "stdout\n" +
+                "Hello, world!\n");
+    }
 }

translator/testFiles/expression/function/cases/KT-921.kt

@@ -61,7 +61,7 @@ fun main(args:Array<String>)
     v.view(it, {(itemLifetime, item)->
       x.add(item)
       Dump(x)          
-      itemLifetime.attach { x.remove(item); Dump(x) }
+      itemLifetime.attach { x.remove(item as Any); Dump(x) }
     })
   })
 }

translator/testFiles/expression/misc/cases/KT-962.kt

@@ -0,0 +1,8 @@
+fun stdout(): java.io.PrintStream? {
+  System.out?.println("stdout")
+  return System.out
+}
+
+fun main(args : Array<String>) {
+  stdout()?.println("Hello, world!")
+}

translator/testFiles/expression/misc/cases/safecallComputesExpressionOnlyOnce.kt

@@ -0,0 +1,14 @@
+package foo
+
+var i = 0
+
+fun test() : Int? = i++
+
+fun box() : Boolean {
+    if (i != 0) return false
+    test()?.plus(1)
+    if (i != 1) return false
+    test()?.minus(2)
+    if (i != 2) return false
+    return true
+}