Skip to content

feat: add JSpecify migration and nullmarked automation script#13889

Draft
nnicolee wants to merge 3 commits into
mainfrom
feat/jspecify-migration-script
Draft

feat: add JSpecify migration and nullmarked automation script#13889
nnicolee wants to merge 3 commits into
mainfrom
feat/jspecify-migration-script

Conversation

@nnicolee

Copy link
Copy Markdown
Contributor

This PR introduces jspecify_migration.py at the repository root. This utility script automates the mechanical steps of migrating package/class files to JSpecify null-safety conventions:

Features

  1. Class-level @NullMarked Injection: Scans package definitions and class declarations to safely prepend @NullMarked and its import (correctly placing it relative to existing class-level annotations like @Generated or @AutoValue).
  2. Javax Import Translation: Replaces javax.annotation.Nullable references with org.jspecify.annotations.Nullable.
  3. Access Modifiers Repositioning: Rewrites @Nullable keyword placement for all 14 common access modifier declarations to move the annotation to the correct type-use position.

Usage

python jspecify_migration.py <directory_path>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread jspecify_migration.py
Comment on lines +17 to +34
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:]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current class-matching regex has a few limitations:

  1. It misses several common modifiers such as private, protected, static, sealed, and non-sealed.
  2. It does not support Java 14+ record types.
  3. 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:]

Comment thread jspecify_migration.py
Comment on lines +42 to +60
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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Instead of maintaining 14 separate hardcoded regex patterns, we can simplify this to a single, highly robust regex.

This simplification has several major benefits:

  1. It automatically supports any combination/order of modifiers.
  2. It supports package-private members, parameters, and local variables (which don't have access modifiers but might have other modifiers like final or static).
  3. It avoids issues with complex generic types or type parameters (e.g., <T>) because it only matches the modifiers and repositions @Nullable right 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)

Comment thread jspecify_migration.py
Comment on lines +11 to +14
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;")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
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:]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant