Fix test of conditional operator intention

conditional_operator_intention/conditional_operator_intention.iml

@@ -1,11 +1,12 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <module type="PLUGIN_MODULE" version="4">
-  <component name="DevKit.ModuleBuildProperties" url="file://$MODULE_DIR$/source/META-INF/plugin.xml" />
+  <component name="DevKit.ModuleBuildProperties" url="file://$MODULE_DIR$/META-INF/plugin.xml" />
   <component name="NewModuleRootManager" inherit-compiler-output="true">
     <exclude-output />
     <content url="file://$MODULE_DIR$">
       <sourceFolder url="file://$MODULE_DIR$/source" isTestSource="false" />
       <sourceFolder url="file://$MODULE_DIR$/testSource" isTestSource="true" />
+      <sourceFolder url="file://$MODULE_DIR$/testData" type="java-test-resource" />
     </content>
     <orderEntry type="inheritedJdk" />
     <orderEntry type="sourceFolder" forTests="false" />

conditional_operator_intention/source/META-INF/plugin.xml

@@ -1,35 +0,0 @@
-<idea-plugin>
-  <id>com.your.company.unique.plugin.id</id>
-  <name>Plugin display name here</name>
-  <version>1.0</version>
-  <vendor email="support@yourcompany.com" url="http://www.yourcompany.com">YourCompany</vendor>
-
-  <description><![CDATA[
-      Enter short description for your plugin here.<br>
-      <em>most HTML tags may be used</em>
-    ]]></description>
-
-  <change-notes><![CDATA[
-      Add change notes here.<br>
-      <em>most HTML tags may be used</em>
-    ]]>
-  </change-notes>
-
-  <!-- please see http://www.jetbrains.org/intellij/sdk/docs/basics/getting_started/build_number_ranges.html for description -->
-  <idea-version since-build="141.0"/>
-
-  <!-- please see http://www.jetbrains.org/intellij/sdk/docs/basics/getting_started/plugin_compatibility.html
-       on how to target different products -->
-  <!-- uncomment to enable plugin in all products
-  <depends>com.intellij.modules.lang</depends>
-  -->
-
-  <extensions defaultExtensionNs="com.intellij">
-    <!-- Add your extensions here -->
-  </extensions>
-
-  <actions>
-    <!-- Add your actions here -->
-  </actions>
-
-</idea-plugin>

conditional_operator_intention/source/com/intellij/codeInsight/intention/ConditionalOperatorConvertor.java

@@ -1,3 +1,7 @@
+/*
+ * Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+ */
+
 package com.intellij.codeInsight.intention;
 
 import com.intellij.openapi.editor.Editor;
@@ -9,59 +13,115 @@ import com.intellij.util.IncorrectOperationException;
 import org.jetbrains.annotations.*;
 
 /**
+ * Implements an intention action to replace a ternary statement with if-then-else
+ *
  * @author dsl
  */
 @NonNls
 public class ConditionalOperatorConvertor extends PsiElementBaseIntentionAction implements IntentionAction {
 
+  /**
+   * If this action is applicable, returns the text to be shown in the list of
+   * intention actions available.
+   */
   @NotNull
   public String getText() {
     return "Convert ternary operator to if statement";
   }
 
+
+  /**
+   * Returns text for name of this family of intentions. It is used to externalize
+   * "auto-show" state of intentions. Only one intention action is being provided,
+   * so the family name is the same text as the intention action list entry.
+   *
+   * @return the intention family name.
+   * @see ConditionalOperatorConvertor#getText()
+   */
   @NotNull
   public String getFamilyName() {
     return getText();
   }
 
-  public boolean isAvailable(@NotNull Project project, Editor editor, @Nullable PsiElement element) {
-    if (element == null) return false;
-    if (!element.isWritable()) return false;
 
+  /**
+   * Checks whether this intention is available at the caret offset in file - the caret
+   * must sit just before a "?" character in a ternary statement. If this condition is met,
+   * this intention's entry is shown in the available intentions list.
+   *
+   * Note: this method must do its checks quickly and return.
+   *
+   * @param project a reference to the Project object being edited.
+   * @param editor  a reference to the object editing the project source
+   * @param element a reference to the PSI element currently under the caret
+   * @return
+   * <ul>
+   * <li> true if the caret is in a literal string element, so this functionality
+   * should be added to the intention menu.</li>
+   * <li> false for all other types of caret positions</li>
+   * </ul>
+   */
+  public boolean isAvailable(@NotNull Project project, Editor editor, @Nullable PsiElement element) {
+
+    // Quick sanity check
+    if (element == null) return false;
+
+    // Is this a token of type representing a "?" character?
     if (element instanceof PsiJavaToken) {
       final PsiJavaToken token = (PsiJavaToken) element;
       if (token.getTokenType() != JavaTokenType.QUEST) return false;
+      // Is this token part of a fully formed conditional, i.e. a ternary?
       if (token.getParent() instanceof PsiConditionalExpression) {
         final PsiConditionalExpression conditionalExpression = (PsiConditionalExpression) token.getParent();
         if (conditionalExpression.getThenExpression() == null
             || conditionalExpression.getElseExpression() == null) {
           return false;
         }
-        return true;
+        return true;    // Satisfies all criteria; call back invoke method
       }
       return false;
     }
     return false;
   }
 
+  /**
+   * Modifies the Psi to change a ternary expression to an if-then-else statement.
+   * If the ternary is part of a declaration, the declaration is separated and
+   * moved above the if-then-else statement. Called when user selects this intention action
+   * from the available intentions list.
+   *
+   *   @param  project   a reference to the Project object being edited.
+   *   @param  editor    a reference to the object editing the project source
+   *   @param  element   a reference to the PSI element currently under the caret
+   *   @throws IncorrectOperationException Thrown by underlying (Psi model) write action context
+   *   when manipulation of the psi tree fails.
+   *   @see ConditionalOperatorConvertor#startInWriteAction()
+   */
   public void invoke(@NotNull Project project, Editor editor, PsiElement element) throws IncorrectOperationException {
-    final int offset = editor.getCaretModel().getOffset();
-    PsiConditionalExpression conditionalExpression = PsiTreeUtil.getParentOfType(element,
-                                                                                 PsiConditionalExpression.class, false);
+
+    // Get the factory for making new PsiElements, and the code style manager to format new statements
+    final PsiElementFactory factory = JavaPsiFacade.getInstance(project).getElementFactory();
+    final CodeStyleManager codeStylist = CodeStyleManager.getInstance(project);
+
+    // Get the parent of the "?" element in the ternary statement to find the conditional expression that contains it
+    PsiConditionalExpression conditionalExpression = PsiTreeUtil.getParentOfType(element, PsiConditionalExpression.class, false);
+    // Verify the conditional expression exists and has two outcomes in the ternary statement.
     if (conditionalExpression == null) return;
     if (conditionalExpression.getThenExpression() == null || conditionalExpression.getElseExpression() == null) return;
 
-    final PsiElementFactory factory = JavaPsiFacade.getInstance(project).getElementFactory();
-
+    // Keep searching up the Psi Tree in case the ternary is part of a FOR statement.
     PsiElement originalStatement = PsiTreeUtil.getParentOfType(conditionalExpression, PsiStatement.class, false);
     while (originalStatement instanceof PsiForStatement) {
       originalStatement = PsiTreeUtil.getParentOfType(originalStatement, PsiStatement.class, true);
     }
     if (originalStatement == null) return;
 
-    // Maintain declrations
+    // If the original statement is a declaration based on a ternary operator,
+    // split the declaration and assignment
     if (originalStatement instanceof PsiDeclarationStatement) {
       final PsiDeclarationStatement declaration = (PsiDeclarationStatement) originalStatement;
+
+      // Find the local variable within the declaration statement
       final PsiElement[] declaredElements = declaration.getDeclaredElements();
       PsiLocalVariable variable = null;
       for (PsiElement declaredElement : declaredElements) {
@@ -72,38 +132,73 @@ public class ConditionalOperatorConvertor extends PsiElementBaseIntentionAction
         }
       }
       if (variable == null) return;
+
+      // Ensure that the variable declaration is not combined with other declarations, and add a mark
       variable.normalizeDeclaration();
       final Object marker = new Object();
       PsiTreeUtil.mark(conditionalExpression, marker);
+
+      // Create a new expression to declare the local variable
       PsiExpressionStatement statement =
           (PsiExpressionStatement) factory.createStatementFromText(variable.getName() + " = 0;", null);
-      statement = (PsiExpressionStatement) CodeStyleManager.getInstance(project).reformat(statement);
+      statement = (PsiExpressionStatement) codeStylist.reformat(statement);
+
+      // Replace initializer with the ternary expression, making an assignment statement using the ternary
       ((PsiAssignmentExpression) statement.getExpression()).getRExpression().replace(variable.getInitializer());
+
+      // Remove the initializer portion of the local variable statement,
+      // making it a declaration statement with no initializer
       variable.getInitializer().delete();
+
+      // Get the grandparent of the local var declaration, and add the new declaration just beneath it
       final PsiElement variableParent = variable.getParent();
       originalStatement = variableParent.getParent().addAfter(statement, variableParent);
       conditionalExpression = (PsiConditionalExpression) PsiTreeUtil.releaseMark(originalStatement, marker);
     }
 
-    // create then and else branches
-    final PsiElement[] originalElements = new PsiElement[]{originalStatement, conditionalExpression};
-    final PsiExpression condition = (PsiExpression) conditionalExpression.getCondition().copy();
-    final PsiElement[] thenElements = PsiTreeUtil.copyElements(originalElements);
-    final PsiElement[] elseElements = PsiTreeUtil.copyElements(originalElements);
-    thenElements[1].replace(conditionalExpression.getThenExpression());
-    elseElements[1].replace(conditionalExpression.getElseExpression());
+    // Create an IF statement from a string with placeholder elements.
+    // This will replace the ternary statement
+    PsiIfStatement newIfStmt = (PsiIfStatement) factory.createStatementFromText("if (true) {a=b;} else {c=d;}",null);
+    newIfStmt = (PsiIfStatement) codeStylist.reformat(newIfStmt);
 
-    PsiIfStatement statement = (PsiIfStatement) factory.createStatementFromText("if (true) { a = b } else { c = d }",
-                                                                                null);
-    statement = (PsiIfStatement) CodeStyleManager.getInstance(project).reformat(statement);
-    statement.getCondition().replace(condition);
-    statement = (PsiIfStatement) originalStatement.replace(statement);
+    // Replace the conditional expression with the one from the original ternary expression
+    final PsiReferenceExpression condition = (PsiReferenceExpression) conditionalExpression.getCondition().copy();
+    newIfStmt.getCondition().replace(condition);
+
+    // Begin building the assignment string for the THEN and ELSE clauses using the
+    // parent of the ternary conditional expression
+    PsiAssignmentExpression assignmentExpression = PsiTreeUtil.getParentOfType(conditionalExpression, PsiAssignmentExpression.class, false);
+    // Get the contents of the assignment expression up to the start of the ternary expression
+    String exprFrag = assignmentExpression.getLExpression().getText() + assignmentExpression.getOperationSign().getText() ;
+
+    // Build the THEN statement string for the new IF statement,
+    // make a PsiExpressionStatement from the string, and switch the placeholder
+    String thenStr = exprFrag + conditionalExpression.getThenExpression().getText() + ";" ;
+    PsiExpressionStatement thenStmt = (PsiExpressionStatement) factory.createStatementFromText(thenStr, null);
+    ( (PsiBlockStatement) newIfStmt.getThenBranch() ).getCodeBlock().getStatements()[0].replace(thenStmt);
+
+    // Build the ELSE statement string for the new IF statement,
+    // make a PsiExpressionStatement from the string, and switch the placeholder
+    String elseStr = exprFrag + conditionalExpression.getElseExpression().getText() + ";" ;
+    PsiExpressionStatement elseStmt = (PsiExpressionStatement) factory.createStatementFromText(elseStr, null);
+    ( (PsiBlockStatement) newIfStmt.getElseBranch() ).getCodeBlock().getStatements()[0].replace(elseStmt);
+
+    // Replace the entire original statement with the new IF
+    newIfStmt = (PsiIfStatement) originalStatement.replace(newIfStmt);
 
-    ((PsiBlockStatement) statement.getThenBranch()).getCodeBlock().getStatements()[0].replace(thenElements[0]);
-    ((PsiBlockStatement) statement.getElseBranch()).getCodeBlock().getStatements()[0].replace(elseElements[0]);
   }
 
-  public boolean startInWriteAction() {
-    return true;
-  }
+  /**
+   * Indicates this intention action expects the Psi framework to provide the write action
+   * context for any changes.
+   *
+   * @return <ul>
+   * <li> true if the intention requires a write action context to be provided</li>
+   * <li> false if this intention action will start a write action</li>
+   * </ul>
+   */
+  public boolean startInWriteAction() {return true;}
+
+
 }
+

conditional_operator_intention/testData/before.template.java

@@ -1,6 +1,6 @@
 public class X {
   void f(boolean isMale) {
-    String title = isMale < caret > ? "Mr." : "Ms.";
+    String title = isMale <caret>? "Mr." : "Ms.";
     System.out.println("title = " + title);
   }
 }

conditional_operator_intention/testSource/testPlugin/YourTest.java

@@ -1,6 +1,7 @@
 package testPlugin;
 
 import com.intellij.codeInsight.intention.IntentionAction;
+import com.intellij.openapi.application.PathManager;
 import com.intellij.testFramework.UsefulTestCase;
 import com.intellij.testFramework.builders.JavaModuleFixtureBuilder;
 import com.intellij.testFramework.fixtures.*;
@@ -17,10 +18,7 @@ import org.junit.*;
 
 public class YourTest extends UsefulTestCase {
   protected CodeInsightTestFixture myFixture;
-  // Specify path to your test data
-  // e.g.  final String dataPath = "c:\\users\\john
-  // .doe\\idea\\community\\samples\\conditionalOperatorConvertor/testData";
-  final String dataPath = "c:\\users\\John.Doe\\idea\\community\\samples\\conditionalOperatorConvertor/testData";
+  final String dataPath = PathManager.getResourceRoot(YourTest.class, "/testPlugin/YourTest.class");
 
   @Before
 
@@ -41,10 +39,9 @@ public class YourTest extends UsefulTestCase {
   @After
   public void tearDown() throws Exception {
     myFixture.tearDown();
-    myFixture = null;
   }
 
-  protected void doTest(String testName, String hint) throws Throwable {
+  protected void doTest(String testName, String hint) {
     // Messages.showInfoMessage("Test started", "Info");
     myFixture.configureByFile(testName + ".java");
     final IntentionAction action = myFixture.findSingleIntention(hint);
@@ -54,7 +51,7 @@ public class YourTest extends UsefulTestCase {
   }
 
   @Test
-  public void test() throws Throwable {
+  public void test() {
     doTest("before.template", "Convert ternary operator to if statement");
   }