-
Notifications
You must be signed in to change notification settings - Fork 302
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
Support .dependOnClassesThat() for Methods #1060
Comments
I'm open for the suggestion, but I'd also be interested what "too all-encompassing" means in your context 🤔 |
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. 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 Sidenote: Anything that could be done to Addendum: by "too all encompassing" I mean: |
Ah, I think I understand. But even if |
That is technically correct but b) let me extend the feature request to also support predicates of the form Or should b) be a separate feature? |
Hi, i would like to work on this issue 😄 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. b) I'm not sure if I understood this correctly. Is this covered by @ArchRule
static final ArchRule methodsShouldNotExposeBadParameters = noMethods()
.that(HasParameterTypes.Predicates.rawParameterTypes(BadClass.class))
.should().beFinal(); Or what would you expect |
Issue: TNG#1060 Signed-off-by: Leonard Husmann <[email protected]>
Having looked at the 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" |
I'd like to write a test for
<no non-private Methods should "depend" on a specific (dangerous) class>
After writing lots of
DescribedPredicate
s I noticed thatnoClasses().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
The text was updated successfully, but these errors were encountered: