From cf915c3c8892f73a7f8cf04c1d98ad4021c88d57 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Fri, 1 Nov 2019 23:52:38 -0700 Subject: [PATCH] Fix null ref in super() (#1739) * Remove stale reference * Super declaring module * Persistence * Use builtins --- .../BuiltinsSpecializations.cs | 6 +-- src/Analysis/Ast/Impl/Types/LocatedMember.cs | 6 ++- src/Analysis/Ast/Impl/Types/Location.cs | 4 +- .../Ast/Impl/Types/PythonSuperType.cs | 37 +++++++++---------- src/Caching/Impl/ModuleFactory.cs | 2 +- 5 files changed, 28 insertions(+), 27 deletions(-) diff --git a/src/Analysis/Ast/Impl/Specializations/BuiltinsSpecializations.cs b/src/Analysis/Ast/Impl/Specializations/BuiltinsSpecializations.cs index e439fbfd3..fed1cc41b 100644 --- a/src/Analysis/Ast/Impl/Specializations/BuiltinsSpecializations.cs +++ b/src/Analysis/Ast/Impl/Specializations/BuiltinsSpecializations.cs @@ -100,7 +100,7 @@ public static IMember Super(IPythonModule declaringModule, IPythonFunctionOverlo //Zero argument form only works inside a class definition foreach (var s in argSet.Eval.CurrentScope.EnumerateTowardsGlobal.Where(s => s.Node is ClassDefinition)) { var classType = s.Variables["__class__"].GetPythonType(); - return PythonSuperType.CreateSuper(classType)?.CreateInstance(argSet); + return PythonSuperType.Create(classType)?.CreateInstance(argSet); } return null; } @@ -114,14 +114,14 @@ public static IMember Super(IPythonModule declaringModule, IPythonFunctionOverlo // second argument optional bool isUnbound = args.Count == 1; if (isUnbound) { - return PythonSuperType.CreateSuper(firstCls)?.CreateInstance(argSet); + return PythonSuperType.Create(firstCls)?.CreateInstance(argSet); } var secondCls = args[1].GetPythonType(); if (secondCls?.Equals(firstCls) == true || secondCls?.IsSubClassOf(firstCls) == true) { // We walk the mro of the second parameter looking for the first - return PythonSuperType.CreateSuper(secondCls, typeToFind: firstCls)?.CreateInstance(argSet); + return PythonSuperType.Create(secondCls, typeToFind: firstCls)?.CreateInstance(argSet); } return null; diff --git a/src/Analysis/Ast/Impl/Types/LocatedMember.cs b/src/Analysis/Ast/Impl/Types/LocatedMember.cs index ca863d95b..1d11632dd 100644 --- a/src/Analysis/Ast/Impl/Types/LocatedMember.cs +++ b/src/Analysis/Ast/Impl/Types/LocatedMember.cs @@ -18,12 +18,16 @@ using System.Linq; using Microsoft.Python.Analysis.Modules; using Microsoft.Python.Core; +using Microsoft.Python.Core.Diagnostics; namespace Microsoft.Python.Analysis.Types { internal abstract class LocatedMember : ILocatedMember { private HashSet _references; - protected LocatedMember(IPythonModule module) : this(new Location(module)) { } + protected LocatedMember(IPythonModule module) : this(new Location(module)) { + Check.InvalidOperation(module != null || this is IPythonModule, + "Located member can only have null declaring module if it is the module."); + } protected LocatedMember(Location location) { Location = location; diff --git a/src/Analysis/Ast/Impl/Types/Location.cs b/src/Analysis/Ast/Impl/Types/Location.cs index e7f49e0bd..53060d859 100644 --- a/src/Analysis/Ast/Impl/Types/Location.cs +++ b/src/Analysis/Ast/Impl/Types/Location.cs @@ -18,9 +18,7 @@ namespace Microsoft.Python.Analysis.Types { public readonly struct Location { - public Location(IPythonModule module) : this(module, default) { } - - public Location(IPythonModule module, IndexSpan indexSpan) { + public Location(IPythonModule module, IndexSpan indexSpan = default) { Module = module; IndexSpan = indexSpan; } diff --git a/src/Analysis/Ast/Impl/Types/PythonSuperType.cs b/src/Analysis/Ast/Impl/Types/PythonSuperType.cs index 6f4dcea81..8511c6173 100644 --- a/src/Analysis/Ast/Impl/Types/PythonSuperType.cs +++ b/src/Analysis/Ast/Impl/Types/PythonSuperType.cs @@ -21,12 +21,13 @@ namespace Microsoft.Python.Analysis.Types { internal sealed class PythonSuperType : PythonType, IPythonSuperType { /// - /// more info at https://docs.python.org/3/library/functions.html#super + /// Implements 'super' specialization type. + /// See also https://docs.python.org/3/library/functions.html#super /// - /// - /// Should be a list of IPythonType - public PythonSuperType(IReadOnlyList mro) - : base("super", new Location(), string.Empty, BuiltinTypeId.Type) { + /// The derived class MRO. + /// Builtins module. + public PythonSuperType(IReadOnlyList mro, IBuiltinsPythonModule builtins) + : base("super", new Location(builtins), string.Empty, BuiltinTypeId.Type) { Mro = mro; } @@ -34,21 +35,23 @@ public PythonSuperType(IReadOnlyList mro) public IReadOnlyList Mro { get; } - public override IMember GetMember(string name) => Mro.MaybeEnumerate().Select(c => c.GetMember(name)).ExcludeDefault().FirstOrDefault(); - - public override IEnumerable GetMemberNames() => Mro.MaybeEnumerate().SelectMany(cls => cls.GetMemberNames()).Distinct(); + public override IMember GetMember(string name) + => Mro.MaybeEnumerate().Select(c => c.GetMember(name)).ExcludeDefault().FirstOrDefault(); + public override IEnumerable GetMemberNames() + => Mro.MaybeEnumerate().SelectMany(cls => cls.GetMemberNames()).Distinct(); /// - /// This will return PythonSuperType with a mro list that starts with the next class in line in classType's mro, - /// or it will search classType's mro for typeToFild then build an mro list for the remaining classses after typeToFind. + /// Creates PythonSuperType with a MRO list that starts with the next class in line in classType's MRO, + /// or searches classType's MRO for , then builds an MRO list for + /// the remaining classes after . /// /// /// /// - internal static PythonSuperType CreateSuper(IPythonClassType classType, IPythonType typeToFind = null) { + internal static PythonSuperType Create(IPythonClassType classType, IPythonType typeToFind = null) { var mro = classType?.Mro ?? Array.Empty(); - if (mro.Count == 0) { + if (classType == null || mro.Count == 0) { return null; } @@ -64,13 +67,9 @@ internal static PythonSuperType CreateSuper(IPythonClassType classType, IPythonT } var nextClassInLine = mro?.FirstOrDefault(); - if (nextClassInLine != null) { - // Skip the first element, super's search starts at the next elemement in the mro for both super() and super(cls, typeToFind) - return new PythonSuperType(mro.Skip(1).ToArray()); - } - - return null; + var builtins = classType.DeclaringModule.Interpreter.ModuleResolution.BuiltinsModule; + // Skip the first element, super's search starts at the next element in the mro for both super() and super(cls, typeToFind) + return nextClassInLine != null ? new PythonSuperType(mro.Skip(1).ToArray(), builtins) : null; } } } - diff --git a/src/Caching/Impl/ModuleFactory.cs b/src/Caching/Impl/ModuleFactory.cs index a7c393aba..c9794a714 100644 --- a/src/Caching/Impl/ModuleFactory.cs +++ b/src/Caching/Impl/ModuleFactory.cs @@ -205,7 +205,7 @@ private IMember GetBuiltinMember(IBuiltinsPythonModule builtins, string memberNa case "Unknown": return builtins.Interpreter.UnknownType; case "SuperType": - return new PythonSuperType(typeArgs); + return new PythonSuperType(typeArgs, builtins); } return builtins.GetMember(memberName); }