Skip to content

Commit

Permalink
Update linters and fix reported issues
Browse files Browse the repository at this point in the history
Removed linters:
  - deadcode (obsolete)
  - structcheck (obsolete)
  - varcheck (obsolete)

Enabled linters:
  - asciicheck
  - bidichk
  - decorder
  - dogsled
  - dupl
  - durationcheck
  - errchkjson
  - errname
  - goconst
  - gocritic
  - gofumpt
  - gomoddirectives
  - gomodguard
  - grouper
  - nilerr
  - nilnil
  - predeclared
  - unparam
  • Loading branch information
Danielius1922 committed Sep 8, 2022
1 parent a66d3c9 commit 72f5679
Show file tree
Hide file tree
Showing 42 changed files with 209 additions and 171 deletions.
4 changes: 3 additions & 1 deletion .github/workflows/golangci-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ name: Golangci-lint
# has dozens of linters included.
# see: https://github.com/golangci/golangci-lint-action

on: [pull_request]
on:
pull_request:
workflow_dispatch:
jobs:
golangci:
name: lint
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/staticAnalysis.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
# Run static analysis checks
name: Static Analysis

on: [pull_request]
on:
pull_request:
workflow_dispatch:
jobs:
analysis:
runs-on: ubuntu-latest
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test-for-fork.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Workflow to run tests for PRs from forked repository
name: Test for forked repository

# Run on push to whatever branch
# Run on pull requests events from forked repository
on: [pull_request]

jobs:
Expand Down
18 changes: 10 additions & 8 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
# This is a basic workflow to help you get started with Actions

# Workflow to run tests, publish coverage to codecov and run SonarCloud scan
name: Test

# Controls when the action will run. Triggers the workflow on push or pull request
# events but only for the master branch
# Run for events in main repository (for forked repository look in test-for-fork.yml)
on:
push:
branches:
- main
pull_request:
workflow_dispatch:

# A workflow run is made up of one or more jobs that can run sequentially or in parallel
jobs:
# This workflow contains a single job called "build"
test:
# The type of runner that the job will run on
# don't run for forks
if: github.event_name == 'push' ||
(github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository) ||
github.event_name == 'workflow_dispatch'
runs-on: ubuntu-latest
strategy:
matrix:
Expand All @@ -23,7 +27,6 @@ jobs:
coverage: true
tag: ghcr.io/iotivity/iotivity-lite/cloud-server-discovery-resource-observable-debug:master

# Steps represent a sequence of tasks that will be executed as part of the job
steps:
# Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it
- uses: actions/checkout@v3
Expand All @@ -49,4 +52,3 @@ jobs:
- name: Publish the coverage
if: ${{ matrix.coverage }}
run: bash <(curl -s https://codecov.io/bash)

45 changes: 23 additions & 22 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,18 @@ linters-settings:

linters:
enable:
# - asciicheck # Simple linter to check that your code does not contain non-ASCII identifiers
# - bidichk # Checks for dangerous unicode character sequences
- 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
# - contextcheck # check the function whether use a non-inherited context
- deadcode # Finds unused code
# - decorder # check declaration order and count of types, constants, variables and functions
- decorder # check declaration order and count of types, constants, variables and functions
- depguard # Go linter that checks if package imports are in a list of acceptable packages
# - 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
- 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
# - 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`.
- 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`.
- errorlint # errorlint is a linter for that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13.
# - exhaustive # check exhaustiveness of enum switch statements
- exportloopref # checks for pointers to enclosing loop variables
Expand All @@ -37,43 +36,41 @@ linters:
# - 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
- 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
# - godox # Tool for detection of FIXME, TODO and other comment keywords
# - goerr113 # 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.
- 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.
- 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.
- grouper # An analyzer to analyze expression groups.
- importas # Enforces consistent import aliases
- ineffassign # Detects when assignments to existing variables are not used
- misspell # Finds commonly misspelled English words in comments
# - nakedret # Finds naked returns in functions greater than a specified function length
# - 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.
- 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.
# - noctx # noctx finds sending http request without context.Context
- nolintlint # Reports ill-formed or insufficient nolint directives
# - paralleltest # paralleltest detects missing usage of t.Parallel() method in your Go test
# - predeclared # find code that shadows one of Go's predeclared identifiers
- 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
# - structcheck # Finds unused struct fields
# - stylecheck # Stylecheck is a replacement for golint
# - 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
- 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
- unparam # Reports unused function parameters
- unused # Checks Go code for unused constants, variables, functions and types
- varcheck # Finds unused global variables and constants
# - wastedassign # wastedassign finds wasted assignment statements
- whitespace # Tool for detection of leading and trailing whitespace
disable:
Expand All @@ -96,7 +93,6 @@ linters:
- 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.
- structcheck # Finds unused struct fields
- tagliatelle # Checks the struct tags.
- 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
Expand All @@ -118,6 +114,11 @@ issues:
linters:
- gocognit

- path: pkg/error
text: "package name error has same name as predeclared identifier"
linters:
- predeclared

# Fix found issues (if it's supported by the linter).
# fix: true

Expand Down
18 changes: 9 additions & 9 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,21 +195,21 @@ func (c *Client) popSubscriptions() map[string]subscription {
return s
}

func (c *Client) popSubscription(ID string) (subscription, error) {
func (c *Client) popSubscription(id string) (subscription, error) {
c.subscriptionsLock.Lock()
defer c.subscriptionsLock.Unlock()
v, ok := c.subscriptions[ID]
v, ok := c.subscriptions[id]
if !ok {
return nil, fmt.Errorf("cannot find observation %v", ID)
return nil, fmt.Errorf("cannot find observation %v", id)
}
delete(c.subscriptions, ID)
delete(c.subscriptions, id)
return v, nil
}

func (c *Client) insertSubscription(ID string, s subscription) {
func (c *Client) insertSubscription(id string, s subscription) {
c.subscriptionsLock.Lock()
defer c.subscriptionsLock.Unlock()
c.subscriptions[ID] = s
c.subscriptions[id] = s
}

func (c *Client) CoreClient() *core.Client {
Expand All @@ -231,13 +231,13 @@ func NewDeviceOwnerFromConfig(cfg *Config, dialTLS core.DialTLS, dialDTLS core.D
return nil, fmt.Errorf("cannot create sdk signers: %w", err)
}
return c, nil
} else if cfg.DeviceOwnershipBackend != nil {
}
if cfg.DeviceOwnershipBackend != nil {
c, err := NewDeviceOwnershipBackendFromConfig(app, dialTLS, dialDTLS, cfg.DeviceOwnershipBackend, errors)
if err != nil {
return nil, fmt.Errorf("cannot create server signers: %w", err)
}
return c, nil
} else {
return NewDeviceOwnershipNone(), nil
}
return NewDeviceOwnershipNone(), nil
}
4 changes: 1 addition & 3 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,4 @@ func disown(t *testing.T, c *client.Client, deviceID string) {
require.NoError(t, err)
}

var (
CertIdentity = "00000000-0000-0000-0000-000000000001"
)
var CertIdentity = "00000000-0000-0000-0000-000000000001"
2 changes: 1 addition & 1 deletion client/core/certificateSigner.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ import (
)

type CertificateSigner = interface {
//csr is encoded by PEM and returns PEM
// csr is encoded by PEM and returns PEM
Sign(ctx context.Context, csr []byte) ([]byte, error)
}
10 changes: 6 additions & 4 deletions client/core/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,12 @@ func WithErr(errFunc ErrFunc) OptionFunc {
}
}

type DialDTLS = func(ctx context.Context, addr string, dtlsCfg *dtls.Config, opts ...coap.DialOptionFunc) (*coap.ClientCloseHandler, error)
type DialTLS = func(ctx context.Context, addr string, tlsCfg *tls.Config, opts ...coap.DialOptionFunc) (*coap.ClientCloseHandler, error)
type DialUDP = func(ctx context.Context, addr string, opts ...coap.DialOptionFunc) (*coap.ClientCloseHandler, error)
type DialTCP = func(ctx context.Context, addr string, opts ...coap.DialOptionFunc) (*coap.ClientCloseHandler, error)
type (
DialDTLS = func(ctx context.Context, addr string, dtlsCfg *dtls.Config, opts ...coap.DialOptionFunc) (*coap.ClientCloseHandler, error)
DialTLS = func(ctx context.Context, addr string, tlsCfg *tls.Config, opts ...coap.DialOptionFunc) (*coap.ClientCloseHandler, error)
DialUDP = func(ctx context.Context, addr string, opts ...coap.DialOptionFunc) (*coap.ClientCloseHandler, error)
DialTCP = func(ctx context.Context, addr string, opts ...coap.DialOptionFunc) (*coap.ClientCloseHandler, error)
)

func WithDialDTLS(dial DialDTLS) OptionFunc {
return func(cfg config) config {
Expand Down
7 changes: 3 additions & 4 deletions client/core/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ func NewTestSecureClientWithCert(cert tls.Certificate, disableDTLS, disableTCPTL
},
GetCertificateAuthorities: func() ([]*x509.Certificate, error) {
return identityIntermediateCA, nil
}}),
},
}),
)

c := core.NewClient(opts...)
Expand Down Expand Up @@ -152,6 +153,4 @@ func (c *Client) Close() error {
return c.Device.Close(timeout)
}

var (
CertIdentity = "00000000-0000-0000-0000-000000000001"
)
var CertIdentity = "00000000-0000-0000-0000-000000000001"
8 changes: 3 additions & 5 deletions client/core/device.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,15 +137,13 @@ func (d *Device) popConnections() []*conn {
// Close closes open connections to the device.
func (d *Device) Close(ctx context.Context) error {
var errs []error
err := d.stopObservations(ctx)
if err != nil {
if err := d.stopObservations(ctx); err != nil {
errs = append(errs, err)
}

for _, conn := range d.popConnections() {
err = conn.Close()
if err != nil && !errors.Is(err, goNet.ErrClosed) {
errs = append(errs, err)
if errC := conn.Close(); errC != nil && !errors.Is(errC, goNet.ErrClosed) {
errs = append(errs, errC)
}
// wait for closing socket
<-conn.Done()
Expand Down
7 changes: 4 additions & 3 deletions client/core/disownDevice.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func connectionWasClosed(ctx context.Context, err error) bool {
return false
}

// DisownDevice remove ownership of device
// DisownDevice removes ownership of device
func (d *Device) Disown(
ctx context.Context,
links schema.ResourceLinks,
Expand Down Expand Up @@ -65,7 +65,8 @@ func (d *Device) Disown(

return MakeInternal(cannotDisownErr(err))
}
d.Close(ctx)

if errC := d.Close(ctx); errC != nil {
d.cfg.ErrFunc(fmt.Errorf("device disown error: %w", errC))
}
return nil
}
12 changes: 9 additions & 3 deletions client/core/getDevice.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ func (c *Client) GetDeviceByIP(ctx context.Context, ip string) (*Device, error)
}
defer func() {
for _, conn := range multicastConn {
conn.Close()
if errC := conn.Close(); errC != nil {
c.errFunc(fmt.Errorf("get device by ip error: cannot close connection(%s): %w", conn.mcastaddr, errC))
}
}
}()

Expand Down Expand Up @@ -60,7 +62,9 @@ func (c *Client) GetDeviceByMulticast(ctx context.Context, deviceID string, disc
}
defer func() {
for _, conn := range multicastConn {
conn.Close()
if errC := conn.Close(); errC != nil {
c.errFunc(fmt.Errorf("get device by multicast error: cannot close connection(%s): %w", conn.mcastaddr, errC))
}
}
}()

Expand Down Expand Up @@ -109,7 +113,9 @@ func (h *deviceHandler) Device() *Device {
}

func (h *deviceHandler) Handle(ctx context.Context, conn *client.ClientConn, links schema.ResourceLinks) {
conn.Close()
if errC := conn.Close(); errC != nil {
h.deviceCfg.ErrFunc(fmt.Errorf("device handler cannot close connection: %w", errC))
}
h.lock.Lock()
defer h.lock.Unlock()

Expand Down
12 changes: 9 additions & 3 deletions client/core/getDevices.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ func (h deprecatedDeviceHandler) Handle(ctx context.Context, device *Device) {
eps := device.GetEndpoints()
links, err := device.GetResourceLinks(ctx, eps)
if err != nil {
device.Close(ctx)
if errC := device.Close(ctx); errC != nil {
h.Error(errC)
}
h.Error(err)
return
}
Expand Down Expand Up @@ -64,7 +66,9 @@ func (c *Client) GetDevicesV2(ctx context.Context, discoveryConfiguration Discov
}
defer func() {
for _, conn := range multicastConn {
conn.Close()
if errC := conn.Close(); errC != nil {
c.errFunc(fmt.Errorf("get devices error: cannot close connection(%s): %w", conn.mcastaddr, errC))
}
}
}()
// we want to just get "oic.wk.d" resource, because links will be get via unicast to /oic/res
Expand All @@ -88,7 +92,9 @@ type discoveryHandler struct {
}

func (h *discoveryHandler) Handle(ctx context.Context, conn *client.ClientConn, links schema.ResourceLinks) {
conn.Close()
if errC := conn.Close(); errC != nil {
h.handler.Error(fmt.Errorf("discovery handler cannot close connection: %w", errC))
}
link, err := GetResourceLink(links, device.ResourceURI)
if err != nil {
h.handler.Error(err)
Expand Down
3 changes: 1 addition & 2 deletions client/core/getDevices_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ func TestDeviceDiscovery(t *testing.T) {
require.NoError(t, err)
}

type testDeviceHandler struct {
}
type testDeviceHandler struct{}

func (h *testDeviceHandler) Handle(ctx context.Context, d *core.Device) {
defer func() {
Expand Down
Loading

0 comments on commit 72f5679

Please sign in to comment.