From ead5c7bb4ca956fc3e5d8652c2e65d5f34aa1237 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Fri, 28 Jun 2019 15:55:31 -0700 Subject: [PATCH 01/17] first working path classfier, breaks existing path code for now --- src/Analysis/Ast/Impl/get_search_paths.py | 10 +- .../Ast/Test/PathClassificationTests.cs | 149 +++++++++++++++ .../Impl/Interpreter/PythonLibraryPath.cs | 177 ++++++++++++++++-- src/Core/Impl/Extensions/StringExtensions.cs | 3 + 4 files changed, 319 insertions(+), 20 deletions(-) create mode 100644 src/Analysis/Ast/Test/PathClassificationTests.cs diff --git a/src/Analysis/Ast/Impl/get_search_paths.py b/src/Analysis/Ast/Impl/get_search_paths.py index 290662f48..53cec1cf0 100644 --- a/src/Analysis/Ast/Impl/get_search_paths.py +++ b/src/Analysis/Ast/Impl/get_search_paths.py @@ -45,6 +45,11 @@ def clean(path): BEFORE_SITE = set(clean(p) for p in BEFORE_SITE) AFTER_SITE = set(clean(p) for p in AFTER_SITE) +try: + SITE_PKGS = set(clean(p) for p in site.getsitepackages()) +except AttributeError: + SITE_PKGS = set() + for prefix in [ sys.prefix, sys.exec_prefix, @@ -69,4 +74,7 @@ def clean(path): if p in BEFORE_SITE: print("%s|stdlib|" % p) elif p in AFTER_SITE: - print("%s||" % p) + if p in SITE_PKGS: + print("%s|site|" % p) + else: + print("%s|pth|" % p) diff --git a/src/Analysis/Ast/Test/PathClassificationTests.cs b/src/Analysis/Ast/Test/PathClassificationTests.cs new file mode 100644 index 000000000..c0e436e88 --- /dev/null +++ b/src/Analysis/Ast/Test/PathClassificationTests.cs @@ -0,0 +1,149 @@ +// Copyright(c) Microsoft Corporation +// All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the License); you may not use +// this file except in compliance with the License. You may obtain a copy of the +// License at http://www.apache.org/licenses/LICENSE-2.0 +// +// THIS CODE IS PROVIDED ON AN *AS IS* BASIS, WITHOUT WARRANTIES OR CONDITIONS +// OF ANY KIND, EITHER EXPRESS OR IMPLIED, INCLUDING WITHOUT LIMITATION ANY +// IMPLIED WARRANTIES OR CONDITIONS OF TITLE, FITNESS FOR A PARTICULAR PURPOSE, +// MERCHANTABILITY OR NON-INFRINGEMENT. +// +// See the Apache Version 2.0 License for specific language governing +// permissions and limitations under the License. + +using System; +using System.IO; +using FluentAssertions; +using Microsoft.Python.Analysis.Core.Interpreter; +using Microsoft.Python.Core.IO; +using Microsoft.Python.Core.OS; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using TestUtilities; + + +namespace Microsoft.Python.Analysis.Tests { + [TestClass] + public class PathClassificationTests { + private readonly FileSystem _fs = new FileSystem(new OSPlatform()); + + public TestContext TestContext { get; set; } + + [TestInitialize] + public void TestInitialize() + => TestEnvironmentImpl.TestInitialize($"{TestContext.FullyQualifiedTestClassName}.{TestContext.TestName}"); + + [TestCleanup] + public void Cleanup() => TestEnvironmentImpl.TestCleanup(); + + [TestMethod] + public void Plain() { + var appPath = TestData.GetTestSpecificPath("app.py"); + var root = Path.GetDirectoryName(appPath); + + var venv = Path.Combine(root, "venv"); + var venvLib = Path.Combine(venv, "Lib"); + var venvSitePackages = Path.Combine(venvLib, "site-packages"); + + var fromInterpreter = new[] { + new PythonLibraryPath(venvLib, PythonLibraryPathType.StdLib), + new PythonLibraryPath(venv, PythonLibraryPathType.StdLib), + new PythonLibraryPath(venvSitePackages, PythonLibraryPathType.Site), + }; + + var (interpreterPaths, userPaths) = PythonLibraryPath.ClassifyPaths(root, _fs, fromInterpreter, Array.Empty()); + + interpreterPaths.Should().BeEquivalentTo(new[] { + new PythonLibraryPath(venvLib, PythonLibraryPathType.StdLib), + new PythonLibraryPath(venv, PythonLibraryPathType.StdLib), + new PythonLibraryPath(venvSitePackages, PythonLibraryPathType.Site), + }); + + userPaths.Should().BeEmpty(); + } + + [TestMethod] + public void WithSrcDir() { + var appPath = TestData.GetTestSpecificPath("app.py"); + var root = Path.GetDirectoryName(appPath); + + var venv = Path.Combine(root, "venv"); + var venvLib = Path.Combine(venv, "Lib"); + var venvSitePackages = Path.Combine(venvLib, "site-packages"); + + var src = Path.Combine(root, "src"); + + var fromInterpreter = new[] { + new PythonLibraryPath(venvLib, PythonLibraryPathType.StdLib), + new PythonLibraryPath(venv, PythonLibraryPathType.StdLib), + new PythonLibraryPath(venvSitePackages, PythonLibraryPathType.Site), + }; + + var fromUser = new[] { + "./src", + }; + + var (interpreterPaths, userPaths) = PythonLibraryPath.ClassifyPaths(root, _fs, fromInterpreter, fromUser); + + interpreterPaths.Should().BeEquivalentTo(new[] { + new PythonLibraryPath(venvLib, PythonLibraryPathType.StdLib), + new PythonLibraryPath(venv, PythonLibraryPathType.StdLib), + new PythonLibraryPath(venvSitePackages, PythonLibraryPathType.Site), + }); + + userPaths.Should().BeEquivalentTo(new[] { + new PythonLibraryPath(src, PythonLibraryPathType.Unspecified), + }); + } + + [TestMethod] + public void NormalizeUser() { + var appPath = TestData.GetTestSpecificPath("app.py"); + var root = Path.GetDirectoryName(appPath); + + var src = Path.Combine(root, "src"); + + var fromUser = new[] { + "./src/", + }; + + var (interpreterPaths, userPaths) = PythonLibraryPath.ClassifyPaths(root, _fs, Array.Empty(), fromUser); + + interpreterPaths.Should().BeEmpty(); + + userPaths.Should().BeEquivalentTo(new[] { + new PythonLibraryPath(src, PythonLibraryPathType.Unspecified), + }); + } + + [TestMethod] + public void NestedUser() { + var appPath = TestData.GetTestSpecificPath("app.py"); + var root = Path.GetDirectoryName(appPath); + + var src = Path.Combine(root, "src"); + var srcSomething = Path.Combine(src, "something"); + var srcFoo = Path.Combine(src, "foo"); + var srcFooBar = Path.Combine(srcFoo, "bar"); + + var fromUser = new[] { + "./src", + "./src/something", + "./src/foo", + "./src/foo/bar", + }; + + var (interpreterPaths, userPaths) = PythonLibraryPath.ClassifyPaths(root, _fs, Array.Empty(), fromUser); + + interpreterPaths.Should().BeEmpty(); + + userPaths.Should().BeEquivalentTo(new[] { + new PythonLibraryPath(srcFooBar, PythonLibraryPathType.Unspecified), + new PythonLibraryPath(srcFoo, PythonLibraryPathType.Unspecified), + new PythonLibraryPath(srcSomething, PythonLibraryPathType.Unspecified), + new PythonLibraryPath(src, PythonLibraryPathType.Unspecified), + }); + } + } +} diff --git a/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs b/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs index cb9eecb27..5b67a81b3 100644 --- a/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs +++ b/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs @@ -14,6 +14,7 @@ // permissions and limitations under the License. using System; +using System.Collections; using System.Collections.Generic; using System.Diagnostics; using System.IO; @@ -28,41 +29,80 @@ using IOPath = System.IO.Path; namespace Microsoft.Python.Analysis.Core.Interpreter { - public sealed class PythonLibraryPath { - private readonly string _modulePrefix; + public enum PythonLibraryPathType { + Unspecified, + StdLib, + Site, + Pth, + } + public sealed class PythonLibraryPath : IEquatable { private static readonly Regex ParseRegex = new Regex( - @"(?[^|]+)\|(?stdlib)?\|(?[^|]+)?" + @"(?[^|]+)\|(?[^|]+)\|(?[^|]+)?" ); - public PythonLibraryPath(string path, bool isStandardLibrary, string modulePrefix) { - Path = path; - IsStandardLibrary = isStandardLibrary; - _modulePrefix = modulePrefix; + public PythonLibraryPath(string path, PythonLibraryPathType type = PythonLibraryPathType.Unspecified, string modulePrefix = null) { + Path = PathUtils.TrimEndSeparator(PathUtils.NormalizePath(path)); + Type = type; + ModulePrefix = modulePrefix ?? string.Empty; } + public PythonLibraryPath(string path, bool isStandardLibrary, string modulePrefix) : + this(path, isStandardLibrary ? PythonLibraryPathType.StdLib : PythonLibraryPathType.Unspecified, modulePrefix) { } + public string Path { get; } - public bool IsStandardLibrary { get; } + public PythonLibraryPathType Type { get; } - public string ModulePrefix => _modulePrefix ?? string.Empty; + public string ModulePrefix { get; } = string.Empty; - public override string ToString() - => "{0}|{1}|{2}".FormatInvariant(Path, IsStandardLibrary ? "stdlib" : "", _modulePrefix ?? ""); + public bool IsStandardLibrary => Type == PythonLibraryPathType.StdLib; + + public override string ToString() { + var type = string.Empty; + + switch (Type) { + case PythonLibraryPathType.StdLib: + type = "stdlib"; + break; + case PythonLibraryPathType.Site: + type = "site"; + break; + case PythonLibraryPathType.Pth: + type = "pth"; + break; + } + + return "{0}|{1}|{2}".FormatInvariant(Path, type, ModulePrefix); + } public static PythonLibraryPath Parse(string s) { if (string.IsNullOrEmpty(s)) { throw new ArgumentNullException("source"); } - + var m = ParseRegex.Match(s); - if (!m.Success || !m.Groups["path"].Success) { + if (!m.Success || !m.Groups["path"].Success || !m.Groups["type"].Success) { throw new FormatException(); } - + + PythonLibraryPathType type = PythonLibraryPathType.Unspecified; + + switch (m.Groups["type"].Value) { + case "stdlib": + type = PythonLibraryPathType.StdLib; + break; + case "site": + type = PythonLibraryPathType.Site; + break; + case "pth": + type = PythonLibraryPathType.Pth; + break; + } + return new PythonLibraryPath( m.Groups["path"].Value, - m.Groups["stdlib"].Success, + type, m.Groups["prefix"].Success ? m.Groups["prefix"].Value : null ); } @@ -80,16 +120,16 @@ public static List GetDefaultSearchPaths(string library) { return result; } - result.Add(new PythonLibraryPath(library, true, null)); + result.Add(new PythonLibraryPath(library, PythonLibraryPathType.StdLib)); var sitePackages = IOPath.Combine(library, "site-packages"); if (!Directory.Exists(sitePackages)) { return result; } - result.Add(new PythonLibraryPath(sitePackages, false, null)); + result.Add(new PythonLibraryPath(sitePackages)); result.AddRange(ModulePath.ExpandPathFiles(sitePackages) - .Select(p => new PythonLibraryPath(p, false, null)) + .Select(p => new PythonLibraryPath(p)) ); return result; @@ -152,7 +192,7 @@ public static async Task> GetSearchPathsFromInterpreterA try { var output = await ps.ExecuteAndCaptureOutputAsync(startInfo, cancellationToken); - return output.Split(new [] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries).Select(s => { + return output.Split(new[] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries).Select(s => { if (s.PathStartsWith(tempWorkingDir)) { return null; } @@ -170,5 +210,104 @@ public static async Task> GetSearchPathsFromInterpreterA fs.DeleteDirectory(tempWorkingDir, true); } } + + public static (IReadOnlyList interpreterPaths, IReadOnlyList userPaths) ClassifyPaths( + string root, + IFileSystem fs, + IEnumerable fromInterpreter, + IEnumerable fromUser + ) { + // PRECONDITIONS: + // - root has already been normalized and had its end separator trimmed. + // - All paths in fromInterpreter were normalised and end separator trimmed. + + // Clean up user configured paths. + // 1) Noramlize paths. + // 2) If a path isn't rooted, then root it relative to the workspace root. If there is no root, just continue. + // 3) Trim off any ending separators for consistency. + // 4) Remove any empty paths, FS root paths (bad idea), or paths equal to the root. + fromUser = fromUser + .Select(PathUtils.NormalizePath) + .Select(p => root == null || IOPath.IsPathRooted(p) ? p : IOPath.GetFullPath(IOPath.Combine(root, p))) // TODO: Replace with GetFullPath(p, root) when .NET Standard 2.1 is out. + .Select(PathUtils.TrimEndSeparator) + .Where(p => !string.IsNullOrWhiteSpace(p) && p != "/" && !p.PathEquals(root)); + + // Deduplicate, and keep in a set to quickly check interpreter paths against. + var fromUserSet = new HashSet(fromUser, PathEqualityComparer.Instance); + + // Remove any interpreter paths specified in the user config so they can be reclassified. + fromInterpreter = fromInterpreter.Where(p => !fromUserSet.Contains(p.Path)); + + var stdlibLookup = fromInterpreter.ToLookup(p => p.Type == PythonLibraryPathType.StdLib); + + // Pull out stdlib paths, and make them always be interpreter paths. + var stdlib = stdlibLookup[true].ToList(); + var interpreterPaths = new List(stdlib); + fromInterpreter = stdlibLookup[false]; + + var userPaths = new SortedSet(PathDepthComparer.Instance); + + var allPaths = fromUserSet.Select(p => new PythonLibraryPath(p)) + .Concat(fromInterpreter.Where(p => !p.Path.PathEquals(root))); + + foreach (var p in allPaths) { + // If path is within a stdlib path, then treat it as interpreter. + if (stdlib.Any(s => fs.IsPathUnderRoot(s.Path, p.Path))) { + interpreterPaths.Add(p); + continue; + } + + // If path is outside the workspace, then treat it as interpreter. + if (root == null || !fs.IsPathUnderRoot(root, p.Path)) { + interpreterPaths.Add(p); + continue; + } + + userPaths.Add(p); + } + + return (interpreterPaths, userPaths.ToList()); + } + + public override bool Equals(object obj) => obj is PythonLibraryPath other && Equals(other); + + public override int GetHashCode() { + // TODO: Replace with HashCode.Combine when .NET Standard 2.1 is out. + unchecked { + var hashCode = Path.GetHashCode(); + hashCode = (hashCode * 397) ^ Type.GetHashCode(); + hashCode = (hashCode * 397) ^ ModulePrefix.GetHashCode(); + return hashCode; + } + } + + public bool Equals(PythonLibraryPath other) => Path == other.Path && Type == other.Type && ModulePrefix == other.ModulePrefix; + + public static bool operator ==(PythonLibraryPath left, PythonLibraryPath right) => left.Equals(right); + + public static bool operator !=(PythonLibraryPath left, PythonLibraryPath right) => !left.Equals(right); + + private class PathDepthComparer : IComparer, IComparer { + public static readonly PathDepthComparer Instance = new PathDepthComparer(); + + private PathDepthComparer() { } + + public int Compare(object x, object y) { + return Compare((PythonLibraryPath)x, (PythonLibraryPath)y); + } + + public int Compare(PythonLibraryPath x, PythonLibraryPath y) { + var xSeps = x.Path.Count(c => c == IOPath.DirectorySeparatorChar); + var ySeps = y.Path.Count(c => c == IOPath.DirectorySeparatorChar); + + var sepComp = xSeps.CompareTo(ySeps); + if (sepComp != 0) { + // Deepest first. + return -sepComp; + } + + return x.Path.PathCompare(y.Path); + } + } } } diff --git a/src/Core/Impl/Extensions/StringExtensions.cs b/src/Core/Impl/Extensions/StringExtensions.cs index 88ee18f16..b27ed2d29 100644 --- a/src/Core/Impl/Extensions/StringExtensions.cs +++ b/src/Core/Impl/Extensions/StringExtensions.cs @@ -204,6 +204,9 @@ public static bool EqualsOrdinal(this string s, string other) public static bool PathEquals(this string s, string other) => string.Equals(s, other, PathsStringComparison); + public static int PathCompare(this string s, string other) + => string.Compare(s, other, PathsStringComparison); + public static bool EqualsOrdinal(this string s, int index, string other, int otherIndex, int length, bool ignoreCase = false) => string.Compare(s, index, other, otherIndex, length, ignoreCase ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal) == 0; public static bool ContainsOrdinal(this string s, string value, bool ignoreCase = false) From 10f42be7bc71eb57b444df5a1804acc6f35a67f7 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Fri, 28 Jun 2019 16:00:47 -0700 Subject: [PATCH 02/17] inherit Comparer instead of implementing both --- src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs b/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs index 5b67a81b3..4f47f3b8d 100644 --- a/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs +++ b/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs @@ -287,16 +287,12 @@ public override int GetHashCode() { public static bool operator !=(PythonLibraryPath left, PythonLibraryPath right) => !left.Equals(right); - private class PathDepthComparer : IComparer, IComparer { + private class PathDepthComparer : Comparer { public static readonly PathDepthComparer Instance = new PathDepthComparer(); private PathDepthComparer() { } - public int Compare(object x, object y) { - return Compare((PythonLibraryPath)x, (PythonLibraryPath)y); - } - - public int Compare(PythonLibraryPath x, PythonLibraryPath y) { + public override int Compare(PythonLibraryPath x, PythonLibraryPath y) { var xSeps = x.Path.Count(c => c == IOPath.DirectorySeparatorChar); var ySeps = y.Path.Count(c => c == IOPath.DirectorySeparatorChar); From 5567c37b77345a4ca790c4e338791e01f77b5979 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 1 Jul 2019 14:33:06 -0700 Subject: [PATCH 03/17] don't do path depth comparison, preserve user ordering as before --- .../Ast/Test/PathClassificationTests.cs | 55 ++++++++++++++++--- .../Impl/Interpreter/PythonLibraryPath.cs | 38 ++++--------- 2 files changed, 58 insertions(+), 35 deletions(-) diff --git a/src/Analysis/Ast/Test/PathClassificationTests.cs b/src/Analysis/Ast/Test/PathClassificationTests.cs index c0e436e88..263d1c376 100644 --- a/src/Analysis/Ast/Test/PathClassificationTests.cs +++ b/src/Analysis/Ast/Test/PathClassificationTests.cs @@ -19,6 +19,7 @@ using Microsoft.Python.Analysis.Core.Interpreter; using Microsoft.Python.Core.IO; using Microsoft.Python.Core.OS; +using Microsoft.Python.Tests.Utilities.FluentAssertions; using Microsoft.VisualStudio.TestTools.UnitTesting; using TestUtilities; @@ -54,7 +55,7 @@ public void Plain() { var (interpreterPaths, userPaths) = PythonLibraryPath.ClassifyPaths(root, _fs, fromInterpreter, Array.Empty()); - interpreterPaths.Should().BeEquivalentTo(new[] { + interpreterPaths.Should().BeEquivalentToWithStrictOrdering(new[] { new PythonLibraryPath(venvLib, PythonLibraryPathType.StdLib), new PythonLibraryPath(venv, PythonLibraryPathType.StdLib), new PythonLibraryPath(venvSitePackages, PythonLibraryPathType.Site), @@ -86,13 +87,13 @@ public void WithSrcDir() { var (interpreterPaths, userPaths) = PythonLibraryPath.ClassifyPaths(root, _fs, fromInterpreter, fromUser); - interpreterPaths.Should().BeEquivalentTo(new[] { + interpreterPaths.Should().BeEquivalentToWithStrictOrdering(new[] { new PythonLibraryPath(venvLib, PythonLibraryPathType.StdLib), new PythonLibraryPath(venv, PythonLibraryPathType.StdLib), new PythonLibraryPath(venvSitePackages, PythonLibraryPathType.Site), }); - userPaths.Should().BeEquivalentTo(new[] { + userPaths.Should().BeEquivalentToWithStrictOrdering(new[] { new PythonLibraryPath(src, PythonLibraryPathType.Unspecified), }); } @@ -112,7 +113,7 @@ public void NormalizeUser() { interpreterPaths.Should().BeEmpty(); - userPaths.Should().BeEquivalentTo(new[] { + userPaths.Should().BeEquivalentToWithStrictOrdering(new[] { new PythonLibraryPath(src, PythonLibraryPathType.Unspecified), }); } @@ -130,18 +131,58 @@ public void NestedUser() { var fromUser = new[] { "./src", "./src/something", - "./src/foo", "./src/foo/bar", + "./src/foo", }; var (interpreterPaths, userPaths) = PythonLibraryPath.ClassifyPaths(root, _fs, Array.Empty(), fromUser); interpreterPaths.Should().BeEmpty(); - userPaths.Should().BeEquivalentTo(new[] { + userPaths.Should().BeEquivalentToWithStrictOrdering(new[] { + new PythonLibraryPath(src, PythonLibraryPathType.Unspecified), + new PythonLibraryPath(srcSomething, PythonLibraryPathType.Unspecified), new PythonLibraryPath(srcFooBar, PythonLibraryPathType.Unspecified), new PythonLibraryPath(srcFoo, PythonLibraryPathType.Unspecified), - new PythonLibraryPath(srcSomething, PythonLibraryPathType.Unspecified), + }); + } + + [TestMethod] + public void InsideStdLib() { + var appPath = TestData.GetTestSpecificPath("app.py"); + var root = Path.GetDirectoryName(appPath); + + var venv = Path.Combine(root, "venv"); + var venvLib = Path.Combine(venv, "Lib"); + var venvSitePackages = Path.Combine(venvLib, "site-packages"); + var inside = Path.Combine(venvSitePackages, "inside"); + var what = Path.Combine(venvSitePackages, "what"); + + var src = Path.Combine(root, "src"); + + var fromInterpreter = new[] { + new PythonLibraryPath(venvLib, PythonLibraryPathType.StdLib), + new PythonLibraryPath(venv, PythonLibraryPathType.StdLib), + new PythonLibraryPath(venvSitePackages, PythonLibraryPathType.Site), + new PythonLibraryPath(inside, PythonLibraryPathType.Pth), + }; + + var fromUser = new[] { + "./src", + "./venv/Lib/site-packages/what", + }; + + var (interpreterPaths, userPaths) = PythonLibraryPath.ClassifyPaths(root, _fs, fromInterpreter, fromUser); + + interpreterPaths.Should().BeEquivalentToWithStrictOrdering(new[] { + new PythonLibraryPath(venvLib, PythonLibraryPathType.StdLib), + new PythonLibraryPath(venv, PythonLibraryPathType.StdLib), + new PythonLibraryPath(what, PythonLibraryPathType.Unspecified), + new PythonLibraryPath(venvSitePackages, PythonLibraryPathType.Site), + new PythonLibraryPath(inside, PythonLibraryPathType.Pth), + }); + + userPaths.Should().BeEquivalentToWithStrictOrdering(new[] { new PythonLibraryPath(src, PythonLibraryPathType.Unspecified), }); } diff --git a/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs b/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs index 4f47f3b8d..e409568ab 100644 --- a/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs +++ b/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs @@ -226,17 +226,18 @@ IEnumerable fromUser // 2) If a path isn't rooted, then root it relative to the workspace root. If there is no root, just continue. // 3) Trim off any ending separators for consistency. // 4) Remove any empty paths, FS root paths (bad idea), or paths equal to the root. - fromUser = fromUser + // 5) Deduplicate, preserving the order specified by the user. + var fromUserList = fromUser .Select(PathUtils.NormalizePath) .Select(p => root == null || IOPath.IsPathRooted(p) ? p : IOPath.GetFullPath(IOPath.Combine(root, p))) // TODO: Replace with GetFullPath(p, root) when .NET Standard 2.1 is out. .Select(PathUtils.TrimEndSeparator) - .Where(p => !string.IsNullOrWhiteSpace(p) && p != "/" && !p.PathEquals(root)); - - // Deduplicate, and keep in a set to quickly check interpreter paths against. - var fromUserSet = new HashSet(fromUser, PathEqualityComparer.Instance); + .Where(p => !string.IsNullOrWhiteSpace(p) && p != "/" && !p.PathEquals(root)) + .Distinct(PathEqualityComparer.Instance) + .ToList(); // Remove any interpreter paths specified in the user config so they can be reclassified. - fromInterpreter = fromInterpreter.Where(p => !fromUserSet.Contains(p.Path)); + // The user list is usually small; List.Contains should not be too slow. + fromInterpreter = fromInterpreter.Where(p => !fromUserList.Contains(p.Path, PathEqualityComparer.Instance)); var stdlibLookup = fromInterpreter.ToLookup(p => p.Type == PythonLibraryPathType.StdLib); @@ -245,9 +246,9 @@ IEnumerable fromUser var interpreterPaths = new List(stdlib); fromInterpreter = stdlibLookup[false]; - var userPaths = new SortedSet(PathDepthComparer.Instance); + var userPaths = new List(); - var allPaths = fromUserSet.Select(p => new PythonLibraryPath(p)) + var allPaths = fromUserList.Select(p => new PythonLibraryPath(p)) .Concat(fromInterpreter.Where(p => !p.Path.PathEquals(root))); foreach (var p in allPaths) { @@ -281,29 +282,10 @@ public override int GetHashCode() { } } - public bool Equals(PythonLibraryPath other) => Path == other.Path && Type == other.Type && ModulePrefix == other.ModulePrefix; + public bool Equals(PythonLibraryPath other) => Path.PathEquals(other.Path) && Type == other.Type && ModulePrefix == other.ModulePrefix; public static bool operator ==(PythonLibraryPath left, PythonLibraryPath right) => left.Equals(right); public static bool operator !=(PythonLibraryPath left, PythonLibraryPath right) => !left.Equals(right); - - private class PathDepthComparer : Comparer { - public static readonly PathDepthComparer Instance = new PathDepthComparer(); - - private PathDepthComparer() { } - - public override int Compare(PythonLibraryPath x, PythonLibraryPath y) { - var xSeps = x.Path.Count(c => c == IOPath.DirectorySeparatorChar); - var ySeps = y.Path.Count(c => c == IOPath.DirectorySeparatorChar); - - var sepComp = xSeps.CompareTo(ySeps); - if (sepComp != 0) { - // Deepest first. - return -sepComp; - } - - return x.Path.PathCompare(y.Path); - } - } } } From 98cc56fe80ef9f2c3d6bf4211f641ebf3dced196 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Tue, 2 Jul 2019 15:43:34 -0700 Subject: [PATCH 04/17] switch to using new path classification code in main module resolution --- .../Resolution/MainModuleResolution.cs | 45 ++++--------------- .../Ast/Test/PathClassificationTests.cs | 1 - .../Impl/Interpreter/PythonLibraryPath.cs | 14 ++++-- .../Impl/Implementation/Server.cs | 5 --- 4 files changed, 20 insertions(+), 45 deletions(-) diff --git a/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs b/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs index 514ccea1c..7987f3d04 100644 --- a/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs +++ b/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs @@ -37,7 +37,6 @@ namespace Microsoft.Python.Analysis.Modules.Resolution { internal sealed class MainModuleResolution : ModuleResolutionBase, IModuleManagement { private readonly ConcurrentDictionary _specialized = new ConcurrentDictionary(); private IRunningDocumentTable _rdt; - private IReadOnlyList _searchPaths; public MainModuleResolution(string root, IServiceContainer services) : base(root, services) { } @@ -62,29 +61,6 @@ internal async Task InitializeAsync(CancellationToken cancellationToken = defaul cancellationToken.ThrowIfCancellationRequested(); } - public async Task> GetSearchPathsAsync(CancellationToken cancellationToken = default) { - if (_searchPaths != null) { - return _searchPaths; - } - - _searchPaths = await GetInterpreterSearchPathsAsync(cancellationToken); - Debug.Assert(_searchPaths != null, "Should have search paths"); - _searchPaths = _searchPaths ?? Array.Empty(); - - _log?.Log(TraceEventType.Verbose, "Python search paths:"); - foreach (var s in _searchPaths) { - _log?.Log(TraceEventType.Verbose, $" {s}"); - } - - var configurationSearchPaths = Configuration.SearchPaths ?? Array.Empty(); - - _log?.Log(TraceEventType.Verbose, "Configuration search paths:"); - foreach (var s in configurationSearchPaths) { - _log?.Log(TraceEventType.Verbose, $" {s}"); - } - return _searchPaths; - } - protected override IPythonModule CreateModule(string name) { var moduleImport = CurrentPathResolver.GetModuleImportFromModuleName(name); if (moduleImport == null) { @@ -135,11 +111,11 @@ protected override IPythonModule CreateModule(string name) { return GetRdt().AddModule(mco); } - private async Task> GetInterpreterSearchPathsAsync(CancellationToken cancellationToken = default) { + private async Task> GetInterpreterSearchPathsAsync(CancellationToken cancellationToken = default) { if (!_fs.FileExists(Configuration.InterpreterPath)) { _log?.Log(TraceEventType.Warning, "Interpreter does not exist:", Configuration.InterpreterPath); _ui?.ShowMessageAsync(Resources.InterpreterNotFound, TraceEventType.Error); - return Array.Empty(); + return Array.Empty(); } _log?.Log(TraceEventType.Information, "GetCurrentSearchPaths", Configuration.InterpreterPath); @@ -148,11 +124,11 @@ private async Task> GetInterpreterSearchPathsAsync(Cancell var ps = _services.GetService(); var paths = await PythonLibraryPath.GetSearchPathsAsync(Configuration, fs, ps, cancellationToken); cancellationToken.ThrowIfCancellationRequested(); - return paths.MaybeEnumerate().Select(p => p.Path).ToArray(); + return paths.MaybeEnumerate().ToArray(); } catch (InvalidOperationException ex) { _log?.Log(TraceEventType.Warning, "Exception getting search paths", ex); _ui?.ShowMessageAsync(Resources.ExceptionGettingSearchPaths, TraceEventType.Error); - return Array.Empty(); + return Array.Empty(); } } @@ -211,16 +187,13 @@ public async Task ReloadAsync(CancellationToken cancellationToken = default) { var addedRoots = new HashSet(); addedRoots.UnionWith(PathResolver.SetRoot(Root)); - InterpreterPaths = await GetSearchPathsAsync(cancellationToken); + var ps = _services.GetService(); - IEnumerable userSearchPaths = Configuration.SearchPaths; - InterpreterPaths = InterpreterPaths.Except(userSearchPaths, StringExtensions.PathsStringComparer).Where(p => !p.PathEquals(Root)); + var paths = await GetInterpreterSearchPathsAsync(cancellationToken); + var (interpreterPaths, userPaths) = PythonLibraryPath.ClassifyPaths(Root, _fs, paths, Configuration.SearchPaths); - if (Root != null) { - var underRoot = userSearchPaths.ToLookup(p => _fs.IsPathUnderRoot(Root, p)); - userSearchPaths = underRoot[true]; - InterpreterPaths = underRoot[false].Concat(InterpreterPaths); - } + InterpreterPaths = interpreterPaths.Select(p => p.Path); + var userSearchPaths = userPaths.Select(p => p.Path); _log?.Log(TraceEventType.Information, "Interpreter search paths:"); foreach (var s in InterpreterPaths) { diff --git a/src/Analysis/Ast/Test/PathClassificationTests.cs b/src/Analysis/Ast/Test/PathClassificationTests.cs index 263d1c376..df17075ff 100644 --- a/src/Analysis/Ast/Test/PathClassificationTests.cs +++ b/src/Analysis/Ast/Test/PathClassificationTests.cs @@ -23,7 +23,6 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; using TestUtilities; - namespace Microsoft.Python.Analysis.Tests { [TestClass] public class PathClassificationTests { diff --git a/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs b/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs index e409568ab..a85919cfd 100644 --- a/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs +++ b/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs @@ -282,10 +282,18 @@ public override int GetHashCode() { } } - public bool Equals(PythonLibraryPath other) => Path.PathEquals(other.Path) && Type == other.Type && ModulePrefix == other.ModulePrefix; + public bool Equals(PythonLibraryPath other) { + if (other is null) { + return false; + } + + return Path.PathEquals(other.Path) + && Type == other.Type + && ModulePrefix == other.ModulePrefix; + } - public static bool operator ==(PythonLibraryPath left, PythonLibraryPath right) => left.Equals(right); + public static bool operator ==(PythonLibraryPath left, PythonLibraryPath right) => left?.Equals(right) ?? right is null; - public static bool operator !=(PythonLibraryPath left, PythonLibraryPath right) => !left.Equals(right); + public static bool operator !=(PythonLibraryPath left, PythonLibraryPath right) => !(left?.Equals(right) ?? right is null); } } diff --git a/src/LanguageServer/Impl/Implementation/Server.cs b/src/LanguageServer/Impl/Implementation/Server.cs index 61324ece6..b60e1a276 100644 --- a/src/LanguageServer/Impl/Implementation/Server.cs +++ b/src/LanguageServer/Impl/Implementation/Server.cs @@ -133,11 +133,6 @@ public async Task InitializeAsync(InitializeParams @params, Ca // 6) Remove duplicates. SearchPaths = @params.initializationOptions.searchPaths .Select(p => p.Split(';', StringSplitOptions.RemoveEmptyEntries)).SelectMany() - .Select(PathUtils.NormalizePath) - .Select(p => _rootDir == null || Path.IsPathRooted(p) ? p : Path.GetFullPath(p, _rootDir)) - .Select(PathUtils.TrimEndSeparator) - .Where(p => !string.IsNullOrWhiteSpace(p) && p != "/" && !p.PathEquals(_rootDir)) - .Distinct(PathEqualityComparer.Instance) .ToList(), TypeshedPath = @params.initializationOptions.typeStubSearchPaths.FirstOrDefault() }; From dc9259dc3791f23d33acf2c87247162beda3c7e2 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 8 Jul 2019 15:06:19 -0700 Subject: [PATCH 05/17] fix typo --- src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs b/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs index a85919cfd..aaf887ae3 100644 --- a/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs +++ b/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs @@ -222,7 +222,7 @@ IEnumerable fromUser // - All paths in fromInterpreter were normalised and end separator trimmed. // Clean up user configured paths. - // 1) Noramlize paths. + // 1) Normalize paths. // 2) If a path isn't rooted, then root it relative to the workspace root. If there is no root, just continue. // 3) Trim off any ending separators for consistency. // 4) Remove any empty paths, FS root paths (bad idea), or paths equal to the root. From 01339ce18226ebdc866f2dc39423151a1c70b85b Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 8 Jul 2019 15:22:27 -0700 Subject: [PATCH 06/17] clean up comment about paths in Server.cs --- src/LanguageServer/Impl/Implementation/Server.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/LanguageServer/Impl/Implementation/Server.cs b/src/LanguageServer/Impl/Implementation/Server.cs index b60e1a276..dbf294e5d 100644 --- a/src/LanguageServer/Impl/Implementation/Server.cs +++ b/src/LanguageServer/Impl/Implementation/Server.cs @@ -125,12 +125,8 @@ public async Task InitializeAsync(InitializeParams @params, Ca interpreterPath: @params.initializationOptions.interpreter.properties?.InterpreterPath, version: version ) { - // 1) Split on ';' to support older VS Code extension versions which send paths as a single entry separated by ';'. TODO: Eventually remove. - // 2) Normalize paths. - // 3) If a path isn't rooted, then root it relative to the workspace root. If _rootDir is null, then accept the path as-is. - // 4) Trim off any ending separator for a consistent style. - // 5) Filter out any entries which are the same as the workspace root; they are redundant. Also ignore "/" to work around the extension (for now). - // 6) Remove duplicates. + // Split on ';' to support older VS Code extension versions which send paths as a single entry separated by ';'. TODO: Eventually remove. + // Note that the actual classification of these paths as user/library is done later in MainModuleResolution.ReloadAsync. SearchPaths = @params.initializationOptions.searchPaths .Select(p => p.Split(';', StringSplitOptions.RemoveEmptyEntries)).SelectMany() .ToList(), From b7f010ab5971f3632052a1786584843d3c75afd4 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Tue, 9 Jul 2019 09:39:34 -0700 Subject: [PATCH 07/17] use String.Split instead of regex when parsing script output --- .../Impl/Interpreter/PythonLibraryPath.cs | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs b/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs index aaf887ae3..1e0887895 100644 --- a/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs +++ b/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs @@ -14,13 +14,10 @@ // permissions and limitations under the License. using System; -using System.Collections; using System.Collections.Generic; using System.Diagnostics; using System.IO; using System.Linq; -using System.Text; -using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; using Microsoft.Python.Core; @@ -37,10 +34,6 @@ public enum PythonLibraryPathType { } public sealed class PythonLibraryPath : IEquatable { - private static readonly Regex ParseRegex = new Regex( - @"(?[^|]+)\|(?[^|]+)\|(?[^|]+)?" - ); - public PythonLibraryPath(string path, PythonLibraryPathType type = PythonLibraryPathType.Unspecified, string modulePrefix = null) { Path = PathUtils.TrimEndSeparator(PathUtils.NormalizePath(path)); Type = type; @@ -81,14 +74,18 @@ public static PythonLibraryPath Parse(string s) { throw new ArgumentNullException("source"); } - var m = ParseRegex.Match(s); - if (!m.Success || !m.Groups["path"].Success || !m.Groups["type"].Success) { + var parts = s.Split(new[] { '|' }, 3); + if (parts.Length < 3) { throw new FormatException(); } + var path = parts[0]; + var ty = parts[1]; + var prefix = parts[2]; + PythonLibraryPathType type = PythonLibraryPathType.Unspecified; - switch (m.Groups["type"].Value) { + switch (ty) { case "stdlib": type = PythonLibraryPathType.StdLib; break; @@ -100,11 +97,7 @@ public static PythonLibraryPath Parse(string s) { break; } - return new PythonLibraryPath( - m.Groups["path"].Value, - type, - m.Groups["prefix"].Success ? m.Groups["prefix"].Value : null - ); + return new PythonLibraryPath(path, type, prefix); } /// From 78010cc8d6ba0d9da3a0d31d702b52d6aadc50ac Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Tue, 9 Jul 2019 10:03:06 -0700 Subject: [PATCH 08/17] test ordering of user provided paths --- .../Ast/Test/PathClassificationTests.cs | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/Analysis/Ast/Test/PathClassificationTests.cs b/src/Analysis/Ast/Test/PathClassificationTests.cs index df17075ff..429df72b7 100644 --- a/src/Analysis/Ast/Test/PathClassificationTests.cs +++ b/src/Analysis/Ast/Test/PathClassificationTests.cs @@ -146,6 +146,35 @@ public void NestedUser() { }); } + [TestMethod] + public void NestedUserOrdering() { + var appPath = TestData.GetTestSpecificPath("app.py"); + var root = Path.GetDirectoryName(appPath); + + var src = Path.Combine(root, "src"); + var srcSomething = Path.Combine(src, "something"); + var srcFoo = Path.Combine(src, "foo"); + var srcFooBar = Path.Combine(srcFoo, "bar"); + + var fromUser = new[] { + "./src/foo", + "./src/foo/bar", + "./src", + "./src/something", + }; + + var (interpreterPaths, userPaths) = PythonLibraryPath.ClassifyPaths(root, _fs, Array.Empty(), fromUser); + + interpreterPaths.Should().BeEmpty(); + + userPaths.Should().BeEquivalentToWithStrictOrdering(new[] { + new PythonLibraryPath(srcFoo, PythonLibraryPathType.Unspecified), + new PythonLibraryPath(srcFooBar, PythonLibraryPathType.Unspecified), + new PythonLibraryPath(src, PythonLibraryPathType.Unspecified), + new PythonLibraryPath(srcSomething, PythonLibraryPathType.Unspecified), + }); + } + [TestMethod] public void InsideStdLib() { var appPath = TestData.GetTestSpecificPath("app.py"); From 524ae5b10d91a961b198935fa2d87944196633d3 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Tue, 9 Jul 2019 10:25:31 -0700 Subject: [PATCH 09/17] add new normalize and trim helper, check preconditions as debug asserts rather than commenting on them --- src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs | 9 +++++---- src/Core/Impl/IO/PathUtils.cs | 2 ++ src/LanguageServer/Impl/Implementation/Server.cs | 3 +-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs b/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs index 1e0887895..39abfca5f 100644 --- a/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs +++ b/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs @@ -35,7 +35,7 @@ public enum PythonLibraryPathType { public sealed class PythonLibraryPath : IEquatable { public PythonLibraryPath(string path, PythonLibraryPathType type = PythonLibraryPathType.Unspecified, string modulePrefix = null) { - Path = PathUtils.TrimEndSeparator(PathUtils.NormalizePath(path)); + Path = PathUtils.NormalizePathAndTrim(path); Type = type; ModulePrefix = modulePrefix ?? string.Empty; } @@ -210,9 +210,10 @@ public static (IReadOnlyList interpreterPaths, IReadOnlyList< IEnumerable fromInterpreter, IEnumerable fromUser ) { - // PRECONDITIONS: - // - root has already been normalized and had its end separator trimmed. - // - All paths in fromInterpreter were normalised and end separator trimmed. +#if DEBUG + Debug.Assert(root == null || root.PathEquals(PathUtils.NormalizePathAndTrim(root))); + Debug.Assert(!fromInterpreter.Any(p => !p.Path.PathEquals(PathUtils.NormalizePathAndTrim(p.Path)))); +#endif // Clean up user configured paths. // 1) Normalize paths. diff --git a/src/Core/Impl/IO/PathUtils.cs b/src/Core/Impl/IO/PathUtils.cs index a9513ee59..a5140c1a0 100644 --- a/src/Core/Impl/IO/PathUtils.cs +++ b/src/Core/Impl/IO/PathUtils.cs @@ -493,5 +493,7 @@ public static string NormalizePath(string path) { ); return isDir ? EnsureEndSeparator(newPath) : newPath; } + + public static string NormalizePathAndTrim(string path) => TrimEndSeparator(NormalizePath(path)); } } diff --git a/src/LanguageServer/Impl/Implementation/Server.cs b/src/LanguageServer/Impl/Implementation/Server.cs index dbf294e5d..7bb4571a6 100644 --- a/src/LanguageServer/Impl/Implementation/Server.cs +++ b/src/LanguageServer/Impl/Implementation/Server.cs @@ -115,8 +115,7 @@ public async Task InitializeAsync(InitializeParams @params, Ca _rootDir = @params.rootUri != null ? @params.rootUri.ToAbsolutePath() : @params.rootPath; if (_rootDir != null) { - _rootDir = PathUtils.NormalizePath(_rootDir); - _rootDir = PathUtils.TrimEndSeparator(_rootDir); + _rootDir = PathUtils.NormalizePathAndTrim(_rootDir); } Version.TryParse(@params.initializationOptions.interpreter.properties?.Version, out var version); From 51b1fecbd0048c29e9d9408bf5d7fd1970e548bc Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Tue, 9 Jul 2019 16:41:54 -0700 Subject: [PATCH 10/17] Bring back search path watcher for interpreter paths --- .../Resolution/MainModuleResolution.cs | 48 +++++++++++-------- .../Impl/Implementation/Server.cs | 47 +++++++++++++++--- .../Impl/LanguageServer.Configuration.cs | 2 +- src/LanguageServer/Impl/LanguageServer.cs | 31 +----------- src/LanguageServer/Impl/PathsWatcher.cs | 9 +++- 5 files changed, 79 insertions(+), 58 deletions(-) diff --git a/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs b/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs index 7987f3d04..4dd19f4bb 100644 --- a/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs +++ b/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs @@ -38,6 +38,8 @@ internal sealed class MainModuleResolution : ModuleResolutionBase, IModuleManage private readonly ConcurrentDictionary _specialized = new ConcurrentDictionary(); private IRunningDocumentTable _rdt; + private IEnumerable _userPaths = Enumerable.Empty(); + public MainModuleResolution(string root, IServiceContainer services) : base(root, services) { } @@ -75,7 +77,7 @@ protected override IPythonModule CreateModule(string name) { return module; } } - + // If there is a stub, make sure it is loaded and attached // First check stub next to the module. if (!TryCreateModuleStub(name, moduleImport.ModulePath, out var stub)) { @@ -169,6 +171,26 @@ internal async Task LoadBuiltinTypesAsync(CancellationToken cancellationToken = } } + internal async Task ReloadSearchPaths(CancellationToken cancellationToken = default) { + var ps = _services.GetService(); + + var paths = await GetInterpreterSearchPathsAsync(cancellationToken); + var (interpreterPaths, userPaths) = PythonLibraryPath.ClassifyPaths(Root, _fs, paths, Configuration.SearchPaths); + + InterpreterPaths = interpreterPaths.Select(p => p.Path); + _userPaths = userPaths.Select(p => p.Path); + + _log?.Log(TraceEventType.Information, "Interpreter search paths:"); + foreach (var s in InterpreterPaths) { + _log?.Log(TraceEventType.Information, $" {s}"); + } + + _log?.Log(TraceEventType.Information, "User search paths:"); + foreach (var s in _userPaths) { + _log?.Log(TraceEventType.Information, $" {s}"); + } + } + public async Task ReloadAsync(CancellationToken cancellationToken = default) { foreach (var uri in Modules .Where(m => m.Value.Value?.Name != BuiltinModuleName) @@ -176,7 +198,7 @@ public async Task ReloadAsync(CancellationToken cancellationToken = default) { .ExcludeDefault()) { GetRdt()?.UnlockDocument(uri); } - + // Preserve builtins, they don't need to be reloaded since interpreter does not change. var builtins = Modules[BuiltinModuleName]; Modules.Clear(); @@ -187,30 +209,14 @@ public async Task ReloadAsync(CancellationToken cancellationToken = default) { var addedRoots = new HashSet(); addedRoots.UnionWith(PathResolver.SetRoot(Root)); - var ps = _services.GetService(); - - var paths = await GetInterpreterSearchPathsAsync(cancellationToken); - var (interpreterPaths, userPaths) = PythonLibraryPath.ClassifyPaths(Root, _fs, paths, Configuration.SearchPaths); - - InterpreterPaths = interpreterPaths.Select(p => p.Path); - var userSearchPaths = userPaths.Select(p => p.Path); - - _log?.Log(TraceEventType.Information, "Interpreter search paths:"); - foreach (var s in InterpreterPaths) { - _log?.Log(TraceEventType.Information, $" {s}"); - } - - _log?.Log(TraceEventType.Information, "User search paths:"); - foreach (var s in userSearchPaths) { - _log?.Log(TraceEventType.Information, $" {s}"); - } + await ReloadSearchPaths(cancellationToken); addedRoots.UnionWith(PathResolver.SetInterpreterSearchPaths(InterpreterPaths)); - addedRoots.UnionWith(SetUserSearchPaths(userSearchPaths)); + addedRoots.UnionWith(SetUserSearchPaths(_userPaths)); ReloadModulePaths(addedRoots); } - public IEnumerable SetUserSearchPaths(in IEnumerable searchPaths) + public IEnumerable SetUserSearchPaths(in IEnumerable searchPaths) => PathResolver.SetUserSearchPaths(searchPaths); // For tests diff --git a/src/LanguageServer/Impl/Implementation/Server.cs b/src/LanguageServer/Impl/Implementation/Server.cs index 7bb4571a6..478f618bd 100644 --- a/src/LanguageServer/Impl/Implementation/Server.cs +++ b/src/LanguageServer/Impl/Implementation/Server.cs @@ -49,6 +49,10 @@ public sealed partial class Server : IDisposable { private IIndexManager _indexManager; private string _rootDir; + private bool _watchSearchPaths; + private PathsWatcher _pathsWatcher; + private string[] _searchPaths; + public Server(IServiceManager services) { _services = services; @@ -58,6 +62,7 @@ public Server(IServiceManager services) { ext.Dispose(); } }) + .Add(() => _pathsWatcher?.Dispose()) .Add(() => _shutdownCts.Cancel()); } @@ -173,11 +178,11 @@ public void DidChangeConfiguration(DidChangeConfigurationParams @params, Cancell _disposableBag.ThrowIfDisposed(); switch (@params.settings) { case ServerSettings settings: { - if (HandleConfigurationChanges(settings)) { - RestartAnalysis(); + if (HandleConfigurationChanges(settings)) { + RestartAnalysis(); + } + break; } - break; - } default: _log?.Log(TraceEventType.Error, "change configuration notification sent unsupported settings"); break; @@ -233,7 +238,36 @@ private IDocumentationSource ChooseDocumentationSource(string[] kinds) { } #endregion - public void NotifyPackagesChanged(CancellationToken cancellationToken) { + public void HandlePathWatchChanges(bool watchSearchPaths) { + if (!watchSearchPaths) { + _searchPaths = null; + _watchSearchPaths = false; + _pathsWatcher?.Dispose(); + _pathsWatcher = null; + return; + } + + if (!_watchSearchPaths) { + _watchSearchPaths = true; + ResetPathWatcher(); + } + } + + private void ResetPathWatcher() { + if (!_watchSearchPaths) { + return; + } + + var paths = _interpreter.ModuleResolution.InterpreterPaths.ToArray(); + + if (_searchPaths == null || !_searchPaths.SequenceEqual(paths)) { + _searchPaths = paths; + _pathsWatcher?.Dispose(); + _pathsWatcher = new PathsWatcher(_searchPaths, () => NotifyPackagesChanged(), _log); + } + } + + public void NotifyPackagesChanged(CancellationToken cancellationToken = default) { var interpreter = _services.GetService(); _log?.Log(TraceEventType.Information, Resources.ReloadingModules); // No need to reload typeshed resolution since it is a static storage. @@ -243,12 +277,13 @@ public void NotifyPackagesChanged(CancellationToken cancellationToken) { _log?.Log(TraceEventType.Information, Resources.Done); _log?.Log(TraceEventType.Information, Resources.AnalysisRestarted); RestartAnalysis(); + ResetPathWatcher(); }, cancellationToken).DoNotWait(); } private void RestartAnalysis() { - var analyzer = Services.GetService();; + var analyzer = Services.GetService(); ; analyzer.ResetAnalyzer(); foreach (var doc in _rdt.GetDocuments()) { doc.Reset(null); diff --git a/src/LanguageServer/Impl/LanguageServer.Configuration.cs b/src/LanguageServer/Impl/LanguageServer.Configuration.cs index 180a8fb21..1129c6a83 100644 --- a/src/LanguageServer/Impl/LanguageServer.Configuration.cs +++ b/src/LanguageServer/Impl/LanguageServer.Configuration.cs @@ -51,7 +51,7 @@ public async Task DidChangeConfiguration(JToken token, CancellationToken cancell settings.symbolsHierarchyMaxSymbols = GetSetting(analysis, "symbolsHierarchyMaxSymbols", 1000); _logger.LogLevel = GetLogLevel(analysis).ToTraceEventType(); - HandlePathWatchChanges(token, cancellationToken); + HandlePathWatchChanges(token); HandleDiagnosticsChanges(pythonSection, settings); _server.DidChangeConfiguration(new DidChangeConfigurationParams { settings = settings }, cancellationToken); diff --git a/src/LanguageServer/Impl/LanguageServer.cs b/src/LanguageServer/Impl/LanguageServer.cs index cb172f7c1..bae5e54bf 100644 --- a/src/LanguageServer/Impl/LanguageServer.cs +++ b/src/LanguageServer/Impl/LanguageServer.cs @@ -55,12 +55,8 @@ public sealed partial class LanguageServer : IDisposable { private JsonRpc _rpc; private JsonSerializer _jsonSerializer; - private PathsWatcher _pathsWatcher; private IIdleTimeTracker _idleTimeTracker; - private bool _watchSearchPaths; - private string[] _searchPaths = Array.Empty(); - public CancellationToken Start(IServiceManager services, JsonRpc rpc) { _server = new Server(services); _services = services; @@ -78,7 +74,6 @@ public CancellationToken Start(IServiceManager services, JsonRpc rpc) { _disposables .Add(() => _shutdownCts.Cancel()) .Add(_prioritizer) - .Add(() => _pathsWatcher?.Dispose()) .Add(() => _rpc.TraceSource.Listeners.Remove(rpcTraceListener)); services.AddService(_optionsProvider); @@ -357,30 +352,8 @@ private MessageType GetLogLevel(JToken analysisKey) { return MessageType.Error; } - private void HandlePathWatchChanges(JToken section, CancellationToken cancellationToken) { - var watchSearchPaths = GetSetting(section, "watchSearchPaths", true); - if (!watchSearchPaths) { - // No longer watching. - _pathsWatcher?.Dispose(); - _searchPaths = Array.Empty(); - _watchSearchPaths = false; - return; - } - - // Now watching. - if (!_watchSearchPaths || (_watchSearchPaths && _searchPaths.SetEquals(_initParams.initializationOptions.searchPaths))) { - // Were not watching OR were watching but paths have changed. Recreate the watcher. - _pathsWatcher?.Dispose(); - _pathsWatcher = new PathsWatcher( - _initParams.initializationOptions.searchPaths, - () =>_server.NotifyPackagesChanged(cancellationToken), - _services.GetService() - ); - - _watchSearchPaths = true; - _searchPaths = _initParams.initializationOptions.searchPaths; - } - } + private void HandlePathWatchChanges(JToken section) + => _server.HandlePathWatchChanges(GetSetting(section, "watchSearchPaths", true)); private static CancellationToken GetToken(CancellationToken original) => Debugger.IsAttached ? CancellationToken.None : original; diff --git a/src/LanguageServer/Impl/PathsWatcher.cs b/src/LanguageServer/Impl/PathsWatcher.cs index fe331385d..5f8b5e239 100644 --- a/src/LanguageServer/Impl/PathsWatcher.cs +++ b/src/LanguageServer/Impl/PathsWatcher.cs @@ -43,6 +43,7 @@ public PathsWatcher(string[] paths, Action onChanged, ILogger log) { _onChanged = onChanged; var reduced = ReduceToCommonRoots(paths); + foreach (var p in reduced) { try { if (!Directory.Exists(p)) { @@ -53,20 +54,26 @@ public PathsWatcher(string[] paths, Action onChanged, ILogger log) { continue; } + _log.Log(TraceEventType.Verbose, $"Watching {p}"); + try { var fsw = new System.IO.FileSystemWatcher(p) { IncludeSubdirectories = true, EnableRaisingEvents = true, - NotifyFilter = NotifyFilters.FileName | NotifyFilters.DirectoryName + NotifyFilter = NotifyFilters.FileName | NotifyFilters.DirectoryName | NotifyFilters.LastWrite }; + fsw.Changed += OnChanged; fsw.Created += OnChanged; fsw.Deleted += OnChanged; + fsw.Renamed += OnChanged; _disposableBag .Add(() => _throttleTimer?.Dispose()) + .Add(() => fsw.Changed -= OnChanged) .Add(() => fsw.Created -= OnChanged) .Add(() => fsw.Deleted -= OnChanged) + .Add(() => fsw.Renamed -= OnChanged) .Add(() => fsw.EnableRaisingEvents = false) .Add(fsw); } catch (ArgumentException ex) { From 8fcd0d47e0da3a954ec09ea71d79e6ceba279069 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Wed, 10 Jul 2019 14:56:12 -0700 Subject: [PATCH 11/17] make watchSearchPaths change code more obvious, remove stray semicolon --- .../Impl/Implementation/Server.cs | 18 ++++++++++-------- src/LanguageServer/Impl/LanguageServer.cs | 2 +- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/LanguageServer/Impl/Implementation/Server.cs b/src/LanguageServer/Impl/Implementation/Server.cs index 478f618bd..b87d55fed 100644 --- a/src/LanguageServer/Impl/Implementation/Server.cs +++ b/src/LanguageServer/Impl/Implementation/Server.cs @@ -238,19 +238,21 @@ private IDocumentationSource ChooseDocumentationSource(string[] kinds) { } #endregion - public void HandlePathWatchChanges(bool watchSearchPaths) { - if (!watchSearchPaths) { + public void HandleWatchPathsChange(bool watchSearchPaths) { + if (watchSearchPaths == _watchSearchPaths) { + return; + } + + _watchSearchPaths = watchSearchPaths; + + if (!_watchSearchPaths) { _searchPaths = null; - _watchSearchPaths = false; _pathsWatcher?.Dispose(); _pathsWatcher = null; return; } - if (!_watchSearchPaths) { - _watchSearchPaths = true; - ResetPathWatcher(); - } + ResetPathWatcher(); } private void ResetPathWatcher() { @@ -283,7 +285,7 @@ public void NotifyPackagesChanged(CancellationToken cancellationToken = default) } private void RestartAnalysis() { - var analyzer = Services.GetService(); ; + var analyzer = Services.GetService(); analyzer.ResetAnalyzer(); foreach (var doc in _rdt.GetDocuments()) { doc.Reset(null); diff --git a/src/LanguageServer/Impl/LanguageServer.cs b/src/LanguageServer/Impl/LanguageServer.cs index 655797ead..04f48b9fe 100644 --- a/src/LanguageServer/Impl/LanguageServer.cs +++ b/src/LanguageServer/Impl/LanguageServer.cs @@ -353,7 +353,7 @@ private MessageType GetLogLevel(JToken analysisKey) { } private void HandlePathWatchChanges(JToken section) - => _server.HandlePathWatchChanges(GetSetting(section, "watchSearchPaths", true)); + => _server.HandleWatchPathsChange(GetSetting(section, "watchSearchPaths", true)); private static CancellationToken GetToken(CancellationToken original) => Debugger.IsAttached ? CancellationToken.None : original; From e2343e55c1e9a538ba78bed78b6e28105cf3c9a3 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Wed, 10 Jul 2019 14:58:06 -0700 Subject: [PATCH 12/17] perform check only once --- src/LanguageServer/Impl/Implementation/Server.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/LanguageServer/Impl/Implementation/Server.cs b/src/LanguageServer/Impl/Implementation/Server.cs index b87d55fed..980b9dbf2 100644 --- a/src/LanguageServer/Impl/Implementation/Server.cs +++ b/src/LanguageServer/Impl/Implementation/Server.cs @@ -256,10 +256,6 @@ public void HandleWatchPathsChange(bool watchSearchPaths) { } private void ResetPathWatcher() { - if (!_watchSearchPaths) { - return; - } - var paths = _interpreter.ModuleResolution.InterpreterPaths.ToArray(); if (_searchPaths == null || !_searchPaths.SequenceEqual(paths)) { @@ -278,8 +274,12 @@ public void NotifyPackagesChanged(CancellationToken cancellationToken = default) interpreter.ModuleResolution.ReloadAsync(cancellationToken).ContinueWith(t => { _log?.Log(TraceEventType.Information, Resources.Done); _log?.Log(TraceEventType.Information, Resources.AnalysisRestarted); + RestartAnalysis(); - ResetPathWatcher(); + + if (_watchSearchPaths) { + ResetPathWatcher(); + } }, cancellationToken).DoNotWait(); } From a437527755640c13d9c61972245a84e26389ade6 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Thu, 11 Jul 2019 14:11:26 -0700 Subject: [PATCH 13/17] filter files to python-ish ones, up buffer size to maximum --- src/LanguageServer/Impl/PathsWatcher.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/LanguageServer/Impl/PathsWatcher.cs b/src/LanguageServer/Impl/PathsWatcher.cs index 5f8b5e239..f92b7fd8e 100644 --- a/src/LanguageServer/Impl/PathsWatcher.cs +++ b/src/LanguageServer/Impl/PathsWatcher.cs @@ -43,7 +43,7 @@ public PathsWatcher(string[] paths, Action onChanged, ILogger log) { _onChanged = onChanged; var reduced = ReduceToCommonRoots(paths); - + foreach (var p in reduced) { try { if (!Directory.Exists(p)) { @@ -60,7 +60,8 @@ public PathsWatcher(string[] paths, Action onChanged, ILogger log) { var fsw = new System.IO.FileSystemWatcher(p) { IncludeSubdirectories = true, EnableRaisingEvents = true, - NotifyFilter = NotifyFilters.FileName | NotifyFilters.DirectoryName | NotifyFilters.LastWrite + NotifyFilter = NotifyFilters.FileName | NotifyFilters.DirectoryName | NotifyFilters.LastWrite, + InternalBufferSize = 1 << 16, // Max buffer size of 64 KB }; fsw.Changed += OnChanged; @@ -68,6 +69,8 @@ public PathsWatcher(string[] paths, Action onChanged, ILogger log) { fsw.Deleted += OnChanged; fsw.Renamed += OnChanged; + fsw.Filter = "*.p*"; // .py, .pyc, .pth - TODO: Use Filters in .NET Core 3.0. + _disposableBag .Add(() => _throttleTimer?.Dispose()) .Add(() => fsw.Changed -= OnChanged) From af303d602dc16161324cbf1c2bda36698d0fa7ca Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Thu, 11 Jul 2019 16:36:33 -0700 Subject: [PATCH 14/17] avoid reanalysis of non-existent files on reload by removing all unopened files from the table --- .../Documents/Definitions/IRunningDocumentTable.cs | 6 ++++++ .../Ast/Impl/Documents/RunningDocumentTable.cs | 13 +++++++++++++ src/LanguageServer/Impl/Implementation/Server.cs | 6 +++++- 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/Analysis/Ast/Impl/Documents/Definitions/IRunningDocumentTable.cs b/src/Analysis/Ast/Impl/Documents/Definitions/IRunningDocumentTable.cs index 0debc17ad..b9f92e1bd 100644 --- a/src/Analysis/Ast/Impl/Documents/Definitions/IRunningDocumentTable.cs +++ b/src/Analysis/Ast/Impl/Documents/Definitions/IRunningDocumentTable.cs @@ -51,6 +51,12 @@ public interface IRunningDocumentTable { /// void CloseDocument(Uri uri); + /// + /// Forcibly removes the document from the table. This + /// shouldn't be called if the document is locked/has references. + /// + void RemoveDocument(Uri uri); + /// /// Fetches document by its URI. Returns null if document is not loaded. /// diff --git a/src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs b/src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs index a88533762..852ea1a10 100644 --- a/src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs +++ b/src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs @@ -189,6 +189,19 @@ public void CloseDocument(Uri documentUri) { } } + public void RemoveDocument(Uri documentUri) { + DocumentEntry entry; + lock (_lock) { + if (_documentsByUri.TryGetValue(documentUri, out entry)) { + Debug.Assert(entry.LockCount == 0); + _documentsByUri.Remove(documentUri); + entry.Document.Dispose(); + } + } + Closed?.Invoke(this, new DocumentEventArgs(entry.Document)); + Removed?.Invoke(this, new DocumentEventArgs(entry.Document)); + } + public void Dispose() { lock (_lock) { foreach (var d in _documentsByUri.Values.OfType()) { diff --git a/src/LanguageServer/Impl/Implementation/Server.cs b/src/LanguageServer/Impl/Implementation/Server.cs index 980b9dbf2..5f62b07ca 100644 --- a/src/LanguageServer/Impl/Implementation/Server.cs +++ b/src/LanguageServer/Impl/Implementation/Server.cs @@ -288,7 +288,11 @@ private void RestartAnalysis() { var analyzer = Services.GetService(); analyzer.ResetAnalyzer(); foreach (var doc in _rdt.GetDocuments()) { - doc.Reset(null); + if (!doc.IsOpen) { + _rdt.RemoveDocument(doc.Uri); + } else { + doc.Reset(null); + } } } } From 70a149fe634ed89c6b12cb6dbd67fe5d41db8eb9 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Fri, 12 Jul 2019 14:40:45 -0700 Subject: [PATCH 15/17] fix build --- .../Ast/Impl/Modules/Resolution/MainModuleResolution.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs b/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs index 7f722df6a..2ebee0d95 100644 --- a/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs +++ b/src/Analysis/Ast/Impl/Modules/Resolution/MainModuleResolution.cs @@ -212,7 +212,7 @@ public async Task ReloadAsync(CancellationToken cancellationToken = default) { await ReloadSearchPaths(cancellationToken); addedRoots.UnionWith(PathResolver.SetInterpreterSearchPaths(InterpreterPaths)); - addedRoots.UnionWith(PathResolver.SetUserSearchPaths(userSearchPaths)); + addedRoots.UnionWith(PathResolver.SetUserSearchPaths(_userPaths)); ReloadModulePaths(addedRoots); } From 6b1fc9138c1e538bb4523ad5f26ebeb53c037ac8 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Fri, 12 Jul 2019 15:03:26 -0700 Subject: [PATCH 16/17] check parsed path instead of unparsed line --- .../Core/Impl/Interpreter/PythonLibraryPath.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs b/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs index 5a1b3e4a0..1f2c0a91d 100644 --- a/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs +++ b/src/Analysis/Core/Impl/Interpreter/PythonLibraryPath.cs @@ -186,11 +186,14 @@ public static async Task> GetSearchPathsFromInterpreterA try { var output = await ps.ExecuteAndCaptureOutputAsync(startInfo, cancellationToken); return output.Split(new[] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries).Select(s => { - if (PathUtils.PathStartsWith(s, tempWorkingDir)) { - return null; - } try { - return Parse(s); + var p = Parse(s); + + if (PathUtils.PathStartsWith(p.Path, tempWorkingDir)) { + return null; + } + + return p; } catch (ArgumentException) { Debug.Fail("Invalid search path: " + (s ?? "")); return null; From b4697c51edcabd94be214e4516baf20e2c71e038 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Mon, 15 Jul 2019 11:01:41 -0700 Subject: [PATCH 17/17] move RDT reloading into RDT --- .../Definitions/IRunningDocumentTable.cs | 12 ++++----- .../Impl/Documents/RunningDocumentTable.cs | 25 +++++++++++++------ .../Impl/Implementation/Server.cs | 8 +----- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/src/Analysis/Ast/Impl/Documents/Definitions/IRunningDocumentTable.cs b/src/Analysis/Ast/Impl/Documents/Definitions/IRunningDocumentTable.cs index b9f92e1bd..442327bd8 100644 --- a/src/Analysis/Ast/Impl/Documents/Definitions/IRunningDocumentTable.cs +++ b/src/Analysis/Ast/Impl/Documents/Definitions/IRunningDocumentTable.cs @@ -51,12 +51,6 @@ public interface IRunningDocumentTable { /// void CloseDocument(Uri uri); - /// - /// Forcibly removes the document from the table. This - /// shouldn't be called if the document is locked/has references. - /// - void RemoveDocument(Uri uri); - /// /// Fetches document by its URI. Returns null if document is not loaded. /// @@ -76,6 +70,12 @@ public interface IRunningDocumentTable { /// New lock count or -1 if document was not found. int UnlockDocument(Uri uri); + /// + /// Reloads the table by removing all unopened files (which would have been loaded from disk), + /// and resetting the content of all other files to trigger reanalysis. + /// + void ReloadAll(); + /// /// Fires when document is opened. /// diff --git a/src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs b/src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs index 852ea1a10..cb99f182d 100644 --- a/src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs +++ b/src/Analysis/Ast/Impl/Documents/RunningDocumentTable.cs @@ -21,6 +21,7 @@ using Microsoft.Python.Analysis.Analyzer; using Microsoft.Python.Analysis.Modules; using Microsoft.Python.Core; +using Microsoft.Python.Core.Collections; using Microsoft.Python.Core.Logging; namespace Microsoft.Python.Analysis.Documents { @@ -189,17 +190,27 @@ public void CloseDocument(Uri documentUri) { } } - public void RemoveDocument(Uri documentUri) { - DocumentEntry entry; + public void ReloadAll() { + ImmutableArray> opened; + ImmutableArray> closed; + lock (_lock) { - if (_documentsByUri.TryGetValue(documentUri, out entry)) { - Debug.Assert(entry.LockCount == 0); - _documentsByUri.Remove(documentUri); + _documentsByUri.Split(kvp => kvp.Value.Document.IsOpen, out opened, out closed); + + foreach (var (uri, entry) in closed) { + _documentsByUri.Remove(uri); entry.Document.Dispose(); } } - Closed?.Invoke(this, new DocumentEventArgs(entry.Document)); - Removed?.Invoke(this, new DocumentEventArgs(entry.Document)); + + foreach (var (_, entry) in closed) { + Closed?.Invoke(this, new DocumentEventArgs(entry.Document)); + Removed?.Invoke(this, new DocumentEventArgs(entry.Document)); + } + + foreach (var (_, entry) in opened) { + entry.Document.Reset(null); + } } public void Dispose() { diff --git a/src/LanguageServer/Impl/Implementation/Server.cs b/src/LanguageServer/Impl/Implementation/Server.cs index 5f62b07ca..799924384 100644 --- a/src/LanguageServer/Impl/Implementation/Server.cs +++ b/src/LanguageServer/Impl/Implementation/Server.cs @@ -287,13 +287,7 @@ public void NotifyPackagesChanged(CancellationToken cancellationToken = default) private void RestartAnalysis() { var analyzer = Services.GetService(); analyzer.ResetAnalyzer(); - foreach (var doc in _rdt.GetDocuments()) { - if (!doc.IsOpen) { - _rdt.RemoveDocument(doc.Uri); - } else { - doc.Reset(null); - } - } + _rdt.ReloadAll(); } } }