Merge pull request #3 from scaleway/feature/linter

Feature/linter

Author: luxifer

.github/workflows/go.yml

@@ -21,5 +21,11 @@ jobs:
       with:
         go-version: '1.23'
 
+    - name: Install golangci-lint
+      run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin
+
+    - name: Lint
+      run: make golint
+
     - name: Test
       run: make gotest

.gitignore

@@ -10,6 +10,8 @@
 
 # Test binary, built with `go test -c`
 *.test
+coverage.txt
+coverage/
 
 # Output of the go coverage tool, specifically when used with LiteIDE
 *.out

.golangci.yml

@@ -0,0 +1,239 @@
+# 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: 20m
+
+  # 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
+
+  # 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 exclude-dirs-use-default).
+  exclude-dirs:
+    - third_party
+    - local
+    - cmd/otelcontribcol
+    - cmd/oteltestbedcol
+
+  # default is true. Enables skipping of directories:
+  #   vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
+  exclude-dirs-use-default: false
+
+  # 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:
+
+  # by default isn't set. If set we pass it to "go list -mod={option}". From "go help modules":
+  # If invoked with -mod=readonly, the go command is disallowed from the implicit
+  # automatic updating of go.mod described above. Instead, it fails when any changes
+  # to go.mod are needed. This setting is most useful to check that go.mod does
+  # not need updates, such as in a continuous integration and testing system.
+  # If invoked with -mod=vendor, the go command assumes that the vendor
+  # directory holds the correct copies of dependencies and ignores
+  # the dependency descriptions in go.mod.
+  modules-download-mode: readonly
+
+# output configuration options
+output:
+  # colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number"
+  formats: 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:
+  gci:
+    sections:
+      - standard
+      - default
+      - prefix(github.com/scaleway/opentelemetry-collector-scaleway)
+
+  govet:
+    # settings per analyzer
+    settings:
+      printf: # analyzer name, run `go tool vet help` to see all analyzers
+        funcs: # run `go tool vet help printf` to see available settings for `printf` analyzer
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf
+
+    enable-all: true
+    # TODO: Enable this and fix the alignment issues.
+    disable:
+      - fieldalignment
+
+  revive:
+    # minimal confidence for issues, default is 0.8
+    min-confidence: 0.8
+    rules:
+      # Blank import should be only in a main or test package, or have a comment justifying it.
+      - name: blank-imports
+      # context.Context() should be the first parameter of a function when provided as argument.
+      - name: context-as-argument
+      # Basic types should not be used as a key in `context.WithValue`
+      - name: context-keys-type
+      # Importing with `.` makes the programs much harder to understand
+      - name: dot-imports
+      # Empty blocks make code less readable and could be a symptom of a bug or unfinished refactoring.
+      - name: empty-block
+      # for better readability, variables of type `error` must be named with the prefix `err`.
+      - name: error-naming
+      # for better readability, the errors should be last in the list of returned values by a function.
+      - name: error-return
+      # for better readability, error messages should not be capitalized or end with punctuation or a newline.
+      - name: error-strings
+      # report when replacing `errors.New(fmt.Sprintf())` with `fmt.Errorf()` is possible
+      - name: errorf
+      # incrementing an integer variable by 1 is recommended to be done using the `++` operator
+      - name: increment-decrement
+      # highlights redundant else-blocks that can be eliminated from the code
+      - name: indent-error-flow
+      # This rule suggests a shorter way of writing ranges that do not use the second value.
+      - name: range
+      # receiver names in a method should reflect the struct name (p for Person, for example)
+      - name: receiver-naming
+      # redefining built in names (true, false, append, make) can lead to bugs very difficult to detect.
+      - name: redefines-builtin-id
+      # redundant else-blocks that can be eliminated from the code.
+      - name: superfluous-else
+      # prevent confusing name for variables when using `time` package
+      - name: time-naming
+      # warns when an exported function or method returns a value of an un-exported type.
+      - name: unexported-return
+      # spots and proposes to remove unreachable code. also helps to spot errors
+      - name: unreachable-code
+      # Functions or methods with unused parameters can be a symptom of an unfinished refactoring or a bug.
+      - name: unused-parameter
+      # Since Go 1.18, interface{} has an alias: any. This rule proposes to replace instances of interface{} with any.
+      - name: use-any
+      # report when a variable declaration can be simplified
+      - name: var-declaration
+      # warns when initialism, variable or package naming conventions are not followed.
+      - name: var-naming
+
+  goimports:
+    # put imports beginning with prefix after 3rd-party packages;
+    # it's a comma-separated list of prefixes
+    local-prefixes: github.com/open-telemetry/opentelemetry-collector-contrib
+
+  misspell:
+    # Correct spellings using locale preferences for US or UK.
+    # Default is to use a neutral variety of English.
+    # Setting locale to US will correct the British spelling of 'colour' to 'color'.
+    locale: US
+    ignore-words:
+      - cancelled
+      - metre
+      - meter
+      - metres
+      - kilometre
+      - kilometres
+
+  depguard:
+    rules:
+      denied-deps:
+        deny:
+          - pkg: go.uber.org/atomic
+            desc: "Use 'sync/atomic' instead of go.uber.org/atomic"
+          - pkg: github.com/pkg/errors
+            desc: "Use 'errors' or 'fmt' instead of github.com/pkg/errors"
+          - pkg: github.com/hashicorp/go-multierror
+            desc: "Use go.uber.org/multierr instead of github.com/hashicorp/go-multierror"
+          - pkg: "math/rand$"
+            desc: "Use math/rand/v2 instead of math/rand"
+        # Add a different guard rule so that we can ignore tests.
+      ignore-in-test:
+          deny:
+          - pkg: go.opentelemetry.io/proto
+            desc: "Use go.opentelemetry.io/collector/pdata instead"
+            # Allow in tests for testing pdata or other receivers/exporters that expect OTLP.
+          files:
+            - "!**/*_test.go"
+
+  exhaustive:
+    explicit-exhaustive-switch: true
+    ignore-enum-members: "pmetric.MetricTypeEmpty"
+
+  nolintlint:
+    require-specific: true
+
+  predeclared:
+    ignore: copy
+
+  testifylint:
+    disable:
+      - float-compare
+      - require-error
+      - suite-subtest-run
+      - encoded-compare # has false positives that cannot be fixed with testifylint-fix
+    enable-all: true
+  thelper:
+    test:
+      begin: false
+    benchmark:
+      begin: false
+    tb:
+      begin: false
+    fuzz:
+      begin: false
+
+linters:
+  enable:
+    - copyloopvar
+    - decorder
+    - depguard
+    - errcheck
+    - errorlint
+    - exhaustive
+    - gci
+    - gocritic
+    - gofmt
+    - goimports
+    - gosec
+    - govet
+    - misspell
+    - nolintlint
+    - predeclared
+    - reassign
+    - revive
+    - staticcheck
+    - tenv
+    - testifylint
+    - thelper
+    - unconvert
+    - unparam
+    - unused
+    - usestdlibvars
+    - wastedassign
+    - whitespace
+
+issues:
+  # Excluding configuration per-path, per-linter, per-text and per-source
+  exclude-rules:
+    - text: "G404:"
+      linters:
+        - gosec
+    - text: "G402:"
+      linters:
+        - gosec
+    - text: "G115:"
+      linters:
+        - gosec
+    - path: "pagefile.go" # This exclusion is required for Windows only
+      text: "cachedBytes"
+      linters:
+        - unused

Makefile

@@ -20,3 +20,12 @@ for-all-target: $(GOMODULES)
 .PHONY: gotest
 gotest:
 	@$(MAKE) for-all-target TARGET="test"
+
+.PHONY: gotest-with-cover
+gotest-with-cover:
+	@$(MAKE) for-all-target TARGET="test-with-cover"
+	$(GOCMD) tool covdata textfmt -i=./coverage/unit -o ./coverage.txt
+
+.PHONY: golint
+golint:
+	@$(MAKE) for-all-target TARGET="lint"

Makefile.Common

@@ -1,5 +1,16 @@
 GOCMD?= go
+COVER_PKGS := $(shell go list ./... | tr "\n" ",")
+LINT := golangci-lint
 
 .PHONY: tests
 test:
 	$(GOCMD) test ./...
+
+.PHONY: test-with-cover
+test-with-cover:
+	mkdir -p $(PWD)/coverage/unit
+	$(GOCMD) test -cover -covermode=atomic -coverpkg $(COVER_PKGS) -test.gocoverdir="$(PWD)/coverage/unit" ./...
+
+.PHONY: lint
+lint:
+	$(LINT) run

receiver/scwaudittrail/factory_test.go

@@ -23,7 +23,7 @@ func TestCreateReceiver(t *testing.T) {
 	assert.Error(t, err)
 
 	// Override for test.
-	rCfg.makeClient = func(c *Config) (Client, error) {
+	rCfg.makeClient = func(_ *Config) (Client, error) {
 		ctrl := gomock.NewController(t)
 		client := NewMockClient(ctrl)
 		return client, nil

receiver/scwaudittrail/receiver.go

@@ -5,13 +5,12 @@ import (
 	"sync"
 	"time"
 
-	"go.uber.org/zap"
-
 	audit_trail "github.com/scaleway/scaleway-sdk-go/api/audit_trail/v1alpha1"
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/consumer"
 	"go.opentelemetry.io/collector/receiver"
 	"go.opentelemetry.io/collector/receiver/receiverhelper"
+	"go.uber.org/zap"
 )
 
 // auditTrailReceiver implements a custom OpenTelemetry log receiver.

receiver/scwaudittrail/receiver_test.go

@@ -16,7 +16,7 @@ import (
 
 func TestNewLogsReceiver(t *testing.T) {
 	rCfg := createDefaultConfig().(*Config)
-	rCfg.makeClient = func(c *Config) (Client, error) {
+	rCfg.makeClient = func(_ *Config) (Client, error) {
 		ctrl := gomock.NewController(t)
 		client := NewMockClient(ctrl)
 		return client, nil
@@ -37,7 +37,7 @@ func TestNewLogsReceiver(t *testing.T) {
 
 func TestHandleEvent(t *testing.T) {
 	rCfg := createDefaultConfig().(*Config)
-	rCfg.makeClient = func(c *Config) (Client, error) {
+	rCfg.makeClient = func(_ *Config) (Client, error) {
 		ctrl := gomock.NewController(t)
 		client := NewMockClient(ctrl)
 		return client, nil
@@ -67,7 +67,7 @@ func TestFetchEvents(t *testing.T) {
 		Events: []*audit_trail.Event{getEvent()},
 	}
 
-	rCfg.makeClient = func(c *Config) (Client, error) {
+	rCfg.makeClient = func(_ *Config) (Client, error) {
 		ctrl := gomock.NewController(t)
 		client := NewMockClient(ctrl)