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

Commit

Permalink
Merge pull request #92 from MikhailArkhipov/84
Browse files Browse the repository at this point in the history
Fix existing and potentil null references
  • Loading branch information
Mikhail Arkhipov authored Sep 13, 2018
2 parents 64e9503 + 0dbaf33 commit 406a3a4
Show file tree
Hide file tree
Showing 27 changed files with 192 additions and 240 deletions.
52 changes: 25 additions & 27 deletions src/Analysis/Engine/Impl/AnalysisUnit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -369,15 +369,15 @@ internal override void AnalyzeWorker(DDG ddg, CancellationToken cancel) {
var classInfo = ((ClassScope)Scope).Class;
var bases = new List<IAnalysisSet>();

if (Ast.BasesInternal.Length == 0) {
if (Ast.Bases.Length == 0) {
if (ddg.ProjectState.LanguageVersion.Is3x()) {
// 3.x all classes inherit from object by default
bases.Add(ddg.ProjectState.ClassInfos[BuiltinTypeId.Object]);
}
} else {
// Process base classes
for (var i = 0; i < Ast.BasesInternal.Length; i++) {
var baseClassArg = Ast.BasesInternal[i];
for (var i = 0; i < Ast.Bases.Length; i++) {
var baseClassArg = Ast.Bases[i];

if (baseClassArg.Name == null) {
bases.Add(EvaluateBaseClass(ddg, classInfo, i, baseClassArg.Expression));
Expand Down Expand Up @@ -415,33 +415,31 @@ private IAnalysisSet ProcessClassDecorators(DDG ddg, ClassInfo classInfo) {
if (Ast.Decorators != null) {
Expression expr = Ast.NameExpression;

foreach (var d in Ast.Decorators.DecoratorsInternal) {
if (d != null) {
var decorator = ddg._eval.Evaluate(d);
foreach (var d in Ast.Decorators.Decorators.ExcludeDefault()) {
var decorator = ddg._eval.Evaluate(d);

Expression nextExpr;
if (!_decoratorCalls.TryGetValue(d, out nextExpr)) {
nextExpr = _decoratorCalls[d] = new CallExpression(d, new[] { new Arg(expr) });
nextExpr.SetLoc(d.IndexSpan);
}
expr = nextExpr;
var decorated = AnalysisSet.Empty;
var anyResults = false;
foreach (var ns in decorator) {
var fd = ns as FunctionInfo;
if (fd != null && InterpreterScope.EnumerateTowardsGlobal.Any(s => s.AnalysisValue == fd)) {
continue;
}
decorated = decorated.Union(ns.Call(expr, this, new[] { types }, ExpressionEvaluator.EmptyNames));
anyResults = true;
Expression nextExpr;
if (!_decoratorCalls.TryGetValue(d, out nextExpr)) {
nextExpr = _decoratorCalls[d] = new CallExpression(d, new[] { new Arg(expr) });
nextExpr.SetLoc(d.IndexSpan);
}
expr = nextExpr;
var decorated = AnalysisSet.Empty;
var anyResults = false;
foreach (var ns in decorator) {
var fd = ns as FunctionInfo;
if (fd != null && InterpreterScope.EnumerateTowardsGlobal.Any(s => s.AnalysisValue == fd)) {
continue;
}
decorated = decorated.Union(ns.Call(expr, this, new[] { types }, ExpressionEvaluator.EmptyNames));
anyResults = true;
}

// If processing decorators, update the current
// function type. Otherwise, we are acting as if
// each decorator returns the function unmodified.
if (ddg.ProjectState.Limits.ProcessCustomDecorators && anyResults) {
types = decorated;
}
// If processing decorators, update the current
// function type. Otherwise, we are acting as if
// each decorator returns the function unmodified.
if (ddg.ProjectState.Limits.ProcessCustomDecorators && anyResults) {
types = decorated;
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Analysis/Engine/Impl/Analyzer/DDG.cs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ public override bool Walk(NonlocalStatement node) {
public override bool Walk(ClassDefinition node) {
// Evaluate decorators for references
// TODO: Should apply decorators when assigning the class
foreach (var d in (node.Decorators?.DecoratorsInternal).MaybeEnumerate()) {
foreach (var d in (node.Decorators?.Decorators).MaybeEnumerate().ExcludeDefault()) {
_eval.Evaluate(d);
}

Expand Down
76 changes: 37 additions & 39 deletions src/Analysis/Engine/Impl/Analyzer/FunctionAnalysisUnit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -132,44 +132,42 @@ internal IAnalysisSet ProcessFunctionDecorators(DDG ddg) {
if (Ast.Decorators != null) {
Expression expr = Ast.NameExpression;

foreach (var d in Ast.Decorators.DecoratorsInternal) {
if (d != null) {
var decorator = ddg._eval.Evaluate(d);

if (decorator.Contains(State.ClassInfos[BuiltinTypeId.Property])) {
Function.IsProperty = true;
} else if (decorator.Contains(State.ClassInfos[BuiltinTypeId.StaticMethod])) {
// TODO: Warn if IsClassMethod is set
Function.IsStatic = true;
} else if (decorator.Contains(State.ClassInfos[BuiltinTypeId.ClassMethod])) {
// TODO: Warn if IsStatic is set
Function.IsClassMethod = true;
} else if (ProcessAbstractDecorators(decorator)) {
// No-op
} else {
Expression nextExpr;
if (!_decoratorCalls.TryGetValue(d, out nextExpr)) {
nextExpr = _decoratorCalls[d] = new CallExpression(d, new[] { new Arg(expr) });
nextExpr.SetLoc(d.IndexSpan);
}
expr = nextExpr;
var decorated = AnalysisSet.Empty;
var anyResults = false;
foreach (var ns in decorator) {
var fd = ns as FunctionInfo;
if (fd != null && InterpreterScope.EnumerateTowardsGlobal.Any(s => s.AnalysisValue == fd)) {
continue;
}
decorated = decorated.Union(ns.Call(expr, this, new[] { types }, ExpressionEvaluator.EmptyNames));
anyResults = true;
}
foreach (var d in Ast.Decorators.Decorators.ExcludeDefault()) {
var decorator = ddg._eval.Evaluate(d);

// If processing decorators, update the current
// function type. Otherwise, we are acting as if
// each decorator returns the function unmodified.
if (ddg.ProjectState.Limits.ProcessCustomDecorators && anyResults) {
types = decorated;
if (decorator.Contains(State.ClassInfos[BuiltinTypeId.Property])) {
Function.IsProperty = true;
} else if (decorator.Contains(State.ClassInfos[BuiltinTypeId.StaticMethod])) {
// TODO: Warn if IsClassMethod is set
Function.IsStatic = true;
} else if (decorator.Contains(State.ClassInfos[BuiltinTypeId.ClassMethod])) {
// TODO: Warn if IsStatic is set
Function.IsClassMethod = true;
} else if (ProcessAbstractDecorators(decorator)) {
// No-op
} else {
Expression nextExpr;
if (!_decoratorCalls.TryGetValue(d, out nextExpr)) {
nextExpr = _decoratorCalls[d] = new CallExpression(d, new[] { new Arg(expr) });
nextExpr.SetLoc(d.IndexSpan);
}
expr = nextExpr;
var decorated = AnalysisSet.Empty;
var anyResults = false;
foreach (var ns in decorator) {
var fd = ns as FunctionInfo;
if (fd != null && InterpreterScope.EnumerateTowardsGlobal.Any(s => s.AnalysisValue == fd)) {
continue;
}
decorated = decorated.Union(ns.Call(expr, this, new[] { types }, ExpressionEvaluator.EmptyNames));
anyResults = true;
}

// If processing decorators, update the current
// function type. Otherwise, we are acting as if
// each decorator returns the function unmodified.
if (ddg.ProjectState.Limits.ProcessCustomDecorators && anyResults) {
types = decorated;
}
}
}
Expand All @@ -181,8 +179,8 @@ internal IAnalysisSet ProcessFunctionDecorators(DDG ddg) {
internal void AnalyzeDefaultParameters(DDG ddg) {
IVariableDefinition param;
var scope = (FunctionScope)Scope;
for (var i = 0; i < Ast.ParametersInternal.Length; ++i) {
var p = Ast.ParametersInternal[i];
for (var i = 0; i < Ast.Parameters.Length; ++i) {
var p = Ast.Parameters[i];
if (p.Annotation != null) {
var val = ddg._eval.EvaluateAnnotation(p.Annotation);
if (val?.Any() == true && Scope.TryGetVariable(p.Name, out param)) {
Expand Down Expand Up @@ -232,7 +230,7 @@ public override string ToString() {
return "{0}{1}({2})->{3}".FormatInvariant(
base.ToString(),
" def:",
string.Join(", ", Ast.ParametersInternal.Select(p => InterpreterScope.TryGetVariable(p.Name, out var v) ? v.Types.ToString() : "{}")),
string.Join(", ", Ast.Parameters.Select(p => InterpreterScope.TryGetVariable(p.Name, out var v) ? v.Types.ToString() : "{}")),
((FunctionScope)Scope).ReturnValue.Types.ToString()
);
}
Expand Down
6 changes: 3 additions & 3 deletions src/Analysis/Engine/Impl/Analyzer/FunctionScope.cs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public VariableDef GetParameter(string name) {
}

internal void EnsureParameters(FunctionAnalysisUnit unit, bool usePlaceholders) {
var astParams = Function.FunctionDefinition.ParametersInternal;
var astParams = Function.FunctionDefinition.Parameters;
for (int i = 0; i < astParams.Length; ++i) {
var p = astParams[i];
var name = p?.Name;
Expand Down Expand Up @@ -129,7 +129,7 @@ internal void EnsureParameters(FunctionAnalysisUnit unit, bool usePlaceholders)
}

internal void EnsureParameterZero(FunctionAnalysisUnit unit) {
var p = Function.FunctionDefinition.ParametersInternal.FirstOrDefault();
var p = Function.FunctionDefinition.Parameters.FirstOrDefault();
if (!string.IsNullOrEmpty(p?.Name) &&
p.Kind == ParameterKind.Normal &&
!unit.Function.IsStatic &&
Expand Down Expand Up @@ -173,7 +173,7 @@ internal bool UpdateParameters(
) {
EnsureParameters(unit, usePlaceholders);

var astParams = Function.FunctionDefinition.ParametersInternal;
var astParams = Function.FunctionDefinition.Parameters;
var added = false;
var entry = unit.DependencyProject;
var state = unit.State;
Expand Down
2 changes: 1 addition & 1 deletion src/Analysis/Engine/Impl/Analyzer/OverviewWalker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ internal ClassInfo AddClass(ClassDefinition node, AnalysisUnit outerUnit) {
var classScope = (ClassScope)unit.Scope;

var classVar = declScope.AddLocatedVariable(node.Name, node.NameExpression, unit);
if (node.Decorators == null || node.Decorators.DecoratorsInternal.Length == 0) {
if (node.Decorators == null || node.Decorators.Decorators.Length == 0) {
classVar.AddTypes(unit, classScope.Class.SelfSet);
}

Expand Down
4 changes: 2 additions & 2 deletions src/Analysis/Engine/Impl/ExpressionFinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ public override bool Walk(FunctionDefinition node) {
}

node.Decorators?.Walk(this);
foreach (var p in node.ParametersInternal.MaybeEnumerate()) {
foreach (var p in node.Parameters) {
p?.Walk(this);
}
node.ReturnAnnotation?.Walk(this);
Expand Down Expand Up @@ -254,7 +254,7 @@ public override bool Walk(ClassDefinition node) {
node.NameExpression?.Walk(this);
}
node.Decorators?.Walk(this);
foreach (var b in node.BasesInternal.MaybeEnumerate()) {
foreach (var b in node.Bases) {
b.Walk(this);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,6 @@ public static bool SetEquals<T>(this IEnumerable<T> source, IEnumerable<T> other
private static T GetKey<T, U>(KeyValuePair<T, U> keyValue) => keyValue.Key;

public static IEnumerable<T> Keys<T, U>(this IEnumerable<KeyValuePair<T, U>> source) => source.Select(GetKey);
public static IEnumerable<T> ExcludeDefault<T>(this IEnumerable<T> source) => source.Where(i => !Equals(i, default(T)));
}
}
46 changes: 18 additions & 28 deletions src/Analysis/Engine/Impl/Intellisense/ExtractedMethodCreator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Text;
using Microsoft.PythonTools.Analysis.Infrastructure;
using Microsoft.PythonTools.Parsing;
using Microsoft.PythonTools.Parsing.Ast;

Expand Down Expand Up @@ -58,28 +60,19 @@ public ExtractMethodResult GetExtractionResult() {
bool isStaticMethod = false, isClassMethod = false;
var parameters = new List<Parameter>();
NameExpression selfParam = null;
if (_targetScope is ClassDefinition) {
var fromScope = _scopes[_scopes.Count - 1] as FunctionDefinition;
Debug.Assert(fromScope != null); // we don't allow extracting from classes, so we have to be coming from a function
if (fromScope != null) {
if (fromScope.Decorators != null) {
foreach (var decorator in fromScope.Decorators.DecoratorsInternal) {
NameExpression name = decorator as NameExpression;
if (name != null) {
if (name.Name == "staticmethod") {
isStaticMethod = true;
} else if (name.Name == "classmethod") {
isClassMethod = true;
}
}
}
if (_targetScope is ClassDefinition && _scopes[_scopes.Count - 1] is FunctionDefinition fromScope) {
foreach (var name in (fromScope?.Decorators?.Decorators).MaybeEnumerate().ExcludeDefault().OfType<NameExpression>()) {
if (name.Name == "staticmethod") {
isStaticMethod = true;
} else if (name.Name == "classmethod") {
isClassMethod = true;
}
}

if (!isStaticMethod) {
if (fromScope.ParametersInternal.Length > 0) {
selfParam = fromScope.ParametersInternal[0].NameExpression;
parameters.Add(new Parameter(selfParam, ParameterKind.Normal));
}
if (!isStaticMethod) {
if (fromScope.Parameters.Length > 0) {
selfParam = fromScope.Parameters[0].NameExpression;
parameters.Add(new Parameter(selfParam, ParameterKind.Normal));
}
}
}
Expand Down Expand Up @@ -140,7 +133,7 @@ public ExtractMethodResult GetExtractionResult() {
retStmt.SetLeadingWhiteSpace(_ast, leading);

body = new SuiteStatement(
new Statement[] {
new Statement[] {
body,
retStmt
}
Expand All @@ -159,7 +152,7 @@ public ExtractMethodResult GetExtractionResult() {

var res = new FunctionDefinition(new NameExpression(_name), parameters.ToArray(), body, decorators);
res.IsCoroutine = isCoroutine;

StringBuilder newCall = new StringBuilder();
newCall.Append(_target.IndentationLevel);
var method = res.ToCodeString(_ast);
Expand Down Expand Up @@ -235,15 +228,12 @@ public ExtractMethodResult GetExtractionResult() {
newCall.Append("await ");
}

if (_targetScope is ClassDefinition) {
var fromScope = _scopes[_scopes.Count - 1] as FunctionDefinition;
Debug.Assert(fromScope != null); // we don't allow extracting from classes, so we have to be coming from a function

if (_targetScope is ClassDefinition && _scopes[_scopes.Count - 1] is FunctionDefinition fromScope2) {
if (isStaticMethod) {
newCall.Append(_targetScope.Name);
newCall.Append('.');
} else if (fromScope != null && fromScope.ParametersInternal.Length > 0) {
newCall.Append(fromScope.ParametersInternal[0].Name);
} else if (fromScope2 != null && fromScope2.Parameters.Length > 0) {
newCall.Append(fromScope2.Parameters[0].Name);
newCall.Append('.');
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/Analysis/Engine/Impl/Intellisense/FlowChecker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ public override bool Walk(ClassDefinition node) {
} else {
// analyze the class definition itself (it is visited while analyzing parent scope):
Define(node.Name);
foreach (var e in node.BasesInternal) {
foreach (var e in node.Bases) {
e.Expression.Walk(this);
}
return false;
Expand Down Expand Up @@ -365,14 +365,14 @@ public override bool Walk(FromImportStatement node) {
public override bool Walk(FunctionDefinition node) {
if (node == _scope) {
// the function body is being analyzed, go deep:
foreach (Parameter p in node.ParametersInternal) {
foreach (Parameter p in node.Parameters) {
p.Walk(_fdef);
}
return true;
} else {
// analyze the function definition itself (it is visited while analyzing parent scope):
Define(node.Name);
foreach (Parameter p in node.ParametersInternal) {
foreach (Parameter p in node.Parameters) {
if (p.DefaultValue != null) {
p.DefaultValue.Walk(this);
}
Expand Down
Loading

0 comments on commit 406a3a4

Please sign in to comment.