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

The format of the signature in the eu.fasten.core.data.JavaNode is very strange #248

Open
tmortagne opened this issue Feb 25, 2021 · 3 comments

Comments

@tmortagne
Copy link
Member

tmortagne commented Feb 25, 2021

It looks like method()/package.of.the.return.type/ReturnTypeName while I would have expected something closer to what we usually see in Java like "some.package.MyClass.method()" or "some.package.MyClass#method()".

That being said it feels wrong to store this full String instead of separated elements which are only merged when you call getSignature() for example. That way it's a lot easier to create whatever format you want and more importantly it reduces drastically the current huge duplication this storage cause in memory because then you can properly reuse the same String instance for the package (and even imagine a list of strings instead of a single String for the package to reduce even more the duplication), the class, the return types, etc.

Regarding the return type, while it's definitely useful information for some use cases it's not part of the method signature (it's part of the declaration along with other erased stuff like the method parameters annotations and generics) and should probably be moved to the metadata to avoid polluting the signature and, worst, probably cause problems during stitching when you try to match an external callable from one package to an internal callable from another package during stitching for example when the return type slightly changes from one version to another of some dependency.

@proksch
Copy link
Contributor

proksch commented Nov 22, 2021

Not sure if this issue still applies, but I think the FastenURI syntax is baked deeply into the project and hard to remove. The silver lining is that it should not be necessary to work with these String. The FastenURI class should take care of the parsign and provide access to all contained information.

@ashkboos
Copy link
Contributor

ashkboos commented Feb 2, 2022

There are two reasons for this 1) Unfortunately in some cases, JVM and Java code does not produce 1:1 methods. IIRC it was for generic types that JVM would generate synthetic methods. The only way that we could capture everything from bytecode was to include return types in the signature. 2) As Sebastian mentioned FastenUri.
However, I agree that our representation is a bit inefficient. Some of it is caused by the fact that we initially wanted a generic solution for all 3 languages (e.g. FastenUri). Now that we know our pipeline is quite independent for each language, we are revising our steps accordingly. I think in one of our next refactorings where we refactor CG generation and merging, we need to revise our representation drastically, we might even be able to use the serializer/deserializer provided by OPAL itself if we do not need a generic solution. Then such problems will be solved automatically.

@proksch
Copy link
Contributor

proksch commented Mar 10, 2022

@ashkboos Could you please follow up on this ticket or close it once it is resolved?

Anther related ticket is #260 which also points out an issue with the FastenUri creation.

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

No branches or pull requests

3 participants