From 8f53696f2f7c4abae1ab1bb992179000637f4c62 Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Thu, 2 Aug 2018 15:55:58 +0300 Subject: [PATCH] Java: Don't specify visibility for extracted method declared in an interface (IDEA-196426) --- .../extractMethod/ExtractMethodProcessor.java | 3 ++ .../InterfaceMethodVisibility.java | 15 ++++++++ .../InterfaceMethodVisibility_after.java | 17 +++++++++ .../Method2InterfaceFromConstant_after.java | 2 +- .../Method2InterfaceFromStatic_after.java | 2 +- .../extractMethod/Method2Interface_after.java | 2 +- .../java/refactoring/ExtractMethodTest.java | 36 +++++++++++++++++-- 7 files changed, 72 insertions(+), 5 deletions(-) create mode 100644 java/java-tests/testData/refactoring/extractMethod/InterfaceMethodVisibility.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/InterfaceMethodVisibility_after.java diff --git a/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java index 0a0b3e3aac0e..0feddc7d8081 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java @@ -1634,6 +1634,9 @@ public class ExtractMethodProcessor implements MatchProvider { if (containingMethod != null && containingMethod.hasModifierProperty(PsiModifier.DEFAULT)) { PsiUtil.setModifierProperty(newMethod, PsiModifier.DEFAULT, true); } + PsiUtil.setModifierProperty(newMethod, PsiModifier.PUBLIC, false); + PsiUtil.setModifierProperty(newMethod, PsiModifier.PRIVATE, false); + PsiUtil.setModifierProperty(newMethod, PsiModifier.PROTECTED, false); } return (PsiMethod)myStyleManager.reformat(newMethod); } diff --git a/java/java-tests/testData/refactoring/extractMethod/InterfaceMethodVisibility.java b/java/java-tests/testData/refactoring/extractMethod/InterfaceMethodVisibility.java new file mode 100644 index 000000000000..a4d43011c84d --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/InterfaceMethodVisibility.java @@ -0,0 +1,15 @@ +interface TweetParser { + static String getTweetMessageFrom(String fullTweet) { + String fieldName = "\"text\":\""; + int indexOfField = fullTweet.indexOf(fieldName) + fieldName.length(); + int indexOfEndOfField = fullTweet.indexOf("\"", indexOfField); + return fullTweet.substring(indexOfField, indexOfEndOfField); + } + + static String getTwitterHandleFromTweet(String fullTweet) { + String twitterHandleFieldName = "\"screen_name\":\""; + int indexOfTwitterHandleField = fullTweet.indexOf(twitterHandleFieldName) + twitterHandleFieldName.length(); + int indexOfEndOfTwitterHandle = fullTweet.indexOf("\"", indexOfTwitterHandleField); + return fullTweet.substring(indexOfTwitterHandleField, indexOfEndOfTwitterHandle); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/InterfaceMethodVisibility_after.java b/java/java-tests/testData/refactoring/extractMethod/InterfaceMethodVisibility_after.java new file mode 100644 index 000000000000..96473d5f021e --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/InterfaceMethodVisibility_after.java @@ -0,0 +1,17 @@ +interface TweetParser { + static String getTweetMessageFrom(String fullTweet) { + String fieldName = "\"text\":\""; + return newMethod(fullTweet, fieldName); + } + + static String newMethod(String fullTweet, String fieldName) { + int indexOfField = fullTweet.indexOf(fieldName) + fieldName.length(); + int indexOfEndOfField = fullTweet.indexOf("\"", indexOfField); + return fullTweet.substring(indexOfField, indexOfEndOfField); + } + + static String getTwitterHandleFromTweet(String fullTweet) { + String twitterHandleFieldName = "\"screen_name\":\""; + return newMethod(fullTweet, twitterHandleFieldName); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/Method2InterfaceFromConstant_after.java b/java/java-tests/testData/refactoring/extractMethod/Method2InterfaceFromConstant_after.java index fc47db84d46e..6913f366b8a6 100644 --- a/java/java-tests/testData/refactoring/extractMethod/Method2InterfaceFromConstant_after.java +++ b/java/java-tests/testData/refactoring/extractMethod/Method2InterfaceFromConstant_after.java @@ -4,7 +4,7 @@ interface I { String FOO = newMethod(); @NotNull - private static String newMethod() { + static String newMethod() { return "hello"; } } \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/Method2InterfaceFromStatic_after.java b/java/java-tests/testData/refactoring/extractMethod/Method2InterfaceFromStatic_after.java index ba4666f11485..32a49ca4f894 100644 --- a/java/java-tests/testData/refactoring/extractMethod/Method2InterfaceFromStatic_after.java +++ b/java/java-tests/testData/refactoring/extractMethod/Method2InterfaceFromStatic_after.java @@ -3,7 +3,7 @@ interface I { newMethod(); } - private static void newMethod() { + static void newMethod() { System.out.println("hello"); } } \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/Method2Interface_after.java b/java/java-tests/testData/refactoring/extractMethod/Method2Interface_after.java index faed7ac57e2b..e50867b0d977 100644 --- a/java/java-tests/testData/refactoring/extractMethod/Method2Interface_after.java +++ b/java/java-tests/testData/refactoring/extractMethod/Method2Interface_after.java @@ -3,7 +3,7 @@ interface I { newMethod(); } - private default void newMethod() { + default void newMethod() { System.out.println("hello"); } } \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java index 8b930fe44616..0ccf9ec6d36f 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java @@ -17,6 +17,7 @@ package com.intellij.java.refactoring; import com.intellij.JavaTestUtil; import com.intellij.codeInsight.CodeInsightUtil; +import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.lang.java.JavaLanguage; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.editor.Editor; @@ -1158,7 +1159,8 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { configureByFile(BASE_PATH + getTestName(false) + ".java"); final PsiClass psiClass = PsiTreeUtil.getParentOfType(getFile().findElementAt(getEditor().getSelectionModel().getLeadSelectionOffset()), PsiClass.class); assertNotNull(psiClass); - boolean success = performExtractMethod(true, true, getEditor(), getFile(), getProject(), false, null, false, null, psiClass.getContainingClass()); + boolean success = + performExtractMethod(true, true, getEditor(), getFile(), getProject(), false, null, false, null, psiClass.getContainingClass(), null); assertTrue(success); checkResultByFile(BASE_PATH + getTestName(false) + "_after.java"); } @@ -1303,6 +1305,34 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { doDuplicatesTest(); } + public void testInterfaceMethodVisibility() throws Exception { + final String doesNotExist = "foo.bar.baz.DoesNotExist"; + final NullableNotNullManager nullManager = NullableNotNullManager.getInstance(getProject()); + + final List nullables = nullManager.getNullables(); + final List notNulls = nullManager.getNotNulls(); + final String defaultNullable = nullManager.getDefaultNullable(); + final String defaultNotNull = nullManager.getDefaultNotNull(); + try { + nullManager.setNullables(doesNotExist); + nullManager.setNotNulls(doesNotExist); + nullManager.setDefaultNullable(doesNotExist); + nullManager.setDefaultNotNull(doesNotExist); + + configureByFile(BASE_PATH + getTestName(false) + ".java"); + boolean success = + performExtractMethod(true, true, getEditor(), getFile(), getProject(), false, null, false, null, null, PsiModifier.PUBLIC); + assertTrue(success); + checkResultByFile(BASE_PATH + getTestName(false) + "_after.java"); + } + finally { + nullManager.setNullables(ArrayUtil.toStringArray(nullables)); + nullManager.setNotNulls(ArrayUtil.toStringArray(notNulls)); + nullManager.setDefaultNullable(defaultNullable); + nullManager.setDefaultNotNull(defaultNotNull); + } + } + public void testBeforeCommentAfterSelectedFragment() throws Exception { doTest(); } @@ -1428,7 +1458,7 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { int... disabledParams) throws PrepareFailedException, IncorrectOperationException { return performExtractMethod(doRefactor, replaceAllDuplicates, editor, file, project, extractChainedConstructor, returnType, makeStatic, - newNameOfFirstParam, null, disabledParams); + newNameOfFirstParam, null, null, disabledParams); } public static boolean performExtractMethod(boolean doRefactor, @@ -1441,6 +1471,7 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { boolean makeStatic, String newNameOfFirstParam, PsiClass targetClass, + @Nullable @PsiModifier.ModifierConstant String methodVisibility, int... disabledParams) throws PrepareFailedException, IncorrectOperationException { int startOffset = editor.getSelectionModel().getSelectionStart(); @@ -1474,6 +1505,7 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { if (doRefactor) { processor.testTargetClass(targetClass); processor.testPrepare(returnType, makeStatic); + if (methodVisibility != null) processor.setMethodVisibility(methodVisibility); processor.testNullability(); if (disabledParams != null) { for (int param : disabledParams) {