Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate Str\is_empty #52

Open
rob0rt opened this issue Jun 29, 2018 · 2 comments
Open

Deprecate Str\is_empty #52

rob0rt opened this issue Jun 29, 2018 · 2 comments

Comments

@rob0rt
Copy link

rob0rt commented Jun 29, 2018

Functions can't refine input types in Hack, meaning that if the argument to Str\is_empty is non-null, it won't get refined even if the function returns true. This means that the function is, effectively, not valuable, since checking for emptiness via === '' or a nullcheck is more explicit.

We've found this function in practice to confuse developers, who expect type refinement, so we may deprecate this function and suggest explicit equality checks instead.

@lexidor
Copy link
Contributor

lexidor commented Aug 8, 2019

Under

HipHop VM 4.18.0-dev (rel)
Compiler: 1565235181_072085492
Repo schema: 444f82394cc9ef6e84315eb3ea7dde7db022c046
$ns = nullable_string();
if ($ns is nonnull && $ns !== '') { //!Str\is_empty()
  return $ns;
}

refines like it should.

@fredemmott
Copy link
Contributor

diffs up to start migration at FB; need to figure out the best way to do this with HHAST to deal with cases like Str\is_empty($foo[$bar][$baz] ?? null)

At FB, I'm adding a PHPism_FIXME::isEmptyStringOrNull() static method and using that for the non-trivial cases

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants