Add -AinferOutputDirectory command-line argument#7481
Merged
smillst merged 15 commits intotypetools:masterfrom Feb 25, 2026
Merged
Add -AinferOutputDirectory command-line argument#7481smillst merged 15 commits intotypetools:masterfrom
-AinferOutputDirectory command-line argument#7481smillst merged 15 commits intotypetools:masterfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a new -AinferOutputDirectory option (documented in the manual and CHANGELOG). SourceChecker declares the option; AnnotatedTypeFactory reads it (default "build/whole-program-inference") and forwards it to whole-program-inference components. WholeProgramInferenceJavaParserStorage and WholeProgramInferenceScenesStorage constructors now accept the output-directory string, store it as a Path, remove hard-coded path constants, create directories with Files.createDirectories, switch file handling from File to Path, update write-method signatures, and wrap path/IO errors in UserError. Documentation and LocalWords updated for the new option. Possibly related PRs
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceScenesStorage.java (1)
479-490: 🧹 Nitpick | 🔵 TrivialReturn value of
mkdirs()is ignored.
jaifDir.mkdirs()can silently fail. Consider usingFiles.createDirectories(inferOutputDirectory)which throwsIOExceptionon failure, giving a clear error. This would also be consistent with thePath-based approach used elsewhere.,
♻️ Proposed refactor
public void writeScenes(OutputFormat outputFormat, BaseTypeChecker checker) { // Create WPI directory if it doesn't exist already. - File jaifDir = inferOutputDirectory.toFile(); - if (!jaifDir.exists()) { - jaifDir.mkdirs(); + try { + java.nio.file.Files.createDirectories(inferOutputDirectory); + } catch (IOException e) { + throw new BugInCF("Could not create WPI directory: " + inferOutputDirectory, e); }framework/src/main/java/org/checkerframework/framework/type/AnnotatedTypeFactory.java (1)
622-669:⚠️ Potential issue | 🟡 MinorValidate
-AinferOutputDirectoryis non-empty.An empty value (e.g.,
-AinferOutputDirectory=) will resolve to the working directory or an invalid path in downstream storage, which can lead to surprising output placement. Consider rejecting blank/whitespace values early.🛠️ Proposed validation
String inferOutputDirectory = checker.getOption("inferOutputDirectory", "build/whole-program-inference"); + if (inferOutputDirectory.trim().isEmpty()) { + throw new UserError("-AinferOutputDirectory must be a non-empty path"); + }
🤖 Fix all issues with AI agents
In `@docs/manual/introduction.tex`:
- Around line 696-700: The doc comment for the \-AinferOutputDirectory item uses
a lowercase "gradle"; update the example text to capitalize the build tool name
("Gradle") for consistency. Edit the sentence that currently reads "e.g.,
gradle" (near the \-AinferOutputDirectory item and the sentence about build
system directories) to use "Gradle" instead, preserving the rest of the wording
and punctuation.
In
`@framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceJavaParserStorage.java`:
- Around line 1063-1067: The catch block in
WholeProgramInferenceJavaParserStorage that handles
Files.createDirectories(packageDir) currently throws new UserError("Cannot
create " + packageDir.toAbsolutePath()) without passing the caught IOException
as the cause; update the catch to throw the UserError with the original
exception as the cause (e.g., include the IOException e in the UserError
constructor) so packageDir creation failures preserve the underlying exception
for debugging.
- Around line 1054-1061: The code that computes packageDir uses
root.compilationUnit.getPackageDeclaration().get().getNameAsString().replaceAll(...,
File.separator) which is broken on Windows because String.replaceAll treats the
replacement as a regex replacement and File.separator is "\"; change the call to
use String.replace (literal replacement) instead of replaceAll so the package
name's "." are replaced with File.separator correctly, i.e., update the
expression where packageDir is assigned to call replace rather than replaceAll
on the package name string.
In
`@framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceScenesStorage.java`:
- Around line 498-501: The getJaifPath method builds the JAIF filepath by
concatenating inferOutputDirectory and className which misses a path separator;
change it to construct the path using a proper platform-safe join (e.g.,
Paths.get(inferOutputDirectory, className + ".jaif").toString() or new
File(inferOutputDirectory, className + ".jaif").getPath()) so the resulting path
is correct for getJaifPath.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/manual/introduction.tex (1)
1713-1713:⚠️ Potential issue | 🟡 MinorMissing
AinferOutputDirectoryin LocalWords.Other
A-prefixed options likeAinferOutputOriginalare listed here, but the newly addedAinferOutputDirectoryis missing. This will cause spell-checker warnings.✏️ Proposed fix
-% LocalWords: AwarnRedundantAnnotations AinferOutputOriginal +% LocalWords: AwarnRedundantAnnotations AinferOutputOriginal AinferOutputDirectory
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/manual/introduction.tex`:
- Around line 696-701: The LaTeX text for the infer output directory uses an
unescaped dollar sign for "$HOME" which breaks LaTeX math mode; locate the
documentation entry for \<-AinferOutputDirectory> (the line containing
"$HOME/.gradle/workers") and replace the dollar sign with an escaped version
"\$HOME" so the directory path renders correctly in text/monospace; ensure the
change mirrors existing escaped patterns like "\<\%>", "\<\{\}>", and "\<\_>"
elsewhere.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceJavaParserStorage.java`:
- Around line 184-190: The constructor WholeProgramInferenceJavaParserStorage
currently calls Path.of(inferOutputDirectory) which can throw an unchecked
InvalidPathException; catch InvalidPathException around that conversion (in the
WholeProgramInferenceJavaParserStorage constructor) and rethrow a
checked/User-friendly exception (e.g., wrap in or throw a UserError with a clear
message that the -AinferOutputDirectory value is invalid and include the
original exception message) so the framework surfaces a clean user error instead
of an internal crash; ensure you still set this.inferOutputDirectory only after
successful parsing and preserve the original InvalidPathException as the cause.
---
Duplicate comments:
In
`@framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceJavaParserStorage.java`:
- Around line 1056-1063: The code in WholeProgramInferenceJavaParserStorage sets
packageDir using
root.compilationUnit.getPackageDeclaration().get().getNameAsString().replaceAll("\\.",
File.separator) which throws on Windows because replaceAll treats the
replacement as a regex; change the call to use the literal replacement API by
replacing replaceAll("\\.", File.separator) with replace(".", File.separator) so
dots are replaced with the platform File.separator without regex escaping
issues.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceScenesStorage.java`:
- Around line 486-489: Replace the brittle File.mkdirs() usage in
WholeProgramInferenceScenesStorage where inferOutputDirectory is converted to a
File (variable jaifDir) and mkdirs() is called: use
Files.createDirectories(inferOutputDirectory) instead, catching IOException and
rethrowing as a UserError with a clear message (same pattern used in
WholeProgramInferenceJavaParserStorage). Update imports as needed
(java.nio.file.Files, java.io.IOException) and remove the exist-check/mkdirs
branch so directory creation either succeeds or throws a UserError describing
the failure to create inferOutputDirectory.
---
Duplicate comments:
In
`@framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceJavaParserStorage.java`:
- Around line 1061-1068: The code uses getNameAsString().replaceAll("\\.",
File.separator) which fails on Windows because File.separator is "\" and
replaceAll treats the replacement as a regex replacement; change this to use the
literal String.replace call (e.g., getNameAsString().replace(".",
File.separator)) or otherwise escape the replacement with
Matcher.quoteReplacement(File.separator) so packageDir computation in the
packageDir = inferOutputDirectory.resolve(...) expression works cross-platform
(look for the chain starting with
root.compilationUnit.getPackageDeclaration().get().getNameAsString()).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceScenesStorage.java (2)
95-108:⚠️ Potential issue | 🟡 MinorInaccurate Javadoc: paths are NOT relative to
inferOutputDirectory.Both the
scenesmap comment (line 95) andmodifiedScenescomment (lines 100–101) describe keys as "relative toinferOutputDirectory". However,getJaifPathreturnsinferOutputDirectory.resolve(className + ".jaif").toString(), which produces a full path that includesinferOutputDirectoryas a prefix (e.g.,build/whole-program-inference/my.pkg.MyClass.jaif). The keys are complete paths, not paths relative to that directory.📝 Proposed fix
- /** Maps .jaif file paths (Strings) to Scenes. Relative to inferOutputDirectory. */ + /** Maps .jaif file paths (Strings) to Scenes. Each path is under inferOutputDirectory. */ public final Map<String, ASceneWrapper> scenes = new HashMap<>(); /** * Scenes that were modified since the last time all Scenes were written into .jaif files. Each - * String element of this set is a path (relative to inferOutputDirectory) to the .jaif file of - * the corresponding Scene in the set. It is obtained by passing a class name as argument to the + * String element of this set is a full path (under inferOutputDirectory) to the .jaif file of + * the corresponding Scene in the set. It is obtained by passing a class name as argument to the * {`@link` `#getJaifPath`} method.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceScenesStorage.java` around lines 95 - 108, The Javadoc for the fields scenes and modifiedScenes is incorrect: the keys are full jaif paths that include inferOutputDirectory (produced by getJaifPath), not paths relative to it; update the comments for the fields scenes and modifiedScenes to state that their String keys are full jaif file paths (including the inferOutputDirectory prefix), and remove or replace the phrase "Relative to inferOutputDirectory" so it accurately reflects that getJaifPath returns the complete path.
95-108:⚠️ Potential issue | 🟡 MinorInaccurate Javadoc: the stored keys are full paths, not paths relative to
inferOutputDirectory.Lines 95 and 100–101 describe the map/set keys as "relative to
inferOutputDirectory". However,getJaifPathreturnsinferOutputDirectory.resolve(className + ".jaif").toString(), which produces a path containinginferOutputDirectoryas a prefix (e.g.,build/whole-program-inference/my.pkg.MyClass.jaif), not a path relative to it.📝 Proposed fix
- /** Maps .jaif file paths (Strings) to Scenes. Relative to inferOutputDirectory. */ + /** Maps .jaif file paths (Strings) to Scenes. Each key is a full path under inferOutputDirectory. */ public final Map<String, ASceneWrapper> scenes = new HashMap<>(); /** * Scenes that were modified since the last time all Scenes were written into .jaif files. Each - * String element of this set is a path (relative to inferOutputDirectory) to the .jaif file of - * the corresponding Scene in the set. It is obtained by passing a class name as argument to the + * String element of this set is a full path (under inferOutputDirectory) to the .jaif file of + * the corresponding Scene in the set. It is obtained by passing a class name as argument to the * {`@link` `#getJaifPath`} method.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceScenesStorage.java` around lines 95 - 108, The Javadoc is incorrect: the keys stored in the scenes Map and modifiedScenes Set are full paths that include the inferOutputDirectory prefix (as produced by getJaifPath(inferOutputDirectory.resolve(className + ".jaif").toString())), not paths relative to inferOutputDirectory; update the Javadoc on the fields scenes and modifiedScenes to state they hold full jaif file paths (including the inferOutputDirectory prefix) and keep references to methods that produce/consume those keys (e.g., getJaifPath and updateAnnotationSetInScene) so callers understand the expected format.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceScenesStorage.java`:
- Around line 486-490: Add an import for java.nio.file.Files at the top of
WholeProgramInferenceScenesStorage and replace the fully-qualified Files
reference in the writeScenes block with the short name Files; also remove the
redundant fully-qualified java.io.IOException in that try/catch (use the
already-imported IOException) around
Files.createDirectories(inferOutputDirectory) so the code reads try {
Files.createDirectories(inferOutputDirectory); } catch (IOException e) { throw
new UserError("Cannot create " + inferOutputDirectory.toAbsolutePath(), e); }
referencing the writeScenes method and inferOutputDirectory variable.
- Around line 486-490: In WholeProgramInferenceScenesStorage (e.g., in the
writeScenes method) replace the fully-qualified java.nio.file.Files usage with
an import: add "import java.nio.file.Files;" to the imports, change
java.nio.file.Files.createDirectories(inferOutputDirectory) to
Files.createDirectories(inferOutputDirectory), and simplify the catch from
"catch (java.io.IOException e)" to "catch (IOException e)". This keeps the code
consistent with the existing IOException import and avoids FQN usage.
- Around line 141-151: The constructor WholeProgramInferenceScenesStorage
currently uses Path.of(inferOutputDirectory) which is Java 11+; replace it with
Paths.get(inferOutputDirectory) for Java 8 compatibility, add import
java.nio.file.Paths, and keep the existing InvalidPathException handling and
UserError throw intact; modify the assignment to this.inferOutputDirectory in
the WholeProgramInferenceScenesStorage(AnnotatedTypeFactory atypeFactory, String
inferOutputDirectory) constructor accordingly.
---
Outside diff comments:
In
`@framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceScenesStorage.java`:
- Around line 95-108: The Javadoc for the fields scenes and modifiedScenes is
incorrect: the keys are full jaif paths that include inferOutputDirectory
(produced by getJaifPath), not paths relative to it; update the comments for the
fields scenes and modifiedScenes to state that their String keys are full jaif
file paths (including the inferOutputDirectory prefix), and remove or replace
the phrase "Relative to inferOutputDirectory" so it accurately reflects that
getJaifPath returns the complete path.
- Around line 95-108: The Javadoc is incorrect: the keys stored in the scenes
Map and modifiedScenes Set are full paths that include the inferOutputDirectory
prefix (as produced by getJaifPath(inferOutputDirectory.resolve(className +
".jaif").toString())), not paths relative to inferOutputDirectory; update the
Javadoc on the fields scenes and modifiedScenes to state they hold full jaif
file paths (including the inferOutputDirectory prefix) and keep references to
methods that produce/consume those keys (e.g., getJaifPath and
updateAnnotationSetInScene) so callers understand the expected format.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceScenesStorage.java (1)
96-109:⚠️ Potential issue | 🟡 MinorMisleading Javadoc: map keys include
inferOutputDirectoryas a prefix, not relative to it.Both the
scenesfield comment (line 96) and themodifiedScenescomment (lines 101-102) describe the storedStringkeys as "Relative to inferOutputDirectory". However, after the refactorgetJaifPath()returnsinferOutputDirectory.resolve(className + ".jaif").toString(), so the keys actually includeinferOutputDirectoryas their leading component. Saying "relative to inferOutputDirectory" implies a bareclassName.jaifkey, which is the opposite of what is stored.📝 Proposed fix
- /** Maps .jaif file paths (Strings) to Scenes. Relative to inferOutputDirectory. */ + /** Maps absolute (or relative-to-CWD) .jaif file paths (Strings) to Scenes. */ public final Map<String, ASceneWrapper> scenes = new HashMap<>(); ... /** * Scenes that were modified since the last time all Scenes were written into .jaif files. Each - * String element of this set is a path (relative to inferOutputDirectory) to the .jaif file of - * the corresponding Scene in the set. It is obtained by passing a class name as argument to the + * String element of this set is the full path to the .jaif file of + * the corresponding Scene in the set (as returned by the * {`@link` `#getJaifPath`} method.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceScenesStorage.java` around lines 96 - 109, Update the Javadoc for the fields scenes and modifiedScenes to reflect that their String keys include the inferOutputDirectory prefix (because getJaifPath() now returns inferOutputDirectory.resolve(className + ".jaif").toString()), i.e., document that keys are full paths that start with inferOutputDirectory rather than paths relative to it; reference the fields scenes and modifiedScenes and the getJaifPath/inferOutputDirectory behavior so future readers understand the exact format of stored keys.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceScenesStorage.java`:
- Around line 96-109: Update the Javadoc for the fields scenes and
modifiedScenes to reflect that their String keys include the
inferOutputDirectory prefix (because getJaifPath() now returns
inferOutputDirectory.resolve(className + ".jaif").toString()), i.e., document
that keys are full paths that start with inferOutputDirectory rather than paths
relative to it; reference the fields scenes and modifiedScenes and the
getJaifPath/inferOutputDirectory behavior so future readers understand the exact
format of stored keys.
---
Duplicate comments:
In
`@framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceScenesStorage.java`:
- Around line 148-152: Replace the Java 11-only Path.of usage with the Java 7+
compatible Paths.get in WholeProgramInferenceScenesStorage where
inferOutputDirectory is initialized: locate the try block that sets
this.inferOutputDirectory (using Path.of(inferOutputDirectory)) and change it to
use Paths.get(inferOutputDirectory), keeping the same InvalidPathException
handling and UserError wrapping so runtime compatibility with Java 8 is
preserved.
smillst
approved these changes
Feb 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.