From 547ccf8e3e91e8b6cc1ec729ea3194c1be94a7b8 Mon Sep 17 00:00:00 2001 From: Aleksey Dobrynin Date: Thu, 6 Nov 2025 17:21:35 +0100 Subject: [PATCH] [java, quickfix, jigsaw] IDEA-380666 add circular dependency handling for Jigsaw module completion (cherry picked from commit 0beb868ac32c4cabae9392a9c34cb66086935a02) (cherry picked from commit 5a7924c57cb9afd068e60aed6eea4d3b151cf8de) IJ-MR-182986 GitOrigin-RevId: 201885016bf87fce8b6a98413c63d47de5023539 --- .../codeserver/core/JavaPsiModuleUtil.java | 20 +++++++++++-- .../impl/analysis/JavaModuleGraphUtil.java | 4 +++ .../before/.idea/misc.xml | 6 ++++ .../before/.idea/modules.xml | 10 +++++++ .../before/A/A.iml | 11 +++++++ .../before/A/src/module-info.java | 4 +++ .../A/src/org/jetbrains/a/MyAClass.java | 4 +++ .../before/B/B.iml | 11 +++++++ .../before/B/src/module-info.java | 3 ++ .../B/src/org/jetbrains/b/MyBClass.java | 4 +++ .../before/main.iml | 13 +++++++++ .../before/src/Main.java | 5 ++++ .../before/src/module-info.java | 3 ++ .../impl/quickfix/AddModuleDirectiveTest.kt | 8 ++++- .../completion/JigsawCodeCompletionTest.java | 29 +++++++++++++++++-- 15 files changed, 129 insertions(+), 6 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/.idea/misc.xml create mode 100644 java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/.idea/modules.xml create mode 100644 java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/A/A.iml create mode 100644 java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/A/src/module-info.java create mode 100644 java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/A/src/org/jetbrains/a/MyAClass.java create mode 100644 java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/B/B.iml create mode 100644 java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/B/src/module-info.java create mode 100644 java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/B/src/org/jetbrains/b/MyBClass.java create mode 100644 java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/main.iml create mode 100644 java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/src/Main.java create mode 100644 java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/src/module-info.java diff --git a/java/codeserver/core/src/com/intellij/java/codeserver/core/JavaPsiModuleUtil.java b/java/codeserver/core/src/com/intellij/java/codeserver/core/JavaPsiModuleUtil.java index f4ab33e6ca04..9ffe6bab2e3f 100644 --- a/java/codeserver/core/src/com/intellij/java/codeserver/core/JavaPsiModuleUtil.java +++ b/java/codeserver/core/src/com/intellij/java/codeserver/core/JavaPsiModuleUtil.java @@ -431,7 +431,7 @@ public final class JavaPsiModuleUtil { PsiJavaModuleReference ref = statement.getModuleReference(); if (ref != null) { if (JAVA_BASE.equals(ref.getCanonicalText())) explicitJavaBase = true; - for (ResolveResult result : ref.multiResolve(false)) { + for (ResolveResult result : ref.multiResolve(true)) { PsiJavaModule dependency = (PsiJavaModule)result.getElement(); assert dependency != null : result; relations.putValue(module, dependency); @@ -491,6 +491,8 @@ public final class JavaPsiModuleUtil { } public boolean reads(PsiJavaModule source, PsiJavaModule destination) { + source = getPhysicalModule(source); + destination = getPhysicalModule(destination); Collection nodes = myGraph.getNodes(); if (nodes.contains(destination) && nodes.contains(source)) { Iterator directReaders = myGraph.getOut(destination); @@ -505,6 +507,7 @@ public final class JavaPsiModuleUtil { } private @Nullable ModulePackageConflict findConflict(@NotNull PsiJavaModule source) { + source = getPhysicalModule(source); Map exports = new HashMap<>(); return processExports(source, (pkg, m) -> { PsiJavaModule found = exports.put(pkg, m); @@ -516,10 +519,11 @@ public final class JavaPsiModuleUtil { } private @Nullable PsiJavaModule findOrigin(@NotNull PsiJavaModule module, @NotNull String packageName) { - return processExports(module, (pkg, m) -> packageName.equals(pkg) ? m : null); + return processExports(getPhysicalModule(module), (pkg, m) -> packageName.equals(pkg) ? m : null); } private @Nullable T processExports(@NotNull PsiJavaModule start, @NotNull BiFunction processor) { + start = getPhysicalModule(start); return myGraph.getNodes().contains(start) ? processExports(start.getName(), start, true, new HashSet<>(), processor) : null; } @@ -528,6 +532,7 @@ public final class JavaPsiModuleUtil { boolean direct, @NotNull Set visited, @NotNull BiFunction processor) { + module = getPhysicalModule(module); if (visited.add(module)) { if (!direct) { for (PsiPackageAccessibilityStatement statement : module.getExports()) { @@ -556,11 +561,12 @@ public final class JavaPsiModuleUtil { public @NotNull Set getAllDependencies(@NotNull PsiJavaModule module, boolean transitive) { Set requires = new HashSet<>(); - collectDependencies(module, requires, transitive); + collectDependencies(getPhysicalModule(module), requires, transitive); return requires; } private void collectDependencies(@NotNull PsiJavaModule module, @NotNull Set dependencies, boolean transitive) { + module = getPhysicalModule(module); for (Iterator iterator = myGraph.getIn(module); iterator.hasNext();) { PsiJavaModule dependency = iterator.next(); if (!dependencies.contains(dependency) && (!transitive || myTransitiveEdges.contains(key(dependency, module)))) { @@ -569,6 +575,14 @@ public final class JavaPsiModuleUtil { } } } + + private static @NotNull PsiJavaModule getPhysicalModule(@NotNull PsiJavaModule from) { + if (from.isPhysical()) return from; + if (!(from.getContainingFile() instanceof PsiJavaFile file)) return from; + if (!(file.getOriginalFile() instanceof PsiJavaFile origin)) return from; + if (origin.getModuleDeclaration() instanceof PsiJavaModule result) return result; + return from; + } } /** diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaModuleGraphUtil.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaModuleGraphUtil.java index b18cc609ab47..d3d98d676e00 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaModuleGraphUtil.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaModuleGraphUtil.java @@ -104,6 +104,9 @@ public final class JavaModuleGraphUtil { if (to.equals(from.getName())) return false; if (!PsiNameHelper.isValidModuleName(to, from)) return false; if (alreadyContainsRequires(from, to)) return false; + + PsiJavaModule toModule = JavaPsiFacade.getInstance(from.getProject()).findModule(to, from.getResolveScope()); + if (toModule != null && JavaPsiModuleUtil.reads(toModule, from)) return false; // check for circular dependencies PsiUtil.addModuleStatement(from, JavaKeywords.REQUIRES + " " + (isStaticModule(to, scope) ? JavaKeywords.STATIC + " " : "") + (isExported ? JavaKeywords.TRANSITIVE + " " : "") + @@ -133,6 +136,7 @@ public final class JavaModuleGraphUtil { if (!PsiNameHelper.isValidModuleName(to.getName(), to)) return false; if (contains(from.getRequires(), to.getName())) return false; if (JavaPsiModuleUtil.reads(from, to)) return false; + if (JavaPsiModuleUtil.reads(to, from)) return false; // check for circular dependencies PsiUtil.addModuleStatement(from, JavaKeywords.REQUIRES + " " + (isStaticModule(to.getName(), scope) ? JavaKeywords.STATIC + " " : "") + (isExported(from, to) ? JavaKeywords.TRANSITIVE + " " : "") + diff --git a/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/.idea/misc.xml b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/.idea/misc.xml new file mode 100644 index 000000000000..e0844bc7be0a --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/.idea/misc.xml @@ -0,0 +1,6 @@ + + + + + + \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/.idea/modules.xml b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/.idea/modules.xml new file mode 100644 index 000000000000..c43cd5794550 --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/.idea/modules.xml @@ -0,0 +1,10 @@ + + + + + + + + + + \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/A/A.iml b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/A/A.iml new file mode 100644 index 000000000000..c90834f2d607 --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/A/A.iml @@ -0,0 +1,11 @@ + + + + + + + + + + + \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/A/src/module-info.java b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/A/src/module-info.java new file mode 100644 index 000000000000..e739e9dace07 --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/A/src/module-info.java @@ -0,0 +1,4 @@ +module module.a { + requires module.main; + exports org.jetbrains.a; +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/A/src/org/jetbrains/a/MyAClass.java b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/A/src/org/jetbrains/a/MyAClass.java new file mode 100644 index 000000000000..1168a5a5b511 --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/A/src/org/jetbrains/a/MyAClass.java @@ -0,0 +1,4 @@ +package org.jetbrains.a; + +public class MyAClass { +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/B/B.iml b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/B/B.iml new file mode 100644 index 000000000000..c90834f2d607 --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/B/B.iml @@ -0,0 +1,11 @@ + + + + + + + + + + + \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/B/src/module-info.java b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/B/src/module-info.java new file mode 100644 index 000000000000..9372a5b3f3c8 --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/B/src/module-info.java @@ -0,0 +1,3 @@ +module module.b { + exports org.jetbrains.b; +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/B/src/org/jetbrains/b/MyBClass.java b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/B/src/org/jetbrains/b/MyBClass.java new file mode 100644 index 000000000000..c17b957d1969 --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/B/src/org/jetbrains/b/MyBClass.java @@ -0,0 +1,4 @@ +package org.jetbrains.b; + +public class MyBClass { +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/main.iml b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/main.iml new file mode 100644 index 000000000000..94fac919f338 --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/main.iml @@ -0,0 +1,13 @@ + + + + + + + + + + + + + \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/src/Main.java b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/src/Main.java new file mode 100644 index 000000000000..a8e07718627d --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/src/Main.java @@ -0,0 +1,5 @@ +public class Main { + private void foo() { + new MyACla + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/src/module-info.java b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/src/module-info.java new file mode 100644 index 000000000000..834a3a9099c2 --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/jigsaw/circularDependencyCompletion/before/src/module-info.java @@ -0,0 +1,3 @@ +module module.main { + requires module.b; +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/quickfix/AddModuleDirectiveTest.kt b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/quickfix/AddModuleDirectiveTest.kt index 4f81824903c1..2706efd2faea 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/quickfix/AddModuleDirectiveTest.kt +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/quickfix/AddModuleDirectiveTest.kt @@ -1,7 +1,8 @@ -// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.codeInsight.daemon.impl.quickfix import com.intellij.java.testFramework.fixtures.LightJava9ModulesCodeInsightFixtureTestCase +import com.intellij.java.testFramework.fixtures.MultiModuleJava9ProjectDescriptor import com.intellij.modcommand.ModCommandAction import com.intellij.openapi.command.CommandProcessor import com.intellij.psi.PsiJavaFile @@ -43,6 +44,11 @@ class AddModuleDirectiveTest : LightJava9ModulesCodeInsightFixtureTestCase() { "module M { requires M2; }", "module M { requires M2; }") + fun testNoCircularDependency() { + addFile("module-info.java", "module M2 { requires M; }", MultiModuleJava9ProjectDescriptor.ModuleDescriptor.M2) + doRequiresTest("module M { }", "module M { }") + } + fun testRequiresInIncompleteModule(): Unit = doRequiresTest( "module M {", "module M {\n" + diff --git a/java/java-tests/testSrc/com/intellij/java/codeInsight/completion/JigsawCodeCompletionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInsight/completion/JigsawCodeCompletionTest.java index c36564defc69..2aedc79d26f2 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInsight/completion/JigsawCodeCompletionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInsight/completion/JigsawCodeCompletionTest.java @@ -1,4 +1,4 @@ -// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.java.codeInsight.completion; import com.intellij.JavaTestUtil; @@ -152,9 +152,34 @@ public class JigsawCodeCompletionTest extends LightFixtureCompletionTestCase { }"""); } + public void testCircularDependencyCompletion() { + completeBasic("Main.java", """ + public class Main { + private void foo() { + new MyACla + } + } + """) + .variants(new Variant("MyAClass", " org.jetbrains.a", Color.RED)) + .choose(new Variant("MyAClass", " org.jetbrains.a", Color.RED)) + .check("Main.java", """ + import org.jetbrains.a.MyAClass; + + public class Main { + private void foo() { + new MyAClass() + } + } + """) + .check("module-info.java", """ + module module.main { + requires module.b; + }"""); + } + private JigsawCodeCompletionTest variants(Variant @NotNull ... variants) { final LookupElement[] elements = myFixture.getLookupElements(); - assertEquals(Arrays.toString(elements), elements.length, variants.length); + assertEquals(Arrays.toString(elements), variants.length, elements.length); for (int i = 0; i < elements.length; i++) { final LookupElementPresentation element = NormalCompletionTestCase.renderElement(elements[i]); assertEquals(variants[i].text(), element.getItemText());