Skip to content

Commit 044dd93

Browse files
erudenkoclaude
andcommitted
fix(codegen): use = instead of := for error prop with named return params
The ? operator generated err := expr() for functions with named return parameters like (err error), causing "no new variables on left side of :=" compile errors. Two root causes fixed: 1. BARE: generateTupleErrorPropBare always emitted := without checking for named return collision. Now detects named error returns via InferEnclosingFunctionNamedErrorReturn() and uses = when matched. 2. STATEMENT: stmt_finder.go only recognized := (DEFINE), not = (ASSIGN), causing result = getData()? to be classified as bare expression and dropping the user's LHS variable. Added ASSIGN token recognition with IsPlainAssign flag propagated through to code generators. - Reproduction test: tests/golden/error_prop_named_return - Multimodel review: Phase A APPROVE (2/2), Phase B APPROVE (2/2) - Session: dev-fix-20260328-184322-392f98a3 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 41f06ea commit 044dd93

6 files changed

Lines changed: 359 additions & 28 deletions

File tree

pkg/ast/stmt_finder.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@ type StmtLocation struct {
4040
LambdaBodyStart int // Start byte position of lambda body
4141
LambdaBodyEnd int // End byte position of lambda body
4242

43+
// IsPlainAssign is true when the LHS was written with = (not :=).
44+
// Used to emit plain assignment (=) instead of short declaration (:=) in generated code,
45+
// which is required when the target variable is a named return parameter.
46+
IsPlainAssign bool
47+
4348
// Source location for LSP (1-indexed, like GuardLocation)
4449
Line int // 1-indexed line number of the statement
4550
Column int // 1-indexed column number
@@ -124,6 +129,19 @@ func FindErrorPropStatements(src []byte) ([]StmtLocation, error) {
124129
}
125130
continue
126131
}
132+
// Check for "ident =" plain assignment (named return params or pre-declared vars)
133+
if i+1 < len(tokens) && tokens[i+1].Kind == tokenizer.ASSIGN {
134+
loc := scanForQuestionMark(tokens, i)
135+
if loc != nil {
136+
loc.Kind = StmtErrorPropAssign
137+
loc.VarName = t.Lit
138+
loc.IsPlainAssign = true
139+
locations = append(locations, *loc)
140+
// Skip past this statement
141+
i = findTokenAtByte(tokens, loc.End)
142+
}
143+
continue
144+
}
127145
// Check for tuple assignment "a, b :=" pattern
128146
// Scan ahead past comma-separated identifiers to find :=
129147
defineIdx, tupleLHS := findDefineAfterTupleLHSWithNames(tokens, i)

pkg/codegen/type_inference.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,49 @@ func findInnermostEnclosingFunc(src []byte, exprPos int) enclosingFuncInfo {
303303
return enclosingFuncInfo{found: true, name: best.name, results: best.results}
304304
}
305305

306+
// InferEnclosingFunctionNamedReturnParams returns the set of named return
307+
// parameter identifiers for the innermost function enclosing exprPos.
308+
// Returns nil if the function has no named returns or cannot be located.
309+
func InferEnclosingFunctionNamedReturnParams(src []byte, exprPos int) map[string]bool {
310+
info := findInnermostEnclosingFunc(src, exprPos)
311+
if !info.found || info.results == nil {
312+
return nil
313+
}
314+
names := make(map[string]bool)
315+
for _, field := range info.results.List {
316+
for _, id := range field.Names {
317+
if id.Name != "" && id.Name != "_" {
318+
names[id.Name] = true
319+
}
320+
}
321+
}
322+
if len(names) == 0 {
323+
return nil
324+
}
325+
return names
326+
}
327+
328+
// InferEnclosingFunctionNamedErrorReturn returns the name of the named return parameter
329+
// whose type is `error`, for the innermost function enclosing exprPos.
330+
// Returns "" if no such named return exists or the function cannot be located.
331+
func InferEnclosingFunctionNamedErrorReturn(src []byte, exprPos int) string {
332+
info := findInnermostEnclosingFunc(src, exprPos)
333+
if !info.found || info.results == nil {
334+
return ""
335+
}
336+
for _, field := range info.results.List {
337+
// Check if this field's type is the built-in "error" interface
338+
if ident, ok := field.Type.(*ast.Ident); ok && ident.Name == "error" {
339+
for _, id := range field.Names {
340+
if id.Name != "" && id.Name != "_" {
341+
return id.Name
342+
}
343+
}
344+
}
345+
}
346+
return ""
347+
}
348+
306349
// InferEnclosingFunctionReturnsResult checks if the enclosing function returns a Result type.
307350
// Returns the Result's T type if it does, empty string otherwise.
308351
// For example, for `func foo() Result[User, error]`, returns "User".

pkg/transpiler/ast_transformer.go

Lines changed: 98 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -707,9 +707,33 @@ func transformErrorPropStatements(src []byte, originalSrc []byte, filename strin
707707
result := src
708708
var lineMappings []sourcemap.LineMapping
709709
var columnMappings []sourcemap.ColumnMapping
710-
// Start counter at len(locations) and decrement, so first statement in source
711-
// gets tmp/err, second gets tmp1/err1, etc. (we process end-to-beginning)
712-
counter := len(locations)
710+
// Count only the locations that generate fresh (counter-named) variables.
711+
// Locations that use named return parameters directly (isPlainAssign with a named error
712+
// return, or bare statements whose errVar is a named return) do NOT consume a counter slot.
713+
// This ensures: first counter-consuming statement in source gets tmp/err (no suffix),
714+
// second gets tmp1/err1, etc.
715+
counterConsuming := 0
716+
for _, loc := range locations {
717+
switch loc.Kind {
718+
case ast.StmtErrorPropAssign, ast.StmtErrorPropLet:
719+
if loc.IsPlainAssign {
720+
// Uses named error return directly — no counter slot needed
721+
if codegen.InferEnclosingFunctionNamedErrorReturn(src, loc.ExprStart) == "" {
722+
counterConsuming++
723+
}
724+
} else {
725+
counterConsuming++
726+
}
727+
case ast.StmtErrorPropBare:
728+
// Uses named error return directly when the function has one — no counter slot needed
729+
if codegen.InferEnclosingFunctionNamedErrorReturn(src, loc.ExprStart) == "" {
730+
counterConsuming++
731+
}
732+
default:
733+
// Return statements and other kinds do not generate new variables
734+
}
735+
}
736+
counter := counterConsuming
713737

714738
// First pass: calculate all deltas to know final positions
715739
// We need to track byte deltas from transforms to calculate correct Go positions
@@ -764,7 +788,7 @@ func transformErrorPropStatements(src []byte, originalSrc []byte, filename strin
764788
if loc.TupleLHS != "" {
765789
varNameOrTuple = loc.TupleLHS
766790
}
767-
generated = generateErrorPropStatementAdvanced(result, exprBytes, loc.ExprStart, varNameOrTuple, returnTypes, &counter, loc.ErrorKind, loc.ErrorContext, loc.LambdaParam, lambdaBody, resolver)
791+
generated = generateErrorPropStatementAdvanced(result, exprBytes, loc.ExprStart, varNameOrTuple, returnTypes, &counter, loc.ErrorKind, loc.ErrorContext, loc.LambdaParam, lambdaBody, resolver, loc.IsPlainAssign)
768792
// Calculate LHS lengths for column mapping
769793
// Dingo: "varName := " or "a, b := " for tuples
770794
dingoLHSLen = len(varNameOrTuple) + 4 // " := " or " = " (always 4 with leading space consideration)
@@ -946,7 +970,7 @@ func transformErrorPropStatements(src []byte, originalSrc []byte, filename strin
946970
// tmp := expr; if tmp.IsErr() { return dgo.Err[T, E](tmp.MustErr()) }; data := tmp.MustOk()
947971
//
948972
// Context and Lambda variants wrap the error appropriately.
949-
func generateErrorPropStatementAdvanced(src []byte, expr []byte, exprPos int, varName string, returnTypes []string, counter *int, errorKind ast.ErrorPropKind, errorContext string, lambdaParam string, lambdaBody []byte, resolver *codegen.TypeResolver) []byte {
973+
func generateErrorPropStatementAdvanced(src []byte, expr []byte, exprPos int, varName string, returnTypes []string, counter *int, errorKind ast.ErrorPropKind, errorContext string, lambdaParam string, lambdaBody []byte, resolver *codegen.TypeResolver, isPlainAssign bool) []byte {
950974
// Check if expression returns a Result type (use resolver for cross-file types)
951975
isResult, exprOkType, exprErrType := codegen.InferExprReturnsResultWithResolver(src, expr, exprPos, resolver)
952976

@@ -955,7 +979,7 @@ func generateErrorPropStatementAdvanced(src []byte, expr []byte, exprPos int, va
955979
}
956980

957981
// Original tuple-based error propagation
958-
return generateTupleErrorPropStatement(expr, varName, returnTypes, counter, errorKind, errorContext, lambdaParam, lambdaBody, src, exprPos)
982+
return generateTupleErrorPropStatement(expr, varName, returnTypes, counter, errorKind, errorContext, lambdaParam, lambdaBody, src, exprPos, isPlainAssign)
959983
}
960984

961985
// generateResultErrorPropStatement generates code for Result[T, E] error propagation.
@@ -1072,35 +1096,63 @@ func generateResultErrorPropStatement(expr []byte, varName string, counter *int,
10721096
// Pattern for Result-returning enclosing function:
10731097
//
10741098
// tmp, err := expr; if err != nil { return dgo.Err[T, E](err) }; data := tmp
1075-
func generateTupleErrorPropStatement(expr []byte, varName string, returnTypes []string, counter *int, errorKind ast.ErrorPropKind, errorContext string, lambdaParam string, lambdaBody []byte, src []byte, exprPos int) []byte {
1099+
func generateTupleErrorPropStatement(expr []byte, varName string, returnTypes []string, counter *int, errorKind ast.ErrorPropKind, errorContext string, lambdaParam string, lambdaBody []byte, src []byte, exprPos int, isPlainAssign bool) []byte {
10761100
var buf bytes.Buffer
10771101

10781102
// Check if varName contains comma - if so, it's a tuple LHS like "a, b"
10791103
isTupleLHS := strings.Contains(varName, ",")
10801104

1105+
// When isPlainAssign is true, the user wrote `varName = expr?` (plain = not :=).
1106+
// This happens when varName is a named return parameter.
1107+
// In that case, use the actual named error return variable and plain = assignment,
1108+
// instead of counter-generated names and :=.
1109+
var namedErrVar string
1110+
if isPlainAssign {
1111+
namedErrVar = codegen.InferEnclosingFunctionNamedErrorReturn(src, exprPos)
1112+
}
1113+
10811114
// Generate unique variable names
10821115
var tmpVar, errVar string
1083-
if *counter == 1 {
1084-
tmpVar = "tmp"
1085-
errVar = "err"
1116+
if namedErrVar != "" {
1117+
// Plain assign with named return: use named error return directly, don't consume counter slot.
1118+
// The counter is only consumed for generated (counter-named) variables to keep numbering stable.
1119+
errVar = namedErrVar
10861120
} else {
1087-
tmpVar = fmt.Sprintf("tmp%d", *counter-1)
1088-
errVar = fmt.Sprintf("err%d", *counter-1)
1121+
if *counter == 1 {
1122+
tmpVar = "tmp"
1123+
errVar = "err"
1124+
} else {
1125+
tmpVar = fmt.Sprintf("tmp%d", *counter-1)
1126+
errVar = fmt.Sprintf("err%d", *counter-1)
1127+
}
1128+
*counter--
10891129
}
1090-
*counter--
10911130

10921131
// Check if enclosing function returns Result[T, E]
10931132
// If so, we need to return dgo.Err[T, E](err) instead of tuple
10941133
enclosingReturnsResult, enclosingOkType, enclosingErrType := codegen.InferEnclosingFunctionResultTypes(src, exprPos)
10951134

1096-
// For tuple LHS, generate: a, b, err := expr
1097-
// For single LHS, generate: tmp, err := expr
1135+
// For tuple LHS, generate: a, b, err := expr (or a, b, err = expr for plain assign)
1136+
// For single LHS with plain assign (named return), generate: varName, err = expr
1137+
// For single LHS with define, generate: tmp, err := expr
10981138
if isTupleLHS {
10991139
// Tuple LHS: fullKey, keyHash, err := util.GeneratePersonalKey()
1140+
declOp := " := "
1141+
if isPlainAssign {
1142+
declOp = " = "
1143+
}
11001144
buf.WriteString(varName)
11011145
buf.WriteString(", ")
11021146
buf.WriteString(errVar)
1103-
buf.WriteString(" := ")
1147+
buf.WriteString(declOp)
1148+
buf.Write(expr)
1149+
buf.WriteByte('\n')
1150+
} else if isPlainAssign && namedErrVar != "" {
1151+
// Single LHS with named return: result, err = expr
1152+
buf.WriteString(varName)
1153+
buf.WriteString(", ")
1154+
buf.WriteString(errVar)
1155+
buf.WriteString(" = ")
11041156
buf.Write(expr)
11051157
buf.WriteByte('\n')
11061158
} else {
@@ -1177,18 +1229,22 @@ func generateTupleErrorPropStatement(expr []byte, varName string, returnTypes []
11771229
}
11781230
}
11791231

1180-
buf.WriteString("\n}\n")
1181-
11821232
// For tuple LHS, variables are already assigned (fullKey, keyHash, err := expr)
1183-
// For single LHS, we need: varName := tmp
1184-
if !isTupleLHS {
1233+
// For single LHS with plain assign (named return), variables are also already assigned (varName, err = expr)
1234+
// For single LHS with define, we need: varName := tmp
1235+
if !isTupleLHS && !(isPlainAssign && namedErrVar != "") {
1236+
buf.WriteString("\n}\n")
11851237
buf.WriteString(varName)
11861238
if varName == "_" {
11871239
buf.WriteString(" = ")
11881240
} else {
11891241
buf.WriteString(" := ")
11901242
}
11911243
buf.WriteString(tmpVar)
1244+
} else {
1245+
// No trailing assignment — end the if block without an extra newline
1246+
// to avoid introducing a blank line before the next source statement.
1247+
buf.WriteString("\n}")
11921248
}
11931249

11941250
return buf.Bytes()
@@ -1698,14 +1754,28 @@ func generateResultErrorPropBare(expr []byte, counter *int, errorKind ast.ErrorP
16981754
func generateTupleErrorPropBare(src []byte, expr []byte, exprPos int, returnTypes []string, counter *int, errorKind ast.ErrorPropKind, errorContext string, lambdaParam string, lambdaBody []byte, resolver *codegen.TypeResolver) []byte {
16991755
var buf bytes.Buffer
17001756

1757+
// Check if there is a named error return parameter in the enclosing function.
1758+
// If so, use that variable name directly (do NOT consume a counter slot).
1759+
// This avoids "no new variables on left side of :=" compile errors.
1760+
namedErrReturn := codegen.InferEnclosingFunctionNamedErrorReturn(src, exprPos)
1761+
17011762
// Generate unique variable names
17021763
var errVar string
1703-
if *counter == 1 {
1704-
errVar = "err"
1764+
var errDeclOp string
1765+
if namedErrReturn != "" {
1766+
// Use the named error return directly — no new variable needed
1767+
errVar = namedErrReturn
1768+
errDeclOp = " = "
1769+
// Do NOT decrement counter: this location uses named returns, not a fresh counter slot
17051770
} else {
1706-
errVar = fmt.Sprintf("err%d", *counter-1)
1771+
if *counter == 1 {
1772+
errVar = "err"
1773+
} else {
1774+
errVar = fmt.Sprintf("err%d", *counter-1)
1775+
}
1776+
*counter--
1777+
errDeclOp = " := "
17071778
}
1708-
*counter--
17091779

17101780
// Detect return count: 1 = single (error only), 2+ = multi (T, error), -1 = unknown
17111781
// Use the resolver for cross-file type resolution if available
@@ -1719,15 +1789,15 @@ func generateTupleErrorPropBare(src []byte, expr []byte, exprPos int, returnType
17191789
// because the user is explicitly not capturing any non-error return value.
17201790
// If a function returns (T, error) but user wrote a bare `foo()?`, they want to ignore T.
17211791
if returnCount == 1 || returnCount == -1 {
1722-
// Single return or unknown: err := expr
1792+
// Single return or unknown: err := expr (or err = expr for named returns)
17231793
// For bare statements, single-return is safer default (avoids "assignment mismatch" errors)
17241794
buf.WriteString(errVar)
1725-
buf.WriteString(" := ")
1795+
buf.WriteString(errDeclOp)
17261796
} else {
1727-
// Multi-return (returnCount > 1): _, err := expr
1797+
// Multi-return (returnCount > 1): _, err := expr (or _, err = expr for named returns)
17281798
buf.WriteString("_, ")
17291799
buf.WriteString(errVar)
1730-
buf.WriteString(" := ")
1800+
buf.WriteString(errDeclOp)
17311801
}
17321802
buf.Write(expr)
17331803
buf.WriteByte('\n')
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package main
2+
3+
// REGRESSION: named return error prop generates := instead of = — Fixed in /dev:fix session dev-fix-20260328-184322-392f98a3
4+
5+
// mayFail is a helper returning only an error, used in the BARE sub-bug case.
6+
func mayFail() error {
7+
return nil
8+
}
9+
10+
// getData is a helper returning a value and an error, used in the STATEMENT sub-bug case.
11+
func getData() (string, error) {
12+
return "hello", nil
13+
}
14+
15+
// wrapper demonstrates the BARE sub-bug:
16+
// `mayFail()?` inside a function with named return `err error` must generate
17+
// `err = mayFail()` (plain assignment), NOT `err := mayFail()` (short declaration).
18+
func wrapper() (err error) {
19+
mayFail()?
20+
return nil
21+
}
22+
23+
// fetch demonstrates the STATEMENT sub-bug:
24+
// `result = getData()?` inside a function with named returns `(result string, err error)`
25+
// must generate `result, err = getData()` (plain assignment on both sides),
26+
// NOT `_, err := getData()` (which discards result and re-declares err).
27+
func fetch() (result string, err error) {
28+
result = getData()?
29+
return
30+
}
31+
32+
// normal has NO named return parameters — regression guard to ensure the common
33+
// case still generates the short-declaration `:=` form as expected.
34+
func normal() (string, error) {
35+
val := getData()?
36+
return val, nil
37+
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package main
2+
3+
// REGRESSION: named return error prop generates := instead of = — Fixed in /dev:fix session dev-fix-20260328-184322-392f98a3
4+
5+
// mayFail is a helper returning only an error, used in the BARE sub-bug case.
6+
func mayFail() error {
7+
return nil
8+
}
9+
10+
// getData is a helper returning a value and an error, used in the STATEMENT sub-bug case.
11+
func getData() (string, error) {
12+
return "hello", nil
13+
}
14+
15+
// wrapper demonstrates the BARE sub-bug:
16+
// `mayFail()?` inside a function with named return `err error` must generate
17+
// `err = mayFail()` (plain assignment), NOT `err := mayFail()` (short declaration).
18+
func wrapper() (err error) {
19+
err = mayFail()
20+
if err != nil {
21+
return err
22+
}
23+
return nil
24+
}
25+
26+
// fetch demonstrates the STATEMENT sub-bug:
27+
// `result = getData()?` inside a function with named returns `(result string, err error)`
28+
// must generate `result, err = getData()` (plain assignment on both sides),
29+
// NOT `_, err := getData()` (which discards result and re-declares err).
30+
func fetch() (result string, err error) {
31+
result, err = getData()
32+
if err != nil {
33+
return "", err
34+
}
35+
return
36+
}
37+
38+
// normal has NO named return parameters — regression guard to ensure the common
39+
// case still generates the short-declaration `:=` form as expected.
40+
func normal() (string, error) {
41+
tmp, err := getData()
42+
if err != nil {
43+
return "", err
44+
}
45+
val := tmp
46+
return val, nil
47+
}

0 commit comments

Comments
 (0)