Skip to content

Conversation

florianGla
Copy link
Contributor

@florianGla florianGla commented Oct 2, 2025

This PR adds support for these additional number annotations: @Positive, @Negative, @NonPositive, @NonNegative.
Note that it is not checked if multiple annotations contain contradicting information.

@florianGla florianGla force-pushed the CIF-1781-number-annotations branch 3 times, most recently from 19982d2 to 6b0f6bc Compare October 6, 2025 09:20
@florianGla florianGla marked this pull request as ready for review October 6, 2025 09:21
@florianGla florianGla requested review from oetr and simonresch October 6, 2025 09:21
@florianGla florianGla force-pushed the CIF-1781-number-annotations branch 2 times, most recently from f82b7be to 027e7d5 Compare October 6, 2025 14:29
Copy link
Contributor

@oetr oetr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

Since all these new annotations can be used to annotate the same type, I think we really should enforce that only one of them is allowed, because all of them essentially map to @(Float|Double)InRange equivalent.
For example, currently, this results in the domain [0:200]:

@FuzzTest
  public void fuzz_int(@Positive @InRange(min=-100, max=200) @NonNegative  int i) {

However, the Java type system does not let you add two @InRange annotations to the same type (because it is not annotated by @Repeatable).

TBH, I don't think we should be adding these annotations, because they add large complexity (especially if we want to add uniqueness enforcement for all annotation types that map to some other annotation type) with only little upside for the users (we can already specify these domains using @InRange, @FloatInRange, and @DoubleInRange). But we should discuss that.

@florianGla florianGla force-pushed the CIF-1781-number-annotations branch from 027e7d5 to ab80cc2 Compare October 8, 2025 11:53
@florianGla florianGla requested a review from oetr October 8, 2025 11:53
@florianGla florianGla force-pushed the CIF-1781-number-annotations branch 3 times, most recently from dc7d816 to 13eef21 Compare October 8, 2025 12:29
Copy link
Contributor

@oetr oetr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, with some comments!

@florianGla florianGla force-pushed the CIF-1781-number-annotations branch from 13eef21 to 106e0a3 Compare October 9, 2025 11:32
@florianGla florianGla force-pushed the CIF-1781-number-annotations branch from 106e0a3 to 6ea835a Compare October 9, 2025 12:22
@simonresch
Copy link
Contributor

simonresch commented Oct 9, 2025

An addition in the mutation framework docs could make sense for the new annotations.

@florianGla florianGla merged commit fc2854f into main Oct 9, 2025
8 checks passed
@florianGla florianGla deleted the CIF-1781-number-annotations branch October 9, 2025 13:27
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.

3 participants