diff --git a/.claude/lint-rules/example_microflow.star b/.claude/lint-rules/example_microflow.star index c76adbac9c..04bb742bee 100644 --- a/.claude/lint-rules/example_microflow.star +++ b/.claude/lint-rules/example_microflow.star @@ -7,7 +7,15 @@ # VAL_ - Validation microflows # SCH_ - Scheduled event microflows # -# Customize the prefixes and patterns to match your project standards. +# Customize the prefixes in lint-config.yaml rather than here (`mxcli init` +# rewrites this file): +# +# rules: +# CUSTOM002: +# options: +# valid_prefixes: [ACT_, SUB_, DS_, TEST_] # or "ACT_, SUB_, DS_, TEST_" +# +# The option replaces VALID_PREFIXES below; unset keeps it. # # Microflow properties: # .id - Document ID @@ -28,7 +36,7 @@ DESCRIPTION = "Microflows should have standard naming prefixes (ACT_, SUB_, DS_, CATEGORY = "naming" SEVERITY = "info" -# Define valid prefixes - customize for your project +# Built-in valid prefixes, used when the valid_prefixes option is unset VALID_PREFIXES = ["ACT_", "SUB_", "DS_", "VAL_", "SCH_", "IVK_", "OCH_"] def check(): @@ -36,13 +44,14 @@ def check(): Check that microflows use standard naming prefixes. """ violations = [] + valid_prefixes = get_list_option("valid_prefixes", VALID_PREFIXES) for mf in microflows(): name = mf.name # Simple name # Check if name starts with any valid prefix has_valid_prefix = False - for prefix in VALID_PREFIXES: + for prefix in valid_prefixes: if name.startswith(prefix): has_valid_prefix = True break @@ -57,7 +66,7 @@ def check(): message="{} '{}' should start with a standard prefix ({})".format( mf.document_noun_title, name, - ", ".join(VALID_PREFIXES) + ", ".join(valid_prefixes) ), location=loc ) diff --git a/.claude/lint-rules/orphaned_elements.star b/.claude/lint-rules/orphaned_elements.star index 5fe5189a6b..a13b400043 100644 --- a/.claude/lint-rules/orphaned_elements.star +++ b/.claude/lint-rules/orphaned_elements.star @@ -12,6 +12,13 @@ # - Entities with no references at all # # This rule requires REFRESH CATALOG FULL to populate the refs table. +# +# Options (lint-config.yaml, rules.QUAL004.options): +# entry_point_prefixes - microflow name prefixes that mark an entry point, +# as a YAML list or comma-separated text. Replaces +# the built-in list below; unset keeps it. +# +# Edit the option, not this file: `mxcli init` rewrites the shipped rules. RULE_ID = "QUAL004" RULE_NAME = "Orphaned Elements" @@ -19,7 +26,8 @@ DESCRIPTION = "Elements should be referenced somewhere in the project or marked CATEGORY = "quality" SEVERITY = "info" -# Prefixes for microflows that are expected to be entry points +# Prefixes for microflows that are expected to be entry points, unless the +# entry_point_prefixes option says otherwise ENTRY_POINT_PREFIXES = ["ACT_", "SCH_", "WS_", "REST_", "OData_"] # Page name patterns that are likely entry points @@ -33,9 +41,9 @@ MICROFLOW_ENTRY_KINDS = ["call", "schedule", "publish", "datasource", "action", # Reference kinds that mean "something opens this page". PAGE_ENTRY_KINDS = ["show_page", "home_page", "login_page", "menu_item", "action"] -def is_entry_point_microflow(name): +def is_entry_point_microflow(name, prefixes): """Check if a microflow name suggests it's a UI/scheduled entry point.""" - for prefix in ENTRY_POINT_PREFIXES: + for prefix in prefixes: if name.startswith(prefix): return True return False @@ -52,11 +60,13 @@ def check(): Check for orphaned elements that have no incoming references. """ violations = [] + entry_prefixes = get_list_option("entry_point_prefixes", ENTRY_POINT_PREFIXES) + rename_hint = "/".join(entry_prefixes[:2]) if entry_prefixes else "an entry-point" # Check microflows for mf in microflows(): # Skip entry point microflows - if is_entry_point_microflow(mf.name): + if is_entry_point_microflow(mf.name, entry_prefixes): continue # Get references to this microflow @@ -95,7 +105,7 @@ def check(): v = violation( message="{} '{}' is not called from anywhere.".format(mf.document_noun_title, mf.name), location=loc, - suggestion="Remove if unused, or rename with ACT_/SCH_ prefix if it's an entry point." + suggestion="Remove if unused, rename with {} prefix if it's an entry point, or add its prefix to rules.QUAL004.options.entry_point_prefixes in lint-config.yaml.".format(rename_hint) ) violations.append(v) diff --git a/.claude/skills/mendix/write-lint-rules/SKILL.md b/.claude/skills/mendix/write-lint-rules/SKILL.md index 8d9ab2afbc..a32c87198a 100644 --- a/.claude/skills/mendix/write-lint-rules/SKILL.md +++ b/.claude/skills/mendix/write-lint-rules/SKILL.md @@ -615,7 +615,7 @@ Returned by `languages()`, one per language enabled in the project settings. A p | `is_camel_case(s)` | Returns True if string is camelCase | | `matches(s, pattern)` | Returns True if string matches regex | | `find_all(s, pattern)` | Every match of the regex in `s`: a tuple of the capture groups per match, or the matched string when the pattern has no groups | -| `get_option(key, default?)` | The rule's option `key` from the `options:` block under its rule ID in `.claude/lint-config.yaml`, or `default` (`None` if omitted) when unset | +| `get_option(key, default?)` / `get_list_option(key, default?)` | The rule's option `key` from the `options:` block under its rule ID in `.claude/lint-config.yaml`, or `default` (`None` if omitted) when unset. YAML lists and maps arrive as lists and dicts. `get_list_option` returns a list of strings from a YAML list or comma-separated text (`"ACT_, SUB_"`) — use it, not a constant list a project would have to edit (`mxcli init` rewrites shipped rules). A value of the wrong shape is reported as a warning and yields `default` | | `struct(**kwargs)` | Build an ad-hoc struct, e.g. `struct(name="x", count=1)`, to group values inside a rule | ## Common Patterns diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f5503967a..ea8ca9c59d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added - **Lint rule CONV020: an object created in a microflow and never committed, returned or passed on** (mendixlabs/mxcli#1518) — `$o = create Shop.Item (…)` without `commit` makes an object in memory only; if the microflow then never commits it or hands it to anything that could, the object is discarded when the microflow ends, and `check`, `mx check` and the build all pass. In the field an after-startup seed microflow logged "Seeded 20" and the table had 0 rows. CONV020 (warning) reports a create with commit No of a persistent entity in a microflow when nothing in the flow uses the variable except to read a member (`$o/Cnt`) or change it without commit. Any other mention — commit, change with commit, return, an argument to a microflow, Java/JavaScript action, workflow, mapping or page, a list, an association value, delete, rollback, or the bare variable in any expression — counts as passing it on. It is conservative by design: flow-insensitive (a commit on one branch or in an error handler counts), and nanoflows, non-persistent entities and System/Marketplace modules are not reported; anything it cannot classify keeps it quiet. Docs: "Built-in Rules" › CONV020 details. +- **QUAL004's entry-point prefixes and CUSTOM002's valid prefixes are `lint-config.yaml` options** (mendixlabs/mxcli#1509) — both lists were constants in the rule files, and `mxcli init` rewrites those files, so a project whose convention names microflows `TEST_`, `GET_` or `PRS_` could not keep an edit and got one finding per such microflow (84 QUAL004 and 121 CUSTOM002 findings on the reporting project). `rules.QUAL004.options.entry_point_prefixes` and `rules.CUSTOM002.options.valid_prefixes` take a YAML list or comma-separated text and replace the built-in list; unset, the built-in list applies as before. Underneath, a YAML list or map under any rule's `options` reached a Starlark rule as `None` — `get_option` converted scalars only — so a list-valued option was dropped without a word; lists and maps now convert recursively, the new `get_list_option(key, default)` builtin reads a list of strings in either spelling, and an option value a rule cannot use is reported as a warning naming `rules..options.` (kept out of the score) while the rule runs on its default. QUAL004 still counts only callers in the model: a microflow called only from `tests/*.test.mdl` is reported, and its prefix belongs in `entry_point_prefixes`. Docs: "Starlark Rules" › QUAL004 and CUSTOM002 options. - **`layoutheader` and native layout parts — every Atlas layout copies whole** — `describe layout` → `create layout` of Atlas's Phone_ and Tablet_ layouts dropped their header bar (`Forms$Header`, printed as NOT re-executable), so pages bound to its `HeaderLeft` / `HeaderRight` placeholders failed CE1613 and a copy of Phone_Sidebar or Tablet_Sidebar failed CE0611 "A sidebar toggle button is required if a region can toggle". `layoutheader header1 { region left { … } region right { … } }` writes it. A native layout's copy silently lost its header placeholder, bottom bar and sidebar — pages bound to `Header` or a sidebar placeholder then failed CE1613; they are `region header { placeholder Header }`, `showbottombar: true` and `region sidebar { … }`, and the native content is written with the eight keys Studio Pro stores. Measured on a new 11.13.0 app: all 22 Atlas_Core layouts copied, with pages bound to the copied header and sidebar placeholders, build with 0 errors. - **`sidebartoggle` — Atlas's responsive sidebar toggle (`Forms$SidebarToggleButton`) in MDL** (mendixlabs/mxcli#1220) — copying `Atlas_Core.Atlas_Default` with `describe layout` → `create layout` dropped the button that opens the sidebar on phones and tablets: `describe` printed it as a NOT re-executable comment, and `exec` gave no warning. `sidebartoggle sidebarToggle3 (Tooltip: 'Toggle Menu', ButtonStyle: Primary, Icon: 'Atlas_Core.Atlas_Filled.navigation-menu', Class: 'toggle-btn')` takes an action button's caption, tooltip, style and icon, and `RenderMode: Link`; it has no action. The written widget has the keys of Atlas's own, including its caption under `CaptionTemplate` rather than the `Caption` key the vendored metamodel bound. A scroll container region's `ToggleMode` (`ToggleMode: 'ShrinkContentInitiallyClosed'`), which the toggle opens, is described and written too — without it the copy failed CE0628. - **`mxcli marketplace package` — export a module as a `.mpk`** (mendixlabs/mxcli#1382) — `mxcli marketplace package MyModule -p app.mpr [-o dir] [--all-libs] [--exclude REGEX] [--exclude-managed-dependencies] [--json]` is the reverse of `marketplace install --file`: it wraps Mendix's own `mx create-module-package` (resolved for the project's Mendix version), so the package is what Studio Pro's *Export module package* makes, and it installs back with `install --file`. By default only the module's own required `userlib/` JARs go in (`mx -l`; measured 12.5 KB against 24 MB with Business Events installed); `--all-libs` bundles every JAR. `-l` filters only JARs — mx still exports the app's whole `resources/` folder (measured: another module's `resources/EventLog/` came along) — so after the export mxcli opens the package, warns on stderr about each `resources//`, `javasource//` or `themesource//` file of another module, and prints the `--exclude` pattern that drops them in the one form mx accepts (`^resources.EventLog.`; a character class or backslash is rejected by mx with exit 1 and a usage message, and an exit 0 that wrote no package is reported as an error too). A module name that is not in the project lists the project's modules instead of failing with mx's exit 3, and mx's exit codes 1–4 are reported with what they mean and what mx printed. `mx create-module-package` writes into the project it is given — measured on a blank 11.12.5 app: `theme-cache/web/theme.compiled.css` and its map rewritten, `deployment/sass/main.scss` added, the module's `themesource/` folder created, model files untouched — so it runs on a temporary copy, as `docker check` does; a before/after listing of the project (sizes and modification times) was identical. The `mx` that runs must be the project's Mendix version: `ResolveMxForVersion` prefers an `mx` on PATH and falls back to any cached one, so mxcli reads the version of the `mx` it found (its `mx.deps.json`, else its version-named folder) and refuses a different major.minor.patch, or one it cannot identify, naming both versions and the fix (`mxcli setup mxbuild --version X`); `--mxbuild-path` is the override, with a warning on stderr. mx's own output on a successful export goes to stderr, cancelling the command stops mx, and an output directory inside the project is not copied into the temporary copy. Round trip on 11.12.5: export, `install --file` into a second fresh app, `mx check` — 0 errors. diff --git a/cmd/mxcli/init.go b/cmd/mxcli/init.go index d8b322d6ab..c7e0186174 100644 --- a/cmd/mxcli/init.go +++ b/cmd/mxcli/init.go @@ -648,6 +648,9 @@ excludeModules: # severity: warning # options: # maxObjects: 20 +# QUAL004: # orphaned elements: name prefixes of entry points +# options: +# entry_point_prefixes: [ACT_, SCH_, WS_, REST_, OData_] rules: {} ` diff --git a/docs-site/src/tools/mxcli-lint.md b/docs-site/src/tools/mxcli-lint.md index 474b504fa5..4a2ad410da 100644 --- a/docs-site/src/tools/mxcli-lint.md +++ b/docs-site/src/tools/mxcli-lint.md @@ -98,6 +98,12 @@ failure is an error (`Starlark rule error: …`). A rule's configured severity i `lint-config.yaml` applies to its findings, never to its failure. `mxcli report` keeps all rule failures out of the score. +An option in `lint-config.yaml` that a rule cannot use — for example a number +where the rule expects a list of prefixes — is reported at **warning** level as +`lint-config.yaml: rules..options. … the rule ran with its default`. +The rule still runs, on its built-in default, and the warning is kept out of the +score like any other rule failure. + A rule file that does not load at all (for example, one calling a builtin this mxcli lacks) is skipped with a warning on stderr. diff --git a/docs-site/src/tools/starlark-rules.md b/docs-site/src/tools/starlark-rules.md index c3818abffe..ed0dd18617 100644 --- a/docs-site/src/tools/starlark-rules.md +++ b/docs-site/src/tools/starlark-rules.md @@ -30,7 +30,7 @@ In addition to the built-in Go rules, mxcli bundles 29 Starlark-based lint rules | **QUAL001** | McCabe complexity -- Flags microflows with high cyclomatic complexity | | **QUAL002** | Documentation -- Missing documentation across every document type: modules, entities, pages, microflows, workflows, Java/JavaScript actions and their parameters, REST services, mappings, constants ([options](#qual002-options)) | | **QUAL003** | Long microflows -- Warns about microflows with too many activities | -| **QUAL004** | Orphaned elements -- Detects unused entities, microflows, or pages | +| **QUAL004** | Orphaned elements -- Detects unused entities, microflows, or pages ([options](#qual004-and-custom002-options)) | ### Design Rules (DESIGN001) @@ -127,6 +127,44 @@ and one in `_DOC_KINDS` (`missing_documentation.star`) giving the option name an the suggestion text. `TestQUAL002_SweepsEveryAdvertisedDocumentType` fails if the Go side advertises a kind the tests do not cover. +### QUAL004 and CUSTOM002 options {#qual004-and-custom002-options} + +Both rules decide by name prefix. QUAL004 (orphaned elements) does not report a +microflow whose name starts with an **entry-point prefix**, because something +outside the model calls it; CUSTOM002 (microflow prefix convention) reports a +microflow whose name starts with none of the **valid prefixes**. Projects name +things differently, so both lists are options — set them in `lint-config.yaml` +rather than in the rule files, which `mxcli init` rewrites: + +| Rule | Option | Default | +|------|--------|---------| +| QUAL004 | `entry_point_prefixes` | `ACT_, SCH_, WS_, REST_, OData_` | +| CUSTOM002 | `valid_prefixes` | `ACT_, SUB_, DS_, VAL_, SCH_, IVK_, OCH_` | + +```yaml +rules: + QUAL004: + options: + entry_point_prefixes: [ACT_, SCH_, WS_, REST_, OData_, TEST_] + CUSTOM002: + options: + valid_prefixes: "ACT_, SUB_, DS_, VAL_, SCH_, TEST_, GET_" +``` + +Write the list as a YAML list or as one comma-separated string; spaces around +the commas are ignored. A configured list **replaces** the default, so repeat the +default prefixes you still want. An empty list is a real setting: for QUAL004 it +means no microflow is exempt by name. A value of any other shape (a number, a +list holding a number) is not silently ignored — the rule runs with its default +and the run reports a warning naming the option, listed with the rules that could +not run and kept out of the score. + +QUAL004 counts a microflow as called only when something **in the model** runs +it (a call, a schedule, a page or widget, a published service, a project +setting). A call from an MDL test script (`tests/*.test.mdl`) is not in the +model, so a helper used only by tests is reported; give such helpers a prefix +and list it in `entry_point_prefixes`. + ### CONV018 options {#conv018-options} Studio Pro lets a module hold folders, and every Mendix style guide expects them diff --git a/mdl/linter/starlark.go b/mdl/linter/starlark.go index 3c3865d386..3d3522ec28 100644 --- a/mdl/linter/starlark.go +++ b/mdl/linter/starlark.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "regexp" + "sort" "strings" "unicode" @@ -31,6 +32,9 @@ type StarlarkRule struct { globals starlark.StringDict options map[string]any requiredMode CatalogMode + // optionWarnings collects lint-config.yaml values this run could not use; + // Check reports each as a warning after the rule has run on its default. + optionWarnings []string } // RequiredCatalogMode reports the catalog depth this rule needs (CatalogRequirer). @@ -59,6 +63,7 @@ func (r *StarlarkRule) Category() string { return r.category } // Check executes the Starlark check function and returns violations. func (r *StarlarkRule) Check(ctx *LintContext) []Violation { r.ctx = ctx + r.optionWarnings = nil // Create a new thread for execution thread := &starlark.Thread{ @@ -71,11 +76,28 @@ func (r *StarlarkRule) Check(ctx *LintContext) []Violation { // Call the check function result, err := starlark.Call(thread, r.checkFn, nil, nil) if err != nil { - return []Violation{ruleFailureViolation(r.id, err)} + return append(r.optionWarningViolations(), ruleFailureViolation(r.id, err)) } // Convert result to violations - return r.convertViolations(result) + return append(r.convertViolations(result), r.optionWarningViolations()...) +} + +// optionWarningViolations reports the configured option values the rule could +// not use. They are RuleFailure findings: a misconfigured lint-config.yaml says +// nothing about the project, so it must not move the score. +func (r *StarlarkRule) optionWarningViolations() []Violation { + var out []Violation + for _, msg := range r.optionWarnings { + out = append(out, Violation{ + RuleID: r.id, + Severity: SeverityWarning, + Message: msg, + Suggestion: "Fix the value in lint-config.yaml; until then the rule runs with its built-in default.", + RuleFailure: true, + }) + } + return out } // missingStructAttrRe matches the evaluator's message for reading a field a @@ -446,6 +468,9 @@ func (r *StarlarkRule) buildPredeclared() starlark.StringDict { // Options access: get_option("key") or get_option("key", default) "get_option": starlark.NewBuiltin("get_option", r.builtinGetOption), + // get_list_option("key", default): a list of strings, given in YAML + // as a list or as comma-separated text + "get_list_option": starlark.NewBuiltin("get_list_option", r.builtinGetListOption), } } @@ -776,6 +801,9 @@ func (r *StarlarkRule) builtinScheduledEvents(_ *starlark.Thread, _ *starlark.Bu // builtinGetOption returns a rule option value from the lint config file. // Signature: get_option(key, default=None) +// +// A value that cannot be represented in Starlark is reported as a warning and +// the default is returned — never a silent None (mendixlabs/mxcli#1509). func (r *StarlarkRule) builtinGetOption(_ *starlark.Thread, _ *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) { var key starlark.String var defaultVal starlark.Value = starlark.None @@ -785,27 +813,118 @@ func (r *StarlarkRule) builtinGetOption(_ *starlark.Thread, _ *starlark.Builtin, ); err != nil { return nil, err } - if r.options != nil { - if v, ok := r.options[string(key)]; ok { - return goValueToStarlark(v), nil + v, ok := r.options[string(key)] + if !ok || v == nil { + return defaultVal, nil + } + sv, err := goValueToStarlark(v) + if err != nil { + r.warnOption(string(key), v, err.Error()) + return defaultVal, nil + } + return sv, nil +} + +// builtinGetListOption returns a rule option as a list of strings. The option +// may be a YAML list of strings or one comma-separated string; parts are +// trimmed and empty parts dropped. An unset option returns the default; any +// other value is reported as a warning and also returns the default. +// Signature: get_list_option(key, default=None) +func (r *StarlarkRule) builtinGetListOption(_ *starlark.Thread, _ *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) { + var key starlark.String + var defaultVal starlark.Value = starlark.None + if err := starlark.UnpackArgs("get_list_option", args, kwargs, + "key", &key, + "default?", &defaultVal, + ); err != nil { + return nil, err + } + v, ok := r.options[string(key)] + if !ok || v == nil { + return defaultVal, nil + } + var items []starlark.Value + switch val := v.(type) { + case string: + for _, part := range strings.Split(val, ",") { + if part = strings.TrimSpace(part); part != "" { + items = append(items, starlark.String(part)) + } + } + case []any: + for _, e := range val { + s, isStr := e.(string) + if !isStr { + r.warnOption(string(key), v, "must be a list of strings or a comma-separated string") + return defaultVal, nil + } + if s = strings.TrimSpace(s); s != "" { + items = append(items, starlark.String(s)) + } } + default: + r.warnOption(string(key), v, "must be a list of strings or a comma-separated string") + return defaultVal, nil } - return defaultVal, nil + return starlark.NewList(items), nil +} + +// warnOption records a configured option value the rule cannot use. +func (r *StarlarkRule) warnOption(key string, v any, problem string) { + r.optionWarnings = append(r.optionWarnings, fmt.Sprintf( + "lint-config.yaml: rules.%s.options.%s %s (got %T %v); the rule ran with its default", + r.id, key, problem, v, v)) } -// goValueToStarlark converts a Go value (from YAML unmarshaling) to a Starlark value. -func goValueToStarlark(v any) starlark.Value { +// goValueToStarlark converts a Go value (from YAML unmarshaling) to a Starlark +// value. Lists and string-keyed maps convert recursively; anything else is an +// error, so the caller can say which option was dropped instead of handing +// the rule a None it cannot tell from "unset". +func goValueToStarlark(v any) (starlark.Value, error) { switch val := v.(type) { + case nil: + return starlark.None, nil case bool: - return starlark.Bool(val) + return starlark.Bool(val), nil case int: - return starlark.MakeInt(val) + return starlark.MakeInt(val), nil + case int64: + return starlark.MakeInt64(val), nil + case uint64: + return starlark.MakeUint64(val), nil case float64: - return starlark.Float(val) + return starlark.Float(val), nil case string: - return starlark.String(val) + return starlark.String(val), nil + case []any: + items := make([]starlark.Value, 0, len(val)) + for _, e := range val { + sv, err := goValueToStarlark(e) + if err != nil { + return nil, err + } + items = append(items, sv) + } + return starlark.NewList(items), nil + case map[string]any: + keys := make([]string, 0, len(val)) + for k := range val { + keys = append(keys, k) + } + sort.Strings(keys) + d := starlark.NewDict(len(val)) + for _, k := range keys { + sv, err := goValueToStarlark(val[k]) + if err != nil { + return nil, err + } + if err := d.SetKey(starlark.String(k), sv); err != nil { + return nil, err + } + } + return d, nil default: - return starlark.None + return nil, fmt.Errorf("has a value of a type rules cannot read") } } diff --git a/mdl/linter/starlark_catalog_mode_guard_test.go b/mdl/linter/starlark_catalog_mode_guard_test.go index 17833799e0..edc26c2c5c 100644 --- a/mdl/linter/starlark_catalog_mode_guard_test.go +++ b/mdl/linter/starlark_catalog_mode_guard_test.go @@ -87,15 +87,16 @@ var builtinModes = map[string]builtinProbe{ "integration_surface": {CatalogCommunities, ""}, // No catalog access. - "parse_xpath": {CatalogFast, ""}, - "violation": {CatalogFast, ""}, - "location": {CatalogFast, ""}, - "is_pascal_case": {CatalogFast, ""}, - "is_camel_case": {CatalogFast, ""}, - "matches": {CatalogFast, ""}, - "find_all": {CatalogFast, ""}, - "struct": {CatalogFast, ""}, - "get_option": {CatalogFast, ""}, + "parse_xpath": {CatalogFast, ""}, + "violation": {CatalogFast, ""}, + "location": {CatalogFast, ""}, + "is_pascal_case": {CatalogFast, ""}, + "is_camel_case": {CatalogFast, ""}, + "matches": {CatalogFast, ""}, + "find_all": {CatalogFast, ""}, + "struct": {CatalogFast, ""}, + "get_option": {CatalogFast, ""}, + "get_list_option": {CatalogFast, ""}, } // fieldsKnownFull are struct fields filled only by a full build, pinned so the diff --git a/mdl/linter/starlark_options_test.go b/mdl/linter/starlark_options_test.go new file mode 100644 index 0000000000..5e6de9debf --- /dev/null +++ b/mdl/linter/starlark_options_test.go @@ -0,0 +1,161 @@ +// SPDX-License-Identifier: Apache-2.0 + +package linter + +import ( + "path/filepath" + "strings" + "testing" +) + +// mendixlabs/mxcli#1509: a YAML list under a rule's options reached the rule as +// None, because goValueToStarlark knew only scalars. A rule reading a list +// option therefore fell back to its default with no sign that the configured +// value had been dropped. + +const optionEchoRule = ` +RULE_ID = "OPT001" +RULE_NAME = "Echo" +DESCRIPTION = "reports the options it was given" +CATEGORY = "Quality" +SEVERITY = "info" + +def check(): + return [ + violation(message = "get_option=" + repr(get_option("xs", "unset"))), + violation(message = "get_list_option=" + repr(get_list_option("xs", ["DEF_"]))), + ] +` + +func runEcho(t *testing.T, opts map[string]any) (getOpt, getList string, warnings []Violation) { + t.Helper() + r := loadRuleSource(t, "echo.star", optionEchoRule) + if opts != nil { + r.Configure(opts) + } + for _, v := range r.Check(&LintContext{}) { + switch { + case v.RuleFailure: + warnings = append(warnings, v) + case strings.HasPrefix(v.Message, "get_option="): + getOpt = strings.TrimPrefix(v.Message, "get_option=") + case strings.HasPrefix(v.Message, "get_list_option="): + getList = strings.TrimPrefix(v.Message, "get_list_option=") + } + } + return +} + +func TestGetOption_YAMLListBecomesStarlarkList(t *testing.T) { + getOpt, getList, warnings := runEcho(t, map[string]any{"xs": []any{"ACT_", "TEST_"}}) + if getOpt != `["ACT_", "TEST_"]` { + t.Errorf("get_option = %s, want the list (a YAML list used to arrive as None)", getOpt) + } + if getList != `["ACT_", "TEST_"]` { + t.Errorf("get_list_option = %s, want the list", getList) + } + if len(warnings) != 0 { + t.Errorf("unexpected warnings: %+v", warnings) + } +} + +func TestGetOption_NestedValuesConvertRecursively(t *testing.T) { + v, err := goValueToStarlark(map[string]any{"a": []any{1, "b", true, []any{2.5}}}) + if err != nil { + t.Fatal(err) + } + if got := v.String(); got != `{"a": [1, "b", True, [2.5]]}` { + t.Errorf("got %s", got) + } +} + +func TestGetListOption_CommaSeparatedString(t *testing.T) { + _, getList, warnings := runEcho(t, map[string]any{"xs": " ACT_, TEST_ ,,GET_"}) + if getList != `["ACT_", "TEST_", "GET_"]` { + t.Errorf("get_list_option = %s, want the trimmed, non-empty parts", getList) + } + if len(warnings) != 0 { + t.Errorf("unexpected warnings: %+v", warnings) + } +} + +func TestGetListOption_UnsetUsesDefault(t *testing.T) { + getOpt, getList, warnings := runEcho(t, nil) + if getOpt != `"unset"` || getList != `["DEF_"]` { + t.Errorf("get_option = %s, get_list_option = %s, want the defaults", getOpt, getList) + } + if len(warnings) != 0 { + t.Errorf("unexpected warnings: %+v", warnings) + } +} + +// A value of the wrong shape is reported, and the rule still runs on its +// default rather than on None. +func TestGetListOption_MalformedValueWarnsAndUsesDefault(t *testing.T) { + for name, val := range map[string]any{ + "number": 5, + "list of numbers": []any{"ACT_", 7}, + "map": map[string]any{"a": "b"}, + } { + t.Run(name, func(t *testing.T) { + _, getList, warnings := runEcho(t, map[string]any{"xs": val}) + if getList != `["DEF_"]` { + t.Errorf("get_list_option = %s, want the default", getList) + } + if len(warnings) != 1 { + t.Fatalf("got %d warnings, want 1: %+v", len(warnings), warnings) + } + w := warnings[0] + if w.Severity != SeverityWarning || w.RuleID != "OPT001" { + t.Errorf("warning = %+v, want a warning for OPT001", w) + } + for _, want := range []string{"rules.OPT001.options.xs", "list of strings or a comma-separated string", "default"} { + if !strings.Contains(w.Message, want) { + t.Errorf("warning %q does not mention %q", w.Message, want) + } + } + }) + } +} + +// A warning belongs to the run that read the option: a second run with a +// fixed config must not repeat it. +func TestGetListOption_WarningDoesNotLeakIntoNextRun(t *testing.T) { + r := loadRuleSource(t, "echo.star", optionEchoRule) + r.Configure(map[string]any{"xs": 5}) + _ = r.Check(&LintContext{}) + r.Configure(map[string]any{"xs": "A_"}) + for _, v := range r.Check(&LintContext{}) { + if v.RuleFailure { + t.Errorf("stale warning: %+v", v) + } + } +} + +// The options map is what yaml.v3 makes of lint-config.yaml; this pins that a +// flow-style and a block-style list both arrive in the shape get_list_option +// reads. +func TestGetListOption_FromLintConfigYAML(t *testing.T) { + dir := t.TempDir() + write(t, dir, "lint-config.yaml", `rules: + OPT001: + options: + xs: [ACT_, TEST_] + OPT002: + options: + xs: + - GET_ +`) + cfg, err := LoadConfig(filepath.Join(dir, "lint-config.yaml")) + if err != nil { + t.Fatal(err) + } + _, got, warnings := runEcho(t, cfg.Rules["OPT001"].Options) + if got != `["ACT_", "TEST_"]` || len(warnings) != 0 { + t.Errorf("flow list: get_list_option = %s, warnings %+v", got, warnings) + } + _, got, warnings = runEcho(t, cfg.Rules["OPT002"].Options) + if got != `["GET_"]` || len(warnings) != 0 { + t.Errorf("block list: get_list_option = %s, warnings %+v", got, warnings) + } +} diff --git a/mdl/linter/starlark_prefix_options_test.go b/mdl/linter/starlark_prefix_options_test.go new file mode 100644 index 0000000000..4e28d77abb --- /dev/null +++ b/mdl/linter/starlark_prefix_options_test.go @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: Apache-2.0 + +package linter_test + +import ( + "path/filepath" + "sort" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/linter" +) + +// mendixlabs/mxcli#1509: QUAL004's entry-point prefixes and CUSTOM002's valid +// prefixes were constants in the rule files, and `mxcli init` rewrites those +// files, so a project with its own naming convention could not keep a change. +// Both now read a rule option from lint-config.yaml and fall back to the old +// lists when it is unset. + +// flaggedFlows runs a shipped rule over the flow-kinds fixture (Big_ and +// ACT_ for each flavour, all unreferenced) and returns the flagged names +// and the rule-failure messages. +func flaggedFlows(t *testing.T, rule string, opts map[string]any) (names []string, failures []string) { + t.Helper() + r, err := linter.LoadStarlarkRule(filepath.Join(flowRulesDir, rule)) + if err != nil { + t.Fatalf("LoadStarlarkRule(%s): %v", rule, err) + } + if opts != nil { + r.Configure(opts) + } + for _, v := range r.Check(linter.NewLintContextFromDB(flowKindsFixtureDB(t))) { + if v.RuleFailure { + failures = append(failures, v.Message) + continue + } + names = append(names, v.Location.DocumentName) + } + sort.Strings(names) + return names, failures +} + +func TestQUAL004_EntryPointPrefixesOption(t *testing.T) { + const rule = "orphaned_elements.star" + defaults := []string{"Sales.Big_MF", "Sales.Big_NF", "Sales.Big_RU"} + bigOnly := []string{"Sales.ACT_MF", "Sales.ACT_NF", "Sales.ACT_RU"} + + for _, tc := range []struct { + name string + opts map[string]any + want []string + wantFailures int + }{ + // Control: unset, the built-in list (ACT_ among it) still applies. + {"unset keeps the built-in list", nil, defaults, 0}, + {"yaml list replaces the list", map[string]any{"entry_point_prefixes": []any{"Big_"}}, bigOnly, 0}, + {"comma-separated string", map[string]any{"entry_point_prefixes": "Big_, ACT_"}, nil, 0}, + {"malformed value warns, keeps the built-in list", map[string]any{"entry_point_prefixes": 3}, defaults, 1}, + } { + t.Run(tc.name, func(t *testing.T) { + got, failures := flaggedFlows(t, rule, tc.opts) + if strings.Join(got, ",") != strings.Join(tc.want, ",") { + t.Errorf("flagged %v, want %v", got, tc.want) + } + if len(failures) != tc.wantFailures { + t.Errorf("rule failures %v, want %d", failures, tc.wantFailures) + } + }) + } +} + +func TestCUSTOM002_ValidPrefixesOption(t *testing.T) { + const rule = "example_microflow.star" + defaults := []string{"Sales.Big_MF", "Sales.Big_NF", "Sales.Big_RU"} + + got, _ := flaggedFlows(t, rule, nil) + if strings.Join(got, ",") != strings.Join(defaults, ",") { + t.Fatalf("control: flagged %v, want %v", got, defaults) + } + + got, failures := flaggedFlows(t, rule, map[string]any{"valid_prefixes": []any{"ACT_", "Big_"}}) + if len(got) != 0 || len(failures) != 0 { + t.Errorf("with valid_prefixes [ACT_, Big_]: flagged %v, failures %v, want none", got, failures) + } + + // The message names the prefixes in force, not the built-in ones. + r, err := linter.LoadStarlarkRule(filepath.Join(flowRulesDir, rule)) + if err != nil { + t.Fatal(err) + } + r.Configure(map[string]any{"valid_prefixes": "TEST_,GET_"}) + vs := r.Check(linter.NewLintContextFromDB(flowKindsFixtureDB(t))) + if len(vs) != 6 { + t.Fatalf("got %d violations, want all 6 fixture flows", len(vs)) + } + if !strings.Contains(vs[0].Message, "(TEST_, GET_)") { + t.Errorf("message %q does not name the configured prefixes", vs[0].Message) + } +}