Fail soft on malformed ifNonempty expressions

[SemTiOne]

Pull Request

Summary

Closes #557. Both ifNonempty: resolvers (cli/planner.py::resolve_expr and cli/renderer.py::_resolve_expr) now validate the argument count before unpacking: a malformed expression (wrong comma count) returns None, leaving the placeholder unresolved for the existing E071 check. Takes the fail-soft option from the issue, matching how every other expression-parsing path already behaves. Well-formed expressions are untouched (same split(",", 2) semantics, suffix may still contain commas).

Type Of Change

• Bug fix
• Feature
• Refactor
• Docs
• Test only

User Impact

${ifNonempty:config.password} (missing prefix/suffix) in a module template no longer crashes cds validate/test/render with ValueError: not enough values to unpack. It now surfaces as the standard unresolved-expression diagnostic instead.

Validation

List commands run and outcomes.

python -m unittest discover -s tests -p "test_*.py" -v # All passed

Checklist

• Tests added or updated
• Docs updated (README or docs)
• No secrets committed
• Generated artifacts excluded from git

[RonaldHensbergen]

Thanks for the fix — the crash is definitely gone, but I don't think the fail-soft path actually lands where the PR/issue says it does.

The claim: both the PR description ("leaving the placeholder unresolved for the existing E071 check") and issue #557 ("fail closed by returning None/leaving the expression unresolved (so E071 catches it)") say a malformed ifNonempty: expression will surface as E071.

What actually happens: _UNRESOLVED_EXPRESSION_PATTERN in cli/renderer.py is r"\$\{((?:config|bindings|service)\.[^}]*)\}" — it only matches placeholders whose content starts with config./bindings./service.. A leftover ${ifNonempty:config.password} starts with ifNonempty:, so it never matches. Reproduced end-to-end:

from cli.renderer import render_compose

plan = {
    "metadata": {"name": "cds-test"},
    "modules": [{
        "id": "cache",
        "config": {"password": "secret", "port": 6379},
        "service": {"host": "keydb"},
        "implementation": {"kind": "docker-compose", "compose": {"services": {"keydb": {
            "image": "eqalpha/keydb:latest",
            "environment": {"CONN": "redis://${ifNonempty:config.password}${service.host}:${config.port}"},
        }}}},
    }],
}
output, diagnostics = render_compose(plan)
print(diagnostics)  # []  <- no E071, no diagnostic at all
print(output)
# CONN: redis://${ifNonempty:config.password}cache:6379   <- shipped verbatim, broken, silent

So the net effect of this PR is: a malformed ifNonempty: expression no longer crashes, but instead of failing loudly it now silently ships a broken literal string into the rendered docker-compose.yml with zero diagnostics — arguably worse than the crash for anyone who doesn't notice a bad connection string until runtime. This also directly contradicts _check_unresolved_expressions's own docstring ("CDS's own template vocabulary is meant to be fully resolved by render time... instead of silently ship a broken Compose file").

The same gap exists on the planner side too — cli/planner.py has no E071-equivalent check at all for its own resolved Plan output, so a malformed ifNonempty: in a module's provides.contract.spec (e.g. modules/cache/keydb/module.yaml:60's real connectionUri field) will leak the same way into a consuming module's binding.

Suggested fix: extend _UNRESOLVED_EXPRESSION_PATTERN to also match a leftover ifNonempty: prefix (e.g. r"\$\{((?:config|bindings|service)\.[^}]*|ifNonempty:[^}]*)\}"), and add a regression test asserting render_compose() actually reports E071 (not just that _substitute_string() in isolation leaves the placeholder as text) for a malformed ifNonempty: expression. The current tests only assert the isolated resolver output, which is why this gap wasn't caught.

Happy to re-review once this is addressed.