webmozarts / assert

Assertions to validate method input/output with nice error messages.
MIT License
7.56k stars 145 forks source link

Addition of two new assert functions for better validation of different integer types #297

Closed Dropelikeit closed 4 months ago

Dropelikeit commented 1 year ago

To complete the integer checks, nonNegativeInteger and negativeInteger checks have been added.

This PR belongs to the ticket https://github.com/webmozarts/assert/issues/296

loevgaard commented 6 months ago

I guess you are missing a positiveInteger check?

herndlm commented 6 months ago

@loevgaard that one exists already

Dropelikeit commented 6 months ago

@loevgaard sorry, I think the naming is poorly chosen. Maybe it is wise to rename the nonNegativeInteger to unsignedInteger, because it should only be checked if the value is 0 or higher. What do you think?

loevgaard commented 6 months ago

@loevgaard that one exists already

Didn't know that :)

@loevgaard sorry, I think the naming is poorly chosen. Maybe it is wise to rename the nonNegativeInteger to unsignedInteger, because it should only be checked if the value is 0 or higher. What do you think?

I think nonNegativeInteger is the way to go because that's what's used in other places, .e.g https://psalm.dev/docs/annotating_code/type_syntax/scalar_types/

Dropelikeit commented 6 months ago

@loevgaard The psalm was the motivation to write this pull request. I think it would be great if this missing implementation could be added to make such checks so that Psalm understands the variables correctly:


class MyObject {
/**
* @psalm-param non-negative-int $age
*/
public function __construct(private readonly $age)
{
}

public static function create(int $age): self
{
      Assert::nonNegativeInteger($age);

      return new self($age);
}

/**
* @psalm-return non-negative-int
*/
public function getAge(): int
{
      return $this->age;
}

}

loevgaard commented 6 months ago

Yeah, exactly. Makes a lot of sense :)