[SR-2151]NSJSONSerialization.data produces illegal JSON code

[DaveLiu888]

What's in this pull request?
Resolved bug number:SR-2151 (https://bugs.swift.org/browse/SR-2151)
Implementation
Tests (note all tests are done on NSJSONSerialization due to access lvl protection )

Problem :
NSJSONSerialization.data produces illegal JSON code

Cause :

NSJSONSerialization func serializeNumber(_ num: NSNumber) throws
uses NSNumber open override var description: String which calls
open func description(withLocale locale: Locale?) -> String with a nil locale.

The nil locale case is handled by setting the format style to kCFNumberFormatterNoStyle & kCFNumberFormatterMaxFractionDigits
the leading zero does not show up which results in the incorrect format.

["foo": 0.123]
should be

{
   "foo": 0.1234
}

but is

{
    "foo": .1234
}

Proposed Solution:

1. NSJSONSerialization func serializeNumber(_ num: NSNumber) stop calling description(withLocale locale: Locale?) -> String as locale imho should not have any determination for JSON serialization
2. Create a separate internal computed property internal var serializationString: String to return the proper formatted string

• check whether or not the underlying store is a _cfObject is a fractionDigit type which need leading zero padding and conditionally use a custom formatter string "0.###############".

  Reason for using custom format string instead of existing style was largely due to the documentation and programing guide suggestions on the fact exact formats of this sort should be decoupled from Locale

https://developer.apple.com/library/prerelease/content/documentation/CoreFoundation/Conceptual/CFDataFormatting/Articles/dfCreatingCFNumberFormatters.html

To create a CFNumberFormatter, you must specify a locale and a formatter style as illustrated in Listing 1, or a format string, as shown in Listing 2. Format styles do not specify an exact format—they depend on the locale, user preference settings, and operating system version. If you want to specify an exact format, use the CFNumberFormatterSetFormat function to set the format string, and the CFNumberFormatterSetProperty function to change specific properties such as separators, the "Not a number" symbol, and the padding character.

https://developer.apple.com/library/ios/documentation/CoreFoundation/Reference/CFNumberFormatterRef/#//apple_ref/doc/constant_group/Number_Formatter_Styles

The format for these number styles is not exact because they depend on the locale, user preference settings, and operating system version. Do not use these constants if you want an exact format (for example, if you are parsing data in a given format). In general, however, you are encouraged to use these styles to accommodate user preferences.

[parkera]

I think the intent here is correct, but I would rather factor the serialization logic into NSJSON instead of into NSNumber. There are several kinds of serialization (e.g., NSCoding), so putting the JSON-specific logic in NSNumber itself seems like unnecessary coupling.

That also means we could create the number formatter once and reuse it through the entire serialization instead of creating one over and over again with the same settings, which could be very expensive. (Probably we don't even need to use an actual formatter here at all, but it's fine for now. NSJSONSerialization in Foundation uses snprintf).

[DaveLiu888]

@parkera thanks for the review! I totally agree in hindsight it is very obvious of the coupling. I updated the pr, please kindly take a look when you get a chance.

[parkera]

@swift-ci please test linux

[parkera]

Looks like this may need some fixes for Linux (where bridging is unfortunately still a bit different):

TestFoundation/TestNSJSONSerialization.swift:658:51: error: 'Double' is not convertible to 'AnyObject'; did you mean to use 'as!' to force downcast?
                let testDict = [param.0 : param.1 as AnyObject] as [String : AnyObject]
                                          ~~~~~~~~^~~~~~~~~~~~
                                                  as!
TestFoundation/TestNSJSONSerialization.swift:674:51: error: 'Double' is not convertible to 'AnyObject'; did you mean to use 'as!' to force downcast?
                let testDict = [param.0 : param.1 as AnyObject] as [String : AnyObject]
                                          ~~~~~~~~^~~~~~~~~~~~
                                                  as!
TestFoundation/TestNSJSONSerialization.swift:689:51: error: 'Double' is not convertible to 'AnyObject'; did you mean to use 'as!' to force downcast?
                let testDict = [param.0 : param.1 as AnyObject] as [String : AnyObject]
                                          ~~~~~~~~^~~~~~~~~~~~
                                                  as!
TestFoundation/TestNSJSONSerialization.swift:698:42: error: 'Int' is not convertible to 'AnyObject'; did you mean to use 'as!' to force downcast?
                let testDict = [iStr : i as AnyObject] as [String : AnyObject]
                                       ~~^~~~~~~~~~~~
                                         as!

[DaveLiu888]

@parkera I updated and removed the casting and also updated to the latest master

[parkera]

@swift-ci please test linux

[DaveLiu888]

@parkera hi, any update on this pr is there any thing i can help with?

[parkera]

Should be good to go

[parkera]

Oops - ran afoul of our other changes. I need to revert.

[parkera]

Foundation/NSJSONSerialization.swift:261:86: error: value of type 'Int' has no member '_bridgeToObject'
        CFNumberFormatterSetProperty(formatter, kCFNumberFormatterMaxFractionDigits, 15._bridgeToObject())
                                                                                     ^~ ~~~~~~~~~~~~~~~
Foundation/Bridging.swift:46:17: note: did you mean '_bridgeToAnyObject'?
    public func _bridgeToAnyObject() -> AnyObject {
                ^
Foundation/NSNumber.swift:38:17: note: did you mean '_bridgeToObjectiveC'?
    public func _bridgeToObjectiveC() -> _ObjectType {
                ^

[parkera]

@DaveLiu888 can you rebase on top of master and re-submit the PR?

[phausler]

The syntax for bridging has changed; I would suggest to actually just create a NSNumber via NSNumber(value: 15)

[DaveLiu888]

@parkera @phausler 👍 will do