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

Extend syntax to query different kinds of classes #245

Merged
merged 8 commits into from
Jan 10, 2020

Conversation

rweisleder
Copy link
Contributor

This PR adds the following methods to JavaClass

  • isTopLevelClass
  • isMemberClass
  • isAnonymousClass (as replacement for isAnonymous)
  • isLocalClass

This PR also adds classes().that(). and classes().should().

  • [are|be]TopLevelClasses
  • [are|be]NestedClasses
  • [are|be]MemberClasses
  • [are|be]InnerClasses
  • [are|be]AnonymousClasses
  • [are|be]LocalClasses

and their corresponding negations.

The naming is based on the Java Language Specification, chapter 8.

Resolves #207

@ghost
Copy link

ghost commented Oct 6, 2019

DeepCode's analysis on #264220 found:

  • 0 critical issues. ⚠️ 0 warnings and 1 minor issue. ✔️ 0 issues were fixed.

💬 This comment has been generated by the DeepCode bot, installed by the owner of the repository. The DeepCode bot protects your repository by detecting and commenting on security vulnerabilities or other critical issues.


☺️ If you want to provide feedback on our bot, here is how to contact us.

@codecholeric
Copy link
Collaborator

Cool, thank you so much!! 😄 I'm sorry it took me so long to get back to this, my queue is a little overloaded at the moment ☹️ Since this PR will go into my "complex review queue", thus it will also take me a little longer to completely get through with this, since on top of that queue is now #171, which is super useful, but open for a ridiculously long time... I hope I'll be through with it soon. I'm planning on finally releasing 1.0, once #171 is merged, and I'll include this one, too.
If you have any other remarks about terminology, the switch to 1.0 would be the perfect time for breaking changes 😉
Even though the biggest break should probably have been #224...

@codecholeric codecholeric added this to the 1.0.0 milestone Nov 3, 2019
rweisleder and others added 8 commits January 10, 2020 17:33
This commit adds the methods
* isTopLevelClass
* isMemberClass
* isAnonymousClass (as replacement for isAnonymous)
* isLocalClass

The JavaClassProcessor determines the kind based on byte code. The
naming is based on the Java Language Specification, chapter 8.

Signed-off-by: Roland Weisleder <[email protected]>
This commit adds classes().that(). and classes().should().
* [are|be]TopLevelClasses
* [are|be]NestedClasses
* [are|be]MemberClasses
* [are|be]InnerClasses
* [are|be]AnonymousClasses
* [are|be]LocalClasses
and their corresponding negations.

Signed-off-by: Roland Weisleder <[email protected]>
Since the fine details of the definitions of the different types of classes are probably not widely known, I've excluded some more exhaustive explanations and examples into the Javadoc.

Signed-off-by: Peter Gafert <[email protected]>
… static nested class

Since we are already testing the case of an explicitly declared static member class, we might as well add a member interface as another test input, since it must behave exactly the same with respect to the class type.

Signed-off-by: Peter Gafert <[email protected]>
In `PublicAPIRules` we can now also use the predefined `JavaClass.Predicates.ANONYMOUS_CLASSES`.

Signed-off-by: Peter Gafert <[email protected]>
These conditions actually do almost the same, `IsConditionByPredicate` simply needs to be enhanced to customize the description of a single event instead of always taking the predicate description.

Signed-off-by: Peter Gafert <[email protected]>
I think `InnerMemberClass` is more narrow than `NonStaticNestedClass`, because technically a local or anonymous class also is a non-static nested class. It might not always be relevant in the context, but `InnerMemberClass` at least gives me a quick understanding of exactly what type of class this is. Also `ClassAccessingLocalClass` is AFAIS a `ClassBeingAccessedByLocalClass` (it does not access the local class, but vice versa, and this is also correct for the test).

Signed-off-by: Peter Gafert <[email protected]>
@codecholeric
Copy link
Collaborator

Now that #171 is merged, I finally got around to it 😄 Thank you so much for your support!
Really nice and clean 👍
I added some more Javadoc, because I think there are many people out there that would not spontaneously be able to define a "nested class" vs a "member class", so I added more explanation and examples (and a link to the JLS, even though that's a little brittle because there are so many different versions).
Other than that it was really nice on my eyes to review 😉
Since you continuously contribute so much to ArchUnit, do you want to add yourself to the list of developers? (I should probably ask / mention this more often to people that contribute, but somehow always forgot 😦 This way I'm still alone in that section, even though there are quite some other people actively supporting ArchUnit)
If yes, you can create a PR and add yourself to the section in build-steps/release/publish.gradle, you'll then end up in the published POMs of the released artifacts (other than that you have hopefully recognized that I attribute every contribution in the release notes anyway 😉)
If no, that's of course also fine, I just think it's nice to see the people continuously contributing to ArchUnit in that way, instead of the artifacts looking like a one man show when in fact I often just gladly merge in supportive PRs from the community 😉

@codecholeric codecholeric merged commit 120246e into TNG:master Jan 10, 2020
@rweisleder rweisleder deleted the gh-207 branch January 10, 2020 21:52
@rweisleder
Copy link
Contributor Author

Thanks for adding the Javadoc! I wasn't sure how much I should write or if the description is enough.

@codecholeric
Copy link
Collaborator

Yeah, but in general I cannot remember when I though "that Javadoc is really too extensive" 😉 Except if it's obsolete duplication like sets the foo property. On the other hand I've often thought "processes the result, thanks for that info" 😉 Don't think it hurts to have some more docs, in the end it might save people from having to search online for the expressions...

codecholeric added a commit that referenced this pull request Feb 21, 2021
adds the following methods to JavaClass
* `isTopLevelClass()`
* `isMemberClass()`
* `isAnonymousClass()`
* `isLocalClass()`

also adds `classes().that().` and `classes().should().`
* `[are|be]TopLevelClasses()`
* `[are|be]NestedClasses()`
* `[are|be]MemberClasses()`
* `[are|be]InnerClasses()`
* `[are|be]AnonymousClasses()`
* `[are|be]LocalClasses()`

and their corresponding negations.

The naming is based on the [Java Language Specification, chapter 8](https://docs.oracle.com/javase/specs/jls/se8/html/jls-8.html).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend Classes Syntax for anonymous / inner / outer classes
2 participants