Fixed name suggestion for dialog rename

Fixes #1719

Also, introduced tests for element descriptions

Fixes #1555
See also: #1731

(cherry picked from commit bfe1530)

resources/META-INF/plugin.xml

@@ -77,7 +77,8 @@
         <li>Parsing <code>$1x</code> as variable name and <code>x</code> operator, <a href="https://github.com/Camelcade/Perl5-IDEA/issues/1667">#1667</a></li>
         <li>Resolve from methods declared with <code>method</code> keyword in certain frameworks, <a href="https://github.com/Camelcade/Perl5-IDEA/issues/1669">#1669</a></li>
         <li>Additional formatting cases</li>
-        <li>False unresolved sub warning in <code>Exporter</code> array in case of multiple targets available, <a href="https://github.com/Camelcade/Perl5-IDEA/issues/1669">#1726</a></li>
+        <li>False unresolved sub warning in <code>Exporter</code> array in case of multiple targets available, <a href="https://github.com/Camelcade/Perl5-IDEA/issues/1726">#1726</a></li>
+        <li>Dialog rename for accessors now suggests proper name, <a href="https://github.com/Camelcade/Perl5-IDEA/issues/1719">#1719</a></li>
       </ul>
      </p>
     ]]>

src/com/perl5/lang/perl/idea/PerlElementDescriptionProvider.java

@@ -34,8 +34,6 @@
 import com.perl5.lang.perl.psi.*;
 import com.perl5.lang.perl.psi.impl.PerlFileImpl;
 import com.perl5.lang.perl.psi.mixins.PerlFuncDefinitionMixin;
-import com.perl5.lang.perl.psi.properties.PerlIdentifierOwner;
-import com.perl5.lang.perl.psi.properties.PerlPackageMember;
 import com.perl5.lang.perl.psi.utils.PerlVariableType;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -53,26 +51,27 @@ public String getElementDescription(@NotNull PsiElement element, @NotNull Elemen
     if (location == DeleteNameDescriptionLocation.INSTANCE) {
       return ElementDescriptionUtil.getElementDescription(element, UsageViewShortNameLocation.INSTANCE);
     }
-    else if (location == UsageViewNodeTextLocation.INSTANCE) {
-      return ElementDescriptionUtil.getElementDescription(element, UsageViewShortNameLocation.INSTANCE);
-    }
     else if (location == DeleteTypeDescriptionLocation.SINGULAR) {
       return ElementDescriptionUtil.getElementDescription(element, UsageViewTypeLocation.INSTANCE);
     }
     else if (location == DeleteTypeDescriptionLocation.PLURAL) {
       return StringUtil.pluralize(ElementDescriptionUtil.getElementDescription(element, DeleteTypeDescriptionLocation.SINGULAR));
     }
     else if (location == UsageViewShortNameLocation.INSTANCE) {
-      if (element instanceof PerlIdentifierOwner) {
-        return ((PerlIdentifierOwner)element).getPresentableName();
-      }
-      else if (element instanceof PerlPackageMember) {
-        return ((PerlPackageMember)element).getCanonicalName();
-      }
-      else if (element instanceof PsiNamedElement) {
+      if (element instanceof PsiNamedElement) {
         return ((PsiNamedElement)element).getName();
       }
     }
+    else if (location == UsageViewLongNameLocation.INSTANCE) {
+      return MessageFormat.format(
+        "{0} ''{1}''",
+        ElementDescriptionUtil.getElementDescription(element, UsageViewTypeLocation.INSTANCE),
+        ElementDescriptionUtil.getElementDescription(element, UsageViewShortNameLocation.INSTANCE)
+      );
+    }
+    else if (location == UsageViewNodeTextLocation.INSTANCE) {
+      return ElementDescriptionUtil.getElementDescription(element, UsageViewShortNameLocation.INSTANCE);
+    }
     else if (location == UsageViewTypeLocation.INSTANCE) {
       if (element instanceof MojoHelperDefinition) {
         return PerlBundle.message("perl.type.mojo.helper");
@@ -86,7 +85,10 @@ else if (element instanceof PerlClassAccessorMethod) {
       else if (element instanceof PerlLightExceptionClassDefinition) {
         return PerlBundle.message("perl.type.exception");
       }
-      else if (element instanceof PerlMethodDefinition || (element instanceof PerlSubElement && ((PerlSubElement)element).isMethod())) {
+      else if (element instanceof PerlLightConstantDefinitionElement) {
+        return PerlBundle.message("perl.type.constant");
+      }
+      else if (element instanceof PerlMethodDefinition || element instanceof PerlSubDefinition && ((PerlSubDefinition)element).isMethod()) {
         return PerlBundle.message("perl.type.method");
       }
       else if (element instanceof PerlSubDeclarationElement) {
@@ -98,9 +100,6 @@ else if (element instanceof PerlHeredocOpener) {
       else if (element instanceof PerlFuncDefinitionMixin) {
         return PerlBundle.message("perl.type.function");
       }
-      else if (element instanceof PerlLightConstantDefinitionElement) {
-        return PerlBundle.message("perl.type.constant");
-      }
       else if (element instanceof PerlSubDefinitionElement) {
         return PerlBundle.message("perl.type.sub.definition");
       }
@@ -116,12 +115,8 @@ else if (element instanceof PsiDirectoryContainer) {
       else if (element instanceof PerlGlobVariable) {
         return PerlBundle.message("perl.type.typeglob");
       }
-      else if (element instanceof PerlVariable || element instanceof PerlVariableDeclarationElement) {
-        if (element instanceof PerlVariableDeclarationElement) {
-          element = ((PerlVariableDeclarationElement)element).getVariable();
-        }
-
-        PerlVariableType actualType = ((PerlVariable)element).getActualType();
+      else if (element instanceof PerlVariableDeclarationElement) {
+        PerlVariableType actualType = ((PerlVariableDeclarationElement)element).getVariable().getActualType();
         if (actualType == PerlVariableType.ARRAY) {
           return PerlBundle.message("perl.type.array");
         }
@@ -136,17 +131,6 @@ else if (element instanceof PerlVariableNameElement) {
         return getElementDescription(element.getParent(), location);
       }
     }
-    else if (location == UsageViewLongNameLocation.INSTANCE) {
-      return MessageFormat.format(
-        "{0} ''{1}''",
-        ElementDescriptionUtil.getElementDescription(element, UsageViewTypeLocation.INSTANCE),
-        ElementDescriptionUtil.getElementDescription(element, UsageViewShortNameLocation.INSTANCE)
-      );
-    }
-    else {
-      return "Unhandled location " + location;
-    }
-
     return null;
   }
 }

test/base/PerlLightTestCase.java

@@ -33,6 +33,8 @@
 import com.intellij.ide.structureView.StructureViewBuilder;
 import com.intellij.ide.structureView.StructureViewModel;
 import com.intellij.ide.structureView.StructureViewTreeElement;
+import com.intellij.ide.util.DeleteNameDescriptionLocation;
+import com.intellij.ide.util.DeleteTypeDescriptionLocation;
 import com.intellij.ide.util.treeView.smartTree.*;
 import com.intellij.injected.editor.EditorWindow;
 import com.intellij.lang.ASTNode;
@@ -71,11 +73,12 @@
 import com.intellij.psi.impl.PsiManagerEx;
 import com.intellij.psi.impl.source.tree.injected.InjectedFileViewProvider;
 import com.intellij.psi.util.PsiTreeUtil;
+import com.intellij.refactoring.util.NonCodeSearchDescriptionLocation;
 import com.intellij.testFramework.MapDataContext;
 import com.intellij.testFramework.UsefulTestCase;
 import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase;
 import com.intellij.testFramework.fixtures.impl.CodeInsightTestFixtureImpl;
-import com.intellij.usageView.UsageInfo;
+import com.intellij.usageView.*;
 import com.intellij.util.Function;
 import com.intellij.util.ObjectUtils;
 import com.intellij.util.containers.ContainerUtil;
@@ -1181,4 +1184,42 @@ private void appendDescriptors(@NotNull String sectionLabel,
     sb.append("\n");
   }
 
+  private static final List<ElementDescriptionLocation> LOCATIONS = Arrays.asList(
+    UsageViewShortNameLocation.INSTANCE,
+    UsageViewLongNameLocation.INSTANCE,
+    UsageViewTypeLocation.INSTANCE,
+    UsageViewNodeTextLocation.INSTANCE,
+    NonCodeSearchDescriptionLocation.STRINGS_AND_COMMENTS,
+    DeleteNameDescriptionLocation.INSTANCE,
+    DeleteTypeDescriptionLocation.SINGULAR,
+    DeleteTypeDescriptionLocation.PLURAL
+  );
+
+  protected void doElementDescriptionTest() {
+    initWithFileSmartWithoutErrors();
+    doElementDescriptionCheck();
+  }
+
+  protected void doElementDescriptionTest(@NotNull String content) {
+    initWithTextSmart(content);
+    assertNoErrorElements();
+    doElementDescriptionCheck();
+  }
+
+  private void doElementDescriptionCheck() {
+    PsiElement elementAtCaret = myFixture.getElementAtCaret();
+    assertNotNull(elementAtCaret);
+    StringBuilder actualDump = new StringBuilder();
+    LOCATIONS.forEach(
+      location -> {
+        String actual = ElementDescriptionUtil.getElementDescription(elementAtCaret, location);
+        String locationName = location.getClass().getSimpleName();
+        actualDump.append(locationName)
+          .append(": ")
+          .append(actual)
+          .append("\n");
+      }
+    );
+    UsefulTestCase.assertSameLinesWithFile(getTestResultsFilePath(), actualDump.toString());
+  }
 }

test/editor/PerlDescriptionProviderTest.java

@@ -0,0 +1,108 @@
+/*
+ * Copyright 2015-2017 Alexandr Evstigneev
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package editor;
+
+import base.PerlLightTestCase;
+import org.jetbrains.annotations.NotNull;
+
+public class PerlDescriptionProviderTest extends PerlLightTestCase {
+  @Override
+  protected String getTestDataPath() {
+    return "testData/descriptionProvider/perl";
+  }
+
+
+  public void testLocalScalar() {
+    doTest("my $variable<caret>_name;");
+  }
+
+  public void testLocalArray() {
+    doTest("my @variable<caret>_name;");
+  }
+
+  public void testLocalHash() {
+    doTest("my %variable<caret>_name;");
+  }
+
+  public void testGlobalScalar() {
+    doTest("package Foo::Bar; our $variable<caret>_name;");
+  }
+
+  public void testGlobalArray() {
+    doTest("package Foo::Bar; our @variable<caret>_name;");
+  }
+
+  public void testGlobalHash() {
+    doTest("package Foo::Bar; our %variable<caret>_name;");
+  }
+
+  public void testMojoHelper() {
+    doTest("$self->helper(helper_<caret>name=>sub{});");
+  }
+
+  public void testAttribute() {
+    doTest("has attr_n<caret>ame => {};");
+  }
+
+  public void testAccessor() {
+    doTest("__PACKAGE__->follow_best_practices;__PACKAGE__->mk_accessors('accessor<caret>_name');");
+  }
+
+  public void testException() {
+    doTest("use Exception::Class 'Sept<caret>ion1';");
+  }
+
+  public void testMethod() {
+    doTest("method method<caret>_name{}");
+  }
+
+  public void testSubDeclaration() {
+    doTest("sub sub_<caret>name;");
+  }
+
+  public void testHeredocMarker() {
+    doTest();
+  }
+
+  public void testFunction() {
+    doTest("func func_n<caret>ame{}");
+  }
+
+  public void testConstant() {
+    doTest("use constant CONS<caret>T_NAME => 42;");
+  }
+
+  public void testSubDefinition() {
+    doTest("sub sub_n<caret>ame{}");
+  }
+
+  public void testNamespace() {
+    doTest("package Foo::Ba<caret>zzz;");
+  }
+
+  public void testTypeglob() {
+    doTest("*typ<caret>eglob_name = \\$var");
+  }
+
+  private void doTest() {
+    doElementDescriptionTest();
+  }
+
+  private void doTest(@NotNull String content) {
+    doElementDescriptionTest(content);
+  }
+}

testData/descriptionProvider/perl/accessor.pl.txt

@@ -0,0 +1,8 @@
+UsageViewShortNameLocation: accessor_name
+UsageViewLongNameLocation: accessor 'accessor_name'
+UsageViewTypeLocation: accessor
+UsageViewNodeTextLocation: accessor_name
+NonCodeSearchDescriptionLocation: accessor_name
+DeleteNameDescriptionLocation: accessor_name
+DeleteTypeDescriptionLocation: accessor
+DeleteTypeDescriptionLocation: accessors

testData/descriptionProvider/perl/attribute.pl.txt

@@ -0,0 +1,8 @@
+UsageViewShortNameLocation: attr_name
+UsageViewLongNameLocation: method 'attr_name'
+UsageViewTypeLocation: method
+UsageViewNodeTextLocation: attr_name
+NonCodeSearchDescriptionLocation: attr_name
+DeleteNameDescriptionLocation: attr_name
+DeleteTypeDescriptionLocation: method
+DeleteTypeDescriptionLocation: methods

testData/descriptionProvider/perl/constant.pl.txt

@@ -0,0 +1,8 @@
+UsageViewShortNameLocation: CONST_NAME
+UsageViewLongNameLocation: constant 'CONST_NAME'
+UsageViewTypeLocation: constant
+UsageViewNodeTextLocation: CONST_NAME
+NonCodeSearchDescriptionLocation: CONST_NAME
+DeleteNameDescriptionLocation: CONST_NAME
+DeleteTypeDescriptionLocation: constant
+DeleteTypeDescriptionLocation: constants

testData/descriptionProvider/perl/exception.pl.txt

@@ -0,0 +1,8 @@
+UsageViewShortNameLocation: Seption1
+UsageViewLongNameLocation: exception 'Seption1'
+UsageViewTypeLocation: exception
+UsageViewNodeTextLocation: Seption1
+NonCodeSearchDescriptionLocation: Seption1
+DeleteNameDescriptionLocation: Seption1
+DeleteTypeDescriptionLocation: exception
+DeleteTypeDescriptionLocation: exceptions

testData/descriptionProvider/perl/function.pl.txt

@@ -0,0 +1,8 @@
+UsageViewShortNameLocation: func_name
+UsageViewLongNameLocation: function 'func_name'
+UsageViewTypeLocation: function
+UsageViewNodeTextLocation: func_name
+NonCodeSearchDescriptionLocation: func_name
+DeleteNameDescriptionLocation: func_name
+DeleteTypeDescriptionLocation: function
+DeleteTypeDescriptionLocation: functions

testData/descriptionProvider/perl/globalArray.pl.txt

@@ -0,0 +1,8 @@
+UsageViewShortNameLocation: variable_name
+UsageViewLongNameLocation: array 'variable_name'
+UsageViewTypeLocation: array
+UsageViewNodeTextLocation: variable_name
+NonCodeSearchDescriptionLocation: variable_name
+DeleteNameDescriptionLocation: variable_name
+DeleteTypeDescriptionLocation: array
+DeleteTypeDescriptionLocation: arrays

testData/descriptionProvider/perl/globalHash.pl.txt

@@ -0,0 +1,8 @@
+UsageViewShortNameLocation: variable_name
+UsageViewLongNameLocation: hash 'variable_name'
+UsageViewTypeLocation: hash
+UsageViewNodeTextLocation: variable_name
+NonCodeSearchDescriptionLocation: variable_name
+DeleteNameDescriptionLocation: variable_name
+DeleteTypeDescriptionLocation: hash
+DeleteTypeDescriptionLocation: hashes

testData/descriptionProvider/perl/globalScalar.pl.txt

@@ -0,0 +1,8 @@
+UsageViewShortNameLocation: variable_name
+UsageViewLongNameLocation: scalar 'variable_name'
+UsageViewTypeLocation: scalar
+UsageViewNodeTextLocation: variable_name
+NonCodeSearchDescriptionLocation: variable_name
+DeleteNameDescriptionLocation: variable_name
+DeleteTypeDescriptionLocation: scalar
+DeleteTypeDescriptionLocation: scalars

testData/descriptionProvider/perl/heredocMarker.code

@@ -0,0 +1,3 @@
+<<E<caret>OM
+test
+EOM

testData/descriptionProvider/perl/heredocMarker.pl.txt

@@ -0,0 +1,8 @@
+UsageViewShortNameLocation: EOM
+UsageViewLongNameLocation: heredoc marker 'EOM'
+UsageViewTypeLocation: heredoc marker
+UsageViewNodeTextLocation: EOM
+NonCodeSearchDescriptionLocation: EOM
+DeleteNameDescriptionLocation: EOM
+DeleteTypeDescriptionLocation: heredoc marker
+DeleteTypeDescriptionLocation: heredoc markers

testData/descriptionProvider/perl/localArray.pl.txt

@@ -0,0 +1,8 @@
+UsageViewShortNameLocation: variable_name
+UsageViewLongNameLocation: array 'variable_name'
+UsageViewTypeLocation: array
+UsageViewNodeTextLocation: variable_name
+NonCodeSearchDescriptionLocation: variable_name
+DeleteNameDescriptionLocation: variable_name
+DeleteTypeDescriptionLocation: array
+DeleteTypeDescriptionLocation: arrays

testData/descriptionProvider/perl/localHash.pl.txt

@@ -0,0 +1,8 @@
+UsageViewShortNameLocation: variable_name
+UsageViewLongNameLocation: hash 'variable_name'
+UsageViewTypeLocation: hash
+UsageViewNodeTextLocation: variable_name
+NonCodeSearchDescriptionLocation: variable_name
+DeleteNameDescriptionLocation: variable_name
+DeleteTypeDescriptionLocation: hash
+DeleteTypeDescriptionLocation: hashes

testData/descriptionProvider/perl/localScalar.pl.txt

@@ -0,0 +1,8 @@
+UsageViewShortNameLocation: variable_name
+UsageViewLongNameLocation: scalar 'variable_name'
+UsageViewTypeLocation: scalar
+UsageViewNodeTextLocation: variable_name
+NonCodeSearchDescriptionLocation: variable_name
+DeleteNameDescriptionLocation: variable_name
+DeleteTypeDescriptionLocation: scalar
+DeleteTypeDescriptionLocation: scalars