feat: ValuePool can now reference suppliers in any class#1046
feat: ValuePool can now reference suppliers in any class#1046
Conversation
9bfcd68 to
3a489f3
Compare
3a489f3 to
e9fcaad
Compare
There was a problem hiding this comment.
Pull request overview
This pull request enhances the @ValuePool annotation to support supplier methods from any class, not just the fuzz test class. Previously, users could only reference supplier methods by their simple name within the same test class. Now, they can use fully qualified references with the format com.example.ClassName#methodName or com.example.OuterClass$InnerClass#methodName for nested classes.
Changes:
- Added support for fully qualified supplier method references in the format
ClassName#methodName - Refactored
ValuePoolRegistryto resolve and load supplier methods from any class with fallback class loader strategy - Added comprehensive test coverage for external supplier references, nested classes, validation, and edge cases
- Updated documentation in both Javadoc and markdown files with examples and usage guidelines
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/code_intelligence/jazzer/mutation/support/ValuePoolRegistry.java | Refactored supplier resolution to support fully qualified method references with dynamic class loading and validation |
| src/main/java/com/code_intelligence/jazzer/mutation/annotation/ValuePool.java | Updated Javadoc to document the new fully qualified reference syntax |
| src/test/java/com/code_intelligence/jazzer/mutation/support/ValuePoolRegistry.java | Added new test support class with external supplier methods for testing cross-class references |
| src/test/java/com/code_intelligence/jazzer/mutation/support/ValuePoolsTest.java | Added 15 new test cases covering external suppliers, nested classes, validation, caching, and error scenarios |
| src/test/java/com/code_intelligence/jazzer/mutation/support/BUILD.bazel | Added the new test support file to the build configuration |
| docs/mutation-framework.md | Updated documentation with examples of fully qualified supplier references and improved clarity on type propagation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // - 'input' gets Strings from two suppliers: 'edgeCases()' and 'justStrings()' | ||
| // - 'config' keys get Strings, values get Integers, Map itself gets the `map` objects, all from supplier method 'edgeCases()' | ||
| // - 'data' gets byte arrays from both edgeCases() and *.bin files | ||
| // In addition, the Integer values of the Map 'config' have a different configuaration: |
There was a problem hiding this comment.
Spelling error: "configuaration" should be "configuration"
| // In addition, the Integer values of the Map 'config' have a different configuaration: | |
| // In addition, the Integer values of the Map 'config' have a different configuration: |
| // - 'config' keys get Strings, values get Integers, Map itself gets the `map` objects, all from supplier method 'edgeCases()' | ||
| // - 'data' gets byte arrays from both edgeCases() and *.bin files | ||
| // In addition, the Integer values of the Map 'config' have a different configuaration: | ||
| // the values from the value pool will be take with probability 0.01, |
There was a problem hiding this comment.
Spelling error: "take" should be "taken"
| // the values from the value pool will be take with probability 0.01, | |
| // the values from the value pool will be taken with probability 0.01, |
| Map<String, Integer> map = new HashMap<>(); | ||
| map.put("one", 1); | ||
| map.put("two", 2); | ||
|
|
There was a problem hiding this comment.
Line contains a tab character for indentation. The rest of the codebase uses spaces for indentation in code examples. This should be replaced with spaces for consistency.
|
|
||
| // Supplier method is in the fuzz test class | ||
| if (hashIndex == -1) { | ||
| return resolveSupplier(fuzzTestMethod.getDeclaringClass(), supplierRef); |
There was a problem hiding this comment.
For consistency with fully qualified supplier references (which trim whitespace at lines 137-138), local supplier method names should also be trimmed before resolution. Currently, a reference like "@valuepool(" myPool ")" would fail to find the method, while "@valuepool("com.example.MyClass# myPool ")" would succeed after trimming. Consider adding ".trim()" to supplierRef at line 122 for consistent behavior.
| return resolveSupplier(fuzzTestMethod.getDeclaringClass(), supplierRef); | |
| return resolveSupplier(fuzzTestMethod.getDeclaringClass(), supplierRef.trim()); |
| "@ValuePool: supplier method '" | ||
| + supplierRef | ||
| + "' must return a Stream<?> in fuzz test method " | ||
| + supplier.getName()); |
There was a problem hiding this comment.
The error message incorrectly references "supplier.getName()" instead of "fuzzTestMethod.getName()". This results in an inconsistent error message format that shows the supplier method name where the fuzz test method name is expected. The error message should follow the same pattern as line 202, which correctly uses "fuzzTestMethod.getName()".
| + supplier.getName()); | |
| + fuzzTestMethod.getName()); |
When specifying supplier methods for
@ValuePoolannotation, it is now possible to use fully qualified names to access methods of any class, not just the fuzz test class. E.g.: