chore: add golangci-lint

Author: Emyrk

.golangci.yaml

@@ -0,0 +1,269 @@
+# See https://golangci-lint.run/usage/configuration/
+# Over time we should try tightening some of these.
+
+linters-settings:
+  dupl:
+    # goal: 100
+    threshold: 412
+
+  exhaustruct:
+    include:
+      # Gradually extend to cover more of the codebase.
+      - 'httpmw\.\w+'
+      # We want to enforce all values are specified when inserting or updating
+      # a database row. Ref: #9936
+      - 'github.com/coder/coder/v2/coderd/database\.[^G][^e][^t]\w+Params'
+  gocognit:
+    min-complexity: 300
+
+  goconst:
+    min-len: 4 # Min length of string consts (def 3).
+    min-occurrences: 3 # Min number of const occurrences (def 3).
+
+  gocritic:
+    enabled-checks:
+      # - appendAssign
+      # - appendCombine
+      # - assignOp
+      # - badCall
+      - badLock
+      - badRegexp
+      - boolExprSimplify
+      # - builtinShadow
+      - builtinShadowDecl
+      # - commentedOutCode
+      - commentedOutImport
+      - deferUnlambda
+      # - deprecatedComment
+      # - docStub
+      - dupImport
+      # - elseif
+      - emptyFallthrough
+      # - emptyStringTest
+      # - equalFold
+      # - evalOrder
+      # - exitAfterDefer
+      # - exposedSyncMutex
+      # - filepathJoin
+      - hexLiteral
+      # - httpNoBody
+      # - hugeParam
+      # - ifElseChain
+      # - importShadow
+      - indexAlloc
+      - initClause
+      - methodExprCall
+      # - nestingReduce
+      - nilValReturn
+      # - octalLiteral
+      # - paramTypeCombine
+      # - preferStringWriter
+      # - preferWriteByte
+      # - ptrToRefParam
+      # - rangeExprCopy
+      # - rangeValCopy
+      - regexpPattern
+      # - regexpSimplify
+      - ruleguard
+      # - sloppyReassign
+      - sortSlice
+      - sprintfQuotedString
+      - sqlQuery
+      # - stringConcatSimplify
+      # - stringXbytes
+      # - suspiciousSorting
+      - truncateCmp
+      - typeAssertChain
+      # - typeDefFirst
+      # - typeUnparen
+      # - unlabelStmt
+      # - unlambda
+      # - unnamedResult
+      # - unnecessaryBlock
+      # - unnecessaryDefer
+      # - unslice
+      - weakCond
+      # - whyNoLint
+      # - wrapperFunc
+      # - yodaStyleExpr
+    settings:
+      ruleguard:
+        failOn: all
+        rules: "${configDir}/scripts/rules.go"
+
+  staticcheck:
+    # https://staticcheck.io/docs/options#checks
+    # We disable SA1019 because it gets angry about our usage of xerrors. We
+    # intentionally xerrors because stack frame support didn't make it into the
+    # stdlib port.
+    checks: ["all", "-SA1019"]
+
+  goimports:
+    local-prefixes: coder.com,cdr.dev,go.coder.com,github.com/cdr,github.com/coder
+
+  importas:
+    no-unaliased: true
+
+  misspell:
+    locale: US
+    ignore-words:
+      - trialer
+
+  nestif:
+    # goal: 10
+    min-complexity: 20
+
+  revive:
+    # see https://github.com/mgechev/revive#available-rules for details.
+    ignore-generated-header: true
+    severity: warning
+    rules:
+      - name: atomic
+      - name: bare-return
+      - name: blank-imports
+      - name: bool-literal-in-expr
+      - name: call-to-gc
+      - name: confusing-naming
+      - name: confusing-results
+      - name: constant-logical-expr
+      - name: context-as-argument
+      - name: context-keys-type
+      - name: deep-exit
+      - name: defer
+      - name: dot-imports
+      - name: duplicated-imports
+      - name: early-return
+      - name: empty-block
+      - name: empty-lines
+      - name: error-naming
+      - name: error-return
+      - name: error-strings
+      - name: errorf
+      - name: exported
+      - name: flag-parameter
+      - name: get-return
+      - name: identical-branches
+      - name: if-return
+      - name: import-shadowing
+      - name: increment-decrement
+      - name: indent-error-flow
+      # - name: modifies-parameter
+      - name: modifies-value-receiver
+      - name: package-comments
+      - name: range
+      - name: receiver-naming
+      - name: redefines-builtin-id
+      - name: string-of-int
+      - name: struct-tag
+      - name: superfluous-else
+      - name: time-naming
+      - name: unconditional-recursion
+      - name: unexported-naming
+      - name: unexported-return
+      - name: unhandled-error
+      - name: unnecessary-stmt
+      - name: unreachable-code
+      - name: unused-parameter
+        exclude: "**/*_test.go"
+      - name: unused-receiver
+      - name: var-declaration
+      - name: var-naming
+      - name: waitgroup-by-value
+
+  # irrelevant as of Go v1.22: https://go.dev/blog/loopvar-preview
+  govet:
+    disable:
+      - loopclosure
+  gosec:
+    excludes:
+      # Implicit memory aliasing of items from a range statement (irrelevant as of Go v1.22)
+      - G601
+
+issues:
+  exclude-dirs:
+    - coderd/database/dbmem
+    - node_modules
+    - .git
+
+  exclude-files:
+    - scripts/rules.go
+
+  # Rules listed here: https://github.com/securego/gosec#available-rules
+  exclude-rules:
+    - path: _test\.go
+      linters:
+        # We use assertions rather than explicitly checking errors in tests
+        - errcheck
+        - forcetypeassert
+        - exhaustruct # This is unhelpful in tests.
+    - path: scripts/*
+      linters:
+        - exhaustruct
+    - path: scripts/rules.go
+      linters:
+        - ALL
+
+  fix: true
+  max-issues-per-linter: 0
+  max-same-issues: 0
+
+run:
+  timeout: 10m
+
+# Over time, add more and more linters from
+# https://golangci-lint.run/usage/linters/ as the code improves.
+linters:
+  disable-all: true
+  enable:
+    - asciicheck
+    - bidichk
+    - bodyclose
+    - dogsled
+    - errcheck
+    - errname
+    - errorlint
+    - exhaustruct
+    - forcetypeassert
+    - gocritic
+    # gocyclo is may be useful in the future when we start caring
+    # about testing complexity, but for the time being we should
+    # create a good culture around cognitive complexity.
+    # - gocyclo
+    - gocognit
+    - nestif
+    - goimports
+    - gomodguard
+    - gosec
+    - gosimple
+    - govet
+    - importas
+    - ineffassign
+    - makezero
+    - misspell
+    - nilnil
+    - noctx
+    - paralleltest
+    - revive
+
+    # These don't work until the following issue is solved.
+    # https://github.com/golangci/golangci-lint/issues/2649
+    # - rowserrcheck
+    # - sqlclosecheck
+    # - structcheck
+    # - wastedassign
+
+    - staticcheck
+    - tenv
+    # In Go, it's possible for a package to test it's internal functionality
+    # without testing any exported functions. This is enabled to promote
+    # decomposing a package before testing it's internals. A function caller
+    # should be able to test most of the functionality from exported functions.
+    #
+    # There are edge-cases to this rule, but they should be carefully considered
+    # to avoid structural inconsistency.
+    - testpackage
+    - tparallel
+    - typecheck
+    - unconvert
+    - unused
+    - dupl

cli/clidisplay/resources.go

@@ -30,7 +30,7 @@ func WorkspaceTags(writer io.Writer, tags types.TagBlocks) hcl.Diagnostics {
 				k, v := tag.AsStrings()
 				tableWriter.AppendRow(table.Row{k, v, ""})
 				continue
-				//diags = diags.Extend(tDiags)
+				// diags = diags.Extend(tDiags)
 				//if !diags.HasErrors() {
 				//	tableWriter.AppendRow(table.Row{k, v, ""})
 				//	continue
@@ -41,7 +41,7 @@ func WorkspaceTags(writer io.Writer, tags types.TagBlocks) hcl.Diagnostics {
 			refs := tag.References()
 			tableWriter.AppendRow(table.Row{k, "??", strings.Join(refs, "\n")})
 
-			//refs := tb.AllReferences()
+			// refs := tb.AllReferences()
 			//refsStr := make([]string, 0, len(refs))
 			//for _, ref := range refs {
 			//	refsStr = append(refsStr, ref.String())
@@ -55,7 +55,7 @@ func WorkspaceTags(writer io.Writer, tags types.TagBlocks) hcl.Diagnostics {
 
 func Parameters(writer io.Writer, params []types.Parameter, files map[string]*hcl.File) {
 	tableWriter := table.NewWriter()
-	//tableWriter.SetTitle("Parameters")
+	// tableWriter.SetTitle("Parameters")
 	tableWriter.SetStyle(table.StyleLight)
 	tableWriter.Style().Options.SeparateColumns = false
 	row := table.Row{"Parameter"}
@@ -66,7 +66,7 @@ func Parameters(writer io.Writer, params []types.Parameter, files map[string]*hc
 		if p.FormType == provider.ParameterFormTypeMultiSelect {
 			_ = json.Unmarshal([]byte(strVal), &selections)
 		}
-		//value := p.Value.Value
+		// value := p.Value.Value
 		//
 		//if value.IsNull() {
 		//	strVal = "null"

cmd/preview/main.go

@@ -5,8 +5,9 @@ import (
 	"log"
 	"os"
 
-	"github.com/coder/preview/cli"
 	"github.com/hashicorp/hcl/v2"
+
+	"github.com/coder/preview/cli"
 )
 
 func main() {

log.go

@@ -11,5 +11,5 @@ func init() {
 		Level: tlog.LevelDebug,
 	}))
 	var _ = ll
-	//tlog.SetDefault(ll)
+	// tlog.SetDefault(ll)
 }

paramhook.go

@@ -156,7 +156,7 @@ func isForEachKey(key cty.Value) bool {
 func evaluateCoderParameterDefault(b *terraform.Block) (cty.Value, bool) {
 	attributes := b.Attributes()
 
-	//typeAttr, exists := attributes["type"]
+	// typeAttr, exists := attributes["type"]
 	//valueType := cty.String // TODO: Default to string?
 	//if exists {
 	//	typeVal := typeAttr.Value()

plan.go

@@ -23,7 +23,7 @@ func planJSONHook(dfs fs.FS, input Input) (func(ctx *tfcontext.Context, blocks t
 	var contents io.Reader = bytes.NewReader(input.PlanJSON)
 	// Also accept `{}` as an empty plan. If this is stored in postgres or another json
 	// type, then `{}` is the "empty" value.
-	if len(input.PlanJSON) == 0 || bytes.Compare(input.PlanJSON, []byte("{}")) == 0 {
+	if len(input.PlanJSON) == 0 || bytes.Equal(input.PlanJSON, []byte("{}")) {
 		if input.PlanJSONPath == "" {
 			return func(ctx *tfcontext.Context, blocks terraform.Blocks, inputVars map[string]cty.Value) {}, nil
 		}

preview.go

@@ -58,7 +58,7 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn
 
 	// TODO: Fix logging. There is no way to pass in an instanced logger to
 	//   the parser.
-	//slog.SetLogLoggerLevel(slog.LevelDebug)
+	// slog.SetLogLoggerLevel(slog.LevelDebug)
 	//slog.SetDefault(slog.New(log.NewHandler(os.Stderr, nil)))
 
 	varFiles, err := tfVarFiles("", dir)
@@ -127,7 +127,7 @@ func Preview(ctx context.Context, input Input, dir fs.FS) (output *Output, diagn
 			},
 		}
 	}
-	
+
 	outputs := hclext.ExportOutputs(modules)
 
 	diags := make(hcl.Diagnostics, 0)

previewe2e_test.go

@@ -47,7 +47,7 @@ import (
 // The goal of the test is to compare `tfstate` with the output of `preview`.
 // If `preview`'s implementation of terraform is incorrect, the test will fail.
 // TODO: Adding varied parameter inputs would be a good idea.
-// TODO: Add workspace tag comparisions.
+// TODO: Add workspace tag comparisons.
 func Test_VerifyE2E(t *testing.T) {
 	t.Parallel()
 
@@ -135,7 +135,7 @@ func Test_VerifyE2E(t *testing.T) {
 					require.NoError(t, err, "terraform show plan")
 
 					pd, err := json.Marshal(plan)
-					require.NoError(t, err, "marshalling plan")
+					require.NoError(t, err, "marshaling plan")
 
 					err = os.WriteFile(filepath.Join(wp, "plan.json"), pd, 0644)
 					require.NoError(t, err, "writing plan.json")

scripts/rules.go

@@ -0,0 +1,16 @@
+// Package gorules defines custom lint rules for ruleguard.
+//
+// golangci-lint runs these rules via go-critic, which includes support
+// for ruleguard. All Go files in this directory define lint rules
+// in the Ruleguard DSL; see:
+//
+// - https://go-ruleguard.github.io/by-example/
+// - https://pkg.go.dev/github.com/quasilyte/go-ruleguard/dsl
+//
+// You run one of the following commands to execute your go rules only:
+//
+//	golangci-lint run
+//	golangci-lint run --disable-all --enable=gocritic
+//
+// Note: don't forget to run `golangci-lint cache clean`!
+package gorules