Replace duplicated cascades of case statements with utility methods on State

[acb-mv]

This replaces recurring cascades of case statements that mutate State's associated values with one utility method on State: mutateAssociatedData, which take a function that might modify the appropriate associated value, and if the current state has such a value, applies it and returns whether a modification took place. The function is given a StateAssociatedData type, which is a protocol encompassing both the connection and blocked state data's common fields.

The benefit of this change is increased separation of concerns: code that cares only about the connection/error state no longer needs to know which states exist, which will make changing the logic of the state machine simpler, as there will be fewer places where changes are needed.

---

This change is 

[buggmagnet]

Reviewed 7 of 7 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @acb-mv)

---

ios/PacketTunnelCore/Actor/State.swift line 99 at r2 (raw file):

}

// perhaps this should have a better name?

nit
This looks fine, but we should (ideally) be able to understand where it's used from the name alone.
We could rename it ConnectionStateAssociatedData for example, although that might feel a bit redundant.

---

ios/PacketTunnelCore/Actor/State+Extensions.swift line 131 at r2 (raw file):

            var associatedData: StateAssociatedData = connState
            modifier(&associatedData)
            self = self.replacingConnectionData(with: associatedData as! ConnectionData)

It's a bit annoying that we have to do this, but at least we should remove the swiftlint warning here and down in the blocked state.

Let's add a comment that specifices that both ConnectionData and BlockingData do implement StateAssociatedData so the cast is guaranteed to work.

[acb-mv]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @buggmagnet)

---

ios/PacketTunnelCore/Actor/State.swift line 99 at r2 (raw file):

Previously, buggmagnet wrote…

nit
This looks fine, but we should (ideally) be able to understand where it's used from the name alone.
We could rename it ConnectionStateAssociatedData for example, although that might feel a bit redundant.

Except that this is not specific to a connection state, it can also apply to a blockage/error state. While the type this belongs to is named State and protocols cannot live under namespaces, I'm using State as a prefix for the protocol name. Perhaps in future, it may do to move State under PacketTunnelActor and call it PacketTunnelActorStateAssociatedData or some abbreviation thereof (PacketTunnelActorStateData?)

Another option would be to make the name an adjective, which is something that goes well with protocols, though I haven't thought of a good one (StateDataBearing perhaps? Though that sounds awkward).

In any case, the comment is a leftover from before I renamed it and the name was worse, and I can delete it.

---

ios/PacketTunnelCore/Actor/State+Extensions.swift line 131 at r2 (raw file):

Previously, buggmagnet wrote…

It's a bit annoying that we have to do this, but at least we should remove the swiftlint warning here and down in the blocked state.

Let's add a comment that specifices that both ConnectionData and BlockingData do implement StateAssociatedData so the cast is guaranteed to work.

Is there a format for this comment that will shut swiftlint up, or is this just for human consumption?

[acb-mv]

ios/PacketTunnelCore/Actor/State+Extensions.swift line 131 at r2 (raw file):

Previously, acb-mv wrote…

Is there a format for this comment that will shut swiftlint up, or is this just for human consumption?

done

[buggmagnet]

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved