Changes for Swift 4

[liuxuan30]

Thanks to Swift 4's compatibility, this year is easier to upgrade on our own pace. But still works to do :(

Part I

modify All NSAttributedStringKey related methods for Swift 4

Note:

• extension NSString for func size(attributes attrs: [String : Any]? = nil) -> NSSize
  is deleted due to iOS 11 uses size(withAttributes: attrs) now

Known issues:

• There are tons of warnings about @objc usage, a new enhancement to reduce binary size since Swift 4, which is quite confusing, still waiting for newer Xcode 9 beta to see if it can help and investigating a better solution. Related SE: SE-0160. One SO Thread discuss it

[coty10]

Any news on swift 4 version?

[liuxuan30]

This is the least working change now. Waiting for Xcode 9 release and finalize it

[coty10]

so basically not before September....
i'm working on it in Xcode 9... i just have the language warning.. but everything else it's working... hopefully when the final swift 4 version will be released... i won't have to change all the code... but just use a nice pod install... :)
Thanks for the answer..

[liuxuan30]

the warning is because the @objc tag. looks like we have to add @objc for class scope, but this dims the idea it asks for. Anyway, based on last year experience, just wait the GM

[jjatie]

To clarify, SE-0160 doesn't make any recommendations as to where to put @objc tags, only that you now need to put them everywhere you want Objective-C compatibility (which Charts already does).

[liuxuan30]

@jjatie here: https://developer.apple.com/library/content/documentation/Swift/Conceptual/Swift_Programming_Language/Attributes.html#//apple_ref/doc/uid/TP40014097-CH35-ID348

Most code should use the objc attribute instead, to expose only the declarations that are needed. If you need to expose many declarations, you can group them in an extension that has the objc attribute. This attribute is a convenience for libraries that make heavy use of the introspection facilities of the Objective-C runtime. Applying the objc attribute when it isn’t needed can increase your binary size and adversely effect performance.

it seems do not suggest add @objc to class scope, but suggest "to expose only the declarations that are needed", that's what I mean. Currently if we add @objc before class, we loose the optimization? If we add @objc for each func, that gonna be a hug work. Yes eventually we might have to do the former way, but I "wish" something can change when Xcode GM 😹

[mrafayfarooq]

any update on Swift 4 conversion?

[jjatie]

1. The referenced quote is a discussion on using the @objcmembers attribute, not the @objc attribute.
2. On any class that inherits from NSObject we don't lose any optimizations since it implicitly has the @objc attribute.
3. We also need to use @objc at the type scope to provide an Objective-C name. If we want to try and improve performance, we can apply the @nonobjc tag on members that do not need to be exposed. We should not use @objc on classes which inherit from NSObject if we are not providing a different name.

[jjatie]

@mrafayfarooq if you're looking for a Swift 4 version, there is a fully functional one here without objective-c compatibility. I haven't run into any bugs yet, and am migrating more and more to a "pure Swift" implementation, keeping the same API until at least Swift 5.

[pmairoldi]

If CI passes then this should be good to go!

[codecov-io]

Codecov Report

Merging #2507 into master will increase coverage by 0.1%.
The diff coverage is 24.62%.

@@            Coverage Diff            @@
##           master    #2507     +/-   ##
=========================================
+ Coverage   19.67%   19.77%   +0.1%     
=========================================
  Files         113      113             
  Lines       13772    14137    +365     
=========================================
+ Hits         2710     2796     +86     
- Misses      11062    11341    +279

Impacted Files	Coverage Δ
...Data/Implementations/Standard/RadarChartData.swift	0% <ø> (ø)	⬆️
...mplementations/Standard/CandleChartDataEntry.swift	0% <ø> (ø)	⬆️
...Implementations/Standard/RadarChartDataEntry.swift	0% <ø> (ø)	⬆️
Source/Charts/Utils/Platform.swift	13.33% <ø> (ø)	⬆️
Source/Charts/Charts/CombinedChartView.swift	0% <ø> (ø)	⬆️
...a/Implementations/Standard/CombinedChartData.swift	0% <ø> (ø)	⬆️
.../Implementations/Standard/ChartDataEntryBase.swift	8.33% <ø> (ø)	⬆️
Source/Charts/Utils/ChartColorTemplates.swift	0% <ø> (ø)	⬆️
Source/Charts/Highlight/CombinedHighlighter.swift	0% <ø> (ø)	⬆️
Source/Charts/Highlight/RadarHighlighter.swift	0% <ø> (ø)	⬆️
... and 72 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f9eea57...5e3ac75. Read the comment docs.

[liuxuan30]

@petester42 we almost add @objc for each func and var. How about add objcMembers for the class?
According to

objcMembers
Apply this attribute to any class declaration that can have the objc attribute. The objc attribute is implicitly added to Objective-C compatible members of the class, its extensions, its subclasses, and all of the extensions of its subclasses.

Most code should use the objc attribute instead, to expose only the declarations that are needed. If you need to expose many declarations, you can group them in an extension that has the objc attribute. The objcMembers attribute is a convenience for libraries that make heavy use of the introspection facilities of the Objective-C runtime. Applying the objc attribute when it isn’t needed can increase your binary size and adversely affect performance.

Not sure if adding @objc for each func and var won't increase the binary size?

[pmairoldi]

It will increase the binary size but the functionality will be exactly the same as someone using swift 3. So it should not break someone's objective-c code written with an older version of the library.

[FasterThanLlamas]

Is there going to be an update to the CocoaPod?

[pmairoldi]

When this is merged yes.

[osianSmith]

When is this likely to be merged with main branch?

[bardonadam]

Can we merge this now that XCode 9 is available?

[liuxuan30]

I will check with latest Xcode9 and merge later if no issue found

[liuxuan30]

Build via Xcode9 and no build time errors, a green light to merge!
BTW, I made a legacy branch for current master branch, if anyone complains about Swift 4 upgrade, use the legacy branch https://github.com/liuxuan30/Charts/tree/Swift3.2-legacy

[danielgindi]

Are all these @objc tags really required?

[jjatie]

Many of them are not. Only those types, properties, and methods that consumers of the framework need access to in objective-c require the @objc tags. It is tedious to go through the framework and make the decision on each property.
For example: ChartAnimator's methods are exposed to the consumer through ChartViewBase, therefor ChartAnimator doesn't need any @objc tags.

[jjatie]

I may be wrong on this one, but another good example is all of the ChartRenderer classes don't seem to be meant for the consumer to use. In which case, not only can we remove the @objc tags on them, but also make them internal types.

I'd be happy to audit the framework for cases like this, and PR the corresponding changes. I don't want to take it on if it's not desired though.

[danielgindi]

ChartRenderer actually are supposed to be subclasses as necessary...

[danielgindi]

Maybe Apple will do this automatically in the future as a refactoring feature.
And maybe we will phase out objc support.... I don't know.

[pmairoldi]

They give you a option when migrating to swift 4 but picking the one with less caused too many errors. You can toggle it off in the project settings and it will give warnings for the onces that aren’t needed.

[jjatie]

The documentation on the changes to @objc inference:
https://github.com/apple/swift-evolution/blob/master/proposals/0160-objc-inference.md

@danielgindi Dox explains why this can't be a refactoring feature.
@petester42 The inference mode you're referring to is deprecated. If the past is any indication, I'd guess that it's completely removed for Swift 4.