diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java index 6b8560f9ceaa..fc02dc40ba16 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java @@ -96,7 +96,12 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool DataFlowInstructionVisitor visitor = analyzeDfaWithNestedClosures(aClass, holder, runner, Collections.singletonList(runner.createMemoryState())); List states = visitor.getEndOfInitializerStates(); + boolean physical = aClass.isPhysical(); for (PsiMethod method : aClass.getConstructors()) { + if (physical && !method.isPhysical()) { + // Constructor could be provided by, e.g. Lombok plugin: ignore it, we won't report any problems inside anyway + continue; + } List initialStates; PsiMethodCallExpression call = JavaPsiConstructorUtil.findThisOrSuperCallInConstructor(method); if (JavaPsiConstructorUtil.isChainedConstructorCall(call) || (call == null && DfaUtil.hasImplicitImpureSuperCall(aClass, method))) { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java index b1d4a1afc4e8..36656d78c56b 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java @@ -4,6 +4,8 @@ package com.intellij.codeInspection.dataFlow; import com.intellij.codeInspection.dataFlow.instructions.*; import com.intellij.codeInspection.dataFlow.value.*; import com.intellij.codeInspection.util.OptionalUtil; +import com.intellij.openapi.application.Application; +import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.TextRange; @@ -183,7 +185,10 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor { @Nullable TextRange range, @NotNull DfaMemoryState memState) { if (!expression.isPhysical()) { - LOG.error("Non-physical expression is passed" + expression); + Application application = ApplicationManager.getApplication(); + if (application.isEAP() || application.isInternal() || application.isUnitTestMode()) { + throw new IllegalStateException("Non-physical expression is passed"); + } } expression.accept(new ExpressionVisitor(value, memState)); if (range == null) { diff --git a/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/InnerClasses.java b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/InnerClasses.java new file mode 100644 index 000000000000..5c465b31df5b --- /dev/null +++ b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/InnerClasses.java @@ -0,0 +1,16 @@ +package xxx; + +public class InnerClasses { + static class PackagePrivateInnerClass { + } + + static class PackagePrivateInnerClassWithConstructor { + PackagePrivateInnerClassWithConstructor() { + } + } + + public static class ClassWithPackagePrivateConstructor { + ClassWithPackagePrivateConstructor() { + } + } +} \ No newline at end of file diff --git a/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/PackagePrivateClass.java b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/PackagePrivateClass.java new file mode 100644 index 000000000000..afd175aa6cd6 --- /dev/null +++ b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/PackagePrivateClass.java @@ -0,0 +1,4 @@ +package xxx; + +class PackagePrivateClass { +} \ No newline at end of file diff --git a/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/ProtectedConstructors.java b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/ProtectedConstructors.java new file mode 100644 index 000000000000..5d30de8de61a --- /dev/null +++ b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/ProtectedConstructors.java @@ -0,0 +1,9 @@ +package xxx; + +public class ProtectedConstructors { + protected ProtectedConstructors() { + } + + protected ProtectedConstructors(int i) { + } +} \ No newline at end of file diff --git a/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/ProtectedMembers.java b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/ProtectedMembers.java new file mode 100644 index 000000000000..3bae60a51cf9 --- /dev/null +++ b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/ProtectedMembers.java @@ -0,0 +1,12 @@ +package xxx; + +public class ProtectedMembers { + protected void method() { + } + + static protected void staticMethod() { + } + + protected static class StaticInner { + } +} \ No newline at end of file diff --git a/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/PublicClass.java b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/PublicClass.java new file mode 100644 index 000000000000..02fdb18bc7f3 --- /dev/null +++ b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/PublicClass.java @@ -0,0 +1,16 @@ +// 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 xxx; + +public class PublicClass { + public String publicField; + String packagePrivateField; + static public String PUBLIC_STATIC_FIELD; + static String PACKAGE_PRIVATE_STATIC_FIELD; + + public void publicMethod() {} + void packagePrivateMethod() {} + + PublicClass() {} + public PublicClass(int i) {} + PublicClass(boolean b) {} +} \ No newline at end of file diff --git a/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/PublicClassWithDefaultConstructor.java b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/PublicClassWithDefaultConstructor.java new file mode 100644 index 000000000000..c71e3139f08c --- /dev/null +++ b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/PublicClassWithDefaultConstructor.java @@ -0,0 +1,5 @@ +// 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 xxx; + +public class PublicClassWithDefaultConstructor { +} \ No newline at end of file diff --git a/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/StaticMembers.java b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/StaticMembers.java new file mode 100644 index 000000000000..75b10e0e5008 --- /dev/null +++ b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/StaticMembers.java @@ -0,0 +1,7 @@ +package xxx; + +public class StaticMembers { + static String IMPORTED_FIELD = ""; + static void importedMethod() { + } +} \ No newline at end of file diff --git a/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/src/AccessingPackagePrivateMembers.kt b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/src/AccessingPackagePrivateMembers.kt new file mode 100644 index 000000000000..befe8347739c --- /dev/null +++ b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/src/AccessingPackagePrivateMembers.kt @@ -0,0 +1,41 @@ +package xxx + +import xxx.StaticMembers.* + +/** + * @see PackagePrivateClass + * @see PublicClass.packagePrivateField + */ +@Suppress("UNUSED_VARIABLE") +class AccessingPackagePrivateMembers { + private val property = PackagePrivateClass() + + fun main() { + PackagePrivateClass() + var variable: PackagePrivateClass + + val aClass: PublicClass = PublicClass(1); + val aClass2: PublicClassWithDefaultConstructor = PublicClassWithDefaultConstructor(); + PublicClass() + PublicClass(true) + + System.out.println(aClass.publicField) + System.out.println(aClass.packagePrivateField) + System.out.println(PublicClass.PUBLIC_STATIC_FIELD) + System.out.println(PublicClass.PACKAGE_PRIVATE_STATIC_FIELD) + + aClass.publicMethod() + aClass.packagePrivateMethod() + + System.out.println(IMPORTED_FIELD) + importedMethod() + + InnerClasses.PackagePrivateInnerClass() + InnerClasses.PackagePrivateInnerClassWithConstructor() + InnerClasses.ClassWithPackagePrivateConstructor() + } + + companion object { + private val staticProperty = PackagePrivateClass() + } +} \ No newline at end of file diff --git a/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/src/AccessingProtectedMembers.kt b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/src/AccessingProtectedMembers.kt new file mode 100644 index 000000000000..4fc4709420a9 --- /dev/null +++ b/jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/src/AccessingProtectedMembers.kt @@ -0,0 +1,36 @@ +package xxx + +class AccessingProtectedMembersNotFromSubclass { + fun foo() { + val aClass: ProtectedMembers = ProtectedMembers() + aClass.method() + ProtectedMembers.staticMethod() + ProtectedConstructors() + ProtectedConstructors(1) + } +} + +@Suppress("UNUSED_VARIABLE") +class AccessingProtectedMembersFromSubclass : ProtectedMembers() { + fun foo() { + method() + staticMethod() + ProtectedMembers.staticMethod() + + val aClass = ProtectedMembers() + aClass.method() + val myInstance = AccessingProtectedMembersFromSubclass() + myInstance.method() + + var inner1: ProtectedMembers.StaticInner + var inner2: StaticInner + } + + private class StaticInnerImpl1 : ProtectedMembers.StaticInner() + + private class StaticInnerImpl2 : StaticInner() +} + +class AccessingDefaultProtectedConstructorFromSubclass : ProtectedConstructors() + +class AccessingProtectedConstructorFromSubclass : ProtectedConstructors(1) \ No newline at end of file diff --git a/jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/KtSuspiciousPackagePrivateAccessInspectionTest.kt b/jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/KtSuspiciousPackagePrivateAccessInspectionTest.kt new file mode 100644 index 000000000000..837a5bb49182 --- /dev/null +++ b/jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/KtSuspiciousPackagePrivateAccessInspectionTest.kt @@ -0,0 +1,18 @@ +package com.intellij.codeInspection.tests.kotlin + +import com.intellij.jvm.analysis.JvmAnalysisKtTestsUtil +import com.intellij.testFramework.TestDataPath +import com.siyeh.ig.dependency.SuspiciousPackagePrivateAccessInspectionTestCase + +@TestDataPath("/testData/codeInspection/suspiciousPackagePrivateAccess") +class KtSuspiciousPackagePrivateAccessInspectionTest : SuspiciousPackagePrivateAccessInspectionTestCase("kt") { + fun testAccessingPackagePrivateMembers() { + doTestWithDependency() + } + + fun testAccessingProtectedMembers() { + doTestWithDependency() + } + + override fun getBasePath() = "${JvmAnalysisKtTestsUtil.TEST_DATA_PROJECT_RELATIVE_BASE_PATH}/codeInspection/suspiciousPackagePrivateAccess" +} \ No newline at end of file diff --git a/platform/analysis-api/src/com/intellij/codeInsight/daemon/QuickFixActionRegistrar.java b/platform/analysis-api/src/com/intellij/codeInsight/daemon/QuickFixActionRegistrar.java index 2f6761a14bb8..918289aeb5ff 100644 --- a/platform/analysis-api/src/com/intellij/codeInsight/daemon/QuickFixActionRegistrar.java +++ b/platform/analysis-api/src/com/intellij/codeInsight/daemon/QuickFixActionRegistrar.java @@ -1,19 +1,4 @@ -/* - * Copyright 2000-2013 JetBrains s.r.o. - * - * 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. - */ - +// Copyright 2000-2019 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.daemon; import com.intellij.codeInsight.intention.IntentionAction; @@ -22,12 +7,19 @@ import com.intellij.openapi.util.TextRange; import org.jetbrains.annotations.NotNull; public interface QuickFixActionRegistrar { + void register(@NotNull IntentionAction action); + void register(@NotNull TextRange fixRange, @NotNull IntentionAction action, HighlightDisplayKey key); /** - * Allows to replace some of the built-in quickfixes. - * @param condition condition for quickfixes to remove + * Allows to replace some of the built-in quick fixes. + * + * @param condition condition for quick fixes to remove + * @deprecated if some fix may be inapplicable under certain circumstances + * it should be fixed to provide its own EP, so it's possible to plug into the fix directly + * instead of filtering it with this method */ - void unregister(@NotNull Condition condition); + @Deprecated + default void unregister(@NotNull Condition condition) {} } diff --git a/platform/configuration-store-impl/testSrc/ConfigImportHelperTest.kt b/platform/configuration-store-impl/testSrc/ConfigImportHelperTest.kt index 148e55b633bb..669c45391561 100644 --- a/platform/configuration-store-impl/testSrc/ConfigImportHelperTest.kt +++ b/platform/configuration-store-impl/testSrc/ConfigImportHelperTest.kt @@ -9,6 +9,7 @@ import com.intellij.openapi.components.StoragePathMacros import com.intellij.openapi.components.stateStore import com.intellij.testFramework.ApplicationRule import com.intellij.testFramework.rules.InMemoryFsRule +import com.intellij.util.io.createDirectories import com.intellij.util.io.directoryStreamIfExists import com.intellij.util.io.exists import com.intellij.util.io.write @@ -68,7 +69,11 @@ class ConfigImportHelperTest { writeStorageFile("2020.1", 100, isMacOs) writeStorageFile("2021.1", 200, isMacOs) writeStorageFile("2022.1", 300, isMacOs) - val newConfigPath = fs.getPath("/data/${constructConfigPath("2022.1", isMacOs)}") + + val newConfigPath = fs.getPath("/data/${constructConfigPath("2022.3", isMacOs)}") + // create new config dir to test that it will be not suggested too (as on start of new version config dir can be created) + newConfigPath.createDirectories() + assertThat(ConfigImportHelper.findRecentConfigDirectory(newConfigPath, isMacOs).joinToString("\n")).isEqualTo(""" /data/${constructConfigPath("2022.1", isMacOs)} /data/${constructConfigPath("2021.1", isMacOs)} @@ -83,12 +88,35 @@ class ConfigImportHelperTest { """.trimIndent()) } - private fun writeStorageFile(version: String, lastModified: Long, isMacOs: Boolean) { - val path = fsRule.fs.getPath("/data/" + (constructConfigPath(version, isMacOs)), - PathManager.OPTIONS_DIRECTORY + '/' + StoragePathMacros.NOT_ROAMABLE_FILE) - Files.setLastModifiedTime(path.write(version), FileTime.fromMillis(lastModified)) + @Test + fun `sort if no anchor files`() { + val isMacOs = true + fun writeStorageDir(version: String) { + val dir = fsRule.fs.getPath("/data/" + (constructConfigPath(version, isMacOs))) + dir.createDirectories() + } + + val fs = fsRule.fs + writeStorageDir("2022.1") + writeStorageDir("2021.1") + writeStorageDir("2020.1") + + val newConfigPath = fs.getPath("/data/${constructConfigPath("2022.3", isMacOs)}") + // create new config dir to test that it will be not suggested too (as on start of new version config dir can be created) + newConfigPath.createDirectories() + + assertThat(ConfigImportHelper.findRecentConfigDirectory(newConfigPath, isMacOs).joinToString("\n")).isEqualTo(""" + /data/${constructConfigPath("2022.1", isMacOs)} + /data/${constructConfigPath("2021.1", isMacOs)} + /data/${constructConfigPath("2020.1", isMacOs)} + """.trimIndent()) } + private fun writeStorageFile(version: String, lastModified: Long, isMacOs: Boolean) { + val dir = fsRule.fs.getPath("/data/" + (constructConfigPath(version, isMacOs))) + val file = dir.resolve(PathManager.OPTIONS_DIRECTORY + '/' + StoragePathMacros.NOT_ROAMABLE_FILE) + Files.setLastModifiedTime(file.write(version), FileTime.fromMillis(lastModified)) + } } private fun constructConfigPath(version: String, isMacOs: Boolean): String { diff --git a/platform/lang-impl/src/com/intellij/execution/ExecutionHelper.java b/platform/lang-impl/src/com/intellij/execution/ExecutionHelper.java index e05291bb0486..309b5d4ab3b4 100644 --- a/platform/lang-impl/src/com/intellij/execution/ExecutionHelper.java +++ b/platform/lang-impl/src/com/intellij/execution/ExecutionHelper.java @@ -5,13 +5,13 @@ package com.intellij.execution; import com.intellij.execution.configurations.GeneralCommandLine; import com.intellij.execution.executors.DefaultRunExecutor; import com.intellij.execution.impl.ExecutionManagerImpl; +import com.intellij.execution.process.OSProcessHandler; import com.intellij.execution.process.ProcessHandler; import com.intellij.execution.process.ProcessOutput; import com.intellij.execution.ui.RunContentDescriptor; import com.intellij.execution.ui.RunContentManager; import com.intellij.ide.errorTreeView.NewErrorTreeViewPanel; import com.intellij.openapi.actionSystem.DataContext; -import com.intellij.openapi.application.Application; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.command.CommandProcessor; import com.intellij.openapi.components.ServiceManager; @@ -348,15 +348,6 @@ public class ExecutionHelper { if (indicator != null && title2 != null) { indicator.setText2(title2); } - Application application = ApplicationManager.getApplication(); - if (application.isInternal() && !application.isHeadlessEnvironment()) { - if (application.isDispatchThread()) { - LOG.warn("Synchronous execution on EDT: " + processHandler, new Throwable()); - } - else if (application.isReadAccessAllowed()) { - LOG.warn("Synchronous execution under ReadAction: " + processHandler, new Throwable()); - } - } process.run(); } } @@ -419,7 +410,7 @@ public class ExecutionHelper { mySemaphore.down(); ApplicationManager.getApplication().executeOnPooledThread(myWaitThread); ApplicationManager.getApplication().executeOnPooledThread(myCancelListener); - + OSProcessHandler.checkEdtAndReadAction(processHandler); mySemaphore.waitFor(); } }; @@ -448,7 +439,7 @@ public class ExecutionHelper { public void run() { mySemaphore.down(); ApplicationManager.getApplication().executeOnPooledThread(myProcessThread); - + OSProcessHandler.checkEdtAndReadAction(processHandler); mySemaphore.waitFor(); } }; diff --git a/platform/lang-impl/src/com/intellij/ide/actions/searcheverywhere/SearchEverywhereUI.java b/platform/lang-impl/src/com/intellij/ide/actions/searcheverywhere/SearchEverywhereUI.java index 5f6812c00ac0..fe26f8dd12d8 100644 --- a/platform/lang-impl/src/com/intellij/ide/actions/searcheverywhere/SearchEverywhereUI.java +++ b/platform/lang-impl/src/com/intellij/ide/actions/searcheverywhere/SearchEverywhereUI.java @@ -1057,6 +1057,7 @@ public class SearchEverywhereUI extends BigPopupUI implements DataProvider, Quic if (resultsExpired) { retainContributors(itemsMap.keySet()); + clearMoreItems(); itemsMap.forEach((contributor, list) -> { Object[] oldItems = ArrayUtil.toObjectArray(getFoundItems(contributor)); @@ -1076,14 +1077,16 @@ public class SearchEverywhereUI extends BigPopupUI implements DataProvider, Quic else { itemsMap.forEach((contributor, list) -> { int startIndex = contributors().indexOf(contributor); + int insertionIndex = getInsertionPoint(contributor); + int endIndex = insertionIndex + list.size() - 1; + listElements.addAll(insertionIndex, list); + fireIntervalAdded(this, insertionIndex, endIndex); + + // there were items for this contributor before update if (startIndex >= 0) { - addElementsWithPriority(contributor, startIndex, list); - } - else { - startIndex = getInsertionPoint(contributor); - int endIndex = startIndex + list.size() - 1; - listElements.addAll(startIndex, list); - fireIntervalAdded(this, startIndex, endIndex); + listElements.subList(startIndex, endIndex + 1) + .sort(Comparator.comparingInt(SESearcher.ElementInfo::getPriority).reversed()); + fireContentsChanged(this, startIndex, endIndex); } }); } @@ -1115,22 +1118,13 @@ public class SearchEverywhereUI extends BigPopupUI implements DataProvider, Quic } } - private void addElementsWithPriority(SearchEverywhereContributor contributor, int index, List newElements) { - for (SESearcher.ElementInfo newElementInfo : newElements) { - if (index < listElements.size()) { - SESearcher.ElementInfo existingElementInfo = listElements.get(index); - while (existingElementInfo.getContributor() == contributor - && existingElementInfo.getPriority() >= newElementInfo.getPriority() - && existingElementInfo.getElement() != MORE_ELEMENT) { - index++; - if (index >= listElements.size()) break; - existingElementInfo = listElements.get(index); - } - listElements.add(index, newElementInfo); - index++; - } - else { - listElements.add(newElementInfo); + private void clearMoreItems() { + ListIterator iterator = listElements.listIterator(); + while (iterator.hasNext()) { + int index = iterator.nextIndex(); + if (iterator.next().getElement() == MORE_ELEMENT) { + iterator.remove(); + fireContentsChanged(this, index, index); } } } @@ -1266,13 +1260,8 @@ public class SearchEverywhereUI extends BigPopupUI implements DataProvider, Quic return isMoreElement(index) ? index : index + 1; } - for (int i = 0; i < list.size(); i++) { - if (list.get(i).getSortWeight() > contributor.getSortWeight()) { - return i; - } - } - - return listElements.size(); + index = Collections.binarySearch(list, contributor, Comparator.comparingInt(SearchEverywhereContributor::getSortWeight)); + return -index - 1; } } diff --git a/platform/lang-impl/testSources/com/intellij/ide/actions/searcheverywhere/SearchModelTest.java b/platform/lang-impl/testSources/com/intellij/ide/actions/searcheverywhere/SearchModelTest.java index b90300e6cadd..053f35c2c2a0 100644 --- a/platform/lang-impl/testSources/com/intellij/ide/actions/searcheverywhere/SearchModelTest.java +++ b/platform/lang-impl/testSources/com/intellij/ide/actions/searcheverywhere/SearchModelTest.java @@ -26,8 +26,8 @@ public class SearchModelTest extends LightPlatformCodeInsightFixtureTestCase { // adding to empty ----------------------------------------------------------------------- model.addElements(Arrays.asList( - new SESearcher.ElementInfo("item_3_20", 340, STUB_CONTRIBUTOR_3), new SESearcher.ElementInfo("item_2_20", 250, STUB_CONTRIBUTOR_2), + new SESearcher.ElementInfo("item_3_20", 340, STUB_CONTRIBUTOR_3), new SESearcher.ElementInfo("item_1_20", 160, STUB_CONTRIBUTOR_1), new SESearcher.ElementInfo("item_3_30", 330, STUB_CONTRIBUTOR_3), new SESearcher.ElementInfo("item_2_10", 280, STUB_CONTRIBUTOR_2), @@ -72,8 +72,39 @@ public class SearchModelTest extends LightPlatformCodeInsightFixtureTestCase { Assert.assertEquals(expectedItems, actualItems); // expiring results ----------------------------------------------------------------------- - // removing items ----------------------------------------------------------------------- + model.expireResults(); + model.addElements(Arrays.asList( + new SESearcher.ElementInfo("item_3_50", 310, STUB_CONTRIBUTOR_3), + new SESearcher.ElementInfo("item_1_20", 160, STUB_CONTRIBUTOR_1), + new SESearcher.ElementInfo("item_3_10", 350, STUB_CONTRIBUTOR_3), + new SESearcher.ElementInfo("item_2_23", 250, STUB_CONTRIBUTOR_2), + new SESearcher.ElementInfo("item_3_30", 330, STUB_CONTRIBUTOR_3), + new SESearcher.ElementInfo("item_2_05", 290, STUB_CONTRIBUTOR_2), + new SESearcher.ElementInfo("item_2_10", 280, STUB_CONTRIBUTOR_2), + new SESearcher.ElementInfo("item_1_35", 130, STUB_CONTRIBUTOR_1), + new SESearcher.ElementInfo("item_3_20", 340, STUB_CONTRIBUTOR_3), + new SESearcher.ElementInfo("item_1_25", 150, STUB_CONTRIBUTOR_1) + )); + model.setHasMore(STUB_CONTRIBUTOR_1, true); + model.setHasMore(STUB_CONTRIBUTOR_2, true); + actualItems = model.getItems(); + expectedItems = Arrays.asList("item_1_20", "item_1_25", "item_1_35", SearchListModel.MORE_ELEMENT, + "item_2_05", "item_2_10", "item_2_23", SearchListModel.MORE_ELEMENT, + "item_3_10", "item_3_20", "item_3_30", "item_3_50"); + Assert.assertEquals(expectedItems, actualItems); + + // removing items ----------------------------------------------------------------------- + model.removeElement("item_1_25", STUB_CONTRIBUTOR_1); + model.removeElement("item_3_20", STUB_CONTRIBUTOR_3); + model.removeElement("item_3_30", STUB_CONTRIBUTOR_3); + model.setHasMore(STUB_CONTRIBUTOR_1, false); + + actualItems = model.getItems(); + expectedItems = Arrays.asList("item_1_20", "item_1_35", + "item_2_05", "item_2_10", "item_2_23", SearchListModel.MORE_ELEMENT, + "item_3_10", "item_3_50"); + Assert.assertEquals(expectedItems, actualItems); } @NotNull diff --git a/platform/platform-api/src/com/intellij/execution/process/OSProcessHandler.java b/platform/platform-api/src/com/intellij/execution/process/OSProcessHandler.java index e22683b54886..32cc3ff0e47e 100644 --- a/platform/platform-api/src/com/intellij/execution/process/OSProcessHandler.java +++ b/platform/platform-api/src/com/intellij/execution/process/OSProcessHandler.java @@ -15,11 +15,14 @@ package com.intellij.execution.process; import com.intellij.execution.ExecutionException; import com.intellij.execution.configurations.GeneralCommandLine; +import com.intellij.openapi.application.Application; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.util.Key; import com.intellij.openapi.util.io.FileUtil; import com.intellij.openapi.vfs.encoding.EncodingManager; +import com.intellij.util.ExceptionUtil; +import com.intellij.util.containers.ContainerUtil; import com.intellij.util.io.BaseOutputReader; import gnu.trove.THashSet; import org.jetbrains.annotations.NotNull; @@ -32,6 +35,8 @@ import java.util.concurrent.Future; public class OSProcessHandler extends BaseOSProcessHandler { private static final Logger LOG = Logger.getInstance("#com.intellij.execution.process.OSProcessHandler"); + private static final Set REPORTED_EXECUTIONS = ContainerUtil.newConcurrentSet(); + private static final long ALLOWED_TIMEOUT_THRESHHOLD = 10; public static final Key> DELETE_FILES_ON_TERMINATION = Key.create("OSProcessHandler.FileToDelete"); @@ -56,6 +61,42 @@ public class OSProcessHandler extends BaseOSProcessHandler { } } + @Override + public boolean waitFor() { + checkEdtAndReadAction(this); + return super.waitFor(); + } + + @Override + public boolean waitFor(long timeoutInMilliseconds) { + if (timeoutInMilliseconds > ALLOWED_TIMEOUT_THRESHHOLD) { + checkEdtAndReadAction(this); + } + return super.waitFor(timeoutInMilliseconds); + } + + /** + * Checks if we are going to wait for {@code processHandler} to finish on EDT or under ReadAction. Logs error if we do so. + * + * @apiNote works only in internal mode with UI. Reports once per running session per stacktrace per cause. + */ + public static void checkEdtAndReadAction(@NotNull ProcessHandler processHandler) { + Application application = ApplicationManager.getApplication(); + if (!application.isInternal() || application.isHeadlessEnvironment()) { + return; + } + String message = null; + if (application.isDispatchThread()) { + message = "Synchronous execution on EDT: "; + } + else if (application.isReadAccessAllowed()) { + message = "Synchronous execution under ReadAction: "; + } + if (message != null && REPORTED_EXECUTIONS.add(ExceptionUtil.currentStackTrace())) { + LOG.error(message + processHandler); + } + } + private static void deleteTempFiles(Set tempFiles) { if (tempFiles != null) { try { diff --git a/platform/platform-api/src/com/intellij/ui/tabs/impl/JBTabsImpl.java b/platform/platform-api/src/com/intellij/ui/tabs/impl/JBTabsImpl.java index 3bb3c16e5f13..35068e724c30 100644 --- a/platform/platform-api/src/com/intellij/ui/tabs/impl/JBTabsImpl.java +++ b/platform/platform-api/src/com/intellij/ui/tabs/impl/JBTabsImpl.java @@ -1315,7 +1315,7 @@ public class JBTabsImpl extends JComponent @NotNull public List getTabs() { - //ApplicationManager.getApplication().assertIsDispatchThread(); + ApplicationManager.getApplication().assertIsDispatchThread(); if (myAllTabs != null) return myAllTabs; ArrayList result = new ArrayList<>(myVisibleInfos); diff --git a/platform/platform-api/src/com/intellij/ui/treeStructure/SimpleTree.java b/platform/platform-api/src/com/intellij/ui/treeStructure/SimpleTree.java index cc8217246067..3676093963bd 100644 --- a/platform/platform-api/src/com/intellij/ui/treeStructure/SimpleTree.java +++ b/platform/platform-api/src/com/intellij/ui/treeStructure/SimpleTree.java @@ -554,39 +554,38 @@ public class SimpleTree extends Tree implements CellEditorListener { myExpandedHandle = null; myCollapsedHandle = null; - myExpandedHandle = null; + myEmptyHandle = null; } + @Deprecated public Icon getHandleIcon(DefaultMutableTreeNode node, TreePath path) { if (node.getChildCount() == 0) return getEmptyHandle(); - - return isExpanded(path) ? getExpandedHandle() : getCollapsedHandle(); } + @Deprecated public Icon getExpandedHandle() { if (myExpandedHandle == null) { myExpandedHandle = UIUtil.getTreeExpandedIcon(); } - return myExpandedHandle; } + @Deprecated public Icon getCollapsedHandle() { if (myCollapsedHandle == null) { myCollapsedHandle = UIUtil.getTreeCollapsedIcon(); } - return myCollapsedHandle; } + @Deprecated public Icon getEmptyHandle() { if (myEmptyHandle == null) { final Icon expand = getExpandedHandle(); myEmptyHandle = expand != null ? EmptyIcon.create(expand) : EmptyIcon.create(0); } - return myEmptyHandle; } diff --git a/platform/platform-impl/src/com/intellij/openapi/application/ConfigImportHelper.java b/platform/platform-impl/src/com/intellij/openapi/application/ConfigImportHelper.java index f01d39ee6398..7cf4b83841c9 100644 --- a/platform/platform-impl/src/com/intellij/openapi/application/ConfigImportHelper.java +++ b/platform/platform-impl/src/com/intellij/openapi/application/ConfigImportHelper.java @@ -159,7 +159,10 @@ public class ConfigImportHelper { } final List candidates; - try (DirectoryStream stream = Files.newDirectoryStream(configsHome, it -> StringUtil.startsWithIgnoreCase(it.getFileName().toString(), prefix))) { + try (DirectoryStream stream = Files.newDirectoryStream(configsHome, it -> { + //noinspection CodeBlock2Expr + return StringUtil.startsWithIgnoreCase(it.getFileName().toString(), prefix) && !it.equals(isMacOs ? newConfigDir : newConfigDir.getParent()); + })) { candidates = ContainerUtilRt.newArrayList(stream); } catch (IOException ignore) { @@ -194,7 +197,13 @@ public class ConfigImportHelper { for (Object key : fileToLastModified.keys()) { result.add((Path)key); } - result.sort((o1, o2) -> (int)(fileToLastModified.get(o2) - fileToLastModified.get(o1))); + result.sort((o1, o2) -> { + int diff = (int)(fileToLastModified.get(o2) - fileToLastModified.get(o1)); + if (diff == 0) { + return StringUtil.naturalCompare(o2.toString(), o1.toString()); + } + return diff; + }); return result; } diff --git a/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/FileEditorManagerImpl.java b/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/FileEditorManagerImpl.java index cdc648865c04..d9a4956f5469 100644 --- a/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/FileEditorManagerImpl.java +++ b/platform/platform-impl/src/com/intellij/openapi/fileEditor/impl/FileEditorManagerImpl.java @@ -1041,6 +1041,7 @@ public class FileEditorManagerImpl extends FileEditorManagerEx implements Persis @Override public void setSelectedEditor(@NotNull VirtualFile file, @NotNull String fileEditorProviderId) { + ApplicationManager.getApplication().assertIsDispatchThread(); EditorWithProviderComposite composite = getCurrentEditorWithProviderComposite(file); if (composite == null) { final List composites = getEditorComposites(file); @@ -1310,6 +1311,7 @@ public class FileEditorManagerImpl extends FileEditorManagerEx implements Persis @Override @Nullable public FileEditorWithProvider getSelectedEditorWithProvider(@NotNull VirtualFile file) { + ApplicationManager.getApplication().assertIsDispatchThread(); if (file instanceof VirtualFileWindow) file = ((VirtualFileWindow)file).getDelegate(); final EditorWithProviderComposite composite = getCurrentEditorWithProviderComposite(file); if (composite != null) { @@ -1323,8 +1325,7 @@ public class FileEditorManagerImpl extends FileEditorManagerEx implements Persis @Override @NotNull public Pair getEditorsWithProviders(@NotNull final VirtualFile file) { - assertReadAccess(); - + ApplicationManager.getApplication().assertIsDispatchThread(); final EditorWithProviderComposite composite = getCurrentEditorWithProviderComposite(file); if (composite != null) { return Pair.create(composite.getEditors(), composite.getProviders()); diff --git a/platform/platform-impl/src/com/intellij/openapi/options/newEditor/SettingsTreeView.java b/platform/platform-impl/src/com/intellij/openapi/options/newEditor/SettingsTreeView.java index 7b1344925507..90410d0e5920 100644 --- a/platform/platform-impl/src/com/intellij/openapi/options/newEditor/SettingsTreeView.java +++ b/platform/platform-impl/src/com/intellij/openapi/options/newEditor/SettingsTreeView.java @@ -19,6 +19,8 @@ import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.registry.Registry; import com.intellij.ui.*; import com.intellij.ui.components.GradientViewport; +import com.intellij.ui.tree.ui.Control; +import com.intellij.ui.tree.ui.DefaultControl; import com.intellij.ui.treeStructure.*; import com.intellij.ui.treeStructure.filtered.FilteringTreeBuilder; import com.intellij.ui.treeStructure.filtered.FilteringTreeStructure; @@ -74,6 +76,8 @@ public class SettingsTreeView extends JComponent implements Accessible, Disposab private Configurable myQueuedConfigurable; private boolean myPaintInternalInfo; + private MyControl myControl; + public SettingsTreeView(SettingsFilter filter, ConfigurableGroup[] groups) { myFilter = filter; myRoot = new MyRoot(groups); @@ -123,7 +127,7 @@ public class SettingsTreeView extends JComponent implements Accessible, Disposab myHeader.setBorder(BorderFactory.createEmptyBorder(1, 10 + getLeftMargin(0), 0, 0)); } myHeader.setFont(myTree.getFont()); - myHeader.setIcon(myTree.getEmptyHandle()); + myHeader.setIcon(getIcon(null)); int height = myHeader.getPreferredSize().height; String group = findGroupNameAt(0, height + 3); if (group == null || !group.equals(findGroupNameAt(0, 0))) { @@ -181,6 +185,48 @@ public class SettingsTreeView extends JComponent implements Accessible, Disposab Disposer.register(this, myBuilder); } + @Override + public void updateUI() { + super.updateUI(); + myControl = null; + } + + private Icon getIcon(@Nullable DefaultMutableTreeNode node) { + if (myControl == null) myControl = new MyControl(); + if (node == null || 0 == node.getChildCount()) return myControl.empty; + return myTree.isExpanded(new TreePath(node.getPath())) ? myControl.expanded : myControl.collapsed; + } + + private static final class MyControl { + private final Control control = new DefaultControl(); + private final Icon collapsed = new MyIcon(false); + private final Icon expanded = new MyIcon(true); + private final Icon empty = new MyIcon(null); + + private final class MyIcon implements Icon { + private final Boolean expanded; + + private MyIcon(@Nullable Boolean expanded) { + this.expanded = expanded; + } + + @Override + public int getIconWidth() { + return control.getWidth(); + } + + @Override + public int getIconHeight() { + return control.getHeight(); + } + + @Override + public void paintIcon(Component c, Graphics g, int x, int y) { + if (expanded != null) control.paint(g, x, y, getIconWidth(), getIconHeight(), expanded, false); + } + } + } + private static void setComponentPopupMenuTo(JTree tree) { tree.setComponentPopupMenu(new JPopupMenu() { private Transferable transferable; @@ -584,15 +630,7 @@ public class SettingsTreeView extends JComponent implements Accessible, Disposab // configure node icon Icon nodeIcon = null; if (value instanceof DefaultMutableTreeNode) { - DefaultMutableTreeNode treeNode = (DefaultMutableTreeNode)value; - if (0 == treeNode.getChildCount()) { - nodeIcon = myTree.getEmptyHandle(); - } - else { - nodeIcon = myTree.isExpanded(new TreePath(treeNode.getPath())) - ? myTree.getExpandedHandle() - : myTree.getCollapsedHandle(); - } + nodeIcon = getIcon((DefaultMutableTreeNode)value); } myNodeIcon.setIcon(nodeIcon); if (node != null && myPaintInternalInfo) { diff --git a/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/persistent/LocalFileSystemRefreshWorker.java b/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/persistent/LocalFileSystemRefreshWorker.java index b48f051edbec..a45de1715ab8 100644 --- a/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/persistent/LocalFileSystemRefreshWorker.java +++ b/platform/platform-impl/src/com/intellij/openapi/vfs/newvfs/persistent/LocalFileSystemRefreshWorker.java @@ -6,6 +6,7 @@ import com.intellij.openapi.application.ReadAction; import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.SystemInfo; import com.intellij.openapi.util.io.FileAttributes; +import com.intellij.openapi.util.registry.Registry; import com.intellij.openapi.vfs.VFileProperty; import com.intellij.openapi.vfs.VfsUtil; import com.intellij.openapi.vfs.VirtualFile; @@ -31,21 +32,21 @@ import java.nio.file.attribute.DosFileAttributes; import java.nio.file.attribute.PosixFileAttributes; import java.nio.file.attribute.PosixFilePermission; import java.util.*; +import java.util.concurrent.ForkJoinPool; +import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicLong; import static com.intellij.openapi.vfs.newvfs.persistent.VfsEventGenerationHelper.LOG; class LocalFileSystemRefreshWorker { private final boolean myIsRecursive; - private final Queue myRefreshQueue = new Queue<>(100); + private final NewVirtualFile myRefreshRoot; private final VfsEventGenerationHelper myHelper = new VfsEventGenerationHelper(); private volatile boolean myCancelled; LocalFileSystemRefreshWorker(@NotNull NewVirtualFile refreshRoot, boolean isRecursive) { myIsRecursive = isRecursive; - myRefreshQueue.addLast(refreshRoot); + myRefreshRoot = refreshRoot; } @NotNull @@ -58,7 +59,7 @@ class LocalFileSystemRefreshWorker { } public void scan() { - NewVirtualFile root = myRefreshQueue.pullFirst(); + NewVirtualFile root = myRefreshRoot; boolean rootDirty = root.isDirty(); if (LOG.isDebugEnabled()) LOG.debug("root=" + root + " dirty=" + rootDirty); if (!rootDirty) return; @@ -74,72 +75,118 @@ class LocalFileSystemRefreshWorker { fs = PersistentFS.replaceWithNativeFS(fs); } - myRefreshQueue.addLast(root); - - try { - processQueue(fs, PersistentFS.getInstance()); - } - catch (RefreshCancelledException e) { - LOG.debug("refresh cancelled"); - } + RefreshContext context = createRefreshContext(fs, PersistentFS.getInstance(), FilePathHashingStrategy.create(fs.isCaseSensitive())); + context.submitRefreshRequest(() -> processFile(root, context)); + context.waitForRefreshToFinish(); } - private void processQueue(@NotNull NewVirtualFileSystem fs, @NotNull PersistentFS persistence) throws RefreshCancelledException { - TObjectHashingStrategy strategy = FilePathHashingStrategy.create(fs.isCaseSensitive()); + private RefreshContext createRefreshContext(NewVirtualFileSystem fs, PersistentFS persistentFS, TObjectHashingStrategy strategy) { + int parallelism = Registry.intValue("vfs.use.nio-based.local.refresh.worker.parallelism", Runtime.getRuntime().availableProcessors() - 1); + + if (myIsRecursive && parallelism > 0) { + final ForkJoinPool pool = new ForkJoinPool(parallelism); - while (!myRefreshQueue.isEmpty()) { - NewVirtualFile file = myRefreshQueue.pullFirst(); - if (!myHelper.checkDirty(file)) continue; - - checkCancelled(file); - - if (file.isDirectory()) { - boolean fullSync = ((VirtualDirectoryImpl)file).allChildrenLoaded(); - if (fullSync) { - fullDirRefresh(fs, persistence, strategy, (VirtualDirectoryImpl)file); + return new RefreshContext(fs, persistentFS, strategy) { + @Override + void submitRefreshRequest(Runnable action) { + pool.submit(action); } - else { - partialDirRefresh(fs, persistence, strategy, (VirtualDirectoryImpl)file); + + @Override + void doWaitForRefreshToFinish() { + pool.awaitQuiescence(1, TimeUnit.DAYS); + pool.shutdown(); } + }; + } + + return new RefreshContext(fs, persistentFS, strategy) { + final Queue myRefreshRequests = new Queue<>(100); + + @Override + void submitRefreshRequest(Runnable request) { + myRefreshRequests.addLast(request); + } + + @Override + void doWaitForRefreshToFinish() { + while(!myRefreshRequests.isEmpty()) { + Runnable request = myRefreshRequests.pullFirst(); + request.run(); + } + } + }; + } + + private void processFile(NewVirtualFile file, RefreshContext refreshContext) { + if (!myHelper.checkDirty(file)) { + return; + } + + if(checkCancelled(file, refreshContext)) return; + + if (file.isDirectory()) { + boolean fullSync = ((VirtualDirectoryImpl)file).allChildrenLoaded(); + if (fullSync) { + fullDirRefresh((VirtualDirectoryImpl)file, refreshContext); } else { - refreshFile(fs, persistence, strategy, file); + partialDirRefresh((VirtualDirectoryImpl)file, refreshContext); } + } + else { + refreshFile(file, refreshContext); + } - if (myIsRecursive || !file.isDirectory()) { - file.markClean(); + if(checkCancelled(file, refreshContext)) return; + + if (myIsRecursive || !file.isDirectory()) { + file.markClean(); + } + } + + private static abstract class RefreshContext { + final NewVirtualFileSystem fs; + final PersistentFS persistence; + final TObjectHashingStrategy strategy; + final LinkedBlockingQueue filesToBecomeDirty = new LinkedBlockingQueue<>(); + + RefreshContext(NewVirtualFileSystem fs, PersistentFS persistence, TObjectHashingStrategy strategy) { + this.fs = fs; + this.persistence = persistence; + this.strategy = strategy; + } + + abstract void submitRefreshRequest(Runnable action); + abstract void doWaitForRefreshToFinish(); + + final void waitForRefreshToFinish() { + doWaitForRefreshToFinish(); + + for (NewVirtualFile file : filesToBecomeDirty) { + forceMarkDirty(file); } } } - private void refreshFile(@NotNull NewVirtualFileSystem fs, - @NotNull PersistentFS persistence, - @NotNull TObjectHashingStrategy strategy, - @NotNull NewVirtualFile file) { - RefreshingFileVisitor refreshingFileVisitor = new RefreshingFileVisitor(file, persistence, fs, - null, - Collections.singletonList(file), strategy); + private void refreshFile(@NotNull NewVirtualFile file, RefreshContext refreshContext) { + RefreshingFileVisitor refreshingFileVisitor = new RefreshingFileVisitor(file, refreshContext, null, + Collections.singletonList(file)); refreshingFileVisitor.visit(file); myHelper.addAllEventsFrom(refreshingFileVisitor.getHelper()); } - private static final AtomicInteger myRequests = new AtomicInteger(); - private static final AtomicLong myTime = new AtomicLong(); - - private void fullDirRefresh(@NotNull NewVirtualFileSystem fs, - @NotNull PersistentFS persistence, - @NotNull TObjectHashingStrategy strategy, - @NotNull VirtualDirectoryImpl dir) { + private void fullDirRefresh(@NotNull VirtualDirectoryImpl dir, RefreshContext refreshContext) { while (true) { // obtaining directory snapshot - Pair result = getDirectorySnapshot(persistence, dir); + Pair result = getDirectorySnapshot(refreshContext.persistence, dir); if (result == null) return; String[] currentNames = result.getFirst(); VirtualFile[] children = result.getSecond(); RefreshingFileVisitor refreshingFileVisitor = - new RefreshingFileVisitor(dir, persistence, fs, null, Arrays.asList(children), strategy); + new RefreshingFileVisitor(dir, refreshContext, null, Arrays.asList(children)); refreshingFileVisitor.visit(dir); @@ -148,7 +195,7 @@ class LocalFileSystemRefreshWorker { if (ApplicationManager.getApplication().isDisposed()) { return true; } - if (!Arrays.equals(currentNames, persistence.list(dir)) || !Arrays.equals(children, dir.getChildren())) { + if (!Arrays.equals(currentNames, refreshContext.persistence.list(dir)) || !Arrays.equals(children, dir.getChildren())) { if (LOG.isDebugEnabled()) LOG.debug("retry: " + dir); return false; } @@ -171,10 +218,7 @@ class LocalFileSystemRefreshWorker { }); } - private void partialDirRefresh(@NotNull NewVirtualFileSystem fs, - @NotNull PersistentFS persistence, - @NotNull TObjectHashingStrategy strategy, - @NotNull VirtualDirectoryImpl dir) { + private void partialDirRefresh(@NotNull VirtualDirectoryImpl dir, RefreshContext refreshContext) { while (true) { // obtaining directory snapshot Pair, List> result = @@ -184,7 +228,7 @@ class LocalFileSystemRefreshWorker { List wanted = result.getSecond(); if (cached.isEmpty() && wanted.isEmpty()) return; - RefreshingFileVisitor refreshingFileVisitor = new RefreshingFileVisitor(dir, persistence, fs, wanted, cached, strategy); + RefreshingFileVisitor refreshingFileVisitor = new RefreshingFileVisitor(dir, refreshContext, wanted, cached); refreshingFileVisitor.visit(dir); // generating events unless a directory was changed in between @@ -204,18 +248,14 @@ class LocalFileSystemRefreshWorker { } } - private static class RefreshCancelledException extends RuntimeException { - } - - private void checkCancelled(@NotNull NewVirtualFile stopAt) { - if (myCancelled || ourCancellingCondition != null && ourCancellingCondition.fun(stopAt)) { - forceMarkDirty(stopAt); - while (!myRefreshQueue.isEmpty()) { - NewVirtualFile next = myRefreshQueue.pullFirst(); - forceMarkDirty(next); - } - throw new RefreshCancelledException(); + private boolean checkCancelled(@NotNull NewVirtualFile stopAt, RefreshContext refreshContext) { + boolean myRequestedCancel = false; + if (myCancelled || (myRequestedCancel = ourCancellingCondition != null && ourCancellingCondition.fun(stopAt))) { + if (myRequestedCancel) myCancelled = true; + refreshContext.filesToBecomeDirty.offer(stopAt); + return true; } + return false; } private static void forceMarkDirty(@NotNull NewVirtualFile file) { @@ -237,20 +277,16 @@ class LocalFileSystemRefreshWorker { private final Set myChildrenWeAreInterested; // null - no limit private final VirtualFile myFileOrDir; - private final PersistentFS myPersistence; - private final NewVirtualFileSystem myFs; + private final RefreshContext myRefreshContext; RefreshingFileVisitor(@NotNull VirtualFile fileOrDir, - @NotNull PersistentFS persistence, - @NotNull NewVirtualFileSystem fs, + @NotNull RefreshContext refreshContext, @Nullable("null means all") Collection childrenToRefresh, - @NotNull Collection existingPersistentChildren, - @NotNull TObjectHashingStrategy strategy) { + @NotNull Collection existingPersistentChildren) { myFileOrDir = fileOrDir; - myPersistence = persistence; - myFs = fs; - myPersistentChildren = new THashMap<>(existingPersistentChildren.size(), strategy); - myChildrenWeAreInterested = childrenToRefresh != null ? new THashSet<>(childrenToRefresh, strategy) : null; + myRefreshContext = refreshContext; + myPersistentChildren = new THashMap<>(existingPersistentChildren.size(), refreshContext.strategy); + myChildrenWeAreInterested = childrenToRefresh != null ? new THashSet<>(childrenToRefresh, refreshContext.strategy) : null; for (VirtualFile child : existingPersistentChildren) { String name = child.getName(); @@ -275,7 +311,9 @@ class LocalFileSystemRefreshWorker { return FileVisitResult.CONTINUE; } - checkCancelled(child); + if(checkCancelled(child, myRefreshContext)) { + return FileVisitResult.CONTINUE; + } if (!child.isDirty()) { return FileVisitResult.CONTINUE; @@ -312,16 +350,16 @@ class LocalFileSystemRefreshWorker { } if (!directory) { - myHelper.checkContentChanged(child, myPersistence.getTimeStamp(child), attrs.lastModifiedTime().toMillis(), - myPersistence.getLastRecordedLength(child), attrs.size()); + myHelper.checkContentChanged(child, myRefreshContext.persistence.getTimeStamp(child), attrs.lastModifiedTime().toMillis(), + myRefreshContext.persistence.getLastRecordedLength(child), attrs.size()); } else { if (myIsRecursive) { - myRefreshQueue.addLast(child); + myRefreshContext.submitRefreshRequest(() -> processFile(child, myRefreshContext)); } } - boolean currentWritable = myPersistence.isWritable(child); + boolean currentWritable = myRefreshContext.persistence.isWritable(child); boolean isWritable; if (attrs instanceof DosFileAttributes) { @@ -342,7 +380,7 @@ class LocalFileSystemRefreshWorker { } if (isLink) { - myHelper.checkSymbolicLinkChange(child, child.getCanonicalPath(), myFs.resolveSymLink(child)); + myHelper.checkSymbolicLinkChange(child, child.getCanonicalPath(), myRefreshContext.fs.resolveSymLink(child)); } if (!child.isDirectory()) child.markClean(); } @@ -353,9 +391,7 @@ class LocalFileSystemRefreshWorker { return !VfsUtil.isBadName(name); } - public void visit(@NotNull VirtualFile fileOrDir) { - long started = System.nanoTime(); - + void visit(@NotNull VirtualFile fileOrDir) { try { Path path = Paths.get(fileOrDir.getPath()); if (fileOrDir.isDirectory()) { @@ -384,13 +420,6 @@ class LocalFileSystemRefreshWorker { catch (IOException ex) { LOG.error(ex); } - - int requests = myRequests.incrementAndGet(); - long l = myTime.addAndGet(System.nanoTime() - started); - - if (requests % 1000 == 0) { - System.out.println("refresh:" + myRequests + " for " + TimeUnit.NANOSECONDS.toMillis(l) + "ms"); - } } @NotNull diff --git a/platform/platform-impl/src/com/intellij/ui/tree/ui/DefaultControl.java b/platform/platform-impl/src/com/intellij/ui/tree/ui/DefaultControl.java index 09bb7d65b36c..061bdaded057 100644 --- a/platform/platform-impl/src/com/intellij/ui/tree/ui/DefaultControl.java +++ b/platform/platform-impl/src/com/intellij/ui/tree/ui/DefaultControl.java @@ -7,7 +7,7 @@ import org.jetbrains.annotations.NotNull; import java.awt.Graphics; import javax.swing.Icon; -final class DefaultControl implements Control { +public final class DefaultControl implements Control { private final CompoundIcon collapsedIcon = new CompoundIcon("treeCollapsed"); private final CompoundIcon expandedIcon = new CompoundIcon("treeExpanded"); diff --git a/platform/platform-resources/src/idea/LangActions.xml b/platform/platform-resources/src/idea/LangActions.xml index b2a0bcac6abf..4fc39c2983fa 100644 --- a/platform/platform-resources/src/idea/LangActions.xml +++ b/platform/platform-resources/src/idea/LangActions.xml @@ -470,7 +470,9 @@ + + diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/vfs/local/LocalFileSystemTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/vfs/local/LocalFileSystemTest.java index 908440cdc566..e84048a9417d 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/vfs/local/LocalFileSystemTest.java +++ b/platform/platform-tests/testSrc/com/intellij/openapi/vfs/local/LocalFileSystemTest.java @@ -655,7 +655,7 @@ public class LocalFileSystemTest extends BareTestFixtureTestCase { RefreshWorker.setCancellingCondition(null); topDir.refresh(false, true); - assertEquals(processed, files); + assertEquals(files, processed); } finally { connection.disconnect(); diff --git a/platform/script-debugger/backend/src/debugger/sourcemap/NestedSourceMap.kt b/platform/script-debugger/backend/src/debugger/sourcemap/NestedSourceMap.kt index c4a5ebfc9335..59e766ca01e0 100644 --- a/platform/script-debugger/backend/src/debugger/sourcemap/NestedSourceMap.kt +++ b/platform/script-debugger/backend/src/debugger/sourcemap/NestedSourceMap.kt @@ -51,10 +51,10 @@ class NestedSourceMap(private val childMap: SourceMap, private val parentMap: So override fun findSourceIndex(sourceFile: VirtualFile, localFileUrlOnly: Boolean): Int = parentMap.findSourceIndex(sourceFile, localFileUrlOnly) - override fun findSourceIndex(sourceUrls: List, + override fun findSourceIndex(sourceUrl: Url, sourceFile: VirtualFile?, resolver: Lazy?, - localFileUrlOnly: Boolean): Int = parentMap.findSourceIndex(sourceUrls, sourceFile, resolver, localFileUrlOnly) + localFileUrlOnly: Boolean): Int = parentMap.findSourceIndex(sourceUrl, sourceFile, resolver, localFileUrlOnly) override fun processSourceMappingsInLine(sourceIndex: Int, sourceLine: Int, mappingProcessor: MappingsProcessorInLine): Boolean { val childSourceMappings = childMap.findSourceMappings(sourceIndex) diff --git a/platform/script-debugger/backend/src/debugger/sourcemap/SourceMap.kt b/platform/script-debugger/backend/src/debugger/sourcemap/SourceMap.kt index 283a5b6cbd63..73f86f66cef3 100644 --- a/platform/script-debugger/backend/src/debugger/sourcemap/SourceMap.kt +++ b/platform/script-debugger/backend/src/debugger/sourcemap/SourceMap.kt @@ -34,10 +34,10 @@ interface SourceMap { fun findSourceMappings(sourceIndex: Int): Mappings - fun findSourceIndex(sourceUrls: List, sourceFile: VirtualFile?, resolver: Lazy?, localFileUrlOnly: Boolean): Int + fun findSourceIndex(sourceUrl: Url, sourceFile: VirtualFile?, resolver: Lazy?, localFileUrlOnly: Boolean): Int - fun findSourceMappings(sourceUrls: List, sourceFile: VirtualFile?, resolver: Lazy?, localFileUrlOnly: Boolean): Mappings? { - val sourceIndex = findSourceIndex(sourceUrls, sourceFile, resolver, localFileUrlOnly) + fun findSourceMappings(sourceUrl: Url, sourceFile: VirtualFile?, resolver: Lazy?, localFileUrlOnly: Boolean): Mappings? { + val sourceIndex = findSourceIndex(sourceUrl, sourceFile, resolver, localFileUrlOnly) return if (sourceIndex >= 0) findSourceMappings(sourceIndex) else null } @@ -48,8 +48,14 @@ interface SourceMap { fun processSourceMappingsInLine(sourceIndex: Int, sourceLine: Int, mappingProcessor: MappingsProcessorInLine): Boolean fun processSourceMappingsInLine(sourceUrls: List, sourceLine: Int, mappingProcessor: MappingsProcessorInLine, sourceFile: VirtualFile?, resolver: Lazy?, localFileUrlOnly: Boolean): Boolean { - val sourceIndex = findSourceIndex(sourceUrls, sourceFile, resolver, localFileUrlOnly) - return sourceIndex >= 0 && processSourceMappingsInLine(sourceIndex, sourceLine, mappingProcessor) + var result = false + for (sourceUrl in sourceUrls) { + val sourceIndex = findSourceIndex(sourceUrl, sourceFile, resolver, localFileUrlOnly) + if (sourceIndex >= 0 && processSourceMappingsInLine(sourceIndex, sourceLine, mappingProcessor)) { + result = true + } + } + return result } } @@ -62,8 +68,8 @@ class OneLevelSourceMap(override val outFile: String?, override val sources: Array get() = sourceResolver.canonicalizedUrls - override fun findSourceIndex(sourceUrls: List, sourceFile: VirtualFile?, resolver: Lazy?, localFileUrlOnly: Boolean): Int { - val index = sourceResolver.findSourceIndex(sourceUrls, sourceFile, localFileUrlOnly) + override fun findSourceIndex(sourceUrl: Url, sourceFile: VirtualFile?, resolver: Lazy?, localFileUrlOnly: Boolean): Int { + val index = sourceResolver.findSourceIndex(sourceUrl, sourceFile, localFileUrlOnly) if (index == -1 && resolver != null) { return resolver.value?.let { sourceResolver.findSourceIndex(it) } ?: -1 } diff --git a/platform/script-debugger/backend/src/debugger/sourcemap/SourceResolver.kt b/platform/script-debugger/backend/src/debugger/sourcemap/SourceResolver.kt index 2a9c868fd561..766f6db972f0 100644 --- a/platform/script-debugger/backend/src/debugger/sourcemap/SourceResolver.kt +++ b/platform/script-debugger/backend/src/debugger/sourcemap/SourceResolver.kt @@ -78,12 +78,10 @@ class SourceResolver(private val rawSources: List, return if (resolveByCanonicalizedUrls != -1) resolveByCanonicalizedUrls else resolver.resolve(rawSources) } - fun findSourceIndex(sourceUrls: List, sourceFile: VirtualFile?, localFileUrlOnly: Boolean): Int { - for (sourceUrl in sourceUrls) { - val index = canonicalizedUrlToSourceIndex.get(sourceUrl) - if (index != -1) { - return index - } + fun findSourceIndex(sourceUrl: Url, sourceFile: VirtualFile?, localFileUrlOnly: Boolean): Int { + val index = canonicalizedUrlToSourceIndex.get(sourceUrl) + if (index != -1) { + return index } if (sourceFile != null) { diff --git a/platform/util/resources/misc/registry.properties b/platform/util/resources/misc/registry.properties index d81adea7ac43..e73b40b09a3d 100644 --- a/platform/util/resources/misc/registry.properties +++ b/platform/util/resources/misc/registry.properties @@ -1657,6 +1657,8 @@ JavaScript.Language.Service.truncate.traced.messages=true JavaScript.Language.Service.truncate.traced.messages.description=Truncate traced JavaScript language Service messages in log vfs.use.nio-based.local.refresh.worker=false +vfs.use.nio-based.local.refresh.worker.parallelism=7 +vfs.use.nio-based.local.refresh.worker.parallelism.description=How many threads will be used to access file system for detecting changes. Positive value is best suited for SSD because it allows running many operations in parallel vfs.use.new.jar.handler=true vfs.filewatcher.works.in.async.way=true diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/dependency/SuspiciousPackagePrivateAccessInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/dependency/SuspiciousPackagePrivateAccessInspection.java index 03103ec3d6f7..e8c411826e41 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/dependency/SuspiciousPackagePrivateAccessInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/dependency/SuspiciousPackagePrivateAccessInspection.java @@ -15,6 +15,7 @@ import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; import com.intellij.psi.impl.source.resolve.JavaResolveUtil; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.psi.util.PsiTypesUtil; import com.intellij.refactoring.util.RefactoringUIUtil; import com.intellij.uast.UastVisitorAdapter; import com.intellij.ui.ContextHelpLabel; @@ -65,20 +66,49 @@ public class SuspiciousPackagePrivateAccessInspection extends AbstractBaseUastLo } PsiElement resolved = node.resolve(); if (resolved instanceof PsiMember) { - checkAccess(node.getSelector(), (PsiMember)resolved, receiver); + checkAccess(node.getSelector(), (PsiMember)resolved, getAccessObjectType(receiver)); } return true; } @Override public boolean visitSimpleNameReferenceExpression(@NotNull USimpleNameReferenceExpression node) { - PsiElement resolved = node.resolve(); - if (resolved instanceof PsiMember) { - checkAccess(node, (PsiMember)resolved, null); + UElement uastParent = node.getUastParent(); + //we should skip 'checkAccess' here if node is part of UQualifiedReferenceExpression or UCallExpression node, + // otherwise the same problem will be reported twice + if (!isSelectorOfQualifiedReference(node) + && !(uastParent instanceof UCallExpression && isMethodReferenceOfCallExpression(node, (UCallExpression)uastParent) + && (((UCallExpression)uastParent).getKind() == UastCallKind.CONSTRUCTOR_CALL || isSelectorOfQualifiedReference((UExpression)uastParent)))) { + PsiElement resolved = node.resolve(); + if (resolved instanceof PsiMember) { + checkAccess(node, (PsiMember)resolved, null); + } } return true; } + private boolean isSelectorOfQualifiedReference(@Nullable UExpression expression) { + if (expression == null) return false; + UElement parent = expression.getUastParent(); + return parent instanceof UQualifiedReferenceExpression + && referToSameSourceElement(expression, ((UQualifiedReferenceExpression)parent).getSelector()); + } + + private boolean isMethodReferenceOfCallExpression(@NotNull USimpleNameReferenceExpression expression, @NotNull UCallExpression parent) { + UElement methodIdentifier = parent.getMethodIdentifier(); + UReferenceExpression classReference = parent.getClassReference(); + if (methodIdentifier == null && classReference != null) { + methodIdentifier = classReference.getReferenceNameElement(); + } + return referToSameSourceElement(expression.getReferenceNameElement(), methodIdentifier); + } + + private boolean referToSameSourceElement(@Nullable UElement element1, @Nullable UElement element2) { + if (element1 == null || element2 == null) return false; + PsiElement sourcePsi1 = element1.getSourcePsi(); + return sourcePsi1 != null && sourcePsi1.equals(element2.getSourcePsi()); + } + @Override public boolean visitCallableReferenceExpression(@NotNull UCallableReferenceExpression node) { PsiElement resolve = node.resolve(); @@ -86,18 +116,50 @@ public class SuspiciousPackagePrivateAccessInspection extends AbstractBaseUastLo PsiMember member = (PsiMember)resolve; UElement sourceNode = getReferenceNameElement(node); if (sourceNode != null) { - checkAccess(sourceNode, member, node.getQualifierExpression()); + checkAccess(sourceNode, member, getAccessObjectType(node.getQualifierExpression())); } } return true; } - private void checkAccess(@NotNull UElement sourceNode, @NotNull PsiMember target, @Nullable UExpression receiver) { + @Override + public boolean visitTypeReferenceExpression(@NotNull UTypeReferenceExpression node) { + //in Kotlin implementation of UAST USimpleNameReferenceExpression::resolve returns null for reference to type in local variable declaration, + // so we need to have this case specifically + if (!(node.getSourcePsi() instanceof PsiTypeElement)) { + PsiClass resolved = PsiTypesUtil.getPsiClass(node.getType()); + if (resolved != null) { + checkAccess(node, resolved, null); + } + } + return true; + } + + @Override + public boolean visitCallExpression(@NotNull UCallExpression node) { + //regular method calls are handled by visitSimpleNameReferenceExpression or visitQualifiedReferenceExpression, but we need to handle + // constructor calls in a special way because they may refer to classes + if (!isSelectorOfQualifiedReference(node) && node.getKind() == UastCallKind.CONSTRUCTOR_CALL) { + PsiMethod resolved = node.resolve(); + if (resolved != null) { + checkAccess(node, resolved, null); + } + else { + UReferenceExpression classReference = node.getClassReference(); + PsiElement resolvedClass = classReference != null ? classReference.resolve() : null; + if (resolvedClass instanceof PsiClass) { + checkAccess(node, (PsiClass)resolvedClass, null); + } + } + } + return true; + } + + private void checkAccess(@NotNull UElement sourceNode, @NotNull PsiMember target, @Nullable PsiClass accessObjectType) { if (target.hasModifier(JvmModifier.PACKAGE_LOCAL)) { checkPackageLocalAccess(sourceNode, target, "package-private"); } - else if (target.hasModifier(JvmModifier.PROTECTED) && receiver != null - && !(receiver instanceof UThisExpression) && !(receiver instanceof USuperExpression) && !canAccessProtectedMember(receiver, sourceNode, target)) { + else if (target.hasModifier(JvmModifier.PROTECTED) && !canAccessProtectedMember(sourceNode, target, accessObjectType)) { checkPackageLocalAccess(sourceNode, target, "protected and used not through a subclass here"); } } @@ -132,22 +194,26 @@ public class SuspiciousPackagePrivateAccessInspection extends AbstractBaseUastLo return node; } - private static boolean canAccessProtectedMember(UExpression receiver, UElement sourceNode, PsiMember member) { - PsiClass memberClass = member.getContainingClass(); - if (memberClass == null) return false; + @Nullable + private static PsiClass getAccessObjectType(@Nullable UExpression receiver) { + if (receiver == null || receiver instanceof UThisExpression || receiver instanceof USuperExpression) { + return null; + } - PsiClass accessObjectType; PsiType type = receiver.getExpressionType(); if (type != null) { - if (!(type instanceof PsiClassType)) return false; - accessObjectType = ((PsiClassType)type).resolve(); - if (accessObjectType == null) return false; + if (!(type instanceof PsiClassType)) return null; + return ((PsiClassType)type).resolve(); } else { PsiElement element = ((UReferenceExpression)receiver).resolve(); - if (!(element instanceof PsiClass)) return false; - accessObjectType = (PsiClass)element; + return element instanceof PsiClass ? (PsiClass)element : null; } + } + + private static boolean canAccessProtectedMember(UElement sourceNode, PsiMember member, PsiClass accessObjectType) { + PsiClass memberClass = member.getContainingClass(); + if (memberClass == null) return false; PsiElement sourcePsi = sourceNode.getSourcePsi(); UClass sourceClass = UastUtils.findContaining(sourcePsi, UClass.class); diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/dep/xxx/InnerClasses.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/dep/xxx/InnerClasses.java new file mode 100644 index 000000000000..5c465b31df5b --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/dep/xxx/InnerClasses.java @@ -0,0 +1,16 @@ +package xxx; + +public class InnerClasses { + static class PackagePrivateInnerClass { + } + + static class PackagePrivateInnerClassWithConstructor { + PackagePrivateInnerClassWithConstructor() { + } + } + + public static class ClassWithPackagePrivateConstructor { + ClassWithPackagePrivateConstructor() { + } + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/dep/xxx/ProtectedConstructors.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/dep/xxx/ProtectedConstructors.java new file mode 100644 index 000000000000..5d30de8de61a --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/dep/xxx/ProtectedConstructors.java @@ -0,0 +1,9 @@ +package xxx; + +public class ProtectedConstructors { + protected ProtectedConstructors() { + } + + protected ProtectedConstructors(int i) { + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/dep/xxx/PublicClass.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/dep/xxx/PublicClass.java index 22012cd714b6..02fdb18bc7f3 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/dep/xxx/PublicClass.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/dep/xxx/PublicClass.java @@ -9,4 +9,8 @@ public class PublicClass { public void publicMethod() {} void packagePrivateMethod() {} + + PublicClass() {} + public PublicClass(int i) {} + PublicClass(boolean b) {} } \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/dep/xxx/PublicClassWithDefaultConstructor.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/dep/xxx/PublicClassWithDefaultConstructor.java new file mode 100644 index 000000000000..c71e3139f08c --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/dep/xxx/PublicClassWithDefaultConstructor.java @@ -0,0 +1,5 @@ +// 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 xxx; + +public class PublicClassWithDefaultConstructor { +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/src/AccessingPackagePrivateMembers.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/src/AccessingPackagePrivateMembers.java index 097d61e795cf..5c609bca49dd 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/src/AccessingPackagePrivateMembers.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/src/AccessingPackagePrivateMembers.java @@ -7,19 +7,23 @@ import static xxx.StaticMembers.*; * @see PublicClass#packagePrivateField */ public class AccessingPackagePrivateMembers { - Object field = new PackagePrivateClass(); + static Object staticField = new PackagePrivateClass(); + Object field = new PackagePrivateClass(); { - new PackagePrivateClass(); + new PackagePrivateClass(); } static { - new PackagePrivateClass(); + new PackagePrivateClass(); } public void main() { - new PackagePrivateClass(); + new PackagePrivateClass(); PackagePrivateClass variable; - PublicClass aClass = new PublicClass(); + PublicClass aClass = new PublicClass(1); + PublicClassWithDefaultConstructor aClass2 = new PublicClassWithDefaultConstructor(); + new PublicClass(); + new PublicClass(true); System.out.println(aClass.publicField); System.out.println(aClass.packagePrivateField); @@ -32,5 +36,9 @@ public class AccessingPackagePrivateMembers { System.out.println(IMPORTED_FIELD); importedMethod(); + + new InnerClasses.PackagePrivateInnerClass(); + new InnerClasses.PackagePrivateInnerClassWithConstructor(); + new InnerClasses.ClassWithPackagePrivateConstructor(); } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/src/AccessingProtectedMembers.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/src/AccessingProtectedMembers.java index 09d2a28d15d4..bc20f0d11cef 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/src/AccessingProtectedMembers.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/src/AccessingProtectedMembers.java @@ -5,6 +5,10 @@ class AccessingProtectedMembersNotFromSubclass { ProtectedMembers aClass = new ProtectedMembers(); aClass.method(); ProtectedMembers.staticMethod(); + new ProtectedConstructors(); + new ProtectedConstructors(1); + new ProtectedConstructors() {}; + new ProtectedConstructors(1) {}; } } @@ -21,6 +25,13 @@ class AccessingProtectedMembersFromSubclass extends ProtectedMembers { ProtectedMembers.StaticInner inner1; StaticInner inner2; + + new Runnable() { + public void run() { + method(); + staticMethod(); + } + }; } public static class StaticInnerImpl1 extends ProtectedMembers.StaticInner { @@ -28,4 +39,26 @@ class AccessingProtectedMembersFromSubclass extends ProtectedMembers { public static class StaticInnerImpl2 extends StaticInner { } + + public class OwnInner { + void bar() { + method(); + staticMethod(); + } + } + + public static class OwnStaticInner { + void bar() { + staticMethod(); + } + } +} + +class AccessingDefaultProtectedConstructorFromSubclass extends ProtectedConstructors { +} + +class AccessingProtectedConstructorFromSubclass extends ProtectedConstructors { + AccessingProtectedConstructorFromSubclass() { + super(1); + } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/dependency/SuspiciousPackagePrivateAccessInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/dependency/SuspiciousPackagePrivateAccessInspectionTest.java index fed29d115f44..126d372e89d3 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/dependency/SuspiciousPackagePrivateAccessInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/dependency/SuspiciousPackagePrivateAccessInspectionTest.java @@ -1,27 +1,8 @@ // 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.siyeh.ig.dependency; -import com.intellij.codeInspection.InspectionProfileEntry; -import com.intellij.openapi.application.WriteAction; -import com.intellij.openapi.module.Module; -import com.intellij.openapi.module.ModuleManager; -import com.intellij.openapi.project.Project; -import com.intellij.openapi.roots.LanguageLevelModuleExtension; -import com.intellij.openapi.roots.ModuleRootModificationUtil; -import com.intellij.openapi.util.io.FileUtil; -import com.intellij.openapi.vfs.VirtualFile; -import com.intellij.pom.java.LanguageLevel; -import com.intellij.testFramework.LightProjectDescriptor; -import com.siyeh.ig.LightInspectionTestCase; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; -import org.jetbrains.jps.model.java.JavaSourceRootType; - -import java.io.File; -import java.io.IOException; - -public class SuspiciousPackagePrivateAccessInspectionTest extends LightInspectionTestCase { - private final ProjectWithDepModuleDescriptor myProjectDescriptor = new ProjectWithDepModuleDescriptor(LanguageLevel.HIGHEST); +public class SuspiciousPackagePrivateAccessInspectionTest extends SuspiciousPackagePrivateAccessInspectionTestCase { + public SuspiciousPackagePrivateAccessInspectionTest() {super("java");} public void testAccessingPackagePrivateMembers() { doTestWithDependency(); @@ -30,77 +11,4 @@ public class SuspiciousPackagePrivateAccessInspectionTest extends LightInspectio public void testAccessingProtectedMembers() { doTestWithDependency(); } - - @Override - protected void setUp() throws Exception { - super.setUp(); - myFixture.copyDirectoryToProject("dep", ProjectWithDepModuleDescriptor.getDepModuleSourceRoot()); - } - - @Override - protected void tearDown() throws Exception { - try { - myProjectDescriptor.cleanUpSources(); - } - catch (Throwable e) { - addSuppressedException(e); - } - finally { - super.tearDown(); - } - } - - private void doTestWithDependency() { - myFixture.configureByFile("src/" + getTestName(false) + ".java"); - myFixture.testHighlighting(true, false, false); - } - - @NotNull - @Override - protected LightProjectDescriptor getProjectDescriptor() { - return myProjectDescriptor; - } - - @Nullable - @Override - protected InspectionProfileEntry getInspection() { - return new SuspiciousPackagePrivateAccessInspection(); - } - - private static class ProjectWithDepModuleDescriptor extends ProjectDescriptor { - private static final String DEP_MODULE_SOURCE_ROOT = "dep-module-src"; - private VirtualFile mySourceRoot; - - ProjectWithDepModuleDescriptor(@NotNull LanguageLevel languageLevel) { - super(languageLevel); - } - - @Override - public void setUpProject(@NotNull Project project, @NotNull SetupHandler handler) throws Exception { - super.setUpProject(project, handler); - WriteAction.run(() -> { - Module mainModule = ModuleManager.getInstance(project).findModuleByName(TEST_MODULE_NAME); - File depModuleDir = FileUtil.createTempDirectory("dep-module-", null); - Module depModule = createModule(project, depModuleDir + "/dep.iml"); - ModuleRootModificationUtil.updateModel(depModule, model -> { - model.getModuleExtension(LanguageLevelModuleExtension.class).setLanguageLevel(myLanguageLevel); - model.setSdk(getSdk()); - mySourceRoot = createSourceRoot(depModule, DEP_MODULE_SOURCE_ROOT); - model.addContentEntry(mySourceRoot).addSourceFolder(mySourceRoot, JavaSourceRootType.SOURCE); - }); - ModuleRootModificationUtil.addDependency(mainModule, depModule); - }); - } - - public void cleanUpSources() throws IOException { - if (mySourceRoot != null) { - WriteAction.run(() -> mySourceRoot.delete(this)); - } - } - - @NotNull - private static String getDepModuleSourceRoot() { - return "../" + DEP_MODULE_SOURCE_ROOT; - } - } } diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/dependency/SuspiciousPackagePrivateAccessInspectionTestCase.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/dependency/SuspiciousPackagePrivateAccessInspectionTestCase.java new file mode 100644 index 000000000000..53b9164c761c --- /dev/null +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/dependency/SuspiciousPackagePrivateAccessInspectionTestCase.java @@ -0,0 +1,103 @@ +// Copyright 2000-2019 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.siyeh.ig.dependency; + +import com.intellij.codeInspection.InspectionProfileEntry; +import com.intellij.openapi.application.WriteAction; +import com.intellij.openapi.module.Module; +import com.intellij.openapi.module.ModuleManager; +import com.intellij.openapi.project.Project; +import com.intellij.openapi.roots.LanguageLevelModuleExtension; +import com.intellij.openapi.roots.ModuleRootModificationUtil; +import com.intellij.openapi.util.io.FileUtil; +import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.pom.java.LanguageLevel; +import com.intellij.testFramework.LightProjectDescriptor; +import com.siyeh.ig.LightInspectionTestCase; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.jetbrains.jps.model.java.JavaSourceRootType; + +import java.io.File; +import java.io.IOException; + +public class SuspiciousPackagePrivateAccessInspectionTestCase extends LightInspectionTestCase { + private final ProjectWithDepModuleDescriptor myProjectDescriptor = new ProjectWithDepModuleDescriptor(LanguageLevel.HIGHEST); + private final String myExtension; + + public SuspiciousPackagePrivateAccessInspectionTestCase(String extension) { + myExtension = extension; + } + + @Override + protected void setUp() throws Exception { + super.setUp(); + myFixture.copyDirectoryToProject("dep", ProjectWithDepModuleDescriptor.getDepModuleSourceRoot()); + } + + @Override + protected void tearDown() throws Exception { + try { + myProjectDescriptor.cleanUpSources(); + } + catch (Throwable e) { + addSuppressedException(e); + } + finally { + super.tearDown(); + } + } + + protected void doTestWithDependency() { + myFixture.configureByFile("src/" + getTestName(false) + "." + myExtension); + myFixture.testHighlighting(true, false, false); + } + + @NotNull + @Override + protected LightProjectDescriptor getProjectDescriptor() { + return myProjectDescriptor; + } + + @Nullable + @Override + protected InspectionProfileEntry getInspection() { + return new SuspiciousPackagePrivateAccessInspection(); + } + + private static class ProjectWithDepModuleDescriptor extends ProjectDescriptor { + private static final String DEP_MODULE_SOURCE_ROOT = "dep-module-src"; + private VirtualFile mySourceRoot; + + ProjectWithDepModuleDescriptor(@NotNull LanguageLevel languageLevel) { + super(languageLevel); + } + + @Override + public void setUpProject(@NotNull Project project, @NotNull SetupHandler handler) throws Exception { + super.setUpProject(project, handler); + WriteAction.run(() -> { + Module mainModule = ModuleManager.getInstance(project).findModuleByName(TEST_MODULE_NAME); + File depModuleDir = FileUtil.createTempDirectory("dep-module-", null); + Module depModule = createModule(project, depModuleDir + "/dep.iml"); + ModuleRootModificationUtil.updateModel(depModule, model -> { + model.getModuleExtension(LanguageLevelModuleExtension.class).setLanguageLevel(myLanguageLevel); + model.setSdk(getSdk()); + mySourceRoot = createSourceRoot(depModule, DEP_MODULE_SOURCE_ROOT); + model.addContentEntry(mySourceRoot).addSourceFolder(mySourceRoot, JavaSourceRootType.SOURCE); + }); + ModuleRootModificationUtil.addDependency(mainModule, depModule); + }); + } + + public void cleanUpSources() throws IOException { + if (mySourceRoot != null) { + WriteAction.run(() -> mySourceRoot.delete(this)); + } + } + + @NotNull + private static String getDepModuleSourceRoot() { + return "../" + DEP_MODULE_SOURCE_ROOT; + } + } +} diff --git a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/untypedUnresolvedAccess/GrUnresolvedAccessChecker.java b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/untypedUnresolvedAccess/GrUnresolvedAccessChecker.java index 6eea5023dfb1..f5223e765e84 100644 --- a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/untypedUnresolvedAccess/GrUnresolvedAccessChecker.java +++ b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/untypedUnresolvedAccess/GrUnresolvedAccessChecker.java @@ -1,6 +1,4 @@ -// Copyright 2000-2017 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. +// Copyright 2000-2019 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 org.jetbrains.plugins.groovy.codeInspection.untypedUnresolvedAccess; import com.intellij.codeHighlighting.HighlightDisplayLevel; @@ -9,6 +7,7 @@ import com.intellij.codeInsight.daemon.impl.HighlightInfo; import com.intellij.codeInsight.daemon.impl.HighlightInfoType; import com.intellij.codeInsight.daemon.impl.quickfix.QuickFixAction; import com.intellij.codeInsight.intention.EmptyIntentionAction; +import com.intellij.codeInsight.intention.IntentionAction; import com.intellij.codeInsight.intention.QuickFixFactory; import com.intellij.codeInsight.quickfix.UnresolvedReferenceQuickFixProvider; import com.intellij.openapi.diagnostic.Logger; @@ -32,20 +31,14 @@ import org.jetbrains.plugins.groovy.codeInspection.GrInspectionUtil; import org.jetbrains.plugins.groovy.codeInspection.GroovyQuickFixFactory; import org.jetbrains.plugins.groovy.extensions.GroovyUnresolvedHighlightFilter; import org.jetbrains.plugins.groovy.findUsages.MissingMethodAndPropertyUtil; -import org.jetbrains.plugins.groovy.lang.GrCreateClassKind; import org.jetbrains.plugins.groovy.lang.groovydoc.psi.api.GroovyDocPsiElement; import org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes; import org.jetbrains.plugins.groovy.lang.psi.GrReferenceElement; import org.jetbrains.plugins.groovy.lang.psi.GroovyFileBase; -import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElement; import org.jetbrains.plugins.groovy.lang.psi.api.EmptyGroovyResolveResult; import org.jetbrains.plugins.groovy.lang.psi.api.GroovyResolveResult; -import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.modifiers.annotation.GrAnnotation; import org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks.GrClosableBlock; import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.*; -import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrExtendsClause; -import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrImplementsClause; -import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrInterfaceDefinition; import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.members.GrMethod; import org.jetbrains.plugins.groovy.lang.psi.api.toplevel.imports.GrImportStatement; import org.jetbrains.plugins.groovy.lang.psi.api.toplevel.packaging.GrPackageDefinition; @@ -68,6 +61,8 @@ import java.util.List; import java.util.Map; import static com.intellij.psi.util.PsiUtil.isInnerClass; +import static org.jetbrains.plugins.groovy.codeInspection.untypedUnresolvedAccess.ReferenceFixesKt.generateAddImportActions; +import static org.jetbrains.plugins.groovy.codeInspection.untypedUnresolvedAccess.ReferenceFixesKt.generateCreateClassActions; import static org.jetbrains.plugins.groovy.lang.psi.impl.PsiImplUtil.hasArguments; import static org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil.hasEnclosingInstanceInScope; @@ -453,81 +448,17 @@ public class GrUnresolvedAccessChecker { } private static void registerAddImportFixes(GrReferenceElement refElement, @Nullable HighlightInfo info, final HighlightDisplayKey key) { - final String referenceName = refElement.getReferenceName(); - if (StringUtil.isEmpty(referenceName)) return; - if (!(refElement instanceof GrCodeReferenceElement) && Character.isLowerCase(referenceName.charAt(0))) return; - if (refElement.getQualifier() != null) return; - - QuickFixAction.registerQuickFixAction(info, GroovyQuickFixFactory.getInstance().createGroovyAddImportAction(refElement), key); + for (IntentionAction action : generateAddImportActions(refElement)) { + QuickFixAction.registerQuickFixAction(info, action, key); + } } private static void registerCreateClassByTypeFix(@NotNull GrReferenceElement refElement, @Nullable HighlightInfo info, - final HighlightDisplayKey key) { - GrPackageDefinition packageDefinition = PsiTreeUtil.getParentOfType(refElement, GrPackageDefinition.class); - if (packageDefinition != null) return; - - PsiElement parent = refElement.getParent(); - if (parent instanceof GrNewExpression && - refElement.getManager().areElementsEquivalent(((GrNewExpression)parent).getReferenceElement(), refElement)) { - QuickFixAction.registerQuickFixAction(info, GroovyQuickFixFactory.getInstance().createClassFromNewAction((GrNewExpression)parent), key); + HighlightDisplayKey key) { + for (IntentionAction fix : generateCreateClassActions(refElement)) { + QuickFixAction.registerQuickFixAction(info, fix, key); } - else if (canBeClassOrPackage(refElement)) { - if (shouldBeInterface(refElement)) { - QuickFixAction.registerQuickFixAction(info, GroovyQuickFixFactory.getInstance().createClassFixAction(refElement, GrCreateClassKind.INTERFACE), key); - QuickFixAction.registerQuickFixAction(info, GroovyQuickFixFactory.getInstance().createClassFixAction(refElement, GrCreateClassKind.TRAIT), key); - } - else if (shouldBeClass(refElement)) { - QuickFixAction.registerQuickFixAction(info, GroovyQuickFixFactory.getInstance().createClassFixAction(refElement, GrCreateClassKind.CLASS), key); - QuickFixAction.registerQuickFixAction(info, GroovyQuickFixFactory.getInstance().createClassFixAction(refElement, GrCreateClassKind.ENUM), key); - } - else if (shouldBeAnnotation(refElement)) { - QuickFixAction.registerQuickFixAction(info, GroovyQuickFixFactory.getInstance().createClassFixAction(refElement, GrCreateClassKind.ANNOTATION), key); - } - else { - QuickFixAction.registerQuickFixAction(info, GroovyQuickFixFactory.getInstance().createClassFixAction(refElement, GrCreateClassKind.CLASS), key); - QuickFixAction.registerQuickFixAction(info, GroovyQuickFixFactory.getInstance().createClassFixAction(refElement, GrCreateClassKind.INTERFACE), key); - - if (!refElement.isQualified() || resolvesToGroovy(refElement.getQualifier())) { - QuickFixAction.registerQuickFixAction(info, GroovyQuickFixFactory.getInstance().createClassFixAction(refElement, GrCreateClassKind.TRAIT), key); - } - - QuickFixAction.registerQuickFixAction(info, GroovyQuickFixFactory.getInstance().createClassFixAction(refElement, GrCreateClassKind.ENUM), key); - QuickFixAction.registerQuickFixAction(info, GroovyQuickFixFactory.getInstance().createClassFixAction(refElement, GrCreateClassKind.ANNOTATION), key); - } - } - } - - private static boolean resolvesToGroovy(PsiElement qualifier) { - if (qualifier instanceof GrReferenceElement) { - return ((GrReferenceElement)qualifier).resolve() instanceof GroovyPsiElement; - } - if (qualifier instanceof GrExpression) { - PsiType type = ((GrExpression)qualifier).getType(); - if (type instanceof PsiClassType) { - PsiClass resolved = ((PsiClassType)type).resolve(); - return resolved instanceof GroovyPsiElement; - } - } - return false; - } - - private static boolean canBeClassOrPackage(@NotNull GrReferenceElement refElement) { - return !(refElement instanceof GrReferenceExpression) || ResolveUtil.canBeClassOrPackage((GrReferenceExpression)refElement); - } - - private static boolean shouldBeAnnotation(GrReferenceElement element) { - return element.getParent() instanceof GrAnnotation; - } - - private static boolean shouldBeInterface(GrReferenceElement myRefElement) { - PsiElement parent = myRefElement.getParent(); - return parent instanceof GrImplementsClause || parent instanceof GrExtendsClause && parent.getParent() instanceof GrInterfaceDefinition; - } - - private static boolean shouldBeClass(GrReferenceElement myRefElement) { - PsiElement parent = myRefElement.getParent(); - return parent instanceof GrExtendsClause && !(parent.getParent() instanceof GrInterfaceDefinition); } private static boolean shouldHighlightAsUnresolved(@NotNull GrReferenceExpression referenceExpression) { diff --git a/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/untypedUnresolvedAccess/referenceFixes.kt b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/untypedUnresolvedAccess/referenceFixes.kt new file mode 100644 index 000000000000..2c6cfb85e9b1 --- /dev/null +++ b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/untypedUnresolvedAccess/referenceFixes.kt @@ -0,0 +1,99 @@ +// Copyright 2000-2019 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 org.jetbrains.plugins.groovy.codeInspection.untypedUnresolvedAccess + +import com.intellij.codeInsight.intention.IntentionAction +import com.intellij.psi.PsiClassType +import com.intellij.psi.PsiElement +import com.intellij.psi.util.parentOfType +import org.jetbrains.plugins.groovy.codeInspection.GroovyQuickFixFactory +import org.jetbrains.plugins.groovy.lang.GrCreateClassKind +import org.jetbrains.plugins.groovy.lang.psi.GrReferenceElement +import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElement +import org.jetbrains.plugins.groovy.lang.psi.api.auxiliary.modifiers.annotation.GrAnnotation +import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrExpression +import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrNewExpression +import org.jetbrains.plugins.groovy.lang.psi.api.statements.expressions.GrReferenceExpression +import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrExtendsClause +import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrImplementsClause +import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrInterfaceDefinition +import org.jetbrains.plugins.groovy.lang.psi.api.toplevel.packaging.GrPackageDefinition +import org.jetbrains.plugins.groovy.lang.psi.api.types.GrCodeReferenceElement +import org.jetbrains.plugins.groovy.lang.resolve.ResolveUtil.canBeClassOrPackage + +fun generateCreateClassActions(ref: GrReferenceElement<*>): Collection { + if (ref.parentOfType() != null) { + return emptyList() + } + + val factory = GroovyQuickFixFactory.getInstance() + + val parent = ref.parent + if (parent is GrNewExpression && parent.referenceElement === ref) { + return listOf(factory.createClassFromNewAction(parent)) + } + + if (ref is GrReferenceExpression && !canBeClassOrPackage(ref)) { + return emptyList() + } + + return when { + classExpected(parent) -> listOf( + factory.createClassFixAction(ref, GrCreateClassKind.CLASS), + factory.createClassFixAction(ref, GrCreateClassKind.ENUM) + ) + interfaceExpected(parent) -> listOf( + factory.createClassFixAction(ref, GrCreateClassKind.INTERFACE), + factory.createClassFixAction(ref, GrCreateClassKind.TRAIT) + ) + annotationExpected(parent) -> listOf( + factory.createClassFixAction(ref, GrCreateClassKind.ANNOTATION) + ) + else -> { + val result = mutableListOf( + factory.createClassFixAction(ref, GrCreateClassKind.CLASS), + factory.createClassFixAction(ref, GrCreateClassKind.INTERFACE) + ) + if (!ref.isQualified || resolvesToGroovy(ref.qualifier)) { + result += factory.createClassFixAction(ref, GrCreateClassKind.TRAIT) + } + result += factory.createClassFixAction(ref, GrCreateClassKind.ENUM) + result += factory.createClassFixAction(ref, GrCreateClassKind.ANNOTATION) + result + } + } +} + +private fun classExpected(parent: PsiElement?): Boolean { + return parent is GrExtendsClause && parent.parent !is GrInterfaceDefinition +} + +private fun interfaceExpected(parent: PsiElement?): Boolean { + return parent is GrImplementsClause || parent is GrExtendsClause && parent.parent is GrInterfaceDefinition +} + +private fun annotationExpected(parent: PsiElement?): Boolean { + return parent is GrAnnotation +} + +private fun resolvesToGroovy(qualifier: PsiElement?): Boolean { + return when (qualifier) { + is GrReferenceElement<*> -> qualifier.resolve() is GroovyPsiElement + is GrExpression -> { + val type = qualifier.type as? PsiClassType + type?.resolve() is GroovyPsiElement + } + else -> false + } +} + +fun generateAddImportActions(ref: GrReferenceElement<*>): Collection { + return generateAddImportAction(ref)?.let(::listOf) ?: emptyList() +} + +private fun generateAddImportAction(ref: GrReferenceElement<*>): IntentionAction? { + if (ref.isQualified) return null + val referenceName = ref.referenceName ?: return null + if (referenceName.isEmpty()) return null + if (ref !is GrCodeReferenceElement && Character.isLowerCase(referenceName[0])) return null + return GroovyQuickFixFactory.getInstance().createGroovyAddImportAction(ref) +} diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/intentions/closure/ImplicitClosureCallPredicate.kt b/plugins/groovy/src/org/jetbrains/plugins/groovy/intentions/closure/ImplicitClosureCallPredicate.kt index 5ded77c7f0cd..2f56ffc328ad 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/intentions/closure/ImplicitClosureCallPredicate.kt +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/intentions/closure/ImplicitClosureCallPredicate.kt @@ -19,8 +19,12 @@ internal object ImplicitClosureCallPredicate : PsiElementPredicate { return false } val result = element.advancedResolve() - return result.isInvokedOnProperty && element.invokedExpression.type.isClosureType() - || result.element.isClosureCallMethod() + if (element.implicitCallReference == null) { + return result.isInvokedOnProperty && element.invokedExpression.type.isClosureType() + } + else { + return result.element.isClosureCallMethod() + } } private fun PsiType?.isClosureType(): Boolean { diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/MakeClosureCallExplicitIntentionTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/MakeClosureCallExplicitIntentionTest.groovy index 69f236ac9ac4..9c8eeb72a4f3 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/MakeClosureCallExplicitIntentionTest.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/MakeClosureCallExplicitIntentionTest.groovy @@ -54,4 +54,9 @@ class MakeClosureCallExplicitIntentionTest extends GroovyLatestTest implements B void 'closure method'() { doTest 'Closure foo() {}; foo()', null } + + @Test + void 'closure method call'() { + doTest 'Closure foo() {}; foo().call()', null + } } diff --git a/resources/src/idea/JavaActions.xml b/resources/src/idea/JavaActions.xml index 5d8d86e8f1ea..7ee48ad07732 100644 --- a/resources/src/idea/JavaActions.xml +++ b/resources/src/idea/JavaActions.xml @@ -192,9 +192,6 @@ - - - diff --git a/xml/xml-psi-impl/src/com/intellij/lexer/BaseHtmlLexer.java b/xml/xml-psi-impl/src/com/intellij/lexer/BaseHtmlLexer.java index 7a4cf65814ae..e62f52a3197c 100644 --- a/xml/xml-psi-impl/src/com/intellij/lexer/BaseHtmlLexer.java +++ b/xml/xml-psi-impl/src/com/intellij/lexer/BaseHtmlLexer.java @@ -28,13 +28,13 @@ import java.util.Locale; */ public abstract class BaseHtmlLexer extends DelegateLexer { protected static final int BASE_STATE_MASK = 0x3F; - private static final int SEEN_STYLE = 0x40; - private static final int SEEN_TAG = 0x80; - private static final int SEEN_SCRIPT = 0x100; - private static final int SEEN_ATTRIBUTE = 0x200; - private static final int SEEN_CONTENT_TYPE = 0x400; - private static final int SEEN_STYLESHEET_TYPE = 0x800; - protected static final int BASE_STATE_SHIFT = 11; + private static final int SEEN_TAG = 0x40; + private static final int SEEN_ATTRIBUTE = 0x80; + private static final int SEEN_CONTENT_TYPE = 0x100; + private static final int SEEN_STYLESHEET_TYPE = 0x200; + private static final int SEEN_STYLE_SCRIPT_MASK = 0xE00; + private static final int SEEN_STYLE_SCRIPT_SHIFT = 10; + protected static final int BASE_STATE_SHIFT = 12; @Nullable protected static final Language ourDefaultLanguage = Language.findLanguageByID("JavaScript"); @Nullable @@ -45,6 +45,10 @@ public abstract class BaseHtmlLexer extends DelegateLexer { protected boolean seenStyle; protected boolean seenScript; + private static final char SCRIPT = 1; + private static final char STYLE = 2; + private final int[] scriptStyleStack = new int[] {0, 0}; + @Nullable protected String scriptType = null; @Nullable @@ -119,8 +123,7 @@ public abstract class BaseHtmlLexer extends DelegateLexer { return; } - seenStyle = style; - seenScript = script; + pushScriptStyle(script, style); if (!isHtmlTagState(state)) { seenAttribute=true; @@ -133,8 +136,7 @@ public abstract class BaseHtmlLexer extends DelegateLexer { @Override public void handleElement(Lexer lexer) { if (seenAttribute) { - seenStyle = false; - seenScript = false; + popScriptStyle(); seenAttribute = false; } seenContentType = false; @@ -142,6 +144,22 @@ public abstract class BaseHtmlLexer extends DelegateLexer { } } + private void pushScriptStyle(boolean script, boolean style) { + int position = scriptStyleStack[0] == 0 ? 0 : 1; + scriptStyleStack[position] = script ? SCRIPT : + style ? STYLE : + 0; + seenStyle = style; + seenScript = script; + } + + protected void popScriptStyle() { + int position = scriptStyleStack[1] == 0 ? 0 : 1; + scriptStyleStack[position] = 0; + seenStyle = scriptStyleStack[0] == STYLE; + seenScript = scriptStyleStack[0] == SCRIPT; + } + class XmlAttributeValueHandler implements TokenHandler { @Override public void handleElement(Lexer lexer) { @@ -218,9 +236,7 @@ public abstract class BaseHtmlLexer extends DelegateLexer { @Override public void handleElement(Lexer lexer) { if (seenAttribute) { - seenScript=false; - seenStyle=false; - + popScriptStyle(); seenAttribute=false; } else { if (seenStyle || seenScript) { @@ -233,8 +249,7 @@ public abstract class BaseHtmlLexer extends DelegateLexer { class XmlTagEndHandler implements TokenHandler { @Override public void handleElement(Lexer lexer) { - seenStyle=false; - seenScript=false; + popScriptStyle(); seenAttribute=false; seenContentType=false; seenStylesheetType=false; @@ -283,12 +298,18 @@ public abstract class BaseHtmlLexer extends DelegateLexer { } private void initState(final int initialState) { - seenScript = (initialState & SEEN_SCRIPT)!=0; - seenStyle = (initialState & SEEN_STYLE)!=0; seenTag = (initialState & SEEN_TAG)!=0; seenAttribute = (initialState & SEEN_ATTRIBUTE)!=0; seenContentType = (initialState & SEEN_CONTENT_TYPE) != 0; seenStylesheetType = (initialState & SEEN_STYLESHEET_TYPE) != 0; + if (seenTag || seenAttribute) { + int stack = (initialState & SEEN_STYLE_SCRIPT_MASK) >> SEEN_STYLE_SCRIPT_SHIFT + 1; + scriptStyleStack[0] = stack / 3; + scriptStyleStack[1] = stack % 3; + } + int position = scriptStyleStack[1] == 0 ? 0 : 1; + seenStyle = scriptStyleStack[position] == STYLE; + seenScript = scriptStyleStack[position] == SCRIPT; lexerOfCacheBufferSequence = null; cachedBufferSequence = null; } @@ -335,10 +356,8 @@ public abstract class BaseHtmlLexer extends DelegateLexer { if (base.getTokenType() != XmlTokenType.XML_END_TAG_START) { // we are inside comment base.start(buf,lastStart+1,getBufferEnd(),lastState); base.getTokenType(); - base.advance(); - } else { - base.advance(); } + base.advance(); while(XmlTokenType.WHITESPACES.contains(base.getTokenType())) { base.advance(); @@ -399,13 +418,15 @@ public abstract class BaseHtmlLexer extends DelegateLexer { public int getState() { int state = super.getState(); - state |= ((seenScript)?SEEN_SCRIPT:0); state |= ((seenTag)?SEEN_TAG:0); - state |= ((seenStyle)?SEEN_STYLE:0); state |= ((seenAttribute)?SEEN_ATTRIBUTE:0); state |= ((seenContentType)?SEEN_CONTENT_TYPE:0); state |= ((seenStylesheetType)?SEEN_STYLESHEET_TYPE:0); + if (seenTag || seenAttribute) { + state |= (scriptStyleStack[0] * 3 + scriptStyleStack[1] - 1) << 9; + } + return state; }