flowkey / UIKit-cross-platform

Cross-platform Swift implementation of UIKit, mostly for Android
MIT License
594 stars 40 forks source link

Fix Hashable conformance of UIColor (and crashes when storing them in Collections) #302

Closed ephemer closed 4 years ago

ephemer commented 4 years ago

Fixes crashes when storing UIColor or objects containing UIColors in Collections.

Type of change: Bug fix

Motivation (current vs expected behavior)

There are two changes in this PR:

  1. We renamed UIColor.self.red etc. to .redValue to avoid clashing with the static properties UIColor.red etc.

  2. We override the hash property to stop the Swift runtime from using ObjectIdentifier(self).hashValue, which is the default for NSObject.

From NSObject.swift in swift-corelibs-foundation under open var hash: Int:

    /// Returns an integer that can be used as a table address in a hash table structure.
    ///
    /// If two objects are equal (as determined by the `isEqual(_:)` method),
    /// they must have the same hash value. This last point is particularly important
    /// if you define `hash` in a subclass and intend to put instances of that subclass
    /// into a collection.
    ///

Previously if we created two UIColors using the same input values, we would have had two different hash values objects that were otherwise equatable. That is obviously not what we want, because apparently that breaks some kind of contract in collections. Fixing this issue should fix a lot of crashes we were seeing on Android.

Please check if the PR fulfills these requirements

codecov[bot] commented 4 years ago

Codecov Report

Merging #302 into master will not change coverage. The diff coverage is 28.57%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #302   +/-   ##
=======================================
  Coverage   51.97%   51.97%           
=======================================
  Files          87       87           
  Lines        3161     3161           
=======================================
  Hits         1643     1643           
  Misses       1518     1518
Impacted Files Coverage Δ
Sources/UINavigationBarAndroid.swift 57.14% <ø> (ø) :arrow_up:
Sources/CALayer+SDL.swift 0% <0%> (ø) :arrow_up:
Sources/AnimatableProperty.swift 0% <0%> (ø) :arrow_up:
Sources/UIColor.swift 19.29% <50%> (ø) :arrow_up:

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 592a6b4...c802025. Read the comment docs.