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

Support an easy way to obtain all involved JavaClasses of a generic signature #723

Closed
codecholeric opened this issue Nov 21, 2021 · 8 comments · Fixed by #1116
Closed

Support an easy way to obtain all involved JavaClasses of a generic signature #723

codecholeric opened this issue Nov 21, 2021 · 8 comments · Fixed by #1116

Comments

@codecholeric
Copy link
Collaborator

There should be an easy way to obtain all JavaClasses that are involved in composing a generic type signature.

E.g.

class SomeClass {
  Map<? super String, ? extends Serializable> method() {..}
}

Then there should be some API to derive the set of classes [Map, String, Serializable] from method.getReturnType(). We already have some logic like this to calculate the dependencies of a class, since all these classes are also dependencies of the generic return type.

Open question: How should this method be named and where should it live? Should it be something like

JavaType.getRawTypesInvolvedInSignature()

? What would be a good name for this?

@crizzis
Copy link
Contributor

crizzis commented Feb 23, 2022

Hi! I'm thinking of giving it a go, could you describe the intended use case, though?

I was thinking that perhaps it would be somewhat more useful to be able to write sth like:

methods().withReturnTypeThat().hasRawType(Map.class).and().hasNthGenericParameterThat(0).hasUpperBound(String.class)

WDYT?

For this to work, I believe we would need a new TypeThat DSL (perhaps with a bridge to ClassThat of some sort, but I'm not sure).

Also, since collections are going to be common use cases, probably some shorthand methods like isCollectionOfTypeThat(), isMapOfTypeThat(), isCollectionOfRawType(String) would be in order.

@Airblader
Copy link

could you describe the intended use case, though?

This idea originated from Apache Flink where we have annotations for API stability, and we want a rule that makes sure that all types involved in a stable API are marked stable themselves. So we want to express something like

Methods
  that are declared in classes annotated with @Stable
  have parameter types that are annotated with @Stable
  and have a return type that is annotated with @Stable
  and have generic type arguments that are annotated with @Stable

@crizzis
Copy link
Contributor

crizzis commented Feb 23, 2022

@Airblader This is a very specific use case, though, and I'm not sure JavaType.getRawTypesInvolvedInSignature() is the right approach. List<MyStableType> would give you [List.class, MyStableType.class]; would you then expect List to be @Stable?

Also, ? super MyStableType is not necessarily @Stable itself.

I just feel that putting all types involved in the declaration of a given type (regardless of their role in that declaration) in one bag is not going to be very useful. I was wondering if it would work for you if you could instead write some variation of with{Return|Parameter}TypeThat().isAnnotatedWith(Stable.class).or().isCollectionOfTypeThat().isAnnotatedWith(Stable.class)?

In any case, such a rule would still require ...TypeThat().isAnnotatedWith(), which I don't believe we have at the moment.

@Airblader
Copy link

Maybe there's a misunderstanding – we don't need this to be accessible in the Lang API. Having a way to just (easily) get to this information in the Core API is totally fine with me. Apart from that I have no strong preferences.

List<MyStableType> would give you [List.class, MyStableType.class]; would you then expect List to be @Stable?

Of course not. 😬 My example above was heavily simplified, sorry. 😄

We filter the classes further before checking for the annotation, e.g. we only check classes that lie in our org.apache.flink. package, do not reside in shaded packages etc. If you want to see the actual current rule, you can find it here / here.

@codecholeric
Copy link
Collaborator Author

@crizzis Thanks for the initiative 😃 But this issue really only refers to the core API method as a first step. It does not (yet) involve any rule syntax, that would be a later step for me... I think there are valid use cases to get the set of all involved types and in the Flink case this method could be used in a custom transformer/condition with some filter to only assert app classes.
If you want to extend the rules API instead with some more powerful generics assertions that's also cool, but then please open a separate issue with your proposal 🙂

@leonardhusmann
Copy link
Contributor

Hi,
I would like to give this a go.
Just to make sure that I understood it correctly, I'll outline it in the following. Maybe you can confirm if we're on the same page?

Given a Java Class (example from above, modified)

class SomeClass {
  Map<? super String, ? extends Serializable> method(ExampleClass argument) throws MyException {..}
}

We want a new (static ?) method static Set<JavaClass> getRawTypesInvolvedInSignatures(JavaMethod method) residing in JavaType. This method should return a set containing the classes [Map.class, String.class, Serializable.class, ExampleClass.class, MyException.class]

Things I'm unsure about:

  • return type of the method: JavaClasses vs Set<JavaClass>
  • this method should work on method level (takes JavaMethods as arguments), not on Class level, or maybe both?
  • should Exceptions be included in the signature? Maybe just checked Exceptions (discard unchecked Exceptions)?

@hankem
Copy link
Member

hankem commented May 5, 2023

  1. My understanding was that for
    JavaClasses classes = new ClassFileImporter().importClasses(SomeClass.class, Map.class, String.class, Serializable.class);
    JavaMethod method = classes.get(SomeClass.class).getMethod("method", ExampleClass.class);
    JavaType returnType = method.getReturnType();
    the (instance!) method returnType.getRawTypesInvolvedInSignature() should give
    Set.of(classes.get(Map.class), classes.get(String.class), classes.get(Serializable.class))
    
  2. If method returned Map<String, List<Serializable>>, we'd get
    Set.of(classes.get(Map.class), classes.get(String.class), classes.get(List.class), classes.get(Serializable.class))
    
    right? I.e. we'd consider all generic type arguments somehow involved?
    So regarding the naming: maybe javaType.getAllInvolvedRawTypes() might be enough? I must admit that InSignature also reminded me of method signatures, for which:
    
  3. I can also imagine that an instance method on JavaCodeUnit could be useful, such that method.getAllRawTypesInvolvedInSignature() would give me the Set<JavaClasses> combined from
    • method.getReturnType().getAllInvolvedRawTypes(),
    • method.getParameterTypes().stream().map(JavaType::getAllInvolvedRawTypes), and
    • method.getTypeParameters().stream().map(JavaTypeVariable::getBounds).map(JavaType::getAllInvolvedRawTypes).
      

@leonardhusmann
Copy link
Contributor

sry for the late reply.
But thank you for the clarification. Now I got it and will start working on it 😄

leonardhusmann added a commit to leonardhusmann/ArchUnit that referenced this issue May 12, 2023
This adds a new method for JavaType which should return all involved raw types of the given instance. This commit only implements this method for ImportedParameterizedType and JavaClass.

Issue: TNG#723
Signed-off-by: Leonard Husmann <leonard.husmann@tum.de>
leonardhusmann added a commit to leonardhusmann/ArchUnit that referenced this issue May 14, 2023
Issue: TNG#723
Signed-off-by: Leonard Husmann <leonard.husmann@tum.de>
leonardhusmann added a commit to leonardhusmann/ArchUnit that referenced this issue May 29, 2023
Issue: TNG#723
Signed-off-by: Leonard Husmann <leonard.husmann@tum.de>
leonardhusmann added a commit to leonardhusmann/ArchUnit that referenced this issue May 29, 2023
Issue: TNG#723
Signed-off-by: Leonard Husmann <leonard.husmann@tum.de>
leonardhusmann added a commit to leonardhusmann/ArchUnit that referenced this issue May 29, 2023
This method returns the union of all raw types involved in the method's return type, all involved raw types of its parameters and its type parameters.

Issue: TNG#723
Signed-off-by: Leonard Husmann <leonard.husmann@tum.de>
leonardhusmann added a commit to leonardhusmann/ArchUnit that referenced this issue May 30, 2023
This commit adds test cases for getAllInvolvedRawTypes() of JavaClass, JavaTypeVariable and JavaWildcardType. This also adds a test case for getAllInvolvedRawTypesInSignature() of JavaCodeUnit.

Issue: TNG#723
Signed-off-by: Leonard Husmann <leonard.husmann@tum.de>
codecholeric pushed a commit to leonardhusmann/ArchUnit that referenced this issue Nov 5, 2023
Adds a convenience method to quickly determine all `JavaClass`es any `JavaType` depends on. For a complex type, like the parameterized type `Map<? extends Serializable, List<? super Set<String>>>`, this can otherwise be quite tedious and possibly make it necessary to traverse the whole type parameter hierarchy.

Issue: TNG#723
Signed-off-by: Leonard Husmann <leonard.husmann@tum.de>
Signed-off-by: Peter Gafert <peter.gafert@archunit.org>
codecholeric pushed a commit to leonardhusmann/ArchUnit that referenced this issue Nov 5, 2023
Convenience method to find all `JavaClass`es involved in any member's signature. For a field these are only the raw types involved in the field type, for methods and constructors it's the union of all raw types involved in parameter types, return types and type parameters.

Issue: TNG#723
Signed-off-by: Leonard Husmann <leonard.husmann@tum.de>
Signed-off-by: Peter Gafert <peter.gafert@archunit.org>
codecholeric pushed a commit to leonardhusmann/ArchUnit that referenced this issue Nov 5, 2023
Adds a convenience method to quickly determine all `JavaClass`es any `JavaType` depends on. For a complex type, like the parameterized type `Map<? extends Serializable, List<? super Set<String>>>`, this can otherwise be quite tedious and possibly make it necessary to traverse the whole type parameter hierarchy.

Issue: TNG#723
Signed-off-by: Leonard Husmann <leonard.husmann@tum.de>
Signed-off-by: Peter Gafert <peter.gafert@archunit.org>
codecholeric pushed a commit to leonardhusmann/ArchUnit that referenced this issue Nov 5, 2023
Convenience method to find all `JavaClass`es involved in any member's signature. For a field these are only the raw types involved in the field type, for methods and constructors it's the union of all raw types involved in parameter types, return types and type parameters.

Issue: TNG#723
Signed-off-by: Leonard Husmann <leonard.husmann@tum.de>
Signed-off-by: Peter Gafert <peter.gafert@archunit.org>
codecholeric added a commit that referenced this issue Nov 5, 2023
…1116)

add `{JavaType/JavaMember}.getAllInvolvedRawTypes()`

Resolves: #723
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants