Add golangci-lint (#134)

* add golangci-lint

* fix linter errors

* disable indent-error-flow and modifies-value-server in golangci and minor fixes

* add inline documentation to RPCCost, fix struct tag formatting

* fix tag

* fix lint on windows

.github/workflows/test.yml

@@ -15,13 +15,20 @@ jobs:
     runs-on: ${{ matrix.os }}
     timeout-minutes: 10
     steps:
+      - name: Configure git # required for golangci-lint on Windows
+        shell: bash
+        run: git config --global core.autocrlf false
       - uses: actions/checkout@v3
       - uses: actions/setup-go@v3
         with:
           go-version: ${{ matrix.go-version }}
+      - name: Lint
+        uses: golangci/golangci-lint-action@v3
+        with:
+          skip-cache: true
       - name: test
         uses: n8maninger/action-golang-test@v1
         with:
           args: "-race"
           skip-go-install: true
-          show-package-output: true
+          show-package-output: true

.golangci.yml

@@ -0,0 +1,163 @@
+# Based off of the example file at https://github.com/golangci/golangci-lint
+
+# options for analysis running
+run:
+  # default concurrency is a available CPU number
+  concurrency: 4
+
+  # timeout for analysis, e.g. 30s, 5m, default is 1m
+  timeout: 600s
+
+  # exit code when at least one issue was found, default is 1
+  issues-exit-code: 1
+
+  # include test files or not, default is true
+  tests: true
+
+  # list of build tags, all linters use it. Default is empty list.
+  build-tags: []
+
+  # which dirs to skip: issues from them won't be reported;
+  # can use regexp here: generated.*, regexp is applied on full path;
+  # default value is empty list, but default dirs are skipped independently
+  # from this option's value (see skip-dirs-use-default).
+  skip-dirs:
+    - cover
+
+  # default is true. Enables skipping of directories:
+  #   vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
+  skip-dirs-use-default: true
+
+  # which files to skip: they will be analyzed, but issues from them
+  # won't be reported. Default value is empty list, but there is
+  # no need to include all autogenerated files, we confidently recognize
+  # autogenerated files. If it's not please let us know.
+  skip-files: []
+
+# output configuration options
+output:
+  # colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number"
+  format: colored-line-number
+
+  # print lines of code with issue, default is true
+  print-issued-lines: true
+
+  # print linter name in the end of issue text, default is true
+  print-linter-name: true
+
+# all available settings of specific linters
+linters-settings:
+  ## Enabled linters:
+  govet:
+    # report about shadowed variables
+    check-shadowing: false
+    disable-all: false
+
+  tagliatelle:
+    case:
+      rules:
+        json: goCamel
+        yaml: goCamel
+
+
+  gocritic:
+    # Which checks should be enabled; can't be combined with 'disabled-checks';
+    # See https://go-critic.github.io/overview#checks-overview
+    # To check which checks are enabled run `GL_DEBUG=gocritic golangci-lint run`
+    # By default list of stable checks is used.
+    enabled-checks:
+      - argOrder # Diagnostic options
+      - badCond
+      - caseOrder
+      - dupArg
+      - dupBranchBody
+      - dupCase
+      - dupSubExpr
+      - nilValReturn
+      - offBy1
+      - weakCond
+      - boolExprSimplify # Style options here and below.
+      - builtinShadow
+      - emptyFallthrough
+      - hexLiteral
+      - underef
+      - equalFold
+  revive:
+    ignore-generated-header: true
+    rules:
+      - name: blank-imports
+        disabled: false
+      - name: bool-literal-in-expr
+        disabled: false
+      - name: confusing-naming
+        disabled: false
+      - name: confusing-results
+        disabled: false
+      - name: constant-logical-expr
+        disabled: false
+      - name: context-as-argument
+        disabled: false
+      - name: exported
+        disabled: false
+      - name: errorf
+        disabled: false
+      - name: if-return
+        disabled: false
+      - name: indent-error-flow
+        disabled: true
+      - name: increment-decrement
+        disabled: false
+      - name: modifies-value-receiver
+        disabled: true
+      - name: optimize-operands-order
+        disabled: false
+      - name: range-val-in-closure
+        disabled: false
+      - name: struct-tag
+        disabled: false
+      - name: superfluous-else
+        disabled: false
+      - name: time-equal
+        disabled: false
+      - name: unexported-naming
+        disabled: false
+      - name: unexported-return
+        disabled: false
+      - name: unnecessary-stmt
+        disabled: false
+      - name: unreachable-code
+        disabled: false
+      - name: package-comments
+        disabled: true
+
+linters:
+  disable-all: true
+  fast: false
+  enable:
+    - tagliatelle
+    - gocritic
+    - gofmt
+    - revive
+    - govet
+    - misspell
+    - typecheck
+    - whitespace
+
+issues:
+  # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
+  max-issues-per-linter: 0
+
+  # Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
+  max-same-issues: 0
+
+  # List of regexps of issue texts to exclude, empty list by default.
+  # But independently from this option we use default exclude patterns,
+  # it can be disabled by `exclude-use-default: false`. To list all
+  # excluded by default patterns execute `golangci-lint run --help`
+  exclude: []
+
+  # Independently from option `exclude` we use default exclude patterns,
+  # it can be disabled by this option. To list all
+  # excluded by default patterns execute `golangci-lint run --help`.
+  # Default value for this option is true.
+  exclude-use-default: false

chain/db.go

@@ -44,7 +44,6 @@ type versionedState struct {
 func (vs versionedState) EncodeTo(e *types.Encoder) {
 	e.WriteUint8(2)
 	vs.State.EncodeTo(e)
-
 }
 
 func (vs *versionedState) DecodeFrom(d *types.Decoder) {

chain/manager.go

@@ -858,7 +858,7 @@ func (m *Manager) RecommendedFee() types.Currency {
 		Mul64(estPoolWeight).Div64(targetWeight).Mul64(estPoolWeight).
 		Div64(targetWeight).Mul64(estPoolWeight).Div64(targetWeight)
 
-	// finally, an absolute minumum fee: 1 SC / 100 KB
+	// finally, an absolute minimum fee: 1 SC / 100 KB
 	minFee := types.Siacoins(1).Div64(100e3)
 
 	// use the largest of all calculated fees

consensus/state.go

@@ -67,6 +67,7 @@ type Network struct {
 		FixHeight        uint64    `json:"fixHeight"`
 		GenesisTimestamp time.Time `json:"genesisTimestamp"`
 	} `json:"hardforkOak"`
+	//nolint:tagliatelle
 	HardforkASIC struct {
 		Height    uint64        `json:"height"`
 		OakTime   time.Duration `json:"oakTime"`

consensus/validation_test.go

@@ -1030,7 +1030,7 @@ func TestValidateV2Block(t *testing.T) {
 	}
 
 	cs, testCau := consensus.ApplyBlock(cs, validBlock, dbStore.SupplementTipBlock(validBlock), time.Now())
-	if dbStore.ApplyBlock(cs, testCau, true) != true {
+	if !dbStore.ApplyBlock(cs, testCau, true) {
 		t.Fatal("didn't commit block")
 	}
 	updateProofs(testCau, sces, sfes, fces)
@@ -1070,7 +1070,7 @@ func TestValidateV2Block(t *testing.T) {
 			t.Fatal(err)
 		}
 		cs, cau = consensus.ApplyBlock(cs, b, dbStore.SupplementTipBlock(validBlock), time.Now())
-		if dbStore.ApplyBlock(cs, cau, true) != true {
+		if !dbStore.ApplyBlock(cs, cau, true) {
 			t.Fatal("didn't commit block")
 		}
 
@@ -1135,7 +1135,7 @@ func TestValidateV2Block(t *testing.T) {
 				func(b *types.Block) {
 					txn := &b.V2.Transactions[0]
 					rev := testFces[0].V2FileContract
-					rev.RevisionNumber += 1
+					rev.RevisionNumber++
 					txn.FileContractRevisions = []types.V2FileContractRevision{{
 						Parent:   testFces[0],
 						Revision: rev,
@@ -1289,8 +1289,8 @@ func TestValidateV2Transaction(t *testing.T) {
 					Index: types.ChainIndex{Height: uint64(0)},
 					Network: &consensus.Network{
 						HardforkV2: struct {
-							AllowHeight   uint64 "json:\"allowHeight\""
-							RequireHeight uint64 "json:\"requireHeight\""
+							AllowHeight   uint64 `json:"allowHeight"`
+							RequireHeight uint64 `json:"requireHeight"`
 						}{
 							AllowHeight: uint64(2),
 						},

internal/smux/stream.go

@@ -147,10 +147,7 @@ func (s *Stream) SetDeadline(t time.Time) error {
 	if err := s.SetReadDeadline(t); err != nil {
 		return err
 	}
-	if err := s.SetWriteDeadline(t); err != nil {
-		return err
-	}
-	return nil
+	return s.SetWriteDeadline(t)
 }
 
 // session closes the stream

rhp/v2/rhp.go

@@ -303,6 +303,7 @@ type (
 	}
 )
 
+// RPCCost represents the cost of an RPC based on a hosts settings.
 type RPCCost struct {
 	Base       types.Currency
 	Storage    types.Currency
@@ -311,6 +312,7 @@ type RPCCost struct {
 	Collateral types.Currency
 }
 
+// Add adds two RPCCosts by adding each of the constituent costs together.
 func (c RPCCost) Add(o RPCCost) RPCCost {
 	return RPCCost{
 		Base:       c.Base.Add(o.Base),
@@ -321,6 +323,7 @@ func (c RPCCost) Add(o RPCCost) RPCCost {
 	}
 }
 
+// Total returns the total cost and collateral required for an RPC call.
 func (c RPCCost) Total() (cost, collateral types.Currency) {
 	return c.Base.Add(c.Storage).Add(c.Ingress).Add(c.Egress), c.Collateral
 }

rhp/v3/registry.go

@@ -94,7 +94,7 @@ func RegistryHostID(pk types.PublicKey) types.Hash256 {
 func ValidateRegistryEntry(re RegistryEntry) (err error) {
 	switch re.Type {
 	case EntryTypeArbitrary:
-		break // no extra validation required
+		// no extra validation required
 	case EntryTypePubKey:
 		// pub key entries have the first 20 bytes of the host's pub key hash
 		// prefixed to the data.

types/types.go

@@ -179,7 +179,7 @@ type TransactionID Hash256
 // A ChainIndex pairs a block's height with its ID.
 type ChainIndex struct {
 	Height uint64  `json:"height"`
-	ID     BlockID `json:"ID"`
+	ID     BlockID `json:"id"`
 }
 
 // A SiacoinOutput is the recipient of some of the siacoins spent in a