Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Panic if asked to generate multi-argument invokes #1599

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

lunaris
Copy link
Contributor

@lunaris lunaris commented Jan 17, 2025

Presently, Java code generation does not support the MultiArgumentInputs field on invokes. When set, this flag instructs the generation of functions that take multiple independent arguments instead of a single object containing all arguments. For example, instead of:

Class.function(FunctionArgs.build().setA(a).setB(b).build());

we might generate:

Class.function(a, b);

Since it is not supported, the correct behaviour is to panic and abort code generation, as we do in other languages with this limitation (e.g. Python). Currently, however, Java does not, proceeding instead to generate incorrect code. This change fixes that, panicking if MultiArgumentInputs is true. This is technically breaking, but a bug fix (and in theory, any SDKs we have generated would already be panicking for Python, so the feeling is that this likely not a highly used feature).

Fixes #1598

@lunaris lunaris requested a review from a team as a code owner January 17, 2025 11:11
Presently, Java code generation does not support the `MultiArgumentInputs` field
on invokes. When set, this flag instructs the generation of functions that take
multiple independent arguments instead of a single object containing all
arguments. For example, instead of:

```java
Class.function(FunctionArgs.build().setA(a).setB(b).build());
```

we might generate:

```java
Class.function(a, b);
```

Since it is not supported, the correct behaviour is to panic and abort code
generation, as we do in other languages with this limitation (e.g. Python).
Currently, however, Java does not, proceeding instead to generate incorrect
code. This change fixes that, panicking if `MultiArgumentInputs` is true. This
is technically breaking, but a bug fix (and in theory, any SDKs we have
generated would already be panicking for Python, so the feeling is that this
likely not a highly used feature).

Fixes #1598
@lunaris lunaris force-pushed the wjones/multi-arg-sig-panics branch from e2d4d4a to 5774393 Compare January 17, 2025 11:27
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.

Don't generate incorrect code for MultiArgSigs
2 participants