diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e612dc6..4454a133 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,23 @@ version number before tagging the release. ### Fixed +- **E0200 "block setter called as a node builder" rejected every container + of a widget DSL that also declares a `builder`.** The rule from 0.667 + used "the callee's module also defines a `builder`" as its whole + discriminator between a block setter and a DSL container, on the premise + that a widget-style module has no builders. aether-ui's `ui` module has + four (`window`, `render_to`, `record`, `nav_page`) beside some fifty + `_ctx`-first containers, so `ui.vstack(10) { ... }` — every app in that + repository — became a compile error the moment it moved past 0.666 + (aether-ui#147). The rule now also requires that the callee yield no + value: a container RETURNS the handle its block runs inside (that value + is the block's `_ctx`), while a block setter records config and returns + nothing, which is exactly why its own trailing block can never be + entered. aeb's setters are all void, so the misuse it was written for + is still caught. `tests/integration/setter_in_builder_position` gains + `uimod` (a builder beside handle-returning containers and one void + setter): the container compiles, the setter still errors. + - **`os.run_full` and `os.run_capture` are now tested on Windows.** Both have had CreateProcessW backends for some time, but their regression tests still skipped Windows with comments calling them POSIX-only, so diff --git a/compiler/analysis/typechecker.c b/compiler/analysis/typechecker.c index 642cd7ed..1e596c34 100644 --- a/compiler/analysis/typechecker.c +++ b/compiler/analysis/typechecker.c @@ -1385,6 +1385,7 @@ static int is_const_array_element(ASTNode* elem, SymbolTable* table) { /* Defined below, beside typecheck_function_call — used by the * variable-declaration arm, which appears earlier in this file. */ static int call_yields_no_value(ASTNode* call, SymbolTable* table); +static int fn_yields_no_value(ASTNode* fn); // Type compatibility functions /* A `fn` with no signature: nothing known about parameters or result. */ @@ -8233,11 +8234,8 @@ static int tc_has_return_value(ASTNode* node) { * Returns 1 when `call` names a user-defined function that lowers to void. * Externs are excluded: their declared type is the only truth available, and * an `extern f() -> int` may well front a real int-returning C function. */ -static int call_yields_no_value(ASTNode* call, SymbolTable* table) { - if (!call || call->type != AST_FUNCTION_CALL || !call->value) return 0; - Symbol* sym = lookup_symbol(table, call->value); - if (!sym || !sym->node) return 0; - ASTNode* fn = sym->node; +static int fn_yields_no_value(ASTNode* fn) { + if (!fn) return 0; if (fn->type != AST_FUNCTION_DEFINITION && fn->type != AST_BUILDER_FUNCTION) return 0; /* An annotated return type is authoritative, whatever the body does. */ @@ -8249,6 +8247,13 @@ static int call_yields_no_value(ASTNode* call, SymbolTable* table) { return !tc_has_return_value(fn); } +static int call_yields_no_value(ASTNode* call, SymbolTable* table) { + if (!call || call->type != AST_FUNCTION_CALL || !call->value) return 0; + Symbol* sym = lookup_symbol(table, call->value); + if (!sym || !sym->node) return 0; + return fn_yields_no_value(sym->node); +} + int typecheck_function_call(ASTNode* call, SymbolTable* table) { if (!call || call->type != AST_FUNCTION_CALL) return 0; @@ -8577,14 +8582,28 @@ int typecheck_function_call(ASTNode* call, SymbolTable* table) { * 1. the callee is a plain AST_FUNCTION_DEFINITION (not a builder), * whose first param is `_ctx: ptr` (has_ctx_first_param); * 2. this call carries its OWN trailing block (an AST_CLOSURE - * argument valued "trailing"); and - * 3. the callee's module ALSO defines at least one `builder`. - * Condition 3 is the discriminator: a widget-style DSL module has no - * builders and is never flagged; only a builder-DSL module — where a - * `_ctx`-first plain function IS a block setter — is. */ + * argument valued "trailing"); + * 3. the callee's module ALSO defines at least one `builder`; and + * 4. the callee yields NO value (fn_yields_no_value on the + * resolved definition — `symbol` already went through the + * qualified lookup, which a fresh lookup by `mod.name` would not). + * Condition 3 was the original discriminator: a widget-style DSL + * module has no builders and is never flagged. It is not enough on + * its own. A widget DSL can carry a few builders alongside dozens of + * `_ctx`-first containers — aether-ui's `ui` module has `builder + * window(...)` next to `vstack(_ctx, spacing)` — and under 3 alone + * every `ui.vstack(10) { ... }` in every app was rejected as a + * setter (aether-ui#147, the 0.667 pin bump). Condition 4 is the + * semantic difference: a container RETURNS the handle its block runs + * inside (that value is the block's `_ctx`), while a block setter + * records config and returns nothing — which is exactly why its own + * trailing block can never be entered. A void `_ctx`-first function + * with a trailing block in a builder module is the misuse; one that + * returns a value is a container whatever its module also declares. */ if (symbol->node->type == AST_FUNCTION_DEFINITION && has_ctx_first_param(symbol->node) && - module_defines_a_builder(symbol->node)) { + module_defines_a_builder(symbol->node) && + fn_yields_no_value(symbol->node)) { for (int i = 0; i < call->child_count; i++) { ASTNode* c = call->children[i]; if (c && c->type == AST_CLOSURE && c->value && diff --git a/tests/integration/setter_in_builder_position/legit_c.ae b/tests/integration/setter_in_builder_position/legit_c.ae new file mode 100644 index 00000000..bbefcdfa --- /dev/null +++ b/tests/integration/setter_in_builder_position/legit_c.ae @@ -0,0 +1,17 @@ +// LEGIT C (must COMPILE, NO false positive): a widget-style DSL container in +// a module that ALSO has a builder. +// +// `vstack` is `_ctx`-first and takes a trailing block, and its module defines +// `builder window` — conditions 1-3 all hold, which is why this was rejected +// before condition 4. It RETURNS the container handle its block runs inside, +// so it is a container, not a setter. This is every aether-ui app. + +import uimod + +main() { + uimod.window("app") { + vstack(10) { + label("hello") + } + } +} diff --git a/tests/integration/setter_in_builder_position/misuse_b.ae b/tests/integration/setter_in_builder_position/misuse_b.ae new file mode 100644 index 00000000..7dc79fd2 --- /dev/null +++ b/tests/integration/setter_in_builder_position/misuse_b.ae @@ -0,0 +1,11 @@ +// MISUSE B (must ERROR): the void block setter of a widget module, called as +// a node with its own trailing block. Condition 4 must not let a genuine +// setter through just because its module also has containers. + +import uimod + +main() { + uimod.font_size(14) { + label("never attached") + } +} diff --git a/tests/integration/setter_in_builder_position/test_setter_in_builder_position.sh b/tests/integration/setter_in_builder_position/test_setter_in_builder_position.sh index 4e7ddf5e..dd268eac 100755 --- a/tests/integration/setter_in_builder_position/test_setter_in_builder_position.sh +++ b/tests/integration/setter_in_builder_position/test_setter_in_builder_position.sh @@ -12,9 +12,13 @@ # # The check is deliberately NARROW to stay false-positive-free. It fires # ONLY when the callee is a plain `_ctx`-first function, the call carries -# its own trailing block, AND the callee's module also defines a `builder`. -# A widget-style DSL container (`panel(_ctx, title) { button() }`) in a -# module with NO builders is structurally identical but must NOT be flagged. +# its own trailing block, the callee's module also defines a `builder`, +# AND the callee yields no value. A widget-style DSL container +# (`panel(_ctx, title) { button() }`) in a module with NO builders is +# structurally identical but must NOT be flagged — and neither is one in a +# module WITH builders when it returns the handle its block runs inside +# (aether-ui's `ui` module: `builder window` beside `vstack(_ctx, spacing)`, +# which rejected every app under the first cut of this rule). SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" ROOT="$(cd "$SCRIPT_DIR/../../.." && pwd)" @@ -86,5 +90,7 @@ expect_ok() { expect_error "$SCRIPT_DIR/misuse.ae" "MISUSE: setter called as top-level node builder must error" expect_ok "$SCRIPT_DIR/legit_a.ae" "LEGIT A: same setter called inside the builder's block compiles" expect_ok "$SCRIPT_DIR/legit_b.ae" "LEGIT B: widget-style DSL container (no builders) not flagged" +expect_ok "$SCRIPT_DIR/legit_c.ae" "LEGIT C: handle-returning container in a module WITH a builder not flagged" +expect_error "$SCRIPT_DIR/misuse_b.ae" "MISUSE B: void setter of that same widget module still errors" exit $fail diff --git a/tests/integration/setter_in_builder_position/uimod/module.ae b/tests/integration/setter_in_builder_position/uimod/module.ae new file mode 100644 index 00000000..1898873d --- /dev/null +++ b/tests/integration/setter_in_builder_position/uimod/module.ae @@ -0,0 +1,32 @@ +// Widget-style DSL module that ALSO defines a builder — aether-ui's shape. +// +// `window` is a real `builder`. `vstack` is a `_ctx`-first plain function +// that LEGITIMATELY takes a trailing block: it creates a container and +// RETURNS its handle, which is the `_ctx` the block's children attach to. +// Under conditions 1-3 alone this module looks exactly like rubymod, and +// `uimod.vstack(10) { ... }` was rejected as a misused setter. Condition 4 +// tells them apart: a container yields a value, a block setter yields none. +// +// `font_size` is the module's genuine block setter: `_ctx`-first, void, +// records config on the container. Called with a trailing block of its own +// it must still be flagged, builder-module or not. + +exports(window, vstack, label, font_size) + +builder window(title: string) { + println("window ${title}") +} + +vstack(_ctx: ptr, spacing: int) { + println("vstack ${spacing}") + return 42 +} + +label(_ctx: ptr, text: string) { + println("label ${text}") + return 43 +} + +font_size(_ctx: ptr, px: int) { + println("font_size ${px}") +}