diff --git a/src/runtime/Types/ClassBase.cs b/src/runtime/Types/ClassBase.cs index 5f70f18c6..1342b6a3f 100644 --- a/src/runtime/Types/ClassBase.cs +++ b/src/runtime/Types/ClassBase.cs @@ -29,10 +29,21 @@ internal class ClassBase : ManagedType, IDeserializationCallback internal readonly Dictionary richcompare = new(); internal MaybeType type; + // How a member is used from Python, so a missing-attribute hint only suggests members + // usable the same way as the one the user most likely meant. Nested types count as + // callable: `Foo.Bar()` may be an attempted constructor call. A single exposed name can + // carry both flags when e.g. a method and a property collapse to the same snake_case name. + [Flags] + private enum SuggestionKind + { + Callable = 1, + Data = 2, + } + // 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(); + 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). @@ -760,8 +771,8 @@ private static string GetSuggestionHint(Type type, string name) // 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 + // suggested names use the same convention Python exposes members under (see + // GetCandidateMemberNames), 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)); @@ -786,17 +797,18 @@ private static string GetErrorMessage(BorrowedReference value, string fallbackNa return $"object has no attribute '{fallbackName}'"; } - // 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) + // The 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, enum + // values, consts and static-readonly members become UPPER_SNAKE (e.g. DayOfWeek.SUNDAY, + // Math.PI, String.EMPTY), and nested types keep their original name. Each name is tagged + // with how it is used from Python so suggestions can be filtered by usage. + private static Dictionary GetCandidateMemberNames(Type type) { return _candidateNameCache.GetOrAdd(type, static t => { - var names = new HashSet(StringComparer.Ordinal); + var names = new Dictionary(StringComparer.Ordinal); var members = t.GetMembers(BindingFlags.Public | BindingFlags.Instance | BindingFlags.Static | BindingFlags.FlattenHierarchy); @@ -814,7 +826,8 @@ private static HashSet GetCandidateMemberNames(Type type) continue; } - names.Add(ToSnakeCaseMemberName(member)); + var (name, kind) = ToSnakeCaseMemberName(member); + names[name] = names.TryGetValue(name, out var existing) ? existing | kind : kind; } return names; @@ -829,16 +842,16 @@ 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)>(); + var scored = new List<(string Name, int Distance, SuggestionKind Kind)>(); foreach (var candidate in GetCandidateMemberNames(type)) { - var distance = LevenshteinDistance(name, candidate); + var distance = LevenshteinDistance(name, candidate.Key); var related = distance <= threshold - || candidate.IndexOf(name, StringComparison.OrdinalIgnoreCase) >= 0 - || name.IndexOf(candidate, StringComparison.OrdinalIgnoreCase) >= 0; + || candidate.Key.IndexOf(name, StringComparison.OrdinalIgnoreCase) >= 0 + || name.IndexOf(candidate.Key, StringComparison.OrdinalIgnoreCase) >= 0; if (related) { - scored.Add((candidate, distance)); + scored.Add((candidate.Key, distance, candidate.Value)); } } @@ -847,24 +860,38 @@ private static string ComputeSimilarMemberNames(Type type, string name) return string.Empty; } - var suggestions = scored + var ordered = scored .OrderBy(t => t.Distance) .ThenBy(t => t.Name, StringComparer.OrdinalIgnoreCase) + .ToList(); + + // Only suggest members used the same way as the closest match, the member the user + // most likely meant: a miss that best matches a method or nested type gets callable + // suggestions only, one that best matches a field/property gets data suggestions + // only. Mixing the two would suggest names the caller cannot use the same way. + var kind = ordered[0].Kind; + var suggestions = ordered + .Where(t => (t.Kind & kind) != 0) .Take(MaxSuggestions) .Select(t => $"'{t.Name}'"); return " Did you mean: " + string.Join(", ", suggestions) + "?"; } - private static string ToSnakeCaseMemberName(MemberInfo member) + // Converts a member to the name Python exposes it under, tagged with how it is used. + // The field/property overloads of ToSnakeCase are used so const and static-readonly + // members are converted to UPPER_CASE. Nested types keep their original name verbatim: + // ClassManager registers no snake_case alias for them, and they count as callable since + // accessing one may be an attempted constructor call. + private static (string Name, SuggestionKind Kind) ToSnakeCaseMemberName(MemberInfo member) { - // Use the field/property overloads so const and static-readonly members - // are converted to UPPER_CASE, matching how they are exposed to Python. return member switch { - FieldInfo fieldInfo => fieldInfo.ToSnakeCase(), - PropertyInfo propertyInfo => propertyInfo.ToSnakeCase(), - _ => member.Name.ToSnakeCase(), + Type => (member.Name, SuggestionKind.Callable), + MethodBase => (member.Name.ToSnakeCase(), SuggestionKind.Callable), + FieldInfo fieldInfo => (fieldInfo.ToSnakeCase(), SuggestionKind.Data), + PropertyInfo propertyInfo => (propertyInfo.ToSnakeCase(), SuggestionKind.Data), + _ => (member.Name.ToSnakeCase(), SuggestionKind.Data), }; } diff --git a/src/testing/classtest.cs b/src/testing/classtest.cs index 68c0d8c55..0c726e866 100644 --- a/src/testing/classtest.cs +++ b/src/testing/classtest.cs @@ -59,4 +59,32 @@ public ClassCtorTest2(string v) internal class InternalClass { } + + /// + /// Supports missing-attribute suggestion ("Did you mean") unit tests: a nested type, + /// a method and a property with deliberately similar names, so tests can assert that + /// suggestions are filtered by how the intended member is used from Python. + /// + public class SuggestionTest + { + public static class Calculator + { + public static int Add(int a, int b) + { + return a + b; + } + } + + public static int Calculate() + { + return 0; + } + + public static int[] CalculationResults() + { + return new int[0]; + } + + public static int CalculationResult { get; set; } + } } diff --git a/tests/test_class.py b/tests/test_class.py index 7bdaa65c4..df374af92 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -163,6 +163,54 @@ def test_missing_static_field_suggests_similar(): assert "'EMPTY'" in message +def test_missing_nested_type_suggests_original_name(): + """A miss that matches a nested type suggests its original PascalCase name. + + Nested types are exposed under their original name only (no snake_case alias), + so suggesting the snake-cased name would point at another missing attribute. + """ + from Python.Test import SuggestionTest + + with pytest.raises(AttributeError) as exc_info: + _ = SuggestionTest.calculator + + message = str(exc_info.value) + assert "Did you mean" in message + hint = message.split("Did you mean")[1] + assert "'Calculator'" in hint + # The snake-cased nested type name is not accessible, so it must not be suggested. + assert "'calculator'" not in hint + + +def test_missing_method_suggests_callables_only(): + """A miss that best matches a method suggests methods and nested types only. + + Nested types are included because the access may be an attempted constructor + call, but similarly-named properties are excluded: they are not callable. + """ + from Python.Test import SuggestionTest + + with pytest.raises(AttributeError) as exc_info: + _ = SuggestionTest.calculat + + hint = str(exc_info.value).split("Did you mean")[1] + assert "'calculate'" in hint + assert "'Calculator'" in hint + assert "'calculation_result'" not in hint + + +def test_missing_property_suggests_data_only(): + """A miss that best matches a property suggests fields/properties only.""" + from Python.Test import SuggestionTest + + with pytest.raises(AttributeError) as exc_info: + _ = SuggestionTest.calculation_resul + + hint = str(exc_info.value).split("Did you mean")[1] + assert "'calculation_result'" in hint + assert "'calculation_results'" not in hint + + def test_missing_static_member_no_similar(): """A static member with no similar name keeps the standard message (no hint).""" from System import Math