multi: make ci/linter happy

- Use dockerized linter
- Run ci with Go 1.18.2
- golangci: match the same settings that we have in LND

Author: positiveblue

.github/workflows/main.yml

@@ -16,7 +16,7 @@ env:
   # go needs absolute directories, using the $HOME variable doesn't work here.
   GOCACHE: /home/runner/work/go/pkg/build
   GOPATH: /home/runner/work/go
-  GO_VERSION: 1.16.x
+  GO_VERSION: 1.18.x
 
 jobs:
   build:

.golangci.yml

@@ -9,23 +9,94 @@ linters-settings:
   gofmt:
     # simplify code: gofmt with `-s` option, true by default
     simplify: true
+  tagliatelle:
+    case:
+      rules:
+        json: snake
+  whitespace:
+    multi-func: true
+    multi-if: true
+  gosec:
+    excludes:
+      - G402 # Look for bad TLS connection settings.
+      - G306 # Poor file permissions used when writing to a new file.
+  staticcheck: 
+    go: "1.18"
+    checks: ["-SA1019"]
 
 linters:
   enable-all: true
   disable:
-    # Init functions are used by loggers throughout the codebase.
-    - gochecknoinits
+  # Global variables are used in many places throughout the code base.
+  - gochecknoglobals
 
-    # Global variables are used by loggers.
-    - gochecknoglobals
+  # Some lines are over 80 characters on purpose and we don't want to make them
+  # even longer by marking them as 'nolint'.
+  - lll
 
-    # Some lines are over 80 characters on purpose and we don't want to make
-    # them even longer by marking them as 'nolint'.
-    - lll
+  # We want to allow short variable names.
+  - varnamelen
 
-    # We don't care (enough) about misaligned structs to lint that.
-    - maligned
+  # We want to allow TODOs.
+  - godox
+
+  # We have long functions, especially in tests. Moving or renaming those would
+  # trigger funlen problems that we may not want to solve at that time.
+  - funlen
+
+  # Disable for now as we haven't yet tuned the sensitivity to our codebase
+  # yet. Enabling by default for example, would also force new contributors to
+  # potentially extensively refactor code, when they want to smaller change to
+  # land.
+  - gocyclo
+  - gocognit
+  - cyclop
+
+  # Instances of table driven tests that don't pre-allocate shouldn't trigger
+  # the linter.
+  - prealloc
+
+  # Init functions are used by loggers throughout the codebase.
+  - gochecknoinits
+  
+  # Causes stack overflow, see https://github.com/polyfloyd/go-errorlint/issues/19.
+  - errorlint
+
+  # Deprecated linters. See https://golangci-lint.run/usage/linters/.
+  - interfacer
+  - golint
+  - maligned
+  - scopelint
+  
+  # New linters that need a code adjustment first.
+  - wrapcheck
+  - nolintlint
+  - paralleltest
+  - tparallel
+  - testpackage
+  - gofumpt
+  - gomoddirectives
+  - ireturn
+  - maintidx
+  - nlreturn
+  - dogsled
+  - gci
+  - containedctx
+  - contextcheck
+  - errname
+  - exhaustivestruct
+  - goerr113
+  - gomnd
+  - ifshort
+  - noctx
+  - nestif
+  - wsl
+  - exhaustive
+  - forcetypeassert
+  - nilerr
+  - nilnil
+  - stylecheck
+  - thelper
+  - exhaustruct
+  - importas
 
-    # We have long functions, especially in tests. Moving or renaming those
-    # would trigger funlen problems that we may not want to solve at that time.
-    - funlen

Makefile

@@ -1,13 +1,12 @@
 PKG := github.com/lightninglabs/lndclient
 
-LINT_PKG := github.com/golangci/golangci-lint/cmd/golangci-lint
+TOOLS_DIR := tools
 
-GO_BIN := ${GOPATH}/bin
-LINT_BIN := $(GO_BIN)/golangci-lint
+GOIMPORTS_PKG := github.com/rinchsan/gosimports/cmd/gosimports
 
-LINT_COMMIT := v1.18.0
+GO_BIN := ${GOPATH}/bin
+GOIMPORTS_BIN := $(GO_BIN)/gosimports
 
-DEPGET := cd /tmp && go get -v
 GOBUILD := go build -v
 GOINSTALL := go install -v
 GOTEST := go test -v
@@ -29,7 +28,7 @@ ifneq ($(workers),)
 LINT_WORKERS = --concurrency=$(workers)
 endif
 
-LINT = $(LINT_BIN) run -v $(LINT_WORKERS)
+DOCKER_TOOLS = docker run -v $$(pwd):/build lndclient-tools
 
 GREEN := "\\033[0;32m"
 NC := "\\033[0m"
@@ -44,9 +43,9 @@ all: build check install
 # ============
 # DEPENDENCIES
 # ============
-$(LINT_BIN):
-	@$(call print, "Fetching linter")
-	$(DEPGET) $(LINT_PKG)@$(LINT_COMMIT)
+$(GOIMPORTS_BIN):
+	@$(call print, "Installing goimports.")
+	cd $(TOOLS_DIR); go install -trimpath $(GOIMPORTS_PKG)
 
 # ============
 # INSTALLATION
@@ -56,6 +55,10 @@ build:
 	@$(call print, "Building lndclient.")
 	$(GOBUILD) -ldflags="$(LDFLAGS)" $(PKG)
 
+docker-tools:
+	@$(call print, "Building tools docker image.")
+	docker build -q -t lndclient-tools $(TOOLS_DIR)
+
 # =======
 # TESTING
 # =======
@@ -73,13 +76,15 @@ unit-race:
 # =========
 # UTILITIES
 # =========
-fmt:
+fmt: $(GOIMPORTS_BIN)
+	@$(call print, "Fixing imports.")
+	gosimports -w $(GOFILES_NOVENDOR)
 	@$(call print, "Formatting source.")
 	gofmt -l -w -s $(GOFILES_NOVENDOR)
 
-lint: $(LINT_BIN)
+lint: docker-tools
 	@$(call print, "Linting source.")
-	$(LINT)
+	$(DOCKER_TOOLS) golangci-lint run -v $(LINT_WORKERS)
 
 .PHONY: default \
 	build \

basic_client_test.go

@@ -12,7 +12,6 @@ import (
 // Tests that NewBasicConn works correctly when macaroon and TLS certificate
 //  data are passed in directly instead of being supplied as file paths.
 func TestParseTLSAndMacaroon(t *testing.T) {
-
 	tlsData := `-----BEGIN CERTIFICATE-----
 MIIDhzCCAm+gAwIBAgIUEkmdMOVPL92AwgsSYFFBvz4ilmUwDQYJKoZIhvcNAQEL
 BQAwUzELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAk1OMRQwEgYDVQQHDAtNaW5uZWFw

chainnotifier_client.go

@@ -162,7 +162,6 @@ func (s *chainNotifierClient) RegisterConfirmationsNtfn(ctx context.Context,
 			}
 
 			switch c := confEvent.Event.(type) {
-
 			// Script confirmed
 			case *chainrpc.ConfEvent_Conf:
 				tx, err := decodeTx(c.Conf.RawTx)

lightning_client.go

@@ -352,6 +352,7 @@ type ChannelInfo struct {
 
 func (s *lightningClient) newChannelInfo(channel *lnrpc.Channel) (*ChannelInfo,
 	error) {
+
 	remoteVertex, err := route.NewVertexFromStr(channel.RemotePubkey)
 	if err != nil {
 		return nil, err
@@ -1339,9 +1340,8 @@ func (s *lightningClient) payInvoice(ctx context.Context, invoice string,
 
 		if err == nil {
 			// TODO: Use structured payment error when available,
-			// instead of this britle string matching.
+			// instead of this brittle string matching.
 			switch payResp.PaymentError {
-
 			// Paid successfully.
 			case PaymentResultSuccess:
 				log.Infof(

lightning_client_test.go

@@ -81,11 +81,13 @@ func TestLightningClientAddInvoice(t *testing.T) {
 
 	validAddInvoice := func(in *lnrpc.Invoice, opts ...grpc.CallOption) (
 		*lnrpc.AddInvoiceResponse, error) {
+
 		return validResp, nil
 	}
 
 	errorAddInvoice := func(in *lnrpc.Invoice, opts ...grpc.CallOption) (
 		*lnrpc.AddInvoiceResponse, error) {
+
 		return nil, errors.New("error")
 	}
 

lnd_services_test.go

@@ -5,12 +5,10 @@ import (
 	"errors"
 	"testing"
 
-	"google.golang.org/grpc"
-
 	"github.com/lightningnetwork/lnd/lnrpc"
-
 	"github.com/lightningnetwork/lnd/lnrpc/verrpc"
 	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )

router_client.go

@@ -398,8 +398,8 @@ func (r *routerClient) SendPayment(ctx context.Context,
 
 	if request.KeySend {
 		if request.PaymentHash != nil {
-			return nil, nil, fmt.Errorf(
-				"keysend payment must not include a preset payment hash")
+			return nil, nil, fmt.Errorf("keysend payment must not " +
+				"include a preset payment hash")
 		}
 
 		var preimage lntypes.Preimage
@@ -484,7 +484,6 @@ func (r *routerClient) trackPayment(ctx context.Context,
 				}
 
 				switch status.Convert(err).Code() {
-
 				// NotFound is only expected as a response to
 				// TrackPayment.
 				case codes.NotFound:
@@ -674,10 +673,10 @@ func (r *routerClient) SubscribeHtlcEvents(ctx context.Context) (
 // must be thread-safe.
 //
 // There are a few ways in which this method can exit:
-// - ctx canceled: the calling client cancels
-// - r.quit: the router is shut down
-// - lnd stream error: lnd has exited
-// - handler error: a critical error occurred while handing a htlc
+// - ctx canceled: the calling client cancels.
+// - r.quit: the router is shut down.
+// - lnd stream error: lnd has exited.
+// - handler error: a critical error occurred while handing a htlc.
 func (r *routerClient) InterceptHtlcs(ctx context.Context,
 	handler HtlcInterceptHandler) error {
 

tools/Dockerfile

@@ -0,0 +1,16 @@
+FROM golang:1.18.5-buster
+
+RUN apt-get update && apt-get install -y git
+ENV GOCACHE=/tmp/build/.cache
+ENV GOMODCACHE=/tmp/build/.modcache
+
+COPY . /tmp/tools
+
+RUN cd /tmp \
+  && mkdir -p /tmp/build/.cache \
+  && mkdir -p /tmp/build/.modcache \
+  && cd /tmp/tools \
+  && go install -trimpath -tags=tools github.com/golangci/golangci-lint/cmd/golangci-lint \
+  && chmod -R 777 /tmp/build/
+
+WORKDIR /build