Favicon: Issue 6245 - SwiftLinted project files

Favicon: Issue 6245 - SwiftLinted project files

[a.shephard, 2018-01-12 16:51:20]

Updated w/ new patchset containing more linting.

[dean, 2018-01-13 00:43:57]

On 2018/01/12 16:51:20, a.shephard wrote:
> Updated w/ new patchset containing more linting.

LGTM

[d108, 2018-02-13 21:48:45]

https://codereview.adblockplus.org/29664569/diff/29670591/FavIcon/FavIcon.swift
File FavIcon/FavIcon.swift (right):

https://codereview.adblockplus.org/29664569/diff/29670591/FavIcon/FavIcon.swi...
FavIcon/FavIcon.swift:21: #if os(iOS)
Swiftlint detects these imports out of order and produces a warning 

warning: Sorted Imports Violation: Imports should be sorted. (sorted_imports)

https://codereview.adblockplus.org/29664569/diff/29670591/FavIcon/IconExtract...
File FavIcon/IconExtraction.swift (right):

https://codereview.adblockplus.org/29664569/diff/29670591/FavIcon/IconExtract...
FavIcon/IconExtraction.swift:19: import Foundation
This import is duplicated by the one above it and should be removed.

https://codereview.adblockplus.org/29664569/diff/29670591/FavIcon/Utils.swift
File FavIcon/Utils.swift (right):

https://codereview.adblockplus.org/29664569/diff/29670591/FavIcon/Utils.swift...
FavIcon/Utils.swift:24: .map { $0.trimmingCharacters(in: .whitespaces) }
Nit: The original indentation was clearer with respect to the programming
pattern used here and was not a Swiftlint issue.

https://codereview.adblockplus.org/29664569/diff/29670591/FavIcon/Utils.swift...
FavIcon/Utils.swift:29: .filter { $0.contains("=") }
Nit: The original indentation was clearer with respect to the programming
pattern used here and was not a Swiftlint issue.

[a.shephard, 2018-02-16 09:06:44]

Changes have been made in this patchset to reflact the proposed changes from
d108 in the previous patchset.

[d108, 2018-02-16 23:27:18]

https://codereview.adblockplus.org/29664569/diff/29697569/FavIcon/FavIcon.swift
File FavIcon/FavIcon.swift (right):

https://codereview.adblockplus.org/29664569/diff/29697569/FavIcon/FavIcon.swi...
FavIcon/FavIcon.swift:21: #if os(OSX)
Swiftlint is still giving 
warning: Sorted Imports Violation: Imports should be sorted. (sorted_imports)

Instead of changing the order, adding a disable line right before the
conditional imports block results in the warning being removed for me using
Xcode 9.2 and swiftlint 0.25.0.

Code below:

// swiftlint:disable sorted_imports
#if os(iOS)
    import UIKit
    /// Alias for the iOS image type (`UIImage`).
    public typealias ImageType = UIImage
#elseif os(OSX)
    import Cocoa
    /// Alias for the OS X image type (`NSImage`).
    public typealias ImageType = NSImage
#endif

https://codereview.adblockplus.org/29664569/diff/29697569/FavIconTests/FavIco...
File FavIconTests/FavIconsTests.swift (right):

https://codereview.adblockplus.org/29664569/diff/29697569/FavIconTests/FavIco...
FavIconTests/FavIconsTests.swift:20: 
Swiftlint is still giving two instances of 
warning: Sorted Imports Violation: Imports should be sorted. (sorted_imports)

Adding a disable right before the conditional block with the imports results in
the warning being removed for me using Xcode 9.2 and swiftlint 0.25.0.

Code below:

// swiftlint:disable sorted_imports
#if os(iOS)
    import UIKit
#elseif os(OSX)
    import Cocoa
#endif

[a.shephard, 2018-02-19 10:31:14]

On 2018/02/16 23:27:18, d108 wrote:
>
https://codereview.adblockplus.org/29664569/diff/29697569/FavIcon/FavIcon.swift
> File FavIcon/FavIcon.swift (right):
> 
>
https://codereview.adblockplus.org/29664569/diff/29697569/FavIcon/FavIcon.swi...
> FavIcon/FavIcon.swift:21: #if os(OSX)
> Swiftlint is still giving 
> warning: Sorted Imports Violation: Imports should be sorted. (sorted_imports)
> 
> Instead of changing the order, adding a disable line right before the
> conditional imports block results in the warning being removed for me using
> Xcode 9.2 and swiftlint 0.25.0.
> 
> Code below:
> 
> // swiftlint:disable sorted_imports
> #if os(iOS)
>     import UIKit
>     /// Alias for the iOS image type (`UIImage`).
>     public typealias ImageType = UIImage
> #elseif os(OSX)
>     import Cocoa
>     /// Alias for the OS X image type (`NSImage`).
>     public typealias ImageType = NSImage
> #endif
> 
>
https://codereview.adblockplus.org/29664569/diff/29697569/FavIconTests/FavIco...
> File FavIconTests/FavIconsTests.swift (right):
> 
>
https://codereview.adblockplus.org/29664569/diff/29697569/FavIconTests/FavIco...
> FavIconTests/FavIconsTests.swift:20: 
> Swiftlint is still giving two instances of 
> warning: Sorted Imports Violation: Imports should be sorted. (sorted_imports)
> 
> Adding a disable right before the conditional block with the imports results
in
> the warning being removed for me using Xcode 9.2 and swiftlint 0.25.0.
> 
> Code below:
> 
> // swiftlint:disable sorted_imports
> #if os(iOS)
>     import UIKit
> #elseif os(OSX)
>     import Cocoa
> #endif

I've added the sorted_imports line to the FavIconsTest class, however adding it
to the FavIcon.swift class produces an error.
/Users/ashephard/Documents/eyeo/favicon/FavIcon/FavIcon.swift:19:36: error:
Superfluous Disable Command Violation: SwiftLint rule 'sorted_imports' did not
trigger a violation in the disabled region. Please remove the disable command.
(superfluous_disable_command)

It's strange that you receive a warning for sorted imports on FavIcon.swift, as
I don't receive that warning message.

[a.shephard, 2018-02-19 10:46:01]

I've updated the codereview a couple of times with different patchsets.
Explained in the iOS chat, sorted_imports are not raising a warning for me
within Xcode 9.2 and Swiftlint 0.25.0.
If I add the sorted_imports rule, it produces an error about superfluous rules
in SwiftLint.

[d108, 2018-02-20 01:03:13]

On 2018/02/19 10:46:01, a.shephard wrote:
> I've updated the codereview a couple of times with different patchsets.
> Explained in the iOS chat, sorted_imports are not raising a warning for me
> within Xcode 9.2 and Swiftlint 0.25.0.
> If I add the sorted_imports rule, it produces an error about superfluous rules
> in SwiftLint.

I believe I received the warnings due to a corrupted swiftlint installation. I'm
not sure how that happened but I've fixed it by uninstalling and reinstalling
swiftlint. I may have been running 0.22.0 even though it was reporting 0.25.0.

So sorry for misreporting the warnings - LGTM for patch set 6.

[dean, 2018-02-20 14:48:50]

On 2018/02/20 01:03:13, d108 wrote:
> On 2018/02/19 10:46:01, a.shephard wrote:
> > I've updated the codereview a couple of times with different patchsets.
> > Explained in the iOS chat, sorted_imports are not raising a warning for me
> > within Xcode 9.2 and Swiftlint 0.25.0.
> > If I add the sorted_imports rule, it produces an error about superfluous
rules
> > in SwiftLint.
> 
> I believe I received the warnings due to a corrupted swiftlint installation.
I'm
> not sure how that happened but I've fixed it by uninstalling and reinstalling
> swiftlint. I may have been running 0.22.0 even though it was reporting 0.25.0.
> 
> So sorry for misreporting the warnings - LGTM for patch set 6.

Latest revision LGTM