Replacing consulo.ide.impl.idea.openapi.module.ModuleUtil with consulo.language.util.ModuleUtilCore#295
Conversation
…o.language.util.ModuleUtilCore.
There was a problem hiding this comment.
Pull request overview
This PR removes remaining usages of the legacy consulo.ide.impl.idea.openapi.module.ModuleUtil and migrates callers to consulo.language.util.ModuleUtilCore or PsiElement#getModule(), aligning module-resolution logic with the newer Consulo APIs.
Changes:
- Replace
ModuleUtil.findModuleForFile(...)withModuleUtilCore.findModuleForFile(...)in UI + tests. - Replace
ModuleUtil.findModuleForPsiElement(...)withPsiElement#getModule()in inspections/refactoring/reference providers. - Add
@RequiredReadActionannotations in a few PSI-touching code paths.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| plugin/src/test/java_/com/intellij/psi/impl/cache/impl/SCR20733Test.java | Update module lookup assertions to use ModuleUtilCore.findModuleForFile. |
| plugin/src/main/java/com/intellij/java/impl/util/xml/ui/PsiClassControl.java | Switch fragment module user-data key to ModuleUtilCore.KEY_MODULE. |
| plugin/src/main/java/com/intellij/java/impl/refactoring/move/moveClassesOrPackages/DestinationFolderComboBox.java | Use ModuleUtilCore.findModuleForFile when rendering module names for destination roots. |
| plugin/src/main/java/com/intellij/java/impl/refactoring/introduceparameterobject/IntroduceParameterObjectProcessor.java | Replace module lookup with PsiFile#getModule() in class-generation logic. |
| plugin/src/main/java/com/intellij/java/impl/psi/impl/source/resolve/reference/impl/providers/FilePathReferenceProvider.java | Use PsiElement#getModule() for default context root computation. |
| plugin/src/main/java/com/intellij/java/impl/psi/impl/file/PsiPackageImplementationHelperImpl.java | Use PsiFile#getModule() and add @RequiredReadAction to directory suggestion helper. |
| plugin/src/main/java/com/intellij/java/impl/ig/junit/UseOfObsoleteAssertInspection.java | Replace module lookup with expression.getModule() and simplify static-method check. |
| plugin/src/main/java/com/intellij/java/impl/codeInspection/unnecessaryModuleDependency/UnnecessaryModuleDependencyAnnotator.java | Replace module lookup with PsiElement#getModule() and add @RequiredReadAction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @RequiredReadAction | ||
| private static PsiDirectory[] suggestMostAppropriateDirectories(PsiJavaPackage psiPackage) { |
There was a problem hiding this comment.
suggestMostAppropriateDirectories() is now annotated @RequiredReadAction, but its immediate caller in this class (navigate()’s ToolWindow.activate callback) isn’t obviously under a read lock. To avoid read-access violations when touching PSI here, wrap the call in ReadAction.compute/run (or drop the annotation if it’s guaranteed to be called under a read action).
| else { | ||
| Module module = ModuleUtil.findModuleForPsiElement(containingFile); | ||
| Module module = containingFile.getModule(); | ||
| directory = PackageUtil.findOrCreateDirectoryForPackage(module, packageName, containingDirectory, true, true); |
There was a problem hiding this comment.
containingFile.getModule() can be null (other call sites in the refactoring codebase typically assert module != null before using it). Passing a null module into PackageUtil.findOrCreateDirectoryForPackage(...) will NPE (module.getProject() inside). Consider asserting non-null (if guaranteed in this refactoring) or handling the null case explicitly before calling PackageUtil.
| directory = PackageUtil.findOrCreateDirectoryForPackage(module, packageName, containingDirectory, true, true); | |
| directory = module != null ? PackageUtil.findOrCreateDirectoryForPackage(module, packageName, containingDirectory, true, true) : null; |
No description provided.