The decimal's range should not be parsed if parsing fraction digits fails

[fredgan]

No description provided.

[coveralls]

Coverage increased (+0.3%) to 76.357% when pulling 18da630 on fredgan:decimal-type-resolve-optimization into 88fe1d5 on openconfig:master.

[robshakir]

Could you check that the logic here is right? I think integer types may also have a range that isn't nil, but this resolution of the range is now only implemented for decimal64. If we could add a testcase to ensure that this is properly covered (and demonstrate that we didn't break this) that'd be great.

[fredgan]

You're right, parsing the range of integer types is ignored, I've adjusted the logic and added a testcase.

[wenovus]

parseRanges already error checks where FractionDigits is 0 (via decimalValueFromString), so I'm wondering if it's actually better to keep the current behaviour and error out, instead of silently allowing parsing to continue?

[fredgan]

goyang/pkg/yang/types.go

Lines 247 to 250 in fd7d014

	case isDecimal64:
	// If we are directly of type decimal64 then we must specify
	// fraction-digits in the range from 1-18.
	i, err := t.FractionDigits.asRangeInt(1, 18)

In fact, the value 0 indicates that the FractionDigits value has already failed to be parsed (via asRangeInt).

If continue to parse, parserRanges checks the error value 0 as FractionDigits, then reports an unreasonable error msg bad range: invalid number of fraction digits 0: out of range [1..18], and the new testcase also fails because two errors are generated, so I think the value 0 needs to be excluded.

[wenovus]

Thanks! Makes sense.

[wenovus]

I think this is reasonable to add, although I would want to wait until an integration test is added, which should be simple once #188 is merged. If that checks out then LGTM.