From f8851d218ad7ef2a56fc2b41e2f187b3bafb1cf0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 01:37:19 +0000 Subject: [PATCH] fix: parenthesise a struct literal operand in a Go condition [patch] GoGenerator.WriteCondition drops the clause's outer parentheses to match gofmt, which left a struct literal bare in the header of an if: `if p == Point{1, 2} {` is a Go syntax error, because Go reads `Point{` there as the start of the block. Each direct operand of the condition now goes through WriteConditionOperand, which parenthesises a brace-spelled construction, as the Go spec prescribes and gofmt keeps. IsCompositeLiteral now holds the rule GenerateConstructionExpression already used to choose braces over a conversion, so the two cannot disagree. Fixes #105 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01NfxXtpcxX8hkLYrePfHkV4 --- .../GoGeneratedSourceCompilesTests.cs | 84 +++++++++++++++++++ Coder/Languages/GoGenerator.cs | 44 +++++++++- 2 files changed, 124 insertions(+), 4 deletions(-) diff --git a/Coder.Test/Languages/GoGeneratedSourceCompilesTests.cs b/Coder.Test/Languages/GoGeneratedSourceCompilesTests.cs index bbc4f74..4dc1831 100644 --- a/Coder.Test/Languages/GoGeneratedSourceCompilesTests.cs +++ b/Coder.Test/Languages/GoGeneratedSourceCompilesTests.cs @@ -176,6 +176,90 @@ public void ConditionalExpressions_InferParameterTypesAndLowerNestedReturns() }); } + /// + /// Tests that a condition comparing against a struct literal compiles in every position a + /// conditional is lowered to an if, and is still what gofmt writes. + /// + /// + /// Go reads Pair{ in the header of an if as the start of the block, so the literal + /// has to be parenthesised once the clause's own parentheses are left off (issue #105). + /// + [TestMethod] + public void ConditionComparingAgainstAStructLiteral_Compiles() + { + if (ToolchainHarness.FindOnPath("version", "go") is null) + { + Assert.Inconclusive("No Go toolchain on the path, so nothing was compiled."); + return; + } + + SourceFile file = new("literals"); + ClassDeclaration pair = new("Pair") { Kind = TypeDeclarationKind.Struct }; + pair.Members.Add(new VariableDeclaration("x", "int")); + pair.Members.Add(new VariableDeclaration("y", "int")); + file.Members.Add(pair); + + // The return the issue reproduced with, the struct on the right. + FunctionDeclaration returned = new("returned") { ReturnType = new TypeReference("int") }; + returned.Parameters.Add(new Parameter("p", "Pair")); + returned.Body.Add(new ReturnStatement(new ConditionalExpression( + new BinaryExpression(new VariableReference("p"), BinaryOperator.Equal, PairLiteral()), + new LiteralExpression(1), + new LiteralExpression(0)))); + file.Members.Add(returned); + + // An assignment, the struct on the left and spelled with field names. + FunctionDeclaration assigned = new("assigned") { ReturnType = new TypeReference("int") }; + assigned.Parameters.Add(new Parameter("p", "Pair")); + assigned.Body.Add(new VariableDeclaration("r", "int", new LiteralExpression(0))); + ConstructionExpression named = new(new TypeReference("Pair")); + named.Arguments.Add(new MemberInitialiser("x") { Value = new LiteralExpression(1) }); + named.Arguments.Add(new MemberInitialiser("y") { Value = new LiteralExpression(2) }); + assigned.Body.Add(new AssignmentStatement( + new VariableReference("r"), + new ConditionalExpression( + new BinaryExpression(named, BinaryOperator.NotEqual, new VariableReference("p")), + new LiteralExpression(1), + new LiteralExpression(0)))); + assigned.Body.Add(new ReturnStatement(new VariableReference("r"))); + file.Members.Add(assigned); + + // Inside another expression, where the conditional becomes a function literal called in place. + FunctionDeclaration nested = new("nested") { ReturnType = new TypeReference("int") }; + nested.Parameters.Add(new Parameter("p", "Pair")); + nested.Body.Add(new ReturnStatement(new BinaryExpression( + new ConditionalExpression( + new BinaryExpression(new VariableReference("p"), BinaryOperator.Equal, PairLiteral()), + new LiteralExpression(1), + new LiteralExpression(0)), + BinaryOperator.Add, + new LiteralExpression(1)))); + file.Members.Add(nested); + + string generated = new GoGenerator().Generate(file); + Assert.Contains("p == (Pair{1, 2})", generated, StringComparison.Ordinal, generated); + + ToolchainHarness.InTemporaryDirectory(directory => + { + File.WriteAllText(Path.Combine(directory, "go.mod"), Module); + File.WriteAllText(Path.Combine(directory, "literals.go"), generated); + (int exitCode, string output) = ToolchainHarness.Run("go", "build ./...", directory); + Assert.AreEqual(0, exitCode, $"Go rejected the generated source:{Environment.NewLine}{output}{Environment.NewLine}{generated}"); + + (int formatted, string differs) = ToolchainHarness.Run("gofmt", "-l literals.go", directory); + Assert.AreEqual(0, formatted, $"gofmt did not run:{Environment.NewLine}{differs}"); + Assert.AreEqual(string.Empty, differs.Trim(), "gofmt would rewrite the generated source."); + }); + + static ConstructionExpression PairLiteral() + { + ConstructionExpression literal = new(new TypeReference("Pair")); + literal.Arguments.Add(new LiteralExpression(1)); + literal.Arguments.Add(new LiteralExpression(2)); + return literal; + } + } + /// /// Tests that the arguments an entry point is handed are the ones the user typed, without the /// program's own path that Go puts first — which is what they are in every other target. diff --git a/Coder/Languages/GoGenerator.cs b/Coder/Languages/GoGenerator.cs index 18c6a95..01353d7 100644 --- a/Coder/Languages/GoGenerator.cs +++ b/Coder/Languages/GoGenerator.cs @@ -1638,20 +1638,26 @@ private void WriteChoice( /// Only the outermost pair: gofmt takes no view on the ones inside, which are what make /// the expression unambiguous in the first place. /// + /// + /// Taking that pair off exposes the operands to the clause, and a struct literal is the one operand + /// that cannot stand there: Go reads Point{ in the header of an if as the start of + /// the block, so if p == Point{1, 2} { is a syntax error. The language's own answer is to + /// parenthesise the literal, which gofmt keeps, so does. + /// /// private void WriteCondition(AstNode condition, CodeBlocker code) { switch (condition) { case BinaryExpression binary: - GenerateInternal(binary.Left, code); + WriteConditionOperand(binary.Left, code); code.Write($" {GetOperatorSpelling(binary.Operator)} "); - GenerateInternal(binary.Right, code); + WriteConditionOperand(binary.Right, code); return; case UnaryExpression unary: code.Write(GetUnaryOperatorSpelling(unary.Operator)); - GenerateInternal(unary.Operand, code); + WriteConditionOperand(unary.Operand, code); return; default: @@ -1660,6 +1666,25 @@ private void WriteCondition(AstNode condition, CodeBlocker code) } } + /// + /// Writes one operand of the expression an if tests, parenthesising a struct literal so the + /// clause does not end at its opening brace. + /// + /// The operand to write. + /// The writer to emit into. + private void WriteConditionOperand(AstNode operand, CodeBlocker code) + { + if (operand is ConstructionExpression construction && IsCompositeLiteral(construction)) + { + code.Write("("); + GenerateInternal(operand, code); + code.Write(")"); + return; + } + + GenerateInternal(operand, code); + } + /// /// /// Where the statement around it could not take the lowering — nested inside another expression, @@ -1720,7 +1745,7 @@ protected override void GenerateConstructionExpression(ConstructionExpression co string type = SpellType(construction.Type); - if (construction.Arguments.Count != 1 || construction.Arguments.Any(argument => argument is MemberInitialiser)) + if (IsCompositeLiteral(construction)) { code.Write(type); WriteElementList(construction, code, "{", "}", "{}"); @@ -1732,6 +1757,17 @@ protected override void GenerateConstructionExpression(ConstructionExpression co code.Write(")"); } + /// + /// Reports whether writes a construction as a + /// composite literal in braces rather than as a conversion in parentheses. + /// + /// The construction to classify. + /// True when it is written with braces. + private static bool IsCompositeLiteral(ConstructionExpression construction) => + construction.Type is null + || construction.Arguments.Count != 1 + || construction.Arguments.Any(argument => argument is MemberInitialiser); + /// /// /// Go's main takes no arguments and returns nothing, so a program that wants either