a bug in swift3

[Mosaics]

@rs
Encounter problems as shown in the swift3 use, I think it's because your placeholder parameter has a default value, because of swift characteristics, can not pass parameters have default values.Cause it doesn't know what the hell am I was in use which method.Can you repair?

See I upload pictures

[Mosaics]

No response?

[Mosaics]

@rs

[phiren]

@Mosaics facing same problem, any workaround you found?

[Mosaics]

@rs @phiren As I said, via swift class point into your method You will find, as shown in figure, your options option is that there is a default empty array value, based on the characteristics of swift have default values are not pass parameters will be at the time of writing this method automatically appear two kinds, one kind is with this parameter, one kind is not belt, when he appeared with no arguments, in fact just write another way to conflict with you, it don't understand what you are using which method (is to ignore parameters method with options or no options that method), since the OC can be identified, suggest the swift, don't give this method directly the option of the initial value, forcing the incoming.This may be a solution.In fact, because of swift this feature, I was surprised that the other way to solve this problem.But I don't know this change whether the framework has big influence on you?

[phiren]

@Mosaics got your point, i was trying without the options. As i tried with options, now it giving me other error

[Mosaics]

@phiren Maybe you understand the wrong I mean, this is not a framework user error, but the framework itself is a defect.If must avoid, also for your usage, I didn't find there is a mistake.As shown in figure:

[bpoplauschi]

I will look at this soon (also marked it for 4.1.0). In the meantime, see #1627

[skyline75489]

I've doing some research on this. Turns out it isn't really that easy.

The key, which has been point out by @Mosaics is that Swift has internal support for default values of method parameters. This feature does not play well with those methods signatures in UIImageView+WebCache.h, which is designed to make Objective-C support default-values-of-method-parameters-like feature.

In most cases, this will simply cause redundancy, because the interoperation mechanism in Swift can convert nullable method parameters in Objective-C into default value nil in Swift, making the method with most parameters is the one and only method we need. However, the problem here specifically is OptionSet. Swift does not know what initial value we need for OptionSet type and chooses [] for default value. What we actually need is that options is required without a default value.

To resolve this, my suggestion would be creating a new interface for Swift, utilizing NS_REFINED_FOR_SWIFT & NS_SWIFT_UNAVAILABLE. First, make sure only one OC method is refined.

- (void)sd_setImageWithURL:(nullable NSURL *)url NS_SWIFT_UNAVAILABLE("Use refined method for Swift instead");
- (void)sd_setImageWithURL:(nullable NSURL *)url
          placeholderImage:(nullable UIImage *)placeholder NS_SWIFT_UNAVAILABLE("Use refined method for Swift instead");
//...
// We only need the last one.
- (void)sd_setImageWithURL:(nullable NSURL *)url
          placeholderImage:(nullable UIImage *)placeholder
                   options:(SDWebImageOptions)options
                  progress:(nullable SDWebImageDownloaderProgressBlock)progressBlock
                 completed:(nullable SDExternalCompletionBlock)completedBlock NS_REFINED_FOR_SWIFT;

Second, create new Swift interface like this:

extension UIImageView {
    
    func sd_setImage(url: URL?, placeholderImage: UIImage? = nil, options: SDWebImageOptions = .init(rawValue: 0), progress: SDWebImageDownloaderProgressBlock? = nil, completed: SDExternalCompletionBlock? = nil) {
        __sd_setImage(with: url, placeholderImage: placeholderImage, options: options, progress: progress, completed: completed)
    }

}

To avoid signature collision, the method is named sd_setImage(url:) instead of sd_setImage(with:).

Now, old methods won't show up in the IDE auto-completion and using those methods will be forbidden by compiler:

'sd_setImage(with:completed:)' is unavailable in Swift: Use refined method for Swift instead

---

Seems to me there is no other way to do this without creating a new Swift interface. That being said, I really don't like adding Swift source files into pure Objective-C libraries. One concern would be that once you have a Swift file in your library you can not use it as a pure static library any more. You'll have to use it as a dynamic (or embedded in Apple's dictionary) Framework. And you'll have to make your target iOS 8+ because embedded framework works only on iOS8+. This kind of sucks if you need to support lower version of iOS.

I thought about perhaps making a Swift subspec for this? But that isn't very elegant, either.

CC @bpoplauschi

[bpoplauschi]

@skyline75489 thanks for looking into this, your solution sounds like it can work and take care of the issue. Are you available for making a PR with this?

[bpoplauschi]

PS: Regarding a Swift subspec, CocoaLumberjack does the same and it works (not ideal solution but a working one).

@skyline75489 couldn't we mark as NS_SWIFT_UNAVAILABLE all the methods in each category except the ones that have all the params?

[skyline75489]

If I'm not misunderstanding this, we can and probably should do that in each category(e.g. UIImageView+HighlightedWebCache.h) to prevent any further problems regarding naming collision.

I'll try to make a PR tonight.

[skyline75489]

@bpoplauschi Seems like I've been misunderstanding your point. You mean just mark NS_SWIFT_UNAVAILABLE all the methods except the last one, without using any Swift code after all? It indeed is a easy fix, though. I think using Swift gives us more available space for imagination such as further "swifty" API refinement.

[bpoplauschi]

@skyline75489 yeah, that was my point. For now, I would just go with the simple solution (no Swift). Especially because it seems that your branch keeps failing at building in Travis at this pod lib lint --allow-warnings

[skyline75489]

@bpoplauschi Alright then. I'll go and make a more simple fix. The Swift way is actually way more complicated than I thought. And I finally understand why CocoaLumberjack is using a separate Swift framework because otherwise many other problems would arise.

[bpoplauschi]

Exactly. And thanks

[skyline75489]

@bpoplauschi I've been trying to come up with a Objective-C only fix. But the result is not very ideal. The generated Swift method signature for the method that have all the parameters looks like this:

open func sd_setImage(with url: URL?, placeholderImage placeholder: UIImage?, options: SDWebImageOptions = [], progress progressBlock: SDWebImage.SDWebImageDownloaderProgressBlock?, completed completedBlock: SDWebImage.SDExternalCompletionBlock? = nil)

As you can see only the options and completed are granted a default value. I'm not sure why but this is making the API less flexible than in Objective-C.

[bpoplauschi]

@skyline75489

• for the method you posted above, the options param has a default value because it's an enum, and also the completedBlock param has a default value being a trailing clojure param. My only problem there is that the progressBlock has no default value so we kinda have to always add it.
• to fix this, I would suggest instead of just one method left available in each category, make two of them, one with progressBlock and one without it. We can use NS_SWIFT_NAME to make sure the two methods don't collide

The ObjC interface would look like this:

- (void)sd_setImageWithURL:(nullable NSURL *)url NS_SWIFT_UNAVAILABLE("");
...
- (void)sd_setImageWithURL:(nullable NSURL *)url
          placeholderImage:(nullable UIImage *)placeholder
                   options:(SDWebImageOptions)options
                 completed:(nullable SDExternalCompletionBlock)completedBlock 
NS_SWIFT_NAME(sd_setImage(url:placeholder:options:completed:));

- (void)sd_setImageWithURL:(nullable NSURL *)url
          placeholderImage:(nullable UIImage *)placeholder
                   options:(SDWebImageOptions)options
                  progress:(nullable SDWebImageDownloaderProgressBlock)progressBlock
                 completed:(nullable SDExternalCompletionBlock)completedBlock 
NS_SWIFT_NAME(sd_setImageWithProgress(url:placeholder:options:progress:completed:));

and the generated Swift interface

open func sd_setImage(url: URL?, placeholder: UIImage?, options: SDWebImageOptions = [], completed completedBlock: __ObjC.SDExternalCompletionBlock? = nil)

open func sd_setImageWithProgress(url: URL?, placeholder: UIImage?, options: SDWebImageOptions = [], progress progressBlock: __ObjC.SDWebImageDownloaderProgressBlock?, completed completedBlock: __ObjC.SDExternalCompletionBlock? = nil)

How does this sound?

[skyline75489]

This fixes progress but there is still placeholderImage left without a default value? Should we also add it?

[skyline75489]

And by the way, using NS_SWIFT_UNAVAILABLE may not perform as expected. If I try to neglect some parameters in the long method, Xcode still gives something like:

'sd_setImage(with:completed:)' is unavailable in Swift: Use refined method for Swift instead

It seems that Xcode still know the short version is there but refuses to let you use it, which is kind of strange and here just not correct for us. I think NS_REFINED_FOR_SWIFT might be a better idea because it actually rename the method.

[bpoplauschi]

I'm not too worried about placeholder, as you can simply pass nil there when using the APIs. The original issue we started from is compiler errors when using some of the APIs. And the exception for progress is because it's in the middle of two other params with default values.
The general recommendation (for all languages) is to group all params with default value at the end.
Also, I think only a few people use progress and therefore can use a different API when that is the case. For placeholder, I think it's the other way around, most of the people use it.

[bpoplauschi]

About NS_SWIFT_UNAVAILABLE, I see your point.
Another idea for this issue:

• we added the sd_ prefix to avoid colliding with other categories on UIImageView. For instance, AFNetworking had a similar categ with a setImageWithURL: method.
• but for Swift, since we have namespaces, we could use NS_SWIFT_NAME(setImage(url:placeholder:options:completed:)) without the sd_ prefix, so the compiler can distinguish between the two.

[bpoplauschi]

As for NS_REFINED_FOR_SWIFT, I didn't use it before but it looks interesting - see details.

For - (void)sd_setImageWithURL:(nullable NSURL *)url NS_REFINED_FOR_SWIFT("") it will generate open func __sd_setImage(with url: URL?), so with __ as prefix.

This can work as well.

[bpoplauschi]

Last comment from this burst :)

• since we have already a version 4.0.0 and we want to go to 4.1.0, we can't make breaking changes, so dropping the sd_ prefix doesn't work. I'd go with NS_REFINED_FOR_SWIFT in this case.