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

Simplify the handling of parameterized types #66

Closed
metacosm opened this issue Dec 18, 2020 · 3 comments
Closed

Simplify the handling of parameterized types #66

metacosm opened this issue Dec 18, 2020 · 3 comments

Comments

@metacosm
Copy link

Right now, you need to use signature passing it a complex string describing how your type is parameterized. It would be nice if that complexity could be hidden away.

@dufoli
Copy link
Contributor

dufoli commented Dec 21, 2020

currently, when Class is parameterized, we need to add signature like that:

String signature = String.format("Ljavax/enterprise/util/AnnotationLiteral<L%1$s;>;L%1$s;",
        eventAnnotationLiteral.getName().toString().replace('.', '/'));

ClassCreator literalClassCreator = ClassCreator.builder().classOutput(classOutput)
        .className(literalClassName)
        .signature(signature)
        .superClass(AnnotationLiteral.class)
        .interfaces(eventAnnotationLiteral.getName().toString())
        .build();

I have introduce on PR a signature generation.

@dufoli
Copy link
Contributor

dufoli commented Dec 25, 2020

ok so now code is ready for review. I do not think it is ready for merge but size of patch need review to know If I go on right direction. I try to not break API which lead to some stuff

  • formalType is added after MethodDescriptor to avoid break of signature of current API
  • I discover that classCreator use slashed type and methodCreator used jvm types (thanks to methodDescriptor). I do not touch it because it have impact on public API but I translate each time I need. I was thinking to introduce a class Descriptor... for consistency
  • I put deprecated on signature setter
  • I introduce a pseudo format to add generics in method return type. It is just an idea (not very happy of it currently) idea is to have "List" which produce List return for descriptor and keep generics. I am thinking to remove it totally. It was during my early code. But prefer to know if I can work to improve that and keep it and just throw it.

basic API is

//public <K> String transform(K thing) { return K.toString());
// signature must be <K:Ljava/lang/Object;>(TK;)Ljava/lang/String;"
try (ClassCreator creator = ClassCreator.builder().classOutput(cl).className("com.MyTest").formalType("K").build()) {
    MethodCreator method = creator.getMethodCreator("transform", "java.lang.String", "K").formalType("K");
    ResultHandle message = method.invokeStaticMethod(MethodDescriptor.ofMethod(Object.class.getName(), "toString", "Ljava/lang/String;"));
    method.returnValue(message);
}

I was thinking to a system to forward formalType from creator to method but it is not that simple. because some methods needs it and other don't. But Idea was to add it to classCreator and auto add to method if param used it but for that I must be able to go to classCreator from methodCreator if I have param with one letter and match with class one.
Anyway, can be a feature for futur.

current implementation split generic part in SignatureUtils.visitType() but I am thinking to merge it to all type just by adding then remove all generics for descriptor but keep it for signature. Advantage no need for extra field (except formalTypes).

Sgitario added a commit to Sgitario/gizmo that referenced this issue Jun 15, 2022
This pull request handles the support of parameterized types when creating methods. Example:
```java
public List<String> methodName() {
}
```

Where `List<String>` is a parameterized type. 

In order to do this, we need to amend the `signature` of the method as the descriptor does not support it (it says class not found when trying to declare `Ljava.util.List<Ljava.lang.String;>;`). 

In order to resolve this, we have added a wrapper type `ParameterizedClass` where we can specify the wrapper type and the param types. Usage:

```
MethodCreator method = creator.getMethodCreator("methodName", new ParameterizedClass(List.class, String.class));
```

This pull request partially resolves: quarkusio#66 (which is intended to be for classes too)
Moreover, these changes are inspired by the changes in quarkusio/quarkus#26074 (that there will be deprecated once gizmo dependency is bumped).

I've verified these changes in Quarkus upstream too (reverting the changes of the mentioned Quarkus pull request) and it worked fine.
@mkouba
Copy link
Contributor

mkouba commented Jan 19, 2023

I think that this issue was resolved in #147.

@mkouba mkouba closed this as completed Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants