Support .dependOnClassesThat() for Methods

[u3r]

I'd like to write a test for <no non-private Methods should "depend" on a specific (dangerous) class>

After writing lots of DescribedPredicates I noticed that noClasses().should().dependOnClassesThat(). ... already does NEARLY what I need but is too all-encompassing.

Internally everything is there Dependency.tryCreateFromReturnType and friends are all there - just not public api.

Therefore I'd like to propose exposing something like

   noMethods().should().dependOnArgumentsThat(). ... // just the arguments
              .orShould().dependOnReturnTypesThat()...  // just the return type
              .orShould().accessClassesThat() // just access within the method
              .orShould().dependOnClassesThat() // arguments + return type + access within the method

[codecholeric]

I'm open for the suggestion, but I'd also be interested what "too all-encompassing" means in your context :thinking:

[u3r]

Thanks @codecholeric ,

let me try to lay this out then:

I have some difficult-to-work-with 3rd party class, I want to contain the use of. So I have 2 classes that can work correctly with that class but should only use that class internally. So what I want to say is essentially

ArchRule noReturnOfRepresentation = noMethods()
         .that( not( HasModifiers.Predicates.modifier( JavaModifier.PRIVATE ) ) )
         .and().areNotDeclaredIn( AbstractWhitelistedClass.class )
         
         .dependOnArgumentsThat(JavaClass.Predicates.assignableTo(BadClass.class)) // <-- from proposal
         .orShould().dependOnReturnTypesThat(JavaClass.Predicates.assignableTo(BadClass.class))  // <-- from proposal

         .because( "Interfacing with BadClass requires to know the secret internal naming scheme "
               + "and this should be restricted to WhitelistedClass1 + 2 and their parents." ); 

If I'm in the same codebase, I can write

@ArchTest
   ArchRule noUsageOfRoleRepresentation = noClasses()
         .that(
               not( JavaClass.Predicates.assignableTo( WhitelistedClass1.class ) )
                     .and( not( JavaClass.Predicates.assignableTo( WhitelistedClass2.class ) ) )
                     .and( not( JavaClass.Predicates.assignableTo( AbstractWhitelistedClass.class ) ) )
         )
         .should().dependOnClassesThat().areAssignableTo( BadClass.class )

         .because( "Interfacing with BadClass requires to know the secret internal naming scheme "
               + "and this should be restricted to WhitelistedClass1 + 2 and their parents." );

But if I want to verify a public API for some library, I can not use that snipped, as any public method in WhitelistedClass1 can still reference the bad type as long as it is not used anywhere in that library.

Sidenote: Anything that could be done to JavaType to make working with generic types (read: create predicates or conditions for them) easier would also be much appreciated. But I think these methods would cover > 80% of usecases.

Addendum: by "too all encompassing" I mean: Using noClasses().should().dependOnClassesThat(). i need to make an exception for the whitelisted classes - thereby NOT catching public API provided by them.

[codecholeric]

Ah, I think I understand. But even if WhiteListedClass1 would expose BadClass as public API no other class could use it or it would be caught by the rule again, no? Because using that return type or method parameter would then cause a violation of that other class depending on it?

[u3r]

That is technically correct but a) I'd like to check that my LIBRARY does not expose this class in it's api (not requiring the caller to write/import archunit tests) And YES I am aware that the caller STILL could violate this due to transitive dependency import.... ;-) -- I just like MY code to be clean.

b) let me extend the feature request to also support predicates of the form noMethods().that(JavaMethods.Predicates.HasArgumentsThat(<subPredicate using generic types>)

Or should b) be a separate feature?

[leonardhusmann]

Hi, i would like to work on this issue :smile:

Let me rephrase how I understood this issue to make sure that we're on the same page:

a) You want to check for certain methods (matched by your predicate) that these don't expose or interact with a specific class (e.g. BadClass) via their 1) arguments (dependOnArgumentsThat()), 2) return types (dependOnReturnTypesThat()), 3) access to the BadClass in its method body (accessClassesThat()) and 4) the combination of all of them (dependOnClassesThat()). This would allow for more fine granular rules that only check the defined constraints (e.g. only check for return types) and for rules on method level instead of class level. Correct?

b) I'm not sure if I understood this correctly. Is this covered by noMethods().that(HasReturnType.Predicates.rawReturnTypes(...)? e.g.

@ArchRule
static final ArchRule methodsShouldNotExposeBadParameters = noMethods()
    .that(HasParameterTypes.Predicates.rawParameterTypes(BadClass.class))
    .should().beFinal();

Or what would you expect HasArgumentsThat to do differently? Is it that your proposed predicate matches if any of the included arguments is of the specified type?

[u3r]

Having looked at the Dependency type a little more, I wonder, if an API like

noMethods().should().dependOnArgumentsThat(DescribedPredicate<Dependency>...). ... // just the arguments
              .orShould().dependOnReturnTypesThat(DescribedPredicate<Dependency>...)...  // just the return type
              .orShould().accessClassesThat(DescribedPredicate<Dependency>...) // just access within the method
              .orShould().dependOnClassesThat(DescribedPredicate<Dependency>...) // arguments + return type + access within the method

because this way the predicate has origin and target available, which is often needed to say something:

"no methods should depend on Classes that are named "foo" and not part of its own class hierarchy"