From d914c14cd3d2aad47da2ca6f54b549c41d9993d9 Mon Sep 17 00:00:00 2001 From: Paul Irwin Date: Fri, 14 Aug 2026 10:11:41 -0600 Subject: [PATCH] Restore switch expression in EscapeIdentifier and add test coverage Follow-up to #142, which fixed the crash on identifiers named after C# keywords (#147) but converted the switch expression to a switch statement. Convert it back to a switch expression with the new cases included, with the `or` patterns wrapped across lines for readability. The keyword set is unchanged (verified identical, 77 keywords). Add EscapeIdentifierTests covering: - every reserved keyword Roslyn knows about, asserting both that it gets escaped and that the result parses back as an IdentifierToken, so the list cannot silently drift from the language - non-keywords, including contextual keywords like `var` and `record`, which must not be escaped - end-to-end conversion of method, constructor and lambda parameters and field accesses named after keywords, plus a check that the emitted C# parses without diagnostics The undocumented typed-reference keywords (__arglist, __makeref, __reftype, __refvalue) are excluded from the exhaustive check, as they are not valid Java identifiers and so cannot reach EscapeIdentifier from parsed source. Co-Authored-By: Claude Opus 5 (1M context) --- JavaToCSharp.Tests/EscapeIdentifierTests.cs | 176 ++++++++++++++++++++ JavaToCSharp/TypeHelper.cs | 105 +++--------- 2 files changed, 196 insertions(+), 85 deletions(-) create mode 100644 JavaToCSharp.Tests/EscapeIdentifierTests.cs diff --git a/JavaToCSharp.Tests/EscapeIdentifierTests.cs b/JavaToCSharp.Tests/EscapeIdentifierTests.cs new file mode 100644 index 0000000..60a80c4 --- /dev/null +++ b/JavaToCSharp.Tests/EscapeIdentifierTests.cs @@ -0,0 +1,176 @@ +using Microsoft.CodeAnalysis.CSharp; + +namespace JavaToCSharp.Tests; + +public class EscapeIdentifierTests +{ + /// + /// Every reserved keyword in the language, per Roslyn. Contextual keywords (e.g. var, + /// record, value) are excluded: they are legal identifiers and must not be escaped. + /// The undocumented typed-reference keywords (__arglist and friends) are excluded too: + /// they are not valid Java identifiers, so they cannot reach us from a parsed source file. + /// + public static TheoryData ReservedKeywords => + [..SyntaxFacts.GetReservedKeywordKinds() + .Select(SyntaxFacts.GetText) + .Where(keyword => !keyword.StartsWith("__", StringComparison.Ordinal))]; + + [Theory] + [MemberData(nameof(ReservedKeywords))] + public void EscapeIdentifier_GivenReservedKeyword_ShouldPrefixWithAtSign(string keyword) + { + Assert.Equal("@" + keyword, TypeHelper.EscapeIdentifier(keyword)); + } + + [Theory] + [MemberData(nameof(ReservedKeywords))] + public void EscapeIdentifier_GivenReservedKeyword_ShouldParseAsIdentifierToken(string keyword) + { + var token = SyntaxFactory.ParseToken(TypeHelper.EscapeIdentifier(keyword)); + + Assert.Equal(SyntaxKind.IdentifierToken, token.Kind()); + Assert.Equal(keyword, token.ValueText); + } + + [Theory] + [InlineData("struct")] + [InlineData("string")] + [InlineData("ref")] + [InlineData("out")] + [InlineData("in")] + [InlineData("class")] + [InlineData("void")] + public void EscapeIdentifier_GivenKeyword_ShouldEscape(string keyword) + { + Assert.Equal("@" + keyword, TypeHelper.EscapeIdentifier(keyword)); + } + + [Theory] + [InlineData("foo")] + [InlineData("Struct")] // casing matters: keywords are lowercase + [InlineData("structure")] + [InlineData("myClass")] + [InlineData("var")] // contextual keyword, legal as an identifier + [InlineData("record")] + [InlineData("value")] + [InlineData("nameof")] + [InlineData("")] + public void EscapeIdentifier_GivenNonKeyword_ShouldReturnUnchanged(string name) + { + Assert.Equal(name, TypeHelper.EscapeIdentifier(name)); + } + + /// + /// Regression test for #147: a Java parameter named after a C# keyword crashed the converter. + /// + [Fact] + public void ConvertText_GivenParameterNamedAfterKeyword_ShouldEscapeParameter() + { + const string javaCode = """ + public class Foo { + public void bar(Structure struct) { + } + } + """; + + var parsed = Convert(javaCode); + + Assert.Contains("public virtual void Bar(Structure @struct)", parsed); + } + + [Fact] + public void ConvertText_GivenParameterNamedAfterKeyword_ShouldEscapeUsagesOfParameter() + { + const string javaCode = """ + public class Foo { + public void bar(Structure struct) { + struct.baz(); + } + } + """; + + var parsed = Convert(javaCode); + + Assert.Contains("@struct.Baz();", parsed); + } + + [Fact] + public void ConvertText_GivenFieldNamedAfterKeyword_ShouldEscapeFieldAccess() + { + const string javaCode = """ + public class Foo { + public void bar(Structure s) { + s.event = 1; + } + } + """; + + var parsed = Convert(javaCode); + + Assert.Contains("s.@event = 1;", parsed); + } + + [Fact] + public void ConvertText_GivenConstructorParameterNamedAfterKeyword_ShouldEscapeParameter() + { + const string javaCode = """ + public class Foo { + public Foo(int base) { + } + } + """; + + var parsed = Convert(javaCode); + + Assert.Contains("public Foo(int @base)", parsed); + } + + [Fact] + public void ConvertText_GivenLambdaParameterNamedAfterKeyword_ShouldEscapeParameter() + { + const string javaCode = """ + public class Foo { + public void bar(List items) { + items.forEach(object -> print(object)); + } + } + """; + + var parsed = Convert(javaCode); + + Assert.Contains("(@object) => Print(@object)", parsed); + } + + /// + /// Escaped identifiers must survive a round trip through the C# parser without producing + /// diagnostics, which is what the original crash was really about. + /// + [Fact] + public void ConvertText_GivenParameterNamedAfterKeyword_ShouldProduceParseableCSharp() + { + const string javaCode = """ + public class Foo { + public void bar(Structure struct) { + struct.baz(); + } + } + """; + + var parsed = Convert(javaCode); + + var tree = CSharpSyntaxTree.ParseText(parsed); + + Assert.Empty(tree.GetDiagnostics()); + } + + private static string Convert(string javaCode) + { + var options = new JavaConversionOptions + { + IncludeUsings = false, + IncludeNamespace = false, + }; + + return JavaToCSharpConverter.ConvertText(javaCode, options) ?? ""; + } +} diff --git a/JavaToCSharp/TypeHelper.cs b/JavaToCSharp/TypeHelper.cs index 876ae98..7dcaafa 100644 --- a/JavaToCSharp/TypeHelper.cs +++ b/JavaToCSharp/TypeHelper.cs @@ -118,91 +118,26 @@ public static string Capitalize(string name) public static string EscapeIdentifier(string name) { // @ (C# Reference): https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/tokens/verbatim - //C# Keywords: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/ - switch (name) - { - case "abstract": - case "as": - case "base": - case "bool": - case "break": - case "byte": - case "case": - case "catch": - case "char": - case "checked": - case "class": - case "const": - case "continue": - case "decimal": - case "default": - case "delegate": - case "do": - case "double": - case "else": - case "enum": - case "event": - case "explicit": - case "extern": - case "false": - case "finally": - case "fixed": - case "float": - case "for": - case "foreach": - case "goto": - case "if": - case "implicit": - case "in": - case "int": - case "interface": - case "internal": - case "is": - case "lock": - case "long": - case "namespace": - case "new": - case "null": - case "object": - case "operator": - case "out": - case "override": - case "params": - case "private": - case "protected": - case "public": - case "readonly": - case "ref": - case "return": - case "sbyte": - case "sealed": - case "short": - case "sizeof": - case "stackalloc": - case "static": - case "string": - case "struct": - case "switch": - case "this": - case "throw": - case "true": - case "try": - case "typeof": - case "uint": - case "ulong": - case "unchecked": - case "unsafe": - case "ushort": - case "using": - case "virtual": - case "void": - case "volatile": - case "while": - return "@" + name; - - default: - return name; - } + // C# Keywords: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/ + return name switch { + "abstract" or "as" or "base" or "bool" or "break" or + "byte" or "case" or "catch" or "char" or "checked" or + "class" or "const" or "continue" or "decimal" or "default" or + "delegate" or "do" or "double" or "else" or "enum" or + "event" or "explicit" or "extern" or "false" or "finally" or + "fixed" or "float" or "for" or "foreach" or "goto" or + "if" or "implicit" or "in" or "int" or "interface" or + "internal" or "is" or "lock" or "long" or "namespace" or + "new" or "null" or "object" or "operator" or "out" or + "override" or "params" or "private" or "protected" or "public" or + "readonly" or "ref" or "return" or "sbyte" or "sealed" or + "short" or "sizeof" or "stackalloc" or "static" or "string" or + "struct" or "switch" or "this" or "throw" or "true" or + "try" or "typeof" or "uint" or "ulong" or "unchecked" or + "unsafe" or "ushort" or "using" or "virtual" or "void" or + "volatile" or "while" => "@" + name, + _ => name, + }; } public static string ReplaceCommonMethodNames(string name)