Rename Instance to IsInstanceOf

[henriquemoody]

Not critical, but I think Instance is a bit cryptic, and doesn't really reflect the intent.

Side track here is that the best name would be InstanceOf, and the problem is that we can't rename classes like that, but we could still have a method with that name. If we change how the ValidatorFactor works to try loading a validator with a suffix Validator we could achieve that. I'm not saying we should rename all validators by adding a suffix to them, but we could also try loading a validator with that suffix.

[alganet]

I don't see much difference between Instance and IsInstanceOf. What is the confusion here?

Our naming convention for basic types has been coalesced into the ***Type suffix: IntType, StringType, ObjectType. I understand these are for primitive types, however, there is clearly a blurred line, specially in recent PHP versions (callable, iterable are polymorphic).

I don't object the change, but also, I don't take the existing one as cryptic. v::instance(Person::class) looks very legible to me.

My golden rule for renames is to look for the category instead of doing individual renames. What are the other validators that deal with type doing and how they're named?

[henriquemoody]

I would have preferred that the validator would be called instanceOf() than instance, because it's more idiomatic to how we check that in PHP. The name instance() is not as assertive and clear as instanceOf(), but we don't need to get this sorted for version 3.0.

We don't have many introspection-related validators, but I reckon if we had a subclassOf() we would not want to call it `subclass().

[alganet]

That makes a lot of sense. instanceOf (adverbial mode) is more suitable for a fluent interface. However, iterableType (substantive mode) is very similar (it belongs to type checking) and does not follow that. Why not rename both (and all other of same flavour)?. So, if we're going to argue for consistency, we need to tackle it in the whole scope (typing).

I thought a lot about this, and a part of me said "primitive types are different from OOP", but as I mentioned earlier, the PHP interpreter is blurring that line with things such as iterable and callable, which are polymorphic and non-primitive.

Do you agree with the golden rule? We can only argue about consistency if we look at similar validators. If we look at a single one, we'll be dogs chasing our own tails renaming stuff over and over.

[henriquemoody]

I can agree with the golden rule you described. One of the issues we might face in our pursuit for consistency is whether we focus on the fluent interface or on the class name itself. I think the differences are small but if changes how we think about them.

Your example about iterableType, for example, I would be down for rename all the validators.

Type related

Current	Proposed
│ ArrayType │ isArray │
│ BoolType │ isBoolean │
│ FloatType │ isFloat │
│ IntType │ isInteger │
│ StringType │ isString │
│ ObjectType │ isObject │
│ ResourceType │ isResource │
│ IterableVal │ isIterable │

Value related

Current	Proposed
│ IntVal │ isIntegral │
│ ArrayVal │ isArrayAccessible │
│ FloatVal │ isDecimal │
│ StringVal │ isStringable │

Current	Proposed
│ IterableType │ isLoopable │
│ NumericVal │ isNumeric │
│ ScalarVal │ isScalar │
│ CallableType │ isCallable │
│ NullType │ isNull │

Toggle related

Current	Proposed
│ BoolVal │ isToggle │
│ FalseVal │ isToggleOff │
│ TrueVal │ isToggleOn │

We do do some magic in the factory and create some alias, so we don't need to have those is in the chain. We could just have v::array(), for example

[alganet]

What if we made just one set of them?

v::isString();              // is_string()
v::isString(strict: false); // StringVal implementation
v::isTrue(strict: false);   // FalseVal

That solves the issue with collisions with no good name (such as isIterable/isLoopable). It's much more explicit in what it does (strict unless you tell it not to be strict).

If one of them doesn't have a non-strict version, we throw on strict: false

v::isInstance(strict: false); // throws exception, there is no non-strict instance check

One per type, no more no less:

• isTrue (non-strict: TrueVal like, strict: === true)

• isFalse (non-strict: FalseVal like, strict === false)

• isBool (non-strict: TrueVal OR FalseVal, strict: is_bool)

• isInt (non-strict: IntVal, strict: is_int)

• isFloat (non-strict: FloatVal, strict: is_float)

• isString (non-strict: StringVal, strict: is_string)

• isArray (non-strict: isInstance(Traversable) AND isInstance(Countable) AND isInstance(ArrayAccess), strict: is_array)

• isNull: (non-strict: Blank, strict: is_null)

• isNumeric: (non-strict: FloatVal OR IntVal, strict: is_numeric)

• isObject (non-strict: ERROR, strict: is_object)

• isResource (non-strict: ERROR, strict: is_resource)

• isCallable (non-strict: ERROR, strict: is_callable)

• isInstance (non-strict: ERROR, strict: instanceof)

• isCountable: (non-strict: ERROR, strict: is_countable)

• isScalar: (non-strict: ERROR, strict: is_scalar)

• isIterable: (non-strict: ERROR, strict: is_iterable)

The rule becomes simple: there is just one naming convention. You can strict: false some of them to get a more flexible version.

[henriquemoody]

Generally speaking I prefer having one validator for a single validation, because that makes the implementation a bit cleaner. This is is one of the principles behind splitting up validators in version 3.0 (key -> key, keyExists, keyOptional; between -> between, betweenExclusive).

If I think a ahead with what we could do in Respect\Assertion using @psalm-assert and @phpstan-assert, it serves as an extra argument for keeping them as individual validators. Of course, we could do something similar to #1681, but the way IDEs would respond wouldn't be the same.

[alganet]

key was a mess because its implementation is complicated. All those type validators on the other hand are one-liners. Their implementation is already as clean as you can get. They're just aliases to PHP functions. I'll buy the static analysis point, but not the "implementation complexity" one in this case.

My focus here though is less about implementation and keeping one set of names (string, array, int, iterable) and not inventing things such as isLoopable or isIntegral just for the sake of avoiding collisions. That creates confusion.

[henriquemoody]

My main point was not so much about the implementation, but about keeping each validator doing a single validation. It's less about "implementation complexity" and more about "single responsibility", I'm sorry I wasn't clear. But it's a fact that a simple function call, in this case, is cleaner than a a if/else with subsequent function calls. It's not my priority to reduce the amount of ifs, though.

I understand your point about naming, and I think it's a hard one. At least the *Type and *Val validators are a bit more clear right now, they're quite explicit, they just don't sound nice in the chain. Being more explicit would make the validators very long, like canBeUsedInAForeach or looksLikeAnIntergerEvenIfItsNotAnIntegerType. Naming is so hard...

[henriquemoody]

Maybe we could stick with the *Val, but only add them when needed.

Self-explanatory

Current	Proposed
ArrayType	isArray
BoolType	isBoolean
CallableType	isCallable
FloatType	isFloat
IntType	isInt
IterableType	isIterable
NullType	isNull
NumericVal	isNumeric
ObjectType	isObject
ResourceType	isResource
ScalarVal	isScalar
StringType	isString
	isTrue
	isFalse

Needs explanation

Current	Proposed
IntVal	isIntVal
ArrayVal	isArrayVal
FloatVal	isFloatVal
StringVal	isStringVal
IterableVal	isIterableVal
BoolVal	isBoolVal
FalseVal	isFalseVal
TrueVal	isTrueVal

[alganet]

That sounds better.

Some notes:

true and false are types in PHP now. They're also much better than writing v::isBoolean()->equals(true). I would add them to the strict set as isTrue and isFalse for completion and usability.

You missed instanceof, I think isInstance($className) works in this naming scheme. Or perhaps even isA($className) (mimicking php's own is_a function).

I think the is prefix should be used exclusively for the strict type checking validators:

• strict: isString
• non-strict: stringVal (no "is" prefix).

I'm also not sure about Val. It sounds too generic and also doesn't make it explicit that the comparison is not strict. I would rather use stringLike, intLike, floatLike (or any naming scheme that makes it obvious that it's not strict comparison).