From d843df8aa81de2d3ea3e57a95d610e8f20c0612b Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Fri, 21 Feb 2025 17:09:41 +0100 Subject: [PATCH] [java-highlighting] HighlightUtil.checkModuleReferenceAccess migrated Part of IDEA-365344 Create a new Java error highlighter with minimal dependencies (PSI only) GitOrigin-RevId: 72eaed5e89969c19e29dee7a6921fa3b5f45c6c0 --- .../JavaCompilationErrorBundle.properties | 16 +++++-- .../highlighting/ModuleChecker.java | 48 +++++++++++++++---- .../highlighting/errors/JavaErrorKinds.java | 33 +++++++++++++ .../codeInsight/JavaModuleSystemEx.java | 8 ++-- .../daemon/impl/analysis/HighlightUtil.java | 20 -------- .../impl/analysis/HighlightVisitorImpl.java | 6 --- .../impl/analysis/JavaErrorFixProvider.java | 38 +++++++++++++++ .../psi/impl/JavaPlatformModuleSystem.kt | 12 +---- 8 files changed, 128 insertions(+), 53 deletions(-) diff --git a/java/codeserver/highlighting/resources/messages/JavaCompilationErrorBundle.properties b/java/codeserver/highlighting/resources/messages/JavaCompilationErrorBundle.properties index 169c9af34a19..e23da46d615d 100644 --- a/java/codeserver/highlighting/resources/messages/JavaCompilationErrorBundle.properties +++ b/java/codeserver/highlighting/resources/messages/JavaCompilationErrorBundle.properties @@ -512,9 +512,12 @@ module.file.wrong.name=Module declaration should be in a file named 'module-info module.file.duplicate='module-info.java' already exists in the module module.duplicate.requires=Duplicate ''requires'': {0} module.duplicate.exports=Duplicate ''exports'': {0} +module.duplicate.exports.target=Duplicate ''exports'' target: {0} module.duplicate.opens=Duplicate ''opens'': {0} +module.duplicate.opens.target=Duplicate ''opens'' target: {0} module.duplicate.uses=Duplicate ''uses'': {0} module.duplicate.provides=Duplicate ''provides'': {0} +module.duplicate.implementation=Duplicate implementation: {0} module.file.wrong.location=Module declaration should be located in a module's source root module.opens.in.weak.module='opens' is not allowed in an open module module.service.enum=The service definition is an enum: {0} @@ -527,8 +530,15 @@ module.service.no.constructor=The service implementation does not have a public module.not.found=Module not found: {0} module.not.on.path=Module is not in dependencies: {0} module.cyclic.dependence=Cyclic dependency: {0} -module.duplicate.exports.target=Duplicate ''exports'' target: {0} -module.duplicate.opens.target=Duplicate ''opens'' target: {0} -module.duplicate.implementation=Duplicate implementation: {0} module.reference.package.not.found=Package not found: {0} module.reference.package.empty=Package is empty: {0} +module.access.from.named=Package ''{0}'' is declared in module ''{1}'', which does not export it to module ''{2}'' +module.access.from.unnamed=Package ''{0}'' is declared in module ''{1}'', which does not export it to the unnamed module +module.access.to.unnamed=Package ''{0}'' is declared in the unnamed module, but module ''{1}'' does not read it +module.access.package.bad.name=Package ''{0}'' is declared in module with an invalid name (''{1}'') +module.access.bad.name=Module ''{0}'' has an invalid name +module.access.package.not.in.graph=Package ''{0}'' is declared in module ''{1}'', which is not in the module graph +module.access.not.in.graph=Module ''{0}'' is missing from the module graph +module.access.package.does.not.read=Package ''{0}'' is declared in module ''{1}'', but module ''{2}'' does not read it +module.access.does.not.read=Module ''{0}'' fails to read ''{1}'' +module.access.jps.dependency.problem=Module dependency for ''{0}'' is not specified in project structure diff --git a/java/codeserver/highlighting/src/com/intellij/java/codeserver/highlighting/ModuleChecker.java b/java/codeserver/highlighting/src/com/intellij/java/codeserver/highlighting/ModuleChecker.java index 0cff75187dc1..fd069ed2292d 100644 --- a/java/codeserver/highlighting/src/com/intellij/java/codeserver/highlighting/ModuleChecker.java +++ b/java/codeserver/highlighting/src/com/intellij/java/codeserver/highlighting/ModuleChecker.java @@ -3,6 +3,8 @@ package com.intellij.java.codeserver.highlighting; import com.intellij.java.codeserver.core.JavaPsiModuleUtil; import com.intellij.java.codeserver.core.JavaServiceProviderUtil; +import com.intellij.java.codeserver.core.JpmsModuleAccessInfo; +import com.intellij.java.codeserver.core.JpmsModuleInfo; import com.intellij.java.codeserver.highlighting.errors.JavaErrorKind; import com.intellij.java.codeserver.highlighting.errors.JavaErrorKinds; import com.intellij.openapi.module.Module; @@ -217,15 +219,6 @@ final class ModuleChecker { } } - void checkModuleReference(@NotNull PsiImportModuleStatement statement) { - PsiJavaModuleReferenceElement refElement = statement.getModuleReference(); - if (refElement == null) return; - PsiJavaModuleReference ref = refElement.getReference(); - if (ref != null && ref.resolve() == null) { - reportUnresolvedJavaModule(refElement); - } - } - private void reportUnresolvedJavaModule(@NotNull PsiJavaModuleReferenceElement refElement) { PsiJavaModuleReference ref = refElement.getReference(); assert ref != null : refElement.getParent(); @@ -236,7 +229,9 @@ final class ModuleChecker { ? JavaErrorKinds.REFERENCE_PENDING.create(refElement) : JavaErrorKinds.MODULE_NOT_FOUND.create(refElement)); case 1 -> myVisitor.report(JavaErrorKinds.MODULE_NOT_ON_PATH.create(refElement)); - default -> {} + default -> { + // ambiguous module is reported as warning + } } } @@ -303,4 +298,37 @@ final class ModuleChecker { : JavaErrorKinds.MODULE_REFERENCE_PACKAGE_EMPTY; myVisitor.report(kind.create(statement)); } + + JavaErrorKind.Parameterized accessError(@NotNull JpmsModuleAccessInfo.JpmsModuleAccessProblem problem) { + return switch (problem) { + case FROM_NAMED -> JavaErrorKinds.MODULE_ACCESS_FROM_NAMED; + case FROM_UNNAMED -> JavaErrorKinds.MODULE_ACCESS_FROM_UNNAMED; + case TO_UNNAMED -> JavaErrorKinds.MODULE_ACCESS_TO_UNNAMED; + case PACKAGE_BAD_NAME -> JavaErrorKinds.MODULE_ACCESS_PACKAGE_BAD_NAME; + case BAD_NAME -> JavaErrorKinds.MODULE_ACCESS_BAD_NAME; + case PACKAGE_NOT_IN_GRAPH -> JavaErrorKinds.MODULE_ACCESS_PACKAGE_NOT_IN_GRAPH; + case NOT_IN_GRAPH -> JavaErrorKinds.MODULE_ACCESS_NOT_IN_GRAPH; + case PACKAGE_DOES_NOT_READ -> JavaErrorKinds.MODULE_ACCESS_PACKAGE_DOES_NOT_READ; + case DOES_NOT_READ -> JavaErrorKinds.MODULE_ACCESS_DOES_NOT_READ; + case JPS_DEPENDENCY_PROBLEM -> JavaErrorKinds.MODULE_ACCESS_JPS_DEPENDENCY_PROBLEM; + }; + } + + void checkModuleReference(@NotNull PsiImportModuleStatement statement) { + PsiJavaModuleReferenceElement refElement = statement.getModuleReference(); + if (refElement == null) return; + PsiJavaModuleReference ref = refElement.getReference(); + if (ref == null) return; + PsiJavaModule target = ref.resolve(); + if (target == null) { + reportUnresolvedJavaModule(refElement); + return; + } + JpmsModuleAccessInfo moduleAccess = new JpmsModuleInfo.TargetModuleInfo(target, "").accessAt(myVisitor.file().getOriginalFile()); + JpmsModuleAccessInfo.JpmsModuleAccessProblem problem = moduleAccess.checkModuleAccess(statement); + if (problem != null) { + myVisitor.report(accessError(problem).create(statement, moduleAccess)); + } + } + } diff --git a/java/codeserver/highlighting/src/com/intellij/java/codeserver/highlighting/errors/JavaErrorKinds.java b/java/codeserver/highlighting/src/com/intellij/java/codeserver/highlighting/errors/JavaErrorKinds.java index c5c620509974..acb5787f1d92 100644 --- a/java/codeserver/highlighting/src/com/intellij/java/codeserver/highlighting/errors/JavaErrorKinds.java +++ b/java/codeserver/highlighting/src/com/intellij/java/codeserver/highlighting/errors/JavaErrorKinds.java @@ -6,6 +6,7 @@ import com.intellij.codeInsight.daemon.impl.analysis.HighlightMessageUtil; import com.intellij.core.JavaPsiBundle; import com.intellij.java.codeserver.core.JavaPreviewFeatureUtil; import com.intellij.java.codeserver.core.JavaPsiModuleUtil; +import com.intellij.java.codeserver.core.JpmsModuleAccessInfo; import com.intellij.java.codeserver.highlighting.JavaCompilationErrorBundle; import com.intellij.java.codeserver.highlighting.errors.JavaErrorKind.Parameterized; import com.intellij.java.codeserver.highlighting.errors.JavaErrorKind.Simple; @@ -1541,6 +1542,38 @@ public final class JavaErrorKinds { .withAnchor(st -> st.getPackageReference()) .withRawDescription(st -> message("module.reference.package.empty", st.getPackageName())); + public static final Parameterized MODULE_ACCESS_FROM_NAMED = + parameterized(PsiElement.class, JpmsModuleAccessInfo.class, "module.access.from.named") + .withRawDescription((psi, info) -> message( + "module.access.from.named", info.getTarget().getPackageName(), info.getTarget().getModule().getName(), info.getCurrent().getName())); + public static final Parameterized MODULE_ACCESS_FROM_UNNAMED = + parameterized(PsiElement.class, JpmsModuleAccessInfo.class, "module.access.from.unnamed") + .withRawDescription((psi, info) -> message("module.access.from.unnamed", info.getTarget().getPackageName(), info.getTarget().getModule().getName())); + public static final Parameterized MODULE_ACCESS_TO_UNNAMED = + parameterized(PsiElement.class, JpmsModuleAccessInfo.class, "module.access.to.unnamed") + .withRawDescription((psi, info) -> message("module.access.to.unnamed", info.getTarget().getPackageName(), info.getCurrent().getName())); + public static final Parameterized MODULE_ACCESS_PACKAGE_BAD_NAME = + parameterized(PsiElement.class, JpmsModuleAccessInfo.class, "module.access.package.bad.name") + .withRawDescription((psi, info) -> message("module.access.package.bad.name", info.getTarget().getPackageName(), info.getTarget().getModule().getName())); + public static final Parameterized MODULE_ACCESS_BAD_NAME = + parameterized(PsiElement.class, JpmsModuleAccessInfo.class, "module.access.bad.name") + .withRawDescription((psi, info) -> message("module.access.bad.name", info.getTarget().getModule().getName())); + public static final Parameterized MODULE_ACCESS_PACKAGE_NOT_IN_GRAPH = + parameterized(PsiElement.class, JpmsModuleAccessInfo.class, "module.access.package.not.in.graph") + .withRawDescription((psi, info) -> message("module.access.package.not.in.graph", info.getTarget().getPackageName(), info.getTarget().getModule().getName())); + public static final Parameterized MODULE_ACCESS_NOT_IN_GRAPH = + parameterized(PsiElement.class, JpmsModuleAccessInfo.class, "module.access.not.in.graph") + .withRawDescription((psi, info) -> message("module.access.not.in.graph", info.getTarget().getModule().getName())); + public static final Parameterized MODULE_ACCESS_PACKAGE_DOES_NOT_READ = + parameterized(PsiElement.class, JpmsModuleAccessInfo.class, "module.access.package.does.not.read") + .withRawDescription((psi, info) -> message("module.access.package.does.not.read", info.getTarget().getPackageName(), info.getTarget().getModule().getName(), info.getCurrent().getName())); + public static final Parameterized MODULE_ACCESS_DOES_NOT_READ = + parameterized(PsiElement.class, JpmsModuleAccessInfo.class, "module.access.does.not.read") + .withRawDescription((psi, info) -> message("module.access.does.not.read", info.getTarget().getModule().getName(), info.getCurrent().getName())); + public static final Parameterized MODULE_ACCESS_JPS_DEPENDENCY_PROBLEM = + parameterized(PsiElement.class, JpmsModuleAccessInfo.class, "module.access.jps.dependency.problem") + .withRawDescription((psi, info) -> message("module.access.jps.dependency.problem", info.getTarget().getModule().getName())); + private static @NotNull Simple error( @NotNull @PropertyKey(resourceBundle = JavaCompilationErrorBundle.BUNDLE) String key) { return new Simple<>(key); diff --git a/java/java-analysis-api/src/com/intellij/codeInsight/JavaModuleSystemEx.java b/java/java-analysis-api/src/com/intellij/codeInsight/JavaModuleSystemEx.java index 3ab5cc6ad84c..80dad0a26b94 100644 --- a/java/java-analysis-api/src/com/intellij/codeInsight/JavaModuleSystemEx.java +++ b/java/java-analysis-api/src/com/intellij/codeInsight/JavaModuleSystemEx.java @@ -2,7 +2,10 @@ package com.intellij.codeInsight; import com.intellij.codeInsight.intention.IntentionAction; -import com.intellij.psi.*; +import com.intellij.psi.JavaModuleSystem; +import com.intellij.psi.PsiClass; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiFile; import com.intellij.psi.util.PsiUtil; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; @@ -37,7 +40,4 @@ public interface JavaModuleSystemEx extends JavaModuleSystem { @Nullable ErrorWithFixes checkAccess(@NotNull String targetPackageName, @Nullable PsiFile targetFile, @NotNull PsiElement place); - - @Nullable - ErrorWithFixes checkAccess(@NotNull PsiJavaModule module, @NotNull PsiElement place); } \ No newline at end of file diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java index 1a7fceeca410..008b182f6379 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java @@ -106,26 +106,6 @@ public final class HighlightUtil { return null; } - static HighlightInfo.Builder checkModuleReferenceAccess(@NotNull PsiImportModuleStatement statement) { - PsiJavaModuleReferenceElement refElement = statement.getModuleReference(); - if (refElement == null) return null; - PsiJavaModuleReference ref = refElement.getReference(); - assert ref != null : refElement.getParent(); - PsiJavaModule target = ref.resolve(); - if (target == null) return null; - for (JavaModuleSystem moduleSystem : JavaModuleSystem.EP_NAME.getExtensionList()) { - if (!(moduleSystem instanceof JavaModuleSystemEx javaModuleSystemEx)) continue; - ErrorWithFixes fixes = javaModuleSystemEx.checkAccess(target, statement); - if (fixes == null) continue; - HighlightInfo.Builder info = HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR) - .range(statement) - .descriptionAndTooltip(fixes.message); - fixes.fixes.forEach(fix -> info.registerFix(fix, null, null, null, null)); - return info; - } - return null; - } - private static boolean isAccessible(@NotNull JavaModuleSystem system, @NotNull PsiElement target, @NotNull PsiElement place) { if (target instanceof PsiClass psiClass) return system.isAccessible(psiClass, place); if (target instanceof PsiPackage psiPackage) return system.isAccessible(psiPackage.getQualifiedName(), null, place); diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java index 0f6f2260618a..bdbdbe253eb0 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java @@ -229,12 +229,6 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh } } - @Override - public void visitImportModuleStatement(@NotNull PsiImportModuleStatement statement) { - super.visitImportModuleStatement(statement); - if (!hasErrorResults()) add(HighlightUtil.checkModuleReferenceAccess(statement)); - } - static @Nullable JavaResolveResult resolveOptimised(@NotNull PsiJavaCodeReferenceElement ref, @NotNull PsiFile containingFile) { try { if (ref instanceof PsiReferenceExpressionImpl) { diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaErrorFixProvider.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaErrorFixProvider.java index f69c1d6f324c..6538493ed4a8 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaErrorFixProvider.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaErrorFixProvider.java @@ -14,6 +14,7 @@ import com.intellij.codeInspection.dataFlow.fix.RedundantInstanceofFix; import com.intellij.ide.highlighter.JavaFileType; import com.intellij.java.analysis.JavaAnalysisBundle; import com.intellij.java.codeserver.core.JavaPsiModifierUtil; +import com.intellij.java.codeserver.core.JpmsModuleAccessInfo; import com.intellij.java.codeserver.highlighting.JavaErrorCollector; import com.intellij.java.codeserver.highlighting.errors.JavaCompilationError; import com.intellij.java.codeserver.highlighting.errors.JavaErrorKind; @@ -178,6 +179,43 @@ final class JavaErrorFixProvider { }; fix(MODULE_REFERENCE_PACKAGE_NOT_FOUND, createClassInPackage); fix(MODULE_REFERENCE_PACKAGE_EMPTY, createClassInPackage); + + JavaFixProvider fixExports = error -> { + if (error.context().getTarget().getPackageName().isEmpty()) return null; + Module jpsModule = error.context().getCurrent().getJpsModule(); + PsiJavaModule targetModule = error.context().getTarget().getModule(); + if (targetModule instanceof PsiCompiledElement && jpsModule != null) { + return new AddExportsOptionFix(jpsModule, targetModule.getName(), error.context().getTarget().getPackageName(), + error.context().getCurrent().getName()); + } + if (!(targetModule instanceof PsiCompiledElement) && error.context().getCurrent().getModule() != null) { + return new AddExportsDirectiveFix(requireNonNull(targetModule), error.context().getTarget().getPackageName(), + error.context().getCurrent().getName()); + } + return null; + }; + fix(MODULE_ACCESS_FROM_UNNAMED, fixExports); + fix(MODULE_ACCESS_FROM_NAMED, fixExports); + JavaFixProvider fixModuleOptions = + error -> new AddModulesOptionFix(requireNonNull(error.context().getCurrent().getJpsModule()), + requireNonNull(error.context().getTarget().getModule()).getName()); + fix(MODULE_ACCESS_PACKAGE_NOT_IN_GRAPH, fixModuleOptions); + fix(MODULE_ACCESS_NOT_IN_GRAPH, fixModuleOptions); + JavaFixProvider fixRequires = + error -> new AddRequiresDirectiveFix(requireNonNull(error.context().getCurrent().getModule()), + requireNonNull(error.context().getTarget().getModule()).getName()); + fix(MODULE_ACCESS_PACKAGE_DOES_NOT_READ, fixRequires); + fix(MODULE_ACCESS_DOES_NOT_READ, fixRequires); + fixes(MODULE_ACCESS_JPS_DEPENDENCY_PROBLEM, (error, sink) -> { + if (error.psi() instanceof PsiJavaModuleReferenceElement ref) { + PsiJavaModuleReference reference = ref.getReference(); + if (reference != null) { + List list = new ArrayList<>(); + myFactory.registerOrderEntryFixes(reference, list); + list.forEach(sink); + } + } + }); } private void createStatementFixes() { diff --git a/java/java-impl/src/com/intellij/psi/impl/JavaPlatformModuleSystem.kt b/java/java-impl/src/com/intellij/psi/impl/JavaPlatformModuleSystem.kt index 48262e42a767..6ed9ec107d65 100644 --- a/java/java-impl/src/com/intellij/psi/impl/JavaPlatformModuleSystem.kt +++ b/java/java-impl/src/com/intellij/psi/impl/JavaPlatformModuleSystem.kt @@ -42,15 +42,6 @@ internal class JavaPlatformModuleSystem : JavaModuleSystemEx { return TargetModuleInfo(targetModule, "").accessAt(useFile).checkModuleAccess(place) == null } - override fun checkAccess(targetModule: PsiJavaModule, place: PsiElement): ErrorWithFixes? { - val useFile = place.containingFile?.originalFile ?: return null - val moduleAccess = TargetModuleInfo(targetModule, "").accessAt(useFile) - - val access = moduleAccess.checkModuleAccess(place) - return if (access == null) null - else moduleAccess.toErrorWithFixes(access, place) - } - private fun getProblem(targetPackageName: String, targetFile: PsiFile?, place: PsiElement, quick: Boolean, isAccessible: (JpmsModuleAccessInfo) -> Boolean): ErrorWithFixes? { val originalTargetFile = targetFile?.originalFile @@ -83,7 +74,8 @@ internal class JavaPlatformModuleSystem : JavaModuleSystemEx { val error = checkAccess(TargetModuleInfo(dirs[0], target.qualifiedName), useFile, quick, isAccessible) ?: return null return when { dirs.size == 1 -> error - dirs.asSequence().drop(1).any { checkAccess(TargetModuleInfo(it, target.qualifiedName), useFile, true, isAccessible) == null } -> null + dirs.asSequence().drop(1).any { TargetModuleInfo(it, target.qualifiedName) + .accessAt(useFile).checkAccess(useFile, isAccessible) == null } -> null else -> error } }