Few fixes to Introduce Variable (no new features added)

Added few tests to Introduce Variable
More strict requirements for expressions to introduce it.

examples/example-vfs/src/Main.kt

@@ -17,7 +17,8 @@ fun main(args : Array<String>) {
     // add watched directory
     FileSystem.write {
         for (arg in args) {
-            FileSystem.addWatchedDirectory(FileSystem.getFileByPath(arg))
+            val virtualFile = FileSystem.getFileByPath(arg)
+            FileSystem.addWatchedDirectory(virtualFile)
         }
     }
 

examples/example-vfs/src/RefreshQueue.kt

@@ -12,6 +12,7 @@ import java.util.Timer
 import java.util.TimerTask
 
 import org.jetbrains.jet.samples.vfs.utils.*
+import org.jetbrains.jet.samples.vfs.utils.listDifference
 
 /**
  * Singleton which creates thread for periodically checking if there are changes in
@@ -55,6 +56,7 @@ internal object RefreshQueue {
             val oldChildren = fileInfo.children
             val newChildren = file.children()
 
+
             val addedChildren = listDifference(newChildren, oldChildren)
             val deletedChildren = listDifference(oldChildren, newChildren)
             val commonChildren = listIntersection(oldChildren, newChildren)

examples/example-vfs/src/Test.kt

@@ -1,5 +1,8 @@
-fun x(x: Int = 1) {}
+fun a(op: (Int) -> Int) {}
-/*
+fun b() {
-val i = 1
+    a {it}
-fun x(x: Int = i) {}
+    a {
-*/
+      it
+    }
+    2 + 2
+}

idea/src/org/jetbrains/jet/plugin/refactoring/JetRefactoringBundle.properties

@@ -1,5 +1,7 @@
-cannot.refactor.not.expression=Cannot refactor not expression
+cannot.refactor.not.expression=Cannot find an expression to introduce
 expressions.title=Expressions
 introduce.variable=Introduce Variable
 cannot.refactor.no.container=Cannot refactor in this place
-cannot.refactor.no.expression=Cannot perform refactoring without an expression
+cannot.refactor.no.expression=Cannot perform refactoring without an expression
+cannot.refactor.expression.has.unit.type=Cannot introduce expression of unit type
+cannot.refactor.namespace.expression=Cannot intoduce namespace reference

idea/src/org/jetbrains/jet/plugin/refactoring/JetRefactoringUtil.java

@@ -13,12 +13,19 @@ import com.intellij.ui.components.JBList;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.jet.lang.psi.*;
+import org.jetbrains.jet.lang.resolve.BindingContext;
+import org.jetbrains.jet.lang.resolve.java.AnalyzerFacade;
+import org.jetbrains.jet.lang.types.JetStandardLibrary;
+import org.jetbrains.jet.lang.types.JetType;
+import org.jetbrains.jet.lang.types.NamespaceType;
+import org.jetbrains.jet.lang.types.checker.JetTypeChecker;
 
 import javax.swing.*;
 import javax.swing.event.ListSelectionEvent;
 import javax.swing.event.ListSelectionListener;
 import java.awt.*;
-import java.util.ArrayList;
+import java.util.*;
+import java.util.List;
 
 /**
  * User: Alefas
@@ -63,7 +70,31 @@ public class JetRefactoringUtil {
         while (element != null && !(element instanceof JetBlockExpression) && !(element instanceof JetNamedFunction)
                && !(element instanceof JetClassBody) && !(element instanceof JetSecondaryConstructor)) {
             if (element instanceof JetExpression && !(element instanceof JetStatementExpression)) {
-                expressions.add((JetExpression) element);
+                boolean addExpression = true;
+                if (element.getParent() instanceof JetQualifiedExpression) {
+                    JetQualifiedExpression qualifiedExpression = (JetQualifiedExpression) element.getParent();
+                    if (qualifiedExpression.getReceiverExpression() != element) {
+                        addExpression = false;
+                    }
+                } else if (element.getParent() instanceof JetCallElement) {
+                    addExpression = false;
+                } else if (element.getParent() instanceof JetOperationExpression) {
+                    JetOperationExpression operationExpression = (JetOperationExpression) element.getParent();
+                    if (operationExpression.getOperationReference() == element) {
+                        addExpression = false;
+                    }
+                }
+                if (addExpression) {
+                    JetExpression expression = (JetExpression) element;
+                    BindingContext bindingContext = AnalyzerFacade.analyzeFileWithCache((JetFile) expression.getContainingFile(),
+                                                                                        AnalyzerFacade.SINGLE_DECLARATION_PROVIDER);
+                    JetType expressionType = bindingContext.get(BindingContext.EXPRESSION_TYPE, expression);
+                    if (expressionType == null || !(expressionType instanceof NamespaceType) &&
+                                                  !JetTypeChecker.INSTANCE.equalTypes(JetStandardLibrary.
+                            getJetStandardLibrary(element.getProject()).getTuple0Type(), expressionType)) {
+                        expressions.add(expression);
+                    }
+                }
             }
             element = element.getParent();
         }
@@ -138,6 +169,14 @@ public class JetRefactoringUtil {
             throw new IntroduceRefactoringException(JetRefactoringBundle.message("cannot.refactor.not.expression"));
         } else if (!(element instanceof JetExpression)) {
             throw new IntroduceRefactoringException(JetRefactoringBundle.message("cannot.refactor.not.expression"));
+        } else if (element instanceof JetBlockExpression) {
+            List<JetElement> statements = ((JetBlockExpression) element).getStatements();
+            if (statements.size() == 1) {
+                JetElement elem = statements.get(0);
+                if (elem.getText().equals(element.getText()) && elem instanceof JetExpression) {
+                    return (JetExpression) elem;
+                }
+            }
         }
         return (JetExpression) element;
     }

idea/src/org/jetbrains/jet/plugin/refactoring/introduceVariable/JetIntroduceVariableHandler.java

@@ -18,14 +18,18 @@ import com.intellij.refactoring.introduce.inplace.OccurrencesChooser;
 import com.intellij.refactoring.util.CommonRefactoringUtil;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
+import org.jetbrains.jet.lang.descriptors.DeclarationDescriptor;
 import org.jetbrains.jet.lang.psi.*;
+import org.jetbrains.jet.lang.resolve.BindingContext;
+import org.jetbrains.jet.lang.resolve.java.AnalyzerFacade;
+import org.jetbrains.jet.lang.types.JetStandardLibrary;
+import org.jetbrains.jet.lang.types.JetType;
+import org.jetbrains.jet.lang.types.NamespaceType;
+import org.jetbrains.jet.lang.types.checker.JetTypeChecker;
 import org.jetbrains.jet.lexer.JetTokens;
 import org.jetbrains.jet.plugin.refactoring.*;
 
-import java.util.ArrayList;
+import java.util.*;
-import java.util.Collections;
-import java.util.LinkedHashSet;
-import java.util.List;
 
 /**
  * User: Alefas
@@ -59,6 +63,33 @@ public class JetIntroduceVariableHandler extends JetIntroduceHandlerBase {
             _expression = (JetExpression) _expression.getParent();
         }
         final JetExpression expression = _expression;
+        if (expression.getParent() instanceof JetQualifiedExpression) {
+            JetQualifiedExpression qualifiedExpression = (JetQualifiedExpression) expression.getParent();
+            if (qualifiedExpression.getReceiverExpression() != expression) {
+                showErrorHint(project, editor, JetRefactoringBundle.message("cannot.refactor.no.expression"));
+                return;
+            }
+        } else if (expression.getParent() instanceof JetCallElement || expression instanceof JetStatementExpression) {
+            showErrorHint(project, editor, JetRefactoringBundle.message("cannot.refactor.no.expression"));
+            return;
+        } else if (expression.getParent() instanceof JetOperationExpression) {
+            JetOperationExpression operationExpression = (JetOperationExpression) expression.getParent();
+            if (operationExpression.getOperationReference() == expression) {
+                showErrorHint(project, editor, JetRefactoringBundle.message("cannot.refactor.no.expression"));
+                return;
+            }
+        }
+        BindingContext bindingContext = AnalyzerFacade.analyzeFileWithCache((JetFile) expression.getContainingFile(),
+                                                                            AnalyzerFacade.SINGLE_DECLARATION_PROVIDER);
+        JetType expressionType = bindingContext.get(BindingContext.EXPRESSION_TYPE, expression); //can be null or error type
+        if (expressionType instanceof NamespaceType) {
+            showErrorHint(project, editor, JetRefactoringBundle.message("cannot.refactor.namespace.expression"));
+            return;
+        } if (expressionType != null &&
+              JetTypeChecker.INSTANCE.equalTypes(JetStandardLibrary.getJetStandardLibrary(project).getTuple0Type(), expressionType)) {
+            showErrorHint(project, editor, JetRefactoringBundle.message("cannot.refactor.expression.has.unit.type"));
+            return;
+        }
         final PsiElement container = getContainer(expression);
         final PsiElement occurrenceContainer = getOccurrenceContainer(expression);
         if (container == null) {
@@ -228,6 +259,9 @@ public class JetIntroduceVariableHandler extends JetIntroduceHandlerBase {
 
         final ArrayList<JetExpression> result = new ArrayList<JetExpression>();
 
+        final BindingContext bindingContext = AnalyzerFacade.analyzeFileWithCache((JetFile) expression.getContainingFile(),
+                                                                            AnalyzerFacade.SINGLE_DECLARATION_PROVIDER);
+
         JetVisitorVoid visitor = new JetVisitorVoid() {
             @Override
             public void visitJetElement(JetElement element) {
@@ -236,8 +270,26 @@ public class JetIntroduceVariableHandler extends JetIntroduceHandlerBase {
             }
 
             @Override
-            public void visitExpression(JetExpression expression) {
+            public void visitExpression(final JetExpression expression) {
-                if (PsiEquivalenceUtil.areElementsEquivalent(expression, actualExpression)) {
+                if (PsiEquivalenceUtil.areElementsEquivalent(expression, actualExpression, null, new Comparator<PsiElement>() {
+                    @Override
+                    public int compare(PsiElement element1, PsiElement element2) {
+                        if (element1.getNode().getElementType() == JetTokens.IDENTIFIER &&
+                            element2.getNode().getElementType() == JetTokens.IDENTIFIER) {
+                            if (element1.getParent() instanceof JetSimpleNameExpression &&
+                                element2.getParent() instanceof  JetSimpleNameExpression) {
+                                JetSimpleNameExpression expr1 = (JetSimpleNameExpression) element1.getParent();
+                                JetSimpleNameExpression expr2 = (JetSimpleNameExpression) element2.getParent();
+                                DeclarationDescriptor descr1 = bindingContext.get(BindingContext.REFERENCE_TARGET, expr1);
+                                DeclarationDescriptor descr2 = bindingContext.get(BindingContext.REFERENCE_TARGET, expr2);
+                                if (descr1 != descr2) return 1;
+                                else return 0;
+                            }
+                        }
+                        if (!element1.textMatches(element2)) return 1;
+                        else return 0;
+                    }
+                }, null, false)) {
                     PsiElement parent = expression.getParent();
                     if (parent instanceof JetParenthesizedExpression) {
                         result.add((JetParenthesizedExpression) parent);
@@ -262,10 +314,9 @@ public class JetIntroduceVariableHandler extends JetIntroduceHandlerBase {
         while (place != null) {
             PsiElement parent = place.getParent();
             if (parent instanceof JetContainerNode) {
-                if (!(parent.getParent() instanceof JetIfExpression &&
+                if (!isBadContainerNode((JetContainerNode) parent, place)) {
-                      ((JetIfExpression) parent.getParent()).getCondition() == place)) {
                     return parent;
-              }
+                }
             } if (parent instanceof JetBlockExpression || parent instanceof JetWhenEntry ||
                 parent instanceof JetClassBody || parent instanceof JetClassInitializer) {
                 return parent;
@@ -284,6 +335,17 @@ public class JetIntroduceVariableHandler extends JetIntroduceHandlerBase {
         }
         return null;
     }
+    
+    private static boolean isBadContainerNode(JetContainerNode parent, PsiElement place) {
+        if (parent.getParent() instanceof JetIfExpression &&
+            ((JetIfExpression) parent.getParent()).getCondition() == place) {
+            return true;
+        } else if (parent.getParent() instanceof JetLoopExpression &&
+                   ((JetLoopExpression) parent.getParent()).getBody() != place) {
+            return true;
+        }
+        return false;
+    }
 
     @Nullable
     private static PsiElement getOccurrenceContainer(PsiElement place) {
@@ -291,8 +353,7 @@ public class JetIntroduceVariableHandler extends JetIntroduceHandlerBase {
         while (place != null) {
             PsiElement parent = place.getParent();
             if (parent instanceof JetContainerNode) {
-                if (!(place instanceof JetBlockExpression) && !(parent.getParent() instanceof JetIfExpression &&
+                if (!(place instanceof JetBlockExpression) && !isBadContainerNode((JetContainerNode) parent, place)) {
-                                                                ((JetIfExpression) parent.getParent()).getCondition() == place)) {
                     result = parent;
                 }
             } else if (parent instanceof JetClassBody || parent instanceof JetFile || parent instanceof JetClassInitializer) {

idea/testData/refactoring/introduceVariable/FewOccurrences.kt

@@ -0,0 +1,13 @@
+fun a() {
+    1
+    1
+    if (true) <selection>1</selection>
+}
+/*
+fun a() {
+    val i = 1
+    i
+    i
+    if (true) i
+}
+*/

idea/testData/refactoring/introduceVariable/It.kt

@@ -0,0 +1,16 @@
+fun a(op: (Int) -> Int) {}
+fun b() {
+    a {it}
+    a {
+        <selection>it</selection>
+    }
+}
+/*
+fun a(op: (Int) -> Int) {}
+fun b() {
+    a {it}
+    a {
+        val i = it
+    }
+}
+*/

idea/testData/refactoring/introduceVariable/LoopRange.kt

@@ -0,0 +1,9 @@
+fun a() {
+    for (a in <selection>1..2</selection>) {}
+}
+/*
+fun a() {
+    val intRange = 1..2
+    for (a in intRange) {}
+}
+*/

idea/testData/refactoring/introduceVariable/ManyInnerOccurences.kt

@@ -0,0 +1,25 @@
+fun a() {
+    if (true) {
+        if (true) {
+            1
+        }
+    } else {
+        if (true) {
+            <selection>1</selection>
+        }
+    }
+}
+/*
+fun a() {
+    val i = 1
+    if (true) {
+        if (true) {
+            i
+        }
+    } else {
+        if (true) {
+            i
+        }
+    }
+}
+*/

idea/testData/refactoring/introduceVariable/ManyOccurrences.kt

@@ -0,0 +1,33 @@
+fun a() {
+    if (true) {
+        <selection>1</selection>
+    }
+    if (true) {
+        1
+    }
+    if (true) {
+        1
+    } else {
+        if (true) {
+            1
+        }
+    }
+}
+/*
+fun a() {
+    val i = 1
+    if (true) {
+        i
+    }
+    if (true) {
+        i
+    }
+    if (true) {
+        i
+    } else {
+        if (true) {
+            i
+        }
+    }
+}
+*/

idea/testData/refactoring/introduceVariable/ReplaceOccurence.kt

@@ -0,0 +1,11 @@
+fun a(x: Int) {}
+fun b() {
+    a(<selection>1</selection>)
+}
+/*
+fun a(x: Int) {}
+fun b() {
+    val i = 1
+    a(i)
+}
+*/

idea/testData/refactoring/introduceVariable/SimpleCreateValue.kt

@@ -0,0 +1,8 @@
+fun a() {
+    <selection>1</selection>
+}
+/*
+fun a() {
+    val i = 1
+}
+*/

idea/testData/refactoring/introduceVariable/StringInjection.kt

@@ -0,0 +1,9 @@
+fun a() {
+    "it's a number ${<selection>1</selection>}"
+}
+/*
+fun a() {
+    val i = 1
+    "it's a number ${i}"
+}
+*/

idea/testData/refactoring/introduceVariable/WhenParts.kt

@@ -0,0 +1,15 @@
+fun a() {
+    when (1) {
+        is 3 -> 1
+        is 4 -> <selection>1</selection>
+    }
+}
+/*
+fun a() {
+    val i = 1
+    when (i) {
+        is 3 -> i
+        is 4 -> i
+    }
+}
+*/

idea/testData/refactoring/introduceVariable/WhileCondition.kt

@@ -0,0 +1,9 @@
+fun a() {
+    while (<selection>true</selection>) {}
+}
+/*
+fun a() {
+    val b = true
+    while (b) {}
+}
+*/

idea/tests/org/jetbrains/jet/plugin/refactoring/introduceVariable/JetIntroduceVariableTest.java

@@ -21,6 +21,10 @@ public class JetIntroduceVariableTest extends LightCodeInsightFixtureTestCase {
         doTest();
     }
 
+    public void testFewOccurrences() {
+        doTest();
+    }
+
     public void testFunctionAddBlock() {
         doTest();
     }
@@ -37,18 +41,54 @@ public class JetIntroduceVariableTest extends LightCodeInsightFixtureTestCase {
         doTest();
     }
 
+    public void testIt() {
+        doTest();
+    }
+
+    /*public void testLoopRange() {
+        doTest();
+    }*/
+
+    public void testManyInnerOccurences() {
+        doTest();
+    }
+
+    public void testManyOccurrences() {
+        doTest();
+    }
+
+    public void testReplaceOccurence() {
+        doTest();
+    }
+
     public void testSimple() {
         doTest();
     }
 
+    public void testSimpleCreateValue() {
+        doTest();
+    }
+
+    public void testStringInjection() {
+        doTest();
+    }
+
     public void testWhenAddBlock() {
         doTest();
     }
 
+    public void testWhenParts() {
+        doTest();
+    }
+
     public void testWhileAddBlock() {
         doTest();
     }
 
+    public void testWhileCondition() {
+        doTest();
+    }
+
     @Override
     protected void setUp() throws Exception {
         super.setUp();