json-schema-org / JSON-Schema-Test-Suite

A language agnostic test suite for the JSON Schema specifications
MIT License
618 stars 205 forks source link

1.0 listed as not-integer #22

Closed awwright closed 11 years ago

awwright commented 11 years ago

e39d53703cbf4047b4fcd864dc55ad412c4c9d1d added a test saying that, apparently, 1.0 should not validate as an integer.

This is a problem, most notably, for Javascript/ECMAScript (I wonder where JSON comes from), where everything is an IEEE 64-bit floating point, and fails against my library since this is listed as a required test even though ES itself has no other alternative.

Differentiation between floating point and non-floating-point forms is listed as an optional (actually, suggested) part of v4, and is not mentioned at all in v3.

This test should, accordingly, be moved to the "optional tests" section.

fge commented 11 years ago

1.0 is indeed not an integer as per section 3.5 of the core spec which defines an integer as "a number without any fractional or exponent part". RFC 4627, section 2.4:

number = [ minus ] int [ frac ] [ exp ]

The integer removes the frac and exp parts from the picture. It is far from being optional!

This test is therefore normal and should not be moved to "optional" since the vast majority of languages do differentiate between integers and floating point numbers (even Python).

Let us say that it is a test that is expected to fail for JS based validators... Unless someone has actually found out how to do it!

awwright commented 11 years ago

JSON Schema semantics defines otherwise:

It is acknowledged by this specification that some programming languages, and their associated parsers, use different internal representations for floating point numbers and integers, while others do not.

As a consequence, for interoperability reasons, JSON values used in the context of JSON Schema, whether that JSON be a JSON Schema or an instance, SHOULD ensure that mathematical integers be represented as integers as defined by this specification.

The RFC has to distinguish between the two forms because they're different encodings of the same value. But while we're at it, let's also add a tests and a JSON Schema type for "exponent", too:

exp = e [ minus / plus ] 1*DIGIT

I mean NO, of course not, that's silly. If we're going to be citing the JSON RFC as the final authority then let's also add tests to make sure we can't validate application/json documents that aren't arrays or objects, either.

fge commented 11 years ago

Well, an exponent is a part of a number. I mean, 1.298e+232 is legal, right?

OK, it's going to be moved to optional. But really, this JavaScript brain damaged number handling is a royal PITA.

And anyway, I suppose JavaScript is never going to be used in finance, so at least they are safe :p

awwright commented 11 years ago

I might suggest a slightly more descriptive filename, maybe "floatNotInt", otherwise people who want to include this optional test (which should be plenty of them) would see "jsBrainDamage" which makes it sound like it doesn't apply to them.

I was wondering why I was getting a syntax error even though GitHub says it passed, I was going to fix the error but it looks like you got it after all.

Thanks!

fge commented 11 years ago

OK, renamed to zeroTerminatedFloats.json

awwright commented 11 years ago

Perfect

geraintluff commented 11 years ago

It still refers to it as "brain damaged" inside the file.

Change it, Francis.

geraintluff commented 11 years ago

Never mind, I've done it.

Look - I know that you're primarily a Java programmer, and Java deals with numeric types a certain way. And that is appropriate (even necessary) for some situations.

But the distinction between floating-point and integer is one of internal representation. If a language (JS is not the only one) does not treat that internal representation as an essential aspect of the numeric value, then that is also a valid decision.

There is no need to continue your crusade all the way to naming tests.

Julian commented 11 years ago

Yes, sorry, forgot about languages without the distinction. This is good (though I'd prefer it to have been done on develop, I'll merge it there now). We need a page discussing how optional the optional tests are now though. I'll open that as a separate ticket.