using empty namespace for types with no package #398
Conversation
analyzer/javacg-opal/src/test/java/eu/fasten/analyzer/javacgopal/CGandStitchingTest.java
Outdated
Show resolved
Hide resolved
| @@ -104,7 +107,7 @@ public static String getPackageName(final Type parameter) { | |||
| return slashToDot(Objects | |||
| .requireNonNull(getPackageName(parameter.asArrayType().componentType()))); | |||
| } else if (parameter.asObjectType().packageName().equals("")) { | |||
| return null; | |||
| return ""; | |||
There was a problem hiding this comment.
Do we then end up with a double slash in the URLs? So before it was "/null/ClassName", now it is "//ClassName"... or is this being handled through canonicalize?
There was a problem hiding this comment.
Yes, now it is "//ClassName". Canonicalize does not handle it. In fact, if there is no namespace canonicalize complains about it. That is why we now check before canonicalizing to see if there is no namespace please don't try.
There was a problem hiding this comment.
Hmm, I do not like the // construct... I would argue that the correct version would be /ClassName. Is there an easy way to achieve this?
There was a problem hiding this comment.
It is not allowd by the FastenUri class if you write this:
var v = FastenURI.create("/ClassName");
you would get java.lang.IllegalArgumentException: Missing entity
There was a problem hiding this comment.
I also just realized that even "//ClassName" cannot be correctly parsed by FastenUri. It does not cause any exception but It assumes that ClassName is the product name while it is the name of the class. What should we do about this? should we use some meaningful placeholder for the package name such as /no.package/ClassName? or should we create another URI specific for the classes?
There was a problem hiding this comment.
How about using /ClassName and fixing the code that extracts the package/product/class information?
There was a problem hiding this comment.
In general, I think we should revise the FastenURI at some point. We should improve our validation and I also think that it does not make sense to parse the whole thing on initialization. I would prefer storing the string representation and only parse part of it on demand, we can still cache information, once parsed. ... but this is a discussion for another issue.
|
With Mehdi, we decided to close this PR. Because we should do a major refactoring on the FASTEN URIs or use another technique. |
|
@mir-am @ashkboos did you open an issue to track discussion on the "major refactoring on the FASTEN URIs" mentioned in the @mir-am comment? I guess that a refactoring would address issue #138, #248 and #260. Also can you remind me in which deliverable current implementation of URI is specified? Thanks. |
As suggested in issue #138 we now use empty namespace in the FastenUris of types without package instead of using null