Pass message from methods to integer assertion inside

[alexsoft]

Previously in v1, positiveInteger was using !(\is_int($value) && $value > 0) condition. If custom message was passed, it worked correctly. Now before comparing to 0, self::integer is called, but message is not passed to that call.

In my opinion, it is wrong, that's why I created this PR.

I think the same should be done in newly introduced negativeInteger and nonNegativeInteger, so I added this there too.

Maybe, similar should be done in other methods where other methods are also called. But I decided to let that be done by maintainers. Or let me know, I can add to other methods as well.

[shadowhand]

similar should be done in other methods where other methods are also called

I agree, that was an oversight. The $message should always be passed to internal calls.

Would you please update all cases? We should also have unit testing around this.

[alexsoft]

Ok, I will add this to other methods as well. And will check tests as well.

[alexsoft]

@shadowhand so, after checking everything I have some questions 😅

1. Methods isInitialized, notInstanceOf, isInstanceOfAny, isIterable use object or isIterable assertions.
   isNotA uses objectish assertion. uniqueValues, inArray, notInArray use isArray.
   contains, notContains, notWhitespaceOnly, startsWith*, notStartsWith*, endsWith*, notEndsWith, regex, notRegex use string assertions, etc. Most of them are to make sure that different types are not passed to core functions with strict types.
   Imagine a case downstream I use contains assertion. Naturally, I would put message there something like "String does not contain dashes. Got: %s". And if actual value that is passed is not string at all, it will fail on string assertion with the message "String does not contain dashes. Got: integer". Or isInitialized with message "Property must be initialized" and it would fail because passed value is not even an object.
   What I am trying to say is that in some cases, it feels weird passing message from higher level assertion to lower level ones. (e.g. from startsWith to string).
   Contrary to this, passing message to lower level assertion string is absolutely fine in methods like ip, alpha, file, etc.

Theoretically, some specific message could be provided by library itself, like in isInstanceOf. Or pass $message ?: '...' with message provided by library itself.

What do you think is more suitable?

2. Is there any particular reason why positiveInteger, notNegativeInteger and negativeInteger call assertions with self:: while all other methods use static::?

[shadowhand]

@alexsoft ah yes, this is probably why we have not passed the custom message up through to self-assertions, the message context can get too vague or outright inaccurate.

Perhaps it would be better to return to your original (smaller) scope, and address other validations in groups as you have energy for.

[alexsoft]

@shadowhand Can I keep anything else from this PR? Out of all methods that I changed, I would definitely keep changes in positiveInteger, notNegativeInteger, negativeInteger, ip, ipv4, ipv6, email, unicodeLetters, alpha, digits, alnum, lower, upper.

And definitely reverse changes in fileExists, file, directory, readable, writable.

I am only not 100% sure about keyExists, keyNotExists. I'd keep these as well, though not sure. Check the messages in test cases to see examples.

What do you think?

[alexsoft]

And what about this?

Is there any particular reason why positiveInteger, notNegativeInteger and negativeInteger call assertions with self:: while all other methods use static::?

[shadowhand]

@alexsoft ah, regarding self:: vs static::, only static:: should be used. Any usages of self:: are an oversight.

[alexsoft]

1. I changed self to static calls in those integer assertions.
2. Rolled back passing message in mentioned methods

Please have a look, @shadowhand

[shadowhand]

Released in version 2.1.0.

[alexsoft]

Thank you for help!