From b1ab98abd6cb1a9596bf406a2a7bf98094c53f7b Mon Sep 17 00:00:00 2001 From: Jan Martiska Date: Mon, 11 Sep 2023 13:32:25 +0200 Subject: [PATCH] Return checks for void operations, only for Quarkus less than 3.1 --- .../java/MicroProfileGraphQLASTValidator.java | 60 ++++++++++++++----- .../java/MicroProfileGraphQLErrorCode.java | 2 + .../intellij/quarkus/QuarkusModuleUtil.java | 38 ++++++++++++ 3 files changed, 85 insertions(+), 15 deletions(-) diff --git a/src/main/java/com/redhat/devtools/intellij/lsp4mp4ij/psi/internal/graphql/java/MicroProfileGraphQLASTValidator.java b/src/main/java/com/redhat/devtools/intellij/lsp4mp4ij/psi/internal/graphql/java/MicroProfileGraphQLASTValidator.java index 3b14933f8..16054a819 100644 --- a/src/main/java/com/redhat/devtools/intellij/lsp4mp4ij/psi/internal/graphql/java/MicroProfileGraphQLASTValidator.java +++ b/src/main/java/com/redhat/devtools/intellij/lsp4mp4ij/psi/internal/graphql/java/MicroProfileGraphQLASTValidator.java @@ -15,29 +15,20 @@ import java.text.MessageFormat; -import java.util.Arrays; import java.util.logging.Logger; +import java.util.regex.Matcher; -import com.intellij.lang.jvm.JvmMethod; -import com.intellij.lang.jvm.types.JvmArrayType; -import com.intellij.lang.jvm.types.JvmPrimitiveType; -import com.intellij.lang.jvm.types.JvmReferenceType; -import com.intellij.lang.jvm.types.JvmType; -import com.intellij.lang.jvm.types.JvmTypeVisitor; -import com.intellij.lang.jvm.types.JvmWildcardType; import com.intellij.openapi.module.Module; import com.intellij.psi.PsiAnnotation; import com.intellij.psi.PsiAnnotationMemberValue; import com.intellij.psi.PsiArrayInitializerMemberValue; import com.intellij.psi.PsiArrayType; import com.intellij.psi.PsiClass; -import com.intellij.psi.PsiClassType; import com.intellij.psi.PsiEnumConstant; import com.intellij.psi.PsiField; import com.intellij.psi.PsiJvmModifiersOwner; import com.intellij.psi.PsiMethod; import com.intellij.psi.PsiParameter; -import com.intellij.psi.PsiParameterList; import com.intellij.psi.PsiType; import com.intellij.psi.impl.source.PsiClassReferenceType; import com.redhat.devtools.intellij.lsp4mp4ij.psi.core.java.diagnostics.JavaDiagnosticsContext; @@ -45,8 +36,8 @@ import com.redhat.devtools.intellij.lsp4mp4ij.psi.core.utils.PsiTypeUtils; import com.redhat.devtools.intellij.lsp4mp4ij.psi.internal.graphql.MicroProfileGraphQLConstants; import com.redhat.devtools.intellij.lsp4mp4ij.psi.internal.graphql.TypeSystemDirectiveLocation; +import com.redhat.devtools.intellij.quarkus.QuarkusModuleUtil; import org.eclipse.lsp4j.DiagnosticSeverity; -import org.jetbrains.annotations.NotNull; import static com.redhat.devtools.intellij.lsp4mp4ij.psi.core.utils.AnnotationUtils.getAnnotation; import static com.redhat.devtools.intellij.lsp4mp4ij.psi.core.utils.AnnotationUtils.isMatchAnnotation; @@ -64,18 +55,35 @@ public class MicroProfileGraphQLASTValidator extends JavaASTValidator { private static final Logger LOGGER = Logger.getLogger(MicroProfileGraphQLASTValidator.class.getName()); + private static final String NO_VOID_MESSAGE = "Methods annotated with microprofile-graphql's `@Query` cannot have 'void' as a return type."; + private static final String NO_VOID_MUTATION_MESSAGE = "Methods annotated with microprofile-graphql's `@Mutation` cannot have 'void' as a return type."; private static final String WRONG_DIRECTIVE_PLACEMENT = "Directive ''{0}'' is not allowed on element type ''{1}''"; + boolean allowsVoidReturnFromOperations = true; + + @Override public boolean isAdaptedForDiagnostics(JavaDiagnosticsContext context) { Module javaProject = context.getJavaProject(); - // Check if microprofile-graphql is on the path - return PsiTypeUtils.findType(javaProject, MicroProfileGraphQLConstants.QUERY_ANNOTATION) != null; + if(PsiTypeUtils.findType(javaProject, MicroProfileGraphQLConstants.QUERY_ANNOTATION) == null) { + return false; + } + // void GraphQL operations are allowed in Quarkus 3.1 and higher + // if we're on an unknown version, allow them too + allowsVoidReturnFromOperations = QuarkusModuleUtil.checkQuarkusVersion(context.getJavaProject(), + matcher -> !matcher.matches() || (Integer.parseInt(matcher.group(1)) > 3 || + (Integer.parseInt(matcher.group(1)) == 3) && Integer.parseInt(matcher.group(2)) > 0), + true); + LOGGER.fine("Allowing void return from GraphQL operations? " + allowsVoidReturnFromOperations); + return true; } @Override public void visitMethod(PsiMethod node) { validateDirectivesOnMethod(node); + if(!allowsVoidReturnFromOperations) { + validateNoVoidReturnedFromOperations(node); + } super.visitMethod(node); } @@ -116,7 +124,7 @@ private void validateDirectivesOnClass(PsiClass node) { } } // an enum may only have directives allowed on ENUM - if (node.isEnum()) { + else if (node.isEnum()) { validateDirectives(node, TypeSystemDirectiveLocation.ENUM); // enum values may only have directives allowed on ENUM_VALUE for (PsiField field : node.getFields()) { @@ -132,7 +140,7 @@ private void validateDirectives(PsiJvmModifiersOwner node, TypeSystemDirectiveLo for (PsiAnnotation annotation : node.getAnnotations()) { PsiClass directiveDeclaration = getDirectiveDeclaration(annotation); if (directiveDeclaration != null) { - LOGGER.severe("Checking directive: " + annotation.getQualifiedName() + " on node: " + node + " (location type = " + actualLocation.name() + ")"); + LOGGER.fine("Checking directive: " + annotation.getQualifiedName() + " on node: " + node + " (location type = " + actualLocation.name() + ")"); PsiArrayInitializerMemberValue allowedLocations = (PsiArrayInitializerMemberValue) directiveDeclaration .getAnnotation(MicroProfileGraphQLConstants.DIRECTIVE_ANNOTATION) .findAttributeValue("on"); @@ -185,5 +193,27 @@ private PsiClass getDirectiveDeclaration(PsiAnnotation annotation) { return null; } + private void validateNoVoidReturnedFromOperations(PsiMethod node) { + // ignore constructors, and non-void methods for now, it's faster than iterating through all annotations + if (node.getReturnTypeElement() == null || + !PsiType.VOID.equals(node.getReturnType())) { + return; + } + for (PsiAnnotation annotation : node.getAnnotations()) { + if (isMatchAnnotation(annotation, MicroProfileGraphQLConstants.QUERY_ANNOTATION) ) { + super.addDiagnostic(NO_VOID_MESSAGE, // + MicroProfileGraphQLConstants.DIAGNOSTIC_SOURCE, // + node.getReturnTypeElement(), // + MicroProfileGraphQLErrorCode.NO_VOID_QUERIES, // + DiagnosticSeverity.Error); + } else if (isMatchAnnotation(annotation, MicroProfileGraphQLConstants.MUTATION_ANNOTATION)) { + super.addDiagnostic(NO_VOID_MUTATION_MESSAGE, // + MicroProfileGraphQLConstants.DIAGNOSTIC_SOURCE, // + node.getReturnTypeElement(), // + MicroProfileGraphQLErrorCode.NO_VOID_MUTATIONS, // + DiagnosticSeverity.Error); + } + } + } } \ No newline at end of file diff --git a/src/main/java/com/redhat/devtools/intellij/lsp4mp4ij/psi/internal/graphql/java/MicroProfileGraphQLErrorCode.java b/src/main/java/com/redhat/devtools/intellij/lsp4mp4ij/psi/internal/graphql/java/MicroProfileGraphQLErrorCode.java index 750d21a1a..18cd1b5c0 100644 --- a/src/main/java/com/redhat/devtools/intellij/lsp4mp4ij/psi/internal/graphql/java/MicroProfileGraphQLErrorCode.java +++ b/src/main/java/com/redhat/devtools/intellij/lsp4mp4ij/psi/internal/graphql/java/MicroProfileGraphQLErrorCode.java @@ -20,6 +20,8 @@ */ public enum MicroProfileGraphQLErrorCode implements IJavaErrorCode { + NO_VOID_QUERIES, + NO_VOID_MUTATIONS, WRONG_DIRECTIVE_PLACEMENT ; diff --git a/src/main/java/com/redhat/devtools/intellij/quarkus/QuarkusModuleUtil.java b/src/main/java/com/redhat/devtools/intellij/quarkus/QuarkusModuleUtil.java index c2449e86a..ad0af318d 100644 --- a/src/main/java/com/redhat/devtools/intellij/quarkus/QuarkusModuleUtil.java +++ b/src/main/java/com/redhat/devtools/intellij/quarkus/QuarkusModuleUtil.java @@ -42,9 +42,13 @@ import org.slf4j.LoggerFactory; import java.io.File; +import java.util.Arrays; import java.util.HashSet; import java.util.List; +import java.util.Optional; import java.util.Set; +import java.util.function.Predicate; +import java.util.regex.Matcher; import java.util.regex.Pattern; public class QuarkusModuleUtil { @@ -179,6 +183,40 @@ public static boolean isQuarkusDeploymentLibrary(@NotNull LibraryOrderEntry libr libraryOrderEntry.getLibraryName().equalsIgnoreCase(QuarkusConstants.QUARKUS_DEPLOYMENT_LIBRARY_NAME); } + private static final Pattern QUARKUS_CORE_PATTERN = Pattern.compile("quarkus-core-(\\d[a-zA-Z\\d-.]+?).jar"); + public static final Pattern QUARKUS_STANDARD_VERSIONING = Pattern.compile("(\\d+).(\\d+).(\\d+)(.Final)?(-redhat-\\\\d+)?$"); + + /** + * Checks whether the quarkus version used in this module matches the given predicate. + * If we're unable to detect the Quarkus version, this method always returns false. + * The predicate is based on a matcher that is based on the QUARKUS_STANDARD_VERSIONING regular expression, + * that means that `matcher.group(1)` returns the major version, `matcher.group(2)` returns the minor version, + * `matcher.group(3)` returns the patch version. + * If the detected Quarkus version does not follow the standard versioning, the matcher does not match at all. + * If we can't detect the Quarkus version, the returned value will be the value of the `returnIfNoQuarkusDetected` parameter. + */ + public static boolean checkQuarkusVersion(Module module, Predicate predicate, boolean returnIfNoQuarkusDetected) { + Optional quarkusCoreJar = Arrays.stream(ModuleRootManager.getInstance(module).orderEntries() + .runtimeOnly() + .classes() + .getRoots()) + .filter(f -> Pattern.matches(QUARKUS_CORE_PATTERN.pattern(), f.getName())) + .findFirst(); + if (quarkusCoreJar.isPresent()) { + Matcher quarkusCoreArtifactMatcher = QUARKUS_CORE_PATTERN.matcher(quarkusCoreJar.get().getName()); + if(quarkusCoreArtifactMatcher.matches()) { + String quarkusVersion = quarkusCoreArtifactMatcher.group(1); + LOGGER.debug("Detected Quarkus version = " + quarkusVersion); + Matcher quarkusVersionMatcher = QUARKUS_STANDARD_VERSIONING.matcher(quarkusVersion); + return predicate.test(quarkusVersionMatcher); + } else { + return false; + } + } else { + return returnIfNoQuarkusDetected; + } + } + public static Set getModulesURIs(Project project) { Set uris = new HashSet<>(); for(Module module : ModuleManager.getInstance(project).getModules()) {