refactored `resolve` method logic to not use `inet_pton`

[austinheap]

inet_pton throws warnings -- which while prior to this PR were muted with by @ -- are avoidable.

[kelunik]

What's the benefit here?

[austinheap]

@kelunik Suppressing an error message with @ doesn't mean the error didn't occur, just that the PHP runtime won't display a message about it. Custom error handler (set with set_error_handler) are still executed and still view it as an error though, so if you're using custom error handlers (say xdebug, sentry, etc) this single line of code gets super loud with errors.

It didn't seem to make sense to trigger an apps custom error handlers just to detect if an IP was used as an argument and should be returned immediately as a Record as long as the type restriction matched.

TLDR; This PR accomplishes the same thing, but without suppressed errors which other packages still view as actual errors.

(Thanks for @amphp btw!)

[kelunik]

@austinheap The thing is, PHP is stupid. It outputs errors where it shouldn't. If your custom error handler doesn't handle suppressed errors correctly, you'll get other unwanted error messages, because e.g. streams with a full buffer will result in warnings, even if the code is correct and there isn't anything that can be improved.

I agree, however, that there might be a use case for an error handler that doesn't obey the error suppression, because it might be used to hunt down bugs where too many errors are suppressed.

@trowski Thoughts?

(Glad you like it. :-))

[austinheap]

@kelunik Totally agree re: PHP being stupid with warnings like this and from streams, like you mention. A project I'm working on does lots of queries using this package and the xdebug output wasn't useable due to the amount of warning spam related to this suppressed inet_pton call.

There might also be an argument that triggering the built-in/custom error handlers is more CPU expensive than using the built-in filter_var functionality upfront, but that wasn't my goal.

[DaveRandom]

Can't we just fix this in php-src?

The errors emitted by inet_pton()/inet_ntop() are entirely useless, to the extent that it might even be permissible to merge this down to 7.1 (I will ask Joe).

[kelunik]

That'd be my preference here, too. Please, do.

[austinheap]

@DaveRandom super cool idea, thanks so much for pushing this!

[trowski]

Should we merge this anyway? Yes, inet_pton() should be fixed in php-src, but for now this PR addresses a problem people are having now.

[kelunik]

@trowski I guess we can do that, yes.

@austinheap Please add ext-filter to the composer.json requirements.

[austinheap]

@kelunik Done.

[kelunik]

Thanks!