From 3751414aeba966e0b94218fcf23761fb5fb9ec2e Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 26 Sep 2026 21:00:06 +0000 Subject: [PATCH 1/2] mdl: deprecation registry for MDL aliases (MDL-DEPRnnn) One registry of deprecated MDL spellings (ADR-0011 decision 1), generalising the MDL065 pattern: both spellings build the same AST, so the visitor records which one the source used on ast.Program.Deprecations, and check/exec warn through ValidateProgram. --deprecations=error fails the run instead. Grammar aliases are marked with /* @alias MDL-DEPRnnn */ next to the token; TestGrammarAliasesAreRegistered fails on a marker with no registry entry, or an entry no marker names. Seeded with create or replace -> create or modify (MDL-DEPR001) and show -> list (MDL-DEPR002). create or replace is only reported where it builds the same statements as create or modify; view entity, translations, user role and demo user differ today and are exempt, pinned per create kind. Closes #709 Co-Authored-By: Claude Opus 5.5 --- cmd/mxcli/cmd_check.go | 7 + cmd/mxcli/cmd_exec.go | 6 +- cmd/mxcli/deprecations_flag.go | 36 +++ docs-site/src/appendixes/error-messages.md | 15 ++ mdl/ast/ast.go | 18 ++ mdl/deprecation/deprecation.go | 153 ++++++++++++ mdl/executor/validate_deprecations.go | 56 +++++ mdl/executor/validate_deprecations_test.go | 81 +++++++ mdl/executor/validate_program.go | 5 + mdl/grammar/MDLParser.g4 | 2 +- mdl/grammar/alias_registry_test.go | 131 ++++++++++ mdl/grammar/domains/MDLCatalog.g4 | 2 +- mdl/visitor/visitor.go | 4 + mdl/visitor/visitor_deprecations.go | 71 ++++++ mdl/visitor/visitor_deprecations_test.go | 249 ++++++++++++++++++++ mdl/visitor/visitor_document_annotations.go | 1 + 16 files changed, 834 insertions(+), 3 deletions(-) create mode 100644 cmd/mxcli/deprecations_flag.go create mode 100644 mdl/deprecation/deprecation.go create mode 100644 mdl/executor/validate_deprecations.go create mode 100644 mdl/executor/validate_deprecations_test.go create mode 100644 mdl/grammar/alias_registry_test.go create mode 100644 mdl/visitor/visitor_deprecations.go create mode 100644 mdl/visitor/visitor_deprecations_test.go diff --git a/cmd/mxcli/cmd_check.go b/cmd/mxcli/cmd_check.go index 59cea5afc..807a52de7 100644 --- a/cmd/mxcli/cmd_check.go +++ b/cmd/mxcli/cmd_check.go @@ -65,6 +65,11 @@ run. Output includes structured rule IDs (MDL prefix for reference and script rules, E0xx for expression type rules) for each validation issue. +A deprecated MDL spelling — an alias left over from consolidating MDL onto one +canonical form, such as "create or replace" for "create or modify" or "show" for +"list" — is reported as an MDL-DEPRnnn warning naming the canonical form. Pass +--deprecations=error to fail the run on one instead, e.g. in CI over docs. + Use --post-migration to scan an existing project (independent of the script) for legacy native widgets that have pluggable replacements — Studio Pro does not auto-migrate these on a Mendix major-version upgrade. @@ -99,6 +104,7 @@ Examples: checkRefs, _ := cmd.Flags().GetBool("references") checkRefs = checkRefs || projectPath != "" postMigration, _ := cmd.Flags().GetBool("post-migration") + depPolicy := deprecationPolicy(cmd) format := resolveFormat(cmd, "text") isStructured := format != "" && format != "text" @@ -194,6 +200,7 @@ Examples: // refuses exactly what `mxcli check` reports. Adding a check there gives // both commands it at once. violations := append(testProblems, executor.ValidateProgram(prog, projectPath)...) + violations = executor.ApplyDeprecationPolicy(violations, depPolicy) if isStructured { // Always emit structured output (even when clean) diff --git a/cmd/mxcli/cmd_exec.go b/cmd/mxcli/cmd_exec.go index 317a57ba7..cea6849f4 100644 --- a/cmd/mxcli/cmd_exec.go +++ b/cmd/mxcli/cmd_exec.go @@ -25,6 +25,9 @@ applies statements one at a time and cannot roll back, so running a script with a known error leaves the model partly updated. Warnings are printed and do not stop the run. Use --no-check to apply a script anyway. +A deprecated MDL spelling (MDL-DEPRnnn, e.g. "create or replace" for "create or +modify") is a warning; --deprecations=error makes it an error. + By default execution stops at the first error. With --continue-on-error, every statement is attempted; each failure is reported (prefixed with its statement number) and execution continues, exiting non-zero if any statement failed. This @@ -50,6 +53,7 @@ Example: projectPath, _ := cmd.Flags().GetString("project") continueOnError, _ := cmd.Flags().GetBool("continue-on-error") skipCheck, _ := cmd.Flags().GetBool("no-check") + depPolicy := deprecationPolicy(cmd) // Read the script (a path, or "-" for stdin) content, err := readMDLSource(filePath) @@ -103,7 +107,7 @@ Example: // exec is not transactional, so "run it and see" means a half-applied // model. Warnings are printed and do not stop the run. if !skipCheck { - violations := executor.ValidateProgram(prog, projectPath) + violations := executor.ApplyDeprecationPolicy(executor.ValidateProgram(prog, projectPath), depPolicy) if len(violations) > 0 { formatter := linter.GetFormatter(linter.OutputFormatText, true) formatter.Format(violations, os.Stderr) diff --git a/cmd/mxcli/deprecations_flag.go b/cmd/mxcli/deprecations_flag.go new file mode 100644 index 000000000..db892c292 --- /dev/null +++ b/cmd/mxcli/deprecations_flag.go @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "fmt" + "os" + + "github.com/mendixlabs/mxcli/mdl/deprecation" + "github.com/spf13/cobra" +) + +// The --deprecations flag decides what `check` and `exec` do with a deprecated +// MDL spelling (an MDL-DEPRnnn warning, registry in mdl/deprecation). The +// default warns; `error` fails the run, so CI over docs, skills and examples +// can hold them to the canonical form. +const deprecationsFlagUsage = "What to do with a deprecated MDL spelling (MDL-DEPRnnn): " + + "warn, or error to fail the run (for CI over docs, skills and examples)" + +func init() { + checkCmd.Flags().String("deprecations", "warn", deprecationsFlagUsage) + execCmd.Flags().String("deprecations", "warn", deprecationsFlagUsage) +} + +// deprecationPolicy reads --deprecations, exiting with a usage error on a value +// it does not know: silently treating a typo as `warn` would let a CI gate pass +// that was meant to fail. +func deprecationPolicy(cmd *cobra.Command) deprecation.Policy { + raw, _ := cmd.Flags().GetString("deprecations") + policy, err := deprecation.ParsePolicy(raw) + if err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + os.Exit(2) + } + return policy +} diff --git a/docs-site/src/appendixes/error-messages.md b/docs-site/src/appendixes/error-messages.md index 6244249eb..b61ef63b7 100644 --- a/docs-site/src/appendixes/error-messages.md +++ b/docs-site/src/appendixes/error-messages.md @@ -142,6 +142,21 @@ iterator name exists, so mxbuild rejects this with CE0109 "Undefined variable The rule keys on **scope, not on the name**. `$item` is perfectly valid in a predicate when it is the enclosing loop's iterator, which is how the O(N) lookup idiom is written — inside `loop $item in $L`, `find($Others, Key = $item/Key)` navigates the loop's variable and is not flagged. +### MDL-DEPRnnn: Deprecated spelling + +``` +line 1: `create or replace …` (enumeration) is deprecated; write `create or modify …` +— same meaning. Refused from `mdl 2`. [MDL-DEPR001] +``` + +**Cause:** The script uses an alias that MDL is consolidating away (ADR-0010). The alias means exactly what the canonical form means, so the statement runs unchanged. The warning names the canonical form, and the language version whose header (`mdl ;`) will refuse the alias (ADR-0011). + +**Solution:** Write the canonical form the warning names. The rewrite is a mechanical keyword swap, and the suggestion line says which one. + +`check` and `exec` report these as **warnings**. To fail the run on one, for example in CI over documentation and examples, pass `--deprecations=error`. + +The registry of deprecated spellings is `mdl/deprecation/deprecation.go`. It holds the code, old form, canonical form, rewrite and removal version of each entry. A spelling is only registered where it means exactly the same as its canonical form. `create or replace view entity`, for example, drops and recreates the view entity, so it is not reported. + ## mxcli Parser Errors diff --git a/mdl/ast/ast.go b/mdl/ast/ast.go index 5c4aa3257..c01ed07c9 100644 --- a/mdl/ast/ast.go +++ b/mdl/ast/ast.go @@ -53,6 +53,24 @@ type Program struct { // next to knownActivityAnnotations, instead of being spread across the seven // visitor sites that read them. DocumentAnnotations []DocumentAnnotation + // Deprecations records every use of a deprecated spelling registered in + // mdl/deprecation, in source order. Both spellings build the same + // statements, so this is the only trace of which one the source used; it + // drives the MDL-DEPRnnn warnings and nothing else may branch on it. + Deprecations []DeprecatedSpelling +} + +// DeprecatedSpelling is one use of a deprecated spelling in the source. +type DeprecatedSpelling struct { + // Code is the registry code, MDL-DEPRnnn. + Code string + // Line and Column locate the deprecated token (1-based line, 0-based + // column, as ANTLR reports them). + Line int + Column int + // Subject says what the spelling was used on, in MDL's own words ("entity", + // "microflow", …); empty when there is nothing more specific to say. + Subject string } // DocumentAnnotation is one annotation written before a CREATE statement. diff --git a/mdl/deprecation/deprecation.go b/mdl/deprecation/deprecation.go new file mode 100644 index 000000000..3a2c67405 --- /dev/null +++ b/mdl/deprecation/deprecation.go @@ -0,0 +1,153 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Package deprecation is the single registry of deprecated MDL spellings +// (ADR-0011, decision 1). +// +// A deprecated spelling is a respelling: it means exactly what its canonical +// form means, so it keeps parsing, `check` and `exec` warn on it, and +// `mxcli fmt --upgrade` can rewrite it mechanically. A spelling that means +// something different from the proposed canonical form is NOT an alias there, +// and is not reported — rewriting it would silently change a script. +// +// # How an alias is marked +// +// Every grammar token or alternative that exists only as an alias carries a +// block comment naming its registry code, next to the alias itself: +// +// CREATE (OR (MODIFY | REPLACE /* @alias MDL-DEPR001 */))? +// +// ANTLR ignores the comment. TestGrammarAliasesAreRegistered (mdl/grammar) +// reads the .g4 sources and fails when a marker names a code with no entry +// here, or an entry is named by no marker. The marker is what makes a missing +// entry detectable: an alias is marked in the edit that adds it, and the marker +// cannot be satisfied without an entry. +// +// # How a use is detected +// +// Both spellings build the same AST, so the parse tree is the only place the +// source spelling is still visible. The visitor records every use of a +// registered spelling on ast.Program.Deprecations, and the executor's +// ValidateDeprecations turns the records into warnings (errors under +// --deprecations=error). This generalises MDL065, where the AST node itself +// carries the spelling flags. +package deprecation + +import ( + "fmt" + "strings" +) + +// Entry is one deprecated spelling. +type Entry struct { + // Code is the stable warning code, MDL-DEPRnnn. Never reused. + Code string + // Old is the deprecated form, as a reader would write it. + Old string + // Canonical is the form that replaces it (ADR-0010). + Canonical string + // Rewrite is the mechanical rewrite `fmt --upgrade` applies. + Rewrite Rewrite + // RemovedIn is the MDL language version (the `mdl ;` header) under which + // the old form is refused. Under earlier versions it warns (ADR-0011). + RemovedIn int + // Note is shown with the warning: scope limits, or where the canonical form + // is expected to move next. + Note string + // Example is a complete statement in the old form. A test parses it and + // requires exactly this code to be recorded. + Example string + // CanonicalExample is Example after Rewrite. A test requires it to record no + // deprecation and to build the same AST as Example — the proof that the + // rewrite does not change meaning. + CanonicalExample string +} + +// Rewrite replaces one keyword token of the deprecated form with another. It is +// the only shape the seeded entries need; a richer one is added with the first +// entry that needs it. +type Rewrite struct { + // Token is the keyword to replace, lower-case. + Token string + // Replacement is the keyword written in its place, lower-case. + Replacement string +} + +// Codes of the registered entries, for the visitor to record. +const ( + CreateOrReplace = "MDL-DEPR001" + Show = "MDL-DEPR002" +) + +// entries is the registry. Append only: a code is never reused or renumbered, +// because scripts, CI allowlists and docs refer to it. +var entries = []Entry{ + { + Code: CreateOrReplace, + Old: "create or replace …", + Canonical: "create or modify …", + Rewrite: Rewrite{Token: "replace", Replacement: "modify"}, + RemovedIn: 2, + Note: "Not reported where `or replace` means something else today: " + + "`create or replace view entity` (drops and recreates), " + + "`create or replace translations` (replaces the whole set), and " + + "`create or replace user role` / `demo user` (the `replace` is ignored).", + Example: "create or replace enumeration M.Color (Red 'Red');", + CanonicalExample: "create or modify enumeration M.Color (Red 'Red');", + }, + { + Code: Show, + Old: "show …", + Canonical: "list …", + Rewrite: Rewrite{Token: "show", Replacement: "list"}, + RemovedIn: 2, + Note: "`list` is the same statement today. Forms that name a single thing " + + "(`show entity X`, `show navigation`, …) are expected to move on to " + + "`describe`, and session state (`show version`, `show status`) to REPL " + + "commands, in a later consolidation.", + Example: "show entities in M;", + CanonicalExample: "list entities in M;", + }, +} + +// All returns every registered entry, in code order. +func All() []Entry { + out := make([]Entry, len(entries)) + copy(out, entries) + return out +} + +// Lookup returns the entry for code. +func Lookup(code string) (Entry, bool) { + for _, e := range entries { + if e.Code == code { + return e, true + } + } + return Entry{}, false +} + +// IsDeprecationCode reports whether a violation rule ID is a registry code. +func IsDeprecationCode(ruleID string) bool { + return strings.HasPrefix(ruleID, "MDL-DEPR") +} + +// Policy is what `check` and `exec` do with a deprecated spelling. +type Policy int + +const ( + // Warn reports it and carries on. The default. + Warn Policy = iota + // Error fails the run, for CI over docs, skills and examples. + Error +) + +// ParsePolicy reads the --deprecations flag value. +func ParsePolicy(s string) (Policy, error) { + switch strings.ToLower(strings.TrimSpace(s)) { + case "", "warn": + return Warn, nil + case "error": + return Error, nil + } + return Warn, fmt.Errorf("invalid --deprecations value %q: want warn or error", s) +} diff --git a/mdl/executor/validate_deprecations.go b/mdl/executor/validate_deprecations.go new file mode 100644 index 000000000..cd2723846 --- /dev/null +++ b/mdl/executor/validate_deprecations.go @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/deprecation" + "github.com/mendixlabs/mxcli/mdl/linter" +) + +// ValidateDeprecations reports every use of a deprecated spelling the visitor +// recorded, one warning per use, under the entry's MDL-DEPRnnn code. +// +// A warning, never an error by itself: a deprecated spelling means exactly +// what its canonical form means, and scripts in the wild use it. `check` and +// `exec` promote these to errors only under --deprecations=error (see +// ApplyDeprecationPolicy), which is for CI over docs, skills and examples. +func ValidateDeprecations(prog *ast.Program) []linter.Violation { + var out []linter.Violation + for _, d := range prog.Deprecations { + e, ok := deprecation.Lookup(d.Code) + if !ok { + continue // the visitor records only registered codes; pinned by tests + } + on := "" + if d.Subject != "" { + on = " (" + d.Subject + ")" + } + out = append(out, linter.Violation{ + RuleID: e.Code, + Severity: linter.SeverityWarning, + Message: fmt.Sprintf("line %d: `%s`%s is deprecated; write `%s` — same meaning. "+ + "Refused from `mdl %d`.", d.Line, e.Old, on, e.Canonical, e.RemovedIn), + Suggestion: fmt.Sprintf("Replace `%s` with `%s`. %s", e.Rewrite.Token, e.Rewrite.Replacement, e.Note), + }) + } + return out +} + +// ApplyDeprecationPolicy returns violations with every MDL-DEPRnnn warning +// promoted to an error under deprecation.Error, and unchanged otherwise. +func ApplyDeprecationPolicy(violations []linter.Violation, policy deprecation.Policy) []linter.Violation { + if policy != deprecation.Error { + return violations + } + out := make([]linter.Violation, len(violations)) + for i, v := range violations { + if deprecation.IsDeprecationCode(v.RuleID) { + v.Severity = linter.SeverityError + } + out[i] = v + } + return out +} diff --git a/mdl/executor/validate_deprecations_test.go b/mdl/executor/validate_deprecations_test.go new file mode 100644 index 000000000..0a3244021 --- /dev/null +++ b/mdl/executor/validate_deprecations_test.go @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/deprecation" + "github.com/mendixlabs/mxcli/mdl/linter" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +func deprecationViolations(t *testing.T, src string, policy deprecation.Policy) []linter.Violation { + t.Helper() + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + var out []linter.Violation + for _, v := range ApplyDeprecationPolicy(ValidateProgram(prog, ""), policy) { + if deprecation.IsDeprecationCode(v.RuleID) { + out = append(out, v) + } + } + return out +} + +// `check` and `exec` share ValidateProgram, so a deprecated spelling warns in +// both. The canonical script is the control: it must stay silent, or the +// warning would fire on everything. +func TestDeprecatedSpellingsWarnThroughValidateProgram(t *testing.T) { + old := "create or replace enumeration M.Color (Red 'Red');\nshow entities in M;\n" + canon := "create or modify enumeration M.Color (Red 'Red');\nlist entities in M;\n" + + got := deprecationViolations(t, old, deprecation.Warn) + if len(got) != 2 || got[0].RuleID != deprecation.CreateOrReplace || got[1].RuleID != deprecation.Show { + t.Fatalf("got %+v, want MDL-DEPR001 then MDL-DEPR002", got) + } + for _, v := range got { + if v.Severity != linter.SeverityWarning { + t.Errorf("%s severity = %v, want warning — an error would refuse working scripts", v.RuleID, v.Severity) + } + } + if !strings.Contains(got[0].Message, "line 1") || !strings.Contains(got[0].Message, "create or modify") { + t.Errorf("message must name the line and the canonical form: %q", got[0].Message) + } + if !strings.Contains(got[1].Message, "line 2") || !strings.Contains(got[1].Message, "list") { + t.Errorf("message must name the line and the canonical form: %q", got[1].Message) + } + if summary := linter.Summarize(ValidateProgram(mustProgram(t, old), "")); summary.Errors > 0 { + t.Errorf("deprecated spellings produced %d error(s) by default; exec would refuse the script", summary.Errors) + } + + if got := deprecationViolations(t, canon, deprecation.Warn); len(got) != 0 { + t.Errorf("canonical script reported %+v", got) + } +} + +func TestDeprecationPolicyErrorFailsTheRun(t *testing.T) { + src := "show modules;" + got := deprecationViolations(t, src, deprecation.Error) + if len(got) != 1 || got[0].Severity != linter.SeverityError { + t.Fatalf("under --deprecations=error got %+v, want one error", got) + } + // Only registry codes are promoted: another warning stays a warning. + other := []linter.Violation{{RuleID: "MDL065", Severity: linter.SeverityWarning}} + if v := ApplyDeprecationPolicy(other, deprecation.Error); v[0].Severity != linter.SeverityWarning { + t.Errorf("MDL065 promoted to %v; only MDL-DEPRnnn codes may be", v[0].Severity) + } +} + +func mustProgram(t *testing.T, src string) *ast.Program { + t.Helper() + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + return prog +} diff --git a/mdl/executor/validate_program.go b/mdl/executor/validate_program.go index 3c9871861..f290021c1 100644 --- a/mdl/executor/validate_program.go +++ b/mdl/executor/validate_program.go @@ -290,5 +290,10 @@ func ValidateProgram(prog *ast.Program, projectPath string) []linter.Violation { // parsed and did nothing (MDL059, the same rule statements already have). violations = append(violations, ValidateDocumentAnnotations(prog)...) + // Warn on every deprecated spelling (MDL-DEPRnnn): an alias left over from + // consolidating MDL onto one canonical form (ADR-0010/0011). The registry + // is mdl/deprecation. + violations = append(violations, ValidateDeprecations(prog)...) + return violations } diff --git a/mdl/grammar/MDLParser.g4 b/mdl/grammar/MDLParser.g4 index fc2f79d57..96a884caf 100644 --- a/mdl/grammar/MDLParser.g4 +++ b/mdl/grammar/MDLParser.g4 @@ -89,7 +89,7 @@ updateWidgetsStatement createStatement : docComment? annotation* - CREATE (OR (MODIFY | REPLACE))? + CREATE (OR (MODIFY | REPLACE /* @alias MDL-DEPR001 */))? ( createEntityStatement | createAssociationStatement | createModuleStatement diff --git a/mdl/grammar/alias_registry_test.go b/mdl/grammar/alias_registry_test.go new file mode 100644 index 000000000..1a240bf5e --- /dev/null +++ b/mdl/grammar/alias_registry_test.go @@ -0,0 +1,131 @@ +// SPDX-License-Identifier: Apache-2.0 + +package grammar + +import ( + "fmt" + "os" + "path/filepath" + "regexp" + "sort" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/deprecation" +) + +// aliasMarker is how a grammar token or alternative is marked as a deprecated +// alias: a block comment naming its registry code, e.g. +// +// CREATE (OR (MODIFY | REPLACE /* @alias MDL-DEPR001 */))? +// +// ANTLR drops the comment; this test is its only reader. +var aliasMarker = regexp.MustCompile(`@alias\b\s*(\S*)`) + +var deprCode = regexp.MustCompile(`^MDL-DEPR\d{3}$`) + +// checkAliasMarkers returns one problem per marker that is malformed or names a +// code with no registry entry, and per registry entry that no marker names. +// sources maps a file name to its grammar text. +func checkAliasMarkers(sources map[string]string, entries []deprecation.Entry) []string { + registered := map[string]bool{} + for _, e := range entries { + registered[e.Code] = true + } + marked := map[string]bool{} + var problems []string + + names := make([]string, 0, len(sources)) + for n := range sources { + names = append(names, n) + } + sort.Strings(names) + for _, name := range names { + for i, line := range strings.Split(sources[name], "\n") { + for _, m := range aliasMarker.FindAllStringSubmatch(line, -1) { + code := strings.TrimSuffix(m[1], "*/") + switch { + case !deprCode.MatchString(code): + problems = append(problems, fmt.Sprintf( + "%s:%d: malformed alias marker %q — write /* @alias MDL-DEPRnnn */", name, i+1, m[0])) + case !registered[code]: + problems = append(problems, fmt.Sprintf( + "%s:%d: grammar alias %s has no entry in mdl/deprecation — add one "+ + "(old form, canonical form, rewrite, removed-in version, examples)", name, i+1, code)) + default: + marked[code] = true + } + } + } + } + for _, e := range entries { + if !marked[e.Code] { + problems = append(problems, fmt.Sprintf( + "registry entry %s (%s) is named by no /* @alias %s */ marker in the grammar — "+ + "mark the alias where it is parsed", e.Code, e.Old, e.Code)) + } + } + return problems +} + +func readGrammarSources(t *testing.T) map[string]string { + t.Helper() + files, err := filepath.Glob("*.g4") + if err != nil { + t.Fatal(err) + } + domain, err := filepath.Glob("domains/*.g4") + if err != nil { + t.Fatal(err) + } + files = append(files, domain...) + if len(files) == 0 { + t.Fatal("no .g4 files found") + } + sources := map[string]string{} + for _, f := range files { + b, err := os.ReadFile(f) + if err != nil { + t.Fatal(err) + } + sources[f] = string(b) + } + return sources +} + +// An alias in the grammar must have a registry entry, so that it warns and +// `fmt --upgrade` can rewrite it (ADR-0011, decision 1) — and an entry must +// point at a real alias. +func TestGrammarAliasesAreRegistered(t *testing.T) { + for _, p := range checkAliasMarkers(readGrammarSources(t), deprecation.All()) { + t.Error(p) + } +} + +// The control: the checker does fail on an alias with no entry, on an entry +// with no alias, and on a malformed marker. Without this, a checker that found +// nothing would pass the test above against any grammar. +func TestAliasMarkerCheckerDetectsProblems(t *testing.T) { + entries := []deprecation.Entry{{Code: "MDL-DEPR001", Old: "a"}, {Code: "MDL-DEPR002", Old: "b"}} + grammar := map[string]string{"X.g4": strings.Join([]string{ + "r1 : A /* @alias MDL-DEPR001 */ | B ;", + "r2 : C /* @alias MDL-DEPR999 */ | D ;", + "r3 : E /* @alias DEPR3 */ ;", + }, "\n")} + problems := checkAliasMarkers(grammar, entries) + want := []string{"MDL-DEPR999 has no entry", "malformed alias marker", "MDL-DEPR002 (b) is named by no"} + for _, w := range want { + found := false + for _, p := range problems { + if strings.Contains(p, w) { + found = true + } + } + if !found { + t.Errorf("checker did not report %q; got %q", w, problems) + } + } + if len(problems) != len(want) { + t.Errorf("got %d problems, want %d: %q", len(problems), len(want), problems) + } +} diff --git a/mdl/grammar/domains/MDLCatalog.g4 b/mdl/grammar/domains/MDLCatalog.g4 index d5e1a0bf4..97cc259b0 100644 --- a/mdl/grammar/domains/MDLCatalog.g4 +++ b/mdl/grammar/domains/MDLCatalog.g4 @@ -9,7 +9,7 @@ options { tokenVocab = MDLLexer; } // DQL STATEMENTS (Data Query Language) // ============================================================================= -showOrList: SHOW | LIST_KW ; +showOrList: SHOW /* @alias MDL-DEPR002 */ | LIST_KW ; showStatement : showOrList MODULES diff --git a/mdl/visitor/visitor.go b/mdl/visitor/visitor.go index f20b39c75..0d1253969 100644 --- a/mdl/visitor/visitor.go +++ b/mdl/visitor/visitor.go @@ -467,6 +467,9 @@ type Builder struct { // documentAnnotations collects every `@name` written before a CREATE, with // the kind of document it was on — see ExitCreateStatement. documentAnnotations []ast.DocumentAnnotation + // deprecations collects every use of a deprecated spelling — see + // visitor_deprecations.go. + deprecations []ast.DeprecatedSpelling // inLayout is set while a CREATE LAYOUT body is being built. The page body // builder serves both documents and cannot otherwise tell which it is in, @@ -537,6 +540,7 @@ func Build(input string) (*ast.Program, []error) { return &ast.Program{ Statements: builder.statements, DocumentAnnotations: builder.documentAnnotations, + Deprecations: builder.deprecations, }, allErrors } diff --git a/mdl/visitor/visitor_deprecations.go b/mdl/visitor/visitor_deprecations.go new file mode 100644 index 000000000..db81f1662 --- /dev/null +++ b/mdl/visitor/visitor_deprecations.go @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/deprecation" + "github.com/mendixlabs/mxcli/mdl/grammar/parser" + + "github.com/antlr4-go/antlr/v4" +) + +// Deprecated spellings are recorded from the parse tree because it is the only +// place they are visible: `create or replace` and `create or modify`, `show` +// and `list`, build the same statements. The registry of spellings, and how a +// grammar alias is marked, is in mdl/deprecation. + +// recordDeprecation appends one use of a deprecated spelling at tok. +func (b *Builder) recordDeprecation(code string, tok antlr.Token, subject string) { + if tok == nil { + return + } + b.deprecations = append(b.deprecations, ast.DeprecatedSpelling{ + Code: code, + Line: tok.GetLine(), + Column: tok.GetColumn(), + Subject: subject, + }) +} + +// createOrReplaceIsNotAnAlias lists the create kinds (createStatementKind's +// words) where `or replace` does NOT mean `or modify`, so rewriting it would +// change the script. Measured against the visitors, and pinned by +// TestCreateOrReplaceMatchesModifyExceptExemptKinds: +// +// - translations: `or replace` replaces the whole set, `or modify` merges. +// - userrole, demouser: the visitor reads only `or modify`; `or replace` is +// a plain create, which fails on an existing role or user. +// +// `create or replace view entity` (kind "entity") drops and recreates the view +// entity, and is exempted in recordCreateOrReplace. +var createOrReplaceIsNotAnAlias = map[string]bool{ + "translations": true, + "userrole": true, + "demouser": true, +} + +// recordCreateOrReplace records MDL-DEPR001 for a `create or replace` whose +// meaning is exactly `create or modify`. +func (b *Builder) recordCreateOrReplace(ctx *parser.CreateStatementContext) { + if ctx.OR() == nil || ctx.REPLACE() == nil { + return + } + kind := createStatementKind(ctx) + if kind == "" || createOrReplaceIsNotAnAlias[kind] { + return + } + if ent, ok := ctx.CreateEntityStatement().(*parser.CreateEntityStatementContext); ok && ent.VIEW() != nil { + return + } + b.recordDeprecation(deprecation.CreateOrReplace, ctx.REPLACE().GetSymbol(), kind) +} + +// ExitShowOrList records MDL-DEPR002 for `show`. showOrList is used only by +// showStatement, where `show` and `list` build the same statement. +func (b *Builder) ExitShowOrList(ctx *parser.ShowOrListContext) { + if ctx == nil || ctx.SHOW() == nil { + return + } + b.recordDeprecation(deprecation.Show, ctx.SHOW().GetSymbol(), "") +} diff --git a/mdl/visitor/visitor_deprecations_test.go b/mdl/visitor/visitor_deprecations_test.go new file mode 100644 index 000000000..9725846dd --- /dev/null +++ b/mdl/visitor/visitor_deprecations_test.go @@ -0,0 +1,249 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "os" + "reflect" + "regexp" + "sort" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/deprecation" +) + +func deprecationCodes(prog *ast.Program) []string { + var out []string + for _, d := range prog.Deprecations { + out = append(out, d.Code) + } + return out +} + +func mustBuild(t *testing.T, src string) *ast.Program { + t.Helper() + prog, errs := Build(src) + if len(errs) > 0 { + t.Fatalf("parse %q: %v", src, errs) + } + return prog +} + +// Every registry entry's own example must be detected, and its canonical +// rewrite must be silent AND build the same statements. The last part is what +// makes the rewrite safe to apply mechanically (ADR-0011: a wrong rewrite is a +// silent change of meaning). +func TestRegistryExamplesRecordTheirCode(t *testing.T) { + for _, e := range deprecation.All() { + t.Run(e.Code, func(t *testing.T) { + old := mustBuild(t, e.Example) + if got := deprecationCodes(old); !reflect.DeepEqual(got, []string{e.Code}) { + t.Errorf("Example %q recorded %v, want [%s]", e.Example, got, e.Code) + } + canon := mustBuild(t, e.CanonicalExample) + if got := deprecationCodes(canon); len(got) != 0 { + t.Errorf("CanonicalExample %q recorded %v, want none", e.CanonicalExample, got) + } + if !reflect.DeepEqual(old.Statements, canon.Statements) { + t.Errorf("Example and CanonicalExample build different statements:\n old: %#v\n canon: %#v", + old.Statements, canon.Statements) + } + // The rewrite is a token swap; the canonical example must be exactly it. + re := regexp.MustCompile(`(?i)\b` + regexp.QuoteMeta(e.Rewrite.Token) + `\b`) + if got := re.ReplaceAllString(e.Example, e.Rewrite.Replacement); got != e.CanonicalExample { + t.Errorf("Rewrite %s -> %s gives %q, CanonicalExample is %q", + e.Rewrite.Token, e.Rewrite.Replacement, got, e.CanonicalExample) + } + }) + } +} + +// createOrReplaceCases holds one statement body per document kind that +// createStatement accepts. TestCreateOrReplaceMatchesModifyExceptExemptKinds +// builds each with `create or replace` and with `create or modify`. +var createOrReplaceCases = map[string]string{ + "entity": "persistent entity M.Customer (Name: String(200));", + "association": "association M.Order_Customer from M.Order to M.Customer type Reference;", + "module": "module M;", + "microflow": "microflow M.ACT_Recalculate () begin return; end;", + "javaaction": "java action M.FormatCurrency(Amount: Decimal not null) returns String as $$return \"\";$$;", + "javascriptaction": "javascript action M.IsStrictMode() returns Boolean platform Web as $$return true;$$;", + "page": "page M.OrderList (Title: 'Orders', Layout: Atlas_Core.Atlas_Default) { dynamictext txtHeading (Content: 'Orders') };", + "layout": "layout M.App_Default (layouttype: 'Responsive') { placeholder Main }", + "snippet": "snippet M.CustomerInfo { dynamictext t (Content: 'x') }", + "enumeration": "enumeration M.Color (Red 'Red');", + "validationrule": "validation rule for M.Customer.Email regex M.EmailPattern feedback 'Invalid';", + "databaseconnection": "database connection M.Erp type 'PostgreSQL' connection string @M.DbUrl username @M.DbUser password @M.DbPass;", + "constant": "constant M.ApiBaseUrl type String default 'https://api.example.com';", + "restclient": "rest client M.PetStore (BaseUrl: 'https://petstore.example.com', Authentication: NONE) { };", + "index": "index idx_name on M.Customer (Name);", + "odataclient": "odata client M.Api (Version: '1.0', ODataVersion: OData4, MetadataUrl: 'https://api.example.com/$metadata');", + "odataservice": "odata service M.CustomerAPI (path: 'odata/customers/', version: '1.0.0', ODataVersion: OData4, namespace: 'M.Customers') authentication basic { };", + "externalentity": "external entity M.Remote from odata client M.Api (EntitySet: 'Remotes', RemoteName: 'Remote');", + "externalentities": "external entities from M.Api into Integration;", + "navigation": "navigation Responsive home page M.Home_Web;", + "businesseventservice": "business event service M.CustomerEventsApi (ServiceName: 'CustomerEventsApi', EventNamePrefix: '') { message CustomerChangedEvent (CustomerId: Long) publish entity M.PBE_CustomerChangedEvent; };", + "workflow": "workflow M.LeaveApproval parameter $Context: M.LeaveRequest begin end workflow;", + "userrole": "user role Clerk (M.User);", + "demouser": "demo user 'demo' password 'Password1!' (Clerk);", + "imagecollection": "image collection M.AppIcons;", + "annotation": "annotation in M (Caption: 'Orders', Position: (60, 40));", + "queue": "queue M.Q_Orders (Parallelism: 3);", + "scheduledevent": "scheduled event M.NightlyCleanup (Microflow: M.SE_Cleanup, Repeat: Daily, HourOfDay: 4, MinuteOfHour: 0, TimeZone: Server, Enabled: true);", + "regularexpression": "regular expression M.Email (Expression: '.+@.+');", + "jsonstructure": "json structure M.JSON_Pet snippet '{\"id\": 1}';", + "messagedefinitioncollection": "message definition collection M.MD_Order (definition OrderMessage for M.Order as 'Orders' (OrderId));", + "importmapping": "import mapping M.IMM_Order with json structure M.JSON_Order { create M.Order { Id = id } };", + "exportmapping": "export mapping M.EMM_Order with json structure M.JSON_Order { M.Order { orderId = OrderId } };", + "configuration": "configuration 'Default';", + "publishedrestservice": "published rest service M.OrderAPI (Path: 'rest/orders/v1', Version: '1.0.0', ServiceName: 'Order API') { };", + "datatransformer": "data transformer M.Flatten source json '{\"id\": 1}' { jslt '{\"id\": .id}'; };", + "model": "model M.GPT4 (Provider: MxCloudGenAI, Key: M.ModelApiKey);", + "consumedmcpservice": "consumed mcp service M.WebSearch (ProtocolVersion: v2025_03_26, Version: '1.0');", + "knowledgebase": "knowledge base M.Docs (Provider: MxCloudGenAI, Key: M.KBApiKey);", + "agent": "agent M.Summarizer (UsageType: Task, Model: M.GPT4, SystemPrompt: 'Summarize.', UserPrompt: 'Text.');", + "nanoflow": "nanoflow M.NF_Validate () begin return; end;", + "rule": "rule M.Rule_IsSolvent ($c: M.Customer) returns Boolean begin return true; end;", + "menu": "menu M.Main_Menu (menu item 'Plain';);", + "translations": "translations in Administration for nl_NL ('Save' as 'Opslaan');", +} + +// foldPageModeFlags erases the one AST difference between `or replace` and +// `or modify` that means nothing: pages, snippets and layouts keep both flags, +// and every reader in the executor tests `IsModify || IsReplace`. +func foldPageModeFlags(stmts []ast.Statement) { + for _, s := range stmts { + switch n := s.(type) { + case *ast.CreatePageStmtV3: + n.IsModify, n.IsReplace = n.IsModify || n.IsReplace, false + case *ast.CreateSnippetStmtV3: + n.IsModify, n.IsReplace = n.IsModify || n.IsReplace, false + case *ast.CreateLayoutStmt: + n.IsModify, n.IsReplace = n.IsModify || n.IsReplace, false + } + } +} + +// createStatementKinds reads the document kinds createStatement accepts from +// the grammar source, so a kind added there fails this test until it has a case. +func createStatementKinds(t *testing.T) []string { + t.Helper() + src, err := os.ReadFile("../grammar/MDLParser.g4") + if err != nil { + t.Fatal(err) + } + s := string(src) + start := strings.Index(s, "\ncreateStatement\n") + if start < 0 { + t.Fatal("createStatement rule not found in MDLParser.g4") + } + end := strings.Index(s[start:], "\n ;") + body := s[start : start+end] + var kinds []string + for _, m := range regexp.MustCompile(`\bcreate(\w+)Statement\b`).FindAllStringSubmatch(body, -1) { + kinds = append(kinds, strings.ToLower(m[1])) + } + sort.Strings(kinds) + return kinds +} + +// `create or replace` is reported as MDL-DEPR001 exactly where it means +// `create or modify`: the two build the same statements. Where they do not, +// it is not an alias, and reporting it would tell the user to make a change +// that alters their script. Both directions are asserted, so the exemption list +// in the visitor cannot drift from what the visitors actually do. +func TestCreateOrReplaceMatchesModifyExceptExemptKinds(t *testing.T) { + for _, kind := range createStatementKinds(t) { + if _, ok := createOrReplaceCases[kind]; !ok { + t.Errorf("create kind %q has no case in createOrReplaceCases", kind) + } + } + + cases := make(map[string]string, len(createOrReplaceCases)+1) + for k, v := range createOrReplaceCases { + cases[k] = v + } + // A view entity is kind "entity" too, and its `or replace` drops and recreates. + cases["entity (view)"] = "view entity M.V (Name: String(100)) as (select c.Name as Name from M.Customer as c);" + + for name, body := range cases { + t.Run(name, func(t *testing.T) { + rep := mustBuild(t, "create or replace "+body) + mod := mustBuild(t, "create or modify "+body) + if len(rep.Statements) != 1 || len(mod.Statements) != 1 { + t.Fatalf("want one statement each, got %d and %d — the case does not exercise the visitor", + len(rep.Statements), len(mod.Statements)) + } + foldPageModeFlags(rep.Statements) + foldPageModeFlags(mod.Statements) + same := reflect.DeepEqual(rep.Statements, mod.Statements) + exempt := createOrReplaceIsNotAnAlias[name] || name == "entity (view)" + + switch { + case exempt && same: + t.Errorf("exempt, but `or replace` and `or modify` build the same statements: " + + "it is an alias after all — remove the exemption") + case !exempt && !same: + t.Errorf("`or replace` and `or modify` build different statements, so rewriting one " + + "to the other changes the script — exempt this kind or fix the visitor:\n"+ + " replace: %#v\n modify: %#v", rep.Statements, mod.Statements) + } + + got := deprecationCodes(rep) + if exempt { + if len(got) != 0 { + t.Errorf("exempt kind recorded %v", got) + } + return + } + if !reflect.DeepEqual(got, []string{deprecation.CreateOrReplace}) { + t.Errorf("recorded %v, want [%s]", got, deprecation.CreateOrReplace) + } else if rep.Deprecations[0].Subject != name { + t.Errorf("subject = %q, want %q", rep.Deprecations[0].Subject, name) + } + if got := deprecationCodes(mod); len(got) != 0 { + t.Errorf("`create or modify` recorded %v", got) + } + }) + } +} + +func TestShowRecordsDeprecation(t *testing.T) { + cases := []struct { + src string + want []string + }{ + {"show entities;", []string{deprecation.Show}}, + {"SHOW MICROFLOWS IN M;", []string{deprecation.Show}}, + {"show callers of M.MF transitive;", []string{deprecation.Show}}, + {"show version;", []string{deprecation.Show}}, + {"list entities;", nil}, + {"describe entity M.E;", nil}, + // Not the statement keyword: microflow activities spelled `show`. + {"create or modify microflow M.MF () begin show page M.P(); show message 'hi' type Information; end;", nil}, + // `show lint rules` has no `list` form yet, so it is not an alias. + {"show lint rules;", nil}, + } + for _, c := range cases { + t.Run(c.src, func(t *testing.T) { + if got := deprecationCodes(mustBuild(t, c.src)); !reflect.DeepEqual(got, c.want) { + t.Errorf("recorded %v, want %v", got, c.want) + } + }) + } +} + +// The record points at the deprecated token, so a warning in a long script +// names the right line. +func TestDeprecationRecordsPosition(t *testing.T) { + prog := mustBuild(t, "list modules;\n\n create or replace enumeration M.C (A 'A');\nshow modules;") + want := []ast.DeprecatedSpelling{ + {Code: deprecation.CreateOrReplace, Line: 3, Column: 12, Subject: "enumeration"}, + {Code: deprecation.Show, Line: 4, Column: 0}, + } + if !reflect.DeepEqual(prog.Deprecations, want) { + t.Errorf("got %+v\nwant %+v", prog.Deprecations, want) + } +} diff --git a/mdl/visitor/visitor_document_annotations.go b/mdl/visitor/visitor_document_annotations.go index 91fcf805c..a1b499ef2 100644 --- a/mdl/visitor/visitor_document_annotations.go +++ b/mdl/visitor/visitor_document_annotations.go @@ -28,6 +28,7 @@ func (b *Builder) ExitCreateStatement(ctx *parser.CreateStatementContext) { if ctx == nil { return } + b.recordCreateOrReplace(ctx) anns := ctx.AllAnnotation() if len(anns) == 0 { return From ce68cc603bf1be3fb9999f1a43ff6120c85cbb4c Mon Sep 17 00:00:00 2001 From: Ako Date: Sat, 26 Sep 2026 21:23:51 +0000 Subject: [PATCH 2/2] mdl: report `show` as MDL-DEPR002 only where its canonical form is `list` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The proposal's decided mapping for `show` (PROPOSAL_mdl_beta_syntax_freeze.md §3, R6) sends single things to `describe` (show entity X, show navigation, show project security, show structure, show context of X, …) and session state (show version, show status, show connections, show catalog status) to REPL commands. Warning "write `list …`" on those named a non-canonical form, and `fmt --upgrade` consuming the rewrite would have rewritten them twice. They are left unreported until their canonical forms exist (plan item 3.5). Also adds tests for the mdl/deprecation package itself: registry entries are well-formed and unique, and ParsePolicy refuses unknown values. Co-Authored-By: Claude Opus 5.5 --- docs-site/src/appendixes/error-messages.md | 2 +- mdl/deprecation/deprecation.go | 9 ++-- mdl/deprecation/deprecation_test.go | 62 ++++++++++++++++++++++ mdl/visitor/visitor_deprecations.go | 53 ++++++++++++++++-- mdl/visitor/visitor_deprecations_test.go | 27 +++++++++- 5 files changed, 143 insertions(+), 10 deletions(-) create mode 100644 mdl/deprecation/deprecation_test.go diff --git a/docs-site/src/appendixes/error-messages.md b/docs-site/src/appendixes/error-messages.md index b61ef63b7..c4dfbb4ac 100644 --- a/docs-site/src/appendixes/error-messages.md +++ b/docs-site/src/appendixes/error-messages.md @@ -155,7 +155,7 @@ line 1: `create or replace …` (enumeration) is deprecated; write `create or mo `check` and `exec` report these as **warnings**. To fail the run on one, for example in CI over documentation and examples, pass `--deprecations=error`. -The registry of deprecated spellings is `mdl/deprecation/deprecation.go`. It holds the code, old form, canonical form, rewrite and removal version of each entry. A spelling is only registered where it means exactly the same as its canonical form. `create or replace view entity`, for example, drops and recreates the view entity, so it is not reported. +The registry of deprecated spellings is `mdl/deprecation/deprecation.go`. It holds the code, old form, canonical form, rewrite and removal version of each entry. A spelling is only registered where it means exactly the same as its canonical form. `create or replace view entity`, for example, drops and recreates the view entity, so it is not reported. Likewise `show` is reported only where its canonical form is `list` (plurals and relationship queries); `show entity X` or `show version` is not, because it becomes `describe` or a REPL command, not `list`. ## mxcli Parser Errors diff --git a/mdl/deprecation/deprecation.go b/mdl/deprecation/deprecation.go index 3a2c67405..50d5c47eb 100644 --- a/mdl/deprecation/deprecation.go +++ b/mdl/deprecation/deprecation.go @@ -100,10 +100,11 @@ var entries = []Entry{ Canonical: "list …", Rewrite: Rewrite{Token: "show", Replacement: "list"}, RemovedIn: 2, - Note: "`list` is the same statement today. Forms that name a single thing " + - "(`show entity X`, `show navigation`, …) are expected to move on to " + - "`describe`, and session state (`show version`, `show status`) to REPL " + - "commands, in a later consolidation.", + Note: "Reported only for plurals and relationship queries, whose canonical " + + "form is `list`. Forms that name a single thing (`show entity X`, " + + "`show navigation`, `show project security`, …) become `describe`, and " + + "session state (`show version`, `show status`) a REPL command; they are " + + "not reported until those forms exist.", Example: "show entities in M;", CanonicalExample: "list entities in M;", }, diff --git a/mdl/deprecation/deprecation_test.go b/mdl/deprecation/deprecation_test.go new file mode 100644 index 000000000..8084f5c43 --- /dev/null +++ b/mdl/deprecation/deprecation_test.go @@ -0,0 +1,62 @@ +// SPDX-License-Identifier: Apache-2.0 + +package deprecation + +import ( + "regexp" + "testing" +) + +// The registry is append-only and its codes are what scripts, CI allowlists +// and docs key on, so each entry must be complete and its code well-formed and +// unique. +func TestRegistryEntriesAreWellFormed(t *testing.T) { + code := regexp.MustCompile(`^MDL-DEPR\d{3}$`) + seen := map[string]bool{} + for _, e := range All() { + if !code.MatchString(e.Code) { + t.Errorf("code %q is not MDL-DEPRnnn", e.Code) + } + if seen[e.Code] { + t.Errorf("code %s registered twice", e.Code) + } + seen[e.Code] = true + if !IsDeprecationCode(e.Code) { + t.Errorf("IsDeprecationCode(%q) = false", e.Code) + } + if got, ok := Lookup(e.Code); !ok || got.Code != e.Code { + t.Errorf("Lookup(%q) = %v, %v", e.Code, got.Code, ok) + } + if e.Old == "" || e.Canonical == "" || e.Rewrite.Token == "" || e.Rewrite.Replacement == "" || + e.Example == "" || e.CanonicalExample == "" { + t.Errorf("%s is incomplete: %+v", e.Code, e) + } + // ADR-0011: an alias warns under the version that deprecates it (1) + // and is refused from a later one. + if e.RemovedIn < 2 { + t.Errorf("%s RemovedIn = %d, want >= 2", e.Code, e.RemovedIn) + } + } + if _, ok := Lookup("MDL-DEPR999"); ok { + t.Error("Lookup found an unregistered code") + } + if IsDeprecationCode("MDL065") { + t.Error("MDL065 treated as a registry code") + } +} + +// A typo in --deprecations must be an error, not a silent `warn`: that would +// let a CI gate pass that was meant to fail. +func TestParsePolicy(t *testing.T) { + for in, want := range map[string]Policy{"": Warn, "warn": Warn, "error": Error, " ERROR ": Error} { + got, err := ParsePolicy(in) + if err != nil || got != want { + t.Errorf("ParsePolicy(%q) = %v, %v; want %v", in, got, err, want) + } + } + for _, in := range []string{"errors", "fail", "warning", "bogus"} { + if _, err := ParsePolicy(in); err == nil { + t.Errorf("ParsePolicy(%q) accepted an unknown value", in) + } + } +} diff --git a/mdl/visitor/visitor_deprecations.go b/mdl/visitor/visitor_deprecations.go index db81f1662..72e44bc22 100644 --- a/mdl/visitor/visitor_deprecations.go +++ b/mdl/visitor/visitor_deprecations.go @@ -61,10 +61,57 @@ func (b *Builder) recordCreateOrReplace(ctx *parser.CreateStatementContext) { b.recordDeprecation(deprecation.CreateOrReplace, ctx.REPLACE().GetSymbol(), kind) } -// ExitShowOrList records MDL-DEPR002 for `show`. showOrList is used only by -// showStatement, where `show` and `list` build the same statement. +// showNotYetList lists the showStatement forms whose decided canonical form is +// NOT `list` (PROPOSAL_mdl_beta_syntax_freeze.md §3, R6): a single thing +// becomes `describe`, session state a REPL command. Keyed on the token after +// `show` (CATALOG only with STATUS, see showCanonicalIsList). `list` builds the +// same statement for these today, but it is not their canonical form, so +// recommending it would name the wrong form and make `fmt --upgrade` rewrite +// them twice. They get their own registry entries once the canonical forms +// exist (plan item 3.5). Pinned by TestShowRecordsDeprecation. +var showNotYetList = map[int]bool{ + parser.MDLParserENTITY: true, // show entity X -> describe entity X + parser.MDLParserASSOCIATION: true, // show association X -> describe association X + parser.MDLParserPAGE: true, // show page X -> describe page X + parser.MDLParserNAVIGATION: true, // show navigation … -> describe navigation + parser.MDLParserSTRUCTURE: true, // show structure -> describe structure + parser.MDLParserCONTEXT: true, // show context of X -> describe context of X + parser.MDLParserPROJECT: true, // show project security -> describe app security + parser.MDLParserSECURITY: true, // show security matrix -> describe security matrix + parser.MDLParserSETTINGS: true, // show settings -> describe (one thing) + parser.MDLParserVERSION: true, // session state -> REPL command (R7) + parser.MDLParserSTATUS: true, // session state -> REPL command (R7) + parser.MDLParserCONNECTIONS: true, // session state -> REPL command (R7) +} + +// showCanonicalIsList reports whether the showStatement that ctx starts is a +// form whose canonical spelling is `list` (plurals and relationship queries). +func showCanonicalIsList(ctx *parser.ShowOrListContext) bool { + stmt, ok := ctx.GetParent().(*parser.ShowStatementContext) + if !ok || stmt.GetChildCount() < 2 { + return true + } + next := func(i int) int { + if i >= stmt.GetChildCount() { + return antlr.TokenInvalidType + } + if tn, ok := stmt.GetChild(i).(antlr.TerminalNode); ok { + return tn.GetSymbol().GetTokenType() + } + return antlr.TokenInvalidType + } + first := next(1) + if first == parser.MDLParserCATALOG { + return next(2) != parser.MDLParserSTATUS // catalog status is session state + } + return !showNotYetList[first] +} + +// ExitShowOrList records MDL-DEPR002 for `show` where its canonical form is +// `list`. showOrList is used only by showStatement, where `show` and `list` +// build the same statement. func (b *Builder) ExitShowOrList(ctx *parser.ShowOrListContext) { - if ctx == nil || ctx.SHOW() == nil { + if ctx == nil || ctx.SHOW() == nil || !showCanonicalIsList(ctx) { return } b.recordDeprecation(deprecation.Show, ctx.SHOW().GetSymbol(), "") diff --git a/mdl/visitor/visitor_deprecations_test.go b/mdl/visitor/visitor_deprecations_test.go index 9725846dd..6bb425c07 100644 --- a/mdl/visitor/visitor_deprecations_test.go +++ b/mdl/visitor/visitor_deprecations_test.go @@ -186,7 +186,7 @@ func TestCreateOrReplaceMatchesModifyExceptExemptKinds(t *testing.T) { t.Errorf("exempt, but `or replace` and `or modify` build the same statements: " + "it is an alias after all — remove the exemption") case !exempt && !same: - t.Errorf("`or replace` and `or modify` build different statements, so rewriting one " + + t.Errorf("`or replace` and `or modify` build different statements, so rewriting one "+ "to the other changes the script — exempt this kind or fix the visitor:\n"+ " replace: %#v\n modify: %#v", rep.Statements, mod.Statements) } @@ -218,7 +218,30 @@ func TestShowRecordsDeprecation(t *testing.T) { {"show entities;", []string{deprecation.Show}}, {"SHOW MICROFLOWS IN M;", []string{deprecation.Show}}, {"show callers of M.MF transitive;", []string{deprecation.Show}}, - {"show version;", []string{deprecation.Show}}, + {"show catalog tables;", []string{deprecation.Show}}, + {"show access on M.E;", []string{deprecation.Show}}, + {"show design properties;", []string{deprecation.Show}}, + {"show widgets;", []string{deprecation.Show}}, + // Not reported: the decided canonical form of these is NOT `list` + // (PROPOSAL_mdl_beta_syntax_freeze.md §3, R6). Single things become + // `describe`, session state a REPL command, so `list entity M.E` or + // `list version` would name a non-canonical form and make + // `fmt --upgrade` rewrite them twice. + {"show entity M.E;", nil}, + {"show association M.A;", nil}, + {"show page M.P;", nil}, + {"show navigation;", nil}, + {"show navigation homes;", nil}, + {"show navigation menu M.Nav;", nil}, + {"show structure depth 2 in M;", nil}, + {"show context of M.MF;", nil}, + {"show project security;", nil}, + {"show security matrix in M;", nil}, + {"show settings;", nil}, + {"show version;", nil}, + {"show status;", nil}, + {"show connections;", nil}, + {"show catalog status;", nil}, {"list entities;", nil}, {"describe entity M.E;", nil}, // Not the statement keyword: microflow activities spelled `show`.