diff --git a/src/perf_tests/Python.PerformanceTests.csproj b/src/perf_tests/Python.PerformanceTests.csproj index 24c4793a1..fc92a4851 100644 --- a/src/perf_tests/Python.PerformanceTests.csproj +++ b/src/perf_tests/Python.PerformanceTests.csproj @@ -13,7 +13,7 @@ runtime; build; native; contentfiles; analyzers; buildtransitive - + compile @@ -25,7 +25,7 @@ - + diff --git a/src/runtime/AttributeErrorHint.cs b/src/runtime/AttributeErrorHint.cs index 7faa10996..876b2a838 100644 --- a/src/runtime/AttributeErrorHint.cs +++ b/src/runtime/AttributeErrorHint.cs @@ -40,6 +40,10 @@ internal static class AttributeErrorHint private static IntPtr _hookSlot; // Address of PyObject_GenericGetAttr, used to detect types we may safely redirect. private static IntPtr _genericGetAttr; + // Address of type_getattro (PyType_Type.tp_getattro). The CLR metatype uses it, so we + // allow redirecting it too: that is how attribute access on a reflected type object + // (static members, enum values) gets the miss hook. + private static IntPtr _typeGetAttro; private static bool IsReady => _methodDef != IntPtr.Zero && _hookSlot != IntPtr.Zero; @@ -48,6 +52,7 @@ internal static void Initialize() try { _genericGetAttr = Util.ReadIntPtr(Runtime.PyBaseObjectType, TypeOffset.tp_getattro); + _typeGetAttro = Util.ReadIntPtr(Runtime.PyTypeType, TypeOffset.tp_getattro); if (_methodDef == IntPtr.Zero) { @@ -70,6 +75,11 @@ internal static void Initialize() using var probe = globals["__clr_getattr_probe__"]; _hookSlot = Util.ReadIntPtr(probe.Reference, TypeOffset.tp_getattro); + + // Install the hook on the CLR metatype so that a miss on a reflected type + // object's own attribute (a mistyped static member or enum value, e.g. + // DayOfWeek.Sundey) is enriched the same way instance attribute misses are. + Install(MetaType.ClrMetaTypeReference); } catch (Exception e) { @@ -94,7 +104,12 @@ internal static void Install(BorrowedReference type) return; } - if (Util.ReadIntPtr(type, TypeOffset.tp_getattro) != _genericGetAttr) + var getattro = Util.ReadIntPtr(type, TypeOffset.tp_getattro); + // Only redirect types that still use one of the standard lookups: instances use the + // generic getattr, the CLR metatype uses type_getattro. Types with a custom + // tp_getattro (dynamic objects, modules, interfaces, ...) handle misses themselves + // and are left untouched. + if (getattro != _genericGetAttr && getattro != _typeGetAttro) { return; } @@ -158,6 +173,7 @@ internal static void Shutdown() // are reused by the next Initialize. _hookSlot = IntPtr.Zero; _genericGetAttr = IntPtr.Zero; + _typeGetAttro = IntPtr.Zero; } } } diff --git a/src/runtime/Properties/AssemblyInfo.cs b/src/runtime/Properties/AssemblyInfo.cs index 2a7596eeb..bbd75b3db 100644 --- a/src/runtime/Properties/AssemblyInfo.cs +++ b/src/runtime/Properties/AssemblyInfo.cs @@ -4,5 +4,5 @@ [assembly: InternalsVisibleTo("Python.EmbeddingTest, PublicKey=00240000048000009400000006020000002400005253413100040000110000005ffd8f49fb44ab0641b3fd8d55e749f716e6dd901032295db641eb98ee46063cbe0d4a1d121ef0bc2af95f8a7438d7a80a3531316e6b75c2dae92fb05a99f03bf7e0c03980e1c3cfb74ba690aca2f3339ef329313bcc5dccced125a4ffdc4531dcef914602cd5878dc5fbb4d4c73ddfbc133f840231343e013762884d6143189")] [assembly: InternalsVisibleTo("Python.Test, PublicKey=00240000048000009400000006020000002400005253413100040000110000005ffd8f49fb44ab0641b3fd8d55e749f716e6dd901032295db641eb98ee46063cbe0d4a1d121ef0bc2af95f8a7438d7a80a3531316e6b75c2dae92fb05a99f03bf7e0c03980e1c3cfb74ba690aca2f3339ef329313bcc5dccced125a4ffdc4531dcef914602cd5878dc5fbb4d4c73ddfbc133f840231343e013762884d6143189")] -[assembly: AssemblyVersion("2.0.57")] -[assembly: AssemblyFileVersion("2.0.57")] +[assembly: AssemblyVersion("2.0.58")] +[assembly: AssemblyFileVersion("2.0.58")] diff --git a/src/runtime/Python.Runtime.csproj b/src/runtime/Python.Runtime.csproj index 43988bbf0..9f0476cf7 100644 --- a/src/runtime/Python.Runtime.csproj +++ b/src/runtime/Python.Runtime.csproj @@ -5,7 +5,7 @@ Python.Runtime Python.Runtime QuantConnect.pythonnet - 2.0.57 + 2.0.58 false LICENSE https://github.com/pythonnet/pythonnet diff --git a/src/runtime/Types/ClassBase.cs b/src/runtime/Types/ClassBase.cs index 7e831d17f..b1b89a2aa 100644 --- a/src/runtime/Types/ClassBase.cs +++ b/src/runtime/Types/ClassBase.cs @@ -1,5 +1,6 @@ using System; using System.Collections; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; using System.Linq; @@ -28,6 +29,18 @@ internal class ClassBase : ManagedType, IDeserializationCallback internal readonly Dictionary richcompare = new(); internal MaybeType type; + // Reflecting over a managed type's full member set (with FlattenHierarchy) plus the + // snake_case conversion is expensive, and the result never changes for a given type. + // Compute it once per type. + private static readonly ConcurrentDictionary> _candidateNameCache = new(); + + // A miss-heavy workload probes the same missing names over and over (e.g. a per-bar + // getattr(self, "_optional", None) on a .NET-derived object, or a mistyped enum value). + // Memoize the fully-built " Did you mean: ...?" hint (empty when there is nothing to + // suggest) per (type, missing-name) so repeats are a dictionary lookup instead of an + // O(members) reflection + Levenshtein scan on every miss. + private static readonly ConcurrentDictionary<(Type Type, string Name), string> _suggestionCache = new(); + internal ClassBase(Type tp) { if (tp is null) throw new ArgumentNullException(nameof(type)); @@ -663,26 +676,61 @@ internal static void AppendAttributeErrorSuggestions(BorrowedReference ob, Borro /// internal static string BuildMissingAttributeMessage(PyObject self, string name) { - var typeName = "object"; try { - using var pyType = self.GetPythonType(); - typeName = pyType.Name; + if (TryGetSuggestionTarget(self.Reference, out var type, out var staticScope)) + { + // Match CPython's wording: instances say "'T' object ...", whereas an access + // on the type object itself (a missing static member or enum value) says + // "type object 'T' ...". + var baseMessage = staticScope + ? $"type object '{type!.Name}' has no attribute '{name}'" + : $"'{PythonTypeName(self)}' object has no attribute '{name}'"; + return baseMessage + GetSuggestionHint(type!, name); + } } catch { - // fall back to the generic type name + // never let message building turn into a different exception } - var message = $"'{typeName}' object has no attribute '{name}'"; + return $"'{PythonTypeName(self)}' object has no attribute '{name}'"; + } + + private static string PythonTypeName(PyObject self) + { try { - return message + GetSuggestionHint(self.Reference, name); + using var pyType = self.GetPythonType(); + return pyType.Name; } catch { - // never let suggestion building turn into a different exception - return message; + return "object"; + } + } + + /// + /// Resolves the managed whose members should be searched for a + /// missing-attribute suggestion, and whether the access was on the type object itself + /// ( = true, for static members and enum values) rather + /// than on an instance. Returns false for objects that are not reflected .NET types. + /// + private static bool TryGetSuggestionTarget(BorrowedReference ob, out Type? type, out bool staticScope) + { + type = null; + staticScope = false; + switch (GetManagedObject(ob)) + { + case CLRObject clrObj when clrObj.inst is not null: + type = clrObj.inst.GetType(); + return true; + case ClassBase classBase when classBase.type.Valid: + type = classBase.type.Value; + staticScope = true; + return true; + default: + return false; } } @@ -694,23 +742,28 @@ internal static string BuildMissingAttributeMessage(PyObject self, string name) /// private static string GetSuggestionHint(BorrowedReference ob, string name) { - if (string.IsNullOrEmpty(name) || name.StartsWith("__", StringComparison.Ordinal)) + if (!TryGetSuggestionTarget(ob, out var type, out _)) { return string.Empty; } - if (GetManagedObject(ob) is not CLRObject clrObj || clrObj.inst is null) - { - return string.Empty; - } + return GetSuggestionHint(type!, name); + } - var suggestions = GetSimilarMemberNames(clrObj.inst.GetType(), name); - if (suggestions.Count == 0) + private static string GetSuggestionHint(Type type, string name) + { + if (string.IsNullOrEmpty(name) || name.StartsWith("__", StringComparison.Ordinal)) { return string.Empty; } - return " Did you mean: " + string.Join(", ", suggestions.Select(s => $"'{s}'")) + "?"; + // The hint is built and cached once per (type, name); on a repeated miss this is just + // a dictionary lookup. An empty string means there was nothing to suggest. The + // suggested names use the same snake_case convention Python exposes members under + // (see ToSnakeCaseMemberName), so they are independent of whether the access was on + // an instance or the type object. + return _suggestionCache.GetOrAdd((type, name), + static key => ComputeSimilarMemberNames(key.Type, key.Name)); } private static string GetErrorMessage(BorrowedReference value, string fallbackName) @@ -732,38 +785,52 @@ private static string GetErrorMessage(BorrowedReference value, string fallbackNa return $"object has no attribute '{fallbackName}'"; } - private static List GetSimilarMemberNames(Type type, string name) + // The snake_case candidate member names of a type, cached so the reflection and name + // conversion happen at most once per type rather than on every attribute miss. Instance + // and static members are both included, and each is converted with ToSnakeCaseMemberName + // so the suggestion matches the name Python exposes it under: methods become lower_snake, + // while enum values, consts and static-readonly members become UPPER_SNAKE (e.g. + // DayOfWeek.SUNDAY, Math.PI, String.EMPTY). + private static HashSet GetCandidateMemberNames(Type type) { - const int MaxSuggestions = 5; - var threshold = Math.Max(2, name.Length / 3); - - var seen = new HashSet(StringComparer.Ordinal); - var scored = new List<(string Name, int Distance)>(); - - var members = type.GetMembers(BindingFlags.Public | BindingFlags.Instance - | BindingFlags.Static | BindingFlags.FlattenHierarchy); - foreach (var member in members) + return _candidateNameCache.GetOrAdd(type, static t => { - // Skip property/event accessors, operators and other special-name methods, - // as well as compiler-generated members; none are accessible by name. - if (member is MethodBase { IsSpecialName: true }) - { - continue; - } + var names = new HashSet(StringComparer.Ordinal); - if (member.Name.Length == 0 || member.Name[0] == '<') + var members = t.GetMembers(BindingFlags.Public | BindingFlags.Instance + | BindingFlags.Static | BindingFlags.FlattenHierarchy); + foreach (var member in members) { - continue; - } + // Skip property/event accessors, operators and other special-name methods, + // as well as compiler-generated members; none are accessible by name. + if (member is MethodBase { IsSpecialName: true }) + { + continue; + } - // Suggest the snake_case alias, since that is the fork's PEP8-style - // public API surface (members are exposed in both Pascal and snake case). - var candidate = ToSnakeCaseMemberName(member); - if (!seen.Add(candidate)) - { - continue; + if (member.Name.Length == 0 || member.Name[0] == '<') + { + continue; + } + + names.Add(ToSnakeCaseMemberName(member)); } + return names; + }); + } + + // Builds the " Did you mean: 'x', 'y'?" hint for a missing attribute, or an empty + // string when no member is similar enough to suggest. The result is cached in + // _suggestionCache, so this runs at most once per (type, missing-name). + private static string ComputeSimilarMemberNames(Type type, string name) + { + const int MaxSuggestions = 5; + var threshold = Math.Max(2, name.Length / 3); + + var scored = new List<(string Name, int Distance)>(); + foreach (var candidate in GetCandidateMemberNames(type)) + { var distance = LevenshteinDistance(name, candidate); var related = distance <= threshold || candidate.IndexOf(name, StringComparison.OrdinalIgnoreCase) >= 0 @@ -774,12 +841,18 @@ private static List GetSimilarMemberNames(Type type, string name) } } - return scored + if (scored.Count == 0) + { + return string.Empty; + } + + var suggestions = scored .OrderBy(t => t.Distance) .ThenBy(t => t.Name, StringComparer.OrdinalIgnoreCase) .Take(MaxSuggestions) - .Select(t => t.Name) - .ToList(); + .Select(t => $"'{t.Name}'"); + + return " Did you mean: " + string.Join(", ", suggestions) + "?"; } private static string ToSnakeCaseMemberName(MemberInfo member) diff --git a/src/runtime/Types/MetaType.cs b/src/runtime/Types/MetaType.cs index 9a66240d3..36a1a4b40 100644 --- a/src/runtime/Types/MetaType.cs +++ b/src/runtime/Types/MetaType.cs @@ -26,6 +26,13 @@ internal sealed class MetaType : ManagedType "__subclasscheck__", }; + /// + /// The CLR metatype object. Reflected .NET types are instances of it, so wiring the + /// AttributeError miss hook here enriches misses on a type object's own attributes + /// (static members and enum values). + /// + internal static BorrowedReference ClrMetaTypeReference => PyCLRMetaType.Reference; + /// /// Metatype initialization. This bootstraps the CLR metatype to life. /// diff --git a/tests/test_class.py b/tests/test_class.py index bfa40714c..7bdaa65c4 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -96,6 +96,25 @@ def test_missing_attribute_no_similar_members(): assert "Did you mean" not in message +def test_missing_attribute_suggestion_is_cached_and_stable(): + """Repeated misses of the same attribute must return identical suggestions. + + The suggestion list is memoized per (type, name) so a miss-heavy workload does + not re-run the reflection + Levenshtein scan on every access. The cached result + must stay correct and identical across repeated lookups. + """ + s = System.String("this is a test") + + messages = [] + for _ in range(3): + with pytest.raises(AttributeError) as exc_info: + _ = s.lenght + messages.append(str(exc_info.value)) + + assert all("Did you mean" in m and "length" in m for m in messages) + assert messages[0] == messages[1] == messages[2] + + def test_missing_attribute_hasattr_still_false(): """Enriching the AttributeError must not break hasattr() (it must stay False).""" s = System.String("this is a test") @@ -104,6 +123,66 @@ def test_missing_attribute_hasattr_still_false(): assert hasattr(s, "Length") +def test_missing_static_method_suggests_similar(): + """A mistyped static method on a type object suggests the similar member.""" + from System import Math + + with pytest.raises(AttributeError) as exc_info: + _ = Math.Sqrtt + + message = str(exc_info.value) + assert "type object 'Math'" in message + assert "Sqrtt" in message + assert "Did you mean" in message + # Methods are exposed lower_snake, so the suggestion is 'sqrt'. Quoted so the assertion + # matches the suggestion, not the typo 'Sqrtt'. + assert "'sqrt'" in message + + +def test_missing_static_const_suggests_similar(): + """A mistyped static const (Math.PI) suggests the UPPER_SNAKE constant name.""" + from System import Math + + with pytest.raises(AttributeError) as exc_info: + _ = Math.PII + + message = str(exc_info.value) + assert "Did you mean" in message + # Consts are exposed UPPER_SNAKE; quoted so it matches the suggestion, not the typo 'PII'. + assert "'PI'" in message + + +def test_missing_static_field_suggests_similar(): + """A mistyped static-readonly field (String.Empty) suggests the UPPER_SNAKE name.""" + with pytest.raises(AttributeError) as exc_info: + _ = System.String.Empy + + message = str(exc_info.value) + assert "Did you mean" in message + # static-readonly fields are exposed UPPER_SNAKE -> String.EMPTY. + assert "'EMPTY'" in message + + +def test_missing_static_member_no_similar(): + """A static member with no similar name keeps the standard message (no hint).""" + from System import Math + + with pytest.raises(AttributeError) as exc_info: + _ = Math.Zzzzzz + + message = str(exc_info.value) + assert "Zzzzzz" in message + assert "Did you mean" not in message + + +def test_missing_static_member_hasattr_still_false(): + """The type-object miss hook must not break hasattr() on a type.""" + from System import Math + + assert hasattr(Math, "Sqrt") + assert not hasattr(Math, "Sqrtt") + + def test_missing_attribute_hook_is_native(): """The __getattr__ hook must be a native method descriptor. diff --git a/tests/test_enum.py b/tests/test_enum.py index f7cff4a7e..4c15a431e 100644 --- a/tests/test_enum.py +++ b/tests/test_enum.py @@ -31,6 +31,43 @@ def test_enum_get_member(): assert DayOfWeek.Saturday == DayOfWeek(6) +def test_missing_enum_member_suggests_similar(): + """A mistyped enum member suggests the correct member by its .NET name.""" + from System import DayOfWeek + + with pytest.raises(AttributeError) as exc_info: + _ = DayOfWeek.Sundey + + message = str(exc_info.value) + # Access on the type object itself uses the "type object 'T'" wording. + assert "type object 'DayOfWeek'" in message + assert "Sundey" in message + assert "Did you mean" in message + # Enum values are exposed in UPPER_SNAKE (the fork's PEP8 constant convention), so that is + # the form suggested -- DayOfWeek.SUNDAY, not 'Sunday'. + assert "'SUNDAY'" in message + + +def test_missing_enum_member_no_similar(): + """An enum member with no similar name keeps the standard message (no hint).""" + from System import DayOfWeek + + with pytest.raises(AttributeError) as exc_info: + _ = DayOfWeek.Xyzzy + + message = str(exc_info.value) + assert "Xyzzy" in message + assert "Did you mean" not in message + + +def test_missing_enum_member_hasattr_still_false(): + """Enriching the AttributeError must not break hasattr() on enum types.""" + from System import DayOfWeek + + assert hasattr(DayOfWeek, "Sunday") + assert not hasattr(DayOfWeek, "Sundey") + + def test_byte_enum(): """Test byte enum.""" assert Test.ByteEnum.Zero == Test.ByteEnum(0)