| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| The task is to annotate public API classes (marked with `@PublicAPI`) with JSpecify nullability annotations. | ||
|
|
||
| Note that JSpecify is already used in this repository so it's already imported. | ||
|
|
||
| If you see a builder static class, you can label it `@NullUnmarked` and not need to do anymore for this static class in terms of annotations. | ||
|
|
||
| Analyze this Java class and add JSpecify annotations based on: | ||
| 1. Set the class to be `@NullMarked` | ||
| 2. Remove all the redundant `@NonNull` annotations that IntelliJ added | ||
| 3. Check Javadoc @param tags mentioning "null", "nullable", "may be null" | ||
| 4. Check Javadoc @return tags mentioning "null", "optional", "if available" | ||
| 5. Method implementations that return null or check for null | ||
| 6. GraphQL specification details (see details below) | ||
|
|
||
| ## GraphQL Specification Compliance | ||
| This is a GraphQL implementation. When determining nullability, consult the GraphQL specification (https://spec.graphql.org/draft/) for the relevant concept. Key principles: | ||
|
|
||
| The spec defines which elements are required (non-null) vs optional (nullable). Look for keywords like "MUST" to indicate when an element is required, and conditional words such as "IF" to indicate when an element is optional. | ||
|
|
||
| If a class implements or represents a GraphQL specification concept, prioritize the spec's nullability requirements over what IntelliJ inferred. | ||
|
|
||
| ## How to validate | ||
| Finally, please check all this works by running the NullAway compile check. | ||
|
|
||
| If you find NullAway errors, try and make the smallest possible change to fix them. If you must, you can use assertNotNull. Make sure to include a message as well. | ||
|
|
||
| ## Formatting Guidelines | ||
|
|
||
| Do not make spacing or formatting changes. Avoid adjusting whitespace, line breaks, or other formatting when editing code. These changes make diffs messy and harder to review. Only make the minimal changes necessary to accomplish the task. | ||
|
|
||
| ## Cleaning up | ||
| Finally, can you remove this class from the JSpecifyAnnotationsCheck as an exemption | ||
|
|
||
| You do not need to run the JSpecifyAnnotationsCheck. Removing the completed class is enough. | ||
|
|
||
| Remember to delete all unused imports when you're done from the class you've just annotated. | ||
|
|
||
| ## Generics Annotations | ||
|
|
||
| When annotating generic types and methods, follow these JSpecify rules: | ||
|
|
||
| ### Type Parameter Bounds | ||
|
|
||
| The bound on a type parameter determines whether nullable type arguments are allowed: | ||
|
|
||
| | Declaration | Allows `@Nullable` type argument? | | ||
| |-------------|----------------------------------| | ||
| | `<T>` | ❌ No — `Box<@Nullable String>` is illegal | | ||
| | `<T extends @Nullable Object>` | ✅ Yes — `Box<@Nullable String>` is legal | | ||
|
|
||
| **When to use `<T extends @Nullable Object>`:** | ||
| - When callers genuinely need to parameterize with nullable types | ||
| - Example: `DataFetcherResult<T extends @Nullable Object>` — data fetchers may return nullable types | ||
|
|
||
| **When to keep `<T>`:** | ||
| - When the type parameter represents a concrete non-null object | ||
| - Even if some methods return `@Nullable T` (meaning "can be null even if T is non-null") | ||
| - Example: `Edge<T>` with `@Nullable T getNode()` — node may be null, but T represents the object type | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| Expand Up | @@ -3,6 +3,8 @@ | |
|
|
||
| import graphql.execution.ResultPath; | ||
| import graphql.language.SourceLocation; | ||
| import org.jspecify.annotations.NullMarked; | ||
| import org.jspecify.annotations.Nullable; | ||
|
|
||
| import java.util.Collections; | ||
| import java.util.LinkedHashMap; | ||
| Expand All | @@ -16,13 +18,14 @@ | |
| * This graphql error will be used if a runtime exception is encountered while a data fetcher is invoked | ||
| */ | ||
| @PublicApi | ||
| @NullMarked | ||
| public class ExceptionWhileDataFetching implements GraphQLError { | ||
|
Comment thread
Copy link
Copy Markdown
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityThe nullability of these fields depends on a combination of the code and the GraphQL specification
Sorry, something went wrong.
All reactions
|
||
|
|
||
| private final String message; | ||
| private final List<Object> path; | ||
| private final Throwable exception; | ||
| private final List<SourceLocation> locations; | ||
| private final Map<String, Object> extensions; | ||
| private final @Nullable Map<String, Object> extensions; | ||
|
|
||
| public ExceptionWhileDataFetching(ResultPath path, Throwable exception, SourceLocation sourceLocation) { | ||
| this.path = assertNotNull(path).toList(); | ||
| Expand All | @@ -41,7 +44,7 @@ private String mkMessage(ResultPath path, Throwable exception) { | |
| * exception into the ExceptionWhileDataFetching error and hence have custom "extension attributes" | ||
| * per error message. | ||
| */ | ||
| private Map<String, Object> mkExtensions(Throwable exception) { | ||
| private @Nullable Map<String, Object> mkExtensions(Throwable exception) { | ||
| Map<String, Object> extensions = null; | ||
| if (exception instanceof GraphQLError) { | ||
| Map<String, Object> map = ((GraphQLError) exception).getExtensions(); | ||
| Expand Down Expand Up | @@ -73,7 +76,7 @@ public List<Object> getPath() { | |
| } | ||
|
|
||
| @Override | ||
| public Map<String, Object> getExtensions() { | ||
| public @Nullable Map<String, Object> getExtensions() { | ||
| return extensions; | ||
| } | ||
|
|
||
| Expand Down | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,10 @@ | ||
| package graphql; | ||
|
|
||
|
|
||
| import org.jspecify.annotations.NullMarked; | ||
| import org.jspecify.annotations.NullUnmarked; | ||
| import org.jspecify.annotations.Nullable; | ||
|
|
||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.function.Consumer; | ||
| Expand All | @@ -9,6 +13,7 @@ | |
| * This simple value class represents the result of performing a graphql query. | ||
| */ | ||
| @PublicApi | ||
| @NullMarked | ||
| @SuppressWarnings("TypeParameterUnusedInFormals") | ||
| public interface ExecutionResult { | ||
|
|
||
| Expand All | @@ -22,16 +27,16 @@ public interface ExecutionResult { | |
| * | ||
| * @return the data in the result or null if there is none | ||
| */ | ||
| <T> T getData(); | ||
| <T> @Nullable T getData(); | ||
|
Comment thread
Copy link
Copy Markdown
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityAnnotation inferred from the javadoc (and spec too)
Sorry, something went wrong.
All reactions
|
||
|
|
||
| /** | ||
| * The graphql specification specifies: | ||
| * | ||
| * <p> | ||
| * "If an error was encountered before execution begins, the data entry should not be present in the result. | ||
| * If an error was encountered during the execution that prevented a valid response, the data entry in the response should be null." | ||
| * | ||
| * <p> | ||
| * This allows to distinguish between the cases where {@link #getData()} returns null. | ||
| * | ||
| * <p> | ||
| * See : <a href="https://graphql.github.io/graphql-spec/June2018/#sec-Data">https://graphql.github.io/graphql-spec/June2018/#sec-Data</a> | ||
| * | ||
| * @return <code>true</code> if the entry "data" should be present in the result | ||
| Expand All | @@ -42,14 +47,14 @@ public interface ExecutionResult { | |
| /** | ||
| * @return a map of extensions or null if there are none | ||
| */ | ||
| Map<Object, Object> getExtensions(); | ||
| @Nullable Map<Object, Object> getExtensions(); | ||
|
Comment thread
Copy link
Copy Markdown
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityInferred from javadoc (and also spec)
Sorry, something went wrong.
All reactions
|
||
|
|
||
|
|
||
| /** | ||
| * The graphql specification says that result of a call should be a map that follows certain rules on what items | ||
| * should be present. Certain JSON serializers may or may interpret {@link ExecutionResult} to spec, so this method | ||
| * is provided to produce a map that strictly follows the specification. | ||
| * | ||
| * <p> | ||
| * See : <a href="https://spec.graphql.org/October2021/#sec-Response-Format">https://spec.graphql.org/October2021/#sec-Response-Format</a> | ||
| * | ||
| * @return a map of the result that strictly follows the spec | ||
| Expand Down Expand Up | @@ -88,6 +93,7 @@ static Builder<?> newExecutionResult() { | |
| return ExecutionResultImpl.newExecutionResult(); | ||
| } | ||
|
|
||
| @NullUnmarked | ||
|
Comment thread
Copy link
Copy Markdown
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityAll builders are NullUnmarked
Sorry, something went wrong.
All reactions
|
||
| interface Builder<B extends Builder<B>> { | ||
|
|
||
| /** | ||
| Expand Down | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| Expand Up | @@ -559,14 +559,14 @@ private PreparsedDocumentEntry parseAndValidate(AtomicReference<ExecutionInput> | |
|
|
||
| ParseAndValidateResult parseResult = parse(executionInput, graphQLSchema, instrumentationState); | ||
| if (parseResult.isFailure()) { | ||
| return new PreparsedDocumentEntry(parseResult.getSyntaxException().toInvalidSyntaxError()); | ||
| return new PreparsedDocumentEntry(assertNotNull(parseResult.getSyntaxException(), "Parse result syntax exception cannot be null when failed").toInvalidSyntaxError()); | ||
|
Comment thread
Copy link
Copy Markdown
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityA similar change now has to happen in a bunch of places: now we must be asserting not null on selected lines or else you'll get a NullAway compile warning because the nullity is not clear at this point in the execution
Sorry, something went wrong.
All reactions
|
||
| } else { | ||
| final Document document = parseResult.getDocument(); | ||
| // they may have changed the document and the variables via instrumentation so update the reference to it | ||
| executionInput = executionInput.transform(builder -> builder.variables(parseResult.getVariables())); | ||
| executionInputRef.set(executionInput); | ||
|
|
||
| final List<ValidationError> errors = validate(executionInput, document, graphQLSchema, instrumentationState); | ||
| final List<ValidationError> errors = validate(executionInput, assertNotNull(document, "Document cannot be null when parse succeeded"), graphQLSchema, instrumentationState); | ||
| if (!errors.isEmpty()) { | ||
| return new PreparsedDocumentEntry(document, errors); | ||
| } | ||
| Expand Down Expand Up | @@ -599,7 +599,7 @@ private List<ValidationError> validate(ExecutionInput executionInput, Document d | |
| validationCtx.onDispatched(); | ||
|
|
||
| Predicate<Class<?>> validationRulePredicate = executionInput.getGraphQLContext().getOrDefault(ParseAndValidate.INTERNAL_VALIDATION_PREDICATE_HINT, r -> true); | ||
| Locale locale = executionInput.getLocale() != null ? executionInput.getLocale() : Locale.getDefault(); | ||
| Locale locale = executionInput.getLocale(); | ||
|
Comment thread
Copy link
Copy Markdown
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityAnd now there are IDE alerts of null checks we can remove. When we are done with all JSpecify annotations on public API classes, we will find many more null checks to remove
Sorry, something went wrong.
All reactions
|
||
| List<ValidationError> validationErrors = ParseAndValidate.validate(graphQLSchema, document, validationRulePredicate, locale); | ||
|
|
||
| validationCtx.onCompleted(validationErrors, null); | ||
| Expand Down | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,9 @@ | ||
| package graphql; | ||
|
|
||
| import org.jspecify.annotations.NullMarked; | ||
| import org.jspecify.annotations.NullUnmarked; | ||
| import org.jspecify.annotations.Nullable; | ||
|
|
||
| import java.util.Map; | ||
| import java.util.Objects; | ||
| import java.util.Optional; | ||
| Expand Down Expand Up | @@ -32,12 +36,13 @@ | |
| * You can set this up via {@link ExecutionInput#getGraphQLContext()} | ||
| * | ||
| * All keys and values in the context MUST be non null. | ||
| * | ||
| * <p> | ||
| * The class is mutable via a thread safe implementation but it is recommended to try to use this class in an immutable way if you can. | ||
| */ | ||
| @PublicApi | ||
| @ThreadSafe | ||
| @SuppressWarnings("unchecked") | ||
| @NullMarked | ||
| public class GraphQLContext { | ||
|
|
||
| private final ConcurrentMap<Object, Object> map; | ||
| Expand Down Expand Up | @@ -66,7 +71,7 @@ public GraphQLContext delete(Object key) { | |
| * | ||
| * @return a value or null | ||
| */ | ||
| public <T> T get(Object key) { | ||
| public <T> @Nullable T get(Object key) { | ||
|
Comment thread
Copy link
Copy Markdown
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityTODO: another case of the nullable generic issue
Sorry, something went wrong.
All reactions
Copy link
Copy Markdown
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityUpdate - all values stored in the GraphQLContext are non-null. It's only going to return null if the key is not found. Let's keep it like this.
Sorry, something went wrong.
All reactions
|
||
| return (T) map.get(assertNotNull(key)); | ||
| } | ||
|
|
||
| Expand Down Expand Up | @@ -210,7 +215,7 @@ public GraphQLContext putAll(Consumer<GraphQLContext.Builder> contextBuilderCons | |
| * | ||
| * @return the new value associated with the specified key, or null if none | ||
| */ | ||
| public <T> T compute(Object key, BiFunction<Object, ? super T, ? extends T> remappingFunction) { | ||
| public <T> @Nullable T compute(Object key, BiFunction<Object, ? super T, ? extends T> remappingFunction) { | ||
| assertNotNull(remappingFunction); | ||
| return (T) map.compute(assertNotNull(key), (k, v) -> remappingFunction.apply(k, (T) v)); | ||
| } | ||
| Expand All | @@ -226,7 +231,7 @@ public <T> T compute(Object key, BiFunction<Object, ? super T, ? extends T> rema | |
| * @return the current (existing or computed) value associated with the specified key, or null if the computed value is null | ||
| */ | ||
|
|
||
| public <T> T computeIfAbsent(Object key, Function<Object, ? extends T> mappingFunction) { | ||
| public <T> @Nullable T computeIfAbsent(Object key, Function<Object, ? extends T> mappingFunction) { | ||
| return (T) map.computeIfAbsent(assertNotNull(key), assertNotNull(mappingFunction)); | ||
| } | ||
|
|
||
| Expand All | @@ -241,7 +246,7 @@ public <T> T computeIfAbsent(Object key, Function<Object, ? extends T> mappingFu | |
| * @return the new value associated with the specified key, or null if none | ||
| */ | ||
|
|
||
| public <T> T computeIfPresent(Object key, BiFunction<Object, ? super T, ? extends T> remappingFunction) { | ||
| public <T> @Nullable T computeIfPresent(Object key, BiFunction<Object, ? super T, ? extends T> remappingFunction) { | ||
| assertNotNull(remappingFunction); | ||
| return (T) map.computeIfPresent(assertNotNull(key), (k, v) -> remappingFunction.apply(k, (T) v)); | ||
| } | ||
| Expand All | @@ -254,7 +259,7 @@ public Stream<Map.Entry<Object, Object>> stream() { | |
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object o) { | ||
| public boolean equals(@Nullable Object o) { | ||
| if (this == o) { | ||
| return true; | ||
| } | ||
| Expand Down Expand Up | @@ -315,6 +320,7 @@ public static Builder newContext() { | |
| return new Builder(); | ||
| } | ||
|
|
||
| @NullUnmarked | ||
| public static class Builder { | ||
| private final ConcurrentMap<Object, Object> map = new ConcurrentHashMap<>(); | ||
|
|
||
| Expand Down | ||
| Back | FazBrowse Home | New Git URL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityThis prompt is checked in so subagents can all share this context
This prompt will be deleted after this series of JSpecify annotations is completed
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.