Skip to content
This repository was archived by the owner on Nov 4, 2024. It is now read-only.

Commit 41532fd

Browse files
author
Mikhail Arkhipov
authored
Treat default value None as nothing (microsoft#680)
* Fix microsoft#604 * PR feedback
1 parent d18f910 commit 41532fd

File tree

6 files changed

+45
-51
lines changed

6 files changed

+45
-51
lines changed

‎src/Analysis/Ast/Impl/Analyzer/Symbols/FunctionEvaluator.cs

Lines changed: 18 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -141,60 +141,53 @@ private void DeclareParameters(bool declareVariables) {
141141
Eval.DeclareVariable(p0.Name, new PythonInstance(_self), VariableSource.Declaration, p0.NameExpression);
142142
}
143143
// Set parameter info.
144-
var pi = new ParameterInfo(Ast, p0, _self, false);
145-
pi.SetType(_self);
144+
var pi = new ParameterInfo(Ast, p0, _self, null, false);
146145
parameters.Add(pi);
147146
skip++;
148147
}
149148
}
150149

151150
// Declare parameters in scope
151+
IMember defaultValue = null;
152+
var meaningfuleDefautValue = false;
152153
for (var i = skip; i < FunctionDefinition.Parameters.Length; i++) {
153154
var isGeneric = false;
154155
var p = FunctionDefinition.Parameters[i];
155156
if (!string.IsNullOrEmpty(p.Name)) {
156-
// If parameter has default value, look for the annotation locally first
157-
// since outer type may be getting redefined. Consider 's = None; def f(s: s = 123): ...
158157
IPythonType paramType = null;
159158
if (p.DefaultValue != null) {
159+
defaultValue = Eval.GetValueFromExpression(p.DefaultValue);
160+
// If parameter has default value, look for the annotation locally first
161+
// since outer type may be getting redefined. Consider 's = None; def f(s: s = 123): ...
160162
paramType = Eval.GetTypeFromAnnotation(p.Annotation, out isGeneric, LookupOptions.Local | LookupOptions.Builtins);
161-
if (paramType == null) {
162-
var defaultValue = Eval.GetValueFromExpression(p.DefaultValue);
163-
if (!defaultValue.IsUnknown()) {
164-
paramType = defaultValue.GetPythonType();
165-
}
163+
// Default value of None does not mean the parameter is None, just says it can be missing.
164+
defaultValue = defaultValue.IsUnknown() || defaultValue.IsOfType(BuiltinTypeId.NoneType) ? null : defaultValue;
165+
if (paramType == null && defaultValue != null) {
166+
paramType = defaultValue.GetPythonType();
166167
}
167168
}
168169
// If all else fails, look up globally.
169-
paramType = paramType ?? Eval.GetTypeFromAnnotation(p.Annotation, out isGeneric);
170-
171-
var pi = new ParameterInfo(Ast, p, paramType, isGeneric);
172-
DeclareParameter(p, i, pi, declareVariables);
170+
paramType = paramType ?? Eval.GetTypeFromAnnotation(p.Annotation, out isGeneric) ?? Eval.UnknownType;
171+
var pi = new ParameterInfo(Ast, p, paramType, defaultValue, isGeneric);
172+
if (declareVariables) {
173+
DeclareParameter(p, pi);
174+
}
173175
parameters.Add(pi);
174176
}
175177
}
176178
_overload.SetParameters(parameters);
177179
}
178180

179-
private void DeclareParameter(Parameter p, int index, ParameterInfo pi, bool declareVariables) {
181+
private void DeclareParameter(Parameter p, ParameterInfo pi) {
180182
IPythonType paramType;
181-
182183
// If type is known from annotation, use it.
183184
if (pi != null && !pi.Type.IsUnknown() && !pi.Type.IsGenericParameter()) {
184185
// TODO: technically generics may have constraints. Should we consider them?
185186
paramType = pi.Type;
186187
} else {
187-
var defaultValue = Eval.GetValueFromExpression(p.DefaultValue) ?? Eval.UnknownType;
188-
189-
paramType = defaultValue?.GetPythonType();
190-
if (!paramType.IsUnknown()) {
191-
pi?.SetDefaultValueType(paramType);
192-
}
193-
}
194-
195-
if (declareVariables) {
196-
Eval.DeclareVariable(p.Name, new PythonInstance(paramType), VariableSource.Declaration, p.NameExpression);
188+
paramType = pi?.DefaultValue?.GetPythonType() ?? Eval.UnknownType;
197189
}
190+
Eval.DeclareVariable(p.Name, new PythonInstance(paramType), VariableSource.Declaration, p.NameExpression);
198191
}
199192
}
200193
}

‎src/Analysis/Ast/Impl/Extensions/MemberExtensions.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ public static bool IsUnknown(this IMember m) {
3131
}
3232
}
3333

34+
public static bool IsOfType(this IMember m, BuiltinTypeId typeId)
35+
=> m?.GetPythonType().TypeId == typeId;
36+
3437
public static IPythonType GetPythonType(this IMember m) {
3538
switch (m) {
3639
case IPythonType pt:

‎src/Analysis/Ast/Impl/Extensions/PythonTypeExtensions.cs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,14 @@ public static bool IsGenericParameter(this IPythonType value)
2828
public static bool IsGeneric(this IPythonType value)
2929
=> value is IGenericTypeDefinition || value is IGenericType || (value is IPythonClassType c && c.IsGeneric());
3030

31-
public static void TransferDocumentationAndLocation(this IPythonType src, IPythonType dst) {
32-
if (src != null && dst is PythonType pt) {
33-
pt.TrySetTypeId(dst.TypeId);
31+
public static void TransferDocumentationAndLocation(this IPythonType s, IPythonType d) {
32+
if (s != d && s is PythonType src && d is PythonType dst) {
33+
dst.TrySetTypeId(src.TypeId);
3434
var documentation = src.Documentation;
3535
if (!string.IsNullOrEmpty(documentation)) {
36-
if (!string.IsNullOrEmpty(documentation)) {
37-
pt.SetDocumentation(documentation);
38-
}
39-
}
40-
if (src is ILocatedMember lm) {
41-
pt.SetLocation(lm.Location);
36+
dst.SetDocumentationProvider(_ => documentation);
4237
}
38+
dst.SetLocation(src.Location);
4339
}
4440
}
4541

‎src/Analysis/Ast/Impl/Types/Definitions/IParameterInfo.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ public interface IParameterInfo {
5757
string DefaultValueString { get; }
5858

5959
/// <summary>
60-
/// Default value type.
60+
/// Default value.
6161
/// </summary>
62-
IPythonType DefaultValueType { get; }
62+
IMember DefaultValue { get; }
6363
}
6464
}

‎src/Analysis/Ast/Impl/Types/ParameterInfo.cs

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818

1919
namespace Microsoft.Python.Analysis.Types {
2020
internal sealed class ParameterInfo : IParameterInfo {
21-
public ParameterInfo(PythonAst ast, Parameter p, IPythonType type, bool isGeneric) {
21+
public ParameterInfo(PythonAst ast, Parameter p, IPythonType type, IMember defaultValue, bool isGeneric) {
2222
Name = p?.Name ?? throw new ArgumentNullException(nameof(p));
2323
Documentation = string.Empty;
2424
DefaultValueString = p.DefaultValue?.ToCodeString(ast).Trim();
@@ -28,6 +28,7 @@ public ParameterInfo(PythonAst ast, Parameter p, IPythonType type, bool isGeneri
2828
IsParamArray = p.Kind == ParameterKind.List;
2929
IsKeywordDict = p.Kind == ParameterKind.Dictionary;
3030
IsGeneric = isGeneric;
31+
DefaultValue = defaultValue;
3132
Type = type;
3233
}
3334

@@ -36,20 +37,8 @@ public ParameterInfo(PythonAst ast, Parameter p, IPythonType type, bool isGeneri
3637
public bool IsParamArray { get; }
3738
public bool IsKeywordDict { get; }
3839
public bool IsGeneric { get; }
39-
public IPythonType Type { get; private set; }
40+
public IPythonType Type { get; }
4041
public string DefaultValueString { get; }
41-
public IPythonType DefaultValueType { get; private set; }
42-
43-
internal void SetType(IPythonType type) {
44-
if (Type.IsUnknown()) {
45-
Type = type;
46-
}
47-
}
48-
internal void SetDefaultValueType(IPythonType type) {
49-
if (DefaultValueType.IsUnknown()) {
50-
DefaultValueType = type;
51-
SetType(type);
52-
}
53-
}
42+
public IMember DefaultValue { get;}
5443
}
5544
}

‎src/Analysis/Ast/Test/FunctionTests.cs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,19 @@ def f(a, b):
152152
.HaveVariable("y").OfType(BuiltinTypeId.Int);
153153
}
154154

155+
[TestMethod, Priority(0)]
156+
public async Task ReturnValueDefaultNone() {
157+
const string code = @"
158+
def g(x=None):
159+
return x
160+
161+
y = g('4')
162+
";
163+
var analysis = await GetAnalysisAsync(code);
164+
analysis.Should()
165+
.HaveVariable("y").OfType(BuiltinTypeId.Str);
166+
}
167+
155168
[TestMethod, Priority(0)]
156169
public async Task ReturnValueAnnotated() {
157170
const string code = @"

0 commit comments

Comments
 (0)