Details
Details
- Reviewers
valerio.bozzolan - Group Reviewers
O1: Blessed Committers - Maniphest Tasks
- T15316: PHUITagView and similar: it's unclear whenever 'href' should be just a string
- Commits
- rARC6c6f47bf9cf6: (arc) Unify type-checking for `setHref()` type methods
Run this script:
#!/usr/bin/env php <?php require_once './arcanist/scripts/__init_script__.php'; $valid_inputs = array( 'foo', null, new PhutilURI('http://0@domain.com/'), ); $invalid_inputs = array( 9, new Filesystem(), array(), ); echo "Valid inputs: \n"; foreach ($valid_inputs as $v) { echo("Checking ".gettype($v)." \n"); PhutilURI::checkHrefType($v); } echo "\nError inputs: \n"; foreach ($invalid_inputs as $v) { echo("Checking ".gettype($v)." \n"); PhutilURI::checkHrefType($v); } echo "\n";
Diff Detail
Diff Detail
- Repository
- rARC Arcanist
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
src/parser/PhutilURI.php | ||
---|---|---|
573 | I like that, premising that we can replace gettype($value) == 'string' with is_string($value) is_string(false) = bool(false) is_string(true) = bool(false) is_string(NULL) = bool(false) is_string('abc') = bool(true) is_string('23') = bool(true) is_string(23) = bool(false) is_string('23.5') = bool(true) is_string(23.5) = bool(false) is_string('') = bool(true) is_string(' ') = bool(true) is_string('0') = bool(true) is_string(0) = bool(false) https://www.php.net/manual/en/function.is-string.php Also Using is_string() on an object will always return false (even with __toString()). The last thing is not officially documented thought, so, probably the gettype() check is better |
src/parser/PhutilURI.php | ||
---|---|---|
573 | I did a grep, and I can't find any place where we do gettype() == 'string', but we do have a bunch of is_string(), so is_string is probably better. |