ArchUnit
ArchUnit copied to clipboard
Support an easy way to obtain all involved `JavaClasses` of a generic signature
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?
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.
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 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.
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 expectListto 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.
@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:
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:
JavaClassesvsSet<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)?
- My understanding was that for
the (instance!) methodJavaClasses 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();returnType.getRawTypesInvolvedInSignature()should giveSet.of(classes.get(Map.class), classes.get(String.class), classes.get(Serializable.class)) - If
methodreturnedMap<String, List<Serializable>>, we'd get
right? I.e. we'd consider all generic type arguments somehow involved? So regarding the naming: maybeSet.of(classes.get(Map.class), classes.get(String.class), classes.get(List.class), classes.get(Serializable.class))javaType.getAllInvolvedRawTypes()might be enough? I must admit thatInSignaturealso reminded me of method signatures, for which: - I can also imagine that an instance method on
JavaCodeUnitcould be useful, such thatmethod.getAllRawTypesInvolvedInSignature()would give me theSet<JavaClasses>combined frommethod.getReturnType().getAllInvolvedRawTypes(),method.getParameterTypes().stream().map(JavaType::getAllInvolvedRawTypes), andmethod.getTypeParameters().stream().map(JavaTypeVariable::getBounds).map(JavaType::getAllInvolvedRawTypes).
sry for the late reply. But thank you for the clarification. Now I got it and will start working on it :smile: