From 046fcb22fdd512f5f0483dc9c4906a7913fb4c14 Mon Sep 17 00:00:00 2001 From: Jan Martiska Date: Tue, 28 Nov 2023 13:52:16 +0100 Subject: [PATCH] More checks for return types from GraphQL operations --- .../maven/microprofile-graphql/pom.xml | 5 ++ .../graphql/sample/WeatherService.java | 13 ++++ .../graphql/MicroProfileGraphQLConstants.java | 4 +- .../java/MicroProfileGraphQLASTValidator.java | 63 +++++++++++++++++++ .../java/MicroProfileGraphQLErrorCode.java | 5 +- .../MicroProfileGraphQLValidationTest.java | 18 ++++-- 6 files changed, 101 insertions(+), 7 deletions(-) diff --git a/projects/lsp4mp/projects/maven/microprofile-graphql/pom.xml b/projects/lsp4mp/projects/maven/microprofile-graphql/pom.xml index 3bf3eae3c..fe44580e6 100644 --- a/projects/lsp4mp/projects/maven/microprofile-graphql/pom.xml +++ b/projects/lsp4mp/projects/maven/microprofile-graphql/pom.xml @@ -34,6 +34,11 @@ smallrye-graphql-client 1.0.2 + + io.smallrye.reactive + mutiny + 2.5.1 + io.smallrye smallrye-graphql-client-api diff --git a/projects/lsp4mp/projects/maven/microprofile-graphql/src/main/java/io/openliberty/graphql/sample/WeatherService.java b/projects/lsp4mp/projects/maven/microprofile-graphql/src/main/java/io/openliberty/graphql/sample/WeatherService.java index b2270bc48..311fce8b6 100644 --- a/projects/lsp4mp/projects/maven/microprofile-graphql/src/main/java/io/openliberty/graphql/sample/WeatherService.java +++ b/projects/lsp4mp/projects/maven/microprofile-graphql/src/main/java/io/openliberty/graphql/sample/WeatherService.java @@ -11,6 +11,9 @@ package io.openliberty.graphql.sample; +import io.smallrye.graphql.api.Subscription; + +import java.util.concurrent.Flow; import java.util.HashMap; import java.util.LinkedList; import java.util.List; @@ -95,4 +98,14 @@ public void myMethod() { public void myOtherMethod() { } + @Subscription + public String subscriptionReturningNonMulti() { + return null; + } + + @Query + public Flow.Publisher queryReturningMulti() { + return null; + } + } diff --git a/src/main/java/com/redhat/devtools/intellij/lsp4mp4ij/psi/internal/graphql/MicroProfileGraphQLConstants.java b/src/main/java/com/redhat/devtools/intellij/lsp4mp4ij/psi/internal/graphql/MicroProfileGraphQLConstants.java index b86141ff5..3bd3cb049 100644 --- a/src/main/java/com/redhat/devtools/intellij/lsp4mp4ij/psi/internal/graphql/MicroProfileGraphQLConstants.java +++ b/src/main/java/com/redhat/devtools/intellij/lsp4mp4ij/psi/internal/graphql/MicroProfileGraphQLConstants.java @@ -23,10 +23,12 @@ private MicroProfileGraphQLConstants() { public static final String QUERY_ANNOTATION = "org.eclipse.microprofile.graphql.Query"; public static final String MUTATION_ANNOTATION = "org.eclipse.microprofile.graphql.Mutation"; - public static final String SUBSCRIPTION_ANNOTATION = "org.eclipse.microprofile.graphql.Subscription"; + public static final String SUBSCRIPTION_ANNOTATION = "io.smallrye.graphql.api.Subscription"; public static final String GRAPHQL_API_ANNOTATION = "org.eclipse.microprofile.graphql.GraphQLApi"; public static final String UNION_ANNOTATION = "io.smallrye.graphql.api.Union"; public static final String DIRECTIVE_ANNOTATION = "io.smallrye.graphql.api.Directive"; public static final String DIAGNOSTIC_SOURCE = "microprofile-graphql"; + public static final String MULTI = "io.smallrye.mutiny.Multi"; + public static final String FLOW_PUBLISHER = "java.util.concurrent.Flow.Publisher"; } \ No newline at end of file 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 16054a819..e1e0fdc5b 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 @@ -58,6 +58,8 @@ public class MicroProfileGraphQLASTValidator extends JavaASTValidator { 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}''"; + private static final String SUBSCRIPTION_MUST_RETURN_MULTI = "Methods annotated with `@Subscription` have to return either `io.smallrye.mutiny.Multi` or `java.util.concurrent.Flow.Publisher`."; + private static final String SINGLE_RESULT_OPERATION_MUST_NOT_RETURN_MULTI = "Methods annotated with `@Query` or `@Mutation` cannot return `io.smallrye.mutiny.Multi` or `java.util.concurrent.Flow.Publisher`."; boolean allowsVoidReturnFromOperations = true; @@ -84,6 +86,8 @@ public void visitMethod(PsiMethod node) { if(!allowsVoidReturnFromOperations) { validateNoVoidReturnedFromOperations(node); } + validateMultiReturnTypeFromSubscriptions(node); + validateNoMultiReturnTypeFromQueriesAndMutations(node); super.visitMethod(node); } @@ -216,4 +220,63 @@ private void validateNoVoidReturnedFromOperations(PsiMethod node) { } } + /** + * A method annotated with `@Subscription` must return a `Multi` or `Flow.Publisher`. + */ + private void validateMultiReturnTypeFromSubscriptions(PsiMethod node) { + if(node.getReturnType() == null) { + return; + } + for (PsiAnnotation annotation : node.getAnnotations()) { + if (isMatchAnnotation(annotation, MicroProfileGraphQLConstants.SUBSCRIPTION_ANNOTATION)) { + if(node.getReturnType().equals(PsiType.VOID)) { + super.addDiagnostic(SUBSCRIPTION_MUST_RETURN_MULTI, + MicroProfileGraphQLConstants.DIAGNOSTIC_SOURCE, + node.getReturnTypeElement(), + MicroProfileGraphQLErrorCode.SUBSCRIPTION_MUST_RETURN_MULTI, + DiagnosticSeverity.Error); + return; + } + if (node.getReturnType() instanceof PsiClassReferenceType) { + String returnTypeName = ((PsiClassReferenceType) node.getReturnType()).getReference().getQualifiedName(); + if (!returnTypeName.equals(MicroProfileGraphQLConstants.MULTI) + && !returnTypeName.equals(MicroProfileGraphQLConstants.FLOW_PUBLISHER)) { + super.addDiagnostic(SUBSCRIPTION_MUST_RETURN_MULTI, + MicroProfileGraphQLConstants.DIAGNOSTIC_SOURCE, + node.getReturnTypeElement(), + MicroProfileGraphQLErrorCode.SUBSCRIPTION_MUST_RETURN_MULTI, + DiagnosticSeverity.Error); + } + + } + } + } + } + + /** + * Methods annotated with `@Query` or `@Mutation` must NOT return + * a `Multi` or `Flow.Publisher` type. + */ + private void validateNoMultiReturnTypeFromQueriesAndMutations(PsiMethod node) { + if(node.getReturnType() == null) { + return; + } + for (PsiAnnotation annotation : node.getAnnotations()) { + if (isMatchAnnotation(annotation, MicroProfileGraphQLConstants.QUERY_ANNOTATION) + || isMatchAnnotation(annotation, MicroProfileGraphQLConstants.MUTATION_ANNOTATION)) { + if (node.getReturnType() instanceof PsiClassReferenceType) { + String returnTypeName = ((PsiClassReferenceType) node.getReturnType()).getReference().getQualifiedName(); + if (returnTypeName.equals(MicroProfileGraphQLConstants.MULTI) + || returnTypeName.equals(MicroProfileGraphQLConstants.FLOW_PUBLISHER)) { + super.addDiagnostic(SINGLE_RESULT_OPERATION_MUST_NOT_RETURN_MULTI, + MicroProfileGraphQLConstants.DIAGNOSTIC_SOURCE, + node.getReturnTypeElement(), + MicroProfileGraphQLErrorCode.SINGLE_RESULT_OPERATION_MUST_NOT_RETURN_MULTI, + 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 18cd1b5c0..1bcc25323 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 @@ -22,8 +22,9 @@ public enum MicroProfileGraphQLErrorCode implements IJavaErrorCode { NO_VOID_QUERIES, NO_VOID_MUTATIONS, - WRONG_DIRECTIVE_PLACEMENT - ; + WRONG_DIRECTIVE_PLACEMENT, + SUBSCRIPTION_MUST_RETURN_MULTI, + SINGLE_RESULT_OPERATION_MUST_NOT_RETURN_MULTI; @Override public String getCode() { diff --git a/src/test/java/com/redhat/devtools/intellij/lsp4mp4ij/psi/core/graphql/java/MicroProfileGraphQLValidationTest.java b/src/test/java/com/redhat/devtools/intellij/lsp4mp4ij/psi/core/graphql/java/MicroProfileGraphQLValidationTest.java index c7e33c80f..ebe81c109 100644 --- a/src/test/java/com/redhat/devtools/intellij/lsp4mp4ij/psi/core/graphql/java/MicroProfileGraphQLValidationTest.java +++ b/src/test/java/com/redhat/devtools/intellij/lsp4mp4ij/psi/core/graphql/java/MicroProfileGraphQLValidationTest.java @@ -47,23 +47,33 @@ public void testIncorrectDirectivePlacement() throws Exception { diagnosticsParams.setUris(Collections.singletonList(javaFileUri)); diagnosticsParams.setDocumentFormat(DocumentFormat.Markdown); - Diagnostic d1 = d(38, 4, 15, + Diagnostic d1 = d(41, 4, 15, "Directive 'io.openliberty.graphql.sample.Optimistic' is not allowed on element type 'FIELD_DEFINITION'", DiagnosticSeverity.Error, MicroProfileGraphQLConstants.DIAGNOSTIC_SOURCE, MicroProfileGraphQLErrorCode.WRONG_DIRECTIVE_PLACEMENT); - Diagnostic d2 = d(48, 50, 61, + Diagnostic d2 = d(51, 50, 61, "Directive 'io.openliberty.graphql.sample.Optimistic' is not allowed on element type 'ARGUMENT_DEFINITION'", DiagnosticSeverity.Error, MicroProfileGraphQLConstants.DIAGNOSTIC_SOURCE, MicroProfileGraphQLErrorCode.WRONG_DIRECTIVE_PLACEMENT); - Diagnostic d3 = d(72, 59, 70, + Diagnostic d3 = d(75, 59, 70, "Directive 'io.openliberty.graphql.sample.Optimistic' is not allowed on element type 'ARGUMENT_DEFINITION'", DiagnosticSeverity.Error, MicroProfileGraphQLConstants.DIAGNOSTIC_SOURCE, MicroProfileGraphQLErrorCode.WRONG_DIRECTIVE_PLACEMENT); + Diagnostic d4 = d(101, 11, 17, + "Methods annotated with `@Subscription` have to return either `io.smallrye.mutiny.Multi` or `java.util.concurrent.Flow.Publisher`.", + DiagnosticSeverity.Error, MicroProfileGraphQLConstants.DIAGNOSTIC_SOURCE, + MicroProfileGraphQLErrorCode.SUBSCRIPTION_MUST_RETURN_MULTI); + + Diagnostic d5 = d(106, 11, 24, + "Methods annotated with `@Query` or `@Mutation` cannot return `io.smallrye.mutiny.Multi` or `java.util.concurrent.Flow.Publisher`.", + DiagnosticSeverity.Error, MicroProfileGraphQLConstants.DIAGNOSTIC_SOURCE, + MicroProfileGraphQLErrorCode.SINGLE_RESULT_OPERATION_MUST_NOT_RETURN_MULTI); + assertJavaDiagnostics(diagnosticsParams, utils, - d1, d2, d3); + d1, d2, d3, d4, d5); } } \ No newline at end of file