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

Jsii variadic parameter support #397

Closed
Chriscbr opened this issue Oct 31, 2022 · 6 comments · Fixed by #3643 or #3767
Closed

Jsii variadic parameter support #397

Chriscbr opened this issue Oct 31, 2022 · 6 comments · Fixed by #3643 or #3767
Assignees
Labels
🛠️ compiler Compiler good first issue Good for newcomers 📐 language-design Language architecture

Comments

@Chriscbr
Copy link
Contributor

Tracking issue to support variadic parameters from JSII APIs.

@Chriscbr Chriscbr added 📐 language-design Language architecture 🛠️ compiler Compiler labels Oct 31, 2022
@github-actions
Copy link

Hi,

This issue hasn't seen activity in 60 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days.
Feel free to re-open this issue when there's an update or relevant information to be added.
Thanks!

@github-actions github-actions bot added the Stale label Dec 31, 2022
@eladb eladb removed the Stale label Dec 31, 2022
@yoav-steinberg
Copy link
Contributor

Probably depends on #125

@staycoolcall911 staycoolcall911 changed the title Variadic parameter support Jsii variadic parameter support Feb 7, 2023
mergify bot pushed a commit that referenced this issue Feb 13, 2023
Closes #477 

This PR adds the underpinnings of support for `bring`-ing JSII libraries. The following list of limitations is not comprehensive:

1. Our compiler doesn't support static class/resource members yet, so types with static members may not be imported properly. (#107)
2. Our compiler doesn't support interfaces, so many Terraform libraries currently fail to import. (#1024)
3. Our compiler doesn't support variadic parameters, so methods with such parameters are not available. (#397)
4. Our compiler doesn't support importing enums yet. (#1484)
5. Constructs named `App` cannot be imported (these constructs typically do not have a `scope` and `id` so we would need to model them differently in the compiler in order to make them usable) (#1485)


The core of the changes in this PR are in the `jsii_importer`. The JSII importer now imports all types into nested namespaces inside a new symbol environment named `libraries`, based on the fully-qualified names (FQN) of types. For example, when the SDK is imported through `bring cloud;`, the jsii importer creates a symbol named `@winglang/sdk` in the global namespace that points to a `Namespace` struct. This namespace has its own symbol environment (a lookup table), and a symbol named `cloud` is added to it pointing to another `Namespace`. That namespace's symbol environment is then given symbols pointing to each type in the `cloud` namespace.

The motivation for this model is that no type will ever get imported multiple times (it's not possible for `cloud.Bucket` to get imported twice, even if you were theoretically able to do `bring cloud; bring sdk;`). I'm not sure if this will hold water forever (will Wing apps allow multiple copies of a library in its dependency closure...?) but for now it seemed like a helpful assumption.

Types are automatically inserted into the correct namespace -- so whenever types are needed from other namespaces (for example, `cloud.BucketBase` needs to reference it's base type, `core.Resource`), a new namespace is created ad-hoc in the correct place (in this example, as a sub-namespace of `@winglang/sdk`).

Finally, when all types are imported into appropriate namespaces, the JSII importer will create a symbol in the global symbol  environment named `cloud` which points to the `cloud` namespace within `@winglang/sdk` inside `libraries`
. This is the basis of the `bring "my-lib" as my_whatever` import name assignment.

Other changes:
- Namespaces are now allocated in a vector so that we can construct multiple pointers to a single namespace.
- Clean up comments (the convention in Rust is that docstrings are commented with three slashes -- once we do this, the comments appears on mouse hover in IDEs like VS Code)
- StmtKind::Use is renamed StmtKind::Bring
- Several parts of the compiler have been updated to use &Path instead of &str wherever a path is being referenced
- Create a "FQN" type for organizing FQN-related operations.


Future work:

- #1414
- #1487
- #1488
- #1515
- #1516
- #1517

*By submitting this pull request, I confirm that my contribution is made under the terms of the 
[Monada Contribution License](https://docs.winglang.io/terms-and-policies/contribution-license.html)*.
@github-actions
Copy link

Hi,

This issue hasn't seen activity in 60 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days.
Feel free to re-open this issue when there's an update or relevant information to be added.
Thanks!

@github-actions github-actions bot added the Stale label May 10, 2023
skyrpex added a commit that referenced this issue Jun 21, 2023
@mergify mergify bot closed this as completed in #3643 Aug 2, 2023
mergify bot pushed a commit that referenced this issue Aug 2, 2023
This PR adds the possibility of using variadic arguments

Unfortunately, I had to add the `Clone` attribute to many `enum` and `struct` to be able to create a list of arguments compatible with the requirements for verifying the typing of the variadic argument.

I needed to implement `PartialEq` for `Type`, `TypeRef`, `Class`, `Interface`, `Struct`, and `Enum`.
I'm not sure if it was done in the best way, but it fulfills this specific case.

- [x] Validates if the last argument (variadic) of the function accepts multiple elements.
- [x] Validates if the only argument (variadic) of the function accepts multiple elements.
- [x] Validates if all types of the variadic argument are the same.
- [ ] Accept range as a variadic argument.

Closes #125 
Closes #397

## Checklist

- [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [x] Tests added (always)
- [ ] Docs updated (only required for features)
- [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
@monadabot
Copy link
Contributor

Congrats! 🚀 This was released in Wing 0.24.66.

@yoav-steinberg
Copy link
Contributor

Re opening since we still don't support import variadics from JSII:

if self.has_variadic_parameters(params) {
// TODO: support variadic parameters
// or TODO: emit compiler warning https://github.com/winglang/wing/issues/1475
debug!(
"Skipping method {} with variadic parameters (see https://github.com/winglang/wing/issues/397)",
m.name
);
continue;

@yoav-steinberg yoav-steinberg reopened this Aug 9, 2023
@Chriscbr Chriscbr added the good first issue Good for newcomers label Aug 9, 2023
@Chriscbr Chriscbr self-assigned this Aug 9, 2023
@mergify mergify bot closed this as completed in #3767 Aug 9, 2023
mergify bot pushed a commit that referenced this issue Aug 9, 2023
Fixes #397

## Checklist

- [ ] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [ ] Description explains motivation and solution
- [ ] Tests added (always)
- [ ] Docs updated (only required for features)
- [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
@monadabot
Copy link
Contributor

Congrats! 🚀 This was released in Wing 0.24.91.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛠️ compiler Compiler good first issue Good for newcomers 📐 language-design Language architecture
Projects
Archived in project
6 participants