beberlei / assert

Thin assertion library for use in libraries and business-model
Other
2.41k stars 188 forks source link

objectOrClass, propertyExists, and propertiesExist #218

Closed abacaphiliac closed 7 years ago

abacaphiliac commented 7 years ago
rquadling commented 7 years ago

Hmm. Getting the same issue locally with this branch.

abacaphiliac commented 7 years ago

yeah, i thought maybe we could adjust the lint config... but that seems like it could have some side effects and let other things through, so i don't like it. seemed easier to use the underlying magic method, or i guess i could duplicate the assertion logic in a loop (would perform faster anyway).

any other ideas?

rquadling commented 7 years ago

I'd use the static:: call anyway. I'm seeing if I can patch PHPStan to get this to work. @method parsing is fairly new, so I suspect the pattern of including static hasn't been dealt with.

rquadling commented 7 years ago

Waiting for a fix for https://github.com/phpstan/phpstan/pull/183

rquadling commented 7 years ago

Seems I got there on my own. Tests now pass appropriately.

Can you try adding this repo to your composer.json

  "repositories": [
    {
      "type": "vcs",
      "url": "https://github.com/rquadling/phpstan"
    }
  ]

and change the version being loaded to ...

    "phpstan/phpstan": "dev-master",

Hopefully that will prove the fix is operational (I think it is).

If so, we can get Travis to do its testing.

If all passes, then I can merge this in and wait for PHPStan to catch up.

abacaphiliac commented 7 years ago

on it.

rquadling commented 7 years ago

Cheers, if this all works, revert the repo changes and push that to github for this PR.

abacaphiliac commented 7 years ago

cool, nice fix @rquadling . i added your fork as the phpstan repo in the travis config where dev-master is installed for linting. what do you think? build is fixed : )

rquadling commented 7 years ago

Issue with PHPStan has been resolved. Waiting on a release. So can you revert the changes to .travis and composer.json and we'll wait for the release.

abacaphiliac commented 7 years ago

@rquadling removed reference to your fork, but i'm still pointing to master so that the build will pass. do you know when we might expect a release? would you prefer me to revert back to stable as well and have a failing build until that time?

rquadling commented 7 years ago

As this is only a bug in a third party app and has no bearing upon this package other than the false negative, I'm happy to revert back to stable.

abacaphiliac commented 7 years ago

@rquadling reverted travis config, and now the php70 and php71 builds fail as expected.

rquadling commented 7 years ago

I've proposed a change to PSR-5 (https://github.com/php-fig/fig-standards/pull/899). If you'd care to comment, correct, enhance, then that would be appreciated.

abacaphiliac commented 7 years ago

@rquadling thank you for the invitation. i think your proposal is great. i can't think of anything to enhance or correct. what's the protocol for singing praise in fig?

rquadling commented 7 years ago

I've no idea. This is my first one I've interacted with. I think just a +1 may be good, just to show interest and to be able to be kept aware of any changes.

rquadling commented 7 years ago

Added the appropriate error suppression entry for PHPStan.

abacaphiliac commented 7 years ago

@rquadling cool, thank you : )

rquadling commented 7 years ago

When PHPStan supports @method as we need it to, the error suppression will fail as the error wasn't generated. Clever as you don't end up with a load of suppressions, permanently hiding actual errors.