Skip to content

feat(lint): add rest_clients() and rest_operations() Starlark builtins - #1130

Draft
jvegmond-tech wants to merge 1 commit into
mendixlabs:mainfrom
jvegmond-tech:feat/lint-rest-accessors
Draft

jvegmond-tech wants to merge 1 commit into
mendixlabs:mainfrom
jvegmond-tech:feat/lint-rest-accessors

Conversation

@jvegmond-tech

Copy link
Copy Markdown

What
Adds two query functions to the Starlark lint API:
rest_clients() — consumed REST service documents
rest_operations() — their operations, including the Timeout the catalog already stores

Why
We're porting a set of CLEVR ACR quality rules to mxcli custom rules. Most map across cleanly, but this one can't be expressed at all today:
REST/Web services should have a timeout — not using the timeout mechanism when integrating with another system can cause your users to wait extremely long, especially when integrating with multiple services sequentially in one user request.
The data is already there. mdl/catalog/tables.go builds both rest_clientsand rest_operations, and rest_operations carries a Timeout column. There was simply no accessor, so a Starlark rule couldn't reach any of it.
Implementation
Follows the existing DatabaseConnections pattern exactly:
catalog-backed, so no LintReader change is needed
platform modules filtered via notPlatformModule(...)
ctx.IsExcluded(...) honoured
failures routed through recordQueryError(...)
Docs updated in .claude/skills/mendix/write-lint-rules/SKILL.md: the query function table plus property tables for the two new struct types.
What this enables

python

RULE_ID = "CUSTOM010"RULE_NAME = "RestOperationNoTimeout"DESCRIPTION = "Consumed REST operations should have a timeout"CATEGORY = "reliability"SEVERITY = "warning" def check(): return [violation( message = "REST operation '{}.{}' has no timeout".format( op.service_qualified_name, op.name), location = location(module = op.module_name, document_type = "rest_client", document_name = op.service_qualified_name), suggestion = "Set a timeout on this operation.", ) for op in rest_operations() if op.timeout == 0]

Testing
Run in the mxcli dev container:
make build — passes
go test ./mdl/linter/... — passes (mdl/linter and mdl/linter/rules)

Known limitation — worth a reviewer's eye
The project I tested against had no consumed REST service documents, so the accessors compile and the tests pass, but they have not yet returned a live row. A check against a project that has one would be worth doing before merge.
Related observation: that same project did contain a RestCallAction in a microflow while rest_operations stayed empty. If REST calls configured inline on the activity aren't represented in that table, then this PR alone won't fully cover the ACR rule above — the activity's own timeout would also need exposing, on top of the service_ref / action_ref change in my other PR. Happy to follow up on that separately if you can confirm how inline REST calls are meant to be indexed.

The catalog already builds both tables, and rest_operations carries a
Timeout column, but neither was reachable from a Starlark rule. This adds
the two accessors following the DatabaseConnections pattern: catalog
backed, platform modules filtered via notPlatformModule, excluded modules
honoured.

Motivating rule: 'REST/web services should have a timeout', which until
now could not be expressed as a custom rule at all.
@github-actions

Copy link
Copy Markdown

AI Code Review

Critical Issues

None found.

Moderate Issues

None found.

Minor Issues

  • Testing limitation acknowledged: The author notes they tested against a project with no consumed REST service documents, so while the code compiles and tests pass, it hasn't been verified to return live data. This is noted as a known limitation worth checking, but given the implementation follows the existing DatabaseConnections() pattern exactly and the catalog tables are already built by mdl/catalog/tables.go, this is low risk.
  • Related observation about inline REST calls: The author notes that REST calls configured inline on activities (like RestCallAction in microflows) aren't represented in the rest_operations table, so this PR alone won't fully cover the ACR rule about timeouts. They correctly state this would need a separate change to expose activity-level timeouts, and are willing to follow up if confirmed how inline REST calls should be indexed. This is out of scope for this PR but worth keeping in mind for future work.

What Looks Good

  • Pattern compliance: The implementation exactly follows the established DatabaseConnections() pattern - catalog-backed, uses notPlatformModule(), honors ctx.IsExcluded(), routes failures through recordQueryError(), and maintains identical structure and error handling.
  • Documentation: The .claude/skills/mendix/write-lint-rules/SKILL.md file is properly updated with the new query functions and their property tables.
  • Code quality: Clean, readable code that maintains consistency with existing linter builtins. Proper handling of nullable database columns via sql.NullString. Correct Starlark struct conversions.
  • Atomic scope: The PR is tightly scoped to adding just these two Starlark builtins with no extraneous changes.
  • Test confirmation: The author confirms make build and go test ./mdl/linter/... pass.

Recommendation

Approve the PR. The implementation correctly adds the requested Starlark lint API functions by following the existing pattern exactly, with proper error handling, documentation updates, and test coverage. The minor testing limitation is acknowledged and acceptable given the pattern-based implementation. The inline REST call observation is out of scope and appropriately noted for potential follow-up. No changes are needed before merge.


Automated review via OpenRouter (Nemotron Super 120B) — workflow source

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants