Take type names from phunkie instead of keeping a second table - #17
Merged
Conversation
The REPL named types itself, and had drifted from the library it prints values
from. Both showed up in one session:
phunkie > true
$var0: Bool = true
phunkie > ImmList(true)
$var1: List<Boolean> = List(true)
getType() now delegates scalar naming to phunkie's normaliseType(), so there is
one place deciding what a type is called. The hardcoded 'Int', 'Float', 'String',
'Bool' and 'Null' literals scattered through evaluateNode go through getType()
for the same reason: each was a second opinion waiting to disagree.
get_debug_type() feeds it rather than gettype(), because gettype() returns
"NULL" uppercase and spells floats "double", neither of which normaliseType is
built around. get_debug_type() returns the same lowercase names reflection uses,
which is what that table was written for.
Phunkie's names win, so the REPL says Boolean rather than Bool. 57 assertions
across 19 feature files updated to match.
Resources now report Resource rather than Unknown. The old default arm was only
reachable for resources, and Resource is what phunkie calls them, so this closes
a drift rather than opening one.
Requires phunkie 1.4, which standardises floats on Float. Together the two make
the REPL agree with itself: Float and List<Float>, Boolean and List<Boolean>.
341, 384, 433 and 484 scenarios on 8.2 through 8.5, phpstan and cs-fixer clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The REPL named types itself, and had drifted from the library whose values it prints. Both halves of the drift were visible in a single session:
Same value, two names, in both directions.
What changed
getType()delegates scalar naming to phunkie'snormaliseType(), so one place decides what a type is called. The hardcoded'Int','Float','String','Bool'and'Null'literals scattered throughevaluateNode()now go throughgetType()for the same reason: each was a second opinion waiting to disagree.It is fed by
get_debug_type()rather thangettype(). That matters:gettype()returns"NULL"uppercase and spells floats"double", neither of whichnormaliseTypeis built around, whereasget_debug_type()returns the same lowercase names reflection uses, which is exactly what that table was written for.Phunkie's names win, so the REPL now says
Booleanrather thanBool— 57 assertions across 19 feature files updated to match.Callable,Generatorand object naming are unchanged; those are names the REPL genuinely owns, andgetObjectType()already delegates toshowType().Behaviour changes
BoolbecomesBooleaneverywhere in REPL output.Resourcerather thanUnknown. The olddefault => 'Unknown'arm was only ever reachable for resources, andResourceis what phunkie calls them, so this closes a drift rather than opening one. No feature covered resources, so it was untested either way.Requires phunkie 1.4
phunkie 1.4.0 standardises floats on
Float. Together the two make the REPL agree with itself:Verification
Behat and PHPUnit on 8.2, 8.3, 8.4 and 8.5 — 341, 384, 433 and 484 scenarios, 50 unit tests. PHPStan and PHP-CS-Fixer clean,
composer validate --strictpasses.