diff --git a/Makefile b/Makefile index 4bb2c87..8959e4b 100644 --- a/Makefile +++ b/Makefile @@ -41,6 +41,11 @@ markdown: lint: GOTOOLCHAIN=$(TOOLCHAIN) go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.6.2 run ./... +## modernize: 🛠 Run gopls modernize +.PHONY: modernize +modernize: + GOTOOLCHAIN=$(GOVERSION) go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -fix -test=false ./... + ## test: 🚦 Execute all tests .PHONY: test test: diff --git a/cmd/dev.go b/cmd/dev.go index 3e671d8..15da844 100644 --- a/cmd/dev.go +++ b/cmd/dev.go @@ -11,6 +11,7 @@ import ( "os/signal" "path/filepath" "runtime" + "slices" "strconv" "strings" "sync" @@ -374,13 +375,7 @@ func (e *escort) hitExtension(ext string) bool { } // remove '.' ext = ext[1:] - for _, e := range e.extensions { - if ext == e { - return true - } - } - - return false + return slices.Contains(e.extensions, ext) } func (e *escort) ignoredDirs(dir string) bool { @@ -389,23 +384,11 @@ func (e *escort) ignoredDirs(dir string) bool { return true } - for _, d := range e.excludeDirs { - if dir == d { - return true - } - } - - return false + return slices.Contains(e.excludeDirs, dir) } func (e *escort) ignoredFiles(filename string) bool { - for _, f := range e.excludeFiles { - if filename == f { - return true - } - } - - return false + return slices.Contains(e.excludeFiles, filename) } func (e *escort) doPreRun() { diff --git a/cmd/internal/migrations/lists.go b/cmd/internal/migrations/lists.go index 0122103..24dddb1 100644 --- a/cmd/internal/migrations/lists.go +++ b/cmd/internal/migrations/lists.go @@ -38,6 +38,7 @@ var Migrations = []Migration{ v3migrations.MigrateHandlerSignatures, v3migrations.MigrateViewBind, v3migrations.MigrateParserMethods, + v3migrations.MigrateParamsTagKeys, v3migrations.MigrateRedirectMethods, v3migrations.MigrateClientUsage, v3migrations.MigrateGenericHelpers, diff --git a/cmd/internal/migrations/v3/cache_config.go b/cmd/internal/migrations/v3/cache_config.go index 3ecd7bf..af96da9 100644 --- a/cmd/internal/migrations/v3/cache_config.go +++ b/cmd/internal/migrations/v3/cache_config.go @@ -24,8 +24,8 @@ func MigrateCacheConfig(cmd *cobra.Command, cwd string, _, _ *semver.Version) er return "true" } - if strings.HasPrefix(expr, "!") { - return strings.TrimSpace(strings.TrimPrefix(expr, "!")) + if after, ok := strings.CutPrefix(expr, "!"); ok { + return strings.TrimSpace(after) } return "!(" + expr + ")" diff --git a/cmd/internal/migrations/v3/client_usage.go b/cmd/internal/migrations/v3/client_usage.go index c5be3f6..8b135ea 100644 --- a/cmd/internal/migrations/v3/client_usage.go +++ b/cmd/internal/migrations/v3/client_usage.go @@ -3,6 +3,7 @@ package v3 import ( "encoding/base64" "fmt" + "maps" "regexp" "sort" "strconv" @@ -311,10 +312,7 @@ func rewriteAcquireAgentBlocksWithAlias(content, alias string) (string, bool) { } errName := chooseErrName(out, lines, i) - errBlockEnd := blockEndIndex(lines, structStart) - if errBlockEnd < structStart { - errBlockEnd = structStart - } + errBlockEnd := max(blockEndIndex(lines, structStart), structStart) errsNeeded := identifierUsedAfter(lines[errBlockEnd+1:], "errs") errsAlreadyDeclared := identifierDeclared(out, lines, i, "errs") || identifierDeclaredInLines(preservedLines, "errs") @@ -868,9 +866,7 @@ func rewriteSimpleAgentBlocksWithAlias(content, alias string) (string, bool) { failed = true break } - for k, v := range params { - cfg.params[k] = v - } + maps.Copy(cfg.params, params) cfg.config = true continue } @@ -1048,7 +1044,7 @@ func parseQueryParams(expr string) (map[string]string, bool) { return nil, false } result := make(map[string]string) - for _, pair := range strings.Split(value, "&") { + for pair := range strings.SplitSeq(value, "&") { if pair == "" { continue } diff --git a/cmd/internal/migrations/v3/params_tag_keys.go b/cmd/internal/migrations/v3/params_tag_keys.go new file mode 100644 index 0000000..6200b5f --- /dev/null +++ b/cmd/internal/migrations/v3/params_tag_keys.go @@ -0,0 +1,71 @@ +package v3 + +import ( + "bytes" + "fmt" + "go/ast" + "go/format" + "go/parser" + "go/token" + "strconv" + "strings" + + semver "github.com/Masterminds/semver/v3" + "github.com/spf13/cobra" + + "github.com/gofiber/cli/cmd/internal" +) + +func MigrateParamsTagKeys(cmd *cobra.Command, cwd string, _, _ *semver.Version) error { + changed, err := internal.ChangeFileContent(cwd, rewriteParamsTagKeys) + if err != nil { + return fmt.Errorf("failed to migrate params tag keys: %w", err) + } + if !changed { + return nil + } + + cmd.Println("Migrating params tag keys") + return nil +} + +func rewriteParamsTagKeys(content string) string { + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "", content, parser.ParseComments) + if err != nil { + return content + } + + modified := false + ast.Inspect(file, func(n ast.Node) bool { + field, ok := n.(*ast.Field) + if !ok || field.Tag == nil { + return true + } + + tagLiteral, err := strconv.Unquote(field.Tag.Value) + if err != nil || !strings.Contains(tagLiteral, `params:"`) { + return true + } + + updatedTag := strings.ReplaceAll(tagLiteral, `params:"`, `uri:"`) + if updatedTag == tagLiteral { + return true + } + + field.Tag.Value = "`" + updatedTag + "`" + modified = true + return true + }) + + if !modified { + return content + } + + var buf bytes.Buffer + if err := format.Node(&buf, fset, file); err != nil { + return content + } + + return buf.String() +} diff --git a/cmd/internal/migrations/v3/params_tag_keys_test.go b/cmd/internal/migrations/v3/params_tag_keys_test.go new file mode 100644 index 0000000..6f23ef0 --- /dev/null +++ b/cmd/internal/migrations/v3/params_tag_keys_test.go @@ -0,0 +1,119 @@ +package v3_test + +import ( + "bytes" + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/gofiber/cli/cmd/internal/migrations/v3" +) + +func Test_MigrateParamsTagKeys_RewriteWithParserCalls(t *testing.T) { + t.Parallel() + + dir, err := os.MkdirTemp("", "mptagparser") + require.NoError(t, err) + defer func() { require.NoError(t, os.RemoveAll(dir)) }() + + file := writeTempFile(t, dir, `package main +import "github.com/gofiber/fiber/v2" + +type params struct { + ID string `+"`params:\"id\" json:\"id\"`"+` +} + +func handler(c fiber.Ctx) error { + var p params + if err := c.ParamsParser(&p); err != nil { + return err + } + return nil +} +`) + + var buf bytes.Buffer + cmd := newCmd(&buf) + require.NoError(t, v3.MigrateParamsTagKeys(cmd, dir, nil, nil)) + + content := readFile(t, file) + assert.Contains(t, content, "c.ParamsParser(&p)") + assert.Contains(t, content, "`uri:\"id\" json:\"id\"`") + assert.NotContains(t, content, "`params:\"id\" json:\"id\"`") + assert.Contains(t, buf.String(), "Migrating params tag keys") +} + +func Test_MigrateParamsTagKeys_RewriteForNonFiber(t *testing.T) { + t.Parallel() + + dir, err := os.MkdirTemp("", "mptagnonfiber") + require.NoError(t, err) + defer func() { require.NoError(t, os.RemoveAll(dir)) }() + + file := writeTempFile(t, dir, `package main +type params struct { + ID string `+"`params:\"id\"`"+` +} + +type ctx struct{} +func (ctx) ParamsParser(any) {} +func handler(c ctx) { + var p params + c.ParamsParser(&p) +}`) + + var buf bytes.Buffer + cmd := newCmd(&buf) + require.NoError(t, v3.MigrateParamsTagKeys(cmd, dir, nil, nil)) + + content := readFile(t, file) + assert.Contains(t, content, "c.ParamsParser(&p)") + assert.NotContains(t, content, "`params:\"id\"`") + assert.Contains(t, content, "`uri:\"id\"`") + assert.Contains(t, buf.String(), "Migrating params tag keys") +} + +func Test_MigrateParamsTagKeys_RewriteWithoutParserCalls(t *testing.T) { + t.Parallel() + + dir, err := os.MkdirTemp("", "mptagonly") + require.NoError(t, err) + defer func() { require.NoError(t, os.RemoveAll(dir)) }() + + file := writeTempFile(t, dir, `package main +type params struct { + ID string `+"`params:\"id\" json:\"id\"`"+` +}`) + + var buf bytes.Buffer + cmd := newCmd(&buf) + require.NoError(t, v3.MigrateParamsTagKeys(cmd, dir, nil, nil)) + + content := readFile(t, file) + assert.NotContains(t, content, "`params:\"id\" json:\"id\"`") + assert.Contains(t, content, "`uri:\"id\" json:\"id\"`") + assert.Contains(t, buf.String(), "Migrating params tag keys") +} + +func Test_MigrateParamsTagKeys_NoChanges(t *testing.T) { + t.Parallel() + + dir, err := os.MkdirTemp("", "mptagnochange") + require.NoError(t, err) + defer func() { require.NoError(t, os.RemoveAll(dir)) }() + + file := writeTempFile(t, dir, `package main +type params struct { + ID string `+"`uri:\"id\" json:\"id\"`"+` +}`) + + var buf bytes.Buffer + cmd := newCmd(&buf) + require.NoError(t, v3.MigrateParamsTagKeys(cmd, dir, nil, nil)) + + content := readFile(t, file) + assert.Contains(t, content, "`uri:\"id\" json:\"id\"`") + assert.NotContains(t, buf.String(), "Migrating params tag keys") +} diff --git a/cmd/internal/migrations/v3/parser_methods_test.go b/cmd/internal/migrations/v3/parser_methods_test.go index 28d0e5f..76999c3 100644 --- a/cmd/internal/migrations/v3/parser_methods_test.go +++ b/cmd/internal/migrations/v3/parser_methods_test.go @@ -44,6 +44,39 @@ func handler(c fiber.Ctx) error { assert.Contains(t, buf.String(), "Migrating parser methods") } +func Test_MigrateParserMethods_DoesNotRewriteParamsTagToURI(t *testing.T) { + t.Parallel() + + dir, err := os.MkdirTemp("", "mptagrewrite") + require.NoError(t, err) + defer func() { require.NoError(t, os.RemoveAll(dir)) }() + + file := writeTempFile(t, dir, `package main +import "github.com/gofiber/fiber/v2" + +type params struct { + ID string `+"`params:\"id\" json:\"id\"`"+` +} + +func handler(c fiber.Ctx) error { + var p params + if err := c.ParamsParser(&p); err != nil { + return err + } + return nil +} +`) + + var buf bytes.Buffer + cmd := newCmd(&buf) + require.NoError(t, v3.MigrateParserMethods(cmd, dir, nil, nil)) + + content := readFile(t, file) + assert.Contains(t, content, ".Bind().URI(&p)") + assert.Contains(t, content, "`params:\"id\" json:\"id\"`") + assert.NotContains(t, content, "`uri:\"id\" json:\"id\"`") +} + func Test_MigrateParserMethods_SkipNonFiber(t *testing.T) { t.Parallel() diff --git a/cmd/internal/migrations/v3/session_release.go b/cmd/internal/migrations/v3/session_release.go index 5084068..b3b7fbd 100644 --- a/cmd/internal/migrations/v3/session_release.go +++ b/cmd/internal/migrations/v3/session_release.go @@ -221,8 +221,8 @@ func findFunctionBoundaries(src string, lineNum int) (start, end int) { func findSessionPackageAliases(src string) []string { var aliases []string - lines := strings.Split(src, "\n") - for _, line := range lines { + lines := strings.SplitSeq(src, "\n") + for line := range lines { line = strings.TrimSpace(line) if strings.Contains(line, `"github.com/gofiber/fiber/v3/middleware/session"`) { if strings.HasPrefix(line, `"github.com/gofiber/fiber/v3/middleware/session"`) { @@ -292,14 +292,8 @@ func insertDeferStatements(content string, points []releasePoint) string { func hasExistingRelease(lines []string, startLine int, sessVar string) bool { releaseCall := sessVar + ".Release()" - searchStart := startLine - 2 - if searchStart < 0 { - searchStart = 0 - } - searchEnd := startLine + 5 - if searchEnd > len(lines) { - searchEnd = len(lines) - } + searchStart := max(startLine-2, 0) + searchEnd := min(startLine+5, len(lines)) for i := searchStart; i < searchEnd; i++ { if strings.Contains(lines[i], releaseCall) {