diff --git a/.github/dependabot.yml b/.github/dependabot.yml index cd88554..8ceda78 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,3 +1,4 @@ +--- # To get started with Dependabot version updates, you'll need to specify which # package ecosystems to update and where the package manifests are located. # Please see the documentation for all configuration options: @@ -5,7 +6,7 @@ version: 2 updates: - - package-ecosystem: "gomod" # See documentation for possible values - directory: "/" # Location of package manifests + - package-ecosystem: "gomod" # See documentation for possible values + directory: "/" # Location of package manifests schedule: interval: "weekly" diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index fbf8781..78b636f 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -1,3 +1,4 @@ +--- # This workflow will build a golang project # For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-go @@ -6,7 +7,7 @@ name: Go on: workflow_dispatch: push: - branches: [ "main" ] + branches: ["main"] pull_request: jobs: @@ -15,16 +16,17 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - go: [ oldstable, stable ] + go: [oldstable, stable] os: [ubuntu-latest, macos-latest] steps: - - uses: actions/checkout@v4 - - uses: actions/setup-go@v5 - with: - go-version: ${{ matrix.go }} + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: ${{ matrix.go }} - - name: Build - run: go build -v ./... + - name: Build + run: go build -v ./... + + - name: Test + run: go test ./... -coverprofile=./cover.out -covermode=atomic -coverpkg=./... - - name: Test - run: go test ./... -coverprofile=./cover.out -covermode=atomic -coverpkg=./... diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml deleted file mode 100644 index 8bd9fd0..0000000 --- a/.github/workflows/golangci-lint.yml +++ /dev/null @@ -1,29 +0,0 @@ -name: golangci-lint - -on: - push: - branches: [ "main" ] - pull_request: - -permissions: - contents: read - # Optional: allow read access to pull request. Use with `only-new-issues` option. - # pull-requests: read - -jobs: - golangci: - name: lint - runs-on: ${{ matrix.os }} - strategy: - matrix: - go: [ oldstable, stable ] - os: [ubuntu-latest, macos-latest] - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-go@v5 - with: - go-version: ${{ matrix.go }} - - name: golangci-lint - uses: golangci/golangci-lint-action@v7 - with: - version: v2.0.2 diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml new file mode 100644 index 0000000..bad67d5 --- /dev/null +++ b/.github/workflows/pr-checks.yml @@ -0,0 +1,68 @@ +--- +name: Checks +on: + push: + branches: ["main"] + pull_request: + +permissions: + contents: read + # Optional: allow read access to pull request. Use with `only-new-issues` option. + # pull-requests: read + +jobs: + # Check if there is any dirty change for go mod tidy + go-mod: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: 1.23 + - name: Check go mod + run: | + go mod tidy + git diff --exit-code go.mod + git diff --exit-code go.sum + + golangci: + name: lint + runs-on: ${{ matrix.os }} + strategy: + matrix: + go: [oldstable, stable] + os: [ubuntu-latest, macos-latest] + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: ${{ matrix.go }} + - name: golangci-lint + uses: golangci/golangci-lint-action@v7 + with: + version: v2.1.1 + + spell: + name: "Spell check" + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 1 + - uses: codespell-project/actions-codespell@v2 + with: + check_filenames: true + check_hidden: true + typos: + # https://github.com/crate-ci/typos + # Add exceptions to _typos.toml + # install and run locally: cargo install typos-cli && typos + name: typos + runs-on: ubuntu-latest + steps: + - name: Checkout Actions Repository + uses: actions/checkout@v4 + + - name: Check spelling of entire workspace + uses: crate-ci/typos@v1 \ No newline at end of file diff --git a/.golangci.yml b/.golangci.yml index 4566d68..c8a0630 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,3 +1,4 @@ +--- version: "2" formatters: @@ -36,6 +37,7 @@ linters: - exptostd - fatcontext - forbidigo + - funcorder - funlen - gocheckcompilerdirectives - gochecknoglobals diff --git a/README.md b/README.md index b6e5a0a..4646500 100644 --- a/README.md +++ b/README.md @@ -1,19 +1,47 @@ +# 🧐 FuncOrder + [![Go Report Card](https://goreportcard.com/badge/github.com/manuelarte/funcorder)](https://goreportcard.com/report/github.com/manuelarte/funcorder) ![version](https://img.shields.io/github/v/release/manuelarte/funcorder) - [🧐 FuncOrder](#-funcorder) - - [⬇️ Getting Started](#-getting-started) + - [⬇️ Getting Started](#️-getting-started) + - [As A Golangci-lint linter](#as-a-golangci-lint-linter) + - [Standalone application](#standalone-application) - [🚀 Features](#-features) - [Check exported methods are placed before non-exported methods](#check-exported-methods-are-placed-before-non-exported-methods) - [Check `Constructors` functions are placed after struct declaration](#check-constructors-functions-are-placed-after-struct-declaration) + - [Check Constructors/Methods are sorted alphabetically](#check-constructorsmethods-are-sorted-alphabetically) - [Resources](#resources) -# 🧐 FuncOrder - Go Linter to check Functions/Methods Order. ## ⬇️ Getting Started +### As a golangci-lint linter + +Define the rules in your `golangci-lint` configuration file, e.g: + +```yaml +linters: + enable: + - funcorder + ... + + settings: + funcorder: + # Checks that constructors are placed after the structure declaration. + # Default: true + constructor: false + # Checks if the exported methods of a structure are placed before the non-exported ones. + # Default: true + struct-method: false + # Checks if the constructors and/or structure methods are sorted alphabetically. + # Default: false + alphabetical: true +``` + +### Standalone application + Install FuncOrder linter using ```bash @@ -23,13 +51,14 @@ go install github.com/manuelarte/funcorder@latest And then use it with ``` -funcorder [-constructor=true|false] [-struct-method=true|false] ./... +funcorder [-constructor=true|false] [-struct-method=true|false] [-alphabetical=true|false] ./... ``` Parameters: -- `constructor`: `true|false` (default `true`) Check that constructor is placed after struct declaration and before struct's methods. -- `struct-method`: `true|false` (default `true`) Check that exported struct's methods are declared before non-exported. +- `constructor`: `true|false` (default `true`) Checks that constructors are placed after the structure declaration. +- `struct-method`: `true|false` (default `true`) Checks if the exported methods of a structure are placed before the non-exported ones. +- `alphabetical`: `true|false` (default `false`) Checks if the constructors and/or structure methods are sorted alphabetically. ## 🚀 Features @@ -137,6 +166,98 @@ func NewMyStruct() MyStruct { +### Check Constructors/Methods are sorted alphabetically + +This rule checks: + +- `Constructor` functions are sorted alphabetically (if `constructor` setting/parameter is `true`). +- `Methods` are sorted alphabetically (if `struct-method` setting/parameter is `true`) for each group (exported and non-exported). + + + + + + + +
❌ Bad✅ Good
+ +```go +type MyStruct struct { + Name string +} + +func NewMyStruct() MyStruct { + return MyStruct{Name: "John"} +} + +// ❌ constructor "NewAMyStruct" should be placed +// before "NewMyStruct" +func NewAMyStruct() MyStruct { + return MyStruct{Name: "John"} +} + +func (m MyStruct) GoodMorning() string { + return "good morning" +} + +// ❌ method "GoodAfternoon" should be placed +// before "GoodMorning" +func (m MyStruct) GoodAfternoon() string { + return "good afternoon" +} + +func (m MyStruct) hello() string { + return "hello" +} + +// ❌ method "bye" should be placed +// before "hello" +func (m MyStruct) bye() string { + return "bye" +} + +... +``` + + + +```go +type MyStruct struct { + Name string +} + +// ✅ constructors sorted alphabetically +func NewAMyStruct() MyStruct { + return MyStruct{Name: "John"} +} + +func NewMyStruct() MyStruct { + return MyStruct{Name: "John"} +} + +// ✅ exported methods sorted alphabetically +func (m MyStruct) GoodAfternoon() string { + return "good afternoon" +} + +func (m MyStruct) GoodMorning() string { + return "good morning" +} + +// ✅ non-exported methods sorted alphabetically +func (m MyStruct) bye() string { + return "bye" +} + +func (m MyStruct) hello() string { + return "hello" +} + +... +``` + +
+ ## Resources - Following Uber Style Guidelines about [function-grouping-and-ordering](https://github.com/uber-go/guide/blob/master/style.md#function-grouping-and-ordering) diff --git a/analyzer/analyzer.go b/analyzer/analyzer.go index f8aabbf..fd654e9 100644 --- a/analyzer/analyzer.go +++ b/analyzer/analyzer.go @@ -14,6 +14,7 @@ import ( const ( ConstructorCheckName = "constructor" StructMethodCheckName = "struct-method" + AlphabeticalCheckName = "alphabetical" ) func NewAnalyzer() *analysis.Analyzer { @@ -27,10 +28,11 @@ func NewAnalyzer() *analysis.Analyzer { } a.Flags.BoolVar(&f.constructorCheck, ConstructorCheckName, true, - "Enable/disable feature to check constructors are placed after struct declaration") + "Checks that constructors are placed after the structure declaration.") a.Flags.BoolVar(&f.structMethodCheck, StructMethodCheckName, true, - "Enable/disable feature to check whether the exported struct's methods "+ - "are placed before the non-exported") + "Checks if the exported methods of a structure are placed before the non-exported ones.") + a.Flags.BoolVar(&f.alphabeticalCheck, AlphabeticalCheckName, false, + "Checks if the constructors and/or structure methods are sorted alphabetically.") return a } @@ -38,16 +40,21 @@ func NewAnalyzer() *analysis.Analyzer { type funcorder struct { constructorCheck bool structMethodCheck bool + alphabeticalCheck bool } func (f *funcorder) run(pass *analysis.Pass) (any, error) { var enabledCheckers features.Feature if f.constructorCheck { - enabledCheckers |= features.ConstructorCheck + enabledCheckers.Enable(features.ConstructorCheck) } if f.structMethodCheck { - enabledCheckers |= features.StructMethodCheck + enabledCheckers.Enable(features.StructMethodCheck) + } + + if f.alphabeticalCheck { + enabledCheckers.Enable(features.AlphabeticalCheck) } fp := fileprocessor.NewFileProcessor(enabledCheckers) diff --git a/analyzer/analyzer_test.go b/analyzer/analyzer_test.go index 88e0640..df0ca76 100644 --- a/analyzer/analyzer_test.go +++ b/analyzer/analyzer_test.go @@ -13,9 +13,16 @@ func TestAnalyzer(t *testing.T) { options map[string]string }{ { - desc: "all", + desc: "default", patterns: "simple", }, + { + desc: "all options", + patterns: "simple-alphabetical", + options: map[string]string{ + AlphabeticalCheckName: "true", + }, + }, { desc: "constructor check only", patterns: "constructor-check", @@ -24,6 +31,15 @@ func TestAnalyzer(t *testing.T) { StructMethodCheckName: "false", }, }, + { + desc: "constructor check and alphabetical", + patterns: "constructor-check-alphabetical", + options: map[string]string{ + ConstructorCheckName: "true", + StructMethodCheckName: "false", + AlphabeticalCheckName: "true", + }, + }, { desc: "method check only", patterns: "struct-method-check", @@ -32,6 +48,15 @@ func TestAnalyzer(t *testing.T) { StructMethodCheckName: "true", }, }, + { + desc: "alphabetical method check", + patterns: "struct-method-check-alphabetical", + options: map[string]string{ + ConstructorCheckName: "false", + StructMethodCheckName: "true", + AlphabeticalCheckName: "true", + }, + }, } for _, test := range testCases { diff --git a/analyzer/testdata/src/constructor-check-alphabetical/constructors_after_struct_methods.go b/analyzer/testdata/src/constructor-check-alphabetical/constructors_after_struct_methods.go new file mode 100644 index 0000000..d8f709d --- /dev/null +++ b/analyzer/testdata/src/constructor-check-alphabetical/constructors_after_struct_methods.go @@ -0,0 +1,22 @@ +package simple + +type MyStruct2 struct { + Name string +} + +func (m MyStruct2) GetName() string { + return m.Name +} + +func (m *MyStruct2) SetName(name string) { + m.Name = name +} + +func NewOtherMyStruct2() (m *MyStruct2) { // want `constructor "NewOtherMyStruct2" for struct "MyStruct2" should be placed before struct method "GetName"` + m = &MyStruct2{Name: "John"} + return +} + +func NewMyStruct2() *MyStruct2 { // want `constructor "NewMyStruct2" for struct "MyStruct2" should be placed before struct method "GetName"` `constructor \"NewMyStruct2\" for struct \"MyStruct2\" should be placed before constructor \"NewOtherMyStruct2\"` + return &MyStruct2{Name: "John"} +} diff --git a/analyzer/testdata/src/constructor-check-alphabetical/constructors_before_struct.go b/analyzer/testdata/src/constructor-check-alphabetical/constructors_before_struct.go new file mode 100644 index 0000000..10b9ffb --- /dev/null +++ b/analyzer/testdata/src/constructor-check-alphabetical/constructors_before_struct.go @@ -0,0 +1,30 @@ +package simple + +func NewOtherMyStruct() (m *MyStruct) { // want `should be placed after the struct declaration` + m = &MyStruct{Name: "John"} + return +} + +func NewMyStruct() *MyStruct { // want `should be placed after the struct declaration` `constructor \"NewMyStruct\" for struct \"MyStruct\" should be placed before constructor \"NewOtherMyStruct\"` + return &MyStruct{Name: "John"} +} + +func MustMyStruct() *MyStruct { // want `should be placed after the struct declaration` `constructor \"MustMyStruct\" for struct \"MyStruct\" should be placed before constructor \"NewMyStruct\"` + return NewMyStruct() +} + +type MyStruct struct { + Name string +} + +func (m MyStruct) lenName() int { + return len(m.Name) +} + +func (m MyStruct) GetName() string { + return m.Name +} + +func (m *MyStruct) SetName(name string) { + m.Name = name +} diff --git a/analyzer/testdata/src/constructor-check-alphabetical/constructors_not_sorted_alphabetical.go b/analyzer/testdata/src/constructor-check-alphabetical/constructors_not_sorted_alphabetical.go new file mode 100644 index 0000000..8390912 --- /dev/null +++ b/analyzer/testdata/src/constructor-check-alphabetical/constructors_not_sorted_alphabetical.go @@ -0,0 +1,29 @@ +package simple + +type Greetings struct { +} + +func NewOtherGreetings() (m *Greetings) { + m = &Greetings{} + return +} + +func NewGreetings() *Greetings { // want `constructor \"NewGreetings\" for struct \"Greetings\" should be placed before constructor \"NewOtherGreetings\"` + return &Greetings{} +} + +func (m Greetings) GoodMorning() string { + return "hello" +} + +func (m *Greetings) GoodAfternoon(name string) string { + return "bye" +} + +func (m Greetings) hello() string { + return "hello" +} + +func (m *Greetings) bye(name string) string { + return "bye" +} diff --git a/analyzer/testdata/src/constructor-check-alphabetical/struct_not_declared_in_file.go b/analyzer/testdata/src/constructor-check-alphabetical/struct_not_declared_in_file.go new file mode 100644 index 0000000..8d6ecc3 --- /dev/null +++ b/analyzer/testdata/src/constructor-check-alphabetical/struct_not_declared_in_file.go @@ -0,0 +1,13 @@ +package simple + +import ( + "time" +) + +func NewOtherWayMyStruct() MyStruct { + return MyStruct{Name: "John"} +} + +func NewTimeStruct() time.Time { + return time.Now() +} diff --git a/analyzer/testdata/src/simple-alphabetical/constructors_after_struct_methods.go b/analyzer/testdata/src/simple-alphabetical/constructors_after_struct_methods.go new file mode 100644 index 0000000..3e92d2d --- /dev/null +++ b/analyzer/testdata/src/simple-alphabetical/constructors_after_struct_methods.go @@ -0,0 +1,22 @@ +package simple + +type MyStruct2 struct { + Name string +} + +func (m MyStruct2) GetName() string { + return m.Name +} + +func (m *MyStruct2) SetName(name string) { + m.Name = name +} + +func NewOtherMyStruct2() (m *MyStruct2) { // want `constructor \"NewOtherMyStruct2\" for struct \"MyStruct2\" should be placed before struct method \"GetName\"` + m = &MyStruct2{Name: "John"} + return +} + +func NewMyStruct2() *MyStruct2 { // want `constructor \"NewMyStruct2\" for struct \"MyStruct2\" should be placed before struct method \"GetName\"` `constructor \"NewMyStruct2\" for struct \"MyStruct2\" should be placed before constructor \"NewOtherMyStruct2\"` + return &MyStruct2{Name: "John"} +} diff --git a/analyzer/testdata/src/simple-alphabetical/constructors_and_struct_methods_not_sorted_alphabetical.go b/analyzer/testdata/src/simple-alphabetical/constructors_and_struct_methods_not_sorted_alphabetical.go new file mode 100644 index 0000000..4cd8bb4 --- /dev/null +++ b/analyzer/testdata/src/simple-alphabetical/constructors_and_struct_methods_not_sorted_alphabetical.go @@ -0,0 +1,29 @@ +package simple + +type Greetings struct { +} + +func NewOtherGreetings() (m *Greetings) { + m = &Greetings{} + return +} + +func NewGreetings() *Greetings { // want `constructor \"NewGreetings\" for struct \"Greetings\" should be placed before constructor \"NewOtherGreetings\"` + return &Greetings{} +} + +func (m Greetings) GoodMorning() string { + return "hello" +} + +func (m *Greetings) GoodAfternoon(name string) string { // want `method \"GoodAfternoon\" for struct \"Greetings\" should be placed before method \"GoodMorning\"` + return "bye" +} + +func (m Greetings) hello() string { + return "hello" +} + +func (m *Greetings) bye(name string) string { // want `method \"bye\" for struct \"Greetings\" should be placed before method \"hello\"` + return "bye" +} diff --git a/analyzer/testdata/src/simple-alphabetical/constructors_before_struct.go b/analyzer/testdata/src/simple-alphabetical/constructors_before_struct.go new file mode 100644 index 0000000..56795d4 --- /dev/null +++ b/analyzer/testdata/src/simple-alphabetical/constructors_before_struct.go @@ -0,0 +1,30 @@ +package simple + +func NewOtherMyStruct() (m *MyStruct) { // want "should be placed after the struct declaration" + m = &MyStruct{Name: "John"} + return +} + +func NewMyStruct() *MyStruct { // want "should be placed after the struct declaration" `constructor \"NewMyStruct\" for struct \"MyStruct\" should be placed before constructor \"NewOtherMyStruct\"` + return &MyStruct{Name: "John"} +} + +func MustMyStruct() *MyStruct { // want `function \"MustMyStruct\" for struct \"MyStruct\" should be placed after the struct declaration` `constructor \"MustMyStruct\" for struct \"MyStruct\" should be placed before constructor \"NewMyStruct\"` + return NewMyStruct() +} + +type MyStruct struct { + Name string +} + +func (m MyStruct) lenName() int { // want `unexported method \"lenName\" for struct \"MyStruct\" should be placed after the exported method \"SetName\"` + return len(m.Name) +} + +func (m MyStruct) GetName() string { + return m.Name +} + +func (m *MyStruct) SetName(name string) { + m.Name = name +} diff --git a/analyzer/testdata/src/simple-alphabetical/struct_not_declared_in_file.go b/analyzer/testdata/src/simple-alphabetical/struct_not_declared_in_file.go new file mode 100644 index 0000000..8d6ecc3 --- /dev/null +++ b/analyzer/testdata/src/simple-alphabetical/struct_not_declared_in_file.go @@ -0,0 +1,13 @@ +package simple + +import ( + "time" +) + +func NewOtherWayMyStruct() MyStruct { + return MyStruct{Name: "John"} +} + +func NewTimeStruct() time.Time { + return time.Now() +} diff --git a/analyzer/testdata/src/struct-method-check-alphabetical/constructors_after_struct_methods.go b/analyzer/testdata/src/struct-method-check-alphabetical/constructors_after_struct_methods.go new file mode 100644 index 0000000..dd1de9a --- /dev/null +++ b/analyzer/testdata/src/struct-method-check-alphabetical/constructors_after_struct_methods.go @@ -0,0 +1,22 @@ +package simple + +type MyStruct2 struct { + Name string +} + +func (m MyStruct2) GetName() string { + return m.Name +} + +func (m *MyStruct2) SetName(name string) { + m.Name = name +} + +func NewOtherMyStruct2() (m *MyStruct2) { + m = &MyStruct2{Name: "John"} + return +} + +func NewMyStruct2() *MyStruct2 { + return &MyStruct2{Name: "John"} +} diff --git a/analyzer/testdata/src/struct-method-check-alphabetical/constructors_before_struct.go b/analyzer/testdata/src/struct-method-check-alphabetical/constructors_before_struct.go new file mode 100644 index 0000000..5f64786 --- /dev/null +++ b/analyzer/testdata/src/struct-method-check-alphabetical/constructors_before_struct.go @@ -0,0 +1,30 @@ +package simple + +func NewOtherMyStruct() (m *MyStruct) { + m = &MyStruct{Name: "John"} + return +} + +func NewMyStruct() *MyStruct { + return &MyStruct{Name: "John"} +} + +func MustMyStruct() *MyStruct { + return NewMyStruct() +} + +type MyStruct struct { + Name string +} + +func (m MyStruct) lenName() int { // want `unexported method \"lenName\" for struct \"MyStruct\" should be placed after the exported method \"SetName\"` + return len(m.Name) +} + +func (m MyStruct) GetName() string { + return m.Name +} + +func (m *MyStruct) SetName(name string) { + m.Name = name +} diff --git a/analyzer/testdata/src/struct-method-check-alphabetical/struct_methods_not_sorted_alphabetical.go b/analyzer/testdata/src/struct-method-check-alphabetical/struct_methods_not_sorted_alphabetical.go new file mode 100644 index 0000000..3772998 --- /dev/null +++ b/analyzer/testdata/src/struct-method-check-alphabetical/struct_methods_not_sorted_alphabetical.go @@ -0,0 +1,29 @@ +package simple + +type Greetings struct { +} + +func NewOtherGreetings() (m *Greetings) { + m = &Greetings{} + return +} + +func NewGreetings() *Greetings { + return &Greetings{} +} + +func (m Greetings) GoodMorning() string { + return "hello" +} + +func (m *Greetings) GoodAfternoon(name string) string { // want `method \"GoodAfternoon" for struct \"Greetings\" should be placed before method \"GoodMorning\"` + return "bye" +} + +func (m Greetings) hello() string { + return "hello" +} + +func (m *Greetings) bye(name string) string { // want `method \"bye\" for struct \"Greetings\" should be placed before method \"hello\"` + return "bye" +} diff --git a/analyzer/testdata/src/struct-method-check-alphabetical/struct_not_declared_in_file.go b/analyzer/testdata/src/struct-method-check-alphabetical/struct_not_declared_in_file.go new file mode 100644 index 0000000..8d6ecc3 --- /dev/null +++ b/analyzer/testdata/src/struct-method-check-alphabetical/struct_not_declared_in_file.go @@ -0,0 +1,13 @@ +package simple + +import ( + "time" +) + +func NewOtherWayMyStruct() MyStruct { + return MyStruct{Name: "John"} +} + +func NewTimeStruct() time.Time { + return time.Now() +} diff --git a/go.mod b/go.mod index 0e573ff..ea04512 100644 --- a/go.mod +++ b/go.mod @@ -2,9 +2,9 @@ module github.com/manuelarte/funcorder go 1.23.0 -require golang.org/x/tools v0.31.0 +require golang.org/x/tools v0.32.0 require ( golang.org/x/mod v0.24.0 // indirect - golang.org/x/sync v0.12.0 // indirect + golang.org/x/sync v0.13.0 // indirect ) diff --git a/go.sum b/go.sum index 09ebfb8..0fbc883 100644 --- a/go.sum +++ b/go.sum @@ -2,7 +2,7 @@ github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= golang.org/x/mod v0.24.0 h1:ZfthKaKaT4NrhGVZHO1/WDTwGES4De8KtWO0SIbNJMU= golang.org/x/mod v0.24.0/go.mod h1:IXM97Txy2VM4PJ3gI61r1YEk/gAj6zAHN3AdZt6S9Ww= -golang.org/x/sync v0.12.0 h1:MHc5BpPuC30uJk597Ri8TV3CNZcTLu6B6z4lJy+g6Jw= -golang.org/x/sync v0.12.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= -golang.org/x/tools v0.31.0 h1:0EedkvKDbh+qistFTd0Bcwe/YLh4vHwWEkiI0toFIBU= -golang.org/x/tools v0.31.0/go.mod h1:naFTU+Cev749tSJRXJlna0T3WxKvb1kWEx15xA4SdmQ= +golang.org/x/sync v0.13.0 h1:AauUjRAJ9OSnvULf/ARrrVywoJDy0YS2AwQ98I37610= +golang.org/x/sync v0.13.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= +golang.org/x/tools v0.32.0 h1:Q7N1vhpkQv7ybVzLFtTjvQya2ewbwNDZzUgfXGqtMWU= +golang.org/x/tools v0.32.0/go.mod h1:ZxrU41P/wAbZD8EDa6dDCa6XfpkhJ7HFMjHJXfBDu8s= diff --git a/internal/diag/diag.go b/internal/diag/diag.go index 3a5012b..16f8dc5 100644 --- a/internal/diag/diag.go +++ b/internal/diag/diag.go @@ -27,6 +27,18 @@ func NewConstructorNotBeforeStructMethod( } } +func NewAdjacentConstructorsNotSortedAlphabetically( + structSpec *ast.TypeSpec, + constructorNotSorted *ast.FuncDecl, + otherConstructorNotSorted *ast.FuncDecl, +) analysis.Diagnostic { + return analysis.Diagnostic{ + Pos: otherConstructorNotSorted.Pos(), + Message: fmt.Sprintf("constructor %q for struct %q should be placed before constructor %q", + otherConstructorNotSorted.Name, structSpec.Name, constructorNotSorted.Name), + } +} + func NewNonExportedMethodBeforeExportedForStruct( structSpec *ast.TypeSpec, privateMethod *ast.FuncDecl, @@ -38,3 +50,15 @@ func NewNonExportedMethodBeforeExportedForStruct( privateMethod.Name, structSpec.Name, publicMethod.Name), } } + +func NewAdjacentStructMethodsNotSortedAlphabetically( + structSpec *ast.TypeSpec, + method *ast.FuncDecl, + otherMethod *ast.FuncDecl, +) analysis.Diagnostic { + return analysis.Diagnostic{ + Pos: otherMethod.Pos(), + Message: fmt.Sprintf("method %q for struct %q should be placed before method %q", + otherMethod.Name, structSpec.Name, method.Name), + } +} diff --git a/internal/features/features.go b/internal/features/features.go index 244ea64..9092b5f 100644 --- a/internal/features/features.go +++ b/internal/features/features.go @@ -3,10 +3,15 @@ package features const ( ConstructorCheck Feature = 1 << iota StructMethodCheck + AlphabeticalCheck ) type Feature uint8 -func (c Feature) IsEnabled(other Feature) bool { - return c&other != 0 +func (f *Feature) Enable(other Feature) { + *f |= other +} + +func (f *Feature) IsEnabled(other Feature) bool { + return *f&other != 0 } diff --git a/internal/fileprocessor/file_processor.go b/internal/fileprocessor/file_processor.go index 8a2fb0e..686b818 100644 --- a/internal/fileprocessor/file_processor.go +++ b/internal/fileprocessor/file_processor.go @@ -38,16 +38,6 @@ func (fp *FileProcessor) Analyze() []analysis.Diagnostic { return reports } -func (fp *FileProcessor) addConstructor(sc models.StructConstructor) { - sh := fp.getOrCreate(sc.GetStructReturn().Name) - sh.AddConstructor(sc.GetConstructor()) -} - -func (fp *FileProcessor) addMethod(st string, n *ast.FuncDecl) { - sh := fp.getOrCreate(st) - sh.AddMethod(n) -} - func (fp *FileProcessor) NewFileNode(_ *ast.File) { fp.structs = make(map[string]*models.StructHolder) } @@ -68,6 +58,16 @@ func (fp *FileProcessor) NewTypeSpec(n *ast.TypeSpec) { sh.Struct = n } +func (fp *FileProcessor) addConstructor(sc models.StructConstructor) { + sh := fp.getOrCreate(sc.GetStructReturn().Name) + sh.AddConstructor(sc.GetConstructor()) +} + +func (fp *FileProcessor) addMethod(st string, n *ast.FuncDecl) { + sh := fp.getOrCreate(st) + sh.AddMethod(n) +} + func (fp *FileProcessor) getOrCreate(structName string) *models.StructHolder { if holder, ok := fp.structs[structName]; ok { return holder diff --git a/internal/models/struct_holder.go b/internal/models/struct_holder.go index 4b832f3..0d3abdb 100644 --- a/internal/models/struct_holder.go +++ b/internal/models/struct_holder.go @@ -34,7 +34,7 @@ func (sh *StructHolder) AddMethod(fn *ast.FuncDecl) { sh.StructMethods = append(sh.StructMethods, fn) } -//nolint:gocognit,nestif // refactor later +// Analyze applies the linter to the struct holder. func (sh *StructHolder) Analyze() []analysis.Diagnostic { // TODO maybe sort constructors and then report also, like NewXXX before MustXXX @@ -45,47 +45,104 @@ func (sh *StructHolder) Analyze() []analysis.Diagnostic { var reports []analysis.Diagnostic if sh.Features.IsEnabled(features.ConstructorCheck) { - structPos := sh.Struct.Pos() + reports = append(reports, sh.analyzeConstructor()...) + } - for _, c := range sh.Constructors { - if c.Pos() < structPos { - reports = append(reports, diag.NewConstructorNotAfterStructType(sh.Struct, c)) - } + if sh.Features.IsEnabled(features.StructMethodCheck) { + reports = append(reports, sh.analyzeStructMethod()...) + } - if len(sh.StructMethods) > 0 && c.Pos() > sh.StructMethods[0].Pos() { - reports = append(reports, diag.NewConstructorNotBeforeStructMethod(sh.Struct, c, sh.StructMethods[0])) - } + // TODO also check that the methods are declared after the struct + return reports +} + +func (sh *StructHolder) analyzeConstructor() []analysis.Diagnostic { + var reports []analysis.Diagnostic + + for i, constructor := range sh.Constructors { + if constructor.Pos() < sh.Struct.Pos() { + reports = append(reports, diag.NewConstructorNotAfterStructType(sh.Struct, constructor)) + } + + if len(sh.StructMethods) > 0 && constructor.Pos() > sh.StructMethods[0].Pos() { + reports = append(reports, diag.NewConstructorNotBeforeStructMethod(sh.Struct, constructor, sh.StructMethods[0])) + } + + if sh.Features.IsEnabled(features.AlphabeticalCheck) && + i < len(sh.Constructors)-1 && sh.Constructors[i].Name.Name > sh.Constructors[i+1].Name.Name { + reports = append(reports, + diag.NewAdjacentConstructorsNotSortedAlphabetically(sh.Struct, sh.Constructors[i], sh.Constructors[i+1]), + ) } } + return reports +} - if sh.Features.IsEnabled(features.StructMethodCheck) { - var lastExportedMethod *ast.FuncDecl +func (sh *StructHolder) analyzeStructMethod() []analysis.Diagnostic { + var lastExportedMethod *ast.FuncDecl + for _, m := range sh.StructMethods { + if !m.Name.IsExported() { + continue + } + + if lastExportedMethod == nil { + lastExportedMethod = m + } + + if lastExportedMethod.Pos() < m.Pos() { + lastExportedMethod = m + } + } + + var reports []analysis.Diagnostic + + if lastExportedMethod != nil { for _, m := range sh.StructMethods { - if !m.Name.IsExported() { + if m.Name.IsExported() || m.Pos() >= lastExportedMethod.Pos() { continue } - if lastExportedMethod == nil { - lastExportedMethod = m - } - - if lastExportedMethod.Pos() < m.Pos() { - lastExportedMethod = m - } + reports = append(reports, diag.NewNonExportedMethodBeforeExportedForStruct(sh.Struct, m, lastExportedMethod)) } + } - if lastExportedMethod != nil { - for _, m := range sh.StructMethods { - if m.Name.IsExported() || m.Pos() >= lastExportedMethod.Pos() { - continue - } + if sh.Features.IsEnabled(features.AlphabeticalCheck) { + return slices.Concat(reports, + isSorted(sh.Struct, filterMethods(sh.StructMethods, true)), + isSorted(sh.Struct, filterMethods(sh.StructMethods, false)), + ) + } - reports = append(reports, diag.NewNonExportedMethodBeforeExportedForStruct(sh.Struct, m, lastExportedMethod)) - } + return reports +} + +func filterMethods(funcDecls []*ast.FuncDecl, exported bool) []*ast.FuncDecl { + var result []*ast.FuncDecl + + for _, f := range funcDecls { + if f.Name.IsExported() != exported { + continue } + + result = append(result, f) } - // TODO also check that the methods are declared after the struct + return result +} + +func isSorted(typeSpec *ast.TypeSpec, funcDecls []*ast.FuncDecl) []analysis.Diagnostic { + var reports []analysis.Diagnostic + + for i := range funcDecls { + if i >= len(funcDecls)-1 { + continue + } + + if funcDecls[i].Name.Name > funcDecls[i+1].Name.Name { + reports = append(reports, + diag.NewAdjacentStructMethodsNotSortedAlphabetically(typeSpec, funcDecls[i], funcDecls[i+1])) + } + } return reports }