Treat empty unit string as dimensionless in conversions#34
Merged
Conversation
Unit#in!("") (and in!(" "), and the same via #in) crashed with
NoMethodError: undefined method 'each' for nil instead of behaving like
the dimensionless unit. Construction already treats an empty unit string
as dimensionless (Unit(1, "") => Unit("1")), so conversion was
inconsistent with construction for the same input.
System#parse_unit returned result.last, which is nil for an empty or
whitespace-only expression (the tokenizer yields no tokens). validate_unit
then called nil.each. Make parse_unit total by returning [] -- the
dimensionless unit shape every consumer (to_unit, the constructor,
validate_unit) already accepts -- instead of nil.
4692ae7 to
6b12583
Compare
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.
Summary
Unit#in!("")(andin!(" "), and the same via#in) crashed withNoMethodError: undefined method 'each' for nilinstead of behaving like thedimensionless unit. Construction already treats an empty unit string as
dimensionless (
Unit(1, "") # => Unit("1")), so conversion was inconsistentwith construction for the same input.
A
rescue TypeErroraround a conversion used as a dimensional guard thereforedid not catch the empty-string case — it slipped past the coercion guard and
crashed deeper.
Root cause
Unit::System#parse_unitreturnedresult.last. For an empty orwhitespace-only expression the tokenizer yields no tokens, so
resultis[]and
result.lastisnil.validate_unitthen callednil.each.Fix
Make
parse_unittotal: an empty / whitespace-only expression yields thedimensionless unit definition
[](the shape every consumer —to_unit, theconstructor,
validate_unit— already accepts) rather thannil. So:Test plan
bundle exec rake— both phases green (160 no-DSL + 162 DSL examples, 0 failures)#in("")/#in(" ")and#in!("")/#in!(" ")returningthe dimensionless unit, a clean
TypeErrorfrom#in!for an incompatible unit,and
parse_unit("")/parse_unit(" ")returning[]