diff --git a/python/src/com/jetbrains/python/refactoring/introduce/parameter/PyIntroduceParameterHandler.java b/python/src/com/jetbrains/python/refactoring/introduce/parameter/PyIntroduceParameterHandler.java index 0148426ac4d3..e72aca56ec5c 100644 --- a/python/src/com/jetbrains/python/refactoring/introduce/parameter/PyIntroduceParameterHandler.java +++ b/python/src/com/jetbrains/python/refactoring/introduce/parameter/PyIntroduceParameterHandler.java @@ -2,6 +2,7 @@ package com.jetbrains.python.refactoring.introduce.parameter; import com.intellij.codeInsight.CodeInsightUtilBase; import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiNamedElement; import com.intellij.psi.PsiReference; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.refactoring.introduce.inplace.InplaceVariableIntroducer; @@ -61,32 +62,55 @@ public class PyIntroduceParameterHandler extends IntroduceHandler { protected boolean isValidIntroduceContext(PsiElement element) { if (element != null) { - final PyFunction function = PsiTreeUtil.getParentOfType(element, PyFunction.class); - final ScopeOwner scopeOwner = ScopeUtil.getScopeOwner(element); - final boolean[] isValid = {true}; - if (scopeOwner != null) { - new PyRecursiveElementVisitor() { - @Override - public void visitPyReferenceExpression(PyReferenceExpression node) { - super.visitPyReferenceExpression(node); - if (ControlFlowCache.getScope(scopeOwner).containsDeclaration(node.getName())) { - isValid[0] = false; - } - } - }.visitElement(element); - } - final PyStatement nonlocalStatement = - PsiTreeUtil.getParentOfType(element, PyNonlocalStatement.class, PyGlobalStatement.class); - final PyStatementList statementList = - PsiTreeUtil.getParentOfType(element, PyStatementList.class); - PyImportStatement importStatement = PsiTreeUtil.getParentOfType(element, PyImportStatement.class); - return function != null && !isResolvedToParameter(element) && isValid[0] && nonlocalStatement == null && - statementList != null && importStatement == null; + if (!isValidPlace(element)) return false; + + return isNotDeclared(element); } return false; } - private boolean isResolvedToParameter(PsiElement element) { + private static boolean isNotDeclared(PsiElement element) { + final ScopeOwner scopeOwner = ScopeUtil.getScopeOwner(element); + final boolean[] isValid = {true}; + + if (scopeOwner != null) { + final String name = element instanceof PsiNamedElement? ((PsiNamedElement)element).getName() : element.getText(); + if (name != null && ControlFlowCache.getScope(scopeOwner).containsDeclaration(name)) { + return false; + } + new PyRecursiveElementVisitor() { + @Override + public void visitPyReferenceExpression(PyReferenceExpression node) { + super.visitPyReferenceExpression(node); + + final String name = node.getName(); + if (name != null && ControlFlowCache.getScope(scopeOwner).containsDeclaration(name)) { + isValid[0] = false; + } + } + }.visitElement(element); + } + return !isResolvedToParameter(element) && isValid[0]; + } + + private static boolean isValidPlace(PsiElement element) { + final PyFunction function = PsiTreeUtil.getParentOfType(element, PyFunction.class); + final PyForPart forPart = PsiTreeUtil.getParentOfType(element, PyForPart.class); + if (forPart != null) { + final PyExpression target = forPart.getTarget(); + if (target instanceof PyTargetExpression && element.getText().equals(target.getName())) + return false; + } + final PyStatement nonlocalStatement = + PsiTreeUtil.getParentOfType(element, PyNonlocalStatement.class, PyGlobalStatement.class); + final PyStatementList statementList = + PsiTreeUtil.getParentOfType(element, PyStatementList.class); + PyImportStatement importStatement = PsiTreeUtil.getParentOfType(element, PyImportStatement.class); + return nonlocalStatement == null && importStatement == null && + statementList != null && function != null; + } + + private static boolean isResolvedToParameter(PsiElement element) { while (element instanceof PyReferenceExpression) { final PsiReference reference = element.getReference(); if (reference != null && reference.resolve() instanceof PyNamedParameter) diff --git a/python/testData/refactoring/introduceParameter/functionDef.py b/python/testData/refactoring/introduceParameter/functionDef.py new file mode 100644 index 000000000000..61262d42427b --- /dev/null +++ b/python/testData/refactoring/introduceParameter/functionDef.py @@ -0,0 +1,2 @@ +def f(b="myValue"): + pass \ No newline at end of file diff --git a/python/testData/refactoring/introduceParameter/global.py b/python/testData/refactoring/introduceParameter/global.py new file mode 100644 index 000000000000..ac3caa0898a8 --- /dev/null +++ b/python/testData/refactoring/introduceParameter/global.py @@ -0,0 +1,4 @@ +def f2(): + param = 1 + def f(): + global param \ No newline at end of file diff --git a/python/testData/refactoring/introduceParameter/localVariable1.py b/python/testData/refactoring/introduceParameter/localVariable1.py new file mode 100644 index 000000000000..a1c0d883a1af --- /dev/null +++ b/python/testData/refactoring/introduceParameter/localVariable1.py @@ -0,0 +1,3 @@ +def f(): + a = 1 + print a + 3 \ No newline at end of file diff --git a/python/testData/refactoring/introduceParameter/localVariableInFor.py b/python/testData/refactoring/introduceParameter/localVariableInFor.py new file mode 100644 index 000000000000..4b92eea3da06 --- /dev/null +++ b/python/testData/refactoring/introduceParameter/localVariableInFor.py @@ -0,0 +1,2 @@ +for item in range(100): + pass \ No newline at end of file diff --git a/python/testData/refactoring/introduceParameter/localVariableParam.py b/python/testData/refactoring/introduceParameter/localVariableParam.py new file mode 100644 index 000000000000..67011a83d668 --- /dev/null +++ b/python/testData/refactoring/introduceParameter/localVariableParam.py @@ -0,0 +1,2 @@ +def f(b=1): + return b + 1 \ No newline at end of file diff --git a/python/testData/refactoring/introduceParameter/nonLocal.py b/python/testData/refactoring/introduceParameter/nonLocal.py new file mode 100644 index 000000000000..1f42e3c56360 --- /dev/null +++ b/python/testData/refactoring/introduceParameter/nonLocal.py @@ -0,0 +1,4 @@ +def f2(): + param = 1 + def f(): + nonlocal param \ No newline at end of file diff --git a/python/testSrc/com/jetbrains/python/refactoring/PyIntroduceParameterTest.java b/python/testSrc/com/jetbrains/python/refactoring/PyIntroduceParameterTest.java index 19047fc12fea..b31b0fb87048 100644 --- a/python/testSrc/com/jetbrains/python/refactoring/PyIntroduceParameterTest.java +++ b/python/testSrc/com/jetbrains/python/refactoring/PyIntroduceParameterTest.java @@ -3,6 +3,7 @@ package com.jetbrains.python.refactoring; import com.intellij.refactoring.util.CommonRefactoringUtil; import com.intellij.testFramework.TestDataPath; import com.jetbrains.python.PyBundle; +import com.jetbrains.python.psi.LanguageLevel; import com.jetbrains.python.refactoring.introduce.IntroduceHandler; import com.jetbrains.python.refactoring.introduce.parameter.PyIntroduceParameterHandler; @@ -35,6 +36,27 @@ public class PyIntroduceParameterTest extends PyIntroduceTestCase { doTestCannotPerform(PyBundle.message("refactoring.introduce.selection.error")); } + public void testLocalVariable1() { + doTestCannotPerform(PyBundle.message("refactoring.introduce.selection.error")); + } + + public void testLocalVariableParam() { + doTestCannotPerform(PyBundle.message("refactoring.introduce.selection.error")); + } + + public void testNonLocal() { + doTestCannotPerform(PyBundle.message("refactoring.introduce.selection.error"), LanguageLevel.PYTHON32); + } + + public void testGlobal() { + doTestCannotPerform(PyBundle.message("refactoring.introduce.selection.error")); + } + + public void testFunctionDef() { + doTestCannotPerform(PyBundle.message("refactoring.introduce.selection.error")); + } + + @Override protected String getTestDataPath() { return super.getTestDataPath() + "/refactoring/introduceParameter"; @@ -52,4 +74,14 @@ public class PyIntroduceParameterTest extends PyIntroduceTestCase { assertEquals(expected, e.getMessage()); } } + + private void doTestCannotPerform(String expected, LanguageLevel languageLevel) { + setLanguageLevel(languageLevel); + try { + doTestCannotPerform(expected); + } + finally { + setLanguageLevel(null); + } + } }