cue/parser: restrict else to single clause only

+23 -12

cue/parser/parser.go

··· 866 866 expr := p.parseStruct() 867 867 sc.closeExpr(p, expr) 868 868 869 - // Determine if this comprehension starts with a for clause 870 - hasForClause := tok == token.FOR 871 - 872 869 var fallbackClause *ast.FallbackClause 873 870 if p.tok == token.ELSE || p.tok == token.FALLBACK { 874 - fallbackClause = p.parseFallbackClause(hasForClause) 871 + fallbackClause = p.parseFallbackClause(clauses) 875 872 } 876 873 877 874 if p.atComma("struct literal", token.RBRACE) { // TODO: may be EOF ··· 1240 1237 } 1241 1238 1242 1239 // parseFallbackClause parses an else or fallback clause in a comprehension. 1243 - // It accepts the appropriate keyword based on hasForClause: 1244 - // - hasForClause=true: expects FALLBACK, errors on ELSE 1245 - // - hasForClause=false: expects ELSE, errors on FALLBACK 1246 - func (p *parser) parseFallbackClause(hasForClause bool) *ast.FallbackClause { 1240 + // It determines the appropriate keyword based on whether clauses contains a ForClause: 1241 + // - Contains ForClause: expects FALLBACK, errors on ELSE 1242 + // - No ForClause: expects ELSE, errors on FALLBACK 1243 + func (p *parser) parseFallbackClause(clauses []ast.Clause) *ast.FallbackClause { 1247 1244 if p.trace { 1248 1245 defer un(trace(p, "FallbackClause")) 1249 1246 } ··· 1253 1250 p.errf(p.pos, "%s requires @experiment(try)", p.tok) 1254 1251 } 1255 1252 1253 + // Determine clause composition 1254 + hasForClause := false 1255 + for _, clause := range clauses { 1256 + switch clause.(type) { 1257 + case *ast.ForClause: 1258 + hasForClause = true 1259 + } 1260 + } 1261 + 1256 1262 var pos token.Pos 1257 1263 if hasForClause { 1264 + // Has for clause: must use fallback 1258 1265 if p.tok == token.ELSE { 1259 1266 p.errf(p.pos, "use 'fallback' with 'for' clauses") 1260 1267 } 1261 1268 pos = p.expect(token.FALLBACK) 1269 + } else if len(clauses) > 1 { 1270 + // Multiple guard clauses: disallow else (ambiguous) 1271 + if p.tok == token.ELSE { 1272 + p.errf(p.pos, "else clause only allowed with single 'if' or 'try' clause") 1273 + } 1274 + pos = p.expect(token.ELSE) // Will fail, but consume token 1262 1275 } else { 1276 + // Single guard clause: use else 1263 1277 if p.tok == token.FALLBACK { 1264 1278 p.errf(p.pos, "use 'else' with 'if' clauses") 1265 1279 } ··· 1393 1407 expr := p.parseStruct() 1394 1408 sc.closeExpr(p, expr) 1395 1409 1396 - // Determine if this comprehension starts with a for clause 1397 - hasForClause := tok == token.FOR 1398 - 1399 1410 var fallbackClause *ast.FallbackClause 1400 1411 if p.tok == token.ELSE || p.tok == token.FALLBACK { 1401 - fallbackClause = p.parseFallbackClause(hasForClause) 1412 + fallbackClause = p.parseFallbackClause(clauses) 1402 1413 } 1403 1414 1404 1415 if p.atComma("list literal", token.RBRACK) { // TODO: may be EOF

+96

cue/parser/parser_test.go

··· 694 694 fallback requires @experiment(try)`, 695 695 }, 696 696 { 697 + desc: "else with multiple if clauses error", 698 + version: "v0.16.0", 699 + in: `@experiment(try) 700 + a: { 701 + if true if false { x: 1 } else { y: 2 } 702 + }`, 703 + out: `@experiment(try), a: {if true if false {x: 1} else {y: 2}} 704 + else clause only allowed with single 'if' or 'try' clause`, 705 + }, 706 + { 707 + desc: "else with chained try error", 708 + version: "v0.16.0", 709 + in: `@experiment(try) 710 + a: { 711 + try x = val? try y = val2? { result: x } else { fallback: 0 } 712 + }`, 713 + out: `@experiment(try), a: {try x = val? try y = val2? {result: x} else {fallback: 0}} 714 + else clause only allowed with single 'if' or 'try' clause`, 715 + }, 716 + { 717 + desc: "else with if and let error", 718 + version: "v0.16.0", 719 + in: `@experiment(try) 720 + a: { 721 + if true let x = 5 { y: x } else { z: 0 } 722 + }`, 723 + out: `@experiment(try), a: {if true let x=5 {y: x} else {z: 0}} 724 + else clause only allowed with single 'if' or 'try' clause`, 725 + }, 726 + { 727 + desc: "else with if and try error", 728 + version: "v0.16.0", 729 + in: `@experiment(try) 730 + a: { 731 + if true try { x: val? } else { y: 0 } 732 + }`, 733 + out: `@experiment(try), a: {if true try {x: val?} else {y: 0}} 734 + else clause only allowed with single 'if' or 'try' clause`, 735 + }, 736 + { 737 + desc: "else with for clause error", 738 + version: "v0.16.0", 739 + in: `@experiment(try) 740 + a: { 741 + if true for x in [] { y: x } else { empty: true } 742 + }`, 743 + out: `@experiment(try), a: {if true for x in [] {y: x} else {empty: true}} 744 + expected 'fallback', found 'else'`, 745 + }, 746 + { 747 + desc: "fallback without for clause error", 748 + version: "v0.16.0", 749 + in: `@experiment(try) 750 + a: { 751 + if true { x: 1 } fallback { y: 2 } 752 + }`, 753 + out: `@experiment(try), a: {if true {x: 1} else {y: 2}} 754 + expected 'else', found 'fallback'`, 755 + }, 756 + { 757 + desc: "single if with else allowed", 758 + version: "v0.16.0", 759 + in: `@experiment(try) 760 + a: { 761 + if true { x: 1 } else { y: 2 } 762 + }`, 763 + out: `@experiment(try), a: {if true {x: 1} else {y: 2}}`, 764 + }, 765 + { 766 + desc: "single try with else allowed", 767 + version: "v0.16.0", 768 + in: `@experiment(try) 769 + a: { 770 + try { x: val? } else { y: 0 } 771 + }`, 772 + out: `@experiment(try), a: {try {x: val?} else {y: 0}}`, 773 + }, 774 + { 775 + desc: "for with fallback allowed", 776 + version: "v0.16.0", 777 + in: `@experiment(try) 778 + a: { 779 + for x in [] { y: x } fallback { empty: true } 780 + }`, 781 + out: `@experiment(try), a: {for x in [] {y: x} fallback {empty: true}}`, 782 + }, 783 + { 784 + desc: "for with if and fallback allowed", 785 + version: "v0.16.0", 786 + in: `@experiment(try) 787 + a: { 788 + for x in list if x > 0 { y: x } fallback { empty: true } 789 + }`, 790 + out: `@experiment(try), a: {for x in list if x>0 {y: x} fallback {empty: true}}`, 791 + }, 792 + { 697 793 desc: "let declaration", 698 794 in: `{ 699 795 let X = 42

+52

cue/testdata/comprehensions/else.txtar

··· 31 31 for x in [1, 2, 3] if x > 1 { "\(x)": x } fallback { empty: true } 32 32 } 33 33 34 + // if-for-fallback: if true, for empty - fallback yields 35 + ifTrueForEmpty: { 36 + if true for x in [] { "\(x)": x } fallback { empty: true } 37 + } 38 + 39 + // if-for-fallback: if false, for non-empty - fallback yields (if blocks iteration) 40 + ifFalseForNonEmpty: { 41 + if false for x in [1, 2] { "\(x)": x } fallback { empty: true } 42 + } 43 + 44 + // if-for-fallback: if true, for non-empty - no fallback 45 + ifTrueForNonEmpty: { 46 + if true for x in [1, 2] { "\(x)": x } fallback { empty: true } 47 + } 48 + 34 49 // list if-else: true 35 50 listIfTrue: [ 36 51 if true { 1 } else { 2 } ··· 141 156 "2": (int){ 2 } 142 157 "3": (int){ 3 } 143 158 } 159 + ifTrueForEmpty: (struct){ 160 + empty: (bool){ true } 161 + } 162 + ifFalseForNonEmpty: (struct){ 163 + empty: (bool){ true } 164 + } 165 + ifTrueForNonEmpty: (struct){ 166 + "1": (int){ 1 } 167 + "2": (int){ 2 } 168 + } 144 169 listIfTrue: (#list){ 145 170 0: (int){ 1 } 146 171 } ··· 258 283 2, 259 284 3, 260 285 ] if (〈0;x〉 > 1) { 286 + "\(〈1;x〉)": 〈1;x〉 287 + } else { 288 + empty: true 289 + } 290 + } 291 + ifTrueForEmpty: { 292 + if true for _, x in [] { 293 + "\(〈1;x〉)": 〈1;x〉 294 + } else { 295 + empty: true 296 + } 297 + } 298 + ifFalseForNonEmpty: { 299 + if false for _, x in [ 300 + 1, 301 + 2, 302 + ] { 303 + "\(〈1;x〉)": 〈1;x〉 304 + } else { 305 + empty: true 306 + } 307 + } 308 + ifTrueForNonEmpty: { 309 + if true for _, x in [ 310 + 1, 311 + 2, 312 + ] { 261 313 "\(〈1;x〉)": 〈1;x〉 262 314 } else { 263 315 empty: true

+9

cue/testdata/comprehensions/else_compile_errors.txtar

··· 10 10 letVarInElse: { 11 11 for x in [1,2,3] let y = x*2 { "\(x)": y } fallback { bad: y } 12 12 } 13 + 14 + // Note: Multiple clause cases with else are caught at parse time, not compile time: 15 + // - if + let with else: if cond let x = val { ... } else { ... } 16 + // - Two if clauses with else: if cond1 if cond2 { x } else { y } 17 + // - Chained try with else: try x = a? try y = b? { ... } else { ... } 18 + // - Mixed if/try with else: if cond try { x } else { y } 19 + // - if + for with else: if cond for x in [] { x } else { y } 20 + // These all produce parser error: "else clause only allowed with single 'if' or 'try' clause" 21 + // or "use 'fallback' with 'for' clauses" 13 22 -- out/eval/stats -- 14 23 Leaks: 0 15 24 Freed: 2

+10 -14

cue/testdata/comprehensions/try.txtar

··· 427 427 428 428 // Multiple chained try assignments with one failing 429 429 chainedOneFails: { 430 - try x = a? try y = c? { result: x + y } else { fallback: "failed" } 430 + try x = a? try y = c? { result: x + y } 431 431 } 432 432 433 433 // Assignment with real error - should propagate ··· 461 461 462 462 -- out/evalalpha/stats -- 463 463 Leaks: 53 464 - Freed: 355 465 - Reused: 308 464 + Freed: 354 465 + Reused: 307 466 466 Allocs: 100 467 467 Retain: 0 468 468 469 - Unifications: 401 470 - Conjuncts: 509 469 + Unifications: 400 470 + Conjuncts: 507 471 471 Disjuncts: 0 472 472 473 473 NumCloseIDs: 173 ··· 481 481 -Reused: 5 482 482 -Allocs: 3 483 483 +Leaks: 53 484 - +Freed: 355 485 - +Reused: 308 484 + +Freed: 354 485 + +Reused: 307 486 486 +Allocs: 100 487 487 Retain: 0 488 488 ··· 491 491 -Disjuncts: 8 492 492 - 493 493 -NumCloseIDs: 1 494 - +Unifications: 401 495 - +Conjuncts: 509 494 + +Unifications: 400 495 + +Conjuncts: 507 496 496 +Disjuncts: 0 497 497 + 498 498 +NumCloseIDs: 173 ··· 870 870 } 871 871 } 872 872 chainedOneFails: (struct){ 873 - fallback: (string){ "failed" } 874 873 } 875 874 withRealError: (_|_){ 876 875 // [eval] ··· 946 945 } 947 946 single: (struct){ 948 947 x: (int){ 5 } 949 - @@ -9,7 +41,350 @@ 948 + @@ -9,7 +41,349 @@ 950 949 y: (int){ 10 } 951 950 } 952 951 fallbackDef: (int){ 42 } ··· 1269 1268 + } 1270 1269 + } 1271 1270 + chainedOneFails: (struct){ 1272 - + fallback: (string){ "failed" } 1273 1271 + } 1274 1272 + withRealError: (_|_){ 1275 1273 + // [eval] ··· 1801 1799 chainedOneFails: { 1802 1800 try x = 〈2;a〉? try y = 〈3;c〉? { 1803 1801 result: (〈2;x〉 + 〈1;y〉) 1804 - } else { 1805 - fallback: "failed" 1806 1802 } 1807 1803 } 1808 1804 withRealError: {

+52

doc/specs/comprehension-else/spec.md

··· 67 67 - **WHEN** the input is `try { a: x? } fallback { b: 2 }` 68 68 - **THEN** the parser SHALL report an error: "use 'else' with 'try' clauses" 69 69 70 + ### Requirement: Else and fallback clause restrictions 71 + 72 + To avoid ambiguous semantics, the system SHALL enforce the following restrictions: 73 + - `else` is only allowed with a single `if` clause or single `try` clause 74 + - `fallback` requires at least one `for` clause to be present in the comprehension 75 + 76 + **Rationale**: With multiple clauses, it becomes ambiguous which clause the else applies to. Restricting `else` to a single guard clause and requiring `for` for `fallback` keeps the semantics clear and predictable. 77 + 78 + #### Scenario: else with single if clause accepted 79 + - **WHEN** the input is `if enabled { a: 1 } else { b: 2 }` 80 + - **THEN** the parser SHALL accept this as valid 81 + 82 + #### Scenario: else with single try clause accepted 83 + - **WHEN** the input is `try { a: x? } else { b: 2 }` 84 + - **THEN** the parser SHALL accept this as valid 85 + 86 + #### Scenario: else with two if clauses rejected 87 + - **WHEN** the input is `if cond1 if cond2 { a: 1 } else { b: 2 }` 88 + - **THEN** the parser SHALL report an error: "else clause only allowed with single 'if' or 'try' clause" 89 + 90 + #### Scenario: else with chained try clauses rejected 91 + - **WHEN** the input is `try x = a? try y = b? { result: x + y } else { fallback: 0 }` 92 + - **THEN** the parser SHALL report an error: "else clause only allowed with single 'if' or 'try' clause" 93 + 94 + #### Scenario: else with mixed if and try rejected 95 + - **WHEN** the input is `try x = a? if x > 0 { result: x } else { zero: true }` 96 + - **THEN** the parser SHALL report an error: "else clause only allowed with single 'if' or 'try' clause" 97 + 98 + #### Scenario: else with if followed by for rejected 99 + - **WHEN** the input is `if true for x in {} { x } else { a: 1 }` 100 + - **THEN** the parser SHALL report an error indicating else not allowed with for clauses 101 + 102 + #### Scenario: else with try followed by for rejected 103 + - **WHEN** the input is `try x = a? for y in [x] { y } else { empty: true }` 104 + - **THEN** the parser SHALL report an error indicating else not allowed with for clauses; use fallback instead 105 + 106 + #### Scenario: else with if and let rejected 107 + - **WHEN** the input is `if enabled let x = value { a: x } else { b: 2 }` 108 + - **THEN** the parser SHALL report an error: "else clause not allowed with let clauses" 109 + 110 + #### Scenario: fallback with for clause accepted 111 + - **WHEN** the input is `for x in list { x } fallback { empty: true }` 112 + - **THEN** the parser SHALL accept this as valid 113 + 114 + #### Scenario: fallback with for and if clauses accepted 115 + - **WHEN** the input is `for x in list if x > 0 { x } fallback { empty: true }` 116 + - **THEN** the parser SHALL accept this as valid (fallback applies to the for iteration result) 117 + 118 + #### Scenario: fallback with for and let clauses accepted 119 + - **WHEN** the input is `for x in list let y = x * 2 { y } fallback { empty: true }` 120 + - **THEN** the parser SHALL accept this as valid 121 + 70 122 ### Requirement: Single else or fallback clause limit 71 123 72 124 A comprehension SHALL have at most one else or fallback clause. If multiple such clauses are present, the parser SHALL report an error.

+17 -1

internal/core/compile/compile.go

··· 1042 1042 // Compile fallback clause in the outer scope (before comprehension variables). 1043 1043 // The fallback clause should NOT have access to for/let variables. 1044 1044 if x.Fallback != nil { 1045 + // Validate else/fallback restrictions for programmatically created ASTs. 1046 + // The parser enforces these rules, but users can bypass the parser by 1047 + // creating AST nodes programmatically, so we validate here as well. 1048 + hasFor := false 1049 + for _, clause := range x.Clauses { 1050 + if _, ok := clause.(*ast.ForClause); ok { 1051 + hasFor = true 1052 + break 1053 + } 1054 + } 1045 1055 // Both 'else' (with if) and 'fallback' (with for) require the try experiment. 1046 1056 if !c.experiments.Try { 1047 1057 // Use appropriate keyword in error message based on clause type. 1048 1058 keyword := "else" 1049 - if _, isFor := x.Clauses[0].(*ast.ForClause); isFor { 1059 + if hasFor { 1050 1060 keyword = "fallback" 1051 1061 } 1052 1062 return c.errf(x.Fallback, "%s clause requires the try experiment (language version v0.16.0 or later)", keyword) 1053 1063 } 1064 + if !hasFor && len(x.Clauses) > 1 { 1065 + // Can only trigger for programmatically created ASTs, as the parser 1066 + // enforces this rule. 1067 + return c.errf(x.Fallback, "invalid use of else or fallback") 1068 + } 1069 + 1054 1070 // Pop all comprehension scopes temporarily to compile fallback in outer scope. 1055 1071 // We need to compile the fallback body outside the comprehension's scope chain. 1056 1072 savedStack := c.stack

Configure Feed

Configure Feed