diff --git a/src/Analysis/Engine/Impl/Analyzer/DDG.cs b/src/Analysis/Engine/Impl/Analyzer/DDG.cs index a8a3bca6b..7b66102d8 100644 --- a/src/Analysis/Engine/Impl/Analyzer/DDG.cs +++ b/src/Analysis/Engine/Impl/Analyzer/DDG.cs @@ -296,6 +296,9 @@ public override bool Walk(FromImportStatement node) { case ImportNotFound notFound: MakeUnresolvedImport(notFound.FullName, node.Root); return false; + case NoKnownParentPackage _: + MakeNoKnownParentPackageImport(node.Root); + return false; default: return false; } @@ -464,6 +467,10 @@ private void MakeUnresolvedImport(string name, Node spanNode) { ProjectState.AddDiagnostic(spanNode, _unit, ErrorMessages.UnresolvedImport(name), DiagnosticSeverity.Warning, ErrorMessages.UnresolvedImportCode); } + private void MakeNoKnownParentPackageImport(Node spanNode) { + ProjectState.AddDiagnostic(spanNode, _unit, Resources.ErrorRelativeImportNoPackage, DiagnosticSeverity.Warning, ErrorMessages.UnresolvedImportCode); + } + private void ImportModule(in ImportStatement node, in IModule module, in DottedName moduleImportExpression, in NameExpression asNameExpression) { // "import fob.oar as baz" is handled as // baz = import_module('fob.oar') diff --git a/src/Analysis/Engine/Impl/DependencyResolution/NoKnownParentPackage.cs b/src/Analysis/Engine/Impl/DependencyResolution/NoKnownParentPackage.cs new file mode 100644 index 000000000..1d275bf49 --- /dev/null +++ b/src/Analysis/Engine/Impl/DependencyResolution/NoKnownParentPackage.cs @@ -0,0 +1,19 @@ +// Python Tools for Visual Studio +// 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. + +namespace Microsoft.PythonTools.Analysis.DependencyResolution { + internal class NoKnownParentPackage : IImportSearchResult { } +} diff --git a/src/Analysis/Engine/Impl/DependencyResolution/PathResolverSnapshot.cs b/src/Analysis/Engine/Impl/DependencyResolution/PathResolverSnapshot.cs index c1f69c132..b1e776d9d 100644 --- a/src/Analysis/Engine/Impl/DependencyResolution/PathResolverSnapshot.cs +++ b/src/Analysis/Engine/Impl/DependencyResolution/PathResolverSnapshot.cs @@ -186,16 +186,11 @@ public IImportSearchResult GetImportsFromRelativePath(in string modulePath, in i return default; } - if (parentCount > lastEdge.PathLength) { - // Can't get outside of the root - return default; - } - var fullNameList = relativePath.ToList(); if (lastEdge.IsNonRooted) { // Handle relative imports only for modules in the same folder if (parentCount > 1) { - return default; + return new NoKnownParentPackage(); } if (parentCount == 1 && fullNameList.Count == 1 && lastEdge.Start.TryGetChild(fullNameList[0], out var nameNode)) { @@ -208,12 +203,27 @@ public IImportSearchResult GetImportsFromRelativePath(in string modulePath, in i .ToString()); } + var relativeInWorkDirectory = false; + if (parentCount > lastEdge.PathLength - 2) { + relativeInWorkDirectory = _workDirectory.EqualsOrdinal(lastEdge.FirstEdge.End.Name, IgnoreCaseInPaths); + + // Relative path must be only inside package + // Exception for working directory cause it can be a root directory of the package + if (!relativeInWorkDirectory) { + return new NoKnownParentPackage(); + } + } + var relativeParentEdge = lastEdge.GetPrevious(parentCount); var rootEdges = new List(); - for (var i = 0; i < _roots.Count; i++) { - if (RootContains(i, relativeParentEdge, out var rootEdge)) { - rootEdges.Add(rootEdge); + if (relativeInWorkDirectory) { + rootEdges.Add(lastEdge.FirstEdge); + } else { + for (var i = 0; i < _roots.Count; i++) { + if (RootContains(i, relativeParentEdge, out var rootEdge)) { + rootEdges.Add(rootEdge); + } } } @@ -225,7 +235,13 @@ public IImportSearchResult GetImportsFromRelativePath(in string modulePath, in i return default; } - var fullName = GetFullModuleNameBuilder(relativeParentEdge).Append(".", fullNameList).ToString(); + var fullNameBuilder = GetFullModuleNameBuilder(relativeParentEdge); + if (!relativeParentEdge.IsFirst) { + AppendName(fullNameBuilder, relativeParentEdge.End.Name); + fullNameBuilder.Append("."); + } + var fullName = fullNameBuilder.Append(".", fullNameList).ToString(); + return new ImportNotFound(fullName); } @@ -249,7 +265,7 @@ private static bool TryCreateModuleImport(Edge lastEdge, out ModuleImport module => TryCreateModuleImport(lastEdge.FirstEdge.End, lastEdge.End, out moduleImport); private static bool TryCreateModuleImport(Node rootNode, Node moduleNode, out ModuleImport moduleImport) { - if (moduleNode.TryGetChild("__init__", out var initPyNode) && initPyNode.IsModule) { + if (IsPackageWithInitPy(moduleNode, out var initPyNode)) { moduleImport = new ModuleImport(moduleNode.Name, initPyNode.FullModuleName, rootNode.Name, initPyNode.ModulePath, false); return true; } @@ -516,7 +532,7 @@ private static bool TryFindImport(IEnumerable rootEdges, List full private static bool TryFindName(in Edge edge, in IEnumerable nameParts, out Edge lastEdge) { lastEdge = edge; foreach (var name in nameParts) { - if (lastEdge.End.IsModule) { + if (lastEdge.End.IsModule && !IsPackageWithInitPy(lastEdge.End, out _)) { return false; } var index = lastEdge.End.GetChildIndex(name); @@ -652,7 +668,7 @@ private static StringBuilder GetFullModuleNameBuilder(in Edge lastEdge) { while (edge != lastEdge) { AppendName(sb, edge.End.Name); edge = edge.Next; - }; + } return sb; } @@ -743,6 +759,9 @@ private static int GetModuleNameStart(string rootedModulePath) private static int GetModuleNameEnd(string rootedModulePath) => IsPythonCompiled(rootedModulePath) ? rootedModulePath.IndexOf('.', GetModuleNameStart(rootedModulePath)) : rootedModulePath.LastIndexOf('.'); + private static bool IsPackageWithInitPy(Node node, out Node initPyNode) + => node.TryGetChild("__init__", out initPyNode) && initPyNode.IsModule; + private static bool IsNotInitPy(string name) => !name.EqualsOrdinal("__init__"); diff --git a/src/Analysis/Engine/Impl/Infrastructure/Extensions/StringExtensions.cs b/src/Analysis/Engine/Impl/Infrastructure/Extensions/StringExtensions.cs index 6c0eceb39..bd3ca29a9 100644 --- a/src/Analysis/Engine/Impl/Infrastructure/Extensions/StringExtensions.cs +++ b/src/Analysis/Engine/Impl/Infrastructure/Extensions/StringExtensions.cs @@ -195,6 +195,9 @@ public static bool EqualsIgnoreCase(this string s, string other) public static bool EqualsOrdinal(this string s, string other) => string.Equals(s, other, StringComparison.Ordinal); + public static bool EqualsOrdinal(this string s, string other, bool ignoreCase) + => string.Equals(s, other, ignoreCase ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal); + 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; diff --git a/src/Analysis/Engine/Impl/Resources.Designer.cs b/src/Analysis/Engine/Impl/Resources.Designer.cs index f74238ffc..43c598b29 100644 --- a/src/Analysis/Engine/Impl/Resources.Designer.cs +++ b/src/Analysis/Engine/Impl/Resources.Designer.cs @@ -105,6 +105,15 @@ internal static string ErrorNotCallableEmpty { } } + /// + /// Looks up a localized string similar to attempted relative import with no known parent package. + /// + internal static string ErrorRelativeImportNoPackage { + get { + return ResourceManager.GetString("ErrorRelativeImportNoPackage", resourceCulture); + } + } + /// /// Looks up a localized string similar to unresolved import '{0}'. /// diff --git a/src/Analysis/Engine/Impl/Resources.resx b/src/Analysis/Engine/Impl/Resources.resx index da96f9470..ac7308b00 100644 --- a/src/Analysis/Engine/Impl/Resources.resx +++ b/src/Analysis/Engine/Impl/Resources.resx @@ -312,6 +312,9 @@ unresolved import '{0}' + + attempted relative import with no known parent package + '{0}' used before definition diff --git a/src/Analysis/Engine/Test/AddTestSpecificSearchPathAttribute.cs b/src/Analysis/Engine/Test/AddTestSpecificSearchPathAttribute.cs new file mode 100644 index 000000000..939052196 --- /dev/null +++ b/src/Analysis/Engine/Test/AddTestSpecificSearchPathAttribute.cs @@ -0,0 +1,28 @@ +// Python Tools for Visual Studio +// 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; + +namespace AnalysisTests { + [AttributeUsage(AttributeTargets.Method, AllowMultiple = true)] + public class AddTestSpecificSearchPathAttribute : Attribute { + public string RelativeSearchPath { get; } + + public AddTestSpecificSearchPathAttribute(string relativeSearchPath) { + RelativeSearchPath = relativeSearchPath; + } + } +} diff --git a/src/Analysis/Engine/Test/EventTaskSources.cs b/src/Analysis/Engine/Test/EventTaskSources.cs index d24ca8145..9b5f283d9 100644 --- a/src/Analysis/Engine/Test/EventTaskSources.cs +++ b/src/Analysis/Engine/Test/EventTaskSources.cs @@ -37,6 +37,11 @@ public static class Server { new EventTaskSource( (o, e) => o.OnParseComplete += e, (o, e) => o.OnParseComplete -= e); + + public static readonly EventTaskSource OnPublishDiagnostics = + new EventTaskSource( + (o, e) => o.OnPublishDiagnostics += e, + (o, e) => o.OnPublishDiagnostics -= e); } } } diff --git a/src/Analysis/Engine/Test/ImportTests.cs b/src/Analysis/Engine/Test/ImportTests.cs index 4f1ad0acd..84b9dfc0e 100644 --- a/src/Analysis/Engine/Test/ImportTests.cs +++ b/src/Analysis/Engine/Test/ImportTests.cs @@ -19,11 +19,15 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +using FluentAssertions; +using Microsoft.Python.LanguageServer; using Microsoft.Python.LanguageServer.Implementation; using Microsoft.PythonTools.Analysis; +using Microsoft.PythonTools.Analysis.Analyzer; using Microsoft.PythonTools.Analysis.FluentAssertions; using Microsoft.VisualStudio.TestTools.UnitTesting; using TestUtilities; +using Resources = Microsoft.PythonTools.Analysis.Resources; namespace AnalysisTests { [TestClass] @@ -251,6 +255,145 @@ import projectB.foo.baz completion.Should().HaveLabels("foo"); } + [ServerTestMethod(LatestAvailable3X = true, TestSpecificRootUri = true), Priority(0)] + [AddTestSpecificSearchPath("src")] + public async Task Diagnostics_InvalidRelativeImportInUserSearchPath(Server server) { + var modulePath = "src/module.py"; + var moduleCode = "TEST = 0"; + var appPath = "src/app.py"; + var appCode = "from .module import TEST"; + var args = new PublishDiagnosticsEventArgs(); + server.OnPublishDiagnostics += (o, e) => args = e; + + await server.OpenDocumentAndGetUriAsync(modulePath, moduleCode); + await server.OpenDocumentAndGetUriAsync(appPath, appCode); + await server.WaitForCompleteAnalysisAsync(CancellationToken.None); + + args.diagnostics.Should().ContainSingle() + .Which.message.Should().Be(Resources.ErrorRelativeImportNoPackage); + } + + [ServerTestMethod(LatestAvailable3X = true, TestSpecificRootUri = true), Priority(0)] + public async Task Diagnostics_RelativeImportInWorkingDirectory(Server server) { + var modulePath = "module.py"; + var moduleCode = "TEST = 0"; + var appPath = "app.py"; + var appCode = "from .module import *"; + var args = new PublishDiagnosticsEventArgs(); + server.OnPublishDiagnostics += (o, e) => args = e; + + await server.OpenDocumentAndGetUriAsync(modulePath, moduleCode); + await server.OpenDocumentAndGetUriAsync(appPath, appCode); + await server.WaitForCompleteAnalysisAsync(CancellationToken.None); + + args.diagnostics.Should().BeEmpty(); + } + + [ServerTestMethod(LatestAvailable3X = true, TestSpecificRootUri = true), Priority(0)] + [AddTestSpecificSearchPath("src")] + public async Task Diagnostics_InvalidRelativeImportInWorkingDirectory(Server server) { + var modulePath = "src/module.py"; + var moduleCode = "TEST = 0"; + var appPath = "app.py"; + var appCode = "from .module import TEST"; + var args = new PublishDiagnosticsEventArgs(); + server.OnPublishDiagnostics += (o, e) => args = e; + + await server.OpenDocumentAndGetUriAsync(modulePath, moduleCode); + await server.OpenDocumentAndGetUriAsync(appPath, appCode); + await server.WaitForCompleteAnalysisAsync(CancellationToken.None); + + args.diagnostics.Should().ContainSingle() + .Which.message.Should().Be(ErrorMessages.UnresolvedImport("module")); + } + + [ServerTestMethod(LatestAvailable3X = true), Priority(0)] + public async Task Diagnostics_RelativeImportInNonRooted(Server server) { + var modulePath = "module.py"; + var moduleCode = "TEST = 0"; + var appPath = "app.py"; + var appCode = "from .module import TEST"; + var args = new PublishDiagnosticsEventArgs(); + server.OnPublishDiagnostics += (o, e) => args = e; + + await server.OpenDocumentAndGetUriAsync(modulePath, moduleCode); + await server.OpenDocumentAndGetUriAsync(appPath, appCode); + await server.WaitForCompleteAnalysisAsync(CancellationToken.None); + + args.diagnostics.Should().BeEmpty(); + } + + [ServerTestMethod(LatestAvailable3X = true, TestSpecificRootUri = true), Priority(0)] + public async Task Diagnostics_RelativeImportInPackage(Server server) { + var module2Path = "package/module2.py"; + var module2Code = "TEST = 0"; + var module1Path = "package/module1.py"; + var module1Code = "from .module2 import TEST"; + var initPath = "package/__init__.py"; + var args = new PublishDiagnosticsEventArgs(); + server.OnPublishDiagnostics += (o, e) => args = e; + + await server.OpenDocumentAndGetUriAsync(initPath, string.Empty); + await server.OpenDocumentAndGetUriAsync(module1Path, module1Code); + await server.OpenDocumentAndGetUriAsync(module2Path, module2Code); + await server.WaitForCompleteAnalysisAsync(CancellationToken.None); + + args.diagnostics.Should().BeEmpty(); + } + + [ServerTestMethod(LatestAvailable3X = true, TestSpecificRootUri = true), Priority(0)] + public async Task Diagnostics_RelativeImportInPackage_ModuleWithNameOfFolderWithInitPy(Server server) { + var initPyPath = "package/module/__init__.py"; + var module1Path = "package/module.py"; + var module1Code = "TEST = 1"; + var module2Path = "package/module/sub/module.py"; + var module2Code = "TEST = 2"; + var testPath = "package/test.py"; + var testCode = "from .module.sub.module import TEST"; + var args = new PublishDiagnosticsEventArgs(); + + var testUri = await server.OpenDocumentAndGetUriAsync(testPath, testCode); + server.OnPublishDiagnostics += (o, e) => { + if (e.uri == testUri) { + args = e; + } + }; + + await server.OpenDocumentAndGetUriAsync(initPyPath, string.Empty); + await server.OpenDocumentAndGetUriAsync(module1Path, module1Code); + await server.OpenDocumentAndGetUriAsync(module2Path, module2Code); + + await server.WaitForCompleteAnalysisAsync(CancellationToken.None); + + args.diagnostics.Should().BeEmpty(); + } + + [ServerTestMethod(LatestAvailable3X = true, TestSpecificRootUri = true), Priority(0)] + public async Task Diagnostics_InvalidRelativeImportInPackage_ModuleWithNameOfFolder(Server server) { + var module2Path = "package/module/submodule.py"; + var module2Code = "TEST = 2"; + var module1Path = "package/module.py"; + var module1Code = "TEST = 1"; + var testPath = "package/test.py"; + var testCode = "from .module.submodule import TEST"; + var args = new PublishDiagnosticsEventArgs(); + + var testUri = await server.OpenDocumentAndGetUriAsync(testPath, testCode); + server.OnPublishDiagnostics += (o, e) => { + if (e.uri == testUri) { + args = e; + } + }; + + await server.OpenDocumentAndGetUriAsync(module1Path, module1Code); + await server.OpenDocumentAndGetUriAsync(module2Path, module2Code); + + await server.WaitForCompleteAnalysisAsync(CancellationToken.None); + + args.diagnostics.Should().ContainSingle() + .Which.message.Should().Be(ErrorMessages.UnresolvedImport("package.module.submodule")); + } + [ServerTestMethod(LatestAvailable3X = true), Priority(0)] public async Task Completions_SysModuleChain(Server server) { var content1 = @"import module2.mod as mod diff --git a/src/Analysis/Engine/Test/ServerTestMethodAttribute.cs b/src/Analysis/Engine/Test/ServerTestMethodAttribute.cs index d2dffff01..a12a4470d 100644 --- a/src/Analysis/Engine/Test/ServerTestMethodAttribute.cs +++ b/src/Analysis/Engine/Test/ServerTestMethodAttribute.cs @@ -15,6 +15,7 @@ // permissions and limitations under the License. using System; +using System.Linq; using System.Threading.Tasks; using Microsoft.Python.LanguageServer.Implementation; using Microsoft.PythonTools.Analysis; @@ -45,6 +46,7 @@ public override TestResult[] Execute(ITestMethod testMethod) { private TestResult ExecuteWithServer(ITestMethod testMethod) { var arguments = ExtendArguments(testMethod.Arguments); var filesToCreate = testMethod.GetAttributes(false); + var searchPathToAdd = testMethod.GetAttributes(false); TestEnvironmentImpl.AddBeforeAfterTest(async () => { var interpreterConfiguration = GetInterpreterConfiguration(arguments); @@ -53,7 +55,8 @@ private TestResult ExecuteWithServer(ITestMethod testMethod) { await TestData.CreateTestSpecificFileAsync(file.RelativeFilePath, file.Content); } - var server = await new Server().InitializeAsync(interpreterConfiguration, rootUri); + var searchPaths = searchPathToAdd.Select(a => TestData.GetTestSpecificPath(a.RelativeSearchPath)); + var server = await new Server().InitializeAsync(interpreterConfiguration, rootUri, searchPaths); if (DefaultTypeshedPath) { var limits = server.Analyzer.Limits; limits.UseTypeStubPackages = true;