Skip to content

Commit 6a5e575

Browse files
committed
feat: bind constants as upper-case snake-case
additional: add enums unit tests
1 parent 6757e1f commit 6a5e575

File tree

3 files changed

+96
-22
lines changed

3 files changed

+96
-22
lines changed

src/embed_tests/ClassManagerTests.cs

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,13 @@ public void NestedClassDerivingFromParent()
3131

3232
#region Snake case naming tests
3333

34+
public enum SnakeCaseEnum
35+
{
36+
EnumValue1,
37+
EnumValue2,
38+
EnumValue3
39+
}
40+
3441
public class SnakeCaseNamesTesClass
3542
{
3643
// Purposely long names to test snake case conversion
@@ -49,6 +56,8 @@ public class SnakeCaseNamesTesClass
4956
public event EventHandler<string> PublicStringEvent;
5057
public static event EventHandler<string> PublicStaticStringEvent;
5158

59+
public SnakeCaseEnum EnumValue = SnakeCaseEnum.EnumValue2;
60+
5261
public void InvokePublicStringEvent(string value)
5362
{
5463
PublicStringEvent?.Invoke(this, value);
@@ -100,10 +109,11 @@ public void BindsSnakeCaseClassMethods(string originalMethodName, string snakeCa
100109
}
101110

102111
[TestCase("PublicStringField", "public_string_field")]
103-
[TestCase("PublicConstStringField", "public_const_string_field")]
104-
[TestCase("PublicReadonlyStringField", "public_readonly_string_field")]
105112
[TestCase("PublicStaticStringField", "public_static_string_field")]
106-
[TestCase("PublicStaticReadonlyStringField", "public_static_readonly_string_field")]
113+
// Constants
114+
[TestCase("PublicConstStringField", "PUBLIC_CONST_STRING_FIELD")]
115+
[TestCase("PublicReadonlyStringField", "PUBLIC_READONLY_STRING_FIELD")]
116+
[TestCase("PublicStaticReadonlyStringField", "PUBLIC_STATIC_READONLY_STRING_FIELD")]
107117
public void BindsSnakeCaseClassFields(string originalFieldName, string snakeCaseFieldName)
108118
{
109119
using var obj = new SnakeCaseNamesTesClass().ToPython();
@@ -400,6 +410,59 @@ public void CanCallSnakeCasedMethodWithSnakeCasedNamedArguments(string methodNam
400410
Assert.AreEqual("string-c-1-2-3-4.0-True-01052013", result);
401411
}
402412

413+
[Test]
414+
public void BindsEnumValuesWithPEPStyleNaming([Values] bool useSnakeCased)
415+
{
416+
using (Py.GIL())
417+
{
418+
var module = PyModule.FromString("module", $@"
419+
from clr import AddReference
420+
AddReference(""Python.EmbeddingTest"")
421+
422+
from Python.EmbeddingTest import *
423+
424+
def SetEnumValue1(obj):
425+
obj.EnumValue = ClassManagerTests.SnakeCaseEnum.EnumValue1
426+
427+
def SetEnumValue2(obj):
428+
obj.EnumValue = ClassManagerTests.SnakeCaseEnum.EnumValue2
429+
430+
def SetEnumValue3(obj):
431+
obj.EnumValue = ClassManagerTests.SnakeCaseEnum.EnumValue3
432+
433+
def SetEnumValue1SnakeCase(obj):
434+
obj.enum_value = ClassManagerTests.SnakeCaseEnum.ENUM_VALUE1
435+
436+
def SetEnumValue2SnakeCase(obj):
437+
obj.enum_value = ClassManagerTests.SnakeCaseEnum.ENUM_VALUE2
438+
439+
def SetEnumValue3SnakeCase(obj):
440+
obj.enum_value = ClassManagerTests.SnakeCaseEnum.ENUM_VALUE3
441+
");
442+
443+
using var obj = new SnakeCaseNamesTesClass().ToPython();
444+
445+
if (useSnakeCased)
446+
{
447+
module.InvokeMethod("SetEnumValue1SnakeCase", obj);
448+
Assert.AreEqual(SnakeCaseEnum.EnumValue1, obj.GetAttr("enum_value").As<SnakeCaseEnum>());
449+
module.InvokeMethod("SetEnumValue2SnakeCase", obj);
450+
Assert.AreEqual(SnakeCaseEnum.EnumValue2, obj.GetAttr("enum_value").As<SnakeCaseEnum>());
451+
module.InvokeMethod("SetEnumValue3SnakeCase", obj);
452+
Assert.AreEqual(SnakeCaseEnum.EnumValue3, obj.GetAttr("enum_value").As<SnakeCaseEnum>());
453+
}
454+
else
455+
{
456+
module.InvokeMethod("SetEnumValue1", obj);
457+
Assert.AreEqual(SnakeCaseEnum.EnumValue1, obj.GetAttr("EnumValue").As<SnakeCaseEnum>());
458+
module.InvokeMethod("SetEnumValue2", obj);
459+
Assert.AreEqual(SnakeCaseEnum.EnumValue2, obj.GetAttr("EnumValue").As<SnakeCaseEnum>());
460+
module.InvokeMethod("SetEnumValue3", obj);
461+
Assert.AreEqual(SnakeCaseEnum.EnumValue3, obj.GetAttr("EnumValue").As<SnakeCaseEnum>());
462+
}
463+
}
464+
}
465+
403466
#endregion
404467
}
405468

src/runtime/ClassManager.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -515,8 +515,13 @@ private static ClassInfo GetClassInfo(Type type, ClassBase impl)
515515
}
516516
ob = new FieldObject(fi);
517517
ci.members[mi.Name] = ob.AllocObject();
518-
// TODO: Upper-case constants?
519-
ci.members[mi.Name.ToSnakeCase()] = ob.AllocObject();
518+
519+
var pepName = fi.Name.ToSnakeCase();
520+
if (fi.IsLiteral || fi.IsInitOnly)
521+
{
522+
pepName = pepName.ToUpper();
523+
}
524+
ci.members[pepName] = ob.AllocObject();
520525
continue;
521526

522527
case MemberTypes.Event:

src/runtime/MethodBinder.cs

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -452,9 +452,9 @@ internal Binding Bind(BorrowedReference inst, BorrowedReference args, BorrowedRe
452452
// Relevant method variables
453453
var mi = methodInformation.MethodBase;
454454
var pi = methodInformation.ParameterInfo;
455+
var paramNames = methodInformation.ParametersNames;
455456
int pyArgCount = (int)Runtime.PyTuple_Size(args);
456457

457-
458458
// Special case for operators
459459
bool isOperator = OperatorMethod.IsOperatorMethod(mi);
460460
// Binary operator methods will have 2 CLR args but only one Python arg
@@ -479,15 +479,12 @@ internal Binding Bind(BorrowedReference inst, BorrowedReference args, BorrowedRe
479479

480480
// Must be done after IsOperator section
481481
int clrArgCount = pi.Length;
482-
var parametersSnakeCasedNames = kwArgDict == null || methodInformation.IsOriginal
483-
? null
484-
: pi.Select(p => p.Name.ToSnakeCase()).ToArray();
485482

486483
if (CheckMethodArgumentsMatch(clrArgCount,
487484
pyArgCount,
488485
kwArgDict,
489486
pi,
490-
parametersSnakeCasedNames,
487+
paramNames,
491488
out bool paramsArray,
492489
out ArrayList defaultArgList))
493490
{
@@ -506,13 +503,8 @@ internal Binding Bind(BorrowedReference inst, BorrowedReference args, BorrowedRe
506503
object arg; // Python -> Clr argument
507504

508505
// Check our KWargs for this parameter
509-
var hasNamedParam = false;
510-
if (kwArgDict != null)
511-
{
512-
var paramName = methodInformation.IsOriginal ? parameter.Name : parametersSnakeCasedNames[paramIndex];
513-
hasNamedParam = kwArgDict.TryGetValue(paramName, out tempPyObject);
514-
}
515-
if(tempPyObject != null)
506+
bool hasNamedParam = kwArgDict == null ? false : kwArgDict.TryGetValue(paramNames[paramIndex], out tempPyObject);
507+
if (tempPyObject != null)
516508
{
517509
op = tempPyObject;
518510
}
@@ -776,11 +768,16 @@ static BorrowedReference HandleParamsArray(BorrowedReference args, int arrayStar
776768
/// This helper method will perform an initial check to determine if we found a matching
777769
/// method based on its parameters count and type <see cref="Bind(IntPtr,IntPtr,IntPtr,MethodBase,MethodInfo[])"/>
778770
/// </summary>
771+
/// <remarks>
772+
/// We required both the parameters info and the parameters names to perform this check.
773+
/// The CLR method parameters info is required to match the parameters count and type.
774+
/// The names are required to perform an accurate match, since the method can be the snake-cased version.
775+
/// </remarks>
779776
private bool CheckMethodArgumentsMatch(int clrArgCount,
780777
int pyArgCount,
781778
Dictionary<string, PyObject> kwargDict,
782779
ParameterInfo[] parameterInfo,
783-
string[] parametersSnakeCasedNames,
780+
string[] parameterNames,
784781
out bool paramsArray,
785782
out ArrayList defaultArgList)
786783
{
@@ -803,9 +800,7 @@ private bool CheckMethodArgumentsMatch(int clrArgCount,
803800
{
804801
// If the method doesn't have all of these kw args, it is not a match
805802
// Otherwise just continue on to see if it is a match
806-
if (!kwargDict.All(x => parametersSnakeCasedNames == null
807-
? parameterInfo.Any(pi => x.Key == pi.Name)
808-
: parametersSnakeCasedNames.Any(paramName => x.Key == paramName)))
803+
if (!kwargDict.All(x => parameterNames.Any(paramName => x.Key == paramName)))
809804
{
810805
return false;
811806
}
@@ -825,7 +820,7 @@ private bool CheckMethodArgumentsMatch(int clrArgCount,
825820
defaultArgList = new ArrayList();
826821
for (var v = pyArgCount; v < clrArgCount && match; v++)
827822
{
828-
if (kwargDict != null && kwargDict.ContainsKey(parametersSnakeCasedNames == null ? parameterInfo[v].Name : parametersSnakeCasedNames[v]))
823+
if (kwargDict != null && kwargDict.ContainsKey(parameterNames[v]))
829824
{
830825
// we have a keyword argument for this parameter,
831826
// no need to check for a default parameter, but put a null
@@ -996,6 +991,8 @@ internal class MethodInformation
996991

997992
public bool IsOriginal { get; }
998993

994+
public string[] ParametersNames { get; }
995+
999996
public MethodInformation(MethodBase methodBase, ParameterInfo[] parameterInfo)
1000997
: this(methodBase, parameterInfo, true)
1001998
{
@@ -1006,6 +1003,15 @@ public MethodInformation(MethodBase methodBase, ParameterInfo[] parameterInfo, b
10061003
MethodBase = methodBase;
10071004
ParameterInfo = parameterInfo;
10081005
IsOriginal = isOriginal;
1006+
1007+
if (isOriginal)
1008+
{
1009+
ParametersNames = ParameterInfo.Select(pi => pi.Name).ToArray();
1010+
}
1011+
else
1012+
{
1013+
ParametersNames = ParameterInfo.Select(pi => pi.Name.ToSnakeCase()).ToArray();
1014+
}
10091015
}
10101016

10111017
public override string ToString()

0 commit comments

Comments
 (0)