From 7a2321460d66677e340056a5bfa05f6ac5653673 Mon Sep 17 00:00:00 2001 From: Tuomas Tynkkynen Date: Thu, 6 Feb 2014 09:56:51 -0800 Subject: [PATCH] Prevent expression selector from selecting just a label Previously, extract variable could act on e.g. 'break@outerLoop' and extract the label, leading into nonsensical 'val x = @outerLoop'. Prevent JetRefactoringUtil.selectExpression from returning label expressions to fix this. #KT-4515 Fixed --- .../src/org/jetbrains/jet/lang/psi/JetPsiUtil.java | 5 +++++ .../jet/plugin/codeInsight/CodeInsightUtils.java | 13 ++++++++----- .../jet/plugin/refactoring/JetRefactoringUtil.java | 6 +++++- .../expressionSelection/labelledStatement.kt | 7 +++++++ idea/testData/expressionSelection/labelledThis.kt | 6 ++++++ idea/testData/smartSelection/labelledStatement.kt | 7 +++++++ .../plugin/ExpressionSelectionTestGenerated.java | 10 ++++++++++ .../jet/plugin/SmartSelectionTestGenerated.java | 5 +++++ 8 files changed, 53 insertions(+), 6 deletions(-) create mode 100644 idea/testData/expressionSelection/labelledStatement.kt create mode 100644 idea/testData/expressionSelection/labelledThis.kt create mode 100644 idea/testData/smartSelection/labelledStatement.kt diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/psi/JetPsiUtil.java b/compiler/frontend/src/org/jetbrains/jet/lang/psi/JetPsiUtil.java index e307ca503ac..488acfe1946 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/psi/JetPsiUtil.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/psi/JetPsiUtil.java @@ -1042,4 +1042,9 @@ public class JetPsiUtil { expression; return (JetToken) elementType; } + + public static boolean isLabelIdentifierExpression(PsiElement element) { + return element instanceof JetSimpleNameExpression && + ((JetSimpleNameExpression) element).getReferencedNameElementType() == JetTokens.LABEL_IDENTIFIER; + } } diff --git a/idea/src/org/jetbrains/jet/plugin/codeInsight/CodeInsightUtils.java b/idea/src/org/jetbrains/jet/plugin/codeInsight/CodeInsightUtils.java index c90e6d225ce..5bd877608b2 100644 --- a/idea/src/org/jetbrains/jet/plugin/codeInsight/CodeInsightUtils.java +++ b/idea/src/org/jetbrains/jet/plugin/codeInsight/CodeInsightUtils.java @@ -13,10 +13,7 @@ import com.intellij.refactoring.util.CommonRefactoringUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.jet.lang.descriptors.FunctionDescriptor; -import org.jetbrains.jet.lang.psi.JetBlockExpression; -import org.jetbrains.jet.lang.psi.JetElement; -import org.jetbrains.jet.lang.psi.JetExpression; -import org.jetbrains.jet.lang.psi.JetFile; +import org.jetbrains.jet.lang.psi.*; import org.jetbrains.jet.lang.types.JetType; import org.jetbrains.jet.lang.types.lang.KotlinBuiltIns; import org.jetbrains.jet.renderer.DescriptorRenderer; @@ -35,7 +32,13 @@ public class CodeInsightUtils { else if (!(element instanceof JetExpression)) { return null; } - else if (element instanceof JetBlockExpression) { + + // For cases like 'this@outerClass', don't return the label part + if (JetPsiUtil.isLabelIdentifierExpression(element)) { + element = PsiTreeUtil.getParentOfType(element, JetExpression.class); + } + + if (element instanceof JetBlockExpression) { List statements = ((JetBlockExpression) element).getStatements(); if (statements.size() == 1) { JetElement elem = statements.get(0); diff --git a/idea/src/org/jetbrains/jet/plugin/refactoring/JetRefactoringUtil.java b/idea/src/org/jetbrains/jet/plugin/refactoring/JetRefactoringUtil.java index aac35c44a27..d09a5f6aa8d 100644 --- a/idea/src/org/jetbrains/jet/plugin/refactoring/JetRefactoringUtil.java +++ b/idea/src/org/jetbrains/jet/plugin/refactoring/JetRefactoringUtil.java @@ -405,7 +405,11 @@ public class JetRefactoringUtil { && !(element instanceof JetClassBody)) { if (element instanceof JetExpression && !(element instanceof JetStatementExpression)) { boolean addExpression = true; - if (element.getParent() instanceof JetQualifiedExpression) { + + if (JetPsiUtil.isLabelIdentifierExpression(element)) { + addExpression = false; + } + else if (element.getParent() instanceof JetQualifiedExpression) { JetQualifiedExpression qualifiedExpression = (JetQualifiedExpression) element.getParent(); if (qualifiedExpression.getReceiverExpression() != element) { addExpression = false; diff --git a/idea/testData/expressionSelection/labelledStatement.kt b/idea/testData/expressionSelection/labelledStatement.kt new file mode 100644 index 00000000000..5f6cecbed51 --- /dev/null +++ b/idea/testData/expressionSelection/labelledStatement.kt @@ -0,0 +1,7 @@ +fun f() { + @loop while (true) { + break@loop + } +} +// KT-4515: Extract variable can attempt to extract a label from a labelled statement. +/* break@loop */ diff --git a/idea/testData/expressionSelection/labelledThis.kt b/idea/testData/expressionSelection/labelledThis.kt new file mode 100644 index 00000000000..a097f8e5d15 --- /dev/null +++ b/idea/testData/expressionSelection/labelledThis.kt @@ -0,0 +1,6 @@ +class Foo() { + val s = this@Foo.toString() +} + +// KT-4515: Extract variable can attempt to extract a label from a labelled statement. +/* this@Foo */ diff --git a/idea/testData/smartSelection/labelledStatement.kt b/idea/testData/smartSelection/labelledStatement.kt new file mode 100644 index 00000000000..ff436e1ce45 --- /dev/null +++ b/idea/testData/smartSelection/labelledStatement.kt @@ -0,0 +1,7 @@ +fun f() { + @loop while (true) { + break@loop + } +} +// KT-4515: Extract variable can attempt to extract a label from a labelled statement. +/* */ diff --git a/idea/tests/org/jetbrains/jet/plugin/ExpressionSelectionTestGenerated.java b/idea/tests/org/jetbrains/jet/plugin/ExpressionSelectionTestGenerated.java index fedf61d0ca9..cf3de9887d5 100644 --- a/idea/tests/org/jetbrains/jet/plugin/ExpressionSelectionTestGenerated.java +++ b/idea/tests/org/jetbrains/jet/plugin/ExpressionSelectionTestGenerated.java @@ -41,6 +41,16 @@ public class ExpressionSelectionTestGenerated extends AbstractExpressionSelectio doTestExpressionSelection("idea/testData/expressionSelection/binaryExpr.kt"); } + @TestMetadata("labelledStatement.kt") + public void testLabelledStatement() throws Exception { + doTestExpressionSelection("idea/testData/expressionSelection/labelledStatement.kt"); + } + + @TestMetadata("labelledThis.kt") + public void testLabelledThis() throws Exception { + doTestExpressionSelection("idea/testData/expressionSelection/labelledThis.kt"); + } + @TestMetadata("noExpression.kt") public void testNoExpression() throws Exception { doTestExpressionSelection("idea/testData/expressionSelection/noExpression.kt"); diff --git a/idea/tests/org/jetbrains/jet/plugin/SmartSelectionTestGenerated.java b/idea/tests/org/jetbrains/jet/plugin/SmartSelectionTestGenerated.java index 41a9775db8a..b09014eed0d 100644 --- a/idea/tests/org/jetbrains/jet/plugin/SmartSelectionTestGenerated.java +++ b/idea/tests/org/jetbrains/jet/plugin/SmartSelectionTestGenerated.java @@ -36,6 +36,11 @@ public class SmartSelectionTestGenerated extends AbstractSmartSelectionTest { JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.generators.tests.TestsPackage", new File("idea/testData/smartSelection"), Pattern.compile("^([^\\.]+)\\.kt$"), true); } + @TestMetadata("labelledStatement.kt") + public void testLabelledStatement() throws Exception { + doTestSmartSelection("idea/testData/smartSelection/labelledStatement.kt"); + } + @TestMetadata("labelledThis.kt") public void testLabelledThis() throws Exception { doTestSmartSelection("idea/testData/smartSelection/labelledThis.kt");