feat: add JSpecify migration and nullmarked automation script#13889
feat: add JSpecify migration and nullmarked automation script#13889nnicolee wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a Python script, jspecify_migration.py, designed to automate the migration of Java files to JSpecify annotations by adding @NullMarked annotations and repositioning @Nullable to type-use positions. The review feedback highlights several opportunities to make the script more robust and maintainable: updating the class-matching regex to support additional modifiers, Java 14+ records, and files starting directly with class declarations; consolidating 14 repetitive regex patterns for @Nullable repositioning into a single, cleaner regex; and using slice-based insertion for package imports to prevent accidental replacements in comments or string literals.
| class_match = re.search(r"\n(public\s+)?(abstract\s+)?(final\s+)?(class|interface|enum)\s+\w+", content) | ||
| if class_match: | ||
| class_start = class_match.start() | ||
| prefix = content[:class_start] | ||
| lines = prefix.split("\n") | ||
|
|
||
| # Find the correct insertion point (before other class-level annotations) | ||
| insert_idx = len(lines) | ||
| for i in range(len(lines) - 1, -1, -1): | ||
| line = lines[i].strip() | ||
| if line.startswith("@") and not line.startswith("@Override") and not line.startswith("@Test"): | ||
| insert_idx = i | ||
| elif line == "" or line.startswith("//") or line.startswith("/*") or line.startswith("*"): | ||
| break | ||
| else: | ||
| break | ||
| lines.insert(insert_idx, "@NullMarked") | ||
| content = "\n".join(lines) + content[class_start:] |
There was a problem hiding this comment.
The current class-matching regex has a few limitations:
- It misses several common modifiers such as
private,protected,static,sealed, andnon-sealed. - It does not support Java 14+
recordtypes. - It fails to match class declarations at the very beginning of a file (where there is no leading newline).
We can make this regex much more robust by using (\n|^) to support the start of the file, and matching all standard Java modifiers and type declarations (including record).
class_match = re.search(r"(\n|^)\s*(?:(?:public|protected|private|abstract|final|static|sealed|non-sealed)\s+)*(?:class|interface|enum|record)\s+\w+", content)
if class_match:
class_start = class_match.start()
if class_start == 0:
content = "@NullMarked\n" + content
else:
prefix = content[:class_start]
lines = prefix.split("\n")
# Find the correct insertion point (before other class-level annotations)
insert_idx = len(lines)
for i in range(len(lines) - 1, -1, -1):
line = lines[i].strip()
if line.startswith("@") and not line.startswith("@Override") and not line.startswith("@Test"):
insert_idx = i
elif line == "" or line.startswith("//") or line.startswith("/*") or line.startswith("*"):
break
else:
break
lines.insert(insert_idx, "@NullMarked")
content = "\n".join(lines) + content[class_start:]| patterns = [ | ||
| (r"@Nullable\s+public\s+abstract\s+final\s+(\w+)", r"public abstract final @Nullable \1"), | ||
| (r"@Nullable\s+public\s+abstract\s+(\w+)", r"public abstract @Nullable \1"), | ||
| (r"@Nullable\s+public\s+static\s+final\s+(\w+)", r"public static final @Nullable \1"), | ||
| (r"@Nullable\s+public\s+static\s+(\w+)", r"public static @Nullable \1"), | ||
| (r"@Nullable\s+public\s+final\s+(\w+)", r"public final @Nullable \1"), | ||
| (r"@Nullable\s+public\s+(\w+)", r"public @Nullable \1"), | ||
| (r"@Nullable\s+private\s+static\s+final\s+(\w+)", r"private static final @Nullable \1"), | ||
| (r"@Nullable\s+private\s+static\s+(\w+)", r"private static @Nullable \1"), | ||
| (r"@Nullable\s+private\s+final\s+(\w+)", r"private final @Nullable \1"), | ||
| (r"@Nullable\s+private\s+(\w+)", r"private @Nullable \1"), | ||
| (r"@Nullable\s+protected\s+static\s+final\s+(\w+)", r"protected static final @Nullable \1"), | ||
| (r"@Nullable\s+protected\s+static\s+(\w+)", r"protected static @Nullable \1"), | ||
| (r"@Nullable\s+protected\s+final\s+(\w+)", r"protected final @Nullable \1"), | ||
| (r"@Nullable\s+protected\s+(\w+)", r"protected @Nullable \1"), | ||
| ] | ||
|
|
||
| for pattern, replacement in patterns: | ||
| content = re.sub(pattern, replacement, content) |
There was a problem hiding this comment.
Instead of maintaining 14 separate hardcoded regex patterns, we can simplify this to a single, highly robust regex.
This simplification has several major benefits:
- It automatically supports any combination/order of modifiers.
- It supports package-private members, parameters, and local variables (which don't have access modifiers but might have other modifiers like
finalorstatic). - It avoids issues with complex generic types or type parameters (e.g.,
<T>) because it only matches the modifiers and repositions@Nullableright after them, without needing to match the type name itself.
# Reposition @Nullable to be after all modifiers (type-use position)
# This single regex handles any combination of modifiers (including package-private, final parameters, etc.)
modifier_pattern = r"@Nullable\s+((?:(?:public|private|protected|static|final|abstract|synchronized|volatile|transient|native|strictfp|sealed|non-sealed)\s+)+)"
content = re.sub(modifier_pattern, r"\1@Nullable ", content)| package_match = re.search(r"^(package\s+[\w\.]+;)", content, re.MULTILINE) | ||
| if package_match and "org.jspecify.annotations.NullMarked" not in content and "@NullMarked" not in content: | ||
| package_line = package_match.group(1) | ||
| content = content.replace(package_line, package_line + "\nimport org.jspecify.annotations.NullMarked;") |
There was a problem hiding this comment.
Using content.replace(package_line, ...) can lead to accidental replacements if the package declaration string appears elsewhere in the file (e.g., in comments or string literals). Additionally, the package regex can be made more robust to handle spaces or comments before the semicolon.
Using the match's end position (package_match.end()) to perform a slice-based insertion is much safer.
| package_match = re.search(r"^(package\s+[\w\.]+;)", content, re.MULTILINE) | |
| if package_match and "org.jspecify.annotations.NullMarked" not in content and "@NullMarked" not in content: | |
| package_line = package_match.group(1) | |
| content = content.replace(package_line, package_line + "\nimport org.jspecify.annotations.NullMarked;") | |
| package_match = re.search(r"^\s*package\s+[\w\.]+\s*;", content, re.MULTILINE) | |
| if package_match and "org.jspecify.annotations.NullMarked" not in content and "@NullMarked" not in content: | |
| end_pos = package_match.end() | |
| content = content[:end_pos] + "\nimport org.jspecify.annotations.NullMarked;" + content[end_pos:] |
This PR introduces
jspecify_migration.pyat the repository root. This utility script automates the mechanical steps of migrating package/class files to JSpecify null-safety conventions:Features
@NullMarkedInjection: Scans package definitions and class declarations to safely prepend@NullMarkedand its import (correctly placing it relative to existing class-level annotations like@Generatedor@AutoValue).javax.annotation.Nullablereferences withorg.jspecify.annotations.Nullable.@Nullablekeyword placement for all 14 common access modifier declarations to move the annotation to the correct type-use position.Usage