Add JavaClass type parameters to dependencies #484
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hankem
approved these changes
Dec 7, 2020
archunit/src/main/java/com/tngtech/archunit/core/domain/JavaTypeVariable.java
Show resolved
Hide resolved
archunit/src/test/java/com/tngtech/archunit/core/domain/DependencyTest.java
Show resolved
Hide resolved
archunit/src/test/java/com/tngtech/archunit/core/domain/JavaTypeVariableTest.java
Outdated
Show resolved
Hide resolved
archunit/src/test/java/com/tngtech/archunit/testutil/assertion/DependenciesAssertion.java
Show resolved
Hide resolved
codecholeric
force-pushed
the
add-javaclass-type-parameters-to-dependencies
branch
from
82f90d5 to
226f372
Compare
The existence of this method is not immediately clear for users not that familiar with the Java Reflection API. I have thus added a little explanation that this method is simply an alias to make the API more familiar to users of the Reflection API. Since type variables cannot have any lower bounds, the qualification "upper bound" is technically unnecessary (which is probably the motivation of the naming within the Java Reflection API). To me it still seemed practical to have one common interface `HasUpperBounds` to treat type variables and wildcards uniformly, thus `JavaTypeVariable` has as well `getBounds()` as `getUpperBounds()`. Signed-off-by: Peter Gafert <[email protected]>
Since the two OR-patterns are uniform it is likely (arguably?) easier to read and understand like this, even though we need to group the OR statements with brackets. Signed-off-by: Peter Gafert <[email protected]>
This is consistent to how other objects (like throws declaration) are modelled and also how the Reflection API is modelled. Furthermore it is useful to make assertions (or register dependencies for the declaring type in our case) about a type variable without also holding a reference to the declaring type. At the moment the 'owner' of a `JavaTypeVariable` will always be a `JavaClass`, since we only support generic type parameters of classes so far. We will need this for generic type parameters of methods or constructors as well though, so I decided to immediately go for the generic version, analogously to the Reflection API. Unfortunately I had to generify `JavaWildcardTypeBuilder` as well, even though `JavaWildcardType` (analogously to the Reflection API) does not offer a generic declaration. The reason is that on build within a reference creation process (i.e. we have a wildcard like `? extends T` and `T` is a type parameter declared somewhere else in the context), we might need to create a new type variable on the fly, if the referenced type variable is not present in the import context. But then we need to know the owner, so we need to pass this through all the builders, including `JavaWildcardTypeBuilder`. It is not pretty, but at least it is importer implementation detail and not part of the domain model. Signed-off-by: Peter Gafert <[email protected]>
Now that we have the full type parameter info for any `JavaClass` (e.g. `class Foo<T extends Set<? super Bar>>`), we can add type parameter dependencies to `JavaClass.getDirectDependenciesFromSelf()` and `JavaClass.getDirectDependenciesToSelf()`. In particular any other class that appears within the type signature should count as a dependency of the class, e.g. in the former example `Foo` should report type parameter dependencies on `Set` and `Bar`. I also did consider to add some sort of visitor API to the type signature, but in the end I went with a simple instanceof chain in this one place. I could not come up with a generic, yet easy and use-/meaningful visitor interface that I would consider a good addition to the public API. Since within ArchUnit there is also only one use case so far, I decided that this part of the domain model will likely be stable enough to not cause any maintainability issues (after all I can't think of any other `JavaType` to be added in the near future and we have the Reflection API to peek into which sorts of `JavaType` came up in the wider context over the last decades). Signed-off-by: Peter Gafert <[email protected]>
codecholeric
force-pushed
the
add-javaclass-type-parameters-to-dependencies
branch
from
226f372 to
301ccd8
Compare
codecholeric
deleted the
add-javaclass-type-parameters-to-dependencies
branch
codecholeric
added a commit
that referenced
this pull request
Feb 21, 2021
This is the next step of #398. It will make the imported type parameters of `JavaClass` widely useful by adding type parameter dependencies (e.g. `Bar` for `class Foo<T extends Bar>`) to the `JavaClass.directDependencies{From/To}Self`. This way type parameter dependencies will now cause violations in all dependency based `ArchRules` like `LayeredArchitecture` or any `classes()...dependOn...()` fluent API methods.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the next step of #398. It will make the imported type parameters of
JavaClasswidely useful by adding type parameter dependencies (e.g.Barforclass Foo<T extends Bar>) to theJavaClass.directDependencies{From/To}Self.This way type parameter dependencies will now cause violations in all dependency based
ArchRuleslikeLayeredArchitectureor anyclasses()...dependOn...()fluent API methods.