JSONSerialization: add WritingOptions.sortedKeys

[ianpartridge]

This implements the new sortedKeys constant in JSONSerialization.WritingOptions and adds some tests. This constant is documented at https://developer.apple.com/documentation/foundation/jsonserialization.writingoptions - it is new in the macOS 10.13 SDK.

This option is used by the new Swift 4 JSONEncoder which has been recently implemented.

Note: I am still doing performance testing on this to check it doesn't regress the non-sorted case, but I'm opening the PR for review and comment now.

CC: @bubski @itaiferber

[ianpartridge]

By the way, @parkera: is there a reason that sortedKeys is listed under "Type Properties" but prettyPrinted is listed under "Constants" at https://developer.apple.com/documentation/foundation/jsonserialization.writingoptions ?

[pushkarnk]

@swift-ci please test

[ianpartridge]

Performance measurements of my first implementation showed ~8% cost in the default non-sorted case compared to the previous implementation, which is obviously not acceptable.

I've pushed another commit which eliminates this cost. It does make the code slightly more complex (there are now separate for-in loops for the sorted and non-sorted cases) but I don't think there is a way to avoid this.

@pushkarnk - please can you re-run the CI?

[itaiferber]

Looks good, save for the specifics of the string comparison

[itaiferber]

I don't think this will apply the exact same sorting strategy as we do on Darwin platforms. We are currently doing the equivalent of:

NSString * const key1 = ...
NSString * const key2 = ...
const NSRange range = NSMakeRange(0, key1.length);
NSLocale * const locale = [NSLocale systemLocale];
return [key1 compare:key2 options:NSNumericSearch | NSCaseInsensitiveSearch | NSForcedOrderingSearch range:range locale:locale];

Especially for archiving purposes, we should try to be consistent here.

[itaiferber]

@ianpartridge The "Type Properties" vs. "Constants" thing looks like an artifact of the documentation generation system. Likely worth filing a Radar about that

[ianpartridge]

Thanks for the review @itaiferber. Does this look right now?

[itaiferber]

LGTM

[itaiferber]

@swift-ci Please test

[phausler]

@swift-ci please test and merge

[ianpartridge]

Build failed with

Foundation/NSJSONSerialization.swift:538:62: error: cannot convert value of type 'CountableRange<String.Index>' to expected argument type 'Range<String.Index>?'
                return a.compare(b, options: options, range: range, locale: locale) == .orderedAscending

I'll take a look tomorrow.

[ianpartridge]

It looks like SE-0180 means that the compiler now infers let range = string.startIndex..<string.endIndex to be a CountableRange<String.Index> instead of Range<String.Index>.

As compare(_:options:range:locale:) requires a Range<String.Index>, I now specify this explicitly.

@phausler - can you try again please?

[phausler]

@swift-ci please test and merge

[parkera]

@ianpartridge The docs are updated independently of our headers, so we'll need a bug to fix those (Separate from all of this stuff of course).

[ianpartridge]

Thanks @parkera - I've opened a Radar for the doc issue.

[itaiferber]

@ianpartridge What's the Radar number? Want to make sure it gets to the right place. :)

[ianpartridge]

33233023

[itaiferber]

@ianpartridge Thanks!