ArchUnit icon indicating copy to clipboard operation
ArchUnit copied to clipboard

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

Open codecholeric opened this issue 4 years ago • 5 comments

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?

codecholeric avatar Nov 21 '21 12:11 codecholeric

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.

crizzis avatar Feb 23 '22 11:02 crizzis

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

Airblader avatar Feb 23 '22 11:02 Airblader

@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.

crizzis avatar Feb 23 '22 12:02 crizzis

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. :grimacing: My example above was heavily simplified, sorry. :smile:

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.

Airblader avatar Feb 23 '22 16:02 Airblader

@crizzis Thanks for the initiative :smiley: 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 :slightly_smiling_face:

codecholeric avatar Feb 23 '22 16:02 codecholeric

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)?

leonardhusmann avatar Apr 30 '23 18:04 leonardhusmann

  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). 

hankem avatar May 05 '23 07:05 hankem

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

leonardhusmann avatar May 09 '23 06:05 leonardhusmann