From a075a9e640c317c441b4a65fa91a89c9e9c97e20 Mon Sep 17 00:00:00 2001 From: GBNikola Date: Mon, 10 Aug 2026 14:54:50 +0200 Subject: [PATCH 1/2] AVRO-4331: [csharp] Fix unreachable code in generated Get()/Put() avrogen built the Get()/Put() switch body via CodeSnippetExpression, which CodeDom wraps in a CodeExpressionStatement and always suffixes with a ";". Since every switch arm returns or throws, that trailing ";" is unreachable. Roslyn stays quiet about it, but IDE analyzers with fuller flow analysis (ReSharper/Rider) flag it, forcing consumers to suppress CS0162 for all avrogen output. Use CodeSnippetStatement instead, which emits the block verbatim with no appended semicolon. --- .../csharp/src/apache/main/CodeGen/CodeGen.cs | 8 +++--- .../src/apache/test/CodGen/CodeGenTest.cs | 26 +++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/lang/csharp/src/apache/main/CodeGen/CodeGen.cs b/lang/csharp/src/apache/main/CodeGen/CodeGen.cs index 73b95852d7b..36c83a98496 100644 --- a/lang/csharp/src/apache/main/CodeGen/CodeGen.cs +++ b/lang/csharp/src/apache/main/CodeGen/CodeGen.cs @@ -874,15 +874,15 @@ protected virtual CodeTypeDeclaration processRecord(Schema schema) // end switch block for Get() getFieldStmt.AppendLine("\t\t\tdefault: throw new global::Avro.AvroRuntimeException(\"Bad index \" + fieldPos + \" in Get()\");") .Append("\t\t\t}"); - var cseGet = new CodeSnippetExpression(getFieldStmt.ToString()); - cmmGet.Statements.Add(cseGet); + var cssGet = new CodeSnippetStatement(getFieldStmt.ToString()); + cmmGet.Statements.Add(cssGet); ctd.Members.Add(cmmGet); // end switch block for Put() putFieldStmt.AppendLine("\t\t\tdefault: throw new global::Avro.AvroRuntimeException(\"Bad index \" + fieldPos + \" in Put()\");") .Append("\t\t\t}"); - var csePut = new CodeSnippetExpression(putFieldStmt.ToString()); - cmmPut.Statements.Add(csePut); + var cssPut = new CodeSnippetStatement(putFieldStmt.ToString()); + cmmPut.Statements.Add(cssPut); ctd.Members.Add(cmmPut); string nspace = recordSchema.Namespace; diff --git a/lang/csharp/src/apache/test/CodGen/CodeGenTest.cs b/lang/csharp/src/apache/test/CodGen/CodeGenTest.cs index 33c7f0cf6ee..f10f11df8dd 100644 --- a/lang/csharp/src/apache/test/CodGen/CodeGenTest.cs +++ b/lang/csharp/src/apache/test/CodGen/CodeGenTest.cs @@ -110,6 +110,32 @@ public void GetTypesShouldReturnTypes() Assert.That(Regex.Matches(planetEnumCode, "public enum PlanetEnum").Count, Is.EqualTo(1)); } + [Test] + public void RecordGetAndPutSwitchesShouldNotEmitUnreachableStatement() + { + AddSchema(@" +{ + ""name"": ""Sample"", + ""namespace"": ""Avro.Test.CodeGen.UnreachableCodeRegression"", + ""type"": ""record"", + ""fields"": [ + { ""name"": ""value"", ""type"": ""string"" } + ] +} +"); + GenerateCode(); + var types = GetTypes(); + bool hasSampleCode = types.TryGetValue("Sample", out string sampleCode); + Assert.That(hasSampleCode); + + // The exhaustive switch in Get()/Put() must not be followed by a stray ";" - + // that empty statement is unreachable (every case returns or throws) and + // trips IDE-only unreachable-code analysis (e.g. ReSharper) even though Roslyn + // does not flag it. + Assert.That(Regex.IsMatch(sampleCode, @"\}\s*;\s*\}\s*public virtual void Put"), Is.False); + Assert.That(Regex.IsMatch(sampleCode, @"in Put\(\)""\);\s*\}\s*;"), Is.False); + } + [Test] public void EnumWithKeywordSymbolsShouldHavePrefixedSymbols() { From 985e0eee9a6235cda28d1bcdbbcd9b1fca8777a7 Mon Sep 17 00:00:00 2001 From: GBNikola Date: Fri, 25 Sep 2026 16:33:52 +0200 Subject: [PATCH 2/2] AVRO-4331: [csharp] Keep generated switch indented and apply the same fix to protocol Request --- .../csharp/src/apache/main/CodeGen/CodeGen.cs | 12 ++--- .../src/apache/test/CodGen/CodeGenTest.cs | 48 +++++++++++++++++++ 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/lang/csharp/src/apache/main/CodeGen/CodeGen.cs b/lang/csharp/src/apache/main/CodeGen/CodeGen.cs index 36c83a98496..e8c603f28dc 100644 --- a/lang/csharp/src/apache/main/CodeGen/CodeGen.cs +++ b/lang/csharp/src/apache/main/CodeGen/CodeGen.cs @@ -563,7 +563,7 @@ protected virtual void processInterface(Protocol protocol) if (protocol.Messages.Count > 0) { - builder.AppendLine("switch(messageName)"); + builder.AppendLine("\t\t\tswitch(messageName)"); builder.Append("\t\t\t{"); foreach (var a in protocol.Messages) @@ -580,11 +580,11 @@ protected virtual void processInterface(Protocol protocol) } builder.Append("\t\t\t}"); - } - var cseGet = new CodeSnippetExpression(builder.ToString()); + var cssRequest = new CodeSnippetStatement(builder.ToString()); + requestMethod.Statements.Add(cssRequest); + } - requestMethod.Statements.Add(cseGet); ctd.Members.Add(requestMethod); AddMethods(protocol, false, ctd); @@ -770,7 +770,7 @@ protected virtual CodeTypeDeclaration processRecord(Schema schema) cmmGet.Attributes = MemberAttributes.Public; cmmGet.ReturnType = new CodeTypeReference("System.Object"); cmmGet.Parameters.Add(new CodeParameterDeclarationExpression(typeof(int), "fieldPos")); - StringBuilder getFieldStmt = new StringBuilder("switch (fieldPos)") + StringBuilder getFieldStmt = new StringBuilder("\t\t\tswitch (fieldPos)") .AppendLine().AppendLine("\t\t\t{"); // declare Put() to be used by the Reader classes @@ -780,7 +780,7 @@ protected virtual CodeTypeDeclaration processRecord(Schema schema) cmmPut.ReturnType = new CodeTypeReference(typeof(void)); cmmPut.Parameters.Add(new CodeParameterDeclarationExpression(typeof(int), "fieldPos")); cmmPut.Parameters.Add(new CodeParameterDeclarationExpression("System.Object", "fieldValue")); - var putFieldStmt = new StringBuilder("switch (fieldPos)") + var putFieldStmt = new StringBuilder("\t\t\tswitch (fieldPos)") .AppendLine().AppendLine("\t\t\t{"); if (isError) diff --git a/lang/csharp/src/apache/test/CodGen/CodeGenTest.cs b/lang/csharp/src/apache/test/CodGen/CodeGenTest.cs index f10f11df8dd..736e5a69044 100644 --- a/lang/csharp/src/apache/test/CodGen/CodeGenTest.cs +++ b/lang/csharp/src/apache/test/CodGen/CodeGenTest.cs @@ -134,6 +134,54 @@ public void RecordGetAndPutSwitchesShouldNotEmitUnreachableStatement() // does not flag it. Assert.That(Regex.IsMatch(sampleCode, @"\}\s*;\s*\}\s*public virtual void Put"), Is.False); Assert.That(Regex.IsMatch(sampleCode, @"in Put\(\)""\);\s*\}\s*;"), Is.False); + + // CodeDom writes CodeSnippetStatement at column 0, so the snippet carries its own indent. + Assert.That(Regex.Matches(sampleCode, @"(?m)^\t\t\tswitch \(fieldPos\)\r?$").Count, Is.EqualTo(2)); + Assert.That(Regex.Matches(sampleCode, @"(?m)^\s*switch \(fieldPos\)").Count, Is.EqualTo(2)); + } + + [Test] + public void ProtocolRequestSwitchShouldBeIndentedAndNotFollowedByEmptyStatement() + { + AddProtocol(@" +{ + ""protocol"": ""Greeter"", + ""namespace"": ""Avro.Test.CodeGen.ProtocolSwitchRegression"", + ""types"": [], + ""messages"": { + ""hello"": { + ""request"": [ { ""name"": ""greeting"", ""type"": ""string"" } ], + ""response"": ""string"" + } + } +} +"); + GenerateCode(); + var types = GetTypes(); + bool hasGreeterCode = types.TryGetValue("Greeter", out string greeterCode); + Assert.That(hasGreeterCode); + + Assert.That(Regex.Matches(greeterCode, @"(?m)^\t\t\tswitch\(messageName\)\r?$").Count, Is.EqualTo(1)); + Assert.That(Regex.IsMatch(greeterCode, @"break;\s*\}\s*;"), Is.False); + } + + [Test] + public void ProtocolWithoutMessagesShouldGenerateEmptyRequestBody() + { + AddProtocol(@" +{ + ""protocol"": ""Silent"", + ""namespace"": ""Avro.Test.CodeGen.EmptyProtocolRegression"", + ""types"": [], + ""messages"": {} +} +"); + GenerateCode(); + var types = GetTypes(); + bool hasSilentCode = types.TryGetValue("Silent", out string silentCode); + Assert.That(hasSilentCode); + + Assert.That(Regex.IsMatch(silentCode, @"object callback\)\s*\{\s*\}"), Is.True); } [Test]