diff --git a/README.md b/README.md index 2a634d1..d966a92 100644 --- a/README.md +++ b/README.md @@ -149,9 +149,50 @@ return for item in 1..3 { } ``` -Only a structurally recognized final top-level `FOR` without an explicit -terminal `return` is changed. Nested, assigned, expression-contained, -function-contained, non-final, and already-returned loops remain untouched. +For loop migration, only a structurally recognized final top-level `FOR` without +an explicit terminal `return` is changed. Nested, assigned, expression-contained, +function-contained, non-final, and already-returned loops retain their structure. + +The command also migrates safe unqualified legacy stdlib calls to canonical +lowercase names. For example, `JSON_PARSE(body)` becomes +`encoding::json_parse(body)`, `has(obj, key)` becomes `object::has_key(obj, key)`, +and `abs(value)` becomes `math::abs(value)`. Supported mappings include: + +| Namespace | Legacy calls migrated automatically | +| --- | --- | +| `encoding::` | `json_parse`, `json_stringify`, `encode_uri_component` → `query_escape`, `decode_uri_component` → `query_unescape`, `to_base64` → `base64_encode`, `from_base64` → `base64_decode`, `escape_html` → `html_escape`, `unescape_html` → `html_unescape` | +| `crypto::` | `md5`, `sha1`, `sha512`, `random_token` | +| `path::` | `base`, `clean`, `dir`, `ext`, `is_abs`, `separate`, `match` | +| `object::` | `values`, `has` → `has_key`, `zip`, `keep_keys`, `merge`, `merge_recursive` → `merge_deep`, and one-argument `keys(obj)` | +| `datetime::` | `now`, `date` → `parse`, `date_dayofweek` → `day_of_week`, `date_dayofyear` → `day_of_year`, `date_leapyear` → `is_leap_year`; `date_year`, `date_month`, `date_day`, `date_hour`, `date_minute`, `date_second`, `date_millisecond`, `date_quarter`, `date_days_in_month`, `date_format`, `date_add`, `date_subtract` lose their `date_` prefix | +| `math::` | `pi`, `abs`, `acos`, `asin`, `atan`, `atan2`, `ceil`, `cos`, `degrees`, `exp`, `exp2`, `floor`, `log`, `log2`, `log10`, `pow`, `radians`, `round`, `sin`, `sqrt`, `tan` | + +Only parsed call targets are replaced; arguments retain their meaning, and +strings, comments, object keys, and variable names are not matched. Nested calls +are migrated independently, and already-qualified targets are preserved. Object +replacements use immutable operations, never `object::mut`. + +Manual follow-up includes the original path and line and explains why a call +was preserved: + +- `join` is ambiguous between legacy path joining and modern global string joining. +- `keys` with any arity other than one needs argument-aware review. +- `date_compare` has component-range semantics that differ from `datetime::same`; + legacy `date_diff` integer/floating behavior differs from `datetime::diff`. +- `average`, `sum`, `min`, `max`, `median`, `percentile`, `stddev_population`, + `stddev_sample`, `variance_population`, and `variance_sample` have permissive + legacy behavior that differs from strict canonical math. +- A matching function declaration or function alias anywhere in the file may + change call resolution. These checks are deliberately conservative, including + case variants and declarations in nested scopes. A namespace alias blocks a + replacement only when it would redirect that canonical target. + +Other safe calls in the same file still migrate. Array migrations and +`rand`/`range` are deferred and receive no new diagnostics in this pass. +Rerunning migration produces no further edits; unresolved manual actions remain. +If the formatter cannot preserve comments, the entire file is left unchanged +and reported for manual follow-up. + Changed FQL is canonically formatted; files needing only formatting remain byte-for-byte unchanged. FQL-only targets do not require a Go module or Go toolchain and do not change Go dependencies. A directory containing eligible Go @@ -193,6 +234,14 @@ files. ### Checking FQL compatibility +`migrate check` reports final collecting `FOR` compatibility and the same legacy +stdlib findings as `migrate run`: encoding, crypto, path, immutable object, +datetime, and scalar math replacements, plus calls requiring manual review. +It uses the same conservative declaration and alias guards described above. +Already-qualified calls are left alone; arrays and `rand`/`range` remain outside +this pass. A clean check covers these supported rules, not all v1 application +semantics. + Check a standalone FQL file or recursively inspect a directory without modifying source files: @@ -209,19 +258,24 @@ nested Go modules. They skip `.git`, `.hg`, `.svn`, `vendor`, and `node_modules`, and do not follow directory symlinks. Compatibility findings use editor-friendly locations and include a suggested -manual fix: +replacement or a manual-review explanation: ```text 1_hackernews.fql:2:1: Final collecting FOR no longer becomes the script result in Ferret v2. help: Add `return` before this loop. -Found 1 v1 compatibility issue in 1 of 12 FQL files. +query.fql:1:8: Legacy stdlib call `has` should use `object::has_key`. + help: Preview automatic replacements with `ferret migrate run --print`. + +Found 2 v1 compatibility issues in 2 of 12 FQL files. ``` -The command exits nonzero when it finds a compatibility issue or cannot parse -an FQL file. Malformed files are reported while the remaining files continue -to be checked. Filesystem, cancellation, and internal failures stop the check -immediately. +The command exits nonzero for automatic replacement suggestions, manual-review +findings, or FQL parse failures. Use `ferret migrate run --print path/to/source` +to preview edits; the check does not verify whether the formatter can preserve +the source during rewriting. Malformed files are reported while the remaining +files continue to be checked. Filesystem, cancellation, and internal failures +stop the check immediately. ## Module lifecycle diff --git a/cmd/internal/migrate/check.go b/cmd/internal/migrate/check.go index 75f3999..ea11c4c 100644 --- a/cmd/internal/migrate/check.go +++ b/cmd/internal/migrate/check.go @@ -16,6 +16,10 @@ func newCompatibilityCheckCommand(service Service) *cobra.Command { Short: "Check FQL source for Ferret version compatibility", Long: "Check a standalone FQL file or recursively inspect a directory for supported behavior changes " + "between Ferret versions. The check does not require a Go module and never modifies source files.\n\n" + + "Checks final collecting FOR and legacy stdlib calls in encoding, crypto, path, object, datetime, and math. " + + "Findings suggest canonical replacements or manual review for ambiguous calls, semantic differences, " + + "and local function or use alias collisions. Arrays and rand/range are outside this pass. " + + "Preview automatic replacements with migrate run --print.\n\n" + "Directory scans include testdata, hidden and underscore-prefixed directories, and nested Go modules. " + "They skip .git, .hg, .svn, vendor, and node_modules and do not follow directory symlinks.\n\n" + "Compatibility findings and malformed FQL make the command fail after all readable files are checked.", diff --git a/cmd/internal/migrate/command_test.go b/cmd/internal/migrate/command_test.go index 1d91fa0..b1aa688 100644 --- a/cmd/internal/migrate/command_test.go +++ b/cmd/internal/migrate/command_test.go @@ -422,6 +422,11 @@ func TestMigrateCheckHelpDocumentsReadOnlyScanBoundaries(t *testing.T) { for _, expected := range []string{ "migrate check [path]", "does not require a Go module and never modifies source files", + "final collecting FOR and legacy stdlib calls in encoding, crypto, path, object, datetime, and math", + "manual review", + "local function or use alias collisions", + "Arrays and rand/range are outside this pass", + "migrate run --print", "include testdata, hidden and underscore-prefixed directories, and nested Go modules", "skip .git, .hg, .svn, vendor, and node_modules", "currently only v1", diff --git a/cmd/internal/migrate/run.go b/cmd/internal/migrate/run.go index 05b0bad..7143be8 100644 --- a/cmd/internal/migrate/run.go +++ b/cmd/internal/migrate/run.go @@ -26,6 +26,10 @@ func newRunCommand(store *config.Store, service Service) *cobra.Command { "directories, and nested Go modules. The selected directory itself is scanned regardless of its name. " + "Directory symlinks are not followed.\n\n" + "FQL migration returns and canonically formats a structurally recognized final top-level FOR. " + + "It also migrates safe legacy stdlib calls to encoding, crypto, path, object, datetime, and math namespaces. " + + "Ambiguous join, non-unary keys, date_compare, date_diff, aggregate math, and calls affected by " + + "local function or use alias collisions are left for manual follow-up. " + + "Arrays and rand/range are outside this pass. Already-qualified calls are preserved. " + "Malformed FQL is left unchanged and reported for manual follow-up.\n\n" + "The command performs only documented mechanical import, dependency, and source changes. " + "It does not convert application logic or arbitrary APIs to native Ferret v2 equivalents.", diff --git a/cmd/internal/migrate/stdlib_test.go b/cmd/internal/migrate/stdlib_test.go new file mode 100644 index 0000000..80d7baa --- /dev/null +++ b/cmd/internal/migrate/stdlib_test.go @@ -0,0 +1,146 @@ +package migrate + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/MontFerret/cli/v2/internal/migration" +) + +func TestMigrateRunStdlibPreviewAndApply(t *testing.T) { + path := filepath.Join(t.TempDir(), "query.fql") + before := "return [abs(-2), average(xs)]" + want := "return [math::abs(-2), average(xs)]" + if err := os.WriteFile(path, []byte(before), 0o600); err != nil { + t.Fatal(err) + } + + service := migration.New(nil) + for _, mode := range []string{"--dry-run", "--print", "apply"} { + args := []string{"run", path} + if mode != "apply" { + args = append(args, mode) + } + + stdout, stderr, err := executeMigrateCommand(t, service, args...) + if err != nil { + t.Fatal(err) + } + + if !strings.Contains(stderr, "query.fql:1: average(...)") || !strings.Contains(stderr, "heterogeneous") { + t.Fatalf("missing manual warning in mode %s: %s", mode, stderr) + } + + if mode == "--print" { + if !strings.HasPrefix(stdout, "--- a/query.fql\n+++ b/query.fql\n") || + !strings.Contains(stdout, "+"+want) || strings.Contains(stdout, "Manual follow-up") || + strings.Contains(stdout, "Scanned") || !strings.Contains(stderr, "Scanned 1 FQL file") { + t.Fatalf("print mixed diff and diagnostics: stdout=%q stderr=%q", stdout, stderr) + } + } else if !strings.Contains(stdout, "query.fql") || strings.Contains(stdout, "average(...)") { + t.Fatalf("unexpected human output: %s", stdout) + } + + contents, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + + expected := before + if mode == "apply" { + expected = want + } + + if string(contents) != expected { + t.Fatalf("mode %s contents = %q, want %q", mode, contents, expected) + } + } + + stdout, stderr, err := executeMigrateCommand(t, service, "run", "--print", path) + if err != nil { + t.Fatal(err) + } + + if stdout != "" || !strings.Contains(stderr, "average(...)") || !strings.Contains(stderr, "No safe automatic changes available") { + t.Fatalf("manual-only second run: stdout=%q stderr=%q", stdout, stderr) + } +} + +func TestMigrateRunStdlibHelp(t *testing.T) { + stdout, _, err := executeMigrateCommand(t, new(fakeMigrationService), "run", "--help") + if err != nil { + t.Fatal(err) + } + + for _, text := range []string{"safe legacy stdlib", "encoding, crypto, path, object, datetime, and math", "local function or use alias", "Arrays and rand/range"} { + if !strings.Contains(stdout, text) { + t.Fatalf("help does not contain %q: %s", text, stdout) + } + } +} + +func TestMigrateCheckStdlibReportsFindingsWithoutWriting(t *testing.T) { + tests := []struct { + name, input, diagnostic string + }{ + { + name: "has replacement", + input: `return has({ foo: "bar" }, "baz")`, + diagnostic: ":1:8: Legacy stdlib call `has` should use `object::has_key`.\n" + + " help: Preview automatic replacements with `ferret migrate run --print`.\n\n", + }, + { + name: "manual review only", + input: "return average(xs)", + diagnostic: ":1:8: Stdlib call `average` needs manual review.\n" + + " help: legacy aggregate behavior is permissive for heterogeneous collections; " + + "the strict math API requires a separate semantic migration\n\n", + }, + { + name: "canonical source", + input: `return object::has_key({ foo: "bar" }, "baz")`, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "query.fql") + if err := os.WriteFile(path, []byte(test.input), 0o600); err != nil { + t.Fatal(err) + } + + workingDirectory, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + + displayPath, err := filepath.Rel(workingDirectory, path) + if err != nil { + t.Fatal(err) + } + + for range 2 { + stdout, stderr, err := executeMigrateCommand(t, migration.New(nil), "check", path) + if test.diagnostic == "" { + if err != nil || stderr != "" || stdout != "✓ No v1 compatibility issues found in 1 FQL file.\n" { + t.Fatalf("unexpected clean check: stdout=%q stderr=%q err=%v", stdout, stderr, err) + } + } else if err == nil || err.Error() != "Found 1 v1 compatibility issue in 1 of 1 FQL file." || + stdout != "" || stderr != filepath.ToSlash(displayPath)+test.diagnostic { + t.Fatalf("unexpected check finding: stdout=%q stderr=%q err=%v", stdout, stderr, err) + } + + contents, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + + if string(contents) != test.input { + t.Fatalf("check changed source: %q", contents) + } + } + }) + } +} diff --git a/internal/migration/compatibility.go b/internal/migration/compatibility.go index e3e98ec..b6083cb 100644 --- a/internal/migration/compatibility.go +++ b/internal/migration/compatibility.go @@ -59,7 +59,7 @@ func checkFQLCompatibility(ctx context.Context, options CompatibilityOptions) (* } src := source.New(displayPath, string(data)) - loop, err := finalTopLevelFQLFor(src) + program, err := parseFQLSource(src) if err != nil { detail, line, column := fqlDiagnosticDetails(src, err) result.Diagnostics = append(result.Diagnostics, CompatibilityDiagnostic{ @@ -74,6 +74,14 @@ func checkFQLCompatibility(ctx context.Context, options CompatibilityOptions) (* continue } + stdlib, err := checkFQLStdlib(src, program) + if err != nil { + return nil, fmt.Errorf("locate compatibility issue in %s: %w", displayPath, err) + } + + result.Diagnostics = append(result.Diagnostics, stdlib...) + + loop := finalFQLFor(program) if loop == nil { continue } diff --git a/internal/migration/compatibility_stdlib.go b/internal/migration/compatibility_stdlib.go new file mode 100644 index 0000000..501c987 --- /dev/null +++ b/internal/migration/compatibility_stdlib.go @@ -0,0 +1,46 @@ +package migration + +import ( + "fmt" + + "github.com/MontFerret/ferret/v2/pkg/parser/fql" + "github.com/MontFerret/ferret/v2/pkg/source" +) + +func checkFQLStdlib(src source.Source, program *fql.ProgramContext) ([]CompatibilityDiagnostic, error) { + var diagnostics []CompatibilityDiagnostic + for _, finding := range analyzeFQLStdlib(program) { + name := finding.name + spelling := name.GetText() + + span, ok := fqlByteSpan(src.Content(), source.Span{ + Start: name.GetStart().GetStart(), + End: name.GetStop().GetStop() + 1, + }) + if !ok || span.End <= span.Start { + return nil, fmt.Errorf("locate stdlib call %s in Ferret source", spelling) + } + + position := src.PositionAt(span) + if position.Line == 0 || position.Column == 0 { + return nil, fmt.Errorf("resolve stdlib call %s location", spelling) + } + + diagnostic := CompatibilityDiagnostic{ + Path: src.Name(), + Message: fmt.Sprintf("Legacy stdlib call `%s` should use `%s`.", spelling, finding.target), + Help: "Preview automatic replacements with `ferret migrate run --print`.", + Line: position.Line, + Column: position.Column, + Kind: CompatibilityDiagnosticIssue, + } + if finding.reason != "" { + diagnostic.Message = fmt.Sprintf("Stdlib call `%s` needs manual review.", spelling) + diagnostic.Help = finding.reason + } + + diagnostics = append(diagnostics, diagnostic) + } + + return diagnostics, nil +} diff --git a/internal/migration/compatibility_stdlib_test.go b/internal/migration/compatibility_stdlib_test.go new file mode 100644 index 0000000..5350e4c --- /dev/null +++ b/internal/migration/compatibility_stdlib_test.go @@ -0,0 +1,275 @@ +package migration + +import ( + "context" + "fmt" + "maps" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/MontFerret/ferret/v2/pkg/source" +) + +func TestCheckFQLStdlibReplacements(t *testing.T) { + tests := []struct{ call, name, target string }{ + {`has({ foo: "bar" }, "baz")`, "has", "object::has_key"}, + {`JSON_PARSE("{}")`, "JSON_PARSE", "encoding::json_parse"}, + {`Sha1(text)`, "Sha1", "crypto::sha1"}, + {`base(path)`, "base", "path::base"}, + {`keys(obj)`, "keys", "object::keys"}, + {`Date_Year(dt)`, "Date_Year", "datetime::year"}, + {`ABS(-1)`, "ABS", "math::abs"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + diagnostics := checkStdlibTestSource(t, "return "+test.call) + want := []CompatibilityDiagnostic{{ + Path: "query.fql", + Message: fmt.Sprintf("Legacy stdlib call `%s` should use `%s`.", test.name, test.target), + Help: "Preview automatic replacements with `ferret migrate run --print`.", + Line: 1, + Column: 8, + Kind: CompatibilityDiagnosticIssue, + }} + if !reflect.DeepEqual(diagnostics, want) { + t.Fatalf("diagnostics = %#v, want %#v", diagnostics, want) + } + + canonical := "return " + test.target + strings.TrimPrefix(test.call, test.name) + if diagnostics := checkStdlibTestSource(t, canonical); len(diagnostics) != 0 { + t.Fatalf("canonical source has findings: %#v", diagnostics) + } + }) + } +} + +func TestCheckFQLStdlibManualReview(t *testing.T) { + tests := []struct{ call, reason string }{ + {`join("a", "b")`, "legacy path::join"}, + {`join(["a", "b"], ",")`, "modern global string joining"}, + {"keys()", "only keys(obj)"}, + {"keys(obj, true)", "argument-aware"}, + {"keys(obj, false)", "argument-aware"}, + {"keys(obj, option)", "argument-aware"}, + {"keys(obj, true, extra)", "argument-aware"}, + {`date_compare(a, b, "year")`, "component-range"}, + {`date_diff(a, b, "day")`, "integer/floating"}, + {"average(xs)", "heterogeneous"}, + {"sum(xs)", "heterogeneous"}, + {"min(xs)", "heterogeneous"}, + {"max(xs)", "heterogeneous"}, + {"median(xs)", "heterogeneous"}, + {"percentile(xs, 50)", "heterogeneous"}, + {"stddev_population(xs)", "heterogeneous"}, + {"stddev_sample(xs)", "heterogeneous"}, + {"variance_population(xs)", "heterogeneous"}, + {"variance_sample(xs)", "heterogeneous"}, + } + + for _, test := range tests { + t.Run(test.call, func(t *testing.T) { + input := "// original location\nRETURN " + test.call + diagnostics := checkStdlibTestSource(t, input) + + name, _, _ := strings.Cut(test.call, "(") + if len(diagnostics) != 1 || diagnostics[0].Path != "query.fql" || + diagnostics[0].Kind != CompatibilityDiagnosticIssue || + diagnostics[0].Line != 2 || diagnostics[0].Column != 10 || + diagnostics[0].Message != "Stdlib call `"+name+"` needs manual review." || + !strings.Contains(diagnostics[0].Help, test.reason) { + t.Fatalf("unexpected manual finding: %#v", diagnostics) + } + + migration, err := migrateFQLSource(source.New("query.fql", input)) + if err != nil { + t.Fatal(err) + } + + if len(migration.ManualActions) != 1 || migration.ManualActions[0].Reason != diagnostics[0].Help { + t.Fatalf("check and run disagree: diagnostics=%#v actions=%#v", diagnostics, migration.ManualActions) + } + }) + } +} + +func TestCheckFQLStdlibResolutionGuards(t *testing.T) { + tests := []struct{ input, reason string }{ + {"func ABS(x) { return x }\nreturn Abs(-1)", "file-local"}, + {"let value = abs(-1)\nfunc abs(x) { return x }\nreturn value", "file-local"}, + {"func outer() { func abs(x) { return x } return 1 }\nreturn abs(-1)", "file-local"}, + {"use custom::calculate as abs\nreturn ABS(-1)", "function alias"}, + {"use custom as math\nreturn abs(-1)", "redirect the replacement math::abs"}, + {"use custom::calculate as math\nreturn abs(-1)", "redirect the replacement math::abs"}, + {"use custom as math\nuse math as math\nreturn abs(-1)", "redirect"}, + {"func average(xs) { return xs }\nreturn average(xs)", "file-local"}, + } + + for _, test := range tests { + t.Run(test.input, func(t *testing.T) { + diagnostics := checkStdlibTestSource(t, test.input) + if len(diagnostics) != 1 || !strings.Contains(diagnostics[0].Message, "needs manual review") || + !strings.Contains(diagnostics[0].Help, test.reason) { + t.Fatalf("unexpected collision finding: %#v", diagnostics) + } + }) + } + + for _, header := range []string{ + "use custom as other", + "use custom as abs", + "use custom::calculate as other", + "use math as math", + "use custom as MATH", + } { + diagnostics := checkStdlibTestSource(t, header+"\nreturn abs(-1)") + if len(diagnostics) != 1 || diagnostics[0].Message != "Legacy stdlib call `abs` should use `math::abs`." { + t.Fatalf("unrelated alias suppressed replacement: %#v", diagnostics) + } + } +} + +func TestCheckFQLStdlibIgnoresUnrelatedSource(t *testing.T) { + input := `use custom as math +// abs(-1) and average(xs) +let has = "json_parse()" +return { has, abs: "date_diff()", values: [ + math::abs(-1), CUSTOM::HAS(obj, "key"), object::keys(obj), + union(a, b), push(a, 1), nth(a, 0), rand(), range(1, 10) +] }` + if diagnostics := checkStdlibTestSource(t, input); len(diagnostics) != 0 { + t.Fatalf("unrelated source has findings: %#v", diagnostics) + } +} + +func TestCheckFQLCompatibilityCombinesStdlibAndLoopFindings(t *testing.T) { + root := writeCompatibilityFixture(t, map[string]string{ + "a_broken.fql": "return [average(xs),", + "b_mixed.fql": `let label = "日本語" +for item in values(obj) + return ["λ", has(item, "key"), average([abs(-1)]), average(item)]`, + "c_comments.fql": `return [average(xs), sha1 /* preserve me */ (text)]`, + "d_collision.fql": "func ABS(x) { return x }\nreturn [abs(-1), sha1(text)]", + "go.mod": "module example.com/no-toolchain\n", + }) + path := filepath.Join(root, "b_mixed.fql") + if err := os.Chmod(path, 0o600); err != nil { + t.Fatal(err) + } + + before := compatibilityFixtureContents(t, root) + var previous *CompatibilityResult + for range 2 { + // No migration planner or Go runner is configured: check only inspects FQL. + result, err := new(Migrator).CheckCompatibility(context.Background(), CompatibilityOptions{Path: root}) + if err != nil { + t.Fatal(err) + } + + if result.ScannedFiles != 4 || len(result.Diagnostics) != 11 { + t.Fatalf("unexpected combined findings: %#v", result) + } + + broken := result.Diagnostics[0] + if broken.Kind != CompatibilityDiagnosticFailure || !strings.HasPrefix(broken.Message, "Could not check v1 compatibility:") { + t.Fatalf("unexpected parse failure: %#v", broken) + } + + want := []struct { + file, message string + line, column int + }{ + {"b_mixed.fql", finalForMessage, 2, 1}, + {"b_mixed.fql", "Legacy stdlib call `values` should use `object::values`.", 2, 13}, + {"b_mixed.fql", "Legacy stdlib call `has` should use `object::has_key`.", 3, 19}, + {"b_mixed.fql", "Stdlib call `average` needs manual review.", 3, 37}, + {"b_mixed.fql", "Legacy stdlib call `abs` should use `math::abs`.", 3, 46}, + {"b_mixed.fql", "Stdlib call `average` needs manual review.", 3, 57}, + {"c_comments.fql", "Stdlib call `average` needs manual review.", 1, 9}, + {"c_comments.fql", "Legacy stdlib call `sha1` should use `crypto::sha1`.", 1, 22}, + {"d_collision.fql", "Stdlib call `abs` needs manual review.", 2, 9}, + {"d_collision.fql", "Legacy stdlib call `sha1` should use `crypto::sha1`.", 2, 18}, + } + for index, expected := range want { + diagnostic := result.Diagnostics[index+1] + if diagnostic.Path != compatibilityTestDisplayPath(t, filepath.Join(root, expected.file)) || + diagnostic.Kind != CompatibilityDiagnosticIssue || diagnostic.Message != expected.message || + diagnostic.Line != expected.line || diagnostic.Column != expected.column { + t.Fatalf("diagnostic %d = %#v, want %#v", index+1, diagnostic, expected) + } + } + + if previous != nil && !reflect.DeepEqual(result, previous) { + t.Fatalf("retry changed diagnostics: before=%#v after=%#v", previous, result) + } + + previous = result + + if after := compatibilityFixtureContents(t, root); !maps.Equal(before, after) { + t.Fatal("check modified files") + } + + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + + if info.Mode().Perm() != 0o600 { + t.Fatalf("check changed file mode: %v", info.Mode()) + } + } +} + +func BenchmarkCheckFQLStdlibCompatibility(b *testing.B) { + for _, test := range []struct { + name, input string + findings int + }{ + {"legacy", "return [json_parse(text), sha1(text), base(path), has(obj, key), date_year(dt), abs(-1), average(xs)]", 700}, + {"canonical", "return [encoding::json_parse(text), crypto::sha1(text), path::base(path), object::has_key(obj, key), datetime::year(dt), math::abs(-1)]", 0}, + } { + b.Run(test.name, func(b *testing.B) { + root := b.TempDir() + for index := range 100 { + path := filepath.Join(root, fmt.Sprintf("query_%03d.fql", index)) + if err := os.WriteFile(path, []byte(test.input), 0o644); err != nil { + b.Fatal(err) + } + } + + b.ReportAllocs() + b.ResetTimer() + + for range b.N { + result, err := new(Migrator).CheckCompatibility(context.Background(), CompatibilityOptions{Path: root}) + if err != nil { + b.Fatal(err) + } + + if result.ScannedFiles != 100 || len(result.Diagnostics) != test.findings { + b.Fatalf("unexpected benchmark findings: %#v", result) + } + } + }) + } +} + +func checkStdlibTestSource(t *testing.T, input string) []CompatibilityDiagnostic { + t.Helper() + + src := source.New("query.fql", input) + program, err := parseFQLSource(src) + if err != nil { + t.Fatal(err) + } + + diagnostics, err := checkFQLStdlib(src, program) + if err != nil { + t.Fatal(err) + } + + return diagnostics +} diff --git a/internal/migration/fql_edits.go b/internal/migration/fql_edits.go new file mode 100644 index 0000000..5ed9706 --- /dev/null +++ b/internal/migration/fql_edits.go @@ -0,0 +1,53 @@ +package migration + +import ( + "cmp" + "fmt" + "slices" + "unicode/utf8" +) + +type fqlSourceEdit struct { + start int + end int + text string +} + +// Edits use byte offsets in the original source. Applying them from right to left +// lets loop insertions and nested call renames share one parse without shifting spans. +func applyFQLEdits(content string, edits []fqlSourceEdit) (string, error) { + slices.SortFunc(edits, func(a, b fqlSourceEdit) int { + return cmp.Compare(b.start, a.start) + }) + + size := len(content) + boundary := len(content) + for i, edit := range edits { + if edit.start < 0 || edit.end < edit.start || edit.end > boundary || + (i > 0 && edit.start == edits[i-1].start) { + return "", fmt.Errorf("apply Ferret source edits: invalid or overlapping span") + } + + if (edit.start < len(content) && !utf8.RuneStart(content[edit.start])) || + (edit.end < len(content) && !utf8.RuneStart(content[edit.end])) { + return "", fmt.Errorf("apply Ferret source edits: span splits a UTF-8 rune") + } + + size += len(edit.text) - (edit.end - edit.start) + boundary = edit.start + } + + output := make([]byte, size) + read, write := len(content), size + for _, edit := range edits { + write -= read - edit.end + copy(output[write:], content[edit.end:read]) + write -= len(edit.text) + copy(output[write:], edit.text) + read = edit.start + } + + copy(output[:write], content[:read]) + + return string(output), nil +} diff --git a/internal/migration/fql_edits_test.go b/internal/migration/fql_edits_test.go new file mode 100644 index 0000000..10bc06f --- /dev/null +++ b/internal/migration/fql_edits_test.go @@ -0,0 +1,39 @@ +package migration + +import "testing" + +func TestApplyFQLEdits(t *testing.T) { + content := "πabc" + edits := []fqlSourceEdit{ + {start: 0, end: 0, text: "("}, + {start: 3, end: 4, text: "long"}, + {start: 5, end: 5, text: ")"}, + } + got, err := applyFQLEdits(content, edits) + if err != nil { + t.Fatal(err) + } + + if got != "(πalongc)" { + t.Fatalf("edits used shifted offsets: %q", got) + } +} + +func TestApplyFQLEditsRejectsInvalidSpans(t *testing.T) { + tests := [][]fqlSourceEdit{ + {{start: -1, end: 0}}, + {{start: 3, end: 2}}, + {{start: 0, end: 6}}, + {{start: 6, end: 6}}, + {{start: 1, end: 2}}, + {{start: 0, end: 1}}, + {{start: 2, end: 4}, {start: 3, end: 5}}, + {{start: 2, end: 5}, {start: 3, end: 3}}, + {{start: 2, end: 2}, {start: 2, end: 2}}, + } + for _, edits := range tests { + if _, err := applyFQLEdits("πabc", edits); err == nil { + t.Fatalf("accepted invalid edits: %#v", edits) + } + } +} diff --git a/internal/migration/fql_source.go b/internal/migration/fql_source.go index 9f33599..8ece050 100644 --- a/internal/migration/fql_source.go +++ b/internal/migration/fql_source.go @@ -40,6 +40,8 @@ func planFQLSourceChanges(ctx context.Context, project *migrationProject) (*fqlS src := source.New(relative, string(snapshot.Data)) migration, err := migrateFQLSource(src) + result.ManualActions = append(result.ManualActions, migration.ManualActions...) + if err != nil { result.ManualActions = append(result.ManualActions, fqlManualAction(relative, src, err)) @@ -69,64 +71,85 @@ func planFQLSourceChanges(ctx context.Context, project *migrationProject) (*fqlS } func migrateFQLSource(src source.Source) (fqlMigrationResult, error) { - loop, err := finalTopLevelFQLFor(src) + program, err := parseFQLSource(src) if err != nil { return fqlMigrationResult{}, err } - if loop == nil { - return fqlMigrationResult{}, nil + edits, err := finalFQLForEdits(src.Content(), finalFQLFor(program)) + if err != nil { + return fqlMigrationResult{}, err } - migrated, err := rewriteFinalFQLFor(src.Content(), loop) + stdlibEdits, actions, err := planFQLStdlib(src, program) if err != nil { return fqlMigrationResult{}, err } - formatted, err := formatMigratedFQLSource(source.New(src.Name(), migrated)) + result := fqlMigrationResult{ManualActions: actions} + + edits = append(edits, stdlibEdits...) + if len(edits) == 0 { + return result, nil + } + + migrated, err := applyFQLEdits(src.Content(), edits) if err != nil { - return fqlMigrationResult{}, err + return result, err + } + + formatted, err := formatMigratedFQLSource(source.New(src.Name(), migrated), fqlComments(program)) + if err != nil { + return result, err } - return fqlMigrationResult{Data: formatted, Changed: true}, nil + result.Data = formatted + result.Changed = true + + return result, nil } func finalTopLevelFQLFor(src source.Source) (fql.IForExpressionContext, error) { - if !utf8.ValidString(src.Content()) { - return nil, fmt.Errorf("parse Ferret source: source is not valid UTF-8") - } - program, err := parseFQLSource(src) if err != nil { return nil, err } + return finalFQLFor(program), nil +} + +func finalFQLFor(program *fql.ProgramContext) fql.IForExpressionContext { body := program.Body() if body == nil || body.BodyExpression() != nil { - return nil, nil + return nil } statements := body.AllBodyStatement() if len(statements) == 0 { - return nil, nil + return nil } loop := statements[len(statements)-1].ForExpression() if loop == nil || loop.GetStart() == nil { - return nil, nil + return nil } - return loop, nil + return loop } -func rewriteFinalFQLFor(content string, loop fql.IForExpressionContext) (string, error) { +func finalFQLForEdits(content string, loop fql.IForExpressionContext) ([]fqlSourceEdit, error) { + if loop == nil { + return nil, nil + } + start, ok := fqlByteOffset(content, loop.GetStart().GetStart()) if !ok { - return "", fmt.Errorf("locate final top-level FOR in Ferret source") + return nil, fmt.Errorf("locate final top-level FOR in Ferret source") } + edits := []fqlSourceEdit{{start: start, end: start, text: "return "}} if loop.OpenBrace() != nil { - return content[:start] + "return " + content[start:], nil + return edits, nil } headerStop := -1 @@ -137,24 +160,30 @@ func rewriteFinalFQLFor(content string, loop fql.IForExpressionContext) (string, } if headerStop < 0 || loop.GetStop() == nil { - return "", fmt.Errorf("locate final top-level FOR boundaries in Ferret source") + return nil, fmt.Errorf("locate final top-level FOR boundaries in Ferret source") } headerEnd, ok := fqlByteOffset(content, headerStop+1) if !ok { - return "", fmt.Errorf("locate final top-level FOR header in Ferret source") + return nil, fmt.Errorf("locate final top-level FOR header in Ferret source") } loopEnd, ok := fqlByteOffset(content, loop.GetStop().GetStop()+1) if !ok || headerEnd > loopEnd { - return "", fmt.Errorf("locate final top-level FOR body in Ferret source") + return nil, fmt.Errorf("locate final top-level FOR body in Ferret source") } - return content[:start] + "return " + content[start:headerEnd] + " {" + - content[headerEnd:loopEnd] + "\n}" + content[loopEnd:], nil + return append(edits, + fqlSourceEdit{start: headerEnd, end: headerEnd, text: " {"}, + fqlSourceEdit{start: loopEnd, end: loopEnd, text: "\n}"}, + ), nil } func parseFQLSource(src source.Source) (program *fql.ProgramContext, err error) { + if !utf8.ValidString(src.Content()) { + return nil, fmt.Errorf("parse Ferret source: source is not valid UTF-8") + } + defer func() { if recovered := recover(); recovered != nil { program = nil @@ -178,7 +207,7 @@ func parseFQLSource(src source.Source) (program *fql.ProgramContext, err error) return program, nil } -func formatMigratedFQLSource(src source.Source) (data []byte, err error) { +func formatMigratedFQLSource(src source.Source, comments []string) (data []byte, err error) { defer func() { if recovered := recover(); recovered != nil { data = nil @@ -210,13 +239,34 @@ func formatMigratedFQLSource(src source.Source) (data []byte, err error) { return nil, fmt.Errorf("format migrated Ferret source: formatter did not preserve non-ASCII source text") } - if _, err := parseFQLSource(source.New(src.Name(), string(formatted))); err != nil { + program, err := parseFQLSource(source.New(src.Name(), string(formatted))) + if err != nil { return nil, fmt.Errorf("validate formatted Ferret source: %w", err) } + if !slices.Equal(comments, fqlComments(program)) { + return nil, fmt.Errorf("format migrated Ferret source: formatter did not preserve comments") + } + return formatted, nil } +// Hidden-channel comment tokens retain their exact text. Comparing them prevents +// formatter limitations from silently discarding comments after a safe rewrite. +func fqlComments(program *fql.ProgramContext) []string { + stream := program.GetParser().GetTokenStream() + var comments []string + for i := 0; i < stream.Size(); i++ { + token := stream.Get(i) + switch token.GetTokenType() { + case fql.FqlLexerMultiLineComment, fql.FqlLexerSingleLineComment: + comments = append(comments, token.GetText()) + } + } + + return comments +} + type fqlNonASCIIReplacement struct { marker string value rune diff --git a/internal/migration/fql_stdlib.go b/internal/migration/fql_stdlib.go new file mode 100644 index 0000000..43dfc18 --- /dev/null +++ b/internal/migration/fql_stdlib.go @@ -0,0 +1,232 @@ +package migration + +import ( + "fmt" + "strings" + + "github.com/antlr4-go/antlr/v4" + + "github.com/MontFerret/ferret/v2/pkg/parser/fql" + "github.com/MontFerret/ferret/v2/pkg/source" +) + +type ( + fqlStdlibRule struct { + target string + reason string + special func(fql.IFunctionCallContext) (target, reason string) + } + + fqlStdlibCalls struct { + calls []*fql.FunctionCallContext + locals map[string]bool + aliases map[string]string + } + + fqlStdlibFinding struct { + name fql.IFunctionNameContext + target string + reason string + } +) + +const fqlAggregateReason = "legacy aggregate behavior is permissive for heterogeneous collections; " + + "the strict math API requires a separate semantic migration" + +// Only behavior-preserving call-target replacements belong here. Arrays and +// rand/range are intentionally outside this pass, including manual diagnostics. +var fqlStdlibRules = map[string]fqlStdlibRule{ + "json_parse": {target: "encoding::json_parse"}, + "json_stringify": {target: "encoding::json_stringify"}, + "encode_uri_component": {target: "encoding::query_escape"}, + "decode_uri_component": {target: "encoding::query_unescape"}, + "to_base64": {target: "encoding::base64_encode"}, + "from_base64": {target: "encoding::base64_decode"}, + "escape_html": {target: "encoding::html_escape"}, + "unescape_html": {target: "encoding::html_unescape"}, + + "md5": {target: "crypto::md5"}, + "sha1": {target: "crypto::sha1"}, + "sha512": {target: "crypto::sha512"}, + "random_token": {target: "crypto::random_token"}, + + "base": {target: "path::base"}, + "clean": {target: "path::clean"}, + "dir": {target: "path::dir"}, + "ext": {target: "path::ext"}, + "is_abs": {target: "path::is_abs"}, + "separate": {target: "path::separate"}, + "match": {target: "path::match"}, + // There is no source-version marker: rerunning migration must preserve modern JOIN. + "join": {reason: "join may mean legacy path::join or modern global string joining; the source version is ambiguous"}, + + "values": {target: "object::values"}, + "has": {target: "object::has_key"}, + "zip": {target: "object::zip"}, + "keep_keys": {target: "object::keep_keys"}, + "merge": {target: "object::merge"}, + "merge_recursive": {target: "object::merge_deep"}, + "keys": {special: migrateFQLKeys}, + + "now": {target: "datetime::now"}, + "date": {target: "datetime::parse"}, + "date_dayofweek": {target: "datetime::day_of_week"}, + "date_year": {target: "datetime::year"}, + "date_month": {target: "datetime::month"}, + "date_day": {target: "datetime::day"}, + "date_hour": {target: "datetime::hour"}, + "date_minute": {target: "datetime::minute"}, + "date_second": {target: "datetime::second"}, + "date_millisecond": {target: "datetime::millisecond"}, + "date_dayofyear": {target: "datetime::day_of_year"}, + "date_leapyear": {target: "datetime::is_leap_year"}, + "date_quarter": {target: "datetime::quarter"}, + "date_days_in_month": {target: "datetime::days_in_month"}, + "date_format": {target: "datetime::format"}, + "date_add": {target: "datetime::add"}, + "date_subtract": {target: "datetime::subtract"}, + "date_compare": {reason: "legacy component-range comparison semantics differ from datetime::same; " + + "it is not a drop-in replacement"}, + "date_diff": {reason: "legacy integer/floating behavior differs from the canonical datetime::diff contract"}, + + "pi": {target: "math::pi"}, + "abs": {target: "math::abs"}, + "acos": {target: "math::acos"}, + "asin": {target: "math::asin"}, + "atan": {target: "math::atan"}, + "atan2": {target: "math::atan2"}, + "ceil": {target: "math::ceil"}, + "cos": {target: "math::cos"}, + "degrees": {target: "math::degrees"}, + "exp": {target: "math::exp"}, + "exp2": {target: "math::exp2"}, + "floor": {target: "math::floor"}, + "log": {target: "math::log"}, + "log2": {target: "math::log2"}, + "log10": {target: "math::log10"}, + "pow": {target: "math::pow"}, + "radians": {target: "math::radians"}, + "round": {target: "math::round"}, + "sin": {target: "math::sin"}, + "sqrt": {target: "math::sqrt"}, + "tan": {target: "math::tan"}, + + "average": {reason: fqlAggregateReason}, + "sum": {reason: fqlAggregateReason}, + "min": {reason: fqlAggregateReason}, + "max": {reason: fqlAggregateReason}, + "median": {reason: fqlAggregateReason}, + "percentile": {reason: fqlAggregateReason}, + "stddev_population": {reason: fqlAggregateReason}, + "stddev_sample": {reason: fqlAggregateReason}, + "variance_population": {reason: fqlAggregateReason}, + "variance_sample": {reason: fqlAggregateReason}, +} + +func migrateFQLKeys(call fql.IFunctionCallContext) (string, string) { + args := call.ArgumentList() + if args != nil && len(args.AllExpression()) == 1 { + return "object::keys", "" + } + + return "", "only keys(obj) has a mechanical replacement; other arities require separate argument-aware review" +} + +func planFQLStdlib(src source.Source, program *fql.ProgramContext) ([]fqlSourceEdit, []ManualAction, error) { + var edits []fqlSourceEdit + var actions []ManualAction + for _, finding := range analyzeFQLStdlib(program) { + name := finding.name + spelling := name.GetText() + if finding.reason != "" { + actions = append(actions, ManualAction{ + Path: src.Name(), + Detail: spelling + "(...)", + Reason: finding.reason, + Line: name.GetStart().GetLine(), + }) + + continue + } + + span, ok := fqlByteSpan(src.Content(), source.Span{ + Start: name.GetStart().GetStart(), + End: name.GetStop().GetStop() + 1, + }) + if !ok || span.End <= span.Start { + return nil, nil, fmt.Errorf("locate stdlib call %s in Ferret source", spelling) + } + + edits = append(edits, fqlSourceEdit{start: span.Start, end: span.End, text: finding.target}) + } + + return edits, actions, nil +} + +// Share rule selection and resolution guards between read-only checks and edits. +// A reason takes precedence over the proposed target and requires manual review. +func analyzeFQLStdlib(program *fql.ProgramContext) []fqlStdlibFinding { + info := fqlStdlibCalls{locals: make(map[string]bool), aliases: make(map[string]string)} + collectFQLStdlibCalls(program, &info) + + var findings []fqlStdlibFinding + for _, call := range info.calls { + if namespace := call.Namespace(); namespace != nil && namespace.GetText() != "" { + continue + } + + name := call.FunctionName() + spelling := name.GetText() + rule, ok := fqlStdlibRules[strings.ToLower(spelling)] + if !ok { + continue + } + + target, reason := rule.target, rule.reason + if rule.special != nil { + target, reason = rule.special(call) + } + + if info.locals[strings.ToLower(spelling)] { + reason = "a file-local function declaration or use function alias may resolve this call; the stdlib target is ambiguous" + } else if namespace, _, _ := strings.Cut(target, "::"); namespace != "" { + if alias, exists := info.aliases[namespace]; exists && alias != namespace { + reason = fmt.Sprintf("use alias %q would redirect the replacement %s; the canonical target is ambiguous", namespace, target) + } + } + + findings = append(findings, fqlStdlibFinding{name: name, target: target, reason: reason}) + } + + return findings +} + +// File-wide guards deliberately sacrifice some migrations to avoid implementing +// compiler scope resolution here. Collect declarations before considering any call. +func collectFQLStdlibCalls(node antlr.Tree, info *fqlStdlibCalls) { + switch node := node.(type) { + case *fql.FunctionCallContext: + info.calls = append(info.calls, node) + case *fql.FunctionDeclarationContext: + info.locals[strings.ToLower(node.FunctionName().GetText())] = true + case *fql.UseContext: + if alias := node.GetAlias(); alias != nil { + name := alias.GetText() + target := strings.TrimSuffix(node.NamespaceIdentifier().GetText(), "::") + if strings.Contains(target, "::") { + info.locals[strings.ToLower(name)] = true + } + + if previous, exists := info.aliases[name]; exists && previous != target { + // Conflicting declarations must not hide a possible redirection. + target = "" + } + + info.aliases[name] = target + } + } + + for i := 0; i < node.GetChildCount(); i++ { + collectFQLStdlibCalls(node.GetChild(i), info) + } +} diff --git a/internal/migration/fql_stdlib_integration_test.go b/internal/migration/fql_stdlib_integration_test.go new file mode 100644 index 0000000..888fb85 --- /dev/null +++ b/internal/migration/fql_stdlib_integration_test.go @@ -0,0 +1,270 @@ +package migration + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "reflect" + "strings" + "testing" +) + +func TestMigratorStdlibStandaloneModes(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, "query.fql") + before := "// keep location\nreturn [abs(-2), keys(obj, true)]" + want := "// keep location\nreturn [math::abs(-2), keys(obj, true)]" + if err := os.WriteFile(path, []byte(before), 0o600); err != nil { + t.Fatal(err) + } + + migrator, runner := newFixtureMigrator() + var actions []ManualAction + for _, mode := range []Mode{ModeDryRun, ModePrint, ModeApply} { + result, err := migrator.Migrate(context.Background(), Options{Path: path, Mode: mode}) + if err != nil { + t.Fatal(err) + } + + if result.Applied != (mode == ModeApply) || result.ScannedFQLFiles != 1 || result.MigratedFQLFiles != 1 || + len(result.Changes) != 1 || len(result.ManualActions) != 1 || result.DependenciesChanged { + t.Fatalf("unexpected mode %d result: %#v", mode, result) + } + + if string(result.Changes[0].After) != want || result.ManualActions[0].Line != 2 || result.ManualActions[0].Path != "query.fql" { + t.Fatalf("unexpected migration contents: %#v", result) + } + + if actions != nil && !reflect.DeepEqual(actions, result.ManualActions) { + t.Fatalf("manual reports changed between modes: %#v", result.ManualActions) + } + + actions = result.ManualActions + expected := before + if mode == ModeApply { + expected = want + } + + if got := readMigrationFixture(t, path); got != expected { + t.Fatalf("mode %d contents = %q, want %q", mode, got, expected) + } + } + + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + + if info.Mode().Perm() != 0o600 { + t.Fatalf("mode = %o, want 600", info.Mode().Perm()) + } + + second, err := migrator.Migrate(context.Background(), Options{Path: path}) + if err != nil { + t.Fatal(err) + } + + if second.Applied || len(second.Changes) != 0 || !reflect.DeepEqual(second.ManualActions, actions) { + t.Fatalf("second run changed source or lost manual actions: %#v", second) + } + + if runner.runCalls != 0 || runner.getCalls != 0 { + t.Fatalf("FQL-only migration used Go tooling: %#v", runner) + } +} + +func TestMigratorStdlibDirectoryReportingAndExclusions(t *testing.T) { + root := writeMigrationFixture(t, "module example.com/app\n\ngo 1.26.0\n", map[string]string{ + "main.go": "package app\n", + "go.sum": "preserve dependencies\n", + "a_mixed.fql": "let obj = json_parse(body)\nreturn [date_diff(a, b, unit), average(xs)]", + "b_manual.fql": "RETURN join(a, b)", + "c_broken.fql": "return abs(1) + (", + "d_valid.fql": "return sha1(body)", + "e_comments.fql": "return sha1 /* preserve me */ (body)", + "vendor/query.fql": "return abs(1)", + "testdata/query.fql": "return abs(1)", + "node_modules/query.fql": "return abs(1)", + ".hidden/query.fql": "return abs(1)", + "_generated/query.fql": "return abs(1)", + "upper.FQL": "return abs(1)", + "nested/go.mod": "module example.com/nested\n", + "nested/query.fql": "return abs(1)", + }) + before := snapshotMigrationFixture(t, root) + migrator, runner := newFixtureMigrator() + result, err := migrator.Migrate(context.Background(), Options{Path: root}) + if err != nil { + t.Fatal(err) + } + + if !result.Applied || result.ScannedFQLFiles != 5 || result.MigratedFQLFiles != 2 || + len(result.ManualActions) != 5 || result.DependenciesChanged || runner.getCalls != 0 { + t.Fatalf("unexpected directory migration: %#v", result) + } + + wantActions := []struct { + path, detail string + line int + }{ + {"a_mixed.fql", "average(...)", 2}, + {"a_mixed.fql", "date_diff(...)", 2}, + {"b_manual.fql", "join(...)", 1}, + {"c_broken.fql", "", 1}, + {"e_comments.fql", "format migrated Ferret source: formatter did not preserve comments", 1}, + } + for i, want := range wantActions { + got := result.ManualActions[i] + if got.Path != want.path || got.Line != want.line || (want.detail != "" && got.Detail != want.detail) || got.Reason == "" { + t.Fatalf("manual action %d = %#v, want %#v", i, got, want) + } + } + + for path, contents := range before { + if path == "a_mixed.fql" || path == "d_valid.fql" { + continue + } + + if got := readMigrationFixture(t, filepath.Join(root, path)); got != string(contents) { + t.Fatalf("unrelated, excluded, manual-only or malformed file %s changed", path) + } + } +} + +func TestMigratorPreservesManualActionsOnFormattingFailure(t *testing.T) { + for _, multiple := range []bool{false, true} { + for _, mode := range []Mode{ModeApply, ModeDryRun, ModePrint} { + t.Run(fmt.Sprintf("multiple=%t/mode=%d", multiple, mode), func(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, "query.fql") + before := "let value = average(items)\n" + if multiple { + before += "let comparison = date_compare(left, right, \"day\")\n" + } + + before += "return sha1 /* preserve me */ (value)" + writeMigrationTargetFile(t, root, "query.fql", before) + migrator, _ := newFixtureMigrator() + var previous []ManualAction + + for attempt := 0; attempt < 2; attempt++ { + result, err := migrator.Migrate(context.Background(), Options{Path: path, Mode: mode}) + if err != nil { + t.Fatal(err) + } + + if result.Applied || result.MigratedFQLFiles != 0 || len(result.Changes) != 0 || result.ScannedFQLFiles != 1 { + t.Fatalf("failed file produced source changes: %#v", result) + } + + if got := readMigrationFixture(t, path); got != before { + t.Fatalf("attempt %d modified rejected source: %q", attempt, got) + } + + wantCounts := map[string]int{ + "average(...)": 1, + "format migrated Ferret source: formatter did not preserve comments": 1, + } + if multiple { + wantCounts["date_compare(...)"] = 1 + } + + counts := make(map[string]int) + for _, action := range result.ManualActions { + counts[action.Detail]++ + line := 1 + if action.Detail == "date_compare(...)" { + line = 2 + } + + if action.Path != "query.fql" || action.Line != line || action.Reason == "" { + t.Fatalf("unexpected diagnostic location or reason: %#v", action) + } + } + + if !reflect.DeepEqual(counts, wantCounts) { + t.Fatalf("diagnostic counts = %#v, want %#v", counts, wantCounts) + } + + if attempt > 0 && !reflect.DeepEqual(result.ManualActions, previous) { + t.Fatalf("retry changed diagnostics: before=%#v after=%#v", previous, result.ManualActions) + } + + previous = result.ManualActions + } + }) + } + } +} + +func TestMigratorStdlibRollsBackOnCommitFailure(t *testing.T) { + root := t.TempDir() + writeMigrationTargetFile(t, root, "a.fql", "return abs(-1)") + writeMigrationTargetFile(t, root, "b.fql", "return sha1(body)") + before := snapshotMigrationFixture(t, root) + originalRename := renameMigrationFile + calls := 0 + renameMigrationFile = func(oldPath, newPath string) error { + calls++ + if calls == 2 { + return errors.New("stdlib commit failed") + } + + return os.Rename(oldPath, newPath) + } + t.Cleanup(func() { renameMigrationFile = originalRename }) + + migrator, _ := newFixtureMigrator() + _, err := migrator.Migrate(context.Background(), Options{Path: root}) + if err == nil || !strings.Contains(err.Error(), "stdlib commit failed") { + t.Fatalf("unexpected commit error: %v", err) + } + + if after := snapshotMigrationFixture(t, root); !reflect.DeepEqual(before, after) { + t.Fatalf("migration did not restore original files: %#v", after) + } +} + +func BenchmarkPlanFQLStdlibSourceChanges(b *testing.B) { + for _, canonical := range []bool{false, true} { + name := "legacy" + content := "let obj = json_parse(body)\nlet digest = sha1(json_stringify(obj))\nreturn [abs(-2), keys(obj), base(path), date_year(date(text))]" + if canonical { + name = "canonical" + content = "let obj = encoding::json_parse(body)\nlet digest = crypto::sha1(encoding::json_stringify(obj))\nreturn [math::abs(-2), object::keys(obj), path::base(path), datetime::year(datetime::parse(text))]" + } + + b.Run(name, func(b *testing.B) { + root := b.TempDir() + files := make([]string, 100) + for i := range files { + files[i] = filepath.Join(root, fmt.Sprintf("query_%03d.fql", i)) + if err := os.WriteFile(files[i], []byte(content), 0o644); err != nil { + b.Fatal(err) + } + } + + project := &migrationProject{Root: root, FQLFiles: files} + b.ReportAllocs() + b.ResetTimer() + + for range b.N { + result, err := planFQLSourceChanges(context.Background(), project) + if err != nil { + b.Fatal(err) + } + + want := len(files) + if canonical { + want = 0 + } + + if result.MigratedFiles != want { + b.Fatalf("migrated files = %d, want %d", result.MigratedFiles, want) + } + } + }) + } +} diff --git a/internal/migration/fql_stdlib_test.go b/internal/migration/fql_stdlib_test.go new file mode 100644 index 0000000..5001e38 --- /dev/null +++ b/internal/migration/fql_stdlib_test.go @@ -0,0 +1,467 @@ +package migration + +import ( + "context" + "strings" + "testing" + + ferret "github.com/MontFerret/ferret/v2" + "github.com/MontFerret/ferret/v2/pkg/source" + "github.com/MontFerret/ferret/v2/pkg/stdlib" +) + +func TestMigrateFQLStdlibMappings(t *testing.T) { + // These expectations are independent of the registry so omissions and wrong + // destinations cannot make the tests pass by changing both input and expectation. + tests := []struct{ legacy, canonical, args string }{ + {"json_parse", "encoding::json_parse", `"{}"`}, + {"json_stringify", "encoding::json_stringify", "obj"}, + {"encode_uri_component", "encoding::query_escape", "text"}, + {"decode_uri_component", "encoding::query_unescape", "text"}, + {"to_base64", "encoding::base64_encode", "text"}, + {"from_base64", "encoding::base64_decode", "text"}, + {"escape_html", "encoding::html_escape", "text"}, + {"unescape_html", "encoding::html_unescape", "text"}, + {"md5", "crypto::md5", "text"}, + {"sha1", "crypto::sha1", "text"}, + {"sha512", "crypto::sha512", "text"}, + {"random_token", "crypto::random_token", "16"}, + {"base", "path::base", "path"}, + {"clean", "path::clean", "path"}, + {"dir", "path::dir", "path"}, + {"ext", "path::ext", "path"}, + {"is_abs", "path::is_abs", "path"}, + {"separate", "path::separate", "path"}, + {"match", "path::match", "pattern, path"}, + {"values", "object::values", "obj"}, + {"has", "object::has_key", `obj, "key"`}, + {"zip", "object::zip", "names, vals"}, + {"keep_keys", "object::keep_keys", `obj, "a", "b"`}, + {"merge", "object::merge", "left, right"}, + {"merge_recursive", "object::merge_deep", "left, right"}, + {"keys", "object::keys", "obj"}, + {"now", "datetime::now", ""}, + {"date", "datetime::parse", `"2026-09-15"`}, + {"date", "datetime::parse", `text, "2006-01-02"`}, + {"date_dayofweek", "datetime::day_of_week", "dt"}, + {"date_year", "datetime::year", "dt"}, + {"date_month", "datetime::month", "dt"}, + {"date_day", "datetime::day", "dt"}, + {"date_hour", "datetime::hour", "dt"}, + {"date_minute", "datetime::minute", "dt"}, + {"date_second", "datetime::second", "dt"}, + {"date_millisecond", "datetime::millisecond", "dt"}, + {"date_dayofyear", "datetime::day_of_year", "dt"}, + {"date_leapyear", "datetime::is_leap_year", "dt"}, + {"date_quarter", "datetime::quarter", "dt"}, + {"date_days_in_month", "datetime::days_in_month", "dt"}, + {"date_format", "datetime::format", "dt, layout"}, + {"date_add", "datetime::add", `dt, 1, "day"`}, + {"date_subtract", "datetime::subtract", `dt, 1, "day"`}, + {"pi", "math::pi", ""}, + {"abs", "math::abs", "value"}, + {"acos", "math::acos", "value"}, + {"asin", "math::asin", "value"}, + {"atan", "math::atan", "value"}, + {"atan2", "math::atan2", "y, x"}, + {"ceil", "math::ceil", "value"}, + {"cos", "math::cos", "value"}, + {"degrees", "math::degrees", "value"}, + {"exp", "math::exp", "value"}, + {"exp2", "math::exp2", "value"}, + {"floor", "math::floor", "value"}, + {"log", "math::log", "value"}, + {"log2", "math::log2", "value"}, + {"log10", "math::log10", "value"}, + {"pow", "math::pow", "value, 2"}, + {"radians", "math::radians", "value"}, + {"round", "math::round", "value"}, + {"sin", "math::sin", "value"}, + {"sqrt", "math::sqrt", "value"}, + {"tan", "math::tan", "value"}, + } + + for _, test := range tests { + for _, spelling := range []string{test.legacy, strings.ToUpper(test.legacy), strings.ToUpper(test.legacy[:1]) + test.legacy[1:]} { + t.Run(spelling+"/"+test.args, func(t *testing.T) { + input := "return " + spelling + "(" + test.args + ")" + want := "return " + test.canonical + "(" + test.args + ")" + assertFQLStdlibMigration(t, input, want, 0) + }) + } + } +} + +func TestMigrateFQLStdlibStructuralEdits(t *testing.T) { + tests := []struct{ name, input, want string }{ + { + name: "nested and canonical calls", + input: `let obj = json_parse(body) +let digest = sha1(encoding::json_stringify(obj)) +return abs(date_year(date("2026-09-15")) - 2026)`, + want: `let obj = encoding::json_parse(body) +let digest = crypto::sha1(encoding::json_stringify(obj)) +return math::abs(datetime::year(datetime::parse("2026-09-15")) - 2026)`, + }, + { + name: "unicode comments and non-call identifiers", + input: `// json_parse("привет") +let json_parse = "sha1(abs(date_year()))" +/* merge_recursive and café */ +return { json_parse, abs: "λ", digest: sha1("привет") }`, + want: `// json_parse("привет") +let json_parse = "sha1(abs(date_year()))" +/* merge_recursive and café */ +return { + json_parse, + abs: "λ", + digest: crypto::sha1( + "привет" + ) +}`, + }, + { + name: "unbraced final loop header and body", + input: `let label = "日本語" +FOR item IN values(json_parse(body)) + RETURN abs(item)`, + want: `let label = "日本語" +return for item in object::values(encoding::json_parse(body)) { + return math::abs(item) +}`, + }, + { + name: "braced final loop", + input: "for item in values(obj) { return abs(item) }", + want: "return for item in object::values(obj) {\n return math::abs(item)\n}", + }, + { + name: "error operators", + input: "return [json_parse(body)?, abs(-2)]", + want: "return [encoding::json_parse(body)?, math::abs(-2)]", + }, + { + name: "eligible child of a qualified call", + input: "return custom::abs(abs(-2))", + want: "return custom::abs(math::abs(-2))", + }, + { + name: "calls inside functions", + input: `func calculate(x) { return abs(x) } +return calculate(-1)`, + want: `func calculate(x) { + return math::abs(x) +} +return calculate(-1)`, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + assertFQLStdlibMigration(t, test.input, test.want, 0) + }) + } +} + +func TestMigrateFQLStdlibDeferredCalls(t *testing.T) { + tests := []struct{ call, reason string }{ + {`join("a", "b")`, "legacy path::join"}, + {`join(["a", "b"], ",")`, "modern global string joining"}, + {"keys(obj, true)", "argument-aware"}, + {"keys(obj, false)", "argument-aware"}, + {"keys(obj, option)", "argument-aware"}, + {"keys()", "only keys(obj)"}, + {"keys(obj, true, extra)", "argument-aware"}, + {`date_compare(a, b, "year")`, "component-range"}, + {`date_compare(a, b, "year", "day")`, "not a drop-in"}, + {`date_diff(a, b, "day")`, "integer/floating"}, + {`date_diff(a, b, "day", true)`, "datetime::diff"}, + {"average(xs)", "heterogeneous"}, + {"sum(xs)", "heterogeneous"}, + {"min(xs)", "heterogeneous"}, + {"max(xs)", "heterogeneous"}, + {"median(xs)", "heterogeneous"}, + {"percentile(xs, 50)", "heterogeneous"}, + {"stddev_population(xs)", "heterogeneous"}, + {"stddev_sample(xs)", "heterogeneous"}, + {"variance_population(xs)", "heterogeneous"}, + {"variance_sample(xs)", "heterogeneous"}, + } + + for _, test := range tests { + t.Run(test.call, func(t *testing.T) { + input := "// original location\nRETURN " + test.call + result := assertFQLStdlibMigration(t, input, input, 1) + action := result.ManualActions[0] + if action.Path != "query.fql" || action.Line != 2 || !strings.Contains(action.Reason, test.reason) { + t.Fatalf("unexpected manual action: %#v", action) + } + }) + } +} + +func TestMigrateFQLStdlibMixedManualActions(t *testing.T) { + input := `let text = "λ" +FOR item IN values(obj) + RETURN [keys(item, true), average([abs(-2)]), average(item)]` + want := `let text = "λ" +return for item in object::values(obj) { + return [keys(item, true), average([math::abs(-2)]), average(item)] +}` + result := assertFQLStdlibMigration(t, input, want, 3) + for _, action := range result.ManualActions { + if action.Line != 3 || action.Path != "query.fql" { + t.Fatalf("manual action lost its original source location: %#v", action) + } + } +} + +func TestMigrateFQLStdlibLeavesUnrelatedSourceUnchanged(t *testing.T) { + inputs := []string{ + `RETURN [encoding::json_parse(body), crypto::sha1(text), path::base(path), object::merge(a, b), datetime::year(dt), math::sqrt(x)]`, + `RETURN [ENCODING::JSON_PARSE(body), custom::ABS(x), path::join("a", "b"), object::keys(obj)]`, + `RETURN [union(a, b), union_distinct(a, b), nth(a, 0), minus(a, b), push(a, 1), pop(a), shift(a), unshift(a, 1), position(a, 1), remove_nth(a, 0), outersection(a, b)]`, + `RETURN [rand(), rand(10, 1), range(1, 10), random::float(), arrays::at(xs, 0)]`, + `let json_parse = "abs(1)" // json_parse(body) +RETURN { json_parse, abs: "date_diff()" }`, + } + + for _, input := range inputs { + assertFQLStdlibMigration(t, input, input, 0) + } +} + +func TestMigrateFQLStdlibNameCollisions(t *testing.T) { + tests := []struct{ name, input, want, reason string }{ + { + name: "local declaration", + input: `func abs(x) { return x } +return abs(-1)`, + reason: "file-local", + }, + { + name: "forward declaration and safe sibling", + input: `let x = abs(-1) +func abs(x) { return x } +return sha1(x)`, + want: `let x = abs(-1) +func abs(x) { + return x +} +return crypto::sha1(x)`, + reason: "file-local", + }, + { + name: "nested declaration conservatively guards whole file", + input: `func outer() { func abs(x) { return x } return 1 } +return abs(-1)`, + reason: "file-local", + }, + { + name: "case insensitive guard", + input: `func ABS(x) { return x } +return Abs(-1)`, + reason: "file-local", + }, + { + name: "explicit function alias", + input: `use custom::calculate as abs +return ABS(-1)`, + reason: "function alias", + }, + { + name: "destination namespace alias", + input: `use custom as math +return abs(-1)`, + reason: "redirect the replacement math::abs", + }, + { + name: "function alias redirects destination namespace", + input: `use custom::calculate as math +return abs(-1)`, + reason: "redirect the replacement math::abs", + }, + { + name: "conflicting aliases cannot hide redirection", + input: `use custom as math +use math as math +return abs(-1)`, + reason: "redirect", + }, + { + name: "collision precedes semantic deferral", + input: `func average(xs) { return xs } +return average(xs)`, + reason: "file-local", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + want := test.want + if want == "" { + want = test.input + } + + result := assertFQLStdlibMigration(t, test.input, want, 1) + if !strings.Contains(result.ManualActions[0].Reason, test.reason) { + t.Fatalf("unexpected collision explanation: %#v", result.ManualActions) + } + }) + } +} + +func TestMigrateFQLStdlibUnrelatedAliases(t *testing.T) { + for _, head := range []string{ + "use custom as other", + "use custom as abs", + "use custom::calculate as other", + "use math as math", + "use custom as MATH", // Alias resolution is case-sensitive; generated math::abs is unaffected. + } { + assertFQLStdlibMigration(t, head+"\nreturn abs(-1)", head+"\nreturn math::abs(-1)", 0) + } + + input := "use custom as math\nRETURN math::abs(-1)" + assertFQLStdlibMigration(t, input, input, 0) +} + +func TestFQLStdlibEditsPreserveArgumentSource(t *testing.T) { + input := `return SHA1 /* call comment */ ( + JSON_STRINGIFY({ abs: "json_parse()", value: 1.00, text: "日本語" }) // argument comment +)?` + want := `return crypto::sha1 /* call comment */ ( + encoding::json_stringify({ abs: "json_parse()", value: 1.00, text: "日本語" }) // argument comment +)?` + src := source.New("query.fql", input) + program, err := parseFQLSource(src) + if err != nil { + t.Fatal(err) + } + + edits, actions, err := planFQLStdlib(src, program) + if err != nil { + t.Fatal(err) + } + + got, err := applyFQLEdits(input, edits) + if err != nil { + t.Fatal(err) + } + + if got != want || len(actions) != 0 { + t.Fatalf("stdlib edits changed argument source:\n%s\nmanual=%#v", got, actions) + } + + // The current formatter drops the comment between the call name and '('. + // Until core preserves it, the complete migration must fail without output. + result, err := migrateFQLSource(src) + if err == nil || !strings.Contains(err.Error(), "did not preserve comments") || result.Data != nil || result.Changed { + t.Fatalf("unsafe formatting was accepted: result=%#v err=%v", result, err) + } +} + +func TestMigrateFQLStdlibCanonicalCallsExecute(t *testing.T) { + input := `return [json_parse("[1]"), sha1("abc"), base("/tmp/file"), has({ a: 1 }, "a"), date_year(date("2026-09-15T00:00:00Z")), abs(-3)]` + result, err := migrateFQLSource(source.New("query.fql", input)) + if err != nil { + t.Fatal(err) + } + + engine, err := ferret.New(ferret.WithStdlib(stdlib.Full())) + if err != nil { + t.Fatal(err) + } + + t.Cleanup(func() { + if err := engine.Close(); err != nil { + t.Errorf("close engine: %v", err) + } + }) + + output, err := engine.Run(context.Background(), source.New("query.fql", string(result.Data))) + if err != nil { + t.Fatal(err) + } + + want := `[[1],"a9993e364706816aba3e25717850c26c9cd0d89d","file",true,2026,3]` + if string(output.Content) != want { + t.Fatalf("migrated behavior = %s, want %s", output.Content, want) + } +} + +func TestMigrateFQLSourcePreservesManualActionsOnFormattingFailure(t *testing.T) { + tests := []struct { + name string + input string + calls []string + }{ + { + name: "one deferred call", + input: "let value = average(items)\nreturn sha1 /* preserve me */ (value)", + calls: []string{"average(...)"}, + }, + { + name: "multiple deferred calls", + input: "let a = average(items)\nlet b = date_compare(left, right, \"day\")\n" + + "return sha1 /* preserve me */ (a)", + calls: []string{"average(...)", "date_compare(...)"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result, err := migrateFQLSource(source.New("query.fql", test.input)) + if err == nil || !strings.Contains(err.Error(), "formatter did not preserve comments") { + t.Fatalf("expected comment-preservation failure, got %v", err) + } + + if result.Data != nil || result.Changed { + t.Fatalf("failed migration returned transformed source: %#v", result) + } + + if len(result.ManualActions) != len(test.calls) { + t.Fatalf("manual actions = %#v, want %v", result.ManualActions, test.calls) + } + + for i, call := range test.calls { + action := result.ManualActions[i] + if action.Path != "query.fql" || action.Line != i+1 || action.Detail != call || action.Reason == "" { + t.Fatalf("manual action %d lost its original details: %#v", i, action) + } + } + }) + } +} + +func assertFQLStdlibMigration(t *testing.T, input, want string, manualCount int) fqlMigrationResult { + t.Helper() + + result, err := migrateFQLSource(source.New("query.fql", input)) + if err != nil { + t.Fatal(err) + } + + if len(result.ManualActions) != manualCount { + t.Fatalf("manual actions = %#v, want %d", result.ManualActions, manualCount) + } + + if input == want { + if result.Changed || result.Data != nil { + t.Fatalf("unchanged source was rewritten: %s", result.Data) + } + } else if !result.Changed || string(result.Data) != want { + t.Fatalf("unexpected migration:\nwant:\n%s\ngot:\n%s", want, result.Data) + } + + second, err := migrateFQLSource(source.New("query.fql", want)) + if err != nil { + t.Fatal(err) + } + + if second.Changed || second.Data != nil || len(second.ManualActions) != manualCount { + t.Fatalf("second pass was not idempotent: %#v", second) + } + + return result +} diff --git a/internal/migration/types.go b/internal/migration/types.go index 22e226d..cc049f7 100644 --- a/internal/migration/types.go +++ b/internal/migration/types.go @@ -138,9 +138,12 @@ type ( MigratedFiles int } + // fqlMigrationResult retains original-source manual actions on late errors. + // Data and Changed are populated only after all rewrite validation succeeds. fqlMigrationResult struct { - Data []byte - Changed bool + Data []byte + ManualActions []ManualAction + Changed bool } migrationFiles struct {