Repository: pion/ice
Pull request #384 (2021-08-06 16:42:56)

Remove unknown constant
This constant tends to cause collisions with enumerations, and is
in general a source of bugs

#### Description

#### Reference issue
Related to pion/webrtc#1293


codecov[bot]: # [Codecov](https://codecov.io/gh/pion/ice/pull/384?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) Report
> Merging [#384](https://codecov.io/gh/pion/ice/pull/384?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (a4211ba) into [master](https://codecov.io/gh/pion/ice/commit/178704e1d526f1d9625bb167776ad673b045a1f5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (178704e) will **decrease** coverage by `52.96%`.
> The diff coverage is `47.61%`.

> :exclamation: Current head a4211ba differs from pull request most recent head 8891419. Consider uploading reports for the commit 8891419 to get more accurate results
[![Impacted file tree graph](https://codecov.io/gh/pion/ice/pull/384/graphs/tree.svg?width=650&height=150&src=pr&token=1ivIyHDzva&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)](https://codecov.io/gh/pion/ice/pull/384?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)

```diff
@@             Coverage Diff             @@
##           master     #384       +/-   ##
===========================================
- Coverage   78.44%   25.47%   -52.97%     
===========================================
  Files          33       33               
  Lines        2598     2606        +8     
===========================================
- Hits         2038      664     -1374     
- Misses        391     1914     +1523     
+ Partials      169       28      -141     
```

| Flag | Coverage Δ | |
|---|---|---|
| go | `?` | |
| wasm | `25.47% <47.61%> (-0.08%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/pion/ice/pull/384?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | |
|---|---|---|
| [candidate\_base.go](https://codecov.io/gh/pion/ice/pull/384/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-Y2FuZGlkYXRlX2Jhc2UuZ28=) | `54.63% <0.00%> (-28.13%)` | :arrow_down: |
| [candidatepair\_state.go](https://codecov.io/gh/pion/ice/pull/384/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-Y2FuZGlkYXRlcGFpcl9zdGF0ZS5nbw==) | `0.00% <0.00%> (ø)` | |
| [candidatetype.go](https://codecov.io/gh/pion/ice/pull/384/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-Y2FuZGlkYXRldHlwZS5nbw==) | `62.06% <0.00%> (-15.71%)` | :arrow_down: |
| [gather.go](https://codecov.io/gh/pion/ice/pull/384/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-Z2F0aGVyLmdv) | `0.00% <0.00%> (-67.68%)` | :arrow_down: |
| [ice.go](https://codecov.io/gh/pion/ice/pull/384/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-aWNlLmdv) | `100.00% <ø> (ø)` | |
| [url.go](https://codecov.io/gh/pion/ice/pull/384/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-dXJsLmdv) | `94.49% <83.33%> (-1.77%)` | :arrow_down: |
| [candidate\_peer\_reflexive.go](https://codecov.io/gh/pion/ice/pull/384/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-Y2FuZGlkYXRlX3BlZXJfcmVmbGV4aXZlLmdv) | `0.00% <0.00%> (-84.62%)` | :arrow_down: |
| [agent\_config.go](https://codecov.io/gh/pion/ice/pull/384/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-YWdlbnRfY29uZmlnLmdv) | `0.00% <0.00%> (-84.38%)` | :arrow_down: |
| [transport.go](https://codecov.io/gh/pion/ice/pull/384/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-dHJhbnNwb3J0Lmdv) | `0.00% <0.00%> (-82.70%)` | :arrow_down: |
| [agent.go](https://codecov.io/gh/pion/ice/pull/384/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-YWdlbnQuZ28=) | `0.00% <0.00%> (-81.38%)` | :arrow_down: |
| ... and [17 more](https://codecov.io/gh/pion/ice/pull/384/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/ice/pull/384?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pion/ice/pull/384?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Last update [178704e...8891419](https://codecov.io/gh/pion/ice/pull/384?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).


Patch:
diff --git a/AUTHORS.txt b/AUTHORS.txt
index 68666a4..7846911 100644
--- a/AUTHORS.txt
+++ b/AUTHORS.txt
@@ -17,6 +17,7 @@ Chao Yuan <yuanchao0310@163.com>
 David Hamilton <davidhamiltron@gmail.com>
 David Zhao <david@davidzhao.com>
 Henry <cryptix@riseup.net>
+hn8 <10730886+hn8@users.noreply.github.com>
 Hugo Arregui <hugo.arregui@gmail.com>
 Hugo Arregui <hugo@decentraland.org>
 Jason Maldonis <jason.maldonis@i3pd.com>
@@ -41,6 +42,7 @@ Sean DuBois <sean@siobud.com>
 Sebastian Waisbrot <seppo0010@gmail.com>
 Sidney San Martín <sidney@s4y.us>
 Will Forcey <wsforc3y@gmail.com>
+Woodrow Douglass <wdouglass@carnegierobotics.com>
 Yutaka Takeda <yt0916@gmail.com>
 ZHENK <chengzhenyang@gmail.com>
 Zizheng Tai <me@zizheng.me>
diff --git a/candidate_base.go b/candidate_base.go
index 63231e6..45229d2 100644
--- a/candidate_base.go
+++ b/candidate_base.go
@@ -172,6 +172,8 @@ func (c *candidateBase) LocalPreference() uint16 {
 				}
 			case CandidateTypeUnspecified:
 				return 0
+			default:
+				return 0
 			}
 			return 0
 		}()
diff --git a/candidatepair_state.go b/candidatepair_state.go
index 28c7187..99a89f4 100644
--- a/candidatepair_state.go
+++ b/candidatepair_state.go
@@ -20,6 +20,9 @@ const (
 	// CandidatePairStateSucceeded means a check for this pair was already
 	// done and produced a successful result.
 	CandidatePairStateSucceeded
+
+	// CandidatePairStateUnknown probably indicates an error
+	CandidatePairStateUnknown
 )
 
 func (c CandidatePairState) String() string {
@@ -32,6 +35,7 @@ func (c CandidatePairState) String() string {
 		return "failed"
 	case CandidatePairStateSucceeded:
 		return "succeeded"
+	default:
+		return "Unknown candidate pair state"
 	}
-	return "Unknown candidate pair state"
 }
diff --git a/candidatetype.go b/candidatetype.go
index 376c408..382887a 100644
--- a/candidatetype.go
+++ b/candidatetype.go
@@ -10,6 +10,7 @@ const (
 	CandidateTypeServerReflexive
 	CandidateTypePeerReflexive
 	CandidateTypeRelay
+	CandidateTypeUnknown
 )
 
 // String makes CandidateType printable
@@ -25,8 +26,9 @@ func (c CandidateType) String() string {
 		return "relay"
 	case CandidateTypeUnspecified:
 		return "Unknown candidate type"
+	default:
+		return "Unknown candidate type"
 	}
-	return "Unknown candidate type"
 }
 
 // Preference returns the preference weight of a CandidateType
@@ -45,8 +47,9 @@ func (c CandidateType) Preference() uint16 {
 		return 100
 	case CandidateTypeRelay, CandidateTypeUnspecified:
 		return 0
+	default:
+		return 0
 	}
-	return 0
 }
 
 func containsCandidateType(candidateType CandidateType, candidateTypeList []CandidateType) bool {
diff --git a/gather.go b/gather.go
index 6a6588a..7c6931f 100644
--- a/gather.go
+++ b/gather.go
@@ -114,6 +114,7 @@ func (a *Agent) gatherCandidates(ctx context.Context) {
 				wg.Done()
 			}()
 		case CandidateTypePeerReflexive, CandidateTypeUnspecified:
+		default:
 		}
 	}
 	// Block until all STUN and TURN URLs have been gathered (or timed out)
diff --git a/ice.go b/ice.go
index d7094f6..1630fcb 100644
--- a/ice.go
+++ b/ice.go
@@ -25,6 +25,9 @@ const (
 
 	// ConnectionStateClosed ICE agent has finished and is no longer handling requests
 	ConnectionStateClosed
+
+	// Unknown State
+	ConnectionStateUnknown
 )
 
 func (c ConnectionState) String() string {
@@ -60,6 +63,9 @@ const (
 
 	// GatheringStateComplete indicates candidate gatering has been completed
 	GatheringStateComplete
+
+	// GatheringStateUnknown probably indicates an error
+	GatheringStateUnknown
 )
 
 func (t GatheringState) String() string {
diff --git a/url.go b/url.go
index 390591e..263eea6 100644
--- a/url.go
+++ b/url.go
@@ -9,10 +9,6 @@ import (
 // SchemeType indicates the type of server used in the ice.URL structure.
 type SchemeType int
 
-// Unknown defines default public constant to use for "enum" like struct
-// comparisons when no value was defined.
-const Unknown = iota
-
 const (
 	// SchemeTypeSTUN indicates the URL represents a STUN server.
 	SchemeTypeSTUN SchemeType = iota + 1
@@ -25,6 +21,9 @@ const (
 
 	// SchemeTypeTURNS indicates the URL represents a TURNS (secure) server.
 	SchemeTypeTURNS
+
+	// SchemeTypeUnknown indicated an unknown scheme type
+	SchemeTypeUnknown
 )
 
 // NewSchemeType defines a procedure for creating a new SchemeType from a raw
@@ -40,7 +39,7 @@ func NewSchemeType(raw string) SchemeType {
 	case "turns":
 		return SchemeTypeTURNS
 	default:
-		return SchemeType(Unknown)
+		return SchemeTypeUnknown
 	}
 }
 
@@ -69,6 +68,9 @@ const (
 
 	// ProtoTypeTCP indicates the URL uses a TCP transport.
 	ProtoTypeTCP
+
+	// ProtoTypeUnknown indicates an unknown protocol type, which is probably an error
+	ProtoTypeUnknown
 )
 
 // NewProtoType defines a procedure for creating a new ProtoType from a raw
@@ -80,7 +82,7 @@ func NewProtoType(raw string) ProtoType {
 	case "tcp":
 		return ProtoTypeTCP
 	default:
-		return ProtoType(Unknown)
+		return ProtoTypeUnknown
 	}
 }
 
@@ -116,7 +118,7 @@ func ParseURL(raw string) (*URL, error) { //nolint:gocognit
 
 	var u URL
 	u.Scheme = NewSchemeType(rawParts.Scheme)
-	if u.Scheme == SchemeType(Unknown) {
+	if u.Scheme == SchemeTypeUnknown {
 		return nil, ErrSchemeType
 	}
 
@@ -172,7 +174,7 @@ func ParseURL(raw string) (*URL, error) { //nolint:gocognit
 		}
 
 		u.Proto = proto
-		if u.Proto == ProtoType(Unknown) {
+		if u.Proto == ProtoTypeUnknown {
 			u.Proto = ProtoTypeUDP
 		}
 	case SchemeTypeTURNS:
@@ -182,9 +184,11 @@ func ParseURL(raw string) (*URL, error) { //nolint:gocognit
 		}
 
 		u.Proto = proto
-		if u.Proto == ProtoType(Unknown) {
+		if u.Proto == ProtoTypeUnknown {
 			u.Proto = ProtoTypeTCP
 		}
+	default:
+		return nil, ErrSTUNQuery
 	}
 
 	return &u, nil
@@ -193,19 +197,19 @@ func ParseURL(raw string) (*URL, error) { //nolint:gocognit
 func parseProto(raw string) (ProtoType, error) {
 	qArgs, err := url.ParseQuery(raw)
 	if err != nil || len(qArgs) > 1 {
-		return ProtoType(Unknown), ErrInvalidQuery
+		return ProtoTypeUnknown, ErrInvalidQuery
 	}
 
-	var proto ProtoType
+	proto := ProtoTypeUnknown
 	if rawProto := qArgs.Get("transport"); rawProto != "" {
-		if proto = NewProtoType(rawProto); proto == ProtoType(0) {
-			return ProtoType(Unknown), ErrProtoType
+		if proto = NewProtoType(rawProto); proto == ProtoTypeUnknown {
+			return ProtoTypeUnknown, ErrProtoType
 		}
 		return proto, nil
 	}
 
 	if len(qArgs) > 0 {
-		return ProtoType(Unknown), ErrInvalidQuery
+		return ProtoTypeUnknown, ErrInvalidQuery
 	}
 
 	return proto, nil


Test patch:
diff --git a/candidatepair_test.go b/candidatepair_test.go
index 119ab01..ea85515 100644
--- a/candidatepair_test.go
+++ b/candidatepair_test.go
@@ -131,3 +131,11 @@ func TestNilCandidatePairString(t *testing.T) {
 	var nilCandidatePair *CandidatePair
 	assert.Equal(t, nilCandidatePair.String(), "")
 }
+
+func TestInvalidCandidatePairStateString(t *testing.T) {
+	state := CandidatePairState(0)
+	assert.NotEqual(t, "waiting", state.String())
+	assert.NotEqual(t, "in-progress", state.String())
+	assert.NotEqual(t, "failed", state.String())
+	assert.NotEqual(t, "succeeded", state.String())
+}
diff --git a/ice_test.go b/ice_test.go
index 727ae5e..f0d67b3 100644
--- a/ice_test.go
+++ b/ice_test.go
@@ -11,7 +11,7 @@ func TestConnectedState_String(t *testing.T) {
 		connectionState ConnectionState
 		expectedString  string
 	}{
-		{ConnectionState(Unknown), "Invalid"},
+		{ConnectionStateUnknown, "Invalid"},
 		{ConnectionStateNew, "New"},
 		{ConnectionStateChecking, "Checking"},
 		{ConnectionStateConnected, "Connected"},
@@ -35,7 +35,7 @@ func TestGatheringState_String(t *testing.T) {
 		gatheringState GatheringState
 		expectedString string
 	}{
-		{GatheringState(Unknown), ErrUnknownType.Error()},
+		{GatheringStateUnknown, ErrUnknownType.Error()},
 		{GatheringStateNew, "new"},
 		{GatheringStateGathering, "gathering"},
 		{GatheringStateComplete, "complete"},
diff --git a/url_test.go b/url_test.go
index 2527a10..a21a11a 100644
--- a/url_test.go
+++ b/url_test.go
@@ -56,6 +56,7 @@ func TestParseURL(t *testing.T) {
 			{"stun:[::1]:123a", ErrPort},
 			{"google.de", ErrSchemeType},
 			{"stun:", ErrHost},
+			{"stan:", ErrSchemeType},
 			{"stun:google.de:abc", ErrPort},
 			{"stun:google.de?transport=udp", ErrSTUNQuery},
 			{"stuns:google.de?transport=udp", ErrSTUNQuery},