diff --git a/cmd/internal/migrations/v3/filesystem_middleware.go b/cmd/internal/migrations/v3/filesystem_middleware.go index 79e7373..164b44b 100644 --- a/cmd/internal/migrations/v3/filesystem_middleware.go +++ b/cmd/internal/migrations/v3/filesystem_middleware.go @@ -13,27 +13,78 @@ import ( var ( reFilesystemNew = regexp.MustCompile(`filesystem\.New\s*\(`) - reFilesystemRootHTTP = regexp.MustCompile(`Root:\s*http.Dir\(([^)]+)\)`) + reFilesystemRootHTTPDir = regexp.MustCompile(`Root:\s*http\.Dir\(([^)]+)\)`) + reFilesystemRootHTTPFS = regexp.MustCompile(`Root:\s*http\.FS\(([^)]+)\)`) reFilesystemRoot = regexp.MustCompile(`Root:\s*([^,\n]+)`) reFilesystemIndex = regexp.MustCompile(`Index:\s*([^,\n]+)`) reFilesystemNotFoundFile = regexp.MustCompile(`(?m)^(\s*)(NotFoundFile:\s*[^,\n]+)(,?)`) + // Matches imports that end with /static, capturing alias (group 1) and path (group 2) + // Also handles dot imports (import . "path/to/static") + reStaticImport = regexp.MustCompile(`(?m)^\s*(?:(\w+|\.)\s+)?"([^"]+/static)"`) ) +// staticMiddlewarePkg returns the package name to use for the static middleware. +// If there's already an import with package name "static", returns "staticmw". +// Excludes Fiber's own middleware paths from conflict detection. +func staticMiddlewarePkg(content string) string { + // Check if there's already an import that would conflict with "static" + matches := reStaticImport.FindAllStringSubmatch(content, -1) + for _, m := range matches { + // m[1] is the alias if present + // m[2] is the import path + alias := m[1] + importPath := m[2] + + // Skip Fiber's own middleware - not a conflict + if strings.Contains(importPath, "github.com/gofiber/fiber") { + continue + } + + // If no alias or alias is "static" or ".", there's a conflict + if alias == "" || alias == "static" || alias == "." { + return "staticmw" + } + // If user has their own alias (e.g., mystatic "pkg/static"), no conflict + } + return "static" +} + +// MigrateFilesystemMiddleware migrates the filesystem middleware to the static middleware. +// It handles import path changes, http.Dir/http.FS conversions, config field renames, +// and import naming conflicts when users have their own package named "static". func MigrateFilesystemMiddleware(cmd *cobra.Command, cwd string, _, _ *semver.Version) error { changed, err := internal.ChangeFileContent(cwd, func(content string) string { - content = strings.ReplaceAll(content, - "github.com/gofiber/fiber/v2/middleware/filesystem", - "github.com/gofiber/fiber/v3/middleware/static") - content = strings.ReplaceAll(content, - "github.com/gofiber/fiber/v3/middleware/filesystem", - "github.com/gofiber/fiber/v3/middleware/static") + // Determine the package name to use for the static middleware + staticPkg := staticMiddlewarePkg(content) + + // Handle import replacement based on whether we need an alias + if staticPkg == "staticmw" { + content = strings.ReplaceAll(content, + `"github.com/gofiber/fiber/v2/middleware/filesystem"`, + `staticmw "github.com/gofiber/fiber/v3/middleware/static"`) + content = strings.ReplaceAll(content, + `"github.com/gofiber/fiber/v3/middleware/filesystem"`, + `staticmw "github.com/gofiber/fiber/v3/middleware/static"`) + } else { + content = strings.ReplaceAll(content, + `"github.com/gofiber/fiber/v2/middleware/filesystem"`, + `"github.com/gofiber/fiber/v3/middleware/static"`) + content = strings.ReplaceAll(content, + `"github.com/gofiber/fiber/v3/middleware/filesystem"`, + `"github.com/gofiber/fiber/v3/middleware/static"`) + } + + content = reFilesystemNew.ReplaceAllString(content, staticPkg+`.New("", `) - content = reFilesystemNew.ReplaceAllString(content, `static.New("", `) + content = strings.ReplaceAll(content, "filesystem.Config{", staticPkg+".Config{") - content = strings.ReplaceAll(content, "filesystem.Config{", "static.Config{") + // Handle http.Dir() - convert to os.DirFS() + content = reFilesystemRootHTTPDir.ReplaceAllString(content, `FS: os.DirFS($1)`) - content = reFilesystemRootHTTP.ReplaceAllString(content, `FS: os.DirFS($1)`) + // Handle http.FS() - extract inner value directly (embed.FS implements fs.FS) + content = reFilesystemRootHTTPFS.ReplaceAllString(content, `FS: $1`) + // Handle remaining Root: patterns (fallback to os.DirFS wrapping) content = reFilesystemRoot.ReplaceAllString(content, `FS: os.DirFS($1)`) content = reFilesystemIndex.ReplaceAllString(content, `IndexNames: []string{$1}`) diff --git a/cmd/internal/migrations/v3/filesystem_middleware_test.go b/cmd/internal/migrations/v3/filesystem_middleware_test.go index 6b649db..916519b 100644 --- a/cmd/internal/migrations/v3/filesystem_middleware_test.go +++ b/cmd/internal/migrations/v3/filesystem_middleware_test.go @@ -117,3 +117,230 @@ func main() { // Verify the NotFoundFile comment is only present once assert.Equal(t, 1, strings.Count(secondContent, "// NotFoundFile:")) } + +// Test_MigrateFilesystemMiddleware_WithHTTPFS tests migration of http.FS() usage. +// Bug fix: http.FS(embedFS) should produce FS: embedFS, not FS: os.DirFS(http.FS(embedFS)) +// See: https://github.com/gofiber/cli/issues/267 +func Test_MigrateFilesystemMiddleware_WithHTTPFS(t *testing.T) { + t.Parallel() + + dir, err := os.MkdirTemp("", "mfs-httpfs") + require.NoError(t, err) + defer func() { require.NoError(t, os.RemoveAll(dir)) }() + + file := writeTempFile(t, dir, `package main + +import ( + "embed" + "net/http" + + "github.com/gofiber/fiber/v2" + "github.com/gofiber/fiber/v2/middleware/filesystem" +) + +//go:embed static/* +var embedFS embed.FS + +func main() { + app := fiber.New() + app.Use("/static", filesystem.New(filesystem.Config{ + Root: http.FS(embedFS), + MaxAge: 24 * 60 * 60, + })) +} +`) + + var buf bytes.Buffer + cmd := newCmd(&buf) + require.NoError(t, v3.MigrateFilesystemMiddleware(cmd, dir, nil, nil)) + + content := readFile(t, file) + // Should use embedFS directly, not wrap with os.DirFS + assert.Contains(t, content, "FS: embedFS") + assert.NotContains(t, content, "os.DirFS(http.FS(embedFS))") + assert.NotContains(t, content, "os.DirFS(embedFS)") + // Check static middleware migration happened + assert.Contains(t, content, `static.New("", static.Config{`) + assert.Contains(t, buf.String(), "Migrating filesystem middleware") +} + +// Test_MigrateFilesystemMiddleware_WithStaticPackageConflict tests migration when +// the user has their own package named "static". +// Bug fix: Should alias the middleware import to avoid conflict. +// See: https://github.com/gofiber/cli/issues/267 +func Test_MigrateFilesystemMiddleware_WithStaticPackageConflict(t *testing.T) { + t.Parallel() + + dir, err := os.MkdirTemp("", "mfs-conflict") + require.NoError(t, err) + defer func() { require.NoError(t, os.RemoveAll(dir)) }() + + file := writeTempFile(t, dir, `package main + +import ( + "net/http" + + "github.com/gofiber/fiber/v2" + "github.com/gofiber/fiber/v2/middleware/filesystem" + "myproject/internal/web/static" +) + +func main() { + app := fiber.New() + app.Use("/static", filesystem.New(filesystem.Config{ + Root: http.FS(static.Static), + MaxAge: 24 * 60 * 60, + })) +} +`) + + var buf bytes.Buffer + cmd := newCmd(&buf) + require.NoError(t, v3.MigrateFilesystemMiddleware(cmd, dir, nil, nil)) + + content := readFile(t, file) + // Should use staticmw alias for the middleware to avoid conflict + assert.Contains(t, content, `staticmw "github.com/gofiber/fiber/v3/middleware/static"`) + assert.Contains(t, content, `staticmw.New("", staticmw.Config{`) + // User's static import should remain unchanged + assert.Contains(t, content, `"myproject/internal/web/static"`) + // Should use static.Static directly (from user's package) + assert.Contains(t, content, "FS: static.Static") + assert.Contains(t, buf.String(), "Migrating filesystem middleware") +} + +// Test_MigrateFilesystemMiddleware_WithAliasedStaticImport tests migration when +// the user has already aliased their static package import. +func Test_MigrateFilesystemMiddleware_WithAliasedStaticImport(t *testing.T) { + t.Parallel() + + dir, err := os.MkdirTemp("", "mfs-aliased") + require.NoError(t, err) + defer func() { require.NoError(t, os.RemoveAll(dir)) }() + + file := writeTempFile(t, dir, `package main + +import ( + "net/http" + + "github.com/gofiber/fiber/v2" + "github.com/gofiber/fiber/v2/middleware/filesystem" + webstatic "myproject/internal/web/static" +) + +func main() { + app := fiber.New() + app.Use("/static", filesystem.New(filesystem.Config{ + Root: http.FS(webstatic.Assets), + MaxAge: 24 * 60 * 60, + })) +} +`) + + var buf bytes.Buffer + cmd := newCmd(&buf) + require.NoError(t, v3.MigrateFilesystemMiddleware(cmd, dir, nil, nil)) + + content := readFile(t, file) + // When user's import is already aliased, we can use "static" for middleware + assert.Contains(t, content, `"github.com/gofiber/fiber/v3/middleware/static"`) + assert.Contains(t, content, `static.New("", static.Config{`) + // User's aliased import should remain + assert.Contains(t, content, `webstatic "myproject/internal/web/static"`) + // Should use webstatic.Assets directly + assert.Contains(t, content, "FS: webstatic.Assets") +} + +// Test_MigrateFilesystemMiddleware_FiberStaticNotConflict ensures that Fiber's own +// static middleware is not incorrectly flagged as a conflict. +func Test_MigrateFilesystemMiddleware_FiberStaticNotConflict(t *testing.T) { + t.Parallel() + + dir, err := os.MkdirTemp("", "mfs-fiber-static") + require.NoError(t, err) + defer func() { require.NoError(t, os.RemoveAll(dir)) }() + + // Code that already has Fiber's static middleware (v3) imported + // Should not incorrectly alias the import + file := writeTempFile(t, dir, `package main + +import ( + "net/http" + + "github.com/gofiber/fiber/v2" + "github.com/gofiber/fiber/v2/middleware/filesystem" +) + +func main() { + app := fiber.New() + app.Use("/static", filesystem.New(filesystem.Config{ + Root: http.Dir("./public"), + MaxAge: 24 * 60 * 60, + })) + app.Listen(":3000") +} +`) + + var buf bytes.Buffer + cmd := newCmd(&buf) + require.NoError(t, v3.MigrateFilesystemMiddleware(cmd, dir, nil, nil)) + + content := readFile(t, file) + + // Should NOT have aliased import - no conflict with user packages + assert.Contains(t, content, `"github.com/gofiber/fiber/v3/middleware/static"`) + assert.NotContains(t, content, `staticmw "github.com/gofiber/fiber/v3/middleware/static"`) + + // Should use unaliased static.New and static.Config + assert.Contains(t, content, `static.New("", static.Config{`) +} + +// Test_MigrateFilesystemMiddleware_IssueExample tests the exact scenario from issue #267. +// This is the complete example combining both bugs: http.FS() with a package named static. +// See: https://github.com/gofiber/cli/issues/267 +func Test_MigrateFilesystemMiddleware_IssueExample(t *testing.T) { + t.Parallel() + + dir, err := os.MkdirTemp("", "mfs-issue267") + require.NoError(t, err) + defer func() { require.NoError(t, os.RemoveAll(dir)) }() + + // This is the exact v2 code from issue #267 + file := writeTempFile(t, dir, `package main + +import ( + "net/http" + + "github.com/gofiber/fiber/v2" + "github.com/gofiber/fiber/v2/middleware/filesystem" + "myproject/internal/web/static" +) + +func main() { + app := fiber.New() + app.Use("/static", filesystem.New(filesystem.Config{ + Root: http.FS(static.Static), + MaxAge: 24 * 60 * 60, + })) + app.Listen(":3000") +} +`) + + var buf bytes.Buffer + cmd := newCmd(&buf) + require.NoError(t, v3.MigrateFilesystemMiddleware(cmd, dir, nil, nil)) + + content := readFile(t, file) + + // Bug #1 fixed: http.FS(static.Static) should become FS: static.Static (not os.DirFS(...)) + assert.Contains(t, content, "FS: static.Static") + assert.NotContains(t, content, "os.DirFS(http.FS(static.Static))") + assert.NotContains(t, content, "os.DirFS(static.Static)") + + // Bug #2 fixed: middleware import should be aliased to avoid conflict + assert.Contains(t, content, `staticmw "github.com/gofiber/fiber/v3/middleware/static"`) + assert.Contains(t, content, `staticmw.New("", staticmw.Config{`) + + // User's import should remain unchanged + assert.Contains(t, content, `"myproject/internal/web/static"`) +}