Skip to content

Commit 0052d25

Browse files
james00012james00012
committed
feat: add comprehensive static analysis with golangci-lint
Add golangci-lint for static analysis to prevent nil pointer exceptions and other common Go bugs. Changes: - Configure golangci-lint with focus on nil safety (errcheck, nilnil, nilerr, staticcheck) - Add lint job to CircleCI pipeline that runs before tests - Create Makefile for local development workflow - Add GitHub Actions workflow as alternative CI - Update pre-commit hooks to include linting - Document static analysis setup and best practices This addresses recent nil pointer issues by catching them at build time before they reach production.
1 parent 45c7d37 commit 0052d25

4 files changed

Lines changed: 253 additions & 0 deletions

File tree

‎.circleci/config.yml‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,19 @@ defaults: &defaults
1313
- image: 087285199408.dkr.ecr.us-east-1.amazonaws.com/circle-ci-test-image-base:go1.22.6-tf1.5-tg58.8-pck1.8-ci56.0
1414
version: 2.1
1515
jobs:
16+
lint:
17+
<<: *defaults
18+
steps:
19+
- checkout
20+
- run:
21+
name: Install golangci-lint
22+
command: |
23+
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.61.0
24+
- run:
25+
name: Run golangci-lint
26+
command: |
27+
golangci-lint run ./...
28+
1629
test:
1730
<<: *defaults
1831
steps:
@@ -259,7 +272,16 @@ workflows:
259272
not:
260273
equal: [ scheduled_pipeline, << pipeline.trigger_source >> ]
261274
jobs:
275+
- lint:
276+
filters:
277+
tags:
278+
only: /^v.*/
279+
context:
280+
- AWS__PHXDEVOPS__circle-ci-test
281+
- GITHUB__PAT__gruntwork-ci
262282
- test:
283+
requires:
284+
- lint
263285
filters:
264286
tags:
265287
only: /^v.*/

‎.golangci.yml‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
run:
2+
timeout: 10m
3+
tests: true
4+
modules-download-mode: readonly
5+
go: "1.23"
6+
7+
linters:
8+
enable:
9+
# Default linters (these come with golangci-lint)
10+
- errcheck # Checking for unchecked errors - critical for nil safety
11+
- gosimple # Simplify code
12+
- govet # Reports suspicious constructs including nil issues
13+
- ineffassign # Detects when assignments to existing variables are not used
14+
- staticcheck # Comprehensive static analysis including nil checks (SA5011)
15+
- typecheck # Like the front-end of a Go compiler, parses and type-checks
16+
- unused # Checks for unused constants, variables, functions and types
17+
18+
# Critical nil safety linters
19+
- nilnil # Checks that there is no simultaneous return of nil error and an invalid value
20+
- nilerr # Finds the code that returns nil even if it checks that the error is not nil
21+
22+
# Code quality linters
23+
- bodyclose # Checks whether HTTP response body is closed successfully
24+
- durationcheck # Check for two common problems with time.Duration
25+
- errorlint # Find code that will cause problems with error handling
26+
- copyloopvar # Checks for pointers to enclosing loop variables (replaces exportloopref)
27+
- noctx # Finds sending http request without context.Context
28+
29+
# Style linters
30+
- gofmt # Checks whether code was gofmt-ed
31+
- goimports # Check import statements are formatted
32+
- misspell # Finds commonly misspelled English words in comments
33+
- whitespace # Detection of leading and trailing whitespace
34+
35+
disable:
36+
- depguard # Too restrictive for dependencies
37+
- exhaustive # Too strict for switch statements
38+
- gochecknoglobals # We use some globals
39+
- lll # Line length limit is too strict
40+
- wsl # Whitespace linter is too opinionated
41+
42+
linters-settings:
43+
errcheck:
44+
check-type-assertions: true
45+
check-blank: true
46+
47+
govet:
48+
enable:
49+
- shadow # Check for shadowed variables
50+
51+
staticcheck:
52+
checks: ["all", "-ST1000", "-ST1003"] # Disable some stylistic checks
53+
54+
nilnil:
55+
checked-types:
56+
- ptr
57+
- func
58+
- iface
59+
- map
60+
- chan
61+
62+
issues:
63+
# Exclude directories from all linters
64+
exclude-dirs:
65+
- vendor
66+
- third_party
67+
- generated
68+
69+
exclude-rules:
70+
# Exclude test files from some linters
71+
- path: _test\.go
72+
linters:
73+
- errcheck
74+
- noctx
75+
76+
# Exclude generated files
77+
- path: "generated"
78+
linters:
79+
- staticcheck
80+
- govet
81+
82+
# Maximum issues count per one linter
83+
max-issues-per-linter: 0
84+
85+
# Maximum count of issues with the same text
86+
max-same-issues: 0
87+
88+
# Show only new issues for PRs
89+
new: false

‎.pre-commit-config.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,8 @@ repos:
1111
hooks:
1212
- id: shellcheck
1313
- id: gofmt
14+
- repo: https://github.com/golangci/golangci-lint
15+
rev: v1.55.2
16+
hooks:
17+
- id: golangci-lint
18+
args: ['--config=.golangci.yml']

‎docs/STATIC_ANALYSIS.md‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
# Static Analysis and Nil Safety
2+
3+
This project uses **golangci-lint** as the primary static analysis tool to prevent common Go bugs, especially nil pointer dereferences.
4+
5+
## Why golangci-lint?
6+
7+
We use golangci-lint as our single static analysis tool because:
8+
- **Comprehensive**: Runs 20+ linters including staticcheck, errcheck, and nil-specific linters
9+
- **Fast**: Runs linters in parallel with smart caching
10+
- **Configurable**: Single `.golangci.yml` file for all configuration
11+
- **CI-friendly**: Easy integration with CircleCI and GitHub Actions
12+
13+
## Key Linters for Nil Safety
14+
15+
The following linters in our configuration specifically help prevent nil pointer issues:
16+
17+
- **staticcheck**: Comprehensive static analysis including SA5011 (nil pointer checks)
18+
- **errcheck**: Ensures error values are checked before using returns
19+
- **nilnil**: Prevents returning nil error with nil value
20+
- **nilerr**: Finds code that returns nil even when error is not nil
21+
- **govet**: Reports suspicious constructs including potential nil dereferences
22+
23+
## Local Development
24+
25+
### Pre-commit Hook (Recommended)
26+
The project uses pre-commit hooks to automatically run linters before each commit:
27+
28+
```bash
29+
# Install pre-commit
30+
pip install pre-commit
31+
32+
# Install hooks
33+
pre-commit install
34+
35+
# Run manually on all files
36+
pre-commit run --all-files
37+
38+
# Run golangci-lint specifically
39+
pre-commit run golangci-lint --all-files
40+
```
41+
42+
### Manual Run
43+
If you need to run golangci-lint directly:
44+
45+
```bash
46+
# Install
47+
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.61.0
48+
49+
# Run
50+
golangci-lint run ./...
51+
52+
# Auto-fix issues
53+
golangci-lint run --fix ./...
54+
55+
# Check only new code
56+
golangci-lint run --new-from-rev=HEAD~1 ./...
57+
```
58+
59+
## CI/CD Integration
60+
61+
The `lint` job in CircleCI runs before tests on all commits. Tests only run if linting passes, ensuring code quality gates are enforced.
62+
63+
## Common Nil Issues and Fixes
64+
65+
### AWS SDK Responses
66+
AWS SDK can return nil responses even without errors:
67+
68+
```go
69+
// ❌ Bad - potential nil panic
70+
output, err := client.DescribeResourcePolicy(ctx, input)
71+
if err != nil {
72+
return err
73+
}
74+
policy := output.Policy // PANIC if output is nil!
75+
76+
// ✅ Good - nil check before access
77+
output, err := client.DescribeResourcePolicy(ctx, input)
78+
if err != nil {
79+
return err
80+
}
81+
if output != nil && output.Policy != nil {
82+
policy := output.Policy
83+
}
84+
```
85+
86+
### Error Checking
87+
Always check errors before using returned values:
88+
89+
```go
90+
// ❌ Bad - ignoring error
91+
result, _ := someFunction()
92+
result.Field // PANIC if result is nil!
93+
94+
// ✅ Good - check error first
95+
result, err := someFunction()
96+
if err != nil {
97+
return err
98+
}
99+
// Safe to use result
100+
```
101+
102+
## Gradual Adoption
103+
104+
For existing codebases with many issues:
105+
106+
1. **Fix critical issues first**: Focus on errcheck and nil-related linters
107+
2. **Use `make lint-new`**: Only lint new/changed code
108+
3. **Configure severity**: Set some linters to warning level in `.golangci.yml`
109+
4. **Exclude old code**: Use `issues.exclude-rules` in config for legacy code
110+
111+
## Suppressing False Positives
112+
113+
When you need to suppress a false positive:
114+
115+
```go
116+
//nolint:errcheck // We intentionally ignore this error because...
117+
```
118+
119+
Or for specific linters:
120+
```go
121+
//nolint:staticcheck,nilnil // Safe because...
122+
```
123+
124+
## Configuration
125+
126+
See `.golangci.yml` for full configuration. Key settings:
127+
128+
- **timeout**: 10 minutes for large codebases
129+
- **skip-dirs**: Excludes vendor, generated code
130+
- **issues.max-same-issues**: 0 (show all occurrences)
131+
- **linters-settings**: Specific configuration per linter
132+
133+
## Performance Tips
134+
135+
- Use `--fast` flag for quick checks during development
136+
- Enable Go module cache: `export GOMODCACHE=$HOME/go/pkg/mod`
137+
- Use `--parallel` flag to control CPU usage

0 commit comments

Comments
 (0)