-
Notifications
You must be signed in to change notification settings - Fork 299
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
Add modules syntax to rules API #1078
Merged
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
codecholeric
force-pushed
the
add-modules-syntax
branch
2 times, most recently
from
March 5, 2023 09:19
a1ee308
to
eb203f0
Compare
hankem
reviewed
Mar 11, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I didn't make it through all commits yet. I so far have just a few suggestions for minor renamings, but I thought I'd submit those already.
archunit/src/main/java/com/tngtech/archunit/library/modules/ArchModule.java
Outdated
Show resolved
Hide resolved
archunit/src/main/java/com/tngtech/archunit/library/modules/ArchModule.java
Outdated
Show resolved
Hide resolved
archunit/src/test/java/com/tngtech/archunit/library/modules/ArchModuleTest.java
Outdated
Show resolved
Hide resolved
archunit/src/main/java/com/tngtech/archunit/library/modules/ArchModules.java
Outdated
Show resolved
Hide resolved
archunit/src/main/java/com/tngtech/archunit/library/modules/ArchModules.java
Outdated
Show resolved
Hide resolved
archunit/src/test/java/com/tngtech/archunit/library/modules/ArchModulesTest.java
Show resolved
Hide resolved
archunit/src/test/java/com/tngtech/archunit/testutil/assertion/DependenciesAssertion.java
Outdated
Show resolved
Hide resolved
archunit/src/main/java/com/tngtech/archunit/library/modules/ArchModules.java
Show resolved
Hide resolved
archunit/src/test/java/com/tngtech/archunit/library/modules/ArchModulesTest.java
Show resolved
Hide resolved
archunit/src/main/java/com/tngtech/archunit/library/modules/ArchModule.java
Outdated
Show resolved
Hide resolved
codecholeric
force-pushed
the
add-modules-syntax
branch
3 times, most recently
from
March 18, 2023 20:13
e4d4c73
to
2095c3d
Compare
hankem
reviewed
Apr 7, 2023
archunit/src/main/java/com/tngtech/archunit/library/modules/ArchModules.java
Show resolved
Hide resolved
hankem
reviewed
Apr 7, 2023
archunit/src/test/java/com/tngtech/archunit/library/modules/syntax/GivenModulesTest.java
Outdated
Show resolved
Hide resolved
hankem
reviewed
Apr 7, 2023
...xample/example-junit4/src/test/java/com/tngtech/archunit/exampletest/junit4/ModulesTest.java
Outdated
Show resolved
Hide resolved
hankem
reviewed
Apr 7, 2023
archunit-integration-test/src/test/java/com/tngtech/archunit/PublicAPIRules.java
Show resolved
Hide resolved
hankem
reviewed
Apr 9, 2023
archunit/src/main/java/com/tngtech/archunit/library/cycle_detection/Cycle.java
Outdated
Show resolved
Hide resolved
hankem
reviewed
Apr 9, 2023
archunit/src/main/java/com/tngtech/archunit/library/cycle_detection/CycleDetector.java
Outdated
Show resolved
Hide resolved
hankem
reviewed
Apr 9, 2023
archunit/src/test/java/com/tngtech/archunit/library/cycle_detection/CyclesAssertion.java
Outdated
Show resolved
Hide resolved
hankem
reviewed
Apr 9, 2023
...nit/src/main/java/com/tngtech/archunit/library/cycle_detection/rules/CycleArchCondition.java
Outdated
Show resolved
Hide resolved
hankem
reviewed
Apr 9, 2023
...nit/src/main/java/com/tngtech/archunit/library/cycle_detection/rules/CycleArchCondition.java
Outdated
Show resolved
Hide resolved
hankem
reviewed
Apr 11, 2023
...unit/src/main/java/com/tngtech/archunit/library/modules/syntax/ModulesShouldConjunction.java
Outdated
Show resolved
Hide resolved
hankem
reviewed
Apr 11, 2023
...nit/src/main/java/com/tngtech/archunit/library/modules/syntax/ModulesByAnnotationShould.java
Outdated
Show resolved
Hide resolved
hankem
reviewed
Apr 11, 2023
archunit/src/test/java/com/tngtech/archunit/library/modules/syntax/ModulesShouldTest.java
Outdated
Show resolved
Hide resolved
hankem
reviewed
Apr 11, 2023
archunit/src/main/java/com/tngtech/archunit/library/modules/syntax/ModulesShould.java
Outdated
Show resolved
Hide resolved
hankem
reviewed
Apr 11, 2023
...nit/src/main/java/com/tngtech/archunit/library/modules/syntax/ModulesByAnnotationShould.java
Show resolved
Hide resolved
hankem
reviewed
Apr 11, 2023
hankem
reviewed
Apr 11, 2023
hankem
reviewed
Apr 11, 2023
archunit/src/main/java/com/tngtech/archunit/base/ArchUnitException.java
Outdated
Show resolved
Hide resolved
hankem
reviewed
Apr 11, 2023
...nit/src/main/java/com/tngtech/archunit/library/modules/syntax/ModulesByAnnotationShould.java
Show resolved
Hide resolved
hankem
reviewed
Apr 12, 2023
...nit/src/main/java/com/tngtech/archunit/library/cycle_detection/rules/CycleArchCondition.java
Outdated
Show resolved
Hide resolved
hankem
reviewed
Apr 12, 2023
...nit/src/main/java/com/tngtech/archunit/library/cycle_detection/rules/CycleArchCondition.java
Outdated
Show resolved
Hide resolved
hankem
reviewed
Apr 12, 2023
...nit/src/main/java/com/tngtech/archunit/library/modules/syntax/ModulesByAnnotationShould.java
Outdated
Show resolved
Hide resolved
hankem
reviewed
Apr 12, 2023
archunit/src/main/java/com/tngtech/archunit/library/modules/ArchModules.java
Show resolved
Hide resolved
hankem
reviewed
Apr 12, 2023
archunit/src/main/java/com/tngtech/archunit/library/modules/ArchModules.java
Outdated
Show resolved
Hide resolved
codecholeric
force-pushed
the
add-modules-syntax
branch
from
August 25, 2023 15:39
3f9b32d
to
794d4f2
Compare
This first step will allow to more easily modularize a code base by introducing the concept of an ArchUnit module. This can help in cases where other modularization techniques (e.g. Java Modules) are not available or feasible. The concept of a module borrows some common ideas from the Slices API, namely that every module will be uniquely identified by a module identifier and a module is principally a mapping `JavaClass` -> `ArchModule.Identifier`, which will partition the source space into buckets of classes having the same identifier. While principally the Slices API can already be used to write modularization rules, the mental model is a little off. From watching questions on Stackoverflow and ArchUnit issues I always got the feeling that users don't see the possibility to use the Slices API for these purposes, but more for just cutting the code base in slices and asserting freedom of cycles. While this was the original main use case, it now falls short to be noticed as solution for typical modularization problems. Signed-off-by: Peter Gafert <[email protected]>
To allow a more flexible, dynamic module structure, we support defining modules by root classes. I.e. we identify module root packages by finding classes matching a predicate and then taking the packages of these classes. Signed-off-by: Peter Gafert <[email protected]>
To enable more powerful module abstractions it makes sense to support some sort of extended meta-information. This can for example include the allowed dependencies of a module to implement a self-growing module structure without central dependency definitions. To support this we parameterize `ArchModule` by a more powerful `Descriptor` instead of just a name. This way users can supply any custom `Descriptor` if necessary and use it later within their `ArchCondition`s. Signed-off-by: Peter Gafert <[email protected]>
In cases where a certain descriptor root class of a module is used to define the module it makes sense to offer a convenient API to directly derive the module descriptor from this root class instead of having to search the root class again within all the classes contained within the module. One example is if the root class is annotated with a certain annotation containing the module meta-information. In this case we need the root class again to derive the module descriptor. Signed-off-by: Peter Gafert <[email protected]>
This gives a convenient API to add a custom annotation with module meta-data to respective root classes and automatically derive modules from this. This way, we now also support using an annotated `package-info` to define modules in a very concise way: ``` @MyModule(name = "Order", allowedDependencies = {"Customer", "Shipping"}) package com.acme.order ``` Signed-off-by: Peter Gafert <[email protected]>
Signed-off-by: Peter Gafert <[email protected]>
To integrate our new `ArchModules` infrastructure in a consistent way, we have to offer it as a rule definition, similar to the `slices` API. For now this API is quite rudimentary, offering little convenience to define typical rules (e.g. allowed dependencies). Signed-off-by: Peter Gafert <[email protected]>
The naming was a little confusing, i.e. that a set of `SpecificParameterProvider` would be called `singleParameterProviders`, but then there is also a class `SingleParametersProvider`, etc. The new names should reflect the role a little better. I also moved the `SingleParameterProvider` toplevel, so we can adjust it per test level in the next step. Signed-off-by: Peter Gafert <[email protected]>
... so we can adjust it on a per-test level in the next step. Signed-off-by: Peter Gafert <[email protected]>
This should be one of the core use cases of the modules API, so we should make it easy to check for illegal module dependencies. It seems like the most convenient API we can reach is one where the user can simply provide a predicate determining which `ModuleDependency` is allowed and we can then check all module dependencies against this predicate and report violations. As with the PlantUML rule it's safe to assume that users will have the requirement to specify how to handle dependencies (e.g. dependencies to core Java or library classes) and to ignore specific class dependencies when checking the rule. Signed-off-by: Peter Gafert <[email protected]>
This is a simplification that allows to directly write down which modules may depend on which other modules by name. I picked the module name for this definition and not the identifier, because if the two differ I think the name is nicer to read (and also unique as the identifier). Signed-off-by: Peter Gafert <[email protected]>
We will add ArchUnit's cycle detection algorithm to the public API, since it is a) also useful to users (one use case was e.g. to detect cycles within static initializers) and b) we will reuse it from the new modules API. We don't make it public in this step, because we want to move this out into a separate package while making clear which changes are related to moving the files and which changes are related to refactoring the existing classes to allow a clearer API. The cycle detection algorithm so far was designed very closely along the requirements of `SliceCycleArchCondition`. When we open this as public API we redesign it and for example don't use the concept of `ATTACHMENT`, but simply allow users to pass their own implementation of `Edge`. Signed-off-by: Peter Gafert <[email protected]>
Now that the new cycle detection interface has been designed we can relocate all cycle detection related classes to a new package `cycle_detection` and expose the new public API to users. Signed-off-by: Peter Gafert <[email protected]>
This class adds no additional behavior to a simple `Set<SliceDependency>` and only encapsulates the mapping. By that it seems more confusing than useful, since we can just write the mapping inline, making it more clear that we're indeed only interested in a simple `Set<SliceDependency>` and nothing more. Signed-off-by: Peter Gafert <[email protected]>
We know that a cycle always has edges, so finding the edge with the lexicographically minimum description will never yield absent. But we should make this explicit and ignore the warning. Signed-off-by: Peter Gafert <[email protected]>
Just naming this `DescribedPredicate` `predicate` makes it really hard to understand what it's used for down the line. Signed-off-by: Peter Gafert <[email protected]>
This rule doesn't use much that is `Slice`-specific. We can make this easily more generic by replacing `Slice` with a generic `COMPONENT` if we parameterize * how to retrieve the classes contained within a `COMPONENT` * how to retrieve the description of a `COMPONENT` * how to retrieve the outgoing class dependencies of a `COMPONENT` We now make this new generic `CycleArchCondition` public API so users can use it for their own components, and we can reuse it for the new modules API. Signed-off-by: Peter Gafert <[email protected]>
Signed-off-by: Peter Gafert <[email protected]>
This is another common requirement when testing for modularization, that no matter the visibility of individual classes the API of each module is a dedicated subset of all (compile) accessible classes. The reason is that the Java visibility modifiers are typically not powerful enough to model such a module API, unless all classes are thrown together in a single package (where we can use package-visibility), which is not feasible for more complex modules. Signed-off-by: Peter Gafert <[email protected]>
... as a more convenient version of `onlyDependOnEachOtherThroughClassesThat(predicate)` that allows to specify the allowed dependency targets in a fluent way. Unfortunately, this means we either have to open up `ClassesThatInternal` to be used from this other place, or we have to duplicate it. Since it would also implement the same interface `ClassesThat` and thus would have to change in two places every time we adjust that interface, I decided to make it `@Internal` and public. It is not pretty but likely still better than duplication, and since users don't get any guarantees on `@Internal` classes it doesn't impair maintainability. Signed-off-by: Peter Gafert <[email protected]>
While `orShould()` will likely not make any sense for module rules (in particular because we don't support `noModules()`), `andShould()` might be some nice convenience to e.g. combine checking for allowed dependencies and allowed API in one rule. We add `andShould()` and the generic counterpart `andShould(condition)` for now and see if any user ever really misses `orShould()`. Note that we can always only return `ArchRule` from the generic `{and}Should(condition)`, because `ignoreDependency` of `ModulesRule` doesn't make any sense for rules with custom conditions. I.e. there is no way to generally ignore dependencies in this case, because users can freely handle all dependencies within their custom condition. Principally, we could split it into `ModulesRule` and `ModulesRuleCapableOfIgnoringDependencies`, but for now I consider this too much complexity for an uncertain benefit. Because in the end, if users use custom conditions anyway, they might as well `and` or `or` them together directly, like `should(conditionOne.and(conditionTwo))`, so `andShould(..)` doesn't really bring much value here. Signed-off-by: Peter Gafert <[email protected]>
We can support this as a standard case to make it really simple to define an annotation to declare a module including dependencies. Signed-off-by: Peter Gafert <[email protected]>
This makes the use case to expose custom packages for each module via the module descriptor annotation very simple. I.e. users can simply declare a property name of a `String[]` property on the descriptor annotation, and we will treat all the values in this array as package identifiers and only allow access to those classes within the module that match one of the package identifiers. Signed-off-by: Peter Gafert <[email protected]>
Signed-off-by: Peter Gafert <[email protected]>
This gives us the chance to easier break it if necessary, in case we find some really bad oversight somewhere. Signed-off-by: Peter Gafert <[email protected]>
This gives us the chance to easier break it if necessary, in case we find some really bad oversight somewhere. Signed-off-by: Peter Gafert <[email protected]>
codecholeric
force-pushed
the
add-modules-syntax
branch
from
August 25, 2023 15:55
794d4f2
to
2cdb316
Compare
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 PR will add a convenient fluent rules API to partition
JavaClasses
into (architecture) modules and assert conditions on this module structure. The basic syntax has the formWhile some concepts are quite similar to the
Slices
API I've noticed over the years that most people don't see the possibilities to use theSlices
API to check modularization properties. Thus, I decided to not make the existing API more powerful, but instead create this new API which offers most features of theSlices
API (e.g. create from package identifiers, check for cycles) but also more powerful / concise possibilities to introduce modularization into a code base (e.g. by an annotatedpackage-info
) and carry meta-information like allowed dependencies from such an annotated object into the modules rule.