From 87e4f2a8a56727eabb98fc16a891bc61cb0a3128 Mon Sep 17 00:00:00 2001 From: Alexandr Evstigneev Date: Wed, 20 Feb 2019 12:18:20 +0300 Subject: [PATCH 01/20] Introduced EDT/ReadAction control on process execution awaiting External process execution may work unpredictably long and if we are waiting for it to finish directly or indirectly on EDT or holding the ReadAction, this may cause a UI lagging and/or IDE freezing. IDEA-CR-43729 --- .../intellij/execution/ExecutionHelper.java | 15 ++----- .../execution/process/OSProcessHandler.java | 41 +++++++++++++++++++ 2 files changed, 44 insertions(+), 12 deletions(-) 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/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 { From a0c101cd996f59b93d14f095b66b133892783cb9 Mon Sep 17 00:00:00 2001 From: Dmitry Batrak Date: Thu, 21 Feb 2019 11:19:17 +0300 Subject: [PATCH 02/20] reenable assertion in JBTabsImpl, add more assertions in FileEditorManagerImpl --- .../src/com/intellij/ui/tabs/impl/JBTabsImpl.java | 2 +- .../openapi/fileEditor/impl/FileEditorManagerImpl.java | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) 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-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()); From 7a1428329538a4eed2c39a8b3ab023579b064841 Mon Sep 17 00:00:00 2001 From: "Maxim.Mossienko" Date: Wed, 20 Feb 2019 18:12:39 +0100 Subject: [PATCH 03/20] fixed issue with refresh cancellation to ignore keeping files dirty --- .../LocalFileSystemRefreshWorker.java | 46 ++++++------------- 1 file changed, 15 insertions(+), 31 deletions(-) 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..84c3254ee2a5 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 @@ -31,9 +31,6 @@ 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.TimeUnit; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicLong; import static com.intellij.openapi.vfs.newvfs.persistent.VfsEventGenerationHelper.LOG; @@ -76,22 +73,17 @@ class LocalFileSystemRefreshWorker { myRefreshQueue.addLast(root); - try { - processQueue(fs, PersistentFS.getInstance()); - } - catch (RefreshCancelledException e) { - LOG.debug("refresh cancelled"); - } + processQueue(fs, PersistentFS.getInstance()); } - private void processQueue(@NotNull NewVirtualFileSystem fs, @NotNull PersistentFS persistence) throws RefreshCancelledException { + private void processQueue(@NotNull NewVirtualFileSystem fs, @NotNull PersistentFS persistence) { TObjectHashingStrategy strategy = FilePathHashingStrategy.create(fs.isCaseSensitive()); while (!myRefreshQueue.isEmpty()) { NewVirtualFile file = myRefreshQueue.pullFirst(); if (!myHelper.checkDirty(file)) continue; - checkCancelled(file); + if(checkCancelled(file)) break; if (file.isDirectory()) { boolean fullSync = ((VirtualDirectoryImpl)file).allChildrenLoaded(); @@ -106,6 +98,8 @@ class LocalFileSystemRefreshWorker { refreshFile(fs, persistence, strategy, file); } + if(checkCancelled(file)) break; + if (myIsRecursive || !file.isDirectory()) { file.markClean(); } @@ -124,9 +118,6 @@ class LocalFileSystemRefreshWorker { 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, @@ -204,18 +195,18 @@ class LocalFileSystemRefreshWorker { } } - private static class RefreshCancelledException extends RuntimeException { - } - - private void checkCancelled(@NotNull NewVirtualFile stopAt) { - if (myCancelled || ourCancellingCondition != null && ourCancellingCondition.fun(stopAt)) { + private boolean checkCancelled(@NotNull NewVirtualFile stopAt) { + boolean myRequestedCancel = false; + if (myCancelled || (myRequestedCancel = ourCancellingCondition != null && ourCancellingCondition.fun(stopAt))) { + if (myRequestedCancel) myCancelled = true; forceMarkDirty(stopAt); while (!myRefreshQueue.isEmpty()) { NewVirtualFile next = myRefreshQueue.pullFirst(); forceMarkDirty(next); } - throw new RefreshCancelledException(); + return true; } + return false; } private static void forceMarkDirty(@NotNull NewVirtualFile file) { @@ -275,7 +266,9 @@ class LocalFileSystemRefreshWorker { return FileVisitResult.CONTINUE; } - checkCancelled(child); + if(checkCancelled(child)) { + return FileVisitResult.CONTINUE; + } if (!child.isDirty()) { return FileVisitResult.CONTINUE; @@ -353,9 +346,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 +375,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 From f6747e6fe3af41cbd0ae27b47e4e0468520b8557 Mon Sep 17 00:00:00 2001 From: "Maxim.Mossienko" Date: Thu, 21 Feb 2019 10:10:05 +0100 Subject: [PATCH 04/20] cleanup: changed expected and actual values in assertEquals --- .../com/intellij/openapi/vfs/local/LocalFileSystemTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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(); From f0590f29692af7dc945180f66452787b804c2f08 Mon Sep 17 00:00:00 2001 From: "Maxim.Mossienko" Date: Thu, 21 Feb 2019 10:13:22 +0100 Subject: [PATCH 05/20] (performance) ability to do disk access heavy refresh in several threads --- .../LocalFileSystemRefreshWorker.java | 175 +++++++++++------- .../util/resources/misc/registry.properties | 2 + 2 files changed, 112 insertions(+), 65 deletions(-) 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 84c3254ee2a5..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,18 +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 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 @@ -55,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; @@ -71,66 +75,118 @@ class LocalFileSystemRefreshWorker { fs = PersistentFS.replaceWithNativeFS(fs); } - myRefreshQueue.addLast(root); - - processQueue(fs, PersistentFS.getInstance()); + 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) { - 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; - - if(checkCancelled(file)) break; - - 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(checkCancelled(file)) break; + 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<>(); - if (myIsRecursive || !file.isDirectory()) { - file.markClean(); + 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 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); @@ -139,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; } @@ -162,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 = @@ -175,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 @@ -195,15 +248,11 @@ class LocalFileSystemRefreshWorker { } } - private boolean checkCancelled(@NotNull NewVirtualFile stopAt) { + private boolean checkCancelled(@NotNull NewVirtualFile stopAt, RefreshContext refreshContext) { boolean myRequestedCancel = false; if (myCancelled || (myRequestedCancel = ourCancellingCondition != null && ourCancellingCondition.fun(stopAt))) { if (myRequestedCancel) myCancelled = true; - forceMarkDirty(stopAt); - while (!myRefreshQueue.isEmpty()) { - NewVirtualFile next = myRefreshQueue.pullFirst(); - forceMarkDirty(next); - } + refreshContext.filesToBecomeDirty.offer(stopAt); return true; } return false; @@ -228,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(); @@ -266,7 +311,7 @@ class LocalFileSystemRefreshWorker { return FileVisitResult.CONTINUE; } - if(checkCancelled(child)) { + if(checkCancelled(child, myRefreshContext)) { return FileVisitResult.CONTINUE; } @@ -305,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) { @@ -335,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(); } 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 From dd03ec2dbd2cdc8f39ffcb7193fa7fa53343a405 Mon Sep 17 00:00:00 2001 From: Daniil Ovchinnikov Date: Wed, 20 Feb 2019 15:06:51 +0300 Subject: [PATCH 06/20] [groovy] extract class fixes generation into separate file, make it independent from HighlightInfo/HighlightDisplayKey --- .../GrUnresolvedAccessChecker.java | 79 ++--------------- .../untypedUnresolvedAccess/referenceFixes.kt | 86 +++++++++++++++++++ 2 files changed, 92 insertions(+), 73 deletions(-) create mode 100644 plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/untypedUnresolvedAccess/referenceFixes.kt 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..c022eb46fbaa 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,7 @@ 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.generateCreateClassActions; import static org.jetbrains.plugins.groovy.lang.psi.impl.PsiImplUtil.hasArguments; import static org.jetbrains.plugins.groovy.lang.psi.util.PsiUtil.hasEnclosingInstanceInScope; @@ -463,71 +457,10 @@ public class GrUnresolvedAccessChecker { 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..39c4b694ef17 --- /dev/null +++ b/plugins/groovy/groovy-psi/src/org/jetbrains/plugins/groovy/codeInspection/untypedUnresolvedAccess/referenceFixes.kt @@ -0,0 +1,86 @@ +// 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.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 + } +} From 444ff65011020dcfd81af6d05c3b18c95b5791c9 Mon Sep 17 00:00:00 2001 From: Daniil Ovchinnikov Date: Wed, 20 Feb 2019 15:17:20 +0300 Subject: [PATCH 07/20] [groovy] extract import fixes generation into separate file, make it independent from HighlightInfo/HighlightDisplayKey --- .../GrUnresolvedAccessChecker.java | 10 ++++------ .../untypedUnresolvedAccess/referenceFixes.kt | 13 +++++++++++++ 2 files changed, 17 insertions(+), 6 deletions(-) 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 c022eb46fbaa..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 @@ -61,6 +61,7 @@ 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; @@ -447,12 +448,9 @@ 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, 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 index 39c4b694ef17..2c6cfb85e9b1 100644 --- 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 @@ -17,6 +17,7 @@ import org.jetbrains.plugins.groovy.lang.psi.api.statements.typedef.GrExtendsCla 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 { @@ -84,3 +85,15 @@ private fun resolvesToGroovy(qualifier: PsiElement?): Boolean { 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) +} From a581530599f66e077487f7ffc6e105ac918f1c3f Mon Sep 17 00:00:00 2001 From: Alexandr Evstigneev Date: Thu, 21 Feb 2019 12:02:09 +0300 Subject: [PATCH 08/20] Moved hierarchy and optimize actions to the common menu as well Keeping analysis group, because someone may use it outside of JetBrains IDEA-CR-43523 --- platform/platform-resources/src/idea/LangActions.xml | 2 ++ resources/src/idea/JavaActions.xml | 3 --- 2 files changed, 2 insertions(+), 3 deletions(-) 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/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 @@ - - - From ad12b3c82a79a1ec35e995f5e721181790f041aa Mon Sep 17 00:00:00 2001 From: Dennis Ushakov Date: Thu, 21 Feb 2019 12:28:18 +0300 Subject: [PATCH 09/20] style script state stack (WEB-25857, WEB-26169, WEB-26164) --- .../src/com/intellij/lexer/BaseHtmlLexer.java | 67 ++++++++++++------- 1 file changed, 44 insertions(+), 23 deletions(-) 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; } From 4d1ac741d35d091e1003a6f5bc7a03bb40190440 Mon Sep 17 00:00:00 2001 From: Daniil Ovchinnikov Date: Thu, 21 Feb 2019 12:42:48 +0300 Subject: [PATCH 10/20] deprecate QuickFixActionRegistrar#unregister --- .../daemon/QuickFixActionRegistrar.java | 30 +++++++------------ 1 file changed, 11 insertions(+), 19 deletions(-) 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) {} } From f2f078d2a3f49634c5f5db5dd9b7a95cfc0b7829 Mon Sep 17 00:00:00 2001 From: Daniil Ovchinnikov Date: Thu, 21 Feb 2019 12:57:52 +0300 Subject: [PATCH 11/20] [groovy] fix ImplicitClosureCallPredicate to ignore explicit Closure#call (IDEA-207549) --- .../intentions/closure/ImplicitClosureCallPredicate.kt | 8 ++++++-- .../MakeClosureCallExplicitIntentionTest.groovy | 5 +++++ 2 files changed, 11 insertions(+), 2 deletions(-) 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 + } } From e189f7b1dc9d261112434dfd98805c2d15f22a81 Mon Sep 17 00:00:00 2001 From: nik Date: Thu, 21 Feb 2019 12:59:57 +0300 Subject: [PATCH 12/20] inspection: fix bugs in "Suspicious package-private access" inspection (IDEA-200047) Properly report references to protected constructors accessed not via subclass, and fix duplicating warning in Kotlin files. Also add tests for Kotlin. --- .../dep/xxx/InnerClasses.java | 16 +++ .../dep/xxx/PackagePrivateClass.java | 4 + .../dep/xxx/ProtectedConstructors.java | 9 ++ .../dep/xxx/ProtectedMembers.java | 12 ++ .../dep/xxx/PublicClass.java | 16 +++ .../PublicClassWithDefaultConstructor.java | 5 + .../dep/xxx/StaticMembers.java | 7 ++ .../src/AccessingPackagePrivateMembers.kt | 41 +++++++ .../src/AccessingProtectedMembers.kt | 36 ++++++ ...ciousPackagePrivateAccessInspectionTest.kt | 18 +++ ...piciousPackagePrivateAccessInspection.java | 100 ++++++++++++++--- .../dep/xxx/InnerClasses.java | 16 +++ .../dep/xxx/ProtectedConstructors.java | 9 ++ .../dep/xxx/PublicClass.java | 4 + .../PublicClassWithDefaultConstructor.java | 5 + .../src/AccessingPackagePrivateMembers.java | 18 ++- .../src/AccessingProtectedMembers.java | 33 ++++++ ...ousPackagePrivateAccessInspectionTest.java | 96 +--------------- ...ackagePrivateAccessInspectionTestCase.java | 103 ++++++++++++++++++ 19 files changed, 432 insertions(+), 116 deletions(-) create mode 100644 jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/InnerClasses.java create mode 100644 jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/PackagePrivateClass.java create mode 100644 jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/ProtectedConstructors.java create mode 100644 jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/ProtectedMembers.java create mode 100644 jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/PublicClass.java create mode 100644 jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/PublicClassWithDefaultConstructor.java create mode 100644 jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/dep/xxx/StaticMembers.java create mode 100644 jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/src/AccessingPackagePrivateMembers.kt create mode 100644 jvm/jvm-analysis-kotlin-tests/testData/codeInspection/suspiciousPackagePrivateAccess/src/AccessingProtectedMembers.kt create mode 100644 jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/KtSuspiciousPackagePrivateAccessInspectionTest.kt create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/dep/xxx/InnerClasses.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/dep/xxx/ProtectedConstructors.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/dependency/suspicious_package_private_access/dep/xxx/PublicClassWithDefaultConstructor.java create mode 100644 plugins/InspectionGadgets/testsrc/com/siyeh/ig/dependency/SuspiciousPackagePrivateAccessInspectionTestCase.java 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/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; + } + } +} From 5f5677e8d913030e249aaa2a45ae3e07ee35f5a7 Mon Sep 17 00:00:00 2001 From: Mikhail Sokolov Date: Wed, 20 Feb 2019 16:24:41 +0300 Subject: [PATCH 13/20] IDEA-207411 Search Everywhere: Add sorting by element priority --- .../searcheverywhere/SearchEverywhereUI.java | 12 +++++++ .../searcheverywhere/SearchModelTest.java | 33 ++++++++++++++++++- 2 files changed, 44 insertions(+), 1 deletion(-) 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..96f9ae0fec66 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)); @@ -1115,6 +1116,17 @@ public class SearchEverywhereUI extends BigPopupUI implements DataProvider, Quic } } + 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); + } + } + } + private void addElementsWithPriority(SearchEverywhereContributor contributor, int index, List newElements) { for (SESearcher.ElementInfo newElementInfo : newElements) { if (index < listElements.size()) { 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..bddfc6019113 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 @@ -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 From f5377f05b5479bc3a3c2fbbd57304edc94ce926a Mon Sep 17 00:00:00 2001 From: Mikhail Sokolov Date: Wed, 20 Feb 2019 20:15:29 +0300 Subject: [PATCH 14/20] IDEA-207411 Search Everywhere: Add sorting by element priority --- .../searcheverywhere/SearchEverywhereUI.java | 37 +++++-------------- 1 file changed, 10 insertions(+), 27 deletions(-) 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 96f9ae0fec66..5ec7c857db0d 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 @@ -1077,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); } }); } @@ -1127,26 +1129,6 @@ 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 applyChange(Diff.Change change, SearchEverywhereContributor contributor, List newItems) { @@ -1278,6 +1260,7 @@ public class SearchEverywhereUI extends BigPopupUI implements DataProvider, Quic return isMoreElement(index) ? index : index + 1; } + //todo binary search for (int i = 0; i < list.size(); i++) { if (list.get(i).getSortWeight() > contributor.getSortWeight()) { return i; From 95a5d99679eded100904dcb402456c852e1ee4a5 Mon Sep 17 00:00:00 2001 From: Mikhail Sokolov Date: Wed, 20 Feb 2019 20:46:03 +0300 Subject: [PATCH 15/20] IDEA-207411 Search Everywhere: Add sorting by element priority --- .../actions/searcheverywhere/SearchEverywhereUI.java | 10 ++-------- .../ide/actions/searcheverywhere/SearchModelTest.java | 2 +- 2 files changed, 3 insertions(+), 9 deletions(-) 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 5ec7c857db0d..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 @@ -1260,14 +1260,8 @@ public class SearchEverywhereUI extends BigPopupUI implements DataProvider, Quic return isMoreElement(index) ? index : index + 1; } - //todo binary search - 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 bddfc6019113..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), From cde2152359d48b2ba70a827c861b71cadf640d0b Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Thu, 21 Feb 2019 17:07:38 +0700 Subject: [PATCH 16/20] DataFlowInspection: ignore non-physical constructors created by Lombok; Also report non-physical pushes as exception instead of LOG.error (it will be intercepted in DataFlowRunner and useful attachments will be added) Fixes (at least partially) EA-137431 - assert: DataFlowInstructionVisitor.beforeExpressionPush --- .../codeInspection/dataFlow/DataFlowInspectionBase.java | 5 +++++ .../dataFlow/DataFlowInstructionVisitor.java | 7 ++++++- 2 files changed, 11 insertions(+), 1 deletion(-) 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) { From 92efa1bb5802cb970252a6793903985474da1a6f Mon Sep 17 00:00:00 2001 From: Sergey Malenkov Date: Thu, 21 Feb 2019 13:35:02 +0300 Subject: [PATCH 17/20] fix tree control alignment in the settings tree --- .../intellij/ui/treeStructure/SimpleTree.java | 11 ++-- .../options/newEditor/SettingsTreeView.java | 58 +++++++++++++++---- .../intellij/ui/tree/ui/DefaultControl.java | 2 +- 3 files changed, 54 insertions(+), 17 deletions(-) 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/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/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"); From 555f077b6d4c2c294087ce5f5c28777b3018baa7 Mon Sep 17 00:00:00 2001 From: Konstantin Ulitin Date: Thu, 21 Feb 2019 13:50:44 +0300 Subject: [PATCH 18/20] js debugger: fix updating cached file-url mappings (WEB-34557) --- .../src/debugger/sourcemap/NestedSourceMap.kt | 4 ++-- .../src/debugger/sourcemap/SourceMap.kt | 20 ++++++++++++------- .../src/debugger/sourcemap/SourceResolver.kt | 10 ++++------ 3 files changed, 19 insertions(+), 15 deletions(-) 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) { From 291ac583c2e72c73c2d08151c5aa47a6cd3293a5 Mon Sep 17 00:00:00 2001 From: Vladimir Krivosheev Date: Thu, 21 Feb 2019 11:52:50 +0100 Subject: [PATCH 19/20] IDEA-CR-43780 ignore new config dir --- .../testSrc/ConfigImportHelperTest.kt | 7 ++++++- .../intellij/openapi/application/ConfigImportHelper.java | 5 ++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/platform/configuration-store-impl/testSrc/ConfigImportHelperTest.kt b/platform/configuration-store-impl/testSrc/ConfigImportHelperTest.kt index 148e55b633bb..4eba943ffd2a 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)} 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..a69d11b6e19d 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) { From bc94afe67ff88f6e80c3e69540eb08e0f91c19a7 Mon Sep 17 00:00:00 2001 From: Vladimir Krivosheev Date: Thu, 21 Feb 2019 12:10:50 +0100 Subject: [PATCH 20/20] IDEA-CR-43780 sort by name if no anchor files --- .../testSrc/ConfigImportHelperTest.kt | 31 ++++++++++++++++--- .../application/ConfigImportHelper.java | 8 ++++- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/platform/configuration-store-impl/testSrc/ConfigImportHelperTest.kt b/platform/configuration-store-impl/testSrc/ConfigImportHelperTest.kt index 4eba943ffd2a..669c45391561 100644 --- a/platform/configuration-store-impl/testSrc/ConfigImportHelperTest.kt +++ b/platform/configuration-store-impl/testSrc/ConfigImportHelperTest.kt @@ -88,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/platform-impl/src/com/intellij/openapi/application/ConfigImportHelper.java b/platform/platform-impl/src/com/intellij/openapi/application/ConfigImportHelper.java index a69d11b6e19d..7cf4b83841c9 100644 --- a/platform/platform-impl/src/com/intellij/openapi/application/ConfigImportHelper.java +++ b/platform/platform-impl/src/com/intellij/openapi/application/ConfigImportHelper.java @@ -197,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; }