feat: add ErrorProne and NullAway integration guide and JSpecify migration playbook#13882
feat: add ErrorProne and NullAway integration guide and JSpecify migration playbook#13882nnicolee wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces two new documentation files: an integration guide for ErrorProne and NullAway, and an automation playbook for JSpecify migration. The review feedback identifies critical issues in the provided code snippets, including Maven configuration examples that combine multiple compiler arguments within single <arg> tags (which causes compilation failures), and Python regular expressions for automating annotations that discard visibility modifiers or generate invalid Java code when other modifiers like static or final are present.
| <compilerArgs> | ||
| <arg>-XDcompilePolicy=simple</arg> | ||
| <!-- Enforces NullAway JSpecify violations --> | ||
| <arg>-Xplugin:ErrorProne -Xep:NullAway:ERROR -XepOpt:NullAway:AnnotatedPackages=<your.package.prefix> -XepOpt:NullAway:JSpecifyMode=true</arg> |
There was a problem hiding this comment.
In the maven-compiler-plugin configuration, passing multiple space-separated compiler arguments within a single <arg> tag will cause Maven to pass them as a single quoted argument to javac. This results in a compilation error (e.g., invalid flag). Each compiler argument must be defined in its own <arg> element.
| <arg>-Xplugin:ErrorProne -Xep:NullAway:ERROR -XepOpt:NullAway:AnnotatedPackages=<your.package.prefix> -XepOpt:NullAway:JSpecifyMode=true</arg> | |
| <arg>-Xplugin:ErrorProne</arg> | |
| <arg>-Xep:NullAway:ERROR</arg> | |
| <arg>-XepOpt:NullAway:AnnotatedPackages=<your.package.prefix></arg> | |
| <arg>-XepOpt:NullAway:JSpecifyMode=true</arg> |
| package_line = package_match.group(1) | ||
| content = content.replace(package_line, package_line + "\nimport org.jspecify.annotations.NullMarked;") | ||
| # Add @NullMarked before the class/interface/enum | ||
| content = re.sub(r"\n(public\s+)?(class|interface|enum)\s+", r"\n@NullMarked\n\2 ", content) |
There was a problem hiding this comment.
The regular expression replacement string r"\n@NullMarked\n\2 " discards group 1 (public\s+)?. This will strip the public modifier from any public class, interface, or enum (e.g., converting public class MyClass to @NullMarked\nclass MyClass). Update the replacement pattern to include \1 to preserve the modifier.
| content = re.sub(r"\n(public\s+)?(class|interface|enum)\s+", r"\n@NullMarked\n\2 ", content) | |
| content = re.sub(r"\n(public\s+)?(class|interface|enum)\s+", r"\n@NullMarked\n\1\2 ", content) |
| Translate legacy annotations (e.g. `javax.annotation.Nullable`) to JSpecify `@Nullable`. If annotations are in declaration positions, programmatically reposition them to type-use positions: | ||
| ```python | ||
| # Reposition from declaration-use to type-use position if necessary | ||
| content = re.sub(r"@Nullable\s+public\s+(\w+)", r"public @Nullable \1", content) |
There was a problem hiding this comment.
The regular expression r"@Nullable\s+public\s+(\w+)" replaced with r"public @Nullable \1" will produce invalid Java code if other modifiers (such as static or final) are present. For example, @Nullable public static String would become public @Nullable static String, which is a compilation error because @Nullable is a type-use annotation and cannot annotate the static modifier. Additionally, this pattern misses other visibility modifiers like private or protected.
Using a more robust regex that matches any modifiers and places @Nullable immediately before the type name resolves this issue.
| content = re.sub(r"@Nullable\s+public\s+(\w+)", r"public @Nullable \1", content) | |
| content = re.sub(r"@Nullable\s+((?:public|protected|private|static|final)\s+)*(\w+)", r"\1@Nullable \2", content) |
| <arg>-XDaddTypeAnnotationsToSymbol=true</arg> | ||
|
|
||
| <!-- Configures ErrorProne patch checkers to write modifications IN_PLACE using JSpecify annotations --> | ||
| <arg>-Xplugin:ErrorProne -XepDisableAllChecks -Xep:FieldMissingNullable:ERROR -Xep:ParameterMissingNullable:ERROR -Xep:ReturnMissingNullable:ERROR -Xep:EqualsMissingNullable:ERROR -XepPatchChecks:FieldMissingNullable,ParameterMissingNullable,ReturnMissingNullable,EqualsMissingNullable -XepPatchLocation:IN_PLACE -XepOpt:Nullness:DefaultNullnessAnnotation=org.jspecify.annotations.Nullable</arg> |
There was a problem hiding this comment.
Similar to the issue in the integration guide, passing multiple space-separated compiler arguments within a single <arg> tag will cause Maven to pass them as a single quoted argument to javac, leading to compilation failures. Each argument must be split into its own <arg> element.
| <arg>-Xplugin:ErrorProne -XepDisableAllChecks -Xep:FieldMissingNullable:ERROR -Xep:ParameterMissingNullable:ERROR -Xep:ReturnMissingNullable:ERROR -Xep:EqualsMissingNullable:ERROR -XepPatchChecks:FieldMissingNullable,ParameterMissingNullable,ReturnMissingNullable,EqualsMissingNullable -XepPatchLocation:IN_PLACE -XepOpt:Nullness:DefaultNullnessAnnotation=org.jspecify.annotations.Nullable</arg> | |
| <arg>-Xplugin:ErrorProne</arg> | |
| <arg>-XepDisableAllChecks</arg> | |
| <arg>-Xep:FieldMissingNullable:ERROR</arg> | |
| <arg>-Xep:ParameterMissingNullable:ERROR</arg> | |
| <arg>-Xep:ReturnMissingNullable:ERROR</arg> | |
| <arg>-Xep:EqualsMissingNullable:ERROR</arg> | |
| <arg>-XepPatchChecks:FieldMissingNullable,ParameterMissingNullable,ReturnMissingNullable,EqualsMissingNullable</arg> | |
| <arg>-XepPatchLocation:IN_PLACE</arg> | |
| <arg>-XepOpt:Nullness:DefaultNullnessAnnotation=org.jspecify.annotations.Nullable</arg> |
This PR introduces two reference documents under the
docs/directory to guide development teams and tooling through adopting strict null-safety compilation checks.Documents Added
docs/errorprone_nullaway_integration_guide.mddocs/jspecify_automation_migration_playbook.md