Skip to content
This repository was archived by the owner on Apr 14, 2022. It is now read-only.

Commit b6eb8ff

Browse files
author
Mikhail Arkhipov
authored
Clean up type system (#439)
* Types hierarchy cleanup * Test fixes, part I * Restore IPythonBoundFunction * Fix function instance types * More fixes * Fix overload handling * Fix properties * Fix type factory * Remove class built-in type * Simplify builtins, remove obsolete method * Fix typeshed merge * Allow declaring module to be null * Correct assertion * Simplify functions/methods * Fix stub merge tests * Fix tests * Baselines * Null ref * Build break * Overrides signatures * Fix typeinfo tests * PR feedback * Better handle overloads * Build breaks * Baselines * PR fix * PR feedback * Merge conflict * Fix #446 * Fix #446
1 parent 80ce249 commit b6eb8ff

File tree

74 files changed

+1164
-1548
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

74 files changed

+1164
-1548
lines changed

‎src/Analysis/Engine/Impl/AnalysisValueSetExtensions.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ internal static AnalysisValue GetUnionType(this IAnalysisSet types) {
218218
/// Gets instance representations of all members of the set.
219219
/// </summary>
220220
public static IAnalysisSet GetInstanceType(this IAnalysisSet types)
221-
=> AnalysisSet.Create(types.SelectMany(ns => (ns.PythonType as IPythonType2)?.IsClass == true ? ns : ns.GetInstanceType()));
221+
=> AnalysisSet.Create(types.SelectMany(ns => ns.PythonType?.IsTypeFactory == true ? ns : ns.GetInstanceType()));
222222

223223
public static bool IsUnknown(this IAnalysisSet res) {
224224
return res == null ||

‎src/Analysis/Engine/Impl/Analyzer/DDG.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
// THIS CODE IS PROVIDED ON AN *AS IS* BASIS, WITHOUT WARRANTIES OR CONDITIONS
1010
// OF ANY KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY
1111
// IMPLIED WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE,
12-
// MERCHANTABLITY OR NON-INFRINGEMENT.
12+
// MERCHANTABILITY OR NON-INFRINGEMENT.
1313
//
1414
// See the Apache Version 2.0 License for specific language governing
1515
// permissions and limitations under the License.

‎src/Analysis/Engine/Impl/Definitions/IOverloadResult.cs

+23-4
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,40 @@
99
// THIS CODE IS PROVIDED ON AN *AS IS* BASIS, WITHOUT WARRANTIES OR CONDITIONS
1010
// OF ANY KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY
1111
// IMPLIED WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE,
12-
// MERCHANTABLITY OR NON-INFRINGEMENT.
12+
// MERCHANTABILITY OR NON-INFRINGEMENT.
1313
//
1414
// See the Apache Version 2.0 License for specific language governing
1515
// permissions and limitations under the License.
1616

1717
using System.Collections.Generic;
18-
using System.Diagnostics.CodeAnalysis;
1918

2019
namespace Microsoft.PythonTools.Analysis {
2120
public interface IOverloadResult {
21+
/// <summary>
22+
/// Function name.
23+
/// </summary>
2224
string Name { get; }
25+
26+
/// <summary>
27+
/// Function documentation.
28+
/// </summary>
2329
string Documentation { get; }
24-
[SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays",
25-
Justification = "breaking change")]
30+
31+
/// <summary>
32+
/// First parameter if removed from the set.
33+
/// Typically 'self' or 'cls'.
34+
/// </summary>
35+
ParameterResult FirstParameter { get; }
36+
37+
/// <summary>
38+
/// Function parameters. First parameter may be removed, in which case
39+
/// it is present as <see cref="FirstParameter"/>.
40+
/// </summary>
2641
ParameterResult[] Parameters { get; }
42+
43+
/// <summary>
44+
/// Possible return types.
45+
/// </summary>
2746
IReadOnlyList<string> ReturnType { get; }
2847
}
2948
}

‎src/Analysis/Engine/Impl/EmptyBuiltinModule.cs

+3-36
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,11 @@
1616

1717
using System.Collections.Generic;
1818
using Microsoft.PythonTools.Interpreter;
19+
using Microsoft.PythonTools.Interpreter.Ast;
1920

2021
namespace Microsoft.PythonTools.Analysis {
21-
class EmptyBuiltinModule : IBuiltinPythonModule {
22-
private readonly string _name;
23-
24-
public EmptyBuiltinModule(string name) {
25-
_name = name;
26-
}
22+
class EmptyBuiltinModule : PythonModuleType, IBuiltinPythonModule {
23+
public EmptyBuiltinModule(string name): base(name) { }
2724

2825
#region IBuiltinPythonModule Members
2926

@@ -35,42 +32,12 @@ public IMember GetAnyMember(string name) {
3532

3633
#region IPythonModule Members
3734

38-
public string Name {
39-
get { return _name; }
40-
}
41-
4235
public IEnumerable<string> GetChildrenModules() {
4336
yield break;
4437
}
4538

4639
public void Imported(IModuleContext context) {
4740
}
48-
49-
public string Documentation {
50-
get { return string.Empty; }
51-
}
52-
53-
#endregion
54-
55-
#region IMemberContainer Members
56-
57-
public IMember GetMember(IModuleContext context, string name) {
58-
return null;
59-
}
60-
61-
public IEnumerable<string> GetMemberNames(IModuleContext moduleContext) {
62-
yield break;
63-
}
64-
6541
#endregion
66-
67-
#region IMember Members
68-
69-
public PythonMemberType MemberType {
70-
get { return PythonMemberType.Module; }
71-
}
72-
73-
#endregion
74-
7542
}
7643
}

‎src/Analysis/Engine/Impl/Infrastructure/Extensions/EnumerableExtensions.cs

+8-18
Original file line numberDiff line numberDiff line change
@@ -23,31 +23,21 @@ static class EnumerableExtensions {
2323
public static bool IsNullOrEmpty<T>(this IEnumerable<T> source)
2424
=> source == null || !source.Any();
2525

26-
public static T[] MaybeEnumerate<T>(this T[] source) {
27-
return source ?? Array.Empty<T>();
28-
}
26+
public static T[] MaybeEnumerate<T>(this T[] source) => source ?? Array.Empty<T>();
2927

30-
public static IEnumerable<T> MaybeEnumerate<T>(this IEnumerable<T> source) {
31-
return source ?? Enumerable.Empty<T>();
32-
}
28+
public static IEnumerable<T> MaybeEnumerate<T>(this IEnumerable<T> source) => source ?? Enumerable.Empty<T>();
3329

34-
private static T Identity<T>(T source) {
35-
return source;
36-
}
30+
private static T Identity<T>(T source) => source;
3731

38-
public static IEnumerable<T> SelectMany<T>(this IEnumerable<IEnumerable<T>> source) {
39-
return source.SelectMany(Identity);
40-
}
32+
public static IEnumerable<T> SelectMany<T>(this IEnumerable<IEnumerable<T>> source) => source.SelectMany(Identity);
4133

42-
public static IEnumerable<T> Ordered<T>(this IEnumerable<T> source) {
43-
return source.OrderBy(Identity);
44-
}
34+
public static IEnumerable<T> Ordered<T>(this IEnumerable<T> source)
35+
=> source.OrderBy(Identity);
4536

4637
private static bool NotNull<T>(T obj) where T : class => obj != null;
4738

48-
public static IEnumerable<T> WhereNotNull<T>(this IEnumerable<T> source) where T : class {
49-
return source.Where(NotNull);
50-
}
39+
public static IEnumerable<T> WhereNotNull<T>(this IEnumerable<T> source) where T : class
40+
=> source != null ? source.Where(NotNull) : Enumerable.Empty<T>();
5141

5242
public static bool SetEquals<T>(this IEnumerable<T> source, IEnumerable<T> other,
5343
IEqualityComparer<T> comparer = null) where T : class {

‎src/Analysis/Engine/Impl/Interpreter/Ast/AstAnalysisFunctionWalker.cs

+9-3
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@ public void Walk() {
6565
var self = GetSelf();
6666
_selfType = (self as AstPythonConstant)?.Type as AstPythonType;
6767

68-
_overload.ReturnTypes.AddRange(_scope.GetTypesFromAnnotation(Target.ReturnAnnotation).ExcludeDefault());
68+
var annotationTypes = _scope.GetTypesFromAnnotation(Target.ReturnAnnotation).ExcludeDefault();
69+
_overload.ReturnTypes.AddRange(annotationTypes);
70+
6971
_scope.PushScope();
7072

7173
// Declare self, if any
@@ -84,7 +86,10 @@ public void Walk() {
8486
_scope.SetInScope(p.Name, value ?? _scope.UnknownType);
8587
}
8688

87-
Target.Walk(this);
89+
// return type from the annotation always wins, no need to walk the body.
90+
if (!annotationTypes.Any()) {
91+
Target.Walk(this);
92+
}
8893
_scope.PopScope();
8994
}
9095

@@ -165,7 +170,8 @@ public override bool Walk(IfStatement node) {
165170
if (name != null && typeName != null) {
166171
var typeId = typeName.GetTypeId();
167172
if (typeId != BuiltinTypeId.Unknown) {
168-
_scope.SetInScope(name, new AstPythonConstant(new AstPythonBuiltinType(typeName, typeId)));
173+
_scope.SetInScope(name,
174+
new AstPythonConstant(new AstPythonType(typeName, typeId)));
169175
}
170176
}
171177
}

‎src/Analysis/Engine/Impl/Interpreter/Ast/AstAnalysisWalker.cs

+27-22
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ internal LocationInfo GetLoc(ClassDefinition node) {
115115
private LocationInfo GetLoc(Node node) => _scope.GetLoc(node);
116116

117117
private IMember Clone(IMember member) =>
118-
member is IPythonMultipleMembers mm ? AstPythonMultipleMembers.Create(mm.Members) :
118+
member is IPythonMultipleMembers mm ? AstPythonMultipleMembers.Create(mm.GetMembers()) :
119119
member;
120120

121121
public override bool Walk(AssignmentStatement node) {
@@ -423,14 +423,18 @@ public override bool Walk(FunctionDefinition node) {
423423
var dec = (node.Decorators?.Decorators).MaybeEnumerate().ExcludeDefault();
424424
foreach (var d in dec) {
425425
var obj = _scope.GetValueFromExpression(d);
426+
427+
var declaringType = obj as IPythonType;
428+
var declaringModule = declaringType?.DeclaringModule;
429+
426430
if (obj == _interpreter.GetBuiltinType(BuiltinTypeId.Property)) {
427-
AddProperty(node);
431+
AddProperty(node, declaringModule, declaringType);
428432
return false;
429433
}
430-
var mod = (obj as IPythonType)?.DeclaringModule ?? (obj as IPythonFunction)?.DeclaringModule;
431-
var name = (obj as IPythonType)?.Name ?? (obj as IPythonFunction)?.Name;
432-
if (mod?.Name == "abc" && name == "abstractproperty") {
433-
AddProperty(node);
434+
435+
var name = declaringType?.Name;
436+
if (declaringModule?.Name == "abc" && name == "abstractproperty") {
437+
AddProperty(node, declaringModule, declaringType);
434438
return false;
435439
}
436440
}
@@ -450,10 +454,10 @@ public override bool Walk(FunctionDefinition node) {
450454
return false;
451455
}
452456

453-
private void AddProperty(FunctionDefinition node) {
457+
private void AddProperty(FunctionDefinition node, IPythonModule declaringModule, IPythonType declaringType) {
454458
var existing = _scope.LookupNameInScopes(node.Name, NameLookupContext.LookupOptions.Local) as AstPythonProperty;
455459
if (existing == null) {
456-
existing = new AstPythonProperty(_ast, node, GetLoc(node));
460+
existing = new AstPythonProperty(node, declaringModule, declaringType, GetLoc(node));
457461
_scope.SetInScope(node.Name, existing);
458462
}
459463

@@ -471,8 +475,8 @@ private IPythonFunctionOverload CreateFunctionOverload(NameLookupContext funcSco
471475
.ToArray();
472476

473477
var overload = new AstPythonFunctionOverload(
474-
parameters,
475-
funcScope.GetLocOfName(node, node.NameExpression),
478+
parameters,
479+
funcScope.GetLocOfName(node, node.NameExpression),
476480
node.ReturnAnnotation?.ToCodeString(_ast));
477481
_functionWalkers.Add(new AstAnalysisFunctionWalker(funcScope, node, overload));
478482

@@ -485,14 +489,13 @@ private static string GetDoc(SuiteStatement node) {
485489
return ce?.Value as string;
486490
}
487491

488-
private AstPythonType CreateType(ClassDefinition node) {
489-
if (node == null) {
490-
throw new ArgumentNullException(nameof(node));
491-
}
492-
return CreateBuiltinTypes
493-
? new AstPythonBuiltinType(node.Name, _module, node.StartIndex, GetDoc(node.Body as SuiteStatement), GetLoc(node))
494-
: new AstPythonType(node.Name, _module, node.StartIndex, GetDoc(node.Body as SuiteStatement), GetLoc(node));
492+
private AstPythonClass CreateClass(ClassDefinition node) {
493+
node = node ?? throw new ArgumentNullException(nameof(node));
494+
return new AstPythonClass(node, _module,
495+
GetDoc(node.Body as SuiteStatement), GetLoc(node),
496+
CreateBuiltinTypes ? BuiltinTypeId.Unknown : BuiltinTypeId.Type); // built-ins set type later
495497
}
498+
496499
private void CollectTopLevelDefinitions() {
497500
var s = (_ast.Body as SuiteStatement).Statements.ToArray();
498501

@@ -501,7 +504,7 @@ private void CollectTopLevelDefinitions() {
501504
}
502505

503506
foreach (var node in s.OfType<ClassDefinition>()) {
504-
_members[node.Name] = CreateType(node);
507+
_members[node.Name] = CreateClass(node);
505508
}
506509

507510
foreach (var node in s.OfType<AssignmentStatement>().Where(n => n.Right is NameExpression)) {
@@ -516,10 +519,12 @@ private void CollectTopLevelDefinitions() {
516519

517520
public override bool Walk(ClassDefinition node) {
518521
var member = _scope.GetInScope(node.Name);
519-
var t = member as AstPythonType ??
520-
(member as IPythonMultipleMembers)?.Members.OfType<AstPythonType>().FirstOrDefault(pt => pt.StartIndex == node.StartIndex);
522+
var t = member as AstPythonClass;
523+
if (t == null && member is IPythonMultipleMembers mm) {
524+
t = mm.GetMembers().OfType<AstPythonClass>().FirstOrDefault(pt => pt.ClassDefinition.StartIndex == node.StartIndex);
525+
}
521526
if (t == null) {
522-
t = CreateType(node);
527+
t = CreateClass(node);
523528
_scope.SetInScope(node.Name, t);
524529
}
525530

@@ -549,7 +554,7 @@ public void ProcessFunctionDefinition(FunctionDefinition node) {
549554
var existing = _scope.LookupNameInScopes(node.Name, NameLookupContext.LookupOptions.Local) as AstPythonFunction;
550555
if (existing == null) {
551556
var cls = _scope.GetInScope("__class__") as IPythonType;
552-
existing = new AstPythonFunction(_ast, _module, cls, node, GetLoc(node));
557+
existing = new AstPythonFunction(node, _module, cls, GetLoc(node));
553558
_scope.SetInScope(node.Name, existing);
554559
}
555560

‎src/Analysis/Engine/Impl/Interpreter/Ast/AstBuiltinsPythonModule.cs

+6-8
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
// THIS CODE IS PROVIDED ON AN *AS IS* BASIS, WITHOUT WARRANTIES OR CONDITIONS
1010
// OF ANY KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY
1111
// IMPLIED WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE,
12-
// MERCHANTABLITY OR NON-INFRINGEMENT.
12+
// MERCHANTABILITY OR NON-INFRINGEMENT.
1313
//
1414
// See the Apache Version 2.0 License for specific language governing
1515
// permissions and limitations under the License.
@@ -43,8 +43,7 @@ public override IMember GetMember(IModuleContext context, string name) {
4343

4444
public IMember GetAnyMember(string name) {
4545
lock (_members) {
46-
IMember m;
47-
_members.TryGetValue(name, out m);
46+
_members.TryGetValue(name, out var m);
4847
return m;
4948
}
5049
}
@@ -96,13 +95,12 @@ protected override PythonWalker PrepareWalker(AstPythonInterpreter interpreter,
9695
}
9796

9897
protected override void PostWalk(PythonWalker walker) {
99-
AstPythonBuiltinType boolType = null;
100-
AstPythonBuiltinType noneType = null;
98+
IPythonType boolType = null;
99+
IPythonType noneType = null;
101100

102101
foreach (BuiltinTypeId typeId in Enum.GetValues(typeof(BuiltinTypeId))) {
103-
if (_members.TryGetValue("__{0}__".FormatInvariant(typeId), out IMember m) && m is AstPythonBuiltinType biType) {
104-
if (typeId != BuiltinTypeId.Str &&
105-
typeId != BuiltinTypeId.StrIterator) {
102+
if (_members.TryGetValue("__{0}__".FormatInvariant(typeId), out var m) && m is AstPythonType biType && biType.IsBuiltin) {
103+
if (typeId != BuiltinTypeId.Str && typeId != BuiltinTypeId.StrIterator) {
106104
biType.TrySetTypeId(typeId);
107105
}
108106

‎src/Analysis/Engine/Impl/Interpreter/Ast/AstNestedPythonModule.cs

+8-11
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,15 @@
2222
using Microsoft.PythonTools.Analysis.Infrastructure;
2323

2424
namespace Microsoft.PythonTools.Interpreter.Ast {
25-
internal sealed class AstNestedPythonModule : IPythonModule, ILocatedMember {
26-
private readonly string _name;
25+
internal sealed class AstNestedPythonModule : PythonModuleType, IPythonModule, ILocatedMember {
2726
private readonly IPythonInterpreter _interpreter;
2827
private IPythonModule _module;
2928

30-
public AstNestedPythonModule(IPythonInterpreter interpreter, string fullName) {
29+
public AstNestedPythonModule(IPythonInterpreter interpreter, string fullName) : base(fullName) {
3130
_interpreter = interpreter ?? throw new ArgumentNullException(nameof(interpreter));
32-
_name = fullName ?? throw new ArgumentNullException(nameof(fullName));
3331
}
3432

35-
public string Name => MaybeModule?.Name ?? _name;
36-
public string Documentation => MaybeModule?.Documentation ?? string.Empty;
37-
public PythonMemberType MemberType => PythonMemberType.Module;
33+
public override string Documentation => MaybeModule?.Documentation ?? string.Empty;
3834
public IEnumerable<ILocationInfo> Locations => ((MaybeModule as ILocatedMember)?.Locations).MaybeEnumerate();
3935

4036
public bool IsLoaded => MaybeModule != null;
@@ -46,23 +42,24 @@ private IPythonModule GetModule() {
4642
return module;
4743
}
4844

49-
module = _interpreter.ImportModule(_name);
45+
module = _interpreter.ImportModule(Name);
5046
if (module != null) {
5147
Debug.Assert(!(module is AstNestedPythonModule), "ImportModule should not return nested module");
5248
}
5349

5450
if (module == null) {
55-
module = new SentinelModule(_name, false);
51+
module = new SentinelModule(Name, false);
5652
}
5753

5854
return Interlocked.CompareExchange(ref _module, module, null) ?? module;
5955
}
6056

6157
public IEnumerable<string> GetChildrenModules() => GetModule().GetChildrenModules();
6258

63-
public IMember GetMember(IModuleContext context, string name) => GetModule().GetMember(context, name);
59+
public override IMember GetMember(IModuleContext context, string name)
60+
=> GetModule().GetMember(context, name);
6461

65-
public IEnumerable<string> GetMemberNames(IModuleContext context) =>
62+
public override IEnumerable<string> GetMemberNames(IModuleContext context) =>
6663
// TODO: Make GetMemberNames() faster than Imported()
6764
GetModule().GetMemberNames(context);
6865

0 commit comments

Comments
 (0)