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

Commit a5b6e5e

Browse files
AlexanderSherAlexanderSher
authored
Cherry pick fixes from Release branch into PTVS branch (#578)
* Fix #470 (#478) * Fix various common exceptions, log on a failed directory load (#498) * rethrow exception after telemetry instead of disposing * log an error instead of crashing when hitting some exceptions loading files, fix nullref when shutting down without an idle tracker * fix short circuiting of question mark checks to prevent null from being returned * print name of exception instead of relying on ToString * don't use MaybeEnumerate * upgrade message to Show * Mitigate some of #495 via MRO member selection and analysis set optimization (#517) This isn't a complete fix, but does seem to improve #495 in some cases. Adds back the early MRO return removed in #277, so now large class hierarchies won't over-propagate types (where some of the trouble with fastai happens do to the Transform class). I also optimized AnalysisHashSet a little to do length checks when possible. There are still times when things get caught up comparing unions for a while, but it seems to be nondeterministic. * Two path resolution bug fixes - Fix #509: PLS doesn't flag error in a relative import if it is found in another root directory (#519) - Fix #510: PLS doesn't resolve relative paths correctly * Catch exceptions when importing from search paths (#525) * catch exceptions when importing from search paths * retry instead of not found
1 parent 4d7e953 commit a5b6e5e

22 files changed

+319
-40
lines changed

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

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,19 @@ public bool SetEquals(IAnalysisSet other) {
275275
return true;
276276
}
277277

278+
// Only access Count once to prevent wasted time in bucket locks.
279+
var lCount = Count;
280+
var rCount = other.Count;
281+
282+
if (lCount == 0 && rCount == 0) {
283+
return true;
284+
}
285+
278286
if (Comparer == other.Comparer) {
287+
if (lCount != rCount) {
288+
return false;
289+
}
290+
279291
// Quick check for any unmatched hashcodes.
280292
// This can conclusively prove the sets are not equal, but cannot
281293
// prove equality.
@@ -290,17 +302,16 @@ public bool SetEquals(IAnalysisSet other) {
290302
}
291303
}
292304

293-
var otherHc = new HashSet<AnalysisValue>(other, _comparer);
294-
foreach (var key in this) {
295-
if (!otherHc.Remove(key)) {
296-
return false;
297-
}
298-
}
299-
if (otherHc.Any()) {
305+
if (lCount == 0 || rCount == 0) {
300306
return false;
301307
}
302308

303-
return true;
309+
if (lCount == 1 && rCount == 1) {
310+
return _comparer.Equals(this.First(), other.First());
311+
}
312+
313+
var hs = new HashSet<AnalysisValue>(other, _comparer);
314+
return hs.SetEquals(this);
304315
}
305316

306317
/// <summary>

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,8 @@ public static bool ContainsAny(this IAnalysisSet set, IAnalysisSet values) {
534534
sealed class ObjectComparer : IEqualityComparer<AnalysisValue>, IEqualityComparer<IAnalysisSet> {
535535
public static readonly ObjectComparer Instance = new ObjectComparer();
536536

537+
private ObjectComparer() { }
538+
537539
public bool Equals(AnalysisValue x, AnalysisValue y) {
538540
#if FULL_VALIDATION
539541
if (x != null && y != null) {
@@ -574,7 +576,7 @@ sealed class UnionComparer : IEqualityComparer<AnalysisValue>, IEqualityComparer
574576

575577
public readonly int Strength;
576578

577-
public UnionComparer(int strength = 0) {
579+
private UnionComparer(int strength = 0) {
578580
Strength = strength;
579581
}
580582

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,9 @@ public override bool Walk(FromImportStatement node) {
296296
case ImportNotFound notFound:
297297
MakeUnresolvedImport(notFound.FullName, node.Root);
298298
return false;
299+
case NoKnownParentPackage _:
300+
MakeNoKnownParentPackageImport(node.Root);
301+
return false;
299302
default:
300303
return false;
301304
}
@@ -464,6 +467,10 @@ private void MakeUnresolvedImport(string name, Node spanNode) {
464467
ProjectState.AddDiagnostic(spanNode, _unit, ErrorMessages.UnresolvedImport(name), DiagnosticSeverity.Warning, ErrorMessages.UnresolvedImportCode);
465468
}
466469

470+
private void MakeNoKnownParentPackageImport(Node spanNode) {
471+
ProjectState.AddDiagnostic(spanNode, _unit, Resources.ErrorRelativeImportNoPackage, DiagnosticSeverity.Warning, ErrorMessages.UnresolvedImportCode);
472+
}
473+
467474
private void ImportModule(in ImportStatement node, in IModule module, in DottedName moduleImportExpression, in NameExpression asNameExpression) {
468475
// "import fob.oar as baz" is handled as
469476
// baz = import_module('fob.oar')

‎src/Analysis/Engine/Impl/DependencyResolution/NoKnownParentPackage.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Python Tools for Visual Studio
2+
// Copyright(c) Microsoft Corporation
3+
// All rights reserved.
4+
//
5+
// Licensed under the Apache License, Version 2.0 (the License); you may not use
6+
// this file except in compliance with the License. You may obtain a copy of the
7+
// License at http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// THIS CODE IS PROVIDED ON AN *AS IS* BASIS, WITHOUT WARRANTIES OR CONDITIONS
10+
// OF ANY KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY
11+
// IMPLIED WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE,
12+
// MERCHANTABILITY OR NON-INFRINGEMENT.
13+
//
14+
// See the Apache Version 2.0 License for specific language governing
15+
// permissions and limitations under the License.
16+
17+
namespace Microsoft.PythonTools.Analysis.DependencyResolution {
18+
internal class NoKnownParentPackage : IImportSearchResult { }
19+
}

‎src/Analysis/Engine/Impl/DependencyResolution/PathResolverSnapshot.cs

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -187,16 +187,11 @@ public IImportSearchResult GetImportsFromRelativePath(in string modulePath, in i
187187
return default;
188188
}
189189

190-
if (parentCount > lastEdge.PathLength) {
191-
// Can't get outside of the root
192-
return default;
193-
}
194-
195190
var fullNameList = relativePath.ToList();
196191
if (lastEdge.IsNonRooted) {
197192
// Handle relative imports only for modules in the same folder
198193
if (parentCount > 1) {
199-
return default;
194+
return new NoKnownParentPackage();
200195
}
201196

202197
if (parentCount == 1 && fullNameList.Count == 1 && lastEdge.Start.TryGetChild(fullNameList[0], out var nameNode)) {
@@ -209,12 +204,27 @@ public IImportSearchResult GetImportsFromRelativePath(in string modulePath, in i
209204
.ToString());
210205
}
211206

207+
var relativeInWorkDirectory = false;
208+
if (parentCount > lastEdge.PathLength - 2) {
209+
relativeInWorkDirectory = _workDirectory.EqualsOrdinal(lastEdge.FirstEdge.End.Name, IgnoreCaseInPaths);
210+
211+
// Relative path must be only inside package
212+
// Exception for working directory cause it can be a root directory of the package
213+
if (!relativeInWorkDirectory) {
214+
return new NoKnownParentPackage();
215+
}
216+
}
217+
212218
var relativeParentEdge = lastEdge.GetPrevious(parentCount);
213219

214220
var rootEdges = new List<Edge>();
215-
for (var i = 0; i < _roots.Count; i++) {
216-
if (RootContains(i, relativeParentEdge, out var rootEdge)) {
217-
rootEdges.Add(rootEdge);
221+
if (relativeInWorkDirectory) {
222+
rootEdges.Add(lastEdge.FirstEdge);
223+
} else {
224+
for (var i = 0; i < _roots.Count; i++) {
225+
if (RootContains(i, relativeParentEdge, out var rootEdge)) {
226+
rootEdges.Add(rootEdge);
227+
}
218228
}
219229
}
220230

@@ -226,7 +236,13 @@ public IImportSearchResult GetImportsFromRelativePath(in string modulePath, in i
226236
return default;
227237
}
228238

229-
var fullName = GetFullModuleNameBuilder(relativeParentEdge).Append(".", fullNameList).ToString();
239+
var fullNameBuilder = GetFullModuleNameBuilder(relativeParentEdge);
240+
if (!relativeParentEdge.IsFirst) {
241+
AppendName(fullNameBuilder, relativeParentEdge.End.Name);
242+
fullNameBuilder.Append(".");
243+
}
244+
var fullName = fullNameBuilder.Append(".", fullNameList).ToString();
245+
230246
return new ImportNotFound(fullName);
231247
}
232248

@@ -250,7 +266,7 @@ private static bool TryCreateModuleImport(Edge lastEdge, out ModuleImport module
250266
=> TryCreateModuleImport(lastEdge.FirstEdge.End, lastEdge.End, out moduleImport);
251267

252268
private static bool TryCreateModuleImport(Node rootNode, Node moduleNode, out ModuleImport moduleImport) {
253-
if (moduleNode.TryGetChild("__init__", out var initPyNode) && initPyNode.IsModule) {
269+
if (IsPackageWithInitPy(moduleNode, out var initPyNode)) {
254270
moduleImport = new ModuleImport(moduleNode.Name, initPyNode.FullModuleName, rootNode.Name, initPyNode.ModulePath, false);
255271
return true;
256272
}
@@ -517,7 +533,7 @@ private static bool TryFindImport(IEnumerable<Edge> rootEdges, List<string> full
517533
private static bool TryFindName(in Edge edge, in IEnumerable<string> nameParts, out Edge lastEdge) {
518534
lastEdge = edge;
519535
foreach (var name in nameParts) {
520-
if (lastEdge.End.IsModule) {
536+
if (lastEdge.End.IsModule && !IsPackageWithInitPy(lastEdge.End, out _)) {
521537
return false;
522538
}
523539
var index = lastEdge.End.GetChildIndex(name);
@@ -653,7 +669,7 @@ private static StringBuilder GetFullModuleNameBuilder(in Edge lastEdge) {
653669
while (edge != lastEdge) {
654670
AppendName(sb, edge.End.Name);
655671
edge = edge.Next;
656-
};
672+
}
657673

658674
return sb;
659675
}
@@ -744,6 +760,9 @@ private static int GetModuleNameStart(string rootedModulePath)
744760
private static int GetModuleNameEnd(string rootedModulePath)
745761
=> IsPythonCompiled(rootedModulePath) ? rootedModulePath.IndexOf('.', GetModuleNameStart(rootedModulePath)) : rootedModulePath.LastIndexOf('.');
746762

763+
private static bool IsPackageWithInitPy(Node node, out Node initPyNode)
764+
=> node.TryGetChild("__init__", out initPyNode) && initPyNode.IsModule;
765+
747766
private static bool IsNotInitPy(string name)
748767
=> !name.EqualsOrdinal("__init__");
749768

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,9 @@ public static bool EqualsIgnoreCase(this string s, string other)
195195
public static bool EqualsOrdinal(this string s, string other)
196196
=> string.Equals(s, other, StringComparison.Ordinal);
197197

198+
public static bool EqualsOrdinal(this string s, string other, bool ignoreCase)
199+
=> string.Equals(s, other, ignoreCase ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal);
200+
198201
public static bool EqualsOrdinal(this string s, int index, string other, int otherIndex, int length, bool ignoreCase = false)
199202
=> string.Compare(s, index, other, otherIndex, length, ignoreCase ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal) == 0;
200203

‎src/Analysis/Engine/Impl/Intellisense/AnalysisQueue.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ private async Task ConsumerLoop() {
6767
return;
6868
} catch (Exception ex) when (!ex.IsCriticalException()) {
6969
UnhandledException?.Invoke(this, new UnhandledExceptionEventArgs(ex, false));
70-
Dispose();
70+
throw;
7171
}
7272
}
7373
RaiseEventOnThreadPool(AnalysisComplete);
@@ -208,7 +208,7 @@ public async Task Handler(CancellationToken cancellationToken) {
208208
}
209209

210210
private sealed class QueueItemComparer : IEqualityComparer<QueueItem> {
211-
public static IEqualityComparer<QueueItem> Instance { get; } = new QueueItemComparer();
211+
public static readonly IEqualityComparer<QueueItem> Instance = new QueueItemComparer();
212212

213213
private QueueItemComparer() { }
214214
public bool Equals(QueueItem x, QueueItem y) => Equals(x.Key, y.Key);

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,12 @@ public async Task<TryImportModuleResult> TryImportModuleAsync(string name, PathR
205205
} catch (OperationCanceledException) {
206206
_log?.Log(TraceLevel.Error, "ImportTimeout", name, "ImportFromSearchPaths");
207207
return TryImportModuleResult.Timeout;
208+
} catch (Exception ex) when (
209+
ex is IOException // FileNotFoundException, DirectoryNotFoundException, PathTooLongException, etc
210+
|| ex is UnauthorizedAccessException
211+
) {
212+
_log?.Log(TraceLevel.Error, "ImportException", name, "ImportFromSearchPaths", ex.GetType().Name, ex.Message);
213+
return TryImportModuleResult.NeedRetry;
208214
}
209215
}
210216

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,12 @@ public bool Equals(ILocationInfo other) {
8383
/// Provides an IEqualityComparer that compares line, column and project entries. By
8484
/// default locations are equaitable based upon only line/project entry.
8585
/// </summary>
86-
public static IEqualityComparer<ILocationInfo> FullComparer { get; } = new FullLocationComparer();
86+
public static IEqualityComparer<ILocationInfo> FullComparer => FullLocationComparer.Instance;
8787

8888
sealed class FullLocationComparer : IEqualityComparer<ILocationInfo>, IEqualityComparer<LocationInfo> {
89+
public static readonly FullLocationComparer Instance = new FullLocationComparer();
90+
91+
private FullLocationComparer() { }
8992
public bool Equals(LocationInfo x, LocationInfo y) => EqualsImpl(x, y);
9093
public bool Equals(ILocationInfo x, ILocationInfo y) => EqualsImpl(x, y);
9194

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -411,8 +411,8 @@ internal ParameterResult GetParameterResultFromParameterInfo(IParameterInfo para
411411
}
412412

413413
class OverloadResultComparer : EqualityComparer<OverloadResult> {
414-
public static IEqualityComparer<OverloadResult> Instance = new OverloadResultComparer(false);
415-
public static IEqualityComparer<OverloadResult> WeakInstance = new OverloadResultComparer(true);
414+
public static readonly IEqualityComparer<OverloadResult> Instance = new OverloadResultComparer(false);
415+
public static readonly IEqualityComparer<OverloadResult> WeakInstance = new OverloadResultComparer(true);
416416

417417
private readonly bool _weak;
418418

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -586,10 +586,11 @@ internal AnalysisValue GetCached(object key, Func<AnalysisValue> maker) {
586586

587587
internal BuiltinInstanceInfo GetInstance(IPythonType type) => GetBuiltinType(type).Instance;
588588

589-
internal BuiltinClassInfo GetBuiltinType(IPythonType type) =>
590-
(BuiltinClassInfo)GetCached(type,
591-
() => MakeBuiltinType(type)
592-
) ?? ClassInfos[BuiltinTypeId.Object];
589+
internal BuiltinClassInfo GetBuiltinType(IPythonType type)
590+
// Cached value may or may not be a class info. Previous calls to GetAnalysisValueFromObjects
591+
// may have cached a different object for the type. For example, IPythonFunction would cache
592+
// BuiltinFunctionInfo and not BuiltinClassInfo. Therefore, don't use direct cast.
593+
=> GetCached(type, () => MakeBuiltinType(type)) as BuiltinClassInfo ?? MakeBuiltinType(type);
593594

594595
private BuiltinClassInfo MakeBuiltinType(IPythonType type) {
595596
switch (type.TypeId) {
@@ -903,7 +904,9 @@ private AggregateProjectEntry GetAggregateWorker(IProjectEntry[] all) {
903904
}
904905

905906
class AggregateComparer : IEqualityComparer<IProjectEntry[]> {
906-
public static AggregateComparer Instance = new AggregateComparer();
907+
public static readonly AggregateComparer Instance = new AggregateComparer();
908+
909+
private AggregateComparer() { }
907910

908911
public bool Equals(IProjectEntry[] x, IProjectEntry[] y) {
909912
if (x.Length != y.Length) {

‎src/Analysis/Engine/Impl/Resources.Designer.cs

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/Analysis/Engine/Impl/Resources.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,9 @@
312312
<data name="ErrorUnresolvedImport" xml:space="preserve">
313313
<value>unresolved import '{0}'</value>
314314
</data>
315+
<data name="ErrorRelativeImportNoPackage" xml:space="preserve">
316+
<value>attempted relative import with no known parent package</value>
317+
</data>
315318
<data name="ErrorUseBeforeDef" xml:space="preserve">
316319
<value>'{0}' used before definition</value>
317320
</data>

‎src/Analysis/Engine/Impl/Values/BuiltinNamespace.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
using System;
1818
using System.Collections.Generic;
19+
using System.Linq;
1920
using Microsoft.PythonTools.Analysis.Infrastructure;
2021
using Microsoft.PythonTools.Interpreter;
2122
using Microsoft.PythonTools.Parsing.Ast;
@@ -105,7 +106,7 @@ public TMemberContainer ContainedValue {
105106

106107
public virtual ILocatedMember GetLocatedMember() => null;
107108

108-
public override IEnumerable<ILocationInfo> Locations => GetLocatedMember()?.Locations.MaybeEnumerate();
109+
public override IEnumerable<ILocationInfo> Locations => GetLocatedMember()?.Locations ?? Enumerable.Empty<ILocationInfo>();
109110

110111
public override bool Equals(object obj) {
111112
if (obj is BuiltinNamespace<TMemberContainer> bn && GetType() == bn.GetType()) {

‎src/Analysis/Engine/Impl/Values/ClassInfo.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -831,6 +831,10 @@ public static IAnalysisSet GetMemberFromMroNoReferences(IEnumerable<IAnalysisSet
831831
result = result.Union(ns.GetMember(node, unit, name));
832832
}
833833
}
834+
835+
if (result != null && result.Count > 0) {
836+
break;
837+
}
834838
}
835839
return result;
836840
}

‎src/Analysis/Engine/Impl/Values/SpecializedNamespace.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,15 +209,15 @@ public override bool IsOfType(IAnalysisSet klass) {
209209
return _original.IsOfType(klass);
210210
}
211211

212-
public override IEnumerable<ILocationInfo> Locations => _original?.Locations.MaybeEnumerate();
212+
public override IEnumerable<ILocationInfo> Locations => _original?.Locations ?? Enumerable.Empty<ILocationInfo>();
213213

214214
public override string Name => _original == null ? base.Name : _original.Name;
215215

216-
public override IEnumerable<OverloadResult> Overloads => _original?.Overloads.MaybeEnumerate();
216+
public override IEnumerable<OverloadResult> Overloads => _original?.Overloads ?? Enumerable.Empty<OverloadResult>();
217217

218218
public override IPythonType PythonType => _original?.PythonType;
219219

220-
internal override IEnumerable<ILocationInfo> References => _original?.References.MaybeEnumerate();
220+
internal override IEnumerable<ILocationInfo> References => _original?.References ?? Enumerable.Empty<ILocationInfo>();
221221

222222
public override PythonMemberType MemberType => _original == null ? PythonMemberType.Unknown : _original.MemberType;
223223

‎src/Analysis/Engine/Test/AddTestSpecificSearchPathAttribute.cs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Python Tools for Visual Studio
2+
// Copyright(c) Microsoft Corporation
3+
// All rights reserved.
4+
//
5+
// Licensed under the Apache License, Version 2.0 (the License); you may not use
6+
// this file except in compliance with the License. You may obtain a copy of the
7+
// License at http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// THIS CODE IS PROVIDED ON AN *AS IS* BASIS, WITHOUT WARRANTIES OR CONDITIONS
10+
// OF ANY KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY
11+
// IMPLIED WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE,
12+
// MERCHANTABILITY OR NON-INFRINGEMENT.
13+
//
14+
// See the Apache Version 2.0 License for specific language governing
15+
// permissions and limitations under the License.
16+
17+
using System;
18+
19+
namespace AnalysisTests {
20+
[AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
21+
public class AddTestSpecificSearchPathAttribute : Attribute {
22+
public string RelativeSearchPath { get; }
23+
24+
public AddTestSpecificSearchPathAttribute(string relativeSearchPath) {
25+
RelativeSearchPath = relativeSearchPath;
26+
}
27+
}
28+
}

0 commit comments

Comments
 (0)