From a9fb4cec7f203a2e8b6bd737abe50164c5753d6d Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Thu, 5 Sep 2019 13:38:26 -0700 Subject: [PATCH 01/16] Update signing project to 3.0 --- src/Publish/SignLayout.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Publish/SignLayout.csproj b/src/Publish/SignLayout.csproj index b3aac5c65..37a0a6a4a 100644 --- a/src/Publish/SignLayout.csproj +++ b/src/Publish/SignLayout.csproj @@ -1,6 +1,6 @@ - netcoreapp2.2 + netcoreapp3.0 2008;$(NoWarn) From 90d57fa530e5e51c023674e7c264751391127331 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Thu, 5 Sep 2019 13:39:34 -0700 Subject: [PATCH 02/16] Upgrade to .NET 3.0 (#1521) * Upgrade to 3.0 * Get back to 2.0 --- src/Analysis/Ast/Test/FluentAssertions/AssertionsUtilities.cs | 1 + src/Analysis/Ast/Test/Microsoft.Python.Analysis.Tests.csproj | 2 +- src/Core/Test/Microsoft.Python.Core.Tests.csproj | 2 +- src/LanguageServer/Impl/Formatting/BlockFormatter.cs | 1 + src/LanguageServer/Impl/Formatting/LineFormatter.cs | 1 + src/LanguageServer/Impl/LanguageServer.cs | 2 +- src/LanguageServer/Impl/Microsoft.Python.LanguageServer.csproj | 2 +- src/LanguageServer/Impl/Protocol/Classes.cs | 2 +- src/LanguageServer/Impl/Protocol/Messages.cs | 1 + src/LanguageServer/Impl/Sources/HoverSource.cs | 2 +- .../Test/FluentAssertions/TextEditCollectionAssertions.cs | 1 + .../Test/Microsoft.Python.LanguageServer.Tests.csproj | 2 +- src/Parsing/Test/Microsoft.Python.Parsing.Tests.csproj | 2 +- src/UnitTests/Core/Impl/UnitTests.Core.csproj | 2 +- 14 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/Analysis/Ast/Test/FluentAssertions/AssertionsUtilities.cs b/src/Analysis/Ast/Test/FluentAssertions/AssertionsUtilities.cs index 9be29ed15..70ad06871 100644 --- a/src/Analysis/Ast/Test/FluentAssertions/AssertionsUtilities.cs +++ b/src/Analysis/Ast/Test/FluentAssertions/AssertionsUtilities.cs @@ -24,6 +24,7 @@ using Microsoft.Python.Core; using Microsoft.Python.Core.Text; using Microsoft.Python.Parsing; +using Range = Microsoft.Python.Core.Text.Range; namespace Microsoft.Python.Analysis.Tests.FluentAssertions { public static class AssertionsUtilities { diff --git a/src/Analysis/Ast/Test/Microsoft.Python.Analysis.Tests.csproj b/src/Analysis/Ast/Test/Microsoft.Python.Analysis.Tests.csproj index 4a7a9f7a7..2e5dfb031 100644 --- a/src/Analysis/Ast/Test/Microsoft.Python.Analysis.Tests.csproj +++ b/src/Analysis/Ast/Test/Microsoft.Python.Analysis.Tests.csproj @@ -1,6 +1,6 @@  - netcoreapp2.2 + netcoreapp3.0 Microsoft.Python.Analysis.Tests Microsoft.Python.Analysis.Tests diff --git a/src/Core/Test/Microsoft.Python.Core.Tests.csproj b/src/Core/Test/Microsoft.Python.Core.Tests.csproj index 1ca8be126..131762ab6 100644 --- a/src/Core/Test/Microsoft.Python.Core.Tests.csproj +++ b/src/Core/Test/Microsoft.Python.Core.Tests.csproj @@ -1,6 +1,6 @@  - netcoreapp2.2 + netcoreapp3.0 Microsoft.Python.Core.Tests Microsoft.Python.Core.Tests diff --git a/src/LanguageServer/Impl/Formatting/BlockFormatter.cs b/src/LanguageServer/Impl/Formatting/BlockFormatter.cs index 30e820c8a..adbd7cd52 100644 --- a/src/LanguageServer/Impl/Formatting/BlockFormatter.cs +++ b/src/LanguageServer/Impl/Formatting/BlockFormatter.cs @@ -24,6 +24,7 @@ using Microsoft.Python.Core.Diagnostics; using Microsoft.Python.Core.Text; using Microsoft.Python.LanguageServer.Protocol; +using Range = Microsoft.Python.Core.Text.Range; namespace Microsoft.Python.LanguageServer.Formatting { /// diff --git a/src/LanguageServer/Impl/Formatting/LineFormatter.cs b/src/LanguageServer/Impl/Formatting/LineFormatter.cs index 22720232b..88afbe1b7 100644 --- a/src/LanguageServer/Impl/Formatting/LineFormatter.cs +++ b/src/LanguageServer/Impl/Formatting/LineFormatter.cs @@ -24,6 +24,7 @@ using Microsoft.Python.Core.Text; using Microsoft.Python.LanguageServer.Protocol; using Microsoft.Python.Parsing; +using Range = Microsoft.Python.Core.Text.Range; namespace Microsoft.Python.LanguageServer.Formatting { /// diff --git a/src/LanguageServer/Impl/LanguageServer.cs b/src/LanguageServer/Impl/LanguageServer.cs index e522897de..5831b91fa 100644 --- a/src/LanguageServer/Impl/LanguageServer.cs +++ b/src/LanguageServer/Impl/LanguageServer.cs @@ -25,7 +25,6 @@ using Microsoft.Python.Core.Idle; using Microsoft.Python.Core.Logging; using Microsoft.Python.Core.Services; -using Microsoft.Python.Core.Text; using Microsoft.Python.Core.Threading; using Microsoft.Python.LanguageServer.Extensibility; using Microsoft.Python.LanguageServer.Protocol; @@ -33,6 +32,7 @@ using Newtonsoft.Json; using Newtonsoft.Json.Linq; using StreamJsonRpc; +using Range = Microsoft.Python.Core.Text.Range; namespace Microsoft.Python.LanguageServer.Implementation { /// diff --git a/src/LanguageServer/Impl/Microsoft.Python.LanguageServer.csproj b/src/LanguageServer/Impl/Microsoft.Python.LanguageServer.csproj index 58b65f302..c14e1229b 100644 --- a/src/LanguageServer/Impl/Microsoft.Python.LanguageServer.csproj +++ b/src/LanguageServer/Impl/Microsoft.Python.LanguageServer.csproj @@ -1,6 +1,6 @@  - netcoreapp2.2 + netcoreapp3.0 Microsoft.Python.LanguageServer Microsoft.Python.LanguageServer diff --git a/src/LanguageServer/Impl/Protocol/Classes.cs b/src/LanguageServer/Impl/Protocol/Classes.cs index 7f93794d9..a22bccf42 100644 --- a/src/LanguageServer/Impl/Protocol/Classes.cs +++ b/src/LanguageServer/Impl/Protocol/Classes.cs @@ -16,8 +16,8 @@ using System; using System.Collections.Generic; using System.Diagnostics; -using Microsoft.Python.Core.Text; using Newtonsoft.Json; +using Range = Microsoft.Python.Core.Text.Range; namespace Microsoft.Python.LanguageServer.Protocol { [Serializable] diff --git a/src/LanguageServer/Impl/Protocol/Messages.cs b/src/LanguageServer/Impl/Protocol/Messages.cs index 7c70d3686..2fb1305bb 100644 --- a/src/LanguageServer/Impl/Protocol/Messages.cs +++ b/src/LanguageServer/Impl/Protocol/Messages.cs @@ -17,6 +17,7 @@ using System.Runtime.InteropServices; using System.Threading.Tasks; using Microsoft.Python.Core.Text; +using Range = Microsoft.Python.Core.Text.Range; namespace Microsoft.Python.LanguageServer.Protocol { [Serializable] diff --git a/src/LanguageServer/Impl/Sources/HoverSource.cs b/src/LanguageServer/Impl/Sources/HoverSource.cs index 83cdf51fa..22ed0aeb5 100644 --- a/src/LanguageServer/Impl/Sources/HoverSource.cs +++ b/src/LanguageServer/Impl/Sources/HoverSource.cs @@ -13,7 +13,6 @@ // See the Apache Version 2.0 License for specific language governing // permissions and limitations under the License. -using System; using Microsoft.Python.Analysis; using Microsoft.Python.Analysis.Analyzer; using Microsoft.Python.Analysis.Analyzer.Expressions; @@ -25,6 +24,7 @@ using Microsoft.Python.LanguageServer.Completion; using Microsoft.Python.LanguageServer.Protocol; using Microsoft.Python.Parsing.Ast; +using Range = Microsoft.Python.Core.Text.Range; namespace Microsoft.Python.LanguageServer.Sources { internal sealed class HoverSource { diff --git a/src/LanguageServer/Test/FluentAssertions/TextEditCollectionAssertions.cs b/src/LanguageServer/Test/FluentAssertions/TextEditCollectionAssertions.cs index d3e8534ad..cd4a556db 100644 --- a/src/LanguageServer/Test/FluentAssertions/TextEditCollectionAssertions.cs +++ b/src/LanguageServer/Test/FluentAssertions/TextEditCollectionAssertions.cs @@ -23,6 +23,7 @@ using Microsoft.Python.Core.Text; using Microsoft.Python.LanguageServer.Protocol; using static Microsoft.Python.Analysis.Tests.FluentAssertions.AssertionsUtilities; +using Range = Microsoft.Python.Core.Text.Range; namespace Microsoft.Python.LanguageServer.Tests.FluentAssertions { [ExcludeFromCodeCoverage] diff --git a/src/LanguageServer/Test/Microsoft.Python.LanguageServer.Tests.csproj b/src/LanguageServer/Test/Microsoft.Python.LanguageServer.Tests.csproj index 3bf4d084d..85c33691c 100644 --- a/src/LanguageServer/Test/Microsoft.Python.LanguageServer.Tests.csproj +++ b/src/LanguageServer/Test/Microsoft.Python.LanguageServer.Tests.csproj @@ -1,6 +1,6 @@  - netcoreapp2.2 + netcoreapp3.0 Microsoft.Python.LanguageServer.Tests Microsoft.Python.LanguageServer.Tests diff --git a/src/Parsing/Test/Microsoft.Python.Parsing.Tests.csproj b/src/Parsing/Test/Microsoft.Python.Parsing.Tests.csproj index 8eeb7a1ed..23ff93b1d 100644 --- a/src/Parsing/Test/Microsoft.Python.Parsing.Tests.csproj +++ b/src/Parsing/Test/Microsoft.Python.Parsing.Tests.csproj @@ -1,6 +1,6 @@  - netcoreapp2.2 + netcoreapp3.0 Microsoft.Python.Parsing.Tests Microsoft.Python.Parsing.Tests diff --git a/src/UnitTests/Core/Impl/UnitTests.Core.csproj b/src/UnitTests/Core/Impl/UnitTests.Core.csproj index 3a1e90721..165ef15c4 100644 --- a/src/UnitTests/Core/Impl/UnitTests.Core.csproj +++ b/src/UnitTests/Core/Impl/UnitTests.Core.csproj @@ -1,6 +1,6 @@  - netstandard2.0 + netstandard2.1 Microsoft.Python.UnitTests.Core Microsoft.Python.UnitTests.Core From 00670b5da65a8c49a6014b1873d061fe0c22c81a Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 6 Sep 2019 10:34:21 -0700 Subject: [PATCH 03/16] Add global.json for .NET SDK --- src/Publish/global.json | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 src/Publish/global.json diff --git a/src/Publish/global.json b/src/Publish/global.json new file mode 100644 index 000000000..9070cb969 --- /dev/null +++ b/src/Publish/global.json @@ -0,0 +1,5 @@ +{ + "sdk": { + "version": "3.0.100-preview9-014004" + } +} \ No newline at end of file From e721d852b44994cf58c279102b06f0c18ce3e802 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Fri, 6 Sep 2019 13:31:21 -0700 Subject: [PATCH 04/16] Ensure FileInfo is not relative in DirectoryInfoProxy (#1519) * Ensure FileInfo is not relative in DirectoryInfoProxy * Add tests --- src/Core/Impl/IO/DirectoryInfoProxy.cs | 2 +- src/Core/Test/AssemblySetup.cs | 34 ++++++++++++++++ src/Core/Test/DirectoryInfoProxyTests.cs | 51 ++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 src/Core/Test/AssemblySetup.cs create mode 100644 src/Core/Test/DirectoryInfoProxyTests.cs diff --git a/src/Core/Impl/IO/DirectoryInfoProxy.cs b/src/Core/Impl/IO/DirectoryInfoProxy.cs index 2e694045f..8566f2267 100644 --- a/src/Core/Impl/IO/DirectoryInfoProxy.cs +++ b/src/Core/Impl/IO/DirectoryInfoProxy.cs @@ -55,7 +55,7 @@ public IEnumerable EnumerateFileSystemInfos(string[] includePat var matcher = GetMatcher(includePatterns, excludePatterns); PatternMatchingResult matchResult = SafeExecuteMatcher(matcher); return matchResult.Files.Select((filePatternMatch) => { - var path = PathUtils.NormalizePath(filePatternMatch.Path); + var path = PathUtils.NormalizePath(Path.Combine(_directoryInfo.FullName, filePatternMatch.Path)); return CreateFileSystemInfoProxy(new FileInfo(path)); }); } diff --git a/src/Core/Test/AssemblySetup.cs b/src/Core/Test/AssemblySetup.cs new file mode 100644 index 000000000..04083e46a --- /dev/null +++ b/src/Core/Test/AssemblySetup.cs @@ -0,0 +1,34 @@ +// 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 Microsoft.Python.Core.Testing; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using TestUtilities; + +namespace Microsoft.Python.Core.Tests { + [TestClass] + public sealed class AssemblySetup { + [AssemblyInitialize] + public static void Initialize(TestContext testContext) => AnalysisTestEnvironment.Initialize(); + + private class AnalysisTestEnvironment : TestEnvironmentImpl, ITestEnvironment { + public static void Initialize() { + var instance = new AnalysisTestEnvironment(); + Instance = instance; + TestEnvironment.Current = instance; + } + } + } +} diff --git a/src/Core/Test/DirectoryInfoProxyTests.cs b/src/Core/Test/DirectoryInfoProxyTests.cs new file mode 100644 index 000000000..a4a7efa8c --- /dev/null +++ b/src/Core/Test/DirectoryInfoProxyTests.cs @@ -0,0 +1,51 @@ +// 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.IO; +using System.Linq; +using System.Threading.Tasks; +using FluentAssertions; +using Microsoft.Python.Core.IO; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using TestUtilities; + +namespace Microsoft.Python.Core.Tests { + [TestClass] + public class DirectoryInfoProxyTests { + public TestContext TestContext { get; set; } + + [TestInitialize] + public void TestInitialize() + => TestEnvironmentImpl.TestInitialize($"{TestContext.FullyQualifiedTestClassName}.{TestContext.TestName}"); + + [TestCleanup] + public void Cleanup() => TestEnvironmentImpl.TestCleanup(); + + [TestMethod, Priority(0)] + public async Task EnumerateFileSystemInfos() { + var root = TestData.GetTestSpecificPath(); + await TestData.CreateTestSpecificFileAsync("a_y.py", "not important"); + await TestData.CreateTestSpecificFileAsync("b_y.py", "not important"); + await TestData.CreateTestSpecificFileAsync("c_z.py", "not important"); + + var proxy = new DirectoryInfoProxy(root); + var files = proxy.EnumerateFileSystemInfos(new[] { "*.py" }, new[] { "*z.py" }).OrderBy(x => x.FullName).ToArray(); + files.Should().HaveCount(2); + + files[0].FullName.Should().Be(Path.Combine(root, "a_y.py")); + files[1].FullName.Should().Be(Path.Combine(root, "b_y.py")); + } + } +} From ad88dfd032cfd45a3c1c3d9f68c5f0ddc286fd68 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 6 Sep 2019 14:27:57 -0700 Subject: [PATCH 05/16] Hadle import --- .../Impl/Sources/DefinitionSource.cs | 41 ++++++++++++++----- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/src/LanguageServer/Impl/Sources/DefinitionSource.cs b/src/LanguageServer/Impl/Sources/DefinitionSource.cs index 4a100f492..3d9a98469 100644 --- a/src/LanguageServer/Impl/Sources/DefinitionSource.cs +++ b/src/LanguageServer/Impl/Sources/DefinitionSource.cs @@ -15,6 +15,7 @@ using System; using System.Linq; +using System.Text; using Microsoft.Python.Analysis; using Microsoft.Python.Analysis.Analyzer; using Microsoft.Python.Analysis.Analyzer.Expressions; @@ -167,20 +168,40 @@ private Reference HandleImport(IDocumentAnalysis analysis, ImportStatement state return null; } - var index = statement.Names.IndexOf(x => x?.MakeString() == name); - if (index < 0) { - return null; - } + var moduleName = statement.Names.FirstOrDefault(x => x.IndexSpan.Start <= expr.StartIndex && x.IndexSpan.Start <= expr.EndIndex); + if (moduleName != null) { + var module = analysis.Document.Interpreter.ModuleResolution.GetImportedModule(moduleName.Names.First().Name); + foreach (var member in moduleName.Names.Skip(1)) { + if (module == null) { + return null; + } + + if(member.StartIndex >= expr.EndIndex) { + break; + } - var module = analysis.Document.Interpreter.ModuleResolution.GetImportedModule(name); - if (module != null) { - definingMember = module; - return new Reference { range = default, uri = CanNavigateToModule(module) ? module.Uri : null }; + if (member.StartIndex <= expr.EndIndex && member.EndIndex <= expr.EndIndex) { + if (module.Analysis.GlobalScope.Variables[member.Name] is ILocatedMember lm && lm.Location.Module is ILocationConverter lc) { + definingMember = lm; + return new Reference { + range = lm.Location.IndexSpan.ToSourceSpan(lc), + uri = CanNavigateToModule(module) ? module.Uri : null + }; + } + } + module = module.GetMember(member.Name) as IPythonModule; + } + + if (module != null) { + definingMember = module; + return new Reference { range = default, uri = CanNavigateToModule(module) ? module.Uri : null }; + } } // Import A as B - if (index >= 0 && index < statement.AsNames.Count) { - var value = analysis.ExpressionEvaluator.GetValueFromExpression(statement.AsNames[index]); + var asName = statement.AsNames.FirstOrDefault(n => n.IndexSpan.Start <= expr.StartIndex && n.IndexSpan.Start <= expr.EndIndex); + if (asName != null) { + var value = analysis.ExpressionEvaluator.GetValueFromExpression(asName); if (!value.IsUnknown()) { definingMember = value as ILocatedMember; return FromMember(definingMember); From 2bfa5c8d2d8e2a7fbacb075b362a4b670820ad48 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Fri, 6 Sep 2019 15:04:26 -0700 Subject: [PATCH 06/16] Update dependencies (#1526) --- .../FluentAssertions/AssertionsUtilities.cs | 18 +++++++++--------- .../Microsoft.Python.Analysis.Tests.csproj | 8 ++++---- .../Test/Microsoft.Python.Core.Tests.csproj | 6 +++--- .../Microsoft.Python.LanguageServer.csproj | 4 ++-- ...icrosoft.Python.LanguageServer.Tests.csproj | 8 ++++---- .../Test/Microsoft.Python.Parsing.Tests.csproj | 6 +++--- src/UnitTests/Core/Impl/UnitTests.Core.csproj | 4 ++-- 7 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/Analysis/Ast/Test/FluentAssertions/AssertionsUtilities.cs b/src/Analysis/Ast/Test/FluentAssertions/AssertionsUtilities.cs index 70ad06871..83ff1b3a1 100644 --- a/src/Analysis/Ast/Test/FluentAssertions/AssertionsUtilities.cs +++ b/src/Analysis/Ast/Test/FluentAssertions/AssertionsUtilities.cs @@ -188,38 +188,38 @@ public static string DoubleEscape(string input) => input.Replace("\r", "\\\u200Br").Replace("\n", "\\\u200Bn").Replace("\t", @"\t"); [CustomAssertion] - public static Continuation AssertIsNotNull(this AssertionScope assertionScope, T instance, string subjectName, string itemName, string accessorName) + public static Continuation AssertIsNotNull(this IAssertionScope IAssertionScope, T instance, string subjectName, string itemName, string accessorName) where T : class - => assertionScope.ForCondition(!(instance is null)) + => IAssertionScope.ForCondition(!(instance is null)) .FailWith($"Expected {subjectName} to have {itemName}{{reason}}, but {accessorName} is null."); [CustomAssertion] - public static Continuation AssertAtIndex(this AssertionScope assertionScope, IReadOnlyList collection, int index, string subjectName, string itemName) - where T : class => assertionScope.ForCondition(collection.Count > index) + public static Continuation AssertAtIndex(this IAssertionScope IAssertionScope, IReadOnlyList collection, int index, string subjectName, string itemName) + where T : class => IAssertionScope.ForCondition(collection.Count > index) .FailWith($"Expected {subjectName} to have {itemName} of type {typeof(T).Name} at index {index}{{reason}}, but collection has only {collection.Count} items.", subjectName, itemName) .Then .ForCondition(collection[index] is TItem) .FailWith($"Expected {subjectName} to have {itemName} of type `{typeof(T).Name}` at index {index}{{reason}}, but its type is `{collection[index].GetType().Name}`."); [CustomAssertion] - public static Continuation AssertHasMember(this AssertionScope assertionScope, IMember m, string memberName, string analysisValueName, string memberPrintName, out IMember member) { + public static Continuation AssertHasMember(this IAssertionScope IAssertionScope, IMember m, string memberName, string analysisValueName, string memberPrintName, out IMember member) { var t = m.GetPythonType(); t.Should().BeAssignableTo(); try { member = ((IMemberContainer)m).GetMember(memberName); } catch (Exception e) { member = null; - return assertionScope.FailWith($"Expected {analysisValueName} to have a {memberPrintName}{{reason}}, but {nameof(t.GetMember)} has failed with exception: {e}."); + return IAssertionScope.FailWith($"Expected {analysisValueName} to have a {memberPrintName}{{reason}}, but {nameof(t.GetMember)} has failed with exception: {e}."); } - return assertionScope.ForCondition(!(member is null)) + return IAssertionScope.ForCondition(!(member is null)) .FailWith($"Expected {analysisValueName} to have a {memberPrintName}{{reason}}."); } [CustomAssertion] - public static Continuation AssertHasMemberOfType(this AssertionScope assertionScope, IPythonType type, string memberName, string analysisValueName, string memberPrintName, out TMember typedMember) + public static Continuation AssertHasMemberOfType(this IAssertionScope IAssertionScope, IPythonType type, string memberName, string analysisValueName, string memberPrintName, out TMember typedMember) where TMember : class, IPythonType { - var continuation = assertionScope.AssertHasMember(type, memberName, analysisValueName, memberPrintName, out var member) + var continuation = IAssertionScope.AssertHasMember(type, memberName, analysisValueName, memberPrintName, out var member) .Then .ForCondition(member is TMember) .FailWith($"Expected {analysisValueName} to have a {memberPrintName} of type {typeof(TMember)}{{reason}}, but its type is {member.GetType()}."); diff --git a/src/Analysis/Ast/Test/Microsoft.Python.Analysis.Tests.csproj b/src/Analysis/Ast/Test/Microsoft.Python.Analysis.Tests.csproj index 2e5dfb031..70000c11c 100644 --- a/src/Analysis/Ast/Test/Microsoft.Python.Analysis.Tests.csproj +++ b/src/Analysis/Ast/Test/Microsoft.Python.Analysis.Tests.csproj @@ -23,15 +23,15 @@ - + - - + + all runtime; build; native; contentfiles; analyzers - + diff --git a/src/Core/Test/Microsoft.Python.Core.Tests.csproj b/src/Core/Test/Microsoft.Python.Core.Tests.csproj index 131762ab6..f475b7992 100644 --- a/src/Core/Test/Microsoft.Python.Core.Tests.csproj +++ b/src/Core/Test/Microsoft.Python.Core.Tests.csproj @@ -27,10 +27,10 @@ - + - - + + all runtime; build; native; contentfiles; analyzers diff --git a/src/LanguageServer/Impl/Microsoft.Python.LanguageServer.csproj b/src/LanguageServer/Impl/Microsoft.Python.LanguageServer.csproj index c14e1229b..1ee0e8a67 100644 --- a/src/LanguageServer/Impl/Microsoft.Python.LanguageServer.csproj +++ b/src/LanguageServer/Impl/Microsoft.Python.LanguageServer.csproj @@ -30,8 +30,8 @@ runtime; build; native; contentfiles; analyzers - - + + diff --git a/src/LanguageServer/Test/Microsoft.Python.LanguageServer.Tests.csproj b/src/LanguageServer/Test/Microsoft.Python.LanguageServer.Tests.csproj index 85c33691c..dd43c522b 100644 --- a/src/LanguageServer/Test/Microsoft.Python.LanguageServer.Tests.csproj +++ b/src/LanguageServer/Test/Microsoft.Python.LanguageServer.Tests.csproj @@ -23,15 +23,15 @@ - + - - + + all runtime; build; native; contentfiles; analyzers - + diff --git a/src/Parsing/Test/Microsoft.Python.Parsing.Tests.csproj b/src/Parsing/Test/Microsoft.Python.Parsing.Tests.csproj index 23ff93b1d..b0c838cfb 100644 --- a/src/Parsing/Test/Microsoft.Python.Parsing.Tests.csproj +++ b/src/Parsing/Test/Microsoft.Python.Parsing.Tests.csproj @@ -27,10 +27,10 @@ - + - - + + all runtime; build; native; contentfiles; analyzers diff --git a/src/UnitTests/Core/Impl/UnitTests.Core.csproj b/src/UnitTests/Core/Impl/UnitTests.Core.csproj index 165ef15c4..b4b821830 100644 --- a/src/UnitTests/Core/Impl/UnitTests.Core.csproj +++ b/src/UnitTests/Core/Impl/UnitTests.Core.csproj @@ -8,8 +8,8 @@ - - + + all runtime; build; native; contentfiles; analyzers From 4e891a63148ee0c0bddb9758ef6c4ab6219ea496 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 6 Sep 2019 15:19:38 -0700 Subject: [PATCH 07/16] Handle from import --- .../Impl/Sources/DefinitionSource.cs | 144 +++++++++++------- 1 file changed, 89 insertions(+), 55 deletions(-) diff --git a/src/LanguageServer/Impl/Sources/DefinitionSource.cs b/src/LanguageServer/Impl/Sources/DefinitionSource.cs index 3d9a98469..a0fa4d2f5 100644 --- a/src/LanguageServer/Impl/Sources/DefinitionSource.cs +++ b/src/LanguageServer/Impl/Sources/DefinitionSource.cs @@ -25,6 +25,7 @@ using Microsoft.Python.Analysis.Types; using Microsoft.Python.Analysis.Values; using Microsoft.Python.Core; +using Microsoft.Python.Core.Collections; using Microsoft.Python.Core.Text; using Microsoft.Python.LanguageServer.Completion; using Microsoft.Python.LanguageServer.Documents; @@ -114,49 +115,68 @@ public Reference FindDefinition(IDocumentAnalysis analysis, SourceLocation locat private Reference HandleFromImport(IDocumentAnalysis analysis, SourceLocation location, FromImportStatement statement, Node expr, out ILocatedMember definingMember) { definingMember = null; - // Are in the dotted name? + var mres = analysis.Document.Interpreter.ModuleResolution; + var imports = mres.CurrentPathResolver.FindImports(analysis.Document.FilePath, statement); + IPythonModule module = null; + switch (imports) { + case ModuleImport moduleImport: + module = mres.GetImportedModule(moduleImport.FullName); + break; + case ImplicitPackageImport packageImport: + module = mres.GetImportedModule(packageImport.FullName); + break; + } + + // Are we in the module name (i.e. A in 'from A import B')? var locationIndex = location.ToIndex(analysis.Ast); if (statement.Root.StartIndex <= locationIndex && locationIndex <= statement.Root.EndIndex) { - var mres = analysis.Document.Interpreter.ModuleResolution; - var imports = mres.CurrentPathResolver.FindImports(analysis.Document.FilePath, statement); - IPythonModule module = null; - switch (imports) { - case ModuleImport moduleImport: - module = mres.GetImportedModule(moduleImport.FullName); - break; - case ImplicitPackageImport packageImport: - module = mres.GetImportedModule(packageImport.FullName); - break; - } - definingMember = module; return module != null ? new Reference { range = default, uri = CanNavigateToModule(module) ? module.Uri : null } : null; } - // We are in what/as part - var nex = expr as NameExpression; - var name = nex?.Name; + if (module == null) { + return null; + } + + // Are we in the member name part (ie. B in 'from A import B')? + // Handle 'from A import B' similar to 'import A.B' + var partReference = FindModulePartReference(statement.Names, expr, module, out definingMember); + if (partReference != null) { + return partReference; + } + + // Are we in 'as' names? + var name = (expr as NameExpression)?.Name; if (string.IsNullOrEmpty(name)) { return null; } - // From X import A - var value = analysis.ExpressionEvaluator.GetValueFromExpression(nex); - if (value.IsUnknown()) { - // From X import A as B - var index = statement.Names.IndexOf(x => x?.Name == name); - if (index >= 0 && index < statement.AsNames.Count) { - value = analysis.ExpressionEvaluator.GetValueFromExpression(statement.AsNames[index]); + var asName = statement.AsNames.FirstOrDefault(x => x?.Name == name); + if (asName != null) { + var value = analysis.ExpressionEvaluator.GetValueFromExpression(asName); + if (!value.IsUnknown()) { + definingMember = value as ILocatedMember; + return FromMember(definingMember); } } - if (!value.IsUnknown()) { - definingMember = value as ILocatedMember; - return FromMember(definingMember); - } + return null; + } + private static Reference FindModulePartReference(ImmutableArray names, Node expr, IPythonModule module, out ILocatedMember definingMember) { + definingMember = null; + var part = names.FirstOrDefault(x => x.IndexSpan.Start <= expr.StartIndex && x.IndexSpan.Start <= expr.EndIndex); + if (part != null) { + if (module.Analysis.GlobalScope.Variables[part.Name] is ILocatedMember lm && lm.Location.Module is ILocationConverter lc) { + definingMember = lm; + return new Reference { + range = lm.Location.IndexSpan.ToSourceSpan(lc), + uri = CanNavigateToModule(module) ? module.Uri : null + }; + } + } return null; } @@ -168,34 +188,9 @@ private Reference HandleImport(IDocumentAnalysis analysis, ImportStatement state return null; } - var moduleName = statement.Names.FirstOrDefault(x => x.IndexSpan.Start <= expr.StartIndex && x.IndexSpan.Start <= expr.EndIndex); - if (moduleName != null) { - var module = analysis.Document.Interpreter.ModuleResolution.GetImportedModule(moduleName.Names.First().Name); - foreach (var member in moduleName.Names.Skip(1)) { - if (module == null) { - return null; - } - - if(member.StartIndex >= expr.EndIndex) { - break; - } - - if (member.StartIndex <= expr.EndIndex && member.EndIndex <= expr.EndIndex) { - if (module.Analysis.GlobalScope.Variables[member.Name] is ILocatedMember lm && lm.Location.Module is ILocationConverter lc) { - definingMember = lm; - return new Reference { - range = lm.Location.IndexSpan.ToSourceSpan(lc), - uri = CanNavigateToModule(module) ? module.Uri : null - }; - } - } - module = module.GetMember(member.Name) as IPythonModule; - } - - if (module != null) { - definingMember = module; - return new Reference { range = default, uri = CanNavigateToModule(module) ? module.Uri : null }; - } + var reference = FindModuleNamePartReference(analysis, statement.Names, expr, out definingMember); + if(reference != null) { + return reference; } // Import A as B @@ -210,6 +205,45 @@ private Reference HandleImport(IDocumentAnalysis analysis, ImportStatement state return null; } + /// + /// Given dotted name located reference to the part of the name. For example, given + /// 'os.path' and the name expression 'path' locates definition of 'path' part of 'os' module. + /// + private static Reference FindModuleNamePartReference(IDocumentAnalysis analysis, ImmutableArray dottedName, Node expr, out ILocatedMember definingMember) { + definingMember = null; + var moduleName = dottedName.FirstOrDefault(x => x.IndexSpan.Start <= expr.StartIndex && x.IndexSpan.Start <= expr.EndIndex); + if (moduleName == null) { + return null; + } + + var module = analysis.Document.Interpreter.ModuleResolution.GetImportedModule(moduleName.Names.First().Name); + foreach (var member in moduleName.Names.Skip(1)) { + if (module == null) { + return null; + } + + if (member.StartIndex >= expr.EndIndex) { + break; + } + + if (member.StartIndex <= expr.EndIndex && member.EndIndex <= expr.EndIndex) { + if (module.Analysis.GlobalScope.Variables[member.Name] is ILocatedMember lm && lm.Location.Module is ILocationConverter lc) { + definingMember = lm; + return new Reference { + range = lm.Location.IndexSpan.ToSourceSpan(lc), + uri = CanNavigateToModule(module) ? module.Uri : null + }; + } + } + module = module.GetMember(member.Name) as IPythonModule; + } + + if (module != null) { + definingMember = module; + return new Reference { range = default, uri = CanNavigateToModule(module) ? module.Uri : null }; + } + return null; + } private Reference TryFromVariable(string name, IDocumentAnalysis analysis, SourceLocation location, Node statement, out ILocatedMember definingMember) { definingMember = null; From 9d03891227d7e2776b64ac6e67fd8697d34760f6 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 6 Sep 2019 15:38:26 -0700 Subject: [PATCH 08/16] Add test --- .../Impl/Sources/DefinitionSource.cs | 12 ++--- .../Test/GoToDefinitionTests.cs | 52 +++++++++++++++++++ 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/src/LanguageServer/Impl/Sources/DefinitionSource.cs b/src/LanguageServer/Impl/Sources/DefinitionSource.cs index a0fa4d2f5..d22ccf7a8 100644 --- a/src/LanguageServer/Impl/Sources/DefinitionSource.cs +++ b/src/LanguageServer/Impl/Sources/DefinitionSource.cs @@ -169,10 +169,10 @@ private static Reference FindModulePartReference(ImmutableArray definingMember = null; var part = names.FirstOrDefault(x => x.IndexSpan.Start <= expr.StartIndex && x.IndexSpan.Start <= expr.EndIndex); if (part != null) { - if (module.Analysis.GlobalScope.Variables[part.Name] is ILocatedMember lm && lm.Location.Module is ILocationConverter lc) { - definingMember = lm; + var definition = module.Analysis.GlobalScope.Variables[part.Name]?.Definition; + if (definition != null) { return new Reference { - range = lm.Location.IndexSpan.ToSourceSpan(lc), + range = definition.Span, uri = CanNavigateToModule(module) ? module.Uri : null }; } @@ -227,10 +227,10 @@ private static Reference FindModuleNamePartReference(IDocumentAnalysis analysis, } if (member.StartIndex <= expr.EndIndex && member.EndIndex <= expr.EndIndex) { - if (module.Analysis.GlobalScope.Variables[member.Name] is ILocatedMember lm && lm.Location.Module is ILocationConverter lc) { - definingMember = lm; + var definition = module.Analysis.GlobalScope.Variables[member.Name]?.Definition; + if (definition != null) { return new Reference { - range = lm.Location.IndexSpan.ToSourceSpan(lc), + range = definition.Span, uri = CanNavigateToModule(module) ? module.Uri : null }; } diff --git a/src/LanguageServer/Test/GoToDefinitionTests.cs b/src/LanguageServer/Test/GoToDefinitionTests.cs index b7729c25e..173f4b48a 100644 --- a/src/LanguageServer/Test/GoToDefinitionTests.cs +++ b/src/LanguageServer/Test/GoToDefinitionTests.cs @@ -484,5 +484,57 @@ def f(a, b): reference.Should().NotBeNull(); reference.range.Should().Be(3, 0, 3, 5); } + + [TestMethod, Priority(0)] + public async Task ModulePartsNavigation() { + const string code = @" +import os.path +from os import path as os_path +print(os.path.basename('a/b/c')) +print(os_path.basename('a/b/c')) +"; + var analysis = await GetAnalysisAsync(code, PythonVersions.LatestAvailable3X); + var ds = new DefinitionSource(Services); + + var reference = ds.FindDefinition(analysis, new SourceLocation(2, 9), out _); + reference.Should().NotBeNull(); + reference.range.Should().Be(0, 0, 0, 0); + reference.uri.AbsolutePath.Should().Contain("os.py"); + + reference = ds.FindDefinition(analysis, new SourceLocation(2, 13), out _); + reference.Should().NotBeNull(); + var line = File.ReadAllLines(reference.uri.AbsolutePath)[reference.range.start.line]; + line.Should().EndWith("as path"); + line.Substring(reference.range.start.character).Should().Be("path"); + + reference = ds.FindDefinition(analysis, new SourceLocation(3, 7), out _); + reference.Should().NotBeNull(); + reference.range.Should().Be(0, 0, 0, 0); + reference.uri.AbsolutePath.Should().Contain("os.py"); + + reference = ds.FindDefinition(analysis, new SourceLocation(3, 17), out _); + reference.Should().NotBeNull(); + line = File.ReadAllLines(reference.uri.AbsolutePath)[reference.range.start.line]; + line.Should().EndWith("as path"); + line.Substring(reference.range.start.character).Should().Be("path"); + + reference = ds.FindDefinition(analysis, new SourceLocation(3, 27), out _); + reference.Should().NotBeNull(); + line = File.ReadAllLines(reference.uri.AbsolutePath)[reference.range.start.line]; + line.Should().EndWith("as path"); + line.Substring(reference.range.start.character).Should().Be("path"); + + reference = ds.FindDefinition(analysis, new SourceLocation(4, 12), out _); + reference.Should().NotBeNull(); + line = File.ReadAllLines(reference.uri.AbsolutePath)[reference.range.start.line]; + line.Should().EndWith("as path"); + line.Substring(reference.range.start.character).Should().Be("path"); + + reference = ds.FindDefinition(analysis, new SourceLocation(5, 12), out _); + reference.Should().NotBeNull(); + line = File.ReadAllLines(reference.uri.AbsolutePath)[reference.range.start.line]; + line.Should().EndWith("as path"); + line.Substring(reference.range.start.character).Should().Be("path"); + } } } From dc31786fb647e473d193ba3b6a6689945737b990 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Fri, 6 Sep 2019 16:02:22 -0700 Subject: [PATCH 09/16] Fix bad find and replace in AssertionsUtilities (#1527) --- .../FluentAssertions/AssertionsUtilities.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Analysis/Ast/Test/FluentAssertions/AssertionsUtilities.cs b/src/Analysis/Ast/Test/FluentAssertions/AssertionsUtilities.cs index 83ff1b3a1..7318a7c11 100644 --- a/src/Analysis/Ast/Test/FluentAssertions/AssertionsUtilities.cs +++ b/src/Analysis/Ast/Test/FluentAssertions/AssertionsUtilities.cs @@ -188,38 +188,38 @@ public static string DoubleEscape(string input) => input.Replace("\r", "\\\u200Br").Replace("\n", "\\\u200Bn").Replace("\t", @"\t"); [CustomAssertion] - public static Continuation AssertIsNotNull(this IAssertionScope IAssertionScope, T instance, string subjectName, string itemName, string accessorName) + public static Continuation AssertIsNotNull(this IAssertionScope assertionScope, T instance, string subjectName, string itemName, string accessorName) where T : class - => IAssertionScope.ForCondition(!(instance is null)) + => assertionScope.ForCondition(!(instance is null)) .FailWith($"Expected {subjectName} to have {itemName}{{reason}}, but {accessorName} is null."); [CustomAssertion] - public static Continuation AssertAtIndex(this IAssertionScope IAssertionScope, IReadOnlyList collection, int index, string subjectName, string itemName) - where T : class => IAssertionScope.ForCondition(collection.Count > index) + public static Continuation AssertAtIndex(this IAssertionScope assertionScope, IReadOnlyList collection, int index, string subjectName, string itemName) + where T : class => assertionScope.ForCondition(collection.Count > index) .FailWith($"Expected {subjectName} to have {itemName} of type {typeof(T).Name} at index {index}{{reason}}, but collection has only {collection.Count} items.", subjectName, itemName) .Then .ForCondition(collection[index] is TItem) .FailWith($"Expected {subjectName} to have {itemName} of type `{typeof(T).Name}` at index {index}{{reason}}, but its type is `{collection[index].GetType().Name}`."); [CustomAssertion] - public static Continuation AssertHasMember(this IAssertionScope IAssertionScope, IMember m, string memberName, string analysisValueName, string memberPrintName, out IMember member) { + public static Continuation AssertHasMember(this IAssertionScope assertionScope, IMember m, string memberName, string analysisValueName, string memberPrintName, out IMember member) { var t = m.GetPythonType(); t.Should().BeAssignableTo(); try { member = ((IMemberContainer)m).GetMember(memberName); } catch (Exception e) { member = null; - return IAssertionScope.FailWith($"Expected {analysisValueName} to have a {memberPrintName}{{reason}}, but {nameof(t.GetMember)} has failed with exception: {e}."); + return assertionScope.FailWith($"Expected {analysisValueName} to have a {memberPrintName}{{reason}}, but {nameof(t.GetMember)} has failed with exception: {e}."); } - return IAssertionScope.ForCondition(!(member is null)) + return assertionScope.ForCondition(!(member is null)) .FailWith($"Expected {analysisValueName} to have a {memberPrintName}{{reason}}."); } [CustomAssertion] - public static Continuation AssertHasMemberOfType(this IAssertionScope IAssertionScope, IPythonType type, string memberName, string analysisValueName, string memberPrintName, out TMember typedMember) + public static Continuation AssertHasMemberOfType(this IAssertionScope assertionScope, IPythonType type, string memberName, string analysisValueName, string memberPrintName, out TMember typedMember) where TMember : class, IPythonType { - var continuation = IAssertionScope.AssertHasMember(type, memberName, analysisValueName, memberPrintName, out var member) + var continuation = assertionScope.AssertHasMember(type, memberName, analysisValueName, memberPrintName, out var member) .Then .ForCondition(member is TMember) .FailWith($"Expected {analysisValueName} to have a {memberPrintName} of type {typeof(TMember)}{{reason}}, but its type is {member.GetType()}."); From 48888a65d449b2dde5bb8c9ef506ce896c2dd8d5 Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 6 Sep 2019 16:11:35 -0700 Subject: [PATCH 10/16] Merge issues --- src/UnitTests/Core/Impl/UnitTests.Core.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/UnitTests/Core/Impl/UnitTests.Core.csproj b/src/UnitTests/Core/Impl/UnitTests.Core.csproj index 9b5e111e0..b4b821830 100644 --- a/src/UnitTests/Core/Impl/UnitTests.Core.csproj +++ b/src/UnitTests/Core/Impl/UnitTests.Core.csproj @@ -8,7 +8,7 @@ - + all From 4d3d2e2f50ec1a3229cb7baf4157d2ee6816138d Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 6 Sep 2019 16:55:14 -0700 Subject: [PATCH 11/16] Add test (fails) --- .../Test/GoToDefinitionTests.cs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/LanguageServer/Test/GoToDefinitionTests.cs b/src/LanguageServer/Test/GoToDefinitionTests.cs index 173f4b48a..1190074ce 100644 --- a/src/LanguageServer/Test/GoToDefinitionTests.cs +++ b/src/LanguageServer/Test/GoToDefinitionTests.cs @@ -536,5 +536,27 @@ from os import path as os_path line.Should().EndWith("as path"); line.Substring(reference.range.start.character).Should().Be("path"); } + + [TestMethod, Priority(0)] + public async Task Unittest() { + const string code = @" +from unittest import TestCase + +class MyTestCase(TestCase): + def test_example(self): + with self.assertRaises(ZeroDivisionError): + value = 1 / 0 + self.assertNotEqual(value, 1) +"; + var analysis = await GetAnalysisAsync(code, PythonVersions.LatestAvailable3X); + var ds = new DefinitionSource(Services); + + var reference = ds.FindDefinition(analysis, new SourceLocation(6, 24), out _); + reference.Should().NotBeNull(); + reference.range.start.line.Should().BeGreaterThan(0); + reference.uri.AbsolutePath.Should().Contain("unittest.py"); + reference.uri.AbsolutePath.Should().NotContain("pyi"); + } + } } From 44afe3a131dd9f760b60dc66de57aa80c742c30f Mon Sep 17 00:00:00 2001 From: MikhailArkhipov Date: Fri, 6 Sep 2019 16:56:08 -0700 Subject: [PATCH 12/16] using --- src/LanguageServer/Impl/Sources/DefinitionSource.cs | 2 -- src/LanguageServer/Test/GoToDefinitionTests.cs | 2 -- 2 files changed, 4 deletions(-) diff --git a/src/LanguageServer/Impl/Sources/DefinitionSource.cs b/src/LanguageServer/Impl/Sources/DefinitionSource.cs index d22ccf7a8..56f1e8a1a 100644 --- a/src/LanguageServer/Impl/Sources/DefinitionSource.cs +++ b/src/LanguageServer/Impl/Sources/DefinitionSource.cs @@ -15,7 +15,6 @@ using System; using System.Linq; -using System.Text; using Microsoft.Python.Analysis; using Microsoft.Python.Analysis.Analyzer; using Microsoft.Python.Analysis.Analyzer.Expressions; @@ -28,7 +27,6 @@ using Microsoft.Python.Core.Collections; using Microsoft.Python.Core.Text; using Microsoft.Python.LanguageServer.Completion; -using Microsoft.Python.LanguageServer.Documents; using Microsoft.Python.LanguageServer.Protocol; using Microsoft.Python.Parsing.Ast; diff --git a/src/LanguageServer/Test/GoToDefinitionTests.cs b/src/LanguageServer/Test/GoToDefinitionTests.cs index 1190074ce..1b898d7ca 100644 --- a/src/LanguageServer/Test/GoToDefinitionTests.cs +++ b/src/LanguageServer/Test/GoToDefinitionTests.cs @@ -13,7 +13,6 @@ // See the Apache Version 2.0 License for specific language governing // permissions and limitations under the License. -using System; using System.IO; using System.Threading; using System.Threading.Tasks; @@ -557,6 +556,5 @@ with self.assertRaises(ZeroDivisionError): reference.uri.AbsolutePath.Should().Contain("unittest.py"); reference.uri.AbsolutePath.Should().NotContain("pyi"); } - } } From d5cd4bd68cd8d518325131e22d18661cb2d3f7af Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Sat, 7 Sep 2019 18:36:03 -0700 Subject: [PATCH 13/16] Fix merge of classes --- .../Ast/Impl/Analyzer/ModuleWalker.cs | 67 ++++++++++--------- .../Test/GoToDefinitionTests.cs | 2 +- 2 files changed, 37 insertions(+), 32 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs b/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs index fc6ead2de..75909321f 100644 --- a/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs +++ b/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs @@ -226,10 +226,10 @@ public void Complete() { /// Merges data from stub with the data from the module. /// /// - /// Functions are taken from the stub by the function walker since - /// information on the return type is needed during the analysis walk. - /// However, if the module is compiled (scraped), it often lacks some - /// of the definitions. Stub may contains those so we need to merge it in. + /// Types are taken from the stub while location and documentation comes from + /// source so code navigation takes user to the source and not to the stub. + /// Stub data, such as class methods are augmented by methods from source + /// since stub is not guaranteed to be complete. /// private void MergeStub() { _cancellationToken.ThrowIfCancellationRequested(); @@ -245,7 +245,7 @@ private void MergeStub() { // https://github.com/microsoft/python-language-server/issues/907 // Debug.Assert(!(_stubAnalysis is EmptyAnalysis)); - // Note that scrape can pick up more functions than the stub contains + // Scraping process can pick up more functions than the stub contains // Or the stub can have definitions that scraping had missed. Therefore // merge is the combination of the two with the documentation coming // from the library source of from the scraped module. @@ -254,12 +254,14 @@ private void MergeStub() { if (stubType.IsUnknown()) { continue; } + // If stub says 'Any' but we have better type, keep the current type. + if (stubType.DeclaringModule is TypingModule && stubType.Name == "Any") { + continue; + } var sourceVar = Eval.GlobalScope.Variables[v.Name]; var sourceType = sourceVar?.Value.GetPythonType(); - - // If stub says 'Any' but we have better type, keep the current type. - if (stubType.DeclaringModule is TypingModule && stubType.Name == "Any") { + if (sourceType.IsUnknown()) { continue; } @@ -286,15 +288,15 @@ private void TryReplaceMember(IVariable v, IPythonType sourceType, IPythonType s // Otherwise, replace type by one from the stub. switch (sourceType) { case null: - // Nothing in sources, but there is type in the stub. Declare it. + // Nothing in source, but there is type in the stub. Declare it. if (v.Source == VariableSource.Declaration || v.Source == VariableSource.Generic) { Eval.DeclareVariable(v.Name, v.Value, v.Source); } break; - case PythonClassType sourceClass when Module.Equals(sourceClass.DeclaringModule): + case PythonClassType sourceClass: // Transfer documentation first so we get class documentation - // that came from class definition win over one that may + // that comes from the class definition win over one that may // come from __init__ during the member merge below. TransferDocumentationAndLocation(sourceClass, stubType); @@ -306,28 +308,21 @@ private void TryReplaceMember(IVariable v, IPythonType sourceType, IPythonType s // Go through source class members and pick those that are // not present in the stub class. foreach (var name in sourceClass.GetMemberNames()) { - var sourceMember = sourceClass.GetMember(name); if (sourceMember.IsUnknown()) { - continue; // Anything is better than unknowns. + continue; // Do not add unknowns to the stub. } var sourceMemberType = sourceMember?.GetPythonType(); - - var stubMember = stubType.GetMember(name); - var stubMemberType = stubMember.GetPythonType(); - - // Don't augment types that do not come from this module. - if (sourceType.IsBuiltin || stubType.IsBuiltin) { - // If source type does not have an immediate member such as __init__() and - // rather have it inherited from object, we do not want to use the inherited - // since stub class may either have its own of inherits it from the object. - continue; + if (sourceMemberType is IPythonClassMember cm && cm.DeclaringType != sourceClass) { + continue; // Only take members from this class and not from bases. } - - if (stubMemberType?.MemberType == PythonMemberType.Method && stubMemberType?.DeclaringModule.ModuleType == ModuleType.Builtins) { - // Leave methods coming from object at the object and don't copy them into the derived class. + if (!IsAcceptableModule(sourceMemberType)) { + continue; // Member does not come from module or its submodules. } + var stubMember = stubType.GetMember(name); + var stubMemberType = stubMember.GetPythonType(); + // Get documentation from the current type, if any, since stubs // typically do not contain documentation while scraped code does. TransferDocumentationAndLocation(sourceMemberType, stubMemberType); @@ -354,9 +349,8 @@ private void TryReplaceMember(IVariable v, IPythonType sourceType, IPythonType s } // We do not re-declaring variables that are imported. if (v.Source == VariableSource.Declaration) { - // Re-declare variable with the data from the stub. TransferDocumentationAndLocation(sourceType, stubType); - // TODO: choose best type between the scrape and the stub. Stub probably should always win. + // Re-declare variable with the data from the stub. var source = Eval.CurrentScope.Variables[v.Name]?.Source ?? v.Source; Eval.DeclareVariable(v.Name, v.Value, source); } @@ -389,12 +383,16 @@ private void UpdateVariables() { } } - private static void TransferDocumentationAndLocation(IPythonType s, IPythonType d) { - if (s.IsUnknown() || s.IsBuiltin || d == null || d.IsBuiltin) { + private void TransferDocumentationAndLocation(IPythonType s, IPythonType d) { + if (s.IsUnknown() || s.DeclaringModule.ModuleType == ModuleType.Builtins || + d.IsUnknown() || d.DeclaringModule.ModuleType == ModuleType.Builtins) { return; // Do not transfer location of unknowns or builtins } - if (d.DeclaringModule != s.DeclaringModule.Stub) { + // Stub may be one for multiple modules - when module consists of several + // submodules, there is typically only one stub for the main module. + // Types from 'unittest.case' (library) are stubbed in 'unittest' stub. + if (!IsAcceptableModule(s)) { return; // Do not change unrelated types. } @@ -433,5 +431,12 @@ private static void TransferDocumentationAndLocation(IPythonType s, IPythonType } } } + + private bool IsAcceptableModule(IPythonType type) { + var thisModule = Eval.Module; + var typeModule = type.DeclaringModule; + var typeMainModuleName = typeModule.Name.Split('.').FirstOrDefault(); + return typeModule.Equals(thisModule) || typeMainModuleName == thisModule.Name; + } } } diff --git a/src/LanguageServer/Test/GoToDefinitionTests.cs b/src/LanguageServer/Test/GoToDefinitionTests.cs index 1b898d7ca..b4441ecb1 100644 --- a/src/LanguageServer/Test/GoToDefinitionTests.cs +++ b/src/LanguageServer/Test/GoToDefinitionTests.cs @@ -553,7 +553,7 @@ with self.assertRaises(ZeroDivisionError): var reference = ds.FindDefinition(analysis, new SourceLocation(6, 24), out _); reference.Should().NotBeNull(); reference.range.start.line.Should().BeGreaterThan(0); - reference.uri.AbsolutePath.Should().Contain("unittest.py"); + reference.uri.AbsolutePath.Should().Contain("case.py"); reference.uri.AbsolutePath.Should().NotContain("pyi"); } } From f8276c72dbaac1c05afa0c6f19dcdf390fdef88c Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Sun, 8 Sep 2019 11:31:24 -0700 Subject: [PATCH 14/16] Fix regression in bases Split merging into separate class --- .../Ast/Impl/Analyzer/ModuleWalker.cs | 229 +------------ src/Analysis/Ast/Impl/Analyzer/StubMerger.cs | 303 ++++++++++++++++++ .../Impl/Analyzer/Symbols/ClassEvaluator.cs | 18 +- .../Ast/Impl/Specializations/Specialized.cs | 2 +- .../Ast/Impl/Types/PythonPropertyType.cs | 6 +- src/Analysis/Ast/Test/LibraryTests.cs | 4 +- src/Caching/Impl/Models/PropertyModel.cs | 2 +- .../Test/GoToDefinitionTests.cs | 16 + 8 files changed, 342 insertions(+), 238 deletions(-) create mode 100644 src/Analysis/Ast/Impl/Analyzer/StubMerger.cs diff --git a/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs b/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs index 75909321f..35a3ea3c4 100644 --- a/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs +++ b/src/Analysis/Ast/Impl/Analyzer/ModuleWalker.cs @@ -13,20 +13,18 @@ // See the Apache Version 2.0 License for specific language governing // permissions and limitations under the License. -using System.Collections.Generic; -using System.Diagnostics; -using System.Linq; -using System.Threading; using Microsoft.Python.Analysis.Analyzer.Evaluation; using Microsoft.Python.Analysis.Documents; -using Microsoft.Python.Analysis.Modules; -using Microsoft.Python.Analysis.Specializations.Typing; using Microsoft.Python.Analysis.Types; using Microsoft.Python.Analysis.Types.Collections; using Microsoft.Python.Analysis.Values; using Microsoft.Python.Core; using Microsoft.Python.Core.Diagnostics; using Microsoft.Python.Parsing.Ast; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using System.Threading; namespace Microsoft.Python.Analysis.Analyzer { [DebuggerDisplay("{Module.Name} : {Module.ModuleType}")] @@ -205,7 +203,7 @@ public void Complete() { SymbolTable.EvaluateAll(); SymbolTable.ReplacedByStubs.Clear(); - MergeStub(); + new StubMerger(Eval).MergeStub(_stubAnalysis, _cancellationToken); if (_allIsUsable && _allReferencesCount >= 1 && GlobalScope.Variables.TryGetVariable(AllVariableName, out var variable) && variable?.Value is IPythonCollection collection && collection.IsExact) { @@ -221,222 +219,5 @@ public void Complete() { public GlobalScope GlobalScope => Eval.GlobalScope; public IReadOnlyList StarImportMemberNames { get; private set; } - - /// - /// Merges data from stub with the data from the module. - /// - /// - /// Types are taken from the stub while location and documentation comes from - /// source so code navigation takes user to the source and not to the stub. - /// Stub data, such as class methods are augmented by methods from source - /// since stub is not guaranteed to be complete. - /// - private void MergeStub() { - _cancellationToken.ThrowIfCancellationRequested(); - - if (Module.ModuleType == ModuleType.User || Module.ModuleType == ModuleType.Stub) { - return; - } - // No stub, no merge. - if (_stubAnalysis.IsEmpty()) { - return; - } - // TODO: figure out why module is getting analyzed before stub. - // https://github.com/microsoft/python-language-server/issues/907 - // Debug.Assert(!(_stubAnalysis is EmptyAnalysis)); - - // Scraping process can pick up more functions than the stub contains - // Or the stub can have definitions that scraping had missed. Therefore - // merge is the combination of the two with the documentation coming - // from the library source of from the scraped module. - foreach (var v in _stubAnalysis.GlobalScope.Variables) { - var stubType = v.Value.GetPythonType(); - if (stubType.IsUnknown()) { - continue; - } - // If stub says 'Any' but we have better type, keep the current type. - if (stubType.DeclaringModule is TypingModule && stubType.Name == "Any") { - continue; - } - - var sourceVar = Eval.GlobalScope.Variables[v.Name]; - var sourceType = sourceVar?.Value.GetPythonType(); - if (sourceType.IsUnknown()) { - continue; - } - - if (sourceVar?.Source == VariableSource.Import && - sourceVar.GetPythonType()?.DeclaringModule.Stub != null) { - // Keep imported types as they are defined in the library. For example, - // 'requests' imports NullHandler as 'from logging import NullHandler'. - // But 'requests' also declares NullHandler in its stub (but not in the main code) - // and that declaration does not have documentation or location. Therefore avoid - // taking types that are stub-only when similar type is imported from another - // module that also has a stub. - continue; - } - - TryReplaceMember(v, sourceType, stubType); - } - - UpdateVariables(); - } - - private void TryReplaceMember(IVariable v, IPythonType sourceType, IPythonType stubType) { - // If type does not exist in module, but exists in stub, declare it unless it is an import. - // If types are the classes, take class from the stub, then add missing members. - // Otherwise, replace type by one from the stub. - switch (sourceType) { - case null: - // Nothing in source, but there is type in the stub. Declare it. - if (v.Source == VariableSource.Declaration || v.Source == VariableSource.Generic) { - Eval.DeclareVariable(v.Name, v.Value, v.Source); - } - break; - - case PythonClassType sourceClass: - // Transfer documentation first so we get class documentation - // that comes from the class definition win over one that may - // come from __init__ during the member merge below. - TransferDocumentationAndLocation(sourceClass, stubType); - - // Replace the class entirely since stub members may use generic types - // and the class definition is important. We transfer missing members - // from the original class to the stub. - Eval.DeclareVariable(v.Name, v.Value, v.Source); - - // Go through source class members and pick those that are - // not present in the stub class. - foreach (var name in sourceClass.GetMemberNames()) { - var sourceMember = sourceClass.GetMember(name); - if (sourceMember.IsUnknown()) { - continue; // Do not add unknowns to the stub. - } - var sourceMemberType = sourceMember?.GetPythonType(); - if (sourceMemberType is IPythonClassMember cm && cm.DeclaringType != sourceClass) { - continue; // Only take members from this class and not from bases. - } - if (!IsAcceptableModule(sourceMemberType)) { - continue; // Member does not come from module or its submodules. - } - - var stubMember = stubType.GetMember(name); - var stubMemberType = stubMember.GetPythonType(); - - // Get documentation from the current type, if any, since stubs - // typically do not contain documentation while scraped code does. - TransferDocumentationAndLocation(sourceMemberType, stubMemberType); - - // If stub says 'Any' but we have better type, use member from the original class. - if (stubMember != null && !(stubType.DeclaringModule is TypingModule && stubType.Name == "Any")) { - continue; // Stub already have the member, don't replace. - } - - (stubType as PythonType)?.AddMember(name, stubMember, overwrite: true); - } - break; - - case IPythonModule _: - // We do not re-declare modules. - break; - - default: - var stubModule = stubType.DeclaringModule; - if (stubType is IPythonModule || stubModule.ModuleType == ModuleType.Builtins) { - // Modules members that are modules should remain as they are, i.e. os.path - // should remain library with its own stub attached. - break; - } - // We do not re-declaring variables that are imported. - if (v.Source == VariableSource.Declaration) { - TransferDocumentationAndLocation(sourceType, stubType); - // Re-declare variable with the data from the stub. - var source = Eval.CurrentScope.Variables[v.Name]?.Source ?? v.Source; - Eval.DeclareVariable(v.Name, v.Value, source); - } - - break; - } - } - - private void UpdateVariables() { - // Second pass: if we replaced any classes by new from the stub, we need to update - // variables that may still be holding old content. For example, ctypes - // declares 'c_voidp = c_void_p' so when we replace 'class c_void_p' - // by class from the stub, we need to go and update 'c_voidp' variable. - foreach (var v in GlobalScope.Variables) { - var variableType = v.Value.GetPythonType(); - if (!variableType.DeclaringModule.Equals(Module) && !variableType.DeclaringModule.Equals(Module.Stub)) { - continue; - } - // Check if type that the variable references actually declared here. - var typeInScope = GlobalScope.Variables[variableType.Name].GetPythonType(); - if (typeInScope == null || variableType == typeInScope) { - continue; - } - - if (v.Value == variableType) { - Eval.DeclareVariable(v.Name, typeInScope, v.Source); - } else if (v.Value is IPythonInstance) { - Eval.DeclareVariable(v.Name, new PythonInstance(typeInScope), v.Source); - } - } - } - - private void TransferDocumentationAndLocation(IPythonType s, IPythonType d) { - if (s.IsUnknown() || s.DeclaringModule.ModuleType == ModuleType.Builtins || - d.IsUnknown() || d.DeclaringModule.ModuleType == ModuleType.Builtins) { - return; // Do not transfer location of unknowns or builtins - } - - // Stub may be one for multiple modules - when module consists of several - // submodules, there is typically only one stub for the main module. - // Types from 'unittest.case' (library) are stubbed in 'unittest' stub. - if (!IsAcceptableModule(s)) { - return; // Do not change unrelated types. - } - - // Documentation and location are always get transferred from module type - // to the stub type and never the other way around. This makes sure that - // we show documentation from the original module and goto definition - // navigates to the module source and not to the stub. - if (s != d && s is PythonType src && d is PythonType dst) { - // If type is a class, then doc can either come from class definition node of from __init__. - // If class has doc from the class definition, don't stomp on it. - if (src is PythonClassType srcClass && dst is PythonClassType dstClass) { - // Higher lever source wins - if (srcClass.DocumentationSource == PythonClassType.ClassDocumentationSource.Class || - (srcClass.DocumentationSource == PythonClassType.ClassDocumentationSource.Init && dstClass.DocumentationSource == PythonClassType.ClassDocumentationSource.Base)) { - dstClass.SetDocumentation(srcClass.Documentation); - } - } else { - // Sometimes destination (stub type) already has documentation. This happens when stub type - // is used to augment more than one type. For example, in threading module RLock stub class - // replaces both RLock function and _RLock class making 'factory' function RLock to look - // like a class constructor. Effectively a single stub type is used for more than one type - // in the source and two source types may have different documentation. Thus transferring doc - // from one source type affects documentation of another type. It may be better to clone stub - // type and separate instances for separate source type, but for now we'll just avoid stomping - // on the existing documentation. - if (string.IsNullOrEmpty(dst.Documentation)) { - var srcDocumentation = src.Documentation; - if (!string.IsNullOrEmpty(srcDocumentation)) { - dst.SetDocumentation(srcDocumentation); - } - } - } - - if (src.Location.IsValid) { - dst.Location = src.Location; - } - } - } - - private bool IsAcceptableModule(IPythonType type) { - var thisModule = Eval.Module; - var typeModule = type.DeclaringModule; - var typeMainModuleName = typeModule.Name.Split('.').FirstOrDefault(); - return typeModule.Equals(thisModule) || typeMainModuleName == thisModule.Name; - } } } diff --git a/src/Analysis/Ast/Impl/Analyzer/StubMerger.cs b/src/Analysis/Ast/Impl/Analyzer/StubMerger.cs new file mode 100644 index 000000000..98d1b3a43 --- /dev/null +++ b/src/Analysis/Ast/Impl/Analyzer/StubMerger.cs @@ -0,0 +1,303 @@ +// 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 Microsoft.Python.Analysis.Analyzer.Evaluation; +using Microsoft.Python.Analysis.Modules; +using Microsoft.Python.Analysis.Specializations.Typing; +using Microsoft.Python.Analysis.Types; +using Microsoft.Python.Analysis.Values; +using System.Linq; +using System.Threading; + +namespace Microsoft.Python.Analysis.Analyzer { + internal sealed class StubMerger { + private readonly ExpressionEval _eval; + + public StubMerger(ExpressionEval eval) { + _eval = eval; + } + + /// + /// Merges data from stub with the data from the module. + /// + /// + /// Types are taken from the stub while location and documentation comes from + /// source so code navigation takes user to the source and not to the stub. + /// Stub data, such as class methods are augmented by methods from source + /// since stub is not guaranteed to be complete. + /// + public void MergeStub(IDocumentAnalysis stubAnalysis, CancellationToken cancellationToken) { + cancellationToken.ThrowIfCancellationRequested(); + if (_eval.Module.ModuleType == ModuleType.User || _eval.Module.ModuleType == ModuleType.Stub) { + return; + } + // No stub, no merge. + if (stubAnalysis.IsEmpty()) { + return; + } + // TODO: figure out why module is getting analyzed before stub. + // https://github.com/microsoft/python-language-server/issues/907 + // Debug.Assert(!(_stubAnalysis is EmptyAnalysis)); + + // Stub is the primary information source. Take types from the stub + // and replace source types by stub types. Transfer location and documentation + // from source members to the stub types. + TransferTypesFromStub(stubAnalysis, cancellationToken); + + UpdateVariables(); + } + + /// + /// Transfers types from stub to source while preserving documentation and location. + /// + /// + /// Stub is the primary information source. Take types from the stub + /// and replace source types by stub types. Transfer location and documentation + /// from source members to the stub types. + /// + private void TransferTypesFromStub(IDocumentAnalysis stubAnalysis, CancellationToken cancellationToken) { + foreach (var v in stubAnalysis.GlobalScope.Variables) { + cancellationToken.ThrowIfCancellationRequested(); + + var stubType = v.Value.GetPythonType(); + if (stubType.IsUnknown()) { + continue; + } + // If stub says 'Any' but we have better type, keep the current type. + if (stubType.DeclaringModule is TypingModule && stubType.Name == "Any") { + continue; + } + + var sourceVar = _eval.GlobalScope.Variables[v.Name]; + var sourceType = sourceVar?.Value.GetPythonType(); + if (sourceType.IsUnknown()) { + continue; + } + + if (sourceVar?.Source == VariableSource.Import && + sourceVar.GetPythonType()?.DeclaringModule.Stub != null) { + // Keep imported types as they are defined in the library. For example, + // 'requests' imports NullHandler as 'from logging import NullHandler'. + // But 'requests' also declares NullHandler in its stub (but not in the main code) + // and that declaration does not have documentation or location. Therefore avoid + // taking types that are stub-only when similar type is imported from another + // module that also has a stub. + continue; + } + + TryReplaceMember(v, sourceType, stubType, cancellationToken); + } + } + + private void TryReplaceMember(IVariable v, IPythonType sourceType, IPythonType stubType, CancellationToken cancellationToken) { + // If type does not exist in module, but exists in stub, declare it unless it is an import. + // If types are the classes, take class from the stub, then add missing members. + // Otherwise, replace type by one from the stub. + switch (sourceType) { + case null: + // Nothing in source, but there is type in the stub. Declare it. + if (v.Source == VariableSource.Declaration || v.Source == VariableSource.Generic) { + _eval.DeclareVariable(v.Name, v.Value, v.Source); + } + break; + + case PythonClassType sourceClass: + MergeClass(v, sourceClass, stubType, cancellationToken); + break; + + case IPythonModule _: + // We do not re-declare modules. + break; + + default: + var stubModule = stubType.DeclaringModule; + if (stubType is IPythonModule || stubModule.ModuleType == ModuleType.Builtins) { + // Modules members that are modules should remain as they are, i.e. os.path + // should remain library with its own stub attached. + break; + } + // We do not re-declaring variables that are imported. + if (v.Source == VariableSource.Declaration) { + TransferDocumentationAndLocation(sourceType, stubType); + // Re-declare variable with the data from the stub. + var source = _eval.CurrentScope.Variables[v.Name]?.Source ?? v.Source; + _eval.DeclareVariable(v.Name, v.Value, source); + } + + break; + } + } + + private void MergeClass(IVariable v, IPythonClassType sourceClass, IPythonType stubType, CancellationToken cancellationToken) { + // Transfer documentation first so we get class documentation + // that comes from the class definition win over one that may + // come from __init__ during the member merge below. + TransferDocumentationAndLocation(sourceClass, stubType); + + // Replace the class entirely since stub members may use generic types + // and the class definition is important. We transfer missing members + // from the original class to the stub. + _eval.DeclareVariable(v.Name, v.Value, v.Source); + + // First pass: go through source class members and pick those + // that are not present in the stub class. + foreach (var name in sourceClass.GetMemberNames()) { + cancellationToken.ThrowIfCancellationRequested(); + + var sourceMember = sourceClass.GetMember(name); + if (sourceMember.IsUnknown()) { + continue; // Do not add unknowns to the stub. + } + var sourceMemberType = sourceMember?.GetPythonType(); + if (sourceMemberType is IPythonClassMember cm && cm.DeclaringType != sourceClass) { + continue; // Only take members from this class and not from bases. + } + if (!IsAcceptableModule(sourceMemberType)) { + continue; // Member does not come from module or its submodules. + } + + var stubMember = stubType.GetMember(name); + var stubMemberType = stubMember.GetPythonType(); + + // Get documentation from the current type, if any, since stubs + // typically do not contain documentation while scraped code does. + TransferDocumentationAndLocation(sourceMemberType, stubMemberType); + + // If stub says 'Any' but we have better type, use member from the original class. + if (stubMember != null && !(stubType.DeclaringModule is TypingModule && stubType.Name == "Any")) { + continue; // Stub already have the member, don't replace. + } + + (stubType as PythonType)?.AddMember(name, stubMember, overwrite: true); + } + + // Second pass: go through stub class members and if they don't have documentation + // or location, check if source class has same member and fetch it from there. + // The reason is that in the stub sometimes members are specified all in one + // class while in source they may come from bases. Example: datetime. + foreach (var name in stubType.GetMemberNames()) { + cancellationToken.ThrowIfCancellationRequested(); + + var stubMember = stubType.GetMember(name); + if (stubMember.IsUnknown()) { + continue; + } + var stubMemberType = stubMember.GetPythonType(); + if (stubMemberType is IPythonClassMember cm && cm.DeclaringType != stubType) { + continue; // Only take members from this class and not from bases. + } + + var sourceMember = sourceClass.GetMember(name); + if (sourceMember.IsUnknown()) { + continue; + } + + var sourceMemberType = sourceMember.GetPythonType(); + if (sourceMemberType.Location.IsValid && !stubMemberType.Location.IsValid) { + TransferDocumentationAndLocation(sourceMemberType, stubMemberType); + } + } + } + + private void UpdateVariables() { + // Second pass: if we replaced any classes by new from the stub, we need to update + // variables that may still be holding old content. For example, ctypes + // declares 'c_voidp = c_void_p' so when we replace 'class c_void_p' + // by class from the stub, we need to go and update 'c_voidp' variable. + foreach (var v in _eval.GlobalScope.Variables) { + var variableType = v.Value.GetPythonType(); + if (!IsAcceptableModule(variableType)) { + continue; + } + // Check if type that the variable references actually declared here. + var typeInScope = _eval.GlobalScope.Variables[variableType.Name].GetPythonType(); + if (typeInScope == null || variableType == typeInScope) { + continue; + } + + if (v.Value == variableType) { + _eval.DeclareVariable(v.Name, typeInScope, v.Source); + } else if (v.Value is IPythonInstance) { + _eval.DeclareVariable(v.Name, new PythonInstance(typeInScope), v.Source); + } + } + } + + private void TransferDocumentationAndLocation(IPythonType s, IPythonType d) { + if (s.IsUnknown() || s.DeclaringModule.ModuleType == ModuleType.Builtins || + d.IsUnknown() || d.DeclaringModule.ModuleType == ModuleType.Builtins) { + return; // Do not transfer location of unknowns or builtins + } + + // Stub may be one for multiple modules - when module consists of several + // submodules, there is typically only one stub for the main module. + // Types from 'unittest.case' (library) are stubbed in 'unittest' stub. + if (!IsAcceptableModule(s)) { + return; // Do not change unrelated types. + } + + // Documentation and location are always get transferred from module type + // to the stub type and never the other way around. This makes sure that + // we show documentation from the original module and goto definition + // navigates to the module source and not to the stub. + if (s != d && s is PythonType src && d is PythonType dst) { + // If type is a class, then doc can either come from class definition node of from __init__. + // If class has doc from the class definition, don't stomp on it. + if (src is PythonClassType srcClass && dst is PythonClassType dstClass) { + // Higher lever source wins + if (srcClass.DocumentationSource == PythonClassType.ClassDocumentationSource.Class || + (srcClass.DocumentationSource == PythonClassType.ClassDocumentationSource.Init && dstClass.DocumentationSource == PythonClassType.ClassDocumentationSource.Base)) { + dstClass.SetDocumentation(srcClass.Documentation); + } + } else { + // Sometimes destination (stub type) already has documentation. This happens when stub type + // is used to augment more than one type. For example, in threading module RLock stub class + // replaces both RLock function and _RLock class making 'factory' function RLock to look + // like a class constructor. Effectively a single stub type is used for more than one type + // in the source and two source types may have different documentation. Thus transferring doc + // from one source type affects documentation of another type. It may be better to clone stub + // type and separate instances for separate source type, but for now we'll just avoid stomping + // on the existing documentation. + if (string.IsNullOrEmpty(dst.Documentation)) { + var srcDocumentation = src.Documentation; + if (!string.IsNullOrEmpty(srcDocumentation)) { + dst.SetDocumentation(srcDocumentation); + } + } + } + + if (src.Location.IsValid) { + dst.Location = src.Location; + } + } + } + + /// + /// Determines if type comes from module that is part of this package. + /// + /// + /// Single stub file typically applies to all modules while types within + /// the package come from multiple modules. We need to determine if stub + /// does match the type module so we don't accidentally modify documentation + /// or location of unrelated types such as coming from the base object type. + /// + private bool IsAcceptableModule(IPythonType type) { + var thisModule = _eval.Module; + var typeModule = type.DeclaringModule; + var typeMainModuleName = typeModule.Name.Split('.').FirstOrDefault(); + return typeModule.Equals(thisModule) || typeMainModuleName == thisModule.Name; + } + } +} diff --git a/src/Analysis/Ast/Impl/Analyzer/Symbols/ClassEvaluator.cs b/src/Analysis/Ast/Impl/Analyzer/Symbols/ClassEvaluator.cs index c28d465b1..c563428c5 100644 --- a/src/Analysis/Ast/Impl/Analyzer/Symbols/ClassEvaluator.cs +++ b/src/Analysis/Ast/Impl/Analyzer/Symbols/ClassEvaluator.cs @@ -109,16 +109,18 @@ private void ProcessClassBody() { } private IEnumerable ProcessBases() { - var bases = new List(); - foreach (var a in _classDef.Bases.Where(a => string.IsNullOrEmpty(a.Name))) { - if (IsValidBase(a)) { - TryAddBase(bases, a); - } else { - ReportInvalidBase(a); + // Base types must be evaluated in outer scope + using (Eval.OpenScope(Eval.CurrentScope.OuterScope)) { + var bases = new List(); + foreach (var a in _classDef.Bases.Where(a => string.IsNullOrEmpty(a.Name))) { + if (IsValidBase(a)) { + TryAddBase(bases, a); + } else { + ReportInvalidBase(a); + } } + return bases; } - - return bases; } private bool IsValidBase(Arg a) { diff --git a/src/Analysis/Ast/Impl/Specializations/Specialized.cs b/src/Analysis/Ast/Impl/Specializations/Specialized.cs index 6ec0e9e57..cc58646b3 100644 --- a/src/Analysis/Ast/Impl/Specializations/Specialized.cs +++ b/src/Analysis/Ast/Impl/Specializations/Specialized.cs @@ -19,7 +19,7 @@ namespace Microsoft.Python.Analysis.Specializations { internal static class Specialized { public static IPythonPropertyType Property(string name, IPythonModule declaringModule, IPythonType declaringType, IMember returnValue) { var location = new Location(declaringModule); - var prop = new PythonPropertyType(name, location, declaringType, false); + var prop = new PythonPropertyType(name, location, string.Empty, declaringType, false); var o = new PythonFunctionOverload(prop, location); o.AddReturnValue(returnValue); prop.AddOverload(o); diff --git a/src/Analysis/Ast/Impl/Types/PythonPropertyType.cs b/src/Analysis/Ast/Impl/Types/PythonPropertyType.cs index fdf4b7a66..12297dace 100644 --- a/src/Analysis/Ast/Impl/Types/PythonPropertyType.cs +++ b/src/Analysis/Ast/Impl/Types/PythonPropertyType.cs @@ -22,12 +22,12 @@ internal sealed class PythonPropertyType : PythonType, IPythonPropertyType { private IPythonFunctionOverload _getter; public PythonPropertyType(FunctionDefinition fd, Location location, IPythonType declaringType, bool isAbstract) - : this(fd.Name, location, declaringType, isAbstract) { + : this(fd.Name, location, fd.GetDocumentation(), declaringType, isAbstract) { declaringType.DeclaringModule.AddAstNode(this, fd); } - public PythonPropertyType(string name, Location location, IPythonType declaringType, bool isAbstract) - : base(name, location, string.Empty, BuiltinTypeId.Property) { + public PythonPropertyType(string name, Location location, string documentation, IPythonType declaringType, bool isAbstract) + : base(name, location, documentation, BuiltinTypeId.Property) { DeclaringType = declaringType; IsAbstract = isAbstract; } diff --git a/src/Analysis/Ast/Test/LibraryTests.cs b/src/Analysis/Ast/Test/LibraryTests.cs index ddc791936..c68b33eef 100644 --- a/src/Analysis/Ast/Test/LibraryTests.cs +++ b/src/Analysis/Ast/Test/LibraryTests.cs @@ -54,8 +54,10 @@ public async Task Datetime() { module.Name.Should().Be("datetime"); var dt = module.Should().HaveMember("datetime").Which; + var day = dt.Should().HaveReadOnlyProperty("day").Which; + day.Documentation.Should().NotBeNullOrEmpty(); - dt.Should().HaveReadOnlyProperty("day").And.HaveMethod("now") + dt.Should().HaveMethod("now") .Which.Should().BeClassMethod().And.HaveSingleOverload() .Which.Should().HaveReturnType() .Which.Should().HaveMembers( diff --git a/src/Caching/Impl/Models/PropertyModel.cs b/src/Caching/Impl/Models/PropertyModel.cs index 296e50ef7..bea252b81 100644 --- a/src/Caching/Impl/Models/PropertyModel.cs +++ b/src/Caching/Impl/Models/PropertyModel.cs @@ -33,7 +33,7 @@ public PropertyModel(IPythonPropertyType prop) : base(prop) { } public override IMember Create(ModuleFactory mf, IPythonType declaringType, IGlobalScope gs) - => _property ?? (_property = new PythonPropertyType(Name, new Location(mf.Module, IndexSpan.ToSpan()), declaringType, (Attributes & FunctionAttributes.Abstract) != 0)); + => _property ?? (_property = new PythonPropertyType(Name, new Location(mf.Module, IndexSpan.ToSpan()), Documentation, declaringType, (Attributes & FunctionAttributes.Abstract) != 0)); public override void Populate(ModuleFactory mf, IPythonType declaringType, IGlobalScope gs) { _property.SetDocumentation(Documentation); diff --git a/src/LanguageServer/Test/GoToDefinitionTests.cs b/src/LanguageServer/Test/GoToDefinitionTests.cs index b4441ecb1..082ea06c2 100644 --- a/src/LanguageServer/Test/GoToDefinitionTests.cs +++ b/src/LanguageServer/Test/GoToDefinitionTests.cs @@ -556,5 +556,21 @@ with self.assertRaises(ZeroDivisionError): reference.uri.AbsolutePath.Should().Contain("case.py"); reference.uri.AbsolutePath.Should().NotContain("pyi"); } + + [TestMethod, Priority(0)] + public async Task DateTimeProperty() { + const string code = @" +import datetime +x = datetime.datetime.day +"; + var analysis = await GetAnalysisAsync(code, PythonVersions.LatestAvailable3X); + var ds = new DefinitionSource(Services); + + var reference = ds.FindDefinition(analysis, new SourceLocation(3, 15), out _); + reference.Should().NotBeNull(); + reference.range.start.line.Should().BeGreaterThan(0); + reference.uri.AbsolutePath.Should().Contain("datetime.py"); + reference.uri.AbsolutePath.Should().NotContain("pyi"); + } } } From 45d107074c0d51a0b076828aa1d3ef0acab64a72 Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Mon, 9 Sep 2019 10:21:08 -0700 Subject: [PATCH 15/16] Fix null refs --- src/LanguageServer/Impl/Sources/ReferenceSource.cs | 2 +- src/LanguageServer/Test/CompletionTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/LanguageServer/Impl/Sources/ReferenceSource.cs b/src/LanguageServer/Impl/Sources/ReferenceSource.cs index 3f162a173..ca846f513 100644 --- a/src/LanguageServer/Impl/Sources/ReferenceSource.cs +++ b/src/LanguageServer/Impl/Sources/ReferenceSource.cs @@ -58,7 +58,7 @@ public async Task FindAllReferencesAsync(Uri uri, SourceLocation lo // If it is an implicitly declared variable, such as function or a class // then the location is invalid and the module is null. Use current module. - var declaringModule = rootDefinition.DeclaringModule ?? analysis.Document; + var declaringModule = rootDefinition?.DeclaringModule ?? analysis.Document; if (!string.IsNullOrEmpty(name) && (declaringModule.ModuleType == ModuleType.User || options == ReferenceSearchOptions.All)) { return await FindAllReferencesAsync(name, declaringModule, rootDefinition, location, definitionSource, cancellationToken); } diff --git a/src/LanguageServer/Test/CompletionTests.cs b/src/LanguageServer/Test/CompletionTests.cs index bb2cdd82a..625eba996 100644 --- a/src/LanguageServer/Test/CompletionTests.cs +++ b/src/LanguageServer/Test/CompletionTests.cs @@ -412,7 +412,7 @@ public async Task MarkupKindValid() { var cs = new CompletionSource(new PlainTextDocumentationSource(), ServerSettings.completion); var result = cs.GetCompletions(analysis, new SourceLocation(2, 5)); - result.Completions?.Select(i => i.documentation.kind) + result.Completions?.Select(i => i.documentation?.kind).ExcludeDefault() .Should().NotBeEmpty().And.BeSubsetOf(new[] { MarkupKind.PlainText, MarkupKind.Markdown }); } From 87c8a502c6b3e4df8a52a3a50083999ac11b318a Mon Sep 17 00:00:00 2001 From: Mikhail Arkhipov Date: Mon, 9 Sep 2019 14:43:55 -0700 Subject: [PATCH 16/16] Make common CanUpdateAnalysis check --- .../Impl/Analyzer/PythonAnalyzerSession.cs | 98 +++++++++---------- 1 file changed, 48 insertions(+), 50 deletions(-) diff --git a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzerSession.cs b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzerSession.cs index cba3d32f2..4032b0b03 100644 --- a/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzerSession.cs +++ b/src/Analysis/Ast/Impl/Analyzer/PythonAnalyzerSession.cs @@ -252,23 +252,9 @@ private Task StartAnalysis(IDependencyChainNode node, Async private void Analyze(IDependencyChainNode node, AsyncCountdownEvent ace, Stopwatch stopWatch) { try { var entry = node.Value; - - if (!entry.CanUpdateAnalysis(_walker.Version, out var module, out var ast, out var currentAnalysis)) { - if (IsAnalyzedLibraryInLoop(node, currentAnalysis)) { - return; - } else if (ast == default) { - if (currentAnalysis == default) { - // Entry doesn't have ast yet. There should be at least one more session. - Cancel(); - } else { - Debug.Fail($"Library module {module.Name} of type {module.ModuleType} has been analyzed already!"); - } - } - - _log?.Log(TraceEventType.Verbose, $"Analysis of {module.Name}({module.ModuleType}) canceled."); + if (!CanUpdateAnalysis(entry, node, _walker.Version, out var module, out var ast, out var currentAnalysis)) { return; } - var startTime = stopWatch.Elapsed; AnalyzeEntry(node, entry, module, ast, _walker.Version); @@ -284,12 +270,14 @@ private void Analyze(IDependencyChainNode node, AsyncCountd node.MoveNext(); bool isCanceled; + int remaining; lock (_syncObj) { isCanceled = _isCanceled; + remaining = _walker.Remaining; } if (!isCanceled) { - _progress.ReportRemaining(_walker.Remaining); + _progress.ReportRemaining(remaining); } Interlocked.Decrement(ref _runningTasks); @@ -300,22 +288,9 @@ private void Analyze(IDependencyChainNode node, AsyncCountd private void AnalyzeEntry() { var stopWatch = _log != null ? Stopwatch.StartNew() : null; try { - if (!_entry.CanUpdateAnalysis(Version, out var module, out var ast, out var currentAnalysis)) { - if (currentAnalysis is LibraryAnalysis) { - return; - } else if (ast == default) { - if (currentAnalysis == default) { - // Entry doesn't have ast yet. There should be at least one more session. - Cancel(); - } else { - Debug.Fail($"Library module {module.Name} of type {module.ModuleType} has been analyzed already!"); - } - } - - _log?.Log(TraceEventType.Verbose, $"Analysis of {module.Name}({module.ModuleType}) canceled."); + if (!CanUpdateAnalysis(_entry, null, Version, out var module, out var ast, out var currentAnalysis)) { return; } - var startTime = stopWatch?.Elapsed ?? TimeSpan.Zero; AnalyzeEntry(null, _entry, module, ast, Version); @@ -331,6 +306,32 @@ private void AnalyzeEntry() { } } + private bool CanUpdateAnalysis( + PythonAnalyzerEntry entry, + IDependencyChainNode node, + int version, + out IPythonModule module, + out PythonAst ast, + out IDocumentAnalysis currentAnalysis) { + + if (!entry.CanUpdateAnalysis(version, out module, out ast, out currentAnalysis)) { + if (IsAnalyzedLibraryInLoop(node, currentAnalysis)) { + return false; + } else if (ast == default) { + if (currentAnalysis == default) { + // Entry doesn't have ast yet. There should be at least one more session. + Cancel(); + } else { + Debug.Fail($"Library module {module.Name} of type {module.ModuleType} has been analyzed already!"); + } + } + + _log?.Log(TraceEventType.Verbose, $"Analysis of {module.Name}({module.ModuleType}) canceled."); + return false; + } + return true; + } + private void AnalyzeEntry(IDependencyChainNode node, PythonAnalyzerEntry entry, IPythonModule module, PythonAst ast, int version) { // Now run the analysis. var analyzable = module as IAnalyzable; @@ -368,26 +369,17 @@ private IDocumentAnalysis DoAnalyzeEntry(IDependencyChainNode node, IDocument document, PythonAst ast, int version, ModuleWalker walker, bool isCanceled) { + private IDocumentAnalysis CreateAnalysis(IDependencyChainNode node, IDocument document, PythonAst ast, int version, ModuleWalker walker) { var canHaveLibraryAnalysis = false; // Don't try to drop builtins; it causes issues elsewhere. @@ -400,16 +392,22 @@ private IDocumentAnalysis CreateAnalysis(IDependencyChainNode x is ImportStatement || x is FromImportStatement);