From 563956d071cadab5fe4d0f26e2db5113cbd3f259 Mon Sep 17 00:00:00 2001 From: yowl Date: Tue, 19 Nov 2024 20:19:28 -0500 Subject: [PATCH] [C#] Idiomatic formatting (#1083) * minor improvement to reduce unused using statements * use more idiomatic c# casing and bracket style * index on tidy-usings: f38910be use more idiomatic c# casing and bracket style * cargo fmr * remove TODO comment --------- Co-authored-by: Timmy Silesmo --- crates/csharp/src/lib.rs | 188 ++++++++++++++++------ tests/runtime/resource_aggregates/wasm.cs | 12 +- tests/runtime/results/wasm.cs | 12 +- tests/runtime/variants/wasm.cs | 22 +-- 4 files changed, 164 insertions(+), 70 deletions(-) diff --git a/crates/csharp/src/lib.rs b/crates/csharp/src/lib.rs index ff4c767c0..744239a91 100644 --- a/crates/csharp/src/lib.rs +++ b/crates/csharp/src/lib.rs @@ -461,7 +461,7 @@ impl WorldGenerator for CSharp { {access} readonly struct None {{}} [StructLayout(LayoutKind.Sequential)] - {access} readonly struct Result + {access} readonly struct Result {{ {access} readonly byte Tag; private readonly object value; @@ -472,43 +472,50 @@ impl WorldGenerator for CSharp { this.value = value; }} - {access} static Result ok(Ok ok) + {access} static Result Ok(TOk ok) {{ - return new Result(OK, ok!); + return new Result(Tags.Ok, ok!); }} - {access} static Result err(Err err) + {access} static Result Err(TErr err) {{ - return new Result(ERR, err!); + return new Result(Tags.Err, err!); }} - {access} bool IsOk => Tag == OK; - {access} bool IsErr => Tag == ERR; + {access} bool IsOk => Tag == Tags.Ok; + {access} bool IsErr => Tag == Tags.Err; - {access} Ok AsOk + {access} TOk AsOk {{ get {{ - if (Tag == OK) - return (Ok)value; - else - throw new ArgumentException("expected OK, got " + Tag); + if (Tag == Tags.Ok) + {{ + return (TOk)value; + }} + + throw new ArgumentException("expected k, got " + Tag); }} }} - {access} Err AsErr + {access} TErr AsErr {{ get {{ - if (Tag == ERR) - return (Err)value; - else - throw new ArgumentException("expected ERR, got " + Tag); + if (Tag == Tags.Err) + {{ + return (TErr)value; + }} + + throw new ArgumentException("expected Err, got " + Tag); }} }} - {access} const byte OK = 0; - {access} const byte ERR = 1; + {access} class Tags + {{ + {access} const byte Ok = 0; + {access} const byte Err = 1; + }} }} "#, ) @@ -1144,6 +1151,8 @@ impl InterfaceGenerator<'_> { let import_name = &func.name; + self.gen.require_using("System.Runtime.InteropServices"); + let target = if let FunctionKind::Freestanding = &func.kind { self.require_interop_using("System.Runtime.InteropServices"); &mut self.csharp_interop_src @@ -1865,7 +1874,7 @@ impl<'a> wit_bindgen_core::InterfaceGenerator<'a> for InterfaceGenerator<'a> { .iter() .map(|case| { let case_name = case.name.to_csharp_ident(); - let tag = case.name.to_shouty_snake_case(); + let tag = case.name.to_csharp_ident_upper(); let (parameter, argument) = if let Some(ty) = self.non_empty_type(case.ty.as_ref()) { ( @@ -1877,8 +1886,8 @@ impl<'a> wit_bindgen_core::InterfaceGenerator<'a> for InterfaceGenerator<'a> { }; format!( - "{access} static {name} {case_name}({parameter}) {{ - return new {name}({tag}, {argument}); + "{access} static {name} {tag}({parameter}) {{ + return new {name}(Tags.{tag}, {argument}); }} " ) @@ -1892,14 +1901,14 @@ impl<'a> wit_bindgen_core::InterfaceGenerator<'a> for InterfaceGenerator<'a> { .filter_map(|case| { self.non_empty_type(case.ty.as_ref()).map(|ty| { let case_name = case.name.to_upper_camel_case(); - let tag = case.name.to_shouty_snake_case(); + let tag = case.name.to_csharp_ident_upper(); let ty = self.type_name(ty); format!( r#"{access} {ty} As{case_name} {{ get {{ - if (Tag == {tag}) + if (Tag == Tags.{tag}) return ({ty})value!; else throw new ArgumentException("expected {tag}, got " + Tag); @@ -1917,7 +1926,7 @@ impl<'a> wit_bindgen_core::InterfaceGenerator<'a> for InterfaceGenerator<'a> { .iter() .enumerate() .map(|(i, case)| { - let tag = case.name.to_shouty_snake_case(); + let tag = case.name.to_csharp_ident_upper(); format!("{access} const {tag_type} {tag} = {i};") }) .collect::>() @@ -1937,7 +1946,10 @@ impl<'a> wit_bindgen_core::InterfaceGenerator<'a> for InterfaceGenerator<'a> { {constructors} {accessors} - {tags} + + {access} class Tags {{ + {tags} + }} }} " ); @@ -2195,7 +2207,7 @@ impl<'a, 'b> FunctionBindgen<'a, 'b> { String::new() }; - let method = case_name.to_csharp_ident(); + let method = case_name.to_csharp_ident_upper(); let call = if let Some(position) = generics_position { let (ty, generics) = ty.split_at(position); @@ -2557,7 +2569,7 @@ impl Bindgen for FunctionBindgen<'_, '_> { result, .. } => self.lower_variant( - &[("ok", result.ok), ("err", result.err)], + &[("Ok", result.ok), ("Err", result.err)], lowered_types, &operands[0], results, @@ -2565,7 +2577,7 @@ impl Bindgen for FunctionBindgen<'_, '_> { Instruction::ResultLift { result, ty } => self.lift_variant( &Type::Id(*ty), - &[("ok", result.ok), ("err", result.err)], + &[("Ok", result.ok), ("Err", result.err)], &operands[0], results, ), @@ -2844,13 +2856,13 @@ impl Bindgen for FunctionBindgen<'_, '_> { format!( "\ case {index}: {{ - ret = {head}{ty}.err(({err_ty}) e.Value){tail}; + ret = {head}{ty}.Err(({err_ty}) e.Value){tail}; break; }} " ) ); - oks.push(format!("{ty}.ok(")); + oks.push(format!("{ty}.Ok(")); payload_is_void = result.ok.is_none(); } if !self.results.is_empty() { @@ -3382,28 +3394,110 @@ fn is_primitive(ty: &Type) -> bool { } trait ToCSharpIdent: ToOwned { + fn csharp_keywords() -> Vec<&'static str>; fn to_csharp_ident(&self) -> Self::Owned; + fn to_csharp_ident_upper(&self) -> Self::Owned; } impl ToCSharpIdent for str { + // Source: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/ + fn csharp_keywords() -> Vec<&'static str> { + vec![ + "abstract", + "as", + "base", + "bool", + "break", + "byte", + "case", + "catch", + "char", + "checked", + "class", + "const", + "continue", + "decimal", + "default", + "delegate", + "do", + "double", + "else", + "enum", + "event", + "explicit", + "extern", + "false", + "finally", + "fixed", + "float", + "for", + "foreach", + "goto", + "if", + "implicit", + "in", + "int", + "interface", + "internal", + "is", + "lock", + "long", + "namespace", + "new", + "null", + "object", + "operator", + "out", + "override", + "params", + "private", + "protected", + "public", + "readonly", + "ref", + "return", + "sbyte", + "sealed", + "short", + "sizeof", + "stackalloc", + "static", + "string", + "struct", + "switch", + "this", + "throw", + "true", + "try", + "typeof", + "uint", + "ulong", + "unchecked", + "unsafe", + "ushort", + "using", + "virtual", + "void", + "volatile", + "while", + ] + } + fn to_csharp_ident(&self) -> String { // Escape C# keywords - // Source: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/ - - //TODO: Repace with actual keywords - match self { - "abstract" | "as" | "base" | "bool" | "break" | "byte" | "case" | "catch" | "char" - | "checked" | "class" | "const" | "continue" | "decimal" | "default" | "delegate" - | "do" | "double" | "else" | "enum" | "event" | "explicit" | "extern" | "false" - | "finally" | "fixed" | "float" | "for" | "foreach" | "goto" | "if" | "implicit" - | "in" | "int" | "interface" | "internal" | "is" | "lock" | "long" | "namespace" - | "new" | "null" | "object" | "operator" | "out" | "override" | "params" - | "private" | "protected" | "public" | "readonly" | "ref" | "return" | "sbyte" - | "sealed" | "short" | "sizeof" | "stackalloc" | "static" | "string" | "struct" - | "switch" | "this" | "throw" | "true" | "try" | "typeof" | "uint" | "ulong" - | "unchecked" | "unsafe" | "ushort" | "using" | "virtual" | "void" | "volatile" - | "while" => format!("@{self}"), - _ => self.to_lower_camel_case(), + if Self::csharp_keywords().contains(&self) { + format!("@{}", self) + } else { + self.to_lower_camel_case() + } + } + + fn to_csharp_ident_upper(&self) -> String { + // Escape C# keywords + if Self::csharp_keywords().contains(&self) { + format!("@{}", self) + } else { + self.to_upper_camel_case() } } } diff --git a/tests/runtime/resource_aggregates/wasm.cs b/tests/runtime/resource_aggregates/wasm.cs index 964b799a3..28414f62a 100644 --- a/tests/runtime/resource_aggregates/wasm.cs +++ b/tests/runtime/resource_aggregates/wasm.cs @@ -33,8 +33,8 @@ public static uint Foo( var ir3 = new Import.R3(((Thing) r3.thing1).val, ((Thing) r3.thing2).val); var it1 = (((Thing) t1.Item1).val, new Import.R1(((Thing) t1.Item2.thing).val)); var it2 = ((Thing) t2).val; - var iv1 = Import.V1.thing(((Thing) v1.AsThing).val); - var iv2 = Import.V2.thing(((Thing) v2.AsThing).val); + var iv1 = Import.V1.Thing(((Thing) v1.AsThing).val); + var iv2 = Import.V2.Thing(((Thing) v2.AsThing).val); var il1 = new List(); foreach (var thing in l1) { @@ -52,11 +52,11 @@ public static uint Foo( ? ((Thing) o2).val : null; var iresult1 = result1.IsOk - ? Result.ok(((Thing) result1.AsOk).val) - : Result.err(new None()); + ? Result.Ok(((Thing) result1.AsOk).val) + : Result.Err(new None()); var iresult2 = result2.IsOk - ? Result.ok(((Thing) result2.AsOk).val) - : Result.err(new None()); + ? Result.Ok(((Thing) result2.AsOk).val) + : Result.Err(new None()); return Host.Foo(ir1, ir2, ir3, it1, it2, iv1, iv2, il1, il2, io1, io2, iresult1, iresult2) + 4; } diff --git a/tests/runtime/results/wasm.cs b/tests/runtime/results/wasm.cs index bca72de5c..81ca3296b 100644 --- a/tests/runtime/results/wasm.cs +++ b/tests/runtime/results/wasm.cs @@ -44,19 +44,19 @@ public static float VariantError(float a) } catch (WitException e) { var value = (ResultsWorld.wit.imports.test.results.ITest.E3) e.Value; switch (value.Tag) { - case ResultsWorld.wit.imports.test.results.ITest.E3.E1: + case ResultsWorld.wit.imports.test.results.ITest.E3.Tags.E1: switch (value.AsE1) { case ResultsWorld.wit.imports.test.results.ITest.E.A: - throw new WitException(ITest.E3.e1(ITest.E.A), 0); + throw new WitException(ITest.E3.E1(ITest.E.A), 0); case ResultsWorld.wit.imports.test.results.ITest.E.B: - throw new WitException(ITest.E3.e1(ITest.E.B), 0); + throw new WitException(ITest.E3.E1(ITest.E.B), 0); case ResultsWorld.wit.imports.test.results.ITest.E.C: - throw new WitException(ITest.E3.e1(ITest.E.C), 0); + throw new WitException(ITest.E3.E1(ITest.E.C), 0); default: throw new Exception("unreachable"); } - case ResultsWorld.wit.imports.test.results.ITest.E3.E2: { - throw new WitException(ITest.E3.e2(new ITest.E2(value.AsE2.line, value.AsE2.column)), 0); + case ResultsWorld.wit.imports.test.results.ITest.E3.Tags.E2: { + throw new WitException(ITest.E3.E2(new ITest.E2(value.AsE2.line, value.AsE2.column)), 0); } default: throw new Exception("unreachable"); diff --git a/tests/runtime/variants/wasm.cs b/tests/runtime/variants/wasm.cs index 66235c767..6ee29b8fc 100644 --- a/tests/runtime/variants/wasm.cs +++ b/tests/runtime/variants/wasm.cs @@ -14,10 +14,10 @@ public static void TestImports() Debug.Assert(TestInterop.RoundtripOption(null).HasValue == false); Debug.Assert(TestInterop.RoundtripOption(2.0f).Value == 2); - Debug.Assert(TestInterop.RoundtripResult(Result.ok(2)) == 2.0); - Debug.Assert(TestInterop.RoundtripResult(Result.ok(4)) == 4.0); + Debug.Assert(TestInterop.RoundtripResult(Result.Ok(2)) == 2.0); + Debug.Assert(TestInterop.RoundtripResult(Result.Ok(4)) == 4.0); try { - TestInterop.RoundtripResult(Result.err(5.3f)); + TestInterop.RoundtripResult(Result.Err(5.3f)); throw new Exception(); } catch (WitException e) { Debug.Assert((byte)e.Value == 5); @@ -30,7 +30,7 @@ public static void TestImports() Debug.Assert(TestInterop.InvertBool(false) == true); var (a1, a2, a3, a4, a5, a6) = - TestInterop.VariantCasts((ITest.C1.a(1), ITest.C2.a(2), ITest.C3.a(3), ITest.C4.a(4), ITest.C5.a(5), ITest.C6.a(6.0f))); + TestInterop.VariantCasts((ITest.C1.A(1), ITest.C2.A(2), ITest.C3.A(3), ITest.C4.A(4), ITest.C5.A(5), ITest.C6.A(6.0f))); Debug.Assert(a1.AsA == 1); Debug.Assert(a2.AsA == 2); Debug.Assert(a3.AsA == 3); @@ -39,7 +39,7 @@ public static void TestImports() Debug.Assert(a6.AsA == 6.0f); var (b1, b2, b3, b4, b5, b6) = -TestInterop.VariantCasts((ITest.C1.b(1), ITest.C2.b(2), ITest.C3.b(3), ITest.C4.b(4), ITest.C5.b(5), ITest.C6.b(6.0))); +TestInterop.VariantCasts((ITest.C1.B(1), ITest.C2.B(2), ITest.C3.B(3), ITest.C4.B(4), ITest.C5.B(5), ITest.C6.B(6.0))); Debug.Assert(b1.AsB == 1); Debug.Assert(b2.AsB == 2.0f); Debug.Assert(b3.AsB == 3.0f); @@ -48,23 +48,23 @@ public static void TestImports() Debug.Assert(b6.AsB == 6.0); var (za1, za2, za3, za4) = -TestInterop.VariantZeros((ITest.Z1.a(1), ITest.Z2.a(2), ITest.Z3.a(3.0f), ITest.Z4.a(4.0f))); +TestInterop.VariantZeros((ITest.Z1.A(1), ITest.Z2.A(2), ITest.Z3.A(3.0f), ITest.Z4.A(4.0f))); Debug.Assert(za1.AsA == 1); Debug.Assert(za2.AsA == 2); Debug.Assert(za3.AsA == 3.0f); Debug.Assert(za4.AsA == 4.0f); var (zb1, zb2, zb3, zb4) = -TestInterop.VariantZeros((ITest.Z1.b(), ITest.Z2.b(), ITest.Z3.b(), ITest.Z4.b())); +TestInterop.VariantZeros((ITest.Z1.B(), ITest.Z2.B(), ITest.Z3.B(), ITest.Z4.B())); //TODO: Add comparison operator to variants and None //Debug.Assert(zb1.AsB == ITest.Z1.b()); //Debug.Assert(zb2.AsB == ITest.Z2.b()); //Debug.Assert(zb3.AsB == ITest.Z3.b()); //Debug.Assert(zb4.AsB == ITest.Z4.b()); - TestInterop.VariantTypedefs(null, false, Result.err(new None())); + TestInterop.VariantTypedefs(null, false, Result.Err(new None())); - var (a, b, c) = TestInterop.VariantEnums(true, Result.ok(new None()), ITest.MyErrno.SUCCESS); + var (a, b, c) = TestInterop.VariantEnums(true, Result.Ok(new None()), ITest.MyErrno.SUCCESS); Debug.Assert(a == false); var test = b.AsErr; Debug.Assert(c == ITest.MyErrno.A); @@ -85,8 +85,8 @@ public static double RoundtripResult(Result a) { switch (a.Tag) { - case Result.OK: return (double)a.AsOk; - case Result.ERR: throw new WitException((byte)a.AsErr, 0); + case Result.Tags.Ok: return (double)a.AsOk; + case Result.Tags.Err: throw new WitException((byte)a.AsErr, 0); default: throw new ArgumentException(); } }