Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade linters #2985

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
54 changes: 36 additions & 18 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,37 @@ linters-settings:
- ^os.Exit$
- ^panic$
- ^print(ln)?$
varnamelen:
max-distance: 12
min-name-length: 2
ignore-type-assert-ok: true
ignore-map-index-ok: true
ignore-chan-recv-ok: true
ignore-decls:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't do ignore-decls because they are shared between repos :(

I really like this varnamelen. wg and pc are only things you are gonna know if you have wrote a lot of Go/WebRTC. This is really going to help people learn better!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't do ignore-decls because they are shared between repos :(

Even if I only apply it to shared decls like n int? :( I don't think it's necessary for pc and wg anymore since I changed min-name-length to 2.

- pc *PeerConnection
- pc *webrtc.PeerConnection
- wg *sync.WaitGroup
- wg sync.WaitGroup
- i int
- n int
- m map[string]interface{}
- w io.Writer
- r io.Reader
- b []byte

linters:
enable:
- asciicheck # Simple linter to check that your code does not contain non-ASCII identifiers
- bidichk # Checks for dangerous unicode character sequences
- bodyclose # checks whether HTTP response body is closed successfully
- containedctx # containedctx is a linter that detects struct contained context.Context field
- contextcheck # check the function whether use a non-inherited context
- cyclop # checks function and package cyclomatic complexity
- decorder # check declaration order and count of types, constants, variables and functions
- dogsled # Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f())
- dupl # Tool for code clone detection
- durationcheck # check for two durations multiplied together
- err113 # Golang linter to check the errors handling expressions
- errcheck # Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases
- errchkjson # Checks types passed to the json encoding functions. Reports unsupported types and optionally reports occations, where the check for the returned error can be omitted.
- errname # Checks that sentinel errors are prefixed with the `Err` and error types are suffixed with the `Error`.
Expand All @@ -44,68 +64,66 @@ linters:
- exportloopref # checks for pointers to enclosing loop variables
- forbidigo # Forbids identifiers
- forcetypeassert # finds forced type assertions
- funlen # Tool for detection of long functions
- gci # Gci control golang package import order and make it always deterministic.
- gochecknoglobals # Checks that no globals are present in Go code
- gochecknoinits # Checks that no init functions are present in Go code
- gocognit # Computes and checks the cognitive complexity of functions
- goconst # Finds repeated strings that could be replaced by a constant
- gocritic # The most opinionated Go source code linter
- gocyclo # Computes and checks the cyclomatic complexity of functions
- godot # Check if comments end in a period
- godox # Tool for detection of FIXME, TODO and other comment keywords
- err113 # Golang linter to check the errors handling expressions
- gofmt # Gofmt checks whether code was gofmt-ed. By default this tool runs with -s option to check for code simplification
- gofumpt # Gofumpt checks whether code was gofumpt-ed.
- goheader # Checks is file header matches to pattern
- goimports # Goimports does everything that gofmt does. Additionally it checks unused imports
- gomoddirectives # Manage the use of 'replace', 'retract', and 'excludes' directives in go.mod.
- gomodguard # Allow and block list linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations.
- goprintffuncname # Checks that printf-like functions are named with `f` at the end
- gosec # Inspects source code for security problems
- gosimple # Linter for Go source code that specializes in simplifying a code
- govet # Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string
- grouper # An analyzer to analyze expression groups.
- importas # Enforces consistent import aliases
- ineffassign # Detects when assignments to existing variables are not used
- lll # Reports long lines
- maintidx # maintidx measures the maintainability index of each function.
- makezero # Finds slice declarations with non-zero initial length
- misspell # Finds commonly misspelled English words in comments
- nakedret # Finds naked returns in functions greater than a specified function length
- nestif # Reports deeply nested if statements
- nilerr # Finds the code that returns nil even if it checks that the error is not nil.
- nilnil # Checks that there is no simultaneous return of `nil` error and an invalid value.
- nlreturn # nlreturn checks for a new line before return and branch statements to increase code clarity
- noctx # noctx finds sending http request without context.Context
- predeclared # find code that shadows one of Go's predeclared identifiers
- revive # golint replacement, finds style mistakes
- staticcheck # Staticcheck is a go vet on steroids, applying a ton of static analysis checks
- stylecheck # Stylecheck is a replacement for golint
- tagliatelle # Checks the struct tags.
- tenv # tenv is analyzer that detects using os.Setenv instead of t.Setenv since Go1.17
- tparallel # tparallel detects inappropriate usage of t.Parallel() method in your Go test codes
- thelper # thelper detects golang test helpers without t.Helper() call and checks the consistency of test helpers
- typecheck # Like the front-end of a Go compiler, parses and type-checks Go code
- unconvert # Remove unnecessary type conversions
- unparam # Reports unused function parameters
- unused # Checks Go code for unused constants, variables, functions and types
- varnamelen # checks that the length of a variable's name matches its scope
- wastedassign # wastedassign finds wasted assignment statements
- whitespace # Tool for detection of leading and trailing whitespace
disable:
- depguard # Go linter that checks if package imports are in a list of acceptable packages
- containedctx # containedctx is a linter that detects struct contained context.Context field
- cyclop # checks function and package cyclomatic complexity
- funlen # Tool for detection of long functions
- gocyclo # Computes and checks the cyclomatic complexity of functions
- godot # Check if comments end in a period
- gomnd # An analyzer to detect magic numbers.
- gochecknoinits # Checks that no init functions are present in Go code
- gomodguard # Allow and block list linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations.
- interfacebloat # A linter that checks length of interface.
- ireturn # Accept Interfaces, Return Concrete Types
- lll # Reports long lines
- maintidx # maintidx measures the maintainability index of each function.
- makezero # Finds slice declarations with non-zero initial length
- nakedret # Finds naked returns in functions greater than a specified function length
- nestif # Reports deeply nested if statements
- nlreturn # nlreturn checks for a new line before return and branch statements to increase code clarity
- mnd # An analyzer to detect magic numbers
- nolintlint # Reports ill-formed or insufficient nolint directives
- paralleltest # paralleltest detects missing usage of t.Parallel() method in your Go test
- prealloc # Finds slice declarations that could potentially be preallocated
- promlinter # Check Prometheus metrics naming via promlint
- rowserrcheck # checks whether Err of rows is checked successfully
- sqlclosecheck # Checks that sql.Rows and sql.Stmt are closed.
- testpackage # linter that makes you use a separate _test package
- thelper # thelper detects golang test helpers without t.Helper() call and checks the consistency of test helpers
- varnamelen # checks that the length of a variable's name matches its scope
- tparallel # tparallel detects inappropriate usage of t.Parallel() method in your Go test codes
- wrapcheck # Checks that errors returned from external packages are wrapped
- wsl # Whitespace Linter - Forces you to use empty lines!

Expand Down
24 changes: 12 additions & 12 deletions api.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,38 +29,38 @@ type API struct {
// It uses the default Codecs and Interceptors unless you customize them
// using WithMediaEngine and WithInterceptorRegistry respectively.
func NewAPI(options ...func(*API)) *API {
a := &API{
api := &API{
interceptor: &interceptor.NoOp{},
settingEngine: &SettingEngine{},
}

for _, o := range options {
o(a)
o(api)
}

if a.settingEngine.LoggerFactory == nil {
a.settingEngine.LoggerFactory = logging.NewDefaultLoggerFactory()
if api.settingEngine.LoggerFactory == nil {
api.settingEngine.LoggerFactory = logging.NewDefaultLoggerFactory()
}

logger := a.settingEngine.LoggerFactory.NewLogger("api")
logger := api.settingEngine.LoggerFactory.NewLogger("api")

if a.mediaEngine == nil {
a.mediaEngine = &MediaEngine{}
err := a.mediaEngine.RegisterDefaultCodecs()
if api.mediaEngine == nil {
api.mediaEngine = &MediaEngine{}
err := api.mediaEngine.RegisterDefaultCodecs()
if err != nil {
logger.Errorf("Failed to register default codecs %s", err)
}
}

if a.interceptorRegistry == nil {
a.interceptorRegistry = &interceptor.Registry{}
err := RegisterDefaultInterceptors(a.mediaEngine, a.interceptorRegistry)
if api.interceptorRegistry == nil {
api.interceptorRegistry = &interceptor.Registry{}
err := RegisterDefaultInterceptors(api.mediaEngine, api.interceptorRegistry)
if err != nil {
logger.Errorf("Failed to register default interceptors %s", err)
}
}

return a
return api
}

// WithMediaEngine allows providing a MediaEngine to the API.
Expand Down
18 changes: 9 additions & 9 deletions certificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,22 +67,22 @@ func NewCertificate(key crypto.PrivateKey, tpl x509.Certificate) (*Certificate,

// Equals determines if two certificates are identical by comparing both the
// secretKeys and x509Certificates.
func (c Certificate) Equals(o Certificate) bool {
func (c Certificate) Equals(cert Certificate) bool {
switch cSK := c.privateKey.(type) {
case *rsa.PrivateKey:
if oSK, ok := o.privateKey.(*rsa.PrivateKey); ok {
if oSK, ok := cert.privateKey.(*rsa.PrivateKey); ok {
if cSK.N.Cmp(oSK.N) != 0 {
return false
}
return c.x509Cert.Equal(o.x509Cert)
return c.x509Cert.Equal(cert.x509Cert)
}
return false
case *ecdsa.PrivateKey:
if oSK, ok := o.privateKey.(*ecdsa.PrivateKey); ok {
if oSK, ok := cert.privateKey.(*ecdsa.PrivateKey); ok {
if cSK.X.Cmp(oSK.X) != 0 || cSK.Y.Cmp(oSK.Y) != 0 {
return false
}
return c.x509Cert.Equal(o.x509Cert)
return c.x509Cert.Equal(cert.x509Cert)
}
return false
default:
Expand Down Expand Up @@ -213,11 +213,11 @@ func CertificateFromPEM(pems string) (*Certificate, error) {
// certificate and the other for the private key
func (c Certificate) PEM() (string, error) {
// First write the X509 certificate
var o strings.Builder
var builder strings.Builder
xcertBytes := make(
[]byte, base64.StdEncoding.EncodedLen(len(c.x509Cert.Raw)))
base64.StdEncoding.Encode(xcertBytes, c.x509Cert.Raw)
err := pem.Encode(&o, &pem.Block{Type: "CERTIFICATE", Bytes: xcertBytes})
err := pem.Encode(&builder, &pem.Block{Type: "CERTIFICATE", Bytes: xcertBytes})
if err != nil {
return "", fmt.Errorf("failed to pem encode the X certificate: %w", err)
}
Expand All @@ -226,9 +226,9 @@ func (c Certificate) PEM() (string, error) {
if err != nil {
return "", fmt.Errorf("failed to marshal private key: %w", err)
}
err = pem.Encode(&o, &pem.Block{Type: "PRIVATE KEY", Bytes: privBytes})
err = pem.Encode(&builder, &pem.Block{Type: "PRIVATE KEY", Bytes: privBytes})
if err != nil {
return "", fmt.Errorf("failed to encode private key: %w", err)
}
return o.String(), nil
return builder.String(), nil
}
4 changes: 2 additions & 2 deletions configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestConfiguration_getICEServers(t *testing.T) {
}

func TestConfigurationJSON(t *testing.T) {
j := `{
config := `{
"iceServers": [{"urls": ["turn:turn.example.org"],
"username": "jch",
"credential": "topsecret"
Expand All @@ -67,7 +67,7 @@ func TestConfigurationJSON(t *testing.T) {
}

var conf2 Configuration
assert.NoError(t, json.Unmarshal([]byte(j), &conf2))
assert.NoError(t, json.Unmarshal([]byte(config), &conf2))
assert.Equal(t, conf, conf2)

j2, err := json.Marshal(conf2)
Expand Down
12 changes: 6 additions & 6 deletions datachannel.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (api *API) newDataChannel(params *DataChannelParameters, sctpTransport *SCT
return nil, &rtcerr.TypeError{Err: ErrStringSizeLimit}
}

d := &DataChannel{
dataChannel := &DataChannel{
sctpTransport: sctpTransport,
statsID: fmt.Sprintf("DataChannel-%d", time.Now().UnixNano()),
label: params.Label,
Expand All @@ -107,8 +107,8 @@ func (api *API) newDataChannel(params *DataChannelParameters, sctpTransport *SCT
log: log,
}

d.setReadyState(DataChannelStateConnecting)
return d, nil
dataChannel.setReadyState(DataChannelStateConnecting)
return dataChannel, nil
}

// open opens the datachannel over the sctp transport
Expand Down Expand Up @@ -399,11 +399,11 @@ func (d *DataChannel) readLoop() {
return
}

m := DataChannelMessage{Data: make([]byte, n), IsString: isString}
copy(m.Data, buffer[:n])
msg := DataChannelMessage{Data: make([]byte, n), IsString: isString}
copy(msg.Data, buffer[:n])

// NB: Why was DataChannelMessage not passed as a pointer value?
d.onMessage(m) // nolint:staticcheck
d.onMessage(msg) // nolint:staticcheck
}
}

Expand Down
8 changes: 4 additions & 4 deletions datachannel_go_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,14 +301,14 @@ func TestDataChannelBufferedAmount(t *testing.T) {

done := make(chan bool)

answerPC.OnDataChannel(func(d *DataChannel) {
answerPC.OnDataChannel(func(dataChannel *DataChannel) {
// Make sure this is the data channel we were looking for. (Not the one
// created in signalPair).
if d.Label() != expectedLabel {
if dataChannel.Label() != expectedLabel {
return
}
var nPacketsReceived int
d.OnMessage(func(DataChannelMessage) {
dataChannel.OnMessage(func(DataChannelMessage) {
nPacketsReceived++

if nPacketsReceived == 10 {
Expand All @@ -318,7 +318,7 @@ func TestDataChannelBufferedAmount(t *testing.T) {
}()
}
})
assert.True(t, d.Ordered(), "Ordered should be set to true")
assert.True(t, dataChannel.Ordered(), "Ordered should be set to true")
})

dc, err := offerPC.CreateDataChannel(expectedLabel, nil)
Expand Down
8 changes: 4 additions & 4 deletions dtlstransport.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ type simulcastStreamPair struct {
// This constructor is part of the ORTC API. It is not
// meant to be used together with the basic WebRTC API.
func (api *API) NewDTLSTransport(transport *ICETransport, certificates []Certificate) (*DTLSTransport, error) {
t := &DTLSTransport{
trans := &DTLSTransport{
iceTransport: transport,
api: api,
state: DTLSTransportStateNew,
Expand All @@ -84,7 +84,7 @@ func (api *API) NewDTLSTransport(transport *ICETransport, certificates []Certifi
if !x509Cert.Expires().IsZero() && now.After(x509Cert.Expires()) {
return nil, &rtcerr.InvalidAccessError{Err: ErrCertificateExpired}
}
t.certificates = append(t.certificates, x509Cert)
trans.certificates = append(trans.certificates, x509Cert)
}
} else {
sk, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
Expand All @@ -95,10 +95,10 @@ func (api *API) NewDTLSTransport(transport *ICETransport, certificates []Certifi
if err != nil {
return nil, err
}
t.certificates = []Certificate{*certificate}
trans.certificates = []Certificate{*certificate}
}

return t, nil
return trans, nil
}

// ICETransport returns the currently-configured *ICETransport or nil
Expand Down
20 changes: 10 additions & 10 deletions examples/bandwidth-estimation-from-disk/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const (
ivfHeaderSize = 32
)

// nolint: gocognit
// nolint: gocognit, cyclop
func main() {
qualityLevels := []struct {
fileName string
Expand All @@ -58,9 +58,9 @@ func main() {
}
}

i := &interceptor.Registry{}
m := &webrtc.MediaEngine{}
if err := m.RegisterDefaultCodecs(); err != nil {
interceptorRegistry := &interceptor.Registry{}
mediaEngine := &webrtc.MediaEngine{}
if err := mediaEngine.RegisterDefaultCodecs(); err != nil {
panic(err)
}

Expand All @@ -81,17 +81,17 @@ func main() {
estimatorChan <- estimator
})

i.Add(congestionController)
if err = webrtc.ConfigureTWCCHeaderExtensionSender(m, i); err != nil {
interceptorRegistry.Add(congestionController)
if err = webrtc.ConfigureTWCCHeaderExtensionSender(mediaEngine, interceptorRegistry); err != nil {
panic(err)
}

if err = webrtc.RegisterDefaultInterceptors(m, i); err != nil {
if err = webrtc.RegisterDefaultInterceptors(mediaEngine, interceptorRegistry); err != nil {
panic(err)
}

// Create a new RTCPeerConnection
peerConnection, err := webrtc.NewAPI(webrtc.WithInterceptorRegistry(i), webrtc.WithMediaEngine(m)).NewPeerConnection(webrtc.Configuration{
peerConnection, err := webrtc.NewAPI(webrtc.WithInterceptorRegistry(interceptorRegistry), webrtc.WithMediaEngine(mediaEngine)).NewPeerConnection(webrtc.Configuration{
ICEServers: []webrtc.ICEServer{
{
URLs: []string{"stun:stun.l.google.com:19302"},
Expand Down Expand Up @@ -141,8 +141,8 @@ func main() {

// Set the handler for Peer connection state
// This will notify you when the peer has connected/disconnected
peerConnection.OnConnectionStateChange(func(s webrtc.PeerConnectionState) {
fmt.Printf("Peer Connection State has changed: %s\n", s.String())
peerConnection.OnConnectionStateChange(func(state webrtc.PeerConnectionState) {
fmt.Printf("Peer Connection State has changed: %s\n", state.String())
})

// Wait for the offer to be pasted
Expand Down
Loading
Loading