-
Notifications
You must be signed in to change notification settings - Fork 6
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
add method declaration support #125
Conversation
); | ||
}); | ||
|
||
test('multiple type parameters', () => { |
There was a problem hiding this comment.
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.
2d69922
to
cb8c868
Compare
[], | ||
[], | ||
this.visit(node.name), | ||
null |
There was a problem hiding this comment.
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);
}
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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.
Co-authored-by: Knut Wannheden <[email protected]>
Signed-off-by: OlegDokuka <[email protected]>
Signed-off-by: OlegDokuka <[email protected]>
Signed-off-by: OlegDokuka <[email protected]>
b5ce401
to
ca71be3
Compare
Signed-off-by: OlegDokuka <[email protected]>
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)