fix: handle null constructor in NewFilledInstanceInstruction and varargs with too few args - #456
Open
chenjunwenhao wants to merge 1 commit into
Conversation
…rgs with too few args Two bug fixes: 1. NewFilledInstanceInstruction.newInstance() threw NullPointerException when loadConstructor() returned null (e.g., class without no-arg constructor). Added null check matching NewInstanceInstruction pattern, reporting NO_SUITABLE_CONSTRUCTOR instead. 2. MemberResolver.adapt2VarArgTypes() threw ArrayIndexOutOfBoundsException when calling varargs methods with fewer arguments than required parameters. Added pre-check in both resolveConstructor() and resolveMethod() to skip varargs candidates that cannot satisfy the minimum required parameters. Also preserves exception cause chains in NewFilledInstanceInstruction and NewInstanceInstruction catch blocks by passing the caught exception to ErrorReporter.report().
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
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.
Summary
Fix two runtime crashes that expose raw Java exceptions instead of meaningful QLExpress error messages.
Bug 1: NullPointerException in
NewFilledInstanceInstructionWhen using the classified-JSON syntax (
{'@class': 'ClassName', ...}) to instantiate a class that has no no-arg constructor,loadConstructor()returnsnullbut the code proceeds to callconstructor.newInstance(), resulting in a raw NPE.Fix: Added a null check matching the pattern already used in
NewInstanceInstruction, reportingNO_SUITABLE_CONSTRUCTORwith a clear message.Reproduction:
Bug 2: ArrayIndexOutOfBoundsException in
MemberResolver.adapt2VarArgTypes()When calling a varargs Java method with fewer arguments than the number of required (non-vararg) parameters,
System.arraycopycopies more elements than the destination array can hold, causing anArrayIndexOutOfBoundsException.Fix: Added a pre-check in both
resolveConstructor()andresolveMethod()to skip varargs candidates whereargTypes.length < parameterTypes.length - 1, allowing the resolver to gracefully report "method not found" instead of crashing.Reproduction:
Additional: Preserve exception cause chains
The
catch (Exception e)blocks in bothNewFilledInstanceInstruction.newInstance()andNewInstanceInstruction.newObject()did not pass the caught exception toErrorReporter.report(), losing the original stack trace. Noweis passed as thecatchObjparameter, consistent with howInvocationTargetExceptionis already handled in the same methods.Test plan
testNewFilledInstanceWithNoDefaultConstructor— verifiesNO_SUITABLE_CONSTRUCTORerror instead of NPEtestVarArgsMethodWithTooFewArgs— verifies graceful error for too-few-args varargs call, plus validates normal varargs calls (0 args, exact minimum, multiple varargs items) still work correctly