Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@ structalign [flags] [packages]
-tests also analyze _test.go files (skipped by default)
-skip-cache-padded
skip structs with a golang.org/x/sys/cpu.CacheLinePad field
-show-nolint show structs even when their type carries a recognized
//nolint directive (directives are respected by default)
-nolint-linters string
//nolint tokens that suppress a finding (default
"fieldalignment"; a bare //nolint always counts)

-version print version and exit
```
Expand All @@ -122,6 +127,12 @@ In the default `-color=auto`, color is emitted only when stdout is a terminal an
the [`NO_COLOR`](https://no-color.org) environment variable is unset. `NO_COLOR`
(any non-empty value) disables color; an explicit `-color=always` overrides it.

The palette can be switched with the `STRUCTALIGN_THEME` environment variable —
`default` (the standard colors), `cga` (a bright 16-color CGA look), or `green` /
`amber` (single-hue phosphor-monitor emulations). It only affects *which* colors
are used when color is on; it does not turn color on by itself. An unknown value
warns and falls back to `default`.

Exit code is **1 when reorderings are found**, **0 when none** — so it drops into
CI as a check. Inspect mode is informational and always exits 0.

Expand Down Expand Up @@ -313,6 +324,13 @@ structalign -skip-cache-padded ./... # skip structs guarded by cpu.Cache
- **`-skip-cache-padded`** leaves structs with a
[`cpu.CacheLinePad`](https://pkg.go.dev/golang.org/x/sys/cpu#CacheLinePad) field
alone, since reordering would move the pad and defeat its false-sharing guard.
- **`//nolint` directives are respected by default** (diff mode): a struct whose
type declaration carries a recognized `//nolint` — `//nolint:fieldalignment` or
a bare `//nolint` — is suppressed, matching golangci-lint. `-nolint-linters`
customizes which named tokens count (default `fieldalignment`; e.g.
`-nolint-linters=fieldalignment,betteralign`); a bare `//nolint` always counts.
`-show-nolint` reveals suppressed structs (audit mode). Inspect mode ignores
these directives.

### Field tags

Expand Down
10 changes: 8 additions & 2 deletions internal/align/align.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ func (a *Aligner) Findings(t common.Target, opts common.Options) ([]common.Findi
return nil, nil
}
names := structNameIndex(t.Syntax)
nolints := nolintIndex(t.Syntax, t.Fset)
insp := inspector.New(t.Syntax)

var findings []common.Finding
Expand All @@ -55,7 +56,7 @@ func (a *Aligner) Findings(t common.Target, opts common.Options) ([]common.Findi
TypesSizes: t.Sizes, // common.Sizes satisfies types.Sizes
ResultOf: map[*analysis.Analyzer]any{inspect.Analyzer: insp},
Report: func(d analysis.Diagnostic) {
f := buildFinding(t, d, names, opts)
f := buildFinding(t, d, names, nolints, opts)
if f == nil {
return
}
Expand All @@ -78,7 +79,7 @@ func (a *Aligner) Findings(t common.Target, opts common.Options) ([]common.Findi
// buildFinding converts one analyzer diagnostic into a Finding, applying tag
// stripping and all active filters. Returns nil when the finding should be
// suppressed.
func buildFinding(t common.Target, d analysis.Diagnostic, names map[token.Pos]string, opts common.Options) *common.Finding {
func buildFinding(t common.Target, d analysis.Diagnostic, names map[token.Pos]string, nolints map[token.Pos]nolintInfo, opts common.Options) *common.Finding {
f := common.Finding{Fset: t.Fset, Pos: d.Pos, Message: d.Message}
if len(d.SuggestedFixes) > 0 && len(d.SuggestedFixes[0].TextEdits) > 0 {
e := d.SuggestedFixes[0].TextEdits[0]
Expand Down Expand Up @@ -107,6 +108,11 @@ func buildFinding(t common.Target, d analysis.Diagnostic, names map[token.Pos]st
if opts.SkipCachePadded && isCachePadded(t, f.Name) {
return nil
}
if opts.RespectNolint {
if info, ok := nolints[f.Pos]; ok && info.suppressed(opts.NolintLinters) {
return nil
}
}
return &f
}

Expand Down
123 changes: 123 additions & 0 deletions internal/align/nolint.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
package align

import (
"go/ast"
"go/token"
"strings"
)

// nolintInfo records the //nolint directive found on a struct type declaration.
type nolintInfo struct {
bare bool // a bare //nolint (suppresses all linters)
tokens map[string]struct{} // named tokens, e.g. {"fieldalignment"}
}

// suppressed reports whether this directive suppresses a finding given the set
// of honored named linters. A bare //nolint always suppresses.
func (info nolintInfo) suppressed(linters []string) bool {
if info.bare {
return true
}
for _, l := range linters {
if _, ok := info.tokens[l]; ok {
return true
}
}
return false
}

// nolintIndex maps each named struct type's StructType.Pos() to the //nolint
// directive on its declaration. A directive is recognized from the type's doc
// comment (TypeSpec.Doc, or the enclosing GenDecl.Doc for grouped `type ( ... )`
// blocks) and from any comment on the type's opening line (e.g. a trailing
// `type T struct { //nolint`, which the AST does not attach to TypeSpec.Comment).
// The analyzer reports at StructType.Pos(), so this key matches a Finding's Pos.
func nolintIndex(files []*ast.File, fset *token.FileSet) map[token.Pos]nolintInfo {
index := make(map[token.Pos]nolintInfo)
for _, f := range files {
// Directives keyed by source line, from every comment in the file —
// used to catch a trailing directive on a struct's opening line.
byLine := make(map[int]nolintInfo)
for _, cg := range f.Comments {
for _, c := range cg.List {
line := fset.Position(c.Pos()).Line
info := byLine[line]
parseNolint(c.Text, &info)
byLine[line] = info
}
}
for _, decl := range f.Decls {
gd, ok := decl.(*ast.GenDecl)
if !ok || gd.Tok != token.TYPE {
continue
}
for _, spec := range gd.Specs {
ts, ok := spec.(*ast.TypeSpec)
if !ok {
continue
}
st, ok := ts.Type.(*ast.StructType)
if !ok {
continue
}
info := collectNolint(ts.Doc, gd.Doc, ts.Comment)
mergeNolint(&info, byLine[fset.Position(st.Pos()).Line])
if info.bare || len(info.tokens) > 0 {
index[st.Pos()] = info
}
}
}
}
return index
}

// collectNolint scans the given comment groups for //nolint directives.
func collectNolint(groups ...*ast.CommentGroup) nolintInfo {
var info nolintInfo
for _, g := range groups {
if g == nil {
continue
}
for _, c := range g.List {
parseNolint(c.Text, &info)
}
}
return info
}

// mergeNolint folds src into dst (bare wins, tokens unioned).
func mergeNolint(dst *nolintInfo, src nolintInfo) {
if src.bare {
dst.bare = true
}
for tok := range src.tokens {
if dst.tokens == nil {
dst.tokens = make(map[string]struct{})
}
dst.tokens[tok] = struct{}{}
}
}

// parseNolint reads one "//nolint" or "//nolint:a,b,c" comment into info.
// "//nolint" (alone or followed by space) is bare; "//nolint:list" adds named
// tokens. A comment like "//nolintfoo" is not a directive.
func parseNolint(text string, info *nolintInfo) {
text = strings.TrimSpace(strings.TrimPrefix(text, "//"))
if !strings.HasPrefix(text, "nolint") {
return
}
rest := strings.TrimPrefix(text, "nolint")
switch {
case rest == "" || strings.HasPrefix(rest, " "):
info.bare = true
case strings.HasPrefix(rest, ":"):
for tok := range strings.SplitSeq(rest[1:], ",") {
if tok = strings.TrimSpace(tok); tok != "" {
if info.tokens == nil {
info.tokens = make(map[string]struct{})
}
info.tokens[tok] = struct{}{}
}
}
}
}
95 changes: 95 additions & 0 deletions internal/align/nolint_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package align_test

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/peczenyj/structalign/internal/align"
"github.com/peczenyj/structalign/internal/testutil"
"github.com/peczenyj/structalign/pkg/common"
)

const nolintSrc = `package sample

//nolint:fieldalignment
type Suppressed struct {
A bool
B int64
C bool
}

type Visible struct {
A bool
B int64
C bool
}

//nolint:errcheck
type Unrelated struct {
A bool
B int64
C bool
}

type Trailing struct { //nolint
A bool
B int64
C bool
}

type TrailingNamed struct { //nolint:fieldalignment
A bool
B int64
C bool
}

// A non-struct type declaration the index must skip without panicking.
type Celsius float64
`

func findingNames(fs []common.Finding) map[string]bool {
m := make(map[string]bool)
for _, f := range fs {
m[f.Name] = true
}
return m
}

func TestFindingsRespectNolintByDefault(t *testing.T) {
tgt := testutil.Target(t, nolintSrc)
fs, err := align.New().Findings(tgt, common.Options{
RespectNolint: true,
NolintLinters: []string{"fieldalignment"},
})
require.NoError(t, err)
names := findingNames(fs)
assert.False(t, names["Suppressed"], "//nolint:fieldalignment must suppress")
assert.True(t, names["Visible"], "unmarked struct stays")
assert.True(t, names["Unrelated"], "//nolint:errcheck must NOT suppress fieldalignment")
assert.False(t, names["Trailing"], "a bare //nolint (trailing) must suppress")
assert.False(t, names["TrailingNamed"], "a trailing //nolint:fieldalignment must suppress")
}

func TestFindingsShowNolint(t *testing.T) {
tgt := testutil.Target(t, nolintSrc)
fs, err := align.New().Findings(tgt, common.Options{RespectNolint: false})
require.NoError(t, err)
names := findingNames(fs)
assert.True(t, names["Suppressed"], "respect off => suppressed struct reappears")
assert.True(t, names["Trailing"], "respect off => bare-nolint struct reappears")
}

func TestFindingsNolintLintersConfigurable(t *testing.T) {
tgt := testutil.Target(t, nolintSrc)
// Only honoring "betteralign" => :fieldalignment no longer suppresses.
fs, err := align.New().Findings(tgt, common.Options{
RespectNolint: true,
NolintLinters: []string{"betteralign"},
})
require.NoError(t, err)
names := findingNames(fs)
assert.True(t, names["Suppressed"], "fieldalignment token not in the configured set => not suppressed")
assert.False(t, names["Trailing"], "bare //nolint is always honored regardless of the configured set")
}
Loading
Loading