diff --git a/.bestpractices.json b/.bestpractices.json new file mode 100644 index 0000000..649171b --- /dev/null +++ b/.bestpractices.json @@ -0,0 +1,53 @@ +{ + "description_good_status": "Met", + "description_good_justification": "The project has a clear, comprehensive description and summary of its functionality in README.md.", + + "interact_status": "Met", + "interact_justification": "Users can interact with the project and maintainers via GitHub issues, discussions, and pull requests.", + + "contribution_status": "Met", + "contribution_justification": "Contributing guidelines are clearly documented in CONTRIBUTING.md.", + + "contribution_quickstart_status": "Met", + "contribution_quickstart_justification": "The README.md and CONTRIBUTING.md files include a quick-start guide for new contributors.", + + "floss_license_status": "Met", + "floss_license_justification": "The project is licensed under the MIT license, which is a recognized FLOSS license.", + + "floss_license_osi_status": "Met", + "floss_license_osi_justification": "The MIT license is officially approved by the OSI (Open Source Initiative).", + + "build_status": "Met", + "build_justification": "The project uses a standard Go toolchain and Taskfile.yml/Makefile for automated builds.", + + "test_status": "Met", + "test_justification": "The project includes a robust test suite covering over 190 tests (unit, integration, coverage, and fuzz tests).", + + "test_suite_status": "Met", + "test_suite_justification": "All tests are run automatically via a make/task runner on the command line.", + + "test_ci_status": "Met", + "test_ci_test_status": "Met", + "test_ci_justification": "GitHub Actions runs all tests on every push and pull request (defined in ci.yml).", + + "coding_standards_status": "Met", + "coding_standards_justification": "We enforce strict coding standards using golangci-lint, go-consistent, and gofumpt formatting in CI.", + + "coding_standards_tools_status": "Met", + "coding_standards_tools_justification": "All styling, consistency, and syntax audits are fully automated using standard static analysis tools.", + + "report_tracker_status": "Met", + "report_tracker_justification": "GitHub Issues is used as the authoritative bug tracking system.", + + "vulnerability_report_status": "Met", + "vulnerability_report_justification": "The SECURITY.md file outlines the clear process for private/responsible disclosure of vulnerabilities.", + + "static_analysis_status": "Met", + "static_analysis_justification": "Static application security testing (SAST) is fully automated using GitHub CodeQL.", + + "dynamic_analysis_status": "N/A", + "dynamic_analysis_justification": "As a static Go tool analyzing code structures locally, there is no long-running server or dynamic state to fuzz dynamically in runtime, making DAST not applicable. We compensate by running extensive native Go fuzzing on all parsing inputs.", + + "osps_ac_01_01_status": "Met", + "osps_ac_01_01_justification": "GitHub strictly mandates and enforces multi-factor authentication (MFA) for all repository write collaborators." +} diff --git a/.github/scorecard.yml b/.github/scorecard.yml new file mode 100644 index 0000000..cb00716 --- /dev/null +++ b/.github/scorecard.yml @@ -0,0 +1,13 @@ +# OpenSSF Scorecard maintainer annotations +# These do not change the numeric score but provide context to consumers. +annotations: + - checks: + - Code-Review + reasons: + - reason: not-applicable + explanation: >- + This is a solo-maintained project. Requiring a second human reviewer + would block all development. We compensate for this by requiring + all changes to go through a Pull Request workflow, enforcing strict + automated CI/CD status checks, and running static analysis (SAST) + on every change. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 48a19f9..9a1fbd1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,7 +29,7 @@ jobs: repo-token: ${{ secrets.GITHUB_TOKEN }} - name: Install gotestsum - run: go install gotest.tools/gotestsum@latest + run: go install gotest.tools/gotestsum@v1.13.0 - name: Lint uses: golangci/golangci-lint-action@82606bf257cbaff209d206a39f5134f0cfbfd2ee # v9.2.1 @@ -37,7 +37,7 @@ jobs: version: v2.12.2 - name: Install go-consistent - run: go install github.com/quasilyte/go-consistent@latest + run: go install github.com/quasilyte/go-consistent@v0.6.2 - name: Consistency check run: task consistent diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index eedf2ec..d0034ee 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -9,15 +9,17 @@ on: tags: - 'v*' -permissions: - contents: write # create the GitHub Release and upload assets - id-token: write # OIDC token for keyless (Sigstore) signing of the attestation - attestations: write # store the SLSA build-provenance attestation +permissions: read-all jobs: goreleaser: name: Release runs-on: ubuntu-latest + permissions: + contents: write # create the GitHub Release and upload assets + id-token: write # OIDC token for keyless (Sigstore) signing of the attestation + attestations: write # store the SLSA build-provenance attestation + steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: @@ -25,6 +27,8 @@ jobs: - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c # v6.4.0 with: go-version: stable + - name: Install Cosign + uses: sigstore/cosign-installer@6f9f17788090df1f26f669e9d70d6ae9567deba6 # v4.1.2 - name: Run GoReleaser uses: goreleaser/goreleaser-action@5daf1e915a5f0af01ddbcd89a43b8061ff4f1a89 # v7.2.2 with: diff --git a/.github/workflows/scorecard.yml b/.github/workflows/scorecard.yml index f447767..8742f8d 100644 --- a/.github/workflows/scorecard.yml +++ b/.github/workflows/scorecard.yml @@ -73,6 +73,6 @@ jobs: # Upload the results to GitHub's code scanning dashboard (optional). # Commenting out will disable upload of results to your repo's Code Scanning dashboard - name: "Upload to code-scanning" - uses: github/codeql-action/upload-sarif@f52b05f4acaaa234e44466e66d29050e135ea9ef # v4.36.0 + uses: github/codeql-action/upload-sarif@7211b7c8077ea37d8641b6271f6a365a22a5fbfa # v4.36.0 with: sarif_file: results.sarif diff --git a/.golangci.yml b/.golangci.yml index 2bf5f0f..39865d3 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -28,10 +28,16 @@ linters: severity: warning exclusions: rules: - - path: _test\.go + - path: '_test\.go' linters: - gocyclo - errcheck + - linters: + - errcheck + text: 'fmt\.Fprint' + - linters: + - revive + text: 'exported' formatters: enable: diff --git a/.goreleaser.yaml b/.goreleaser.yaml index d5b1f58..de79af9 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -53,3 +53,14 @@ release: name: structalign draft: false prerelease: auto + +signs: + - cmd: cosign + signature: "${artifact}.sigstore.json" + args: + - "sign-blob" + - "--bundle=${signature}" + - "${artifact}" + - "--yes" + artifacts: checksum + diff --git a/AGENTS.md b/AGENTS.md index 9e128e8..075d04b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -24,6 +24,7 @@ The program is split into small, decoupled packages: (runs the analyzer → findings), `layout` (computes struct layouts), `sizes` (`go/types` sizing adapter), `textdiff` (go-udiff line diff), `match` (glob filtering), `structfilter` (generated-file and `cpu.CacheLinePad` predicates), + `config` (.structalignrc and env var mapping), `ui` (the `Printer` — all rendering + color/width helpers), `app` (flag parsing + wiring). Plus `testutil` (in-process `Target` builder for tests) and `mocks` (mockery-generated, test-only). @@ -112,6 +113,41 @@ deterministic implementation. Package load/type errors are surfaced on each `Target.Errors` and printed to stderr but are non-fatal — a partially-resolved package can still produce findings. +**Layered Configuration:** defaults are loaded from four layers before parsing +CLI arguments (highest precedence wins): +1. **CLI flags** (e.g. `-sort`) +2. **Environment variables** (`STRUCTALIGN_`, e.g. `STRUCTALIGN_SORT=true`) +3. **CWD RC file** (`./.structalignrc`, key=value format) +4. **Home RC file** (`~/.structalignrc`) +5. **Built-in defaults** + +The `-no-rc` flag (detected early) disables loading both `.structalignrc` files. +RC files use `key = value` lines; `#` comments and blank lines are ignored. +Keys map directly to flag names. **theme** is not an RC key (use +`STRUCTALIGN_THEME`). + +| Feature | CLI Flag | Environment Variable | RC Key | Default | +|---------|----------|----------------------|--------|---------| +| Diff style | `-diff` | `STRUCTALIGN_DIFF` | `diff` | `unified` | +| Output format | `-format` | `STRUCTALIGN_FORMAT` | `format` | `text` | +| Column width | `-width` | `STRUCTALIGN_WIDTH` | `width` | `0` (auto) | +| Color mode | `-color` | `STRUCTALIGN_COLOR` | `color` | `auto` | +| Theme palette | — | `STRUCTALIGN_THEME` | — | `default` | +| Inspect mode | `-inspect` | `STRUCTALIGN_INSPECT` | `inspect` | `false` | +| Verbose inspect | `-verbose` | `STRUCTALIGN_VERBOSE` | `verbose` | `false` | +| Keep tags | `-tags` | `STRUCTALIGN_TAGS` | `tags` | `false` | +| Show summary | `-summary` | `STRUCTALIGN_SUMMARY` | `summary` | `false` | +| Largest-first sort | `-sort` | `STRUCTALIGN_SORT` | `sort` | `false` | +| Min bytes saved | `-threshold` | `STRUCTALIGN_THRESHOLD` | `threshold` | `0` | +| Type filter | `-type` | `STRUCTALIGN_TYPE` | `type` | (empty) | +| Package exclude | `-exclude` | `STRUCTALIGN_EXCLUDE` | `exclude` | `^unsafe$\|^builtin$` | +| Include generated | `-generated` | `STRUCTALIGN_GENERATED` | `generated` | `false` | +| Include tests | `-tests` | `STRUCTALIGN_TESTS` | `tests` | `false` | +| Skip cache padded | `-skip-cache-padded` | `STRUCTALIGN_SKIP_CACHE_PADDED` | `skip-cache-padded` | `false` | +| Show //nolint | `-show-nolint` | `STRUCTALIGN_SHOW_NOLINT` | `show-nolint` | `false` | +| Nolint linters | `-nolint-linters` | `STRUCTALIGN_NOLINT_LINTERS` | `nolint-linters` | `fieldalignment` | + + **Why go-udiff and not x/tools' own diff:** Go's internal-package rule forbids importing `golang.org/x/tools/internal/diff` from a module not rooted under `golang.org/x/tools/`. `fieldalignment`'s *own* internal imports are fine because @@ -127,6 +163,14 @@ to swap it back for the internal package — it won't compile from this module. - **`align` and `layout` return data, `ui` renders it.** Keep that split: no printing in the logic packages, no analysis in `ui`. New output formatting goes in `ui`; new analysis/derived fields go on the `common` types. +- **`-format=json` (`ui.RenderJSON`) is the machine renderer**, parallel to + `RenderFindings`/`RenderLayouts`. Two deliberate divergences from the text path: + the diff document **always** carries the `summary` block (so consumers always + get totals — `-summary` governs only the text trailing line), and the text-only + presentation flags (`-diff`, `-summary`, `-verbose`, `-color`, `-width`) are + ignored in JSON. `-tags` still gates the inspect field's `tag`. Any encode + error is reported on the printer's `Err` stream (`p.err()`, set to + `App.Stderr`), not the real `os.Stderr`. - **Scan options travel in `common.Options`** (`Patterns`, `KeepTags`, `IncludeGenerated`, `SkipCachePadded`, `RespectNolint`, `NolintLinters`), passed to `Aligner.Findings` / `Inspector.Layouts`. `align`/`layout` apply the filters @@ -136,7 +180,11 @@ to swap it back for the internal package — it won't compile from this module. default** (`-generated` opts in); `_test.go` is loaded only with `-tests` (`loader.New(tests)`); `-exclude` drops packages by import-path regexp in `app`. Add a new scan knob to `Options`, not as another positional arg. -- **`//nolint` is respected by default (diff only).** `align.nolintIndex` maps + - **Config discovery lives in `internal/config`.** It handles `.structalignrc` + parsing and env-name derivation (`-skip-cache-padded` → + `STRUCTALIGN_SKIP_CACHE_PADDED`). `app.Run` wires these as defaults via + `fs.Set` before calling `fs.Parse`. + - **//nolint is respected by default (diff only).** `align.nolintIndex` maps `StructType.Pos()` to the directive parsed from the type's doc comment (`TypeSpec.Doc` / grouped `GenDecl.Doc`) **and** any comment on the type's opening line (a trailing `type T struct { //nolint`, matched by line since the diff --git a/CHANGELOG.md b/CHANGELOG.md index 079b8fa..fae62fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,26 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.7.0] - 2026-05-29 + +### Changed + +- Pre-v0.7.0 Cleanup: Sorting, Efficiency, and Bug Fixes (#68) +- Layered Configuration (Env Vars + .structalignrc) (#69) +- JSON Output Support (-format=json) (#71) + +### Fixed + +- *(ci)* Pin codeql action to its commit SHA, not the tag object (#63) +- *(usage)* List -no-rc in -h and document the theme eggs (#74) +- Address major bugs, unhandled AST aliases, and config issues on devel (#82) +- Graceful RC keys, JSON encode-error stream, and docs (#87) +- *(align)* Strip field comments from both diff sides (#89) + +### Documentation + +- Refresh diff screenshot and drop redundant tag badge (#64) + ## [0.6.1] - 2026-05-27 ### Added @@ -182,6 +202,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Focus the README screenshot on the Record type - Document file/dir args instead of unsupported ./... (#5) +[0.7.0]: https://github.com/peczenyj/structalign/compare/v0.6.1..v0.7.0 [0.6.1]: https://github.com/peczenyj/structalign/compare/v0.6.0..v0.6.1 [0.6.0]: https://github.com/peczenyj/structalign/compare/v0.5.2..v0.6.0 [0.5.2]: https://github.com/peczenyj/structalign/compare/v0.5.1..v0.5.2 diff --git a/README.md b/README.md index 8cc2194..aa02d07 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # structalign -[![tag](https://img.shields.io/github/tag/peczenyj/structalign.svg)](https://github.com/peczenyj/structalign/releases) +[![Latest release](https://img.shields.io/github/release/peczenyj/structalign.svg)](https://github.com/peczenyj/structalign/releases/latest) ![Go Version](https://img.shields.io/badge/Go-%3E%3D%201.25-%23007d9c) [![GoDoc](https://pkg.go.dev/badge/github.com/peczenyj/structalign)](http://pkg.go.dev/github.com/peczenyj/structalign) [![CI](https://github.com/peczenyj/structalign/actions/workflows/ci.yml/badge.svg)](https://github.com/peczenyj/structalign/actions/workflows/ci.yml) @@ -9,12 +9,12 @@ [![CodeQL](https://github.com/peczenyj/structalign/actions/workflows/github-code-scanning/codeql/badge.svg)](https://github.com/peczenyj/structalign/actions/workflows/github-code-scanning/codeql) [![Dependency Review](https://github.com/peczenyj/structalign/actions/workflows/dependency-review.yml/badge.svg)](https://github.com/peczenyj/structalign/actions/workflows/dependency-review.yml) [![License](https://img.shields.io/github/license/peczenyj/structalign)](./LICENSE) -[![Latest release](https://img.shields.io/github/release/peczenyj/structalign.svg)](https://github.com/peczenyj/structalign/releases/latest) [![GitHub Release Date](https://img.shields.io/github/release-date/peczenyj/structalign.svg)](https://github.com/peczenyj/structalign/releases/latest) [![Last commit](https://img.shields.io/github/last-commit/peczenyj/structalign.svg)](https://github.com/peczenyj/structalign/commit/HEAD) [![PRs Welcome](https://img.shields.io/badge/PRs-welcome-brightgreen.svg)](https://github.com/peczenyj/structalign/blob/main/CONTRIBUTING.md#pull-request-process) [![SLSA Build Level 2](https://img.shields.io/badge/SLSA-Build_L2-green.svg)](https://github.com/peczenyj/structalign/attestations) [![OpenSSF Scorecard](https://api.securityscorecards.dev/projects/github.com/peczenyj/structalign/badge)](https://scorecard.dev/viewer/?uri=github.com/peczenyj/structalign) +[![OpenSSF Best Practices](https://bestpractices.coreinfrastructure.org/projects/13027/badge)](https://bestpractices.coreinfrastructure.org/projects/13027) [![Mentioned in Awesome Go](https://awesome.re/mentioned-badge-flat.svg)](https://github.com/avelino/awesome-go#code-analysis) > See how reordering a Go struct's fields could save memory — as a **diff**, not a @@ -27,9 +27,7 @@ also print any struct's offset/size/align/padding layout. The analysis comes straight from the upstream analyzer, so results match `fieldalignment` exactly — only the presentation is new. -

- structalign colored unified-diff output against the bundled sample -

+![structalign colored unified-diff output against the bundled sample](docs/diff.png) ## Quick start @@ -133,6 +131,7 @@ structalign [flags] [packages] single .go files (defaults the go tool understands) -diff value diff style: unified|side|none (default "unified") + -format value output format: text|json (default "text") -width int column width per side for -diff=side (default: auto from terminal) -color value colorize: auto|always|never (default "auto") -inspect inspect layout instead of diffing: print each struct as @@ -158,7 +157,7 @@ structalign [flags] [packages] -nolint-linters string //nolint tokens that suppress a finding (default "fieldalignment"; a bare //nolint always counts) - + -no-rc skip loading .structalignrc files -version print version and exit ``` @@ -166,7 +165,54 @@ 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. +### Configuration + +`structalign` supports persistent defaults via environment variables and +`.structalignrc` files. Precedence (highest wins): + +1. **CLI flags** (e.g. `structalign -sort`) +2. **Environment variables**: `STRUCTALIGN_`, e.g. `STRUCTALIGN_SORT=true`. +3. **Local config**: `.structalignrc` in the current directory. +4. **Global config**: `~/.structalignrc`. + +The configuration files use a simple `key = value` format: + +```ini +# .structalignrc example +sort = true +threshold = 8 +skip-cache-padded = true +``` + +Keys map directly to flag names. To skip loading configuration files (e.g. in +CI), use the `-no-rc` flag. Note that **theme** is not an RC key; set it via the +`STRUCTALIGN_THEME` environment variable. + +#### Configuration Reference + +| Feature | CLI Flag | Environment Variable | RC Key | Default | +|---------|----------|----------------------|--------|---------| +| Diff style | `-diff` | `STRUCTALIGN_DIFF` | `diff` | `unified` | +| Output format | `-format` | `STRUCTALIGN_FORMAT` | `format` | `text` | +| Column width | `-width` | `STRUCTALIGN_WIDTH` | `width` | `0` (auto) | +| Color mode | `-color` | `STRUCTALIGN_COLOR` | `color` | `auto` | +| Theme palette | — | `STRUCTALIGN_THEME` | — | `default` | +| Inspect mode | `-inspect` | `STRUCTALIGN_INSPECT` | `inspect` | `false` | +| Verbose inspect | `-verbose` | `STRUCTALIGN_VERBOSE` | `verbose` | `false` | +| Keep tags | `-tags` | `STRUCTALIGN_TAGS` | `tags` | `false` | +| Show summary | `-summary` | `STRUCTALIGN_SUMMARY` | `summary` | `false` | +| Largest-first sort | `-sort` | `STRUCTALIGN_SORT` | `sort` | `false` | +| Min bytes saved | `-threshold` | `STRUCTALIGN_THRESHOLD` | `threshold` | `0` | +| Type filter | `-type` | `STRUCTALIGN_TYPE` | `type` | (empty) | +| Package exclude | `-exclude` | `STRUCTALIGN_EXCLUDE` | `exclude` | `^unsafe$\|^builtin$` | +| Include generated | `-generated` | `STRUCTALIGN_GENERATED` | `generated` | `false` | +| Include tests | `-tests` | `STRUCTALIGN_TESTS` | `tests` | `false` | +| Skip cache padded | `-skip-cache-padded` | `STRUCTALIGN_SKIP_CACHE_PADDED` | `skip-cache-padded` | `false` | +| Show //nolint | `-show-nolint` | `STRUCTALIGN_SHOW_NOLINT` | `show-nolint` | `false` | +| Nolint linters | `-nolint-linters` | `STRUCTALIGN_NOLINT_LINTERS` | `nolint-linters` | `fieldalignment` | + The palette can be switched with the `STRUCTALIGN_THEME` environment variable — +... Applied fuzzy match at line 147. `default` (the standard colors), `cga` (the iconic cyan/magenta/white CGA palette, with a reverse-video header bar), or `green` / `amber` (single-hue phosphor-monitor emulations). It only affects *which* colors @@ -328,6 +374,36 @@ inspect prints a *struct field layout*, and scalars have no fields. (The see a scalar's size, inspect a struct that contains it — a `string` field shows `size: 16` on a 64-bit target. +### JSON output + +`-format=json` (or `STRUCTALIGN_FORMAT=json`, or `format = json` in +`.structalignrc`) emits a single structured document instead of the rendered +text, for both diff and inspect modes. It carries the same data the text +renderers show — findings include `original` / `proposed`, `oldSize` / +`newSize` / `bytesSaved`; inspect layouts include per-field +`offset` / `size` / `align` / `padding` and the generic `assume` notes. + +``` +$ structalign -format=json -type=Mixed ./_example +{ + "version": "...", + "mode": "diff", + "findings": [ ... ], + "summary": { "structsAffected": 1, "bytesSaved": 8 } +} +``` + +Two things differ from text mode by design: + +- **The diff document always includes the `summary` block** (so a machine + consumer always gets the totals). `-summary` only governs the text renderer's + trailing summary line. +- **The presentation flags don't apply.** `-diff`, `-summary`, `-verbose`, + `-color`, and `-width` shape the *text* output only; in JSON mode they are + ignored, since the consumer renders from the structured fields itself. + `-tags` still applies — it gates whether the inspect document's per-field + `tag` field is emitted (see [Field tags](#field-tags)). + ### Filtering by type name `-type` takes a comma-separated list of glob patterns (`path.Match` syntax: `*`, @@ -343,7 +419,7 @@ structalign -inspect -type='*ID*' ./pkg # inspect just ID-related structs ### Scanning scope -By default structalign analyzes the regular, hand-written source of each package. +By default, structalign analyzes the regular, hand-written source of each package. A few flags adjust what's in scope: ```sh @@ -393,7 +469,10 @@ type Tagged struct { // size: 48, align: 8, padding: 18 ``` Tags never affect the layout numbers (size/offset/alignment are independent of -tags), so stripping them changes only the display, never the analysis. +tags), so stripping them changes only the display, never the analysis. The same +flag governs JSON output: with `-format=json`, the inspect document's `tag` +field is emitted only when `-tags` (or `STRUCTALIGN_TAGS=true`, or `tags = true` +in `.structalignrc`) is in effect. ## How it works @@ -476,6 +555,22 @@ diffing uses `go-udiff` rather than x/tools' own diff package: compiler rejects it. `go-udiff` is a public port of the same gopls diff code, so the results are equivalent. +## Easter eggs + +A few hidden flags are intentionally kept out of `-help` and the flag table above: +**`-cga`, `-green`, `-amber`** — shortcuts for the retro-theme palettes otherwise +selected with `STRUCTALIGN_THEME=…`. Pick one per invocation; the egg flag wins +over the environment variable. + +```sh +structalign -cga ./... # cyan/magenta/white CGA palette +structalign -green -inspect ./_example # single-hue green-phosphor look +structalign -amber -diff=side ./... # amber-phosphor side-by-side diff +``` + +They are stripped before flag parsing, so they never trip *"flag provided but not +defined"* when combined with normal flags. + ## Changelog See [CHANGELOG.md](CHANGELOG.md). Commits follow diff --git a/docs/diff.png b/docs/diff.png index 1ea8c53..46ce807 100644 Binary files a/docs/diff.png and b/docs/diff.png differ diff --git a/internal/align/align.go b/internal/align/align.go index c7c836a..67c5aae 100644 --- a/internal/align/align.go +++ b/internal/align/align.go @@ -12,7 +12,7 @@ import ( "go/types" "os" "regexp" - "sort" + "slices" "strconv" "strings" @@ -43,6 +43,7 @@ func (a *Aligner) Findings(t common.Target, opts common.Options) ([]common.Findi return nil, nil } names := structNameIndex(t.Syntax) + structs := structTypeIndex(t.Syntax, t.TypesInfo) nolints := nolintIndex(t.Syntax, t.Fset) insp := inspector.New(t.Syntax) @@ -56,7 +57,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, nolints, opts) + f := buildFinding(t, d, names, structs, nolints, opts) if f == nil { return } @@ -66,12 +67,12 @@ func (a *Aligner) Findings(t common.Target, opts common.Options) ([]common.Findi if _, err := fieldalignment.Analyzer.Run(pass); err != nil { return nil, fmt.Errorf("%s: analyzer: %w", t.PkgPath, err) } - sort.Slice(findings, func(i, j int) bool { - pi, pj := t.Fset.Position(findings[i].Pos), t.Fset.Position(findings[j].Pos) - if pi.Filename != pj.Filename { - return pi.Filename < pj.Filename + slices.SortFunc(findings, func(a, b common.Finding) int { + pa, pb := t.Fset.Position(a.Pos), t.Fset.Position(b.Pos) + if pa.Filename != pb.Filename { + return strings.Compare(pa.Filename, pb.Filename) } - return pi.Offset < pj.Offset + return pa.Offset - pb.Offset }) return findings, nil } @@ -79,20 +80,23 @@ 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, nolints map[token.Pos]nolintInfo, opts common.Options) *common.Finding { - f := common.Finding{Fset: t.Fset, Pos: d.Pos, Message: d.Message} +func buildFinding(t common.Target, d analysis.Diagnostic, names map[token.Pos]string, structs map[token.Pos]*types.Struct, nolints map[token.Pos]nolintInfo, opts common.Options) *common.Finding { + f := common.Finding{Package: t.PkgPath, 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] f.Pos = e.Pos f.Proposed = string(e.NewText) f.Original = readSource(t.Fset, e.Pos, e.End) - if !opts.KeepTags { - if s, err := stripStructTags(f.Original); err == nil { - f.Original = s - } - if s, err := stripStructTags(f.Proposed); err == nil { - f.Proposed = s - } + // Upstream fieldalignment discards per-field comments from the proposed + // text (it clears each field's Doc/Comment; see golang/go#20744). Run + // the original through the same normalization so the side-by-side diff + // shows only the reordering, not phantom comment deletions. Tags are + // kept on both sides only when KeepTags is set. + if s, err := normalizeStruct(f.Original, opts.KeepTags); err == nil { + f.Original = s + } + if s, err := normalizeStruct(f.Proposed, opts.KeepTags); err == nil { + f.Proposed = s } } f.Name = names[f.Pos] @@ -105,7 +109,7 @@ func buildFinding(t common.Target, d analysis.Diagnostic, names map[token.Pos]st if !opts.IncludeGenerated && structfilter.InGeneratedFile(t, f.Pos) { return nil } - if opts.SkipCachePadded && isCachePadded(t, f.Name) { + if opts.SkipCachePadded && isCachePadded(t, f.Pos, f.Name, structs) { return nil } if opts.RespectNolint { @@ -116,9 +120,12 @@ func buildFinding(t common.Target, d analysis.Diagnostic, names map[token.Pos]st return &f } -// isCachePadded reports whether the named type in t's scope contains a -// cpu.CacheLinePad field. Returns false for anonymous structs or missing names. -func isCachePadded(t common.Target, name string) bool { +// isCachePadded reports whether the struct at pos (or the named type name) +// contains a cpu.CacheLinePad field. +func isCachePadded(t common.Target, pos token.Pos, name string, structs map[token.Pos]*types.Struct) bool { + if st, ok := structs[pos]; ok { + return structfilter.HasCacheLinePad(st) + } if name == "" { return false } @@ -144,7 +151,8 @@ func typeParamNames(t common.Target, name string) string { if !ok { return "" } - named, ok := tn.Type().(*types.Named) + typ := types.Unalias(tn.Type()) + named, ok := typ.(*types.Named) if !ok { return "" } @@ -193,6 +201,25 @@ func structNameIndex(files []*ast.File) map[token.Pos]string { return index } +// structTypeIndex maps the position of every StructType node in the syntax to +// its underlying types.Struct, for both named and anonymous structs. +func structTypeIndex(files []*ast.File, info *types.Info) map[token.Pos]*types.Struct { + index := make(map[token.Pos]*types.Struct) + for _, f := range files { + ast.Inspect(f, func(n ast.Node) bool { + if st, ok := n.(*ast.StructType); ok { + if tv, ok := info.Types[st]; ok { + if s, ok := tv.Type.Underlying().(*types.Struct); ok { + index[st.Pos()] = s + } + } + } + return true + }) + } + return index +} + // readSource returns the raw source text between two positions. func readSource(fset *token.FileSet, pos, end token.Pos) string { pf := fset.File(pos) @@ -211,15 +238,17 @@ func readSource(fset *token.FileSet, pos, end token.Pos) string { return string(data[start:stop]) } -// stripStructTags removes field tags from a struct type's source text. It -// parses the text (wrapped as a type declaration), clears each field's Tag, and -// reprints it with go/format, which also re-aligns the now-tagless fields. On -// any parse/format error it returns the error so the caller can fall back to -// the original text. -func stripStructTags(src string) (string, error) { +// normalizeStruct reprints a struct type's source text with per-field comments +// removed and, unless keepTags is set, field tags cleared. Comments are dropped +// by parsing without parser.ParseComments, so they never enter the AST and +// go/format does not emit them; the reprint also re-aligns the fields. This +// mirrors the normalization upstream fieldalignment applies to the proposed +// text, keeping both sides of the diff symmetric. On any parse/format error it +// returns the error so the caller can fall back to the original text. +func normalizeStruct(src string, keepTags bool) (string, error) { wrapped := "package p\ntype _ " + src fset := token.NewFileSet() - file, err := parser.ParseFile(fset, "", wrapped, parser.ParseComments) + file, err := parser.ParseFile(fset, "", wrapped, parser.SkipObjectResolution) if err != nil { return "", err } @@ -234,7 +263,7 @@ func stripStructTags(src string) (string, error) { if st == nil { return "", fmt.Errorf("no struct type found") } - if st.Fields != nil { + if !keepTags && st.Fields != nil { for _, fld := range st.Fields.List { fld.Tag = nil } diff --git a/internal/align/align_test.go b/internal/align/align_test.go index 53cad63..26568f1 100644 --- a/internal/align/align_test.go +++ b/internal/align/align_test.go @@ -67,6 +67,45 @@ type Mixed struct { } ` +const commentedSrc = `package sample + +type Commented struct { + A bool ` + "`json:\"a\"`" + ` // trailing on A + // leading on B + B int64 ` + "`json:\"b\"`" + ` + C bool +} +` + +func TestFindingsStripsCommentsFromBothSides(t *testing.T) { + tgt := testutil.Target(t, commentedSrc) + findings, err := align.New().Findings(tgt, common.Options{Patterns: []string{"Commented"}}) + require.NoError(t, err) + require.Len(t, findings, 1) + + // Upstream drops comments from the proposed text; we strip them from the + // original too so the diff shows only the reordering (see issue #88). + for _, side := range []string{findings[0].Original, findings[0].Proposed} { + assert.NotContains(t, side, "// trailing on A") + assert.NotContains(t, side, "// leading on B") + } +} + +func TestFindingsKeepTagsStillStripsComments(t *testing.T) { + tgt := testutil.Target(t, commentedSrc) + findings, err := align.New().Findings(tgt, common.Options{Patterns: []string{"Commented"}, KeepTags: true}) + require.NoError(t, err) + require.Len(t, findings, 1) + + // Comments go regardless of KeepTags; tags survive on both sides. + for _, side := range []string{findings[0].Original, findings[0].Proposed} { + assert.NotContains(t, side, "// trailing on A") + assert.NotContains(t, side, "// leading on B") + assert.Contains(t, side, `json:"a"`) + assert.Contains(t, side, `json:"b"`) + } +} + func TestFindingsSkipsGenerated(t *testing.T) { tgt := testutil.Target(t, generatedSrc) @@ -198,3 +237,38 @@ type WithPad struct { require.NoError(t, ferr) assert.Empty(t, findings, "WithPad should be skipped when SkipCachePadded=true") } + +func TestFindingsSkipsAnonymousCachePadded(t *testing.T) { + root := moduleRoot(t) + testPkgDir := filepath.Join(root, "internal", "align", "_anoncachepadtest") + if err := os.MkdirAll(testPkgDir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + t.Cleanup(func() { os.RemoveAll(testPkgDir) }) + + src := `package anoncachepadtest + +import "golang.org/x/sys/cpu" + +var V = struct { + A bool + B int64 + C bool + _ cpu.CacheLinePad +}{A: true, B: 1, C: true} +` + err := os.WriteFile(filepath.Join(testPkgDir, "types.go"), []byte(src), 0o600) + require.NoError(t, err) + + tgt := loadPackageTarget(t, testPkgDir) + + // Without SkipCachePadded, the anonymous struct should be reported. + findings, ferr := align.New().Findings(tgt, common.Options{}) + require.NoError(t, ferr) + assert.NotEmpty(t, findings, "Anonymous struct should be reported without SkipCachePadded") + + // With SkipCachePadded=true, the anonymous struct should be silently skipped. + findings, ferr = align.New().Findings(tgt, common.Options{SkipCachePadded: true}) + require.NoError(t, ferr) + assert.Empty(t, findings, "Anonymous struct should be skipped when SkipCachePadded=true") +} diff --git a/internal/align/internal_test.go b/internal/align/internal_test.go new file mode 100644 index 0000000..35a9fd9 --- /dev/null +++ b/internal/align/internal_test.go @@ -0,0 +1,75 @@ +package align + +import ( + "go/token" + "go/types" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/peczenyj/structalign/pkg/common" +) + +func TestIsCachePaddedFallback(t *testing.T) { + pkg := types.NewPackage("golang.org/x/sys/cpu", "cpu") + padType := types.NewNamed(types.NewTypeName(token.NoPos, pkg, "CacheLinePad", nil), types.Universe.Lookup("int").Type(), nil) + field := types.NewField(token.NoPos, pkg, "_", padType, false) + st := types.NewStruct([]*types.Var{field}, nil) + + samplePkg := types.NewPackage("sample", "sample") + samplePkg.Scope().Insert(types.NewTypeName(token.NoPos, samplePkg, "WithPad", types.NewNamed(types.NewTypeName(token.NoPos, samplePkg, "WithPad", nil), st, nil))) + + tgt := common.Target{Types: samplePkg} + structs := make(map[token.Pos]*types.Struct) + // Empty map ensures we hit the fallback. + + // Named type that exists + assert.True(t, isCachePadded(tgt, token.NoPos, "WithPad", structs)) + + // Named type that doesn't exist + assert.False(t, isCachePadded(tgt, token.NoPos, "NoSuchType", structs)) + + // Anonymous fallback + assert.False(t, isCachePadded(tgt, token.NoPos, "", structs)) +} + +func TestNormalizeStructError(t *testing.T) { + _, err := normalizeStruct("invalid struct {", false) + assert.Error(t, err) + + _, err = normalizeStruct("int", false) // not a struct + assert.Error(t, err) +} + +func TestNormalizeStructStripsComments(t *testing.T) { + src := "struct {\n\tA string // trailing\n\t// leading\n\tB int64\n}" + + got, err := normalizeStruct(src, false) + assert.NoError(t, err) + assert.NotContains(t, got, "// trailing") + assert.NotContains(t, got, "// leading") + assert.Contains(t, got, "A string") + assert.Contains(t, got, "B int64") +} + +func TestFindingsNoSyntax(t *testing.T) { + a := Aligner{} + f, err := a.Findings(common.Target{}, common.Options{}) + assert.NoError(t, err) + assert.Nil(t, f) +} + +func TestReadSourceErrors(t *testing.T) { + fset := token.NewFileSet() + // pf == nil + assert.Empty(t, readSource(fset, token.NoPos, token.NoPos)) + + f := fset.AddFile("missing.go", -1, 10) + // os.ReadFile error + assert.Empty(t, readSource(fset, f.Pos(0), f.Pos(5))) + + // The remaining bounds guards (start < 0, start > stop, stop > len(data)) + // are intentionally not covered here: readSource derives both offsets from + // valid token.Pos values off the same file, so they can't be provoked + // without a corrupt FileSet. The guards stay as defense in depth. +} diff --git a/internal/align/nolint_block_test.go b/internal/align/nolint_block_test.go index 74647b1..2521a16 100644 --- a/internal/align/nolint_block_test.go +++ b/internal/align/nolint_block_test.go @@ -64,7 +64,7 @@ func TestFindingsNolintEmptyGroup(t *testing.T) { _, _ = align.New().Findings(tgt, common.Options{RespectNolint: true}) } -func TestParseNolintInvalid(t *testing.T) { +func TestParseNolintInvalid(_ *testing.T) { // Exercise the default branch in parseNolint switch // Use internal package via link or just call it if internal tests allowed. // Actually, this file is align_test. I can't call internal functions unless I use a link. diff --git a/internal/app/app.go b/internal/app/app.go index 8e07147..b5e44c8 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -9,10 +9,11 @@ import ( "os" "regexp" "runtime/debug" - "sort" + "slices" "strings" "github.com/peczenyj/structalign/internal/align" + "github.com/peczenyj/structalign/internal/config" "github.com/peczenyj/structalign/internal/layout" "github.com/peczenyj/structalign/internal/loader" "github.com/peczenyj/structalign/internal/match" @@ -60,6 +61,7 @@ func New(stdout, stderr io.Writer) *App { type options struct { diff common.DiffStyle + format common.Format width int colorize common.Colorize typeFilter string @@ -76,6 +78,7 @@ type options struct { threshold int showNolint bool nolintLinters string + noRC bool } // savings is the absolute bytes a finding saves, or 0 when sizes are unknown or @@ -98,6 +101,8 @@ func (a *App) Run(args []string) int { fs.SetOutput(a.Stderr) opt.diff = common.DiffUnified // zero value is DiffUnified; set for clarity fs.Var(&opt.diff, "diff", fmt.Sprintf("diff style: %s (default %q)", opt.diff.Type(), opt.diff.String())) + opt.format = common.FormatText // zero value is FormatText; set for clarity + fs.Var(&opt.format, "format", fmt.Sprintf("output format: %s (default %q)", opt.format.Type(), opt.format.String())) fs.IntVar(&opt.width, "width", 0, "column width per side for -diff=side (0 = auto from terminal)") opt.colorize = common.ColorizeAuto // zero value is ColorizeAuto; set for clarity fs.Var(&opt.colorize, "color", fmt.Sprintf("colorize: %s (default %q)", opt.colorize.Type(), opt.colorize.String())) @@ -115,6 +120,7 @@ func (a *App) Run(args []string) int { fs.IntVar(&opt.threshold, "threshold", 0, "in diff mode, only show structs that save at least this many bytes") fs.BoolVar(&opt.showNolint, "show-nolint", false, "show structs even when their type carries a recognized //nolint directive") fs.StringVar(&opt.nolintLinters, "nolint-linters", "fieldalignment", "comma-separated //nolint tokens that suppress a finding (bare //nolint always counts)") + fs.BoolVar(&opt.noRC, "no-rc", false, "skip loading .structalignrc files") fs.Usage = func() { fmt.Fprint(a.Stderr, //nolint:errcheck "structalign: show how a struct's fields could be reordered to use less memory\n\n", @@ -134,7 +140,8 @@ func (a *App) Run(args []string) int { "examples:\n", " structalign ./... scan every package in the module\n", " structalign -diff=side -summary ./... side-by-side diff plus a total\n", - " structalign -inspect -type=Config ./pkg one struct's per-field layout\n\n", + " structalign -inspect -type=Config ./pkg one struct's per-field layout\n", + " structalign -format=json ./... machine-readable findings\n\n", "flags:\n") fs.PrintDefaults() } @@ -155,7 +162,37 @@ func (a *App) Run(args []string) int { // Easter-egg theme flags: -cga/-green/-amber select a retro palette. Like // -fix, they are caught before parsing and stripped from args, so they stay // invisible in -help and never trip "flag provided but not defined". - themeName, args := a.stripEggFlags(args) + // Also peeks at -no-rc so RC loading (which precedes fs.Parse) can honor it; + // the flag itself is still registered with fs so fs.Parse binds opt.noRC and + // -h lists it like any other config flag. + themeName, noRC, args := a.scanEarlyFlags(args) + + // Apply configuration layers as defaults. + if !noRC { + home, _ := os.UserHomeDir() + cwd, _ := os.Getwd() + for k, v := range config.Load(home, cwd) { + // Silently ignore keys that don't map to a flag: this covers the + // documented "theme is not an RC key" exclusion as well as typos, + // so a stray key never surfaces a "no such flag" warning. A key + // that *is* a flag but gets a bad value still warns below. + if fs.Lookup(k) == nil { + continue + } + if err := fs.Set(k, v); err != nil { + fmt.Fprintf(a.Stderr, "structalign: config: %s: %v\n", k, err) + } + } + } + + // Apply environment variables. + fs.VisitAll(func(f *flag.Flag) { + if val := os.Getenv(config.EnvName(f.Name)); val != "" { + if err := fs.Set(f.Name, val); err != nil { + fmt.Fprintf(a.Stderr, "structalign: env: %s: %v\n", f.Name, err) + } + } + }) if err := fs.Parse(args); err != nil { return 2 @@ -196,6 +233,7 @@ func (a *App) Run(args []string) int { printer := &ui.Printer{ Out: a.Stdout, + Err: a.Stderr, Color: ui.WantColor(opt.colorize, stdoutFile(a.Stdout)), Width: width, Theme: theme, @@ -211,7 +249,9 @@ func (a *App) Run(args []string) int { fmt.Fprintf(a.Stderr, "structalign: %v\n", err) return 2 } - sort.Slice(targets, func(i, j int) bool { return targets[i].PkgPath < targets[j].PkgPath }) + slices.SortFunc(targets, func(a, b common.Target) int { + return strings.Compare(a.PkgPath, b.PkgPath) + }) var allFindings []common.Finding var allLayouts []common.Layout @@ -246,10 +286,10 @@ func (a *App) Run(args []string) int { } if !opt.inspect && opt.threshold > 0 { - min := int64(opt.threshold) + minSavings := int64(opt.threshold) kept := allFindings[:0] for _, f := range allFindings { - if savings(f) >= min { + if savings(f) >= minSavings { kept = append(kept, f) } } @@ -258,34 +298,44 @@ func (a *App) Run(args []string) int { if opt.sort { if opt.inspect { - sort.SliceStable(allLayouts, func(i, j int) bool { - return allLayouts[i].Total > allLayouts[j].Total + slices.SortStableFunc(allLayouts, func(a, b common.Layout) int { + return cmp(b.Total, a.Total) }) } else { - sort.SliceStable(allFindings, func(i, j int) bool { - return savings(allFindings[i]) > savings(allFindings[j]) + slices.SortStableFunc(allFindings, func(a, b common.Finding) int { + return cmp(savings(b), savings(a)) }) } } var total int - if opt.inspect { - total = printer.RenderLayouts(allLayouts, opt.verbose, opt.tags) - } else { - total = printer.RenderFindings(allFindings, opt.diff) - } - - if opt.summary && !opt.inspect { - var bytesSaved int64 - for _, f := range allFindings { - bytesSaved += savings(f) + if opt.format == common.FormatJSON { + if opt.inspect { + total = len(allLayouts) + printer.RenderJSON(resolveVersion(), true, nil, allLayouts, opt.tags) + } else { + total = len(allFindings) + printer.RenderJSON(resolveVersion(), false, allFindings, nil, opt.tags) } - printer.RenderSummary(total, bytesSaved) - } else if total == 0 { + } else { if opt.inspect { - fmt.Fprintln(a.Stderr, "no matching structs found") + total = printer.RenderLayouts(allLayouts, opt.verbose, opt.tags) } else { - fmt.Fprintln(a.Stderr, "no struct reorderings found") + total = printer.RenderFindings(allFindings, opt.diff) + } + + if opt.summary && !opt.inspect { + var bytesSaved int64 + for _, f := range allFindings { + bytesSaved += savings(f) + } + printer.RenderSummary(total, bytesSaved) + } else if total == 0 { + if opt.inspect { + fmt.Fprintln(a.Stderr, "structalign: no matching structs found") + } else { + fmt.Fprintln(a.Stderr, "structalign: no struct reorderings found") + } } } if total > 0 && !opt.inspect { @@ -294,13 +344,31 @@ func (a *App) Run(args []string) int { return 0 } +func cmp[T ~int | ~int64](a, b T) int { + if a < b { + return -1 + } + if a > b { + return 1 + } + return 0 +} + var eggRE = regexp.MustCompile(`^--?([^=]+)(?:=(.*))?$`) -// stripEggFlags scans args for retro-theme "easter egg" flags, returning the -// chosen theme name and the args slice with those flags removed. It stops at -// the first "--" separator. -func (a *App) stripEggFlags(args []string) (theme string, filtered []string) { - filtered = args[:0:0] +// scanEarlyFlags peeks at args ahead of fs.Parse for two reasons: the +// retro-theme "easter egg" flags (-cga/-green/-amber) are not registered with +// the FlagSet (they stay invisible in -help) and must be stripped so fs.Parse +// doesn't reject them; and -no-rc must be known before RC loading runs. +// Returns the chosen theme name, whether RC loading is disabled, and the args +// slice. The egg flags are stripped; -no-rc is left in place so fs.Parse can +// also bind it to opt.noRC. Stops at the first "--" separator. +// +//nolint:gocyclo // parsing early flags and easter eggs naturally involves high branching +func (a *App) scanEarlyFlags(args []string) (theme string, noRC bool, filtered []string) { + noRCEnv := os.Getenv("STRUCTALIGN_NO_RC") + noRC = noRCEnv == "true" || noRCEnv == "1" + filtered = make([]string, 0, len(args)) afterDD := false for _, arg := range args { if afterDD || arg == "--" { @@ -311,6 +379,13 @@ func (a *App) stripEggFlags(args []string) (theme string, filtered []string) { if m := eggRE.FindStringSubmatch(arg); m != nil { name, val := m[1], m[2] + if name == "no-rc" { + if !strings.Contains(arg, "=") || val == "true" || val == "1" { + noRC = true + } + filtered = append(filtered, arg) + continue + } if name == "cga" || name == "green" || name == "amber" { if !strings.Contains(arg, "=") || val == "true" || val == "1" { theme = name @@ -320,7 +395,7 @@ func (a *App) stripEggFlags(args []string) (theme string, filtered []string) { } filtered = append(filtered, arg) } - return theme, filtered + return theme, noRC, filtered } // stdoutFile returns the *os.File behind w for terminal queries, or nil when diff --git a/internal/app/config_test.go b/internal/app/config_test.go new file mode 100644 index 0000000..92cef23 --- /dev/null +++ b/internal/app/config_test.go @@ -0,0 +1,129 @@ +package app_test + +import ( + "bytes" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "github.com/peczenyj/structalign/internal/app" + "github.com/peczenyj/structalign/internal/mocks" + "github.com/peczenyj/structalign/pkg/common" +) + +func TestRunLayeredConfig(t *testing.T) { + tmp := t.TempDir() + home := filepath.Join(tmp, "home") + cwd := filepath.Join(tmp, "cwd") + require.NoError(t, os.Mkdir(home, 0o755)) + require.NoError(t, os.Mkdir(cwd, 0o755)) + + // Mock HOME for config.Load + t.Setenv("HOME", home) + + // Mock CWD + oldCWD, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(cwd)) + t.Cleanup(func() { _ = os.Chdir(oldCWD) }) + + // 1. Home RC: threshold = 8 + require.NoError(t, os.WriteFile(filepath.Join(home, ".structalignrc"), []byte("threshold = 8\n"), 0o644)) + + // 2. CWD RC: sort = true, threshold = 16 (overrides home) + require.NoError(t, os.WriteFile(filepath.Join(cwd, ".structalignrc"), []byte("sort = true\nthreshold = 16\n"), 0o644)) + + // 3. Env Var: STRUCTALIGN_THRESHOLD = 32 (overrides CWD RC) + t.Setenv("STRUCTALIGN_THRESHOLD", "32") + + // We want to verify these defaults are set in the flagset. + // Since we can't easily inspect the internal 'opt' struct, we can check + // the usage message or use an easter egg if we had one. + // Actually, we can check if it warns on invalid values from these layers. + + var out, errb bytes.Buffer + ml := &mocks.Loader{} + ma := &mocks.Aligner{} + a := &app.App{Loader: ml, Aligner: ma, Stdout: &out, Stderr: &errb} + + ml.On("Load", "pkg").Return([]common.Target{{PkgPath: "pkg"}}, nil) + ma.On("Findings", mock.Anything, mock.Anything).Return(nil, nil) + + t.Run("Precedence", func(t *testing.T) { + t.Setenv("STRUCTALIGN_THRESHOLD", "garbage") + a.Run([]string{"pkg"}) + assert.Contains(t, errb.String(), "structalign: env: threshold: parse error") + errb.Reset() + }) + + t.Run("NoRC", func(t *testing.T) { + // With -no-rc, threshold should come from Env (garbage -> error) + // but sort (from CWD RC) should NOT be set. + t.Setenv("STRUCTALIGN_THRESHOLD", "32") + // We'll use an invalid key in RC to see if it warns + require.NoError(t, os.WriteFile(filepath.Join(cwd, ".structalignrc"), []byte("invalid-key = true\n"), 0o644)) + + a.Run([]string{"-no-rc", "pkg"}) + assert.NotContains(t, errb.String(), "config: invalid-key") + errb.Reset() + }) + + t.Run("NoRCEnv", func(t *testing.T) { + t.Setenv("STRUCTALIGN_NO_RC", "true") + require.NoError(t, os.WriteFile(filepath.Join(cwd, ".structalignrc"), []byte("invalid-key = true\n"), 0o644)) + + a.Run([]string{"pkg"}) + assert.NotContains(t, errb.String(), "config: invalid-key") + errb.Reset() + }) +} + +// TestRunRCUnknownKeys verifies that RC keys which don't map to a flag — the +// documented "theme is not an RC key" exclusion, and outright typos — are +// skipped silently, while a real flag given a bad value still warns. +func TestRunRCUnknownKeys(t *testing.T) { + tmp := t.TempDir() + t.Setenv("HOME", tmp) // empty home rc + + cwd := filepath.Join(tmp, "cwd") + require.NoError(t, os.Mkdir(cwd, 0o755)) + oldCWD, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(cwd)) + t.Cleanup(func() { _ = os.Chdir(oldCWD) }) + + run := func(t *testing.T, rc string) string { + t.Helper() + require.NoError(t, os.WriteFile(filepath.Join(cwd, ".structalignrc"), []byte(rc), 0o644)) + + var out, errb bytes.Buffer + ml := &mocks.Loader{} + ma := &mocks.Aligner{} + ml.On("Load", "pkg").Return([]common.Target{{PkgPath: "pkg"}}, nil) + ma.On("Findings", mock.Anything, mock.Anything).Return(nil, nil) + a := &app.App{Loader: ml, Aligner: ma, Stdout: &out, Stderr: &errb} + a.Run([]string{"pkg"}) + return errb.String() + } + + t.Run("ThemeKeyIsSilent", func(t *testing.T) { + out := run(t, "theme = cga\n") + assert.NotContains(t, out, "no such flag") + assert.NotContains(t, out, "theme") + }) + + t.Run("TypoIsSilent", func(t *testing.T) { + out := run(t, "totally-bogus-key = 1\n") + assert.NotContains(t, out, "no such flag") + assert.NotContains(t, out, "totally-bogus-key") + }) + + t.Run("BadValueForRealFlagWarns", func(t *testing.T) { + out := run(t, "threshold = garbage\n") + assert.Contains(t, out, "structalign: config: threshold:") + }) +} diff --git a/internal/app/internal_test.go b/internal/app/internal_test.go new file mode 100644 index 0000000..12f370f --- /dev/null +++ b/internal/app/internal_test.go @@ -0,0 +1,21 @@ +package app + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/peczenyj/structalign/pkg/common" +) + +func TestSavings(t *testing.T) { + assert.Equal(t, int64(8), savings(common.Finding{OldSize: 24, NewSize: 16})) + assert.Equal(t, int64(0), savings(common.Finding{OldSize: 16, NewSize: 24})) + assert.Equal(t, int64(0), savings(common.Finding{OldSize: 0, NewSize: 16})) +} + +func TestCmp(t *testing.T) { + assert.Equal(t, -1, cmp(10, 20)) + assert.Equal(t, 1, cmp(20, 10)) + assert.Equal(t, 0, cmp(10, 10)) +} diff --git a/internal/app/json_test.go b/internal/app/json_test.go new file mode 100644 index 0000000..230084e --- /dev/null +++ b/internal/app/json_test.go @@ -0,0 +1,84 @@ +package app_test + +import ( + "bytes" + "encoding/json" + "go/types" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "github.com/peczenyj/structalign/internal/app" + "github.com/peczenyj/structalign/internal/mocks" + "github.com/peczenyj/structalign/pkg/common" +) + +type dummySizes struct{ common.Sizes } + +func TestRunJSONFormat(t *testing.T) { + var out, errb bytes.Buffer + ml := &mocks.Loader{} + ma := &mocks.Aligner{} + a := &app.App{Loader: ml, Aligner: ma, Stdout: &out, Stderr: &errb} + + tgt := common.Target{ + PkgPath: "pkg", + Types: &types.Package{}, + Sizes: dummySizes{}, + } + finding := common.Finding{ + Package: "pkg", + Name: "Mixed", + OldSize: 24, + NewSize: 16, + } + + ml.On("Load", "pkg").Return([]common.Target{tgt}, nil) + ma.On("Findings", mock.Anything, mock.Anything).Return([]common.Finding{finding}, nil) + + code := a.Run([]string{"-format=json", "pkg"}) + assert.Equal(t, 1, code, "exit 1 when findings exist") + assert.Empty(t, errb.String(), "no 'no reorderings' message in JSON mode") + + var doc struct { + Mode string `json:"mode"` + Findings []any `json:"findings"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &doc)) + assert.Equal(t, "diff", doc.Mode) + assert.Len(t, doc.Findings, 1) +} + +func TestRunJSONInspect(t *testing.T) { + var out, errb bytes.Buffer + ml := &mocks.Loader{} + mi := &mocks.Inspector{} + a := &app.App{Loader: ml, Inspector: mi, Stdout: &out, Stderr: &errb} + + tgt := common.Target{ + PkgPath: "pkg", + Types: &types.Package{}, + Sizes: dummySizes{}, + } + layout := common.Layout{ + Package: "pkg", + Name: "Mixed", + Total: 24, + } + + ml.On("Load", "pkg").Return([]common.Target{tgt}, nil) + mi.On("Layouts", mock.Anything, mock.Anything).Return([]common.Layout{layout}) + + code := a.Run([]string{"-inspect", "-format=json", "pkg"}) + assert.Equal(t, 0, code, "inspect always exits 0") + + var doc struct { + Mode string `json:"mode"` + Layouts []any `json:"layouts"` + } + require.NoError(t, json.Unmarshal(out.Bytes(), &doc)) + assert.Equal(t, "inspect", doc.Mode) + assert.Len(t, doc.Layouts, 1) +} diff --git a/internal/app/usage_test.go b/internal/app/usage_test.go index a7715e0..1d49e85 100644 --- a/internal/app/usage_test.go +++ b/internal/app/usage_test.go @@ -34,6 +34,7 @@ func TestUsageHasManPageSections(t *testing.T) { // PrintDefaults still emits the real flags after the header. assert.Contains(t, u, "-diff", "the flag list is still printed") assert.Contains(t, u, "-inspect") + assert.Contains(t, u, "-no-rc", "the -no-rc config flag is discoverable in -h") } func TestUsageOmitsEasterEggs(t *testing.T) { diff --git a/internal/config/config.go b/internal/config/config.go new file mode 100644 index 0000000..c1db967 --- /dev/null +++ b/internal/config/config.go @@ -0,0 +1,73 @@ +// Package config handles persistent defaults via environment variables and +// .structalignrc files. +package config + +import ( + "bufio" + "os" + "path/filepath" + "strings" +) + +// EnvName derives the environment variable name for a flag name, +// e.g. "skip-cache-padded" -> "STRUCTALIGN_SKIP_CACHE_PADDED". +func EnvName(flagName string) string { + name := strings.ReplaceAll(flagName, "-", "_") + return "STRUCTALIGN_" + strings.ToUpper(name) +} + +// Load reads and merges .structalignrc files from the home directory and the +// current working directory. CWD settings override home settings. +// Returns the merged key-value map. +func Load(home, cwd string) map[string]string { + merged := make(map[string]string) + + // Home directory rc (personal base) + if home != "" { + merge(merged, parseRC(filepath.Join(home, ".structalignrc"))) + } + + // CWD directory rc (project overrides) + if cwd != "" { + merge(merged, parseRC(filepath.Join(cwd, ".structalignrc"))) + } + + return merged +} + +func merge(dst, src map[string]string) { + for k, v := range src { + dst[k] = v + } +} + +// parseRC reads a key = value file, ignoring # comments and blank lines. +func parseRC(path string) map[string]string { + kv := make(map[string]string) + f, err := os.Open(path) + if err != nil { + return kv + } + defer f.Close() //nolint:errcheck + + sc := bufio.NewScanner(f) + for sc.Scan() { + line := strings.TrimSpace(sc.Text()) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + + // Split at the first '=' + parts := strings.SplitN(line, "=", 2) + if len(parts) != 2 { + continue + } + + k := strings.TrimSpace(parts[0]) + v := strings.TrimSpace(parts[1]) + if k != "" { + kv[k] = v + } + } + return kv +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go new file mode 100644 index 0000000..e25abae --- /dev/null +++ b/internal/config/config_test.go @@ -0,0 +1,63 @@ +package config_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/peczenyj/structalign/internal/config" +) + +func TestEnvName(t *testing.T) { + assert.Equal(t, "STRUCTALIGN_SORT", config.EnvName("sort")) + assert.Equal(t, "STRUCTALIGN_SKIP_CACHE_PADDED", config.EnvName("skip-cache-padded")) +} + +func TestLoad(t *testing.T) { + tmp := t.TempDir() + home := filepath.Join(tmp, "home") + cwd := filepath.Join(tmp, "cwd") + require.NoError(t, os.Mkdir(home, 0o755)) + require.NoError(t, os.Mkdir(cwd, 0o755)) + + homeRC := ` +# Personal base +sort = true +threshold = 8 +` + cwdRC := ` +# Project override +threshold = 16 +skip-cache-padded = true +` + require.NoError(t, os.WriteFile(filepath.Join(home, ".structalignrc"), []byte(homeRC), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(cwd, ".structalignrc"), []byte(cwdRC), 0o644)) + + got := config.Load(home, cwd) + assert.Equal(t, "true", got["sort"], "home setting preserved") + assert.Equal(t, "16", got["threshold"], "cwd setting overrides home") + assert.Equal(t, "true", got["skip-cache-padded"], "cwd setting added") +} + +func TestLoad_MissingFiles(t *testing.T) { + got := config.Load("/no/such/dir", "/no/such/cwd") + assert.Empty(t, got) +} + +func TestLoad_MalformedLines(t *testing.T) { + tmp := t.TempDir() + rc := ` +valid = yes +missing-value += missing-key +# comment + +` + require.NoError(t, os.WriteFile(filepath.Join(tmp, ".structalignrc"), []byte(rc), 0o644)) + got := config.Load("", tmp) + assert.Len(t, got, 1) + assert.Equal(t, "yes", got["valid"]) +} diff --git a/internal/layout/generics_inspect_test.go b/internal/layout/generics_inspect_test.go index cea01fd..8dbbb09 100644 --- a/internal/layout/generics_inspect_test.go +++ b/internal/layout/generics_inspect_test.go @@ -145,3 +145,27 @@ func TestLayoutsGenericConstraintUnionMulti(t *testing.T) { assert.Equal(t, int64(16), l.Fields[0].Size, "T should default to any (16 bytes) because it has no single core type") assert.Equal(t, "T=any", l.Fields[0].Assume) } + +func TestLayoutsGenericAliasesAndInterfaces(t *testing.T) { + src := `package sample + +type Alias[T any] []T + +type Box[T any] struct { + AliasedField Alias[T] + InterfaceField interface{ M(T) } +} +` + tgt := testutil.Target(t, src) + + got := layout.New().Layouts(tgt, common.Options{Patterns: []string{"Box"}}) + require.Len(t, got, 1) + + l := got[0] + assert.Equal(t, "Box", l.Name) + assert.Equal(t, "[T]", l.TypeParams) + require.Len(t, l.Fields, 2) + + assert.Equal(t, "T=any", l.Fields[0].Assume, "aliased field should capture T") + assert.Equal(t, "T=any", l.Fields[1].Assume, "interface field should capture T") +} diff --git a/internal/layout/layout.go b/internal/layout/layout.go index 7fda9aa..e691950 100644 --- a/internal/layout/layout.go +++ b/internal/layout/layout.go @@ -5,7 +5,7 @@ package layout import ( "go/token" "go/types" - "sort" + "slices" "strings" "github.com/peczenyj/structalign/internal/match" @@ -66,7 +66,7 @@ func (i *Inspector) discoverStructNames(t common.Target) []string { } } } - sort.Strings(names) + slices.Sort(names) return names } @@ -83,6 +83,7 @@ func (i *Inspector) buildLayout(t common.Target, n string, tn *types.TypeName, o return common.Layout{}, false } l := computeLayout(n, st, display, assumed, t.Sizes) + l.Package = t.PkgPath l.TypeParams = typeParams l.Note = note return l, true @@ -214,8 +215,14 @@ func fieldAssume(typ types.Type, assumed []string) string { // collectTypeParams marks used[tp.Index()] for every type parameter referenced // (directly or nested) by typ. seen guards against cycles in recursive types. +// +//nolint:gocyclo // traversing Go AST type structures naturally involves many type cases func collectTypeParams(typ types.Type, used []bool, seen map[types.Type]bool) { - if typ == nil || seen[typ] { + if typ == nil { + return + } + typ = types.Unalias(typ) + if seen[typ] { return } seen[typ] = true @@ -241,6 +248,13 @@ func collectTypeParams(typ types.Type, used []bool, seen map[types.Type]bool) { case *types.Signature: collectTupleTypeParams(t.Params(), used, seen) collectTupleTypeParams(t.Results(), used, seen) + case *types.Interface: + for i := range t.NumExplicitMethods() { + collectTypeParams(t.ExplicitMethod(i).Type(), used, seen) + } + for i := range t.NumEmbeddeds() { + collectTypeParams(t.EmbeddedType(i), used, seen) + } } } diff --git a/internal/match/fuzz_test.go b/internal/match/fuzz_test.go new file mode 100644 index 0000000..cdc8c8c --- /dev/null +++ b/internal/match/fuzz_test.go @@ -0,0 +1,27 @@ +package match_test + +import ( + "testing" + + "github.com/peczenyj/structalign/internal/match" +) + +func FuzzMatchAny(f *testing.F) { + f.Add("Mixed", "Mixed") + f.Add("*", "Mixed") + f.Add("", "Mixed") + + f.Fuzz(func(t *testing.T, pattern string, name string) { + _ = match.MatchAny([]string{pattern}, name) + }) +} + +func FuzzSplitCSV(f *testing.F) { + f.Add("a,b,c") + f.Add(`"a,b",c`) + f.Add("") + + f.Fuzz(func(t *testing.T, s string) { + _ = match.SplitCSV(s) + }) +} diff --git a/internal/structfilter/structfilter.go b/internal/structfilter/structfilter.go index 56fb46c..1a7b8a8 100644 --- a/internal/structfilter/structfilter.go +++ b/internal/structfilter/structfilter.go @@ -25,7 +25,8 @@ func InGeneratedFile(t common.Target, pos token.Pos) bool { // golang.org/x/sys/cpu.CacheLinePad (the false-sharing guard). func HasCacheLinePad(st *types.Struct) bool { for i := range st.NumFields() { - named, ok := st.Field(i).Type().(*types.Named) + typ := types.Unalias(st.Field(i).Type()) + named, ok := typ.(*types.Named) if !ok { continue } diff --git a/internal/structfilter/structfilter_test.go b/internal/structfilter/structfilter_test.go index 3d060c6..010eef2 100644 --- a/internal/structfilter/structfilter_test.go +++ b/internal/structfilter/structfilter_test.go @@ -92,3 +92,21 @@ func TestInGeneratedFile_NotGenerated(t *testing.T) { pos := obj.Pos() assert.False(t, structfilter.InGeneratedFile(tgt, pos), "Foo in non-generated file should return false") } + +func TestHasCacheLinePad_WithAliasedPad(t *testing.T) { + cpuPkg := types.NewPackage("golang.org/x/sys/cpu", "cpu") + underlying := types.NewStruct(nil, nil) + named := types.NewNamed( + types.NewTypeName(token.NoPos, cpuPkg, "CacheLinePad", nil), + underlying, + nil) + + // Create an alias to named + aliasPkg := types.NewPackage("my/pkg", "pkg") + aliasTypeName := types.NewTypeName(token.NoPos, aliasPkg, "MyPadAlias", nil) + alias := types.NewAlias(aliasTypeName, named) + + padField := types.NewField(token.NoPos, aliasPkg, "_", alias, false) + st := types.NewStruct([]*types.Var{padField}, nil) + assert.True(t, structfilter.HasCacheLinePad(st), "struct with aliased CacheLinePad field should be detected") +} diff --git a/internal/ui/json.go b/internal/ui/json.go new file mode 100644 index 0000000..fee8129 --- /dev/null +++ b/internal/ui/json.go @@ -0,0 +1,137 @@ +package ui + +import ( + "encoding/json" + "fmt" + + "github.com/peczenyj/structalign/pkg/common" +) + +type jsonDocument struct { + Version string `json:"version"` + Mode string `json:"mode"` + Findings []jsonFinding `json:"findings,omitempty"` + Layouts []jsonLayout `json:"layouts,omitempty"` + Summary *jsonSummary `json:"summary,omitempty"` +} + +type jsonFinding struct { + Package string `json:"package"` + File string `json:"file"` + Line int `json:"line"` + Column int `json:"column"` + Name string `json:"name"` + TypeParams string `json:"typeParams,omitempty"` + Message string `json:"message"` + OldSize int64 `json:"oldSize"` + NewSize int64 `json:"newSize"` + BytesSaved int64 `json:"bytesSaved"` + Original string `json:"original"` + Proposed string `json:"proposed"` +} + +type jsonSummary struct { + StructsAffected int `json:"structsAffected"` + BytesSaved int64 `json:"bytesSaved"` +} + +type jsonLayout struct { + Package string `json:"package"` + Name string `json:"name"` + TypeParams string `json:"typeParams,omitempty"` + Note string `json:"note,omitempty"` + Size int64 `json:"size"` + Align int64 `json:"align"` + Padding int64 `json:"padding"` + Fields []jsonLayoutField `json:"fields"` +} + +type jsonLayoutField struct { + Name string `json:"name"` + Type string `json:"type"` + Tag string `json:"tag,omitempty"` + Assume string `json:"assume,omitempty"` + Offset int64 `json:"offset"` + Size int64 `json:"size"` + Align int64 `json:"align"` + Padding int64 `json:"padding"` +} + +// RenderJSON emits a structured JSON document for findings or layouts. When +// keepTags is false, struct field tags are omitted from inspect-mode layouts +// (mirroring the text inspect behavior); diff-mode findings carry tags inside +// `original` / `proposed` only when the upstream align phase preserved them. +func (p *Printer) RenderJSON(version string, inspect bool, findings []common.Finding, layouts []common.Layout, keepTags bool) { + doc := jsonDocument{ + Version: version, + } + + if inspect { + doc.Mode = "inspect" + doc.Layouts = make([]jsonLayout, len(layouts)) + for i, l := range layouts { + doc.Layouts[i] = jsonLayout{ + Package: l.Package, + Name: l.Name, + TypeParams: l.TypeParams, + Note: l.Note, + Size: l.Total, + Align: l.Align, + Padding: l.Padding, + Fields: make([]jsonLayoutField, len(l.Fields)), + } + for j, f := range l.Fields { + tag := f.Tag + if !keepTags { + tag = "" + } + doc.Layouts[i].Fields[j] = jsonLayoutField{ + Name: f.Name, + Type: f.Type, + Tag: tag, + Assume: f.Assume, + Offset: f.Offset, + Size: f.Size, + Align: f.Align, + Padding: f.Padding, + } + } + } + } else { + doc.Mode = "diff" + doc.Findings = make([]jsonFinding, len(findings)) + var totalSaved int64 + for i, f := range findings { + pos := f.Fset.Position(f.Pos) + saved := f.OldSize - f.NewSize + if saved < 0 { + saved = 0 + } + totalSaved += saved + doc.Findings[i] = jsonFinding{ + Package: f.Package, + File: relPath(pos.Filename), + Line: pos.Line, + Column: pos.Column, + Name: f.Name, + TypeParams: f.TypeParams, + Message: f.Message, + OldSize: f.OldSize, + NewSize: f.NewSize, + BytesSaved: saved, + Original: f.Original, + Proposed: f.Proposed, + } + } + doc.Summary = &jsonSummary{ + StructsAffected: len(findings), + BytesSaved: totalSaved, + } + } + + enc := json.NewEncoder(p.Out) + enc.SetIndent("", " ") + if err := enc.Encode(doc); err != nil { + fmt.Fprintf(p.err(), "structalign: json encode: %v\n", err) + } +} diff --git a/internal/ui/json_test.go b/internal/ui/json_test.go new file mode 100644 index 0000000..663b53e --- /dev/null +++ b/internal/ui/json_test.go @@ -0,0 +1,57 @@ +package ui_test + +import ( + "bytes" + "path/filepath" + "testing" + + "github.com/peczenyj/structalign/internal/align" + "github.com/peczenyj/structalign/internal/layout" + "github.com/peczenyj/structalign/internal/testutil" + "github.com/peczenyj/structalign/internal/ui" + "github.com/peczenyj/structalign/pkg/common" +) + +func TestRenderJSON(t *testing.T) { + tgt := testutil.Target(t, sampleSrc) + al := align.New() + in := layout.New() + + cases := []struct { + name string + golden string + render func(p *ui.Printer) + }{ + {"json_diff_mixed", "diff_mixed.json.golden", func(p *ui.Printer) { + f, _ := al.Findings(tgt, common.Options{Patterns: []string{"Mixed"}}) + p.RenderJSON("v0.7.0-test", false, f, nil, false) + }}, + {"json_inspect_mixed", "inspect_mixed.json.golden", func(p *ui.Printer) { + l := in.Layouts(tgt, common.Options{Patterns: []string{"Mixed"}}) + p.RenderJSON("v0.7.0-test", true, nil, l, false) + }}, + {"json_inspect_tagged_notags", "inspect_notags_tagged.json.golden", func(p *ui.Printer) { + l := in.Layouts(tgt, common.Options{Patterns: []string{"Tagged"}}) + p.RenderJSON("v0.7.0-test", true, nil, l, false) + }}, + {"json_inspect_tagged_keeptags", "inspect_tags_tagged.json.golden", func(p *ui.Printer) { + l := in.Layouts(tgt, common.Options{Patterns: []string{"Tagged"}}) + p.RenderJSON("v0.7.0-test", true, nil, l, true) + }}, + {"json_empty", "empty.json.golden", func(p *ui.Printer) { + p.RenderJSON("v0.7.0-test", false, nil, nil, false) + }}, + {"json_empty_inspect", "empty_inspect.json.golden", func(p *ui.Printer) { + p.RenderJSON("v0.7.0-test", true, nil, nil, false) + }}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var buf bytes.Buffer + p := &ui.Printer{Out: &buf} + tc.render(p) + compareGolden(t, filepath.Join(testdataDir, tc.golden), buf.Bytes()) + }) + } +} diff --git a/internal/ui/printer.go b/internal/ui/printer.go index 4697658..84761a1 100644 --- a/internal/ui/printer.go +++ b/internal/ui/printer.go @@ -86,11 +86,20 @@ func DefaultTheme() Theme { // Printer renders to Out using the given color/width settings. type Printer struct { Out io.Writer + Err io.Writer // error stream (defaults to os.Stderr if nil) Color bool Width int // per-side column width for side-by-side diffs Theme Theme // zero value resolves to DefaultTheme } +// err returns the configured error stream, or os.Stderr when unset. +func (p *Printer) err() io.Writer { + if p.Err != nil { + return p.Err + } + return os.Stderr +} + // theme returns the configured theme, or DefaultTheme when unset. func (p *Printer) theme() Theme { if (p.Theme == Theme{}) { @@ -335,6 +344,9 @@ func layoutComments(fields []common.LayoutField, verbose bool) (comments []strin func indent(s, pad string) string { lines := strings.Split(s, "\n") for i, l := range lines { + if i == len(lines)-1 && l == "" { + break + } lines[i] = pad + l } return strings.Join(lines, "\n") diff --git a/internal/ui/printer_fuzz_test.go b/internal/ui/printer_fuzz_test.go new file mode 100644 index 0000000..e391c6f --- /dev/null +++ b/internal/ui/printer_fuzz_test.go @@ -0,0 +1,15 @@ +package ui + +import "testing" + +func FuzzTruncPad(f *testing.F) { + f.Add("Hello", 10) + f.Add("世界", 10) + f.Add("世", 1) + f.Add("", 0) + f.Add("VeryLongString", 5) + + f.Fuzz(func(t *testing.T, s string, w int) { + _ = truncPad(s, w) + }) +} diff --git a/internal/ui/relpath_test.go b/internal/ui/relpath_test.go index a490f9f..c2893b4 100644 --- a/internal/ui/relpath_test.go +++ b/internal/ui/relpath_test.go @@ -1,9 +1,12 @@ package ui import ( + "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestRelPath(t *testing.T) { @@ -11,6 +14,9 @@ func TestRelPath(t *testing.T) { assert.Equal(t, "foo.go", relPath("foo.go")) // Path outside current working directory returns absolute path (or original) - // /tmp/foo.go is likely outside this repo's wd - assert.Equal(t, "/tmp/foo.go", relPath("/tmp/foo.go")) + wd, err := os.Getwd() + require.NoError(t, err) + parent := filepath.Dir(wd) + outsideFile := filepath.Join(parent, "some_outside_file.go") + assert.Equal(t, outsideFile, relPath(outsideFile)) } diff --git a/internal/ui/testdata/diff_mixed.json.golden b/internal/ui/testdata/diff_mixed.json.golden new file mode 100644 index 0000000..74bc33e --- /dev/null +++ b/internal/ui/testdata/diff_mixed.json.golden @@ -0,0 +1,23 @@ +{ + "version": "v0.7.0-test", + "mode": "diff", + "findings": [ + { + "package": "sample", + "file": "src.go", + "line": 3, + "column": 12, + "name": "Mixed", + "message": "struct of size 24 could be 16", + "oldSize": 24, + "newSize": 16, + "bytesSaved": 8, + "original": "struct {\n\tA bool\n\tB int64\n\tC bool\n}", + "proposed": "struct {\n\tB int64\n\tA bool\n\tC bool\n}" + } + ], + "summary": { + "structsAffected": 1, + "bytesSaved": 8 + } +} diff --git a/internal/ui/testdata/empty.json.golden b/internal/ui/testdata/empty.json.golden new file mode 100644 index 0000000..dc08e40 --- /dev/null +++ b/internal/ui/testdata/empty.json.golden @@ -0,0 +1,8 @@ +{ + "version": "v0.7.0-test", + "mode": "diff", + "summary": { + "structsAffected": 0, + "bytesSaved": 0 + } +} diff --git a/internal/ui/testdata/empty_inspect.json.golden b/internal/ui/testdata/empty_inspect.json.golden new file mode 100644 index 0000000..f2d3cce --- /dev/null +++ b/internal/ui/testdata/empty_inspect.json.golden @@ -0,0 +1,4 @@ +{ + "version": "v0.7.0-test", + "mode": "inspect" +} diff --git a/internal/ui/testdata/inspect_mixed.json.golden b/internal/ui/testdata/inspect_mixed.json.golden new file mode 100644 index 0000000..011b8e1 --- /dev/null +++ b/internal/ui/testdata/inspect_mixed.json.golden @@ -0,0 +1,39 @@ +{ + "version": "v0.7.0-test", + "mode": "inspect", + "layouts": [ + { + "package": "sample", + "name": "Mixed", + "size": 24, + "align": 8, + "padding": 14, + "fields": [ + { + "name": "A", + "type": "bool", + "offset": 0, + "size": 1, + "align": 1, + "padding": 7 + }, + { + "name": "B", + "type": "int64", + "offset": 8, + "size": 8, + "align": 8, + "padding": 0 + }, + { + "name": "C", + "type": "bool", + "offset": 16, + "size": 1, + "align": 1, + "padding": 7 + } + ] + } + ] +} diff --git a/internal/ui/testdata/inspect_notags_tagged.json.golden b/internal/ui/testdata/inspect_notags_tagged.json.golden new file mode 100644 index 0000000..84d4fb6 --- /dev/null +++ b/internal/ui/testdata/inspect_notags_tagged.json.golden @@ -0,0 +1,55 @@ +{ + "version": "v0.7.0-test", + "mode": "inspect", + "layouts": [ + { + "package": "sample", + "name": "Tagged", + "size": 48, + "align": 8, + "padding": 18, + "fields": [ + { + "name": "Flag", + "type": "bool", + "offset": 0, + "size": 1, + "align": 1, + "padding": 7 + }, + { + "name": "ID", + "type": "string", + "offset": 8, + "size": 16, + "align": 8, + "padding": 0 + }, + { + "name": "Count", + "type": "uint32", + "offset": 24, + "size": 4, + "align": 4, + "padding": 4 + }, + { + "name": "Ptr", + "type": "*uint64", + "offset": 32, + "size": 8, + "align": 8, + "padding": 0 + }, + { + "name": "Enabled", + "type": "bool", + "offset": 40, + "size": 1, + "align": 1, + "padding": 7 + } + ] + } + ] +} diff --git a/internal/ui/testdata/inspect_tags_tagged.json.golden b/internal/ui/testdata/inspect_tags_tagged.json.golden new file mode 100644 index 0000000..38ae116 --- /dev/null +++ b/internal/ui/testdata/inspect_tags_tagged.json.golden @@ -0,0 +1,59 @@ +{ + "version": "v0.7.0-test", + "mode": "inspect", + "layouts": [ + { + "package": "sample", + "name": "Tagged", + "size": 48, + "align": 8, + "padding": 18, + "fields": [ + { + "name": "Flag", + "type": "bool", + "tag": "json:\"flag\"", + "offset": 0, + "size": 1, + "align": 1, + "padding": 7 + }, + { + "name": "ID", + "type": "string", + "tag": "json:\"id\" db:\"id\"", + "offset": 8, + "size": 16, + "align": 8, + "padding": 0 + }, + { + "name": "Count", + "type": "uint32", + "tag": "json:\"count\"", + "offset": 24, + "size": 4, + "align": 4, + "padding": 4 + }, + { + "name": "Ptr", + "type": "*uint64", + "offset": 32, + "size": 8, + "align": 8, + "padding": 0 + }, + { + "name": "Enabled", + "type": "bool", + "tag": "json:\"enabled\"", + "offset": 40, + "size": 1, + "align": 1, + "padding": 7 + } + ] + } + ] +} diff --git a/internal/ui/unicode_test.go b/internal/ui/unicode_test.go index 5a89e75..6ddd565 100644 --- a/internal/ui/unicode_test.go +++ b/internal/ui/unicode_test.go @@ -33,3 +33,10 @@ func TestTruncPadWideRunes(t *testing.T) { // landing exactly on the column with the ellipsis ("世界" = 4 + "…" = 5). assert.Equal(t, "世界…", truncPad("世界世", 5)) } + +func TestIndent(t *testing.T) { + // Without trailing newline + assert.Equal(t, " a\n b", indent("a\nb", " ")) + // With trailing newline: the empty line after the last \n should NOT be indented. + assert.Equal(t, " a\n b\n", indent("a\nb\n", " ")) +} diff --git a/pkg/common/format.go b/pkg/common/format.go new file mode 100644 index 0000000..7f2ee18 --- /dev/null +++ b/pkg/common/format.go @@ -0,0 +1,14 @@ +package common + +//go:generate go tool enumer -type=Format -text -transform=lower -trimprefix=Format -pflag.value + +// Format selects the output presentation style. enumer generates its +// String/parse/text-marshal helpers, a flag.Value implementation (Set), and a +// Type method for usage strings, see format_enumer.go; the names map to +// "text"/"json" via -trimprefix=Format -transform=lower. +type Format uint8 + +const ( + FormatText Format = iota + FormatJSON +) diff --git a/pkg/common/format_enumer.go b/pkg/common/format_enumer.go new file mode 100644 index 0000000..2a35474 --- /dev/null +++ b/pkg/common/format_enumer.go @@ -0,0 +1,102 @@ +// Code generated by "enumer -type=Format -text -transform=lower -trimprefix=Format -pflag.value"; DO NOT EDIT. + +package common + +import ( + "fmt" + "strings" +) + +const _FormatName = "textjson" + +var _FormatIndex = [...]uint8{0, 4, 8} + +const _FormatLowerName = "textjson" + +func (i Format) String() string { + if i >= Format(len(_FormatIndex)-1) { + return fmt.Sprintf("Format(%d)", i) + } + return _FormatName[_FormatIndex[i]:_FormatIndex[i+1]] +} + +// An "invalid array index" compiler error signifies that the constant values have changed. +// Re-run the stringer command to generate them again. +func _FormatNoOp() { + var x [1]struct{} + _ = x[FormatText-(0)] + _ = x[FormatJSON-(1)] +} + +var _FormatValues = []Format{FormatText, FormatJSON} + +var _FormatNameToValueMap = map[string]Format{ + _FormatName[0:4]: FormatText, + _FormatLowerName[0:4]: FormatText, + _FormatName[4:8]: FormatJSON, + _FormatLowerName[4:8]: FormatJSON, +} + +var _FormatNames = []string{ + _FormatName[0:4], + _FormatName[4:8], +} + +// FormatString retrieves an enum value from the enum constants string name. +// Throws an error if the param is not part of the enum. +func FormatString(s string) (Format, error) { + if val, ok := _FormatNameToValueMap[s]; ok { + return val, nil + } + + if val, ok := _FormatNameToValueMap[strings.ToLower(s)]; ok { + return val, nil + } + return 0, fmt.Errorf("%s does not belong to Format values", s) +} + +// FormatValues returns all values of the enum +func FormatValues() []Format { + return _FormatValues +} + +// FormatStrings returns a slice of all String values of the enum +func FormatStrings() []string { + strs := make([]string, len(_FormatNames)) + copy(strs, _FormatNames) + return strs +} + +// IsAFormat returns "true" if the value is listed in the enum definition. "false" otherwise +func (i Format) IsAFormat() bool { + for _, v := range _FormatValues { + if i == v { + return true + } + } + return false +} + +// MarshalText implements the encoding.TextMarshaler interface for Format +func (i Format) MarshalText() ([]byte, error) { + return []byte(i.String()), nil +} + +// UnmarshalText implements the encoding.TextUnmarshaler interface for Format +func (i *Format) UnmarshalText(text []byte) error { + var err error + *i, err = FormatString(string(text)) + return err +} + +// Set allows flag and pflag libraries to set a value dynamically. +func (i *Format) Set(value string) error { + var err error + *i, err = FormatString(value) + return err +} + +// Type returns a string that represents all possible values to this type joined by '|'. +func (Format) Type() string { + return strings.Join(_FormatNames, "|") +} diff --git a/pkg/common/types.go b/pkg/common/types.go index 8920a0f..7c4236d 100644 --- a/pkg/common/types.go +++ b/pkg/common/types.go @@ -24,6 +24,7 @@ type Target struct { // Finding is one struct whose fields could be reordered to use less memory. type Finding struct { + Package string Fset *token.FileSet Pos token.Pos Name string // enclosing named type, or "" for an anonymous struct @@ -49,6 +50,7 @@ type LayoutField struct { // Layout is one named struct's computed memory layout. type Layout struct { + Package string Name string TypeParams string // for a generic type, e.g. "[T]" (else "") Note string // optional caveat shown above the struct (e.g. the generic disclaimer)