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

Layered architecture ignores violations in type parameters #144

Closed
stefanroeck opened this issue Feb 7, 2019 · 2 comments · Fixed by #640
Closed

Layered architecture ignores violations in type parameters #144

stefanroeck opened this issue Feb 7, 2019 · 2 comments · Fixed by #640

Comments

@stefanroeck
Copy link

Given:

  • A layered architecture with 2 packages "presentation" and "shared"
  • presentation might access shared but not vice versa
  • class in shared references class in presentation as type parameter of java.util.list

Expected:

  • Error because of layered architecture violation

Actual:

  • No error.
package com.archunitbug.presentation;
public class ExceptionDetails {}
package com.archunitbug.shared;
import java.util.List;
import com.archunitbug.presentation.ExceptionDetails;

public class ExceptionWithBackReference extends RuntimeException {
    private List<ExceptionDetails> details;
    public ExceptionWithBackReference(List<ExceptionDetails> details) {
        this.details = details;
    }
}

Test:

import static com.tngtech.archunit.library.Architectures.layeredArchitecture;
import org.junit.Test;
import com.tngtech.archunit.core.domain.JavaClasses;
import com.tngtech.archunit.core.importer.ClassFileImporter;
import com.tngtech.archunit.core.importer.ImportOption;
import com.tngtech.archunit.core.importer.ImportOptions;
import com.tngtech.archunit.library.Architectures.LayeredArchitecture;
public class ArchunitLayersException {
    @Test
    public void enforceAllNcrLayersTopDown() {
        JavaClasses classes = new ClassFileImporter(new ImportOptions().with(ImportOption.Predefined.DONT_INCLUDE_TESTS)).importPackages("com.archunitbug");
        LayeredArchitecture rule = layeredArchitecture() //
                .layer("presentation").definedBy("com.archunitbug.presentation") //
                .layer("shared").definedBy("com.archunitbug.shared") //
                .whereLayer("presentation").mayNotBeAccessedByAnyLayer()//
                .whereLayer("shared").mayOnlyBeAccessedByLayers("presentation");

        rule.check(classes);
    }
}
@hankem
Copy link
Member

hankem commented Feb 7, 2019

I believe that ArchUnit (in its current version 0.9.3) does not yet handle type parameters, cf. #115.

If ExceptionWithBackReference had an ExceptionDetails field instead of List<ExceptionDetails>, your LayeredArchitecture test would fail as expected.

@codecholeric
Copy link
Collaborator

Yes, this is correct, the current version cannot detect this, since ArchUnit only sees a raw reference to java.util.List. As soon as #115 is done, dependencies of a JavaClass can be extended with type parameters and this violation will then be detected.

@codecholeric codecholeric added this to Backlog in ArchUnit Apr 9, 2020
@codecholeric codecholeric self-assigned this Apr 9, 2020
@codecholeric codecholeric added this to the 1.0.0 milestone Apr 9, 2020
ArchUnit automation moved this from Backlog to Done Aug 20, 2021
codecholeric added a commit that referenced this issue Aug 20, 2021
As last step to fully support Generics within ArchUnit this adds support for generic method/constructor parameter types. In particular

* `JavaMethod.getParameterTypes()` now returns a `List` of `JavaParameterizedType` for parameterized generic method/constructor parameter types and the raw types otherwise
* all type arguments of generic method parameter types are added to `JavaClass.directDependencies{From/To}Self`

Example: ArchUnit would now detect `String` and `File` as `Dependency` of a method declaration like

```
class SomeClass {
    void someMethod(Set<List<? super String>> first, Map<?, ? extends File> second) {
    }
}
```

Note that analogously to the Java Reflection API `JavaConstructor.getParameterTypes()` and `JavaConstructor.getRawParameterTypes()` do not behave exactly the same for inner classes. While `JavaConstructor.getRawParameterTypes()` contains the enclosing class as first parameter type, `JavaConstructor.getParameterTypes()` does not contain it if the constructor has generic parameters. On the other hand it just returns the same list of raw parameter types if the constructor is non-generic. This might surprise some users, but I decided to stick to the same behavior as the Reflection API, because this has always been the strategy and the solution can never truly satisfy all assumptions a user might have.

Resolves: #115
Resolves: #144
Resolves: #440
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
ArchUnit
  
Done
Development

Successfully merging a pull request may close this issue.

3 participants