Skip to content
This repository was archived by the owner on May 12, 2025. It is now read-only.

Conversation

OlegDokuka
Copy link
Contributor

@OlegDokuka OlegDokuka commented Oct 8, 2024

This PR maps ts.MethodDeclaration into J.MethodDeclaration. The only custom JS LST type it adds is JS.TypeInfo, which is a helper type to keep info about spaces before : token (in js type info is defined as e.g. var a : String = "asa so we need an extra container to keep spaces before and after : sign)


);
});

test('multiple type parameters', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A test with a return type would also be good.

@OlegDokuka OlegDokuka force-pushed the main-method-declaration branch 2 times, most recently from 2d69922 to cb8c868 Compare October 16, 2024 22:21
[],
[],
this.visit(node.name),
null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Adding the constraints can be done separately. Unlike Java I think TS allows specifying anonymous types inline as in:

type Animal = { name: string };
type Dog = { breed: string } & Animal;

function handleAnimal<T extends Animal>(animal: T) {
    console.log(animal.name);
}

function handleAnimals<T extends { breed: string } & Animal>(animal: T) {
    console.log(animal.name);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup. and for now idk how to solve that properly

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the parsing will not be an issue, ad the property is of type TypeTree. We just need to implement the mapping.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, ok

@With
public class TypeReferencePrefix implements Marker {
UUID id;
Space prefix;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder what we should do about this. I know that this is how this was done for Kotlin, but we specifically want to avoid adding any Space properties to markers.

I also haven't yet decided how we should address this in Python, where I for now "inherited" the solution that was implemented there, which was to wrap the return type into a TypeHint LST type, which then contains that additional space as its prefix. Yet another option would be to wrap the J.MethodDeclaration.

None of these options seem quite right to me.

Can you raise this question on the Slack channel and then once we've settled on something we can update this code. Since we really want to avoid Space properties in markers, I suspect we will end up changing this, however.

OlegDokuka and others added 6 commits October 20, 2024 17:54
Co-authored-by: Knut Wannheden <knut@moderne.io>
Signed-off-by: OlegDokuka <oleh@moderne.io>
Signed-off-by: OlegDokuka <oleh@moderne.io>
Signed-off-by: OlegDokuka <oleh@moderne.io>
@OlegDokuka OlegDokuka force-pushed the main-method-declaration branch from b5ce401 to ca71be3 Compare October 21, 2024 11:27
@arodionov arodionov self-requested a review October 21, 2024 11:28
Signed-off-by: OlegDokuka <oleh@moderne.io>
@OlegDokuka OlegDokuka merged commit 9895db1 into main Oct 21, 2024
2 checks passed
@OlegDokuka OlegDokuka deleted the main-method-declaration branch October 21, 2024 11:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants