fix(generator): ensure variadic options in secondary factory methods (#615) #630
+5
−1
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.
This PR fixes Issue #615, where certain generated operation overloads (secondary factory methods) were incorrectly using array types (Options[]) instead of variadic arguments (Options...).
The Problem
In the ClassGenerator, the buildSecondaryFactory method was responsible for creating overloads of operations (e.g., topK without the indexType attribute). While the main factory method correctly used JavaPoet's .varargs() for the options parameter, the secondary factory was simply adding the parameter without setting the variadic flag. This led to an inconsistent API where some methods required an explicit array creation to pass optional attributes.
The Fix
Modified ClassGenerator.java within the buildSecondaryFactory method to check for the options parameter. When the options parameter is encountered, we now explicitly call "factoryBuilder.varargs()" .
Changes in ClassGenerator.java:
if (attr != null && resolver.typeOf(attr).shouldWrapInClass() && defaultTypes.containsKey(attr)) { body.add("$T.class", defaultTypes.get(attr)); } else { factoryBuilder.addParameter(param); // NEW: Ensure variadic property is preserved for Options if (param.name.equals("options")) { factoryBuilder.varargs(); } factoryBuilder.addJavadoc("\n@param $L $L", param.name, paramTags.get(param.name)); typeVars.addAll(new ResolvedType(param.type).findGenerics()); body.add("$L", param.name); }Verification Results
Due to local environment constraints with the native build system, verification was focused on the logic within the tensorflow-core-generator. The fix ensures that the secondary factory now mirrors the variadic behavior of the primary create method, resulting in:
Mentors
Directing this to @karllessard and @Craigacp for review, as discussed in the issue tracker regarding the ClassGenerator logic and the desired consistency for the Java API overloads.