Skip to content

Commit

Permalink
fixes methods taking nullable parameters could generate multiple Meth…
Browse files Browse the repository at this point in the history
…odDefinitions (#245)

this was happening due a mismatch between the registering of the variable used to store the
MethodDefinition object and the query used to retrieve it back when processing references
to such method.
  • Loading branch information
adrianoc committed Aug 16, 2023
1 parent 8399517 commit b4dbf5f
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 1 deletion.
66 changes: 66 additions & 0 deletions Cecilifier.Core.Tests/Tests/Unit/NullableTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
using System.Text.RegularExpressions;
using NUnit.Framework;
using NUnit.Framework.Internal;

namespace Cecilifier.Core.Tests.Tests.Unit;

[TestFixture]
public class NullableTests : CecilifierUnitTestBase
{
[TestCase("object", TestName = "object")]
[TestCase("Foo", TestName = "Foo")]
[TestCase("IFoo", TestName = "IFoo")]
[TestCase("int", TestName = "int")]
public void MethodsWithNullableParameters_AreRegistered_Once(string parameterType)
{
var result = RunCecilifier(
$$"""
interface IFoo {}
class Foo : IFoo
{
void M({{parameterType}}? o) {}
void M2({{parameterType}} p) => M(p);
}
""");

var actual = result.GeneratedCode.ReadToEnd();

Assert.That(
Regex.Count(actual, """var .+ = new MethodDefinition\("M", MethodAttributes.Private \| MethodAttributes.HideBySig, assembly.MainModule.TypeSystem.Void\);\n"""),
Is.EqualTo(1),
"Only only declaration for method M() is expected.");
}

[TestCase("object", @".+TypeSystem\.Object", TestName = "object")]
[TestCase("Foo", "cls_foo_1", TestName = "Foo")]
[TestCase("IFoo", "itf_iFoo_0", TestName = "IFoo")]
[TestCase("int", @".+ImportReference\(typeof\(System.Nullable<>\)\).MakeGenericInstanceType\(.+Int32\)", TestName = "int")]
public void MethodsWithNullableReturnTypes_AreRegistered_Once(string returnType, string expectedReturnTypeInDeclaration)
{
var result = RunCecilifier(
$$"""
interface IFoo {}
class Foo : IFoo
{
{{returnType}}? M() => default({{returnType}});
void M2() => M();
}
""");

var actual = result.GeneratedCode.ReadToEnd();
Assert.That(
Regex.Count(actual, $$"""var m_M_\d+ = new MethodDefinition\("M", MethodAttributes.+, {{expectedReturnTypeInDeclaration}}\);\n"""),
Is.EqualTo(1),
actual);
}

(\s+il_test_\d+\.Emit\(OpCodes\.)Ldarg_0\);
\1Ldarg_1\);
\1NEWOBJ Nullable<int>
\1Call, m_bar_1\);
\1NEWOBJ Nullable<int>
\1Ret\);
"""),
"");
}
}
2 changes: 1 addition & 1 deletion Cecilifier.Core/AST/MethodDeclarationVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ private void ProcessMethodDeclarationInternal(
// the later is a `mangled name` and any reference to the method will use its `unmangled name` for lookups which would fail
// should we use `methodName` as the registered name.
var nameUsedInRegisteredVariable = methodSymbol.MethodKind == MethodKind.LocalFunction ? simpleName : methodName;
WithCurrentMethod(declaringTypeName, methodVar, nameUsedInRegisteredVariable, parameters.Select(p => Context.GetTypeInfo(p.Type).Type.ToDisplayString()).ToArray(), runWithCurrent);
WithCurrentMethod(declaringTypeName, methodVar, nameUsedInRegisteredVariable, parameters.Select(p => Context.SemanticModel.GetDeclaredSymbol(p).Type.ToDisplayString()).ToArray(), runWithCurrent);
if (!methodSymbol.IsAbstract && !node.DescendantNodes().Any(n => n.IsKind(SyntaxKind.ReturnStatement)))
{
Context.EmitCilInstruction(ilVar, OpCodes.Ret);
Expand Down

0 comments on commit b4dbf5f

Please sign in to comment.