Skip to content

Commit

Permalink
Added type assertions for multiple methods not covered by existing `@…
Browse files Browse the repository at this point in the history
…psalm-assert` functionality (webmozarts#160)

* Added type assertions around `Assert::isMap()` to restrict key type

* Added minimal type assertions around `Assert::stringNotEmpty()`

* Added minimal type assertions for `Assert::isArrayAccessible()`

* Minimal type assertions around `Assert::ip()` type inference

* Minimal type assertions around `Assert::ip()` type inference

* Added minimal type assertions for `Assert::email()`

* Added type assertions for `Assert::same()` types

* Corrected type declaration: `Assert::contains()` only works with strings

Note: not a BC break, since the implementation uses `\strpos()`, which
is designed to reject non-string parameters.

* Corrected type declaration: `Assert::notContains()` only works with strings

Note: not a BC break, since the implementation uses `\strpos()`, which
is designed to reject non-string parameters.
* Corrected type declaration: `Assert::notWhitespaceOnly()` only works with strings

Note: not a BC break, since the implementation uses `\preg_match()`, which
is designed to reject non-string parameters.

* Corrected type declaration: `Assert::startsWith()` only works with strings

Note: not a BC break, since the implementation uses `\strpos()`, which
is designed to reject non-string parameters.

* Added type assertions for `Assert::startsWithLetter()`

* Corrected `Assert::endsWith()` parameter type: only works with `string` values anyway

* Corrected `Assert::regex()` parameter types: only works with `string` values anyway

* Corrected `Assert::notRegex()` parameter types: only works with `string` values anyway

* Added type assertions on `Assert::unicodeLetters()`

* Added type assertions on `Assert::alpha()`, `Assert::digits()`, `Assert::alnum()`, `Assert::lower()`, `Assert::upper()`

* Corrected `Assert::length()` types - method only works with strings/integers

* Corrected `Assert::minLength()`, `Assert::maxLength()`, `Assert::lengthBetween()` type declarations

These methods only work with integers, so we should restrict their types.

* Added type assertions for `Assert::fileExists()`

* Added type assertions for `Assert::file()` and `Assert::directory()`

* Corrected type declarations on `Assert::readable()`: works only with `string` values

* Corrected type declarations on `Assert::writable()`: works only with `string` values

* Added type assertions for `Assert::subclassOf()`

* Added type assertions for `Assert::implementsInterface()`

* Corrected type declarations for `Assert::count()`

* Corrected type declarations for `Assert::minCount()`, `Assert::maxCount()` and `Assert::countBetween()`

These methods only work with countable types

* Verifying that `Assert::isList()` preserves list item types

* Adding type assertions for `Assert::isNonEmptyMap()`

* Corrected type declarations around `Assert::uuid()` - method only works with `string` parameters

* Corrected type declarations for `Assert::throws()` - the method only works with `Throwable` class strings

* Applied automated CS fixes

* Skip static analysis tests for scenarios that require an upstream release of psalm first

* PHP 5.3 COMPATIBILITY OMG NGHHHHHHHHHHHHHHHHH LOUD TEAPOT NOISES!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

* Added `@psalm-assert !empty` assertions as per newer multiple-assertion upstream support

Ref: vimeo/psalm@74de32f

* Locking dependency of `vimeo/psalm` for the `ci` directory

We don't want to run static analysis with constantly changing psalm
semantics, as that jeopardizes the stability of CI for now

* Disallow installation of `vimeo/psalm` `<3.7.3` together with `webmozart/assert`

This is required due to upstream support of multiple `@psalm-assert`
annotations. Specifically, we need support that was introduced in
commit vimeo/psalm@74de32f

* `filter_var()` accepts `string|object`, so the input is asserted to be `string|object`

An object implementing `#__toString()` suffices for `filter_var()` to
work, so we can't assume that after the assertion, the value being
asserted upon is a `string`. Instead, `string|object` is our closest
bet.

Ref: webmozarts#160 (comment)

Note: will fail until vimeo/psalm#2452 has some
sort of resolution.

* `ctype_*()` functions can operate with integer ranges `-128~256` too

These are extremely fucky interfaces, but PHP is a shitty language anyway.

We need to support an input of `string|int` when validating for the
`ctype_*()` of an input, but at least we can assert for the value to
not be `empty()` when it passes such validation.

Ref: vimeo/psalm#2452

* Bump vimeo/psalm CI dependency to try out results of vimeo/psalm#2452 discussion

* Removing `@psalm-assert string|object` from `ip`, `ipv4`, `ipv6` and `email` validators (for now)

Psalm currently *cannot* deal with union types in `@psalm-assert`, because all `@psalm-assert` expressions
are implicitly inlined by the tool as `assert(type)` language expression.

This means that `@psalm-assert string|object $foo` becomes `assert(is_string($foo) || is_object($foo))`,
which is non-sensical in contexts where `$foo` may never be a `string` or an `object`, leading to impossible
to avoid `TypeDoesNotContainType` or `RedundantCondition` reported psalm errors.

Therefore, dropping the type assertion is a simplistic (for now) solution, hoping that upstream tooling will
someday be able to deal with this sort of assertions in a more atomic manner.

Ref: vimeo/psalm#2452 (comment)
Ref: vimeo/psalm#2452 (comment)

* More precise type-assertions around `!empty` for `Assert::alpha()`

* Exclude `/ci` dir from package release (now includes a larger `composer.lock` that is otherwise downstream waste)

* Marking pure API with `@psalm-pure` for usage in pure downstream consumers

* Ensure that the lack of usage of the result of a pure operation such as `Assert::string()` will not be considered "redundant"

Ref: vimeo/psalm#2456
Ref: vimeo/psalm@4b715cd
Ref: webmozarts#160 (comment)

* Marked `Assert::reportInvalidArgument()` as `@psalm-pure`

While this method can be replaced via subclassing, adding side-effects to it would break the basic
invariants of the assertion library, specifically that it is *NOT* a hook for more business logic,
but rather just assertions, pure where possible.

* Simpler type tests for `Assert::subclassOf()`, which can lead to a union type result

* Added type restrictions on `Assert::isAOf()` API

* `Assert::same()` is not supposed to couple two types, since the types may already be the same

If the types are already the same, then the type checker would expand the assertion into a redundant
condition (unexpected)

* `Assert::isNotA()` type restrictions added

* `Assert::isAnyOf()` is pure, declared type restrictions

* `Assert::isInstanceOfAny()` is pure, declared type restrictions

* Overhauled static analysis tests, hunted down all `RedundantCondition`

All imprecise `RedundantCondition` raised by `@psalm-assert` restricting on something that may have
already been restricted have been hunted down with extensive type-specific tests.

All methods that may not necessarily be pure have also been removed from purity annotations.

* Conflict with anything but latest `vimeo/psalm`, which understands the `lowercase-string` type

Ref: https://github.com/vimeo/psalm/releases/tag/3.9.0

* `Assert::isMap()` does not imply non-empty array anymore

Ref: webmozarts#160 (comment)

* Removed integer value support from `ctype_*`-based functions

While these functions support integers, we do not necessarily intend
downstream consumers to rely on the widened type.

Ref: webmozarts#160 (comment)

* Removed exclusions from static analysis test existence checks

All functions that have `@psalm-assert` annotations now have static
analysis tests.

Ref: https://github.com/webmozart/assert/pull/160/files#r381317558

* Allow both `int` and `float` on limits for count-based assertions

Ref: webmozarts#160 (comment)

* `Assert::eq()`, `::notEq()` and `::uniqueValues()` are not pure due to `__toString`

In fact, the following explains it:

```php
'foo' == new class { public function __toString() : string {echo 'hi'; return 'bar'; }};
```

This is a fucking mess of a language, but it's what we have to deal with :-(

Ref: webmozarts#160 (comment)
Ref: webmozarts#160 (comment)
Ref: webmozarts#160 (comment)
  • Loading branch information
Ocramius authored Feb 19, 2020
1 parent aed98a4 commit efafa74
Show file tree
Hide file tree
Showing 95 changed files with 2,798 additions and 65 deletions.
3 changes: 2 additions & 1 deletion .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
/.styleci.yml export-ignore
/.travis.yml export-ignore
/appveyor.yml export-ignore
/ci export-ignore
/phpunit.xml.dist export-ignore
/tests export-ignore
/tests export-ignore
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
vendor/
composer.lock
./composer.lock
2 changes: 1 addition & 1 deletion ci/composer.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"require": {
"vimeo/psalm": "^3.8.5"
"vimeo/psalm": "^3.9.1"
}
}
Loading

0 comments on commit efafa74

Please sign in to comment.