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

Commit 23f6d67

Browse files
AlexanderSherAlexanderSher
authored
Two path resolution bug fixes
- Fix #509: PLS doesn't flag error in a relative import if it is found in another root directory (#519) - Fix #510: PLS doesn't resolve relative paths correctly
1 parent 0893287 commit 23f6d67

File tree

10 files changed

+253
-14
lines changed

10 files changed

+253
-14
lines changed

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

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

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

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

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

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

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

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

201196
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
208203
.ToString());
209204
}
210205

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

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

@@ -225,7 +235,13 @@ public IImportSearchResult GetImportsFromRelativePath(in string modulePath, in i
225235
return default;
226236
}
227237

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

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

251267
private static bool TryCreateModuleImport(Node rootNode, Node moduleNode, out ModuleImport moduleImport) {
252-
if (moduleNode.TryGetChild("__init__", out var initPyNode) && initPyNode.IsModule) {
268+
if (IsPackageWithInitPy(moduleNode, out var initPyNode)) {
253269
moduleImport = new ModuleImport(moduleNode.Name, initPyNode.FullModuleName, rootNode.Name, initPyNode.ModulePath, false);
254270
return true;
255271
}
@@ -516,7 +532,7 @@ private static bool TryFindImport(IEnumerable<Edge> rootEdges, List<string> full
516532
private static bool TryFindName(in Edge edge, in IEnumerable<string> nameParts, out Edge lastEdge) {
517533
lastEdge = edge;
518534
foreach (var name in nameParts) {
519-
if (lastEdge.End.IsModule) {
535+
if (lastEdge.End.IsModule && !IsPackageWithInitPy(lastEdge.End, out _)) {
520536
return false;
521537
}
522538
var index = lastEdge.End.GetChildIndex(name);
@@ -652,7 +668,7 @@ private static StringBuilder GetFullModuleNameBuilder(in Edge lastEdge) {
652668
while (edge != lastEdge) {
653669
AppendName(sb, edge.End.Name);
654670
edge = edge.Next;
655-
};
671+
}
656672

657673
return sb;
658674
}
@@ -743,6 +759,9 @@ private static int GetModuleNameStart(string rootedModulePath)
743759
private static int GetModuleNameEnd(string rootedModulePath)
744760
=> IsPythonCompiled(rootedModulePath) ? rootedModulePath.IndexOf('.', GetModuleNameStart(rootedModulePath)) : rootedModulePath.LastIndexOf('.');
745761

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

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

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

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

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

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

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

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

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

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

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ public static class Server {
3737
new EventTaskSource<Microsoft.Python.LanguageServer.Implementation.Server, Microsoft.Python.LanguageServer.ParseCompleteEventArgs>(
3838
(o, e) => o.OnParseComplete += e,
3939
(o, e) => o.OnParseComplete -= e);
40+
41+
public static readonly EventTaskSource<Microsoft.Python.LanguageServer.Implementation.Server, Microsoft.Python.LanguageServer.PublishDiagnosticsEventArgs> OnPublishDiagnostics =
42+
new EventTaskSource<Microsoft.Python.LanguageServer.Implementation.Server, Microsoft.Python.LanguageServer.PublishDiagnosticsEventArgs>(
43+
(o, e) => o.OnPublishDiagnostics += e,
44+
(o, e) => o.OnPublishDiagnostics -= e);
4045
}
4146
}
4247
}

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

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,15 @@
1919
using System.Linq;
2020
using System.Threading;
2121
using System.Threading.Tasks;
22+
using FluentAssertions;
23+
using Microsoft.Python.LanguageServer;
2224
using Microsoft.Python.LanguageServer.Implementation;
2325
using Microsoft.PythonTools.Analysis;
26+
using Microsoft.PythonTools.Analysis.Analyzer;
2427
using Microsoft.PythonTools.Analysis.FluentAssertions;
2528
using Microsoft.VisualStudio.TestTools.UnitTesting;
2629
using TestUtilities;
30+
using Resources = Microsoft.PythonTools.Analysis.Resources;
2731

2832
namespace AnalysisTests {
2933
[TestClass]
@@ -251,6 +255,145 @@ import projectB.foo.baz
251255
completion.Should().HaveLabels("foo");
252256
}
253257

258+
[ServerTestMethod(LatestAvailable3X = true, TestSpecificRootUri = true), Priority(0)]
259+
[AddTestSpecificSearchPath("src")]
260+
public async Task Diagnostics_InvalidRelativeImportInUserSearchPath(Server server) {
261+
var modulePath = "src/module.py";
262+
var moduleCode = "TEST = 0";
263+
var appPath = "src/app.py";
264+
var appCode = "from .module import TEST";
265+
var args = new PublishDiagnosticsEventArgs();
266+
server.OnPublishDiagnostics += (o, e) => args = e;
267+
268+
await server.OpenDocumentAndGetUriAsync(modulePath, moduleCode);
269+
await server.OpenDocumentAndGetUriAsync(appPath, appCode);
270+
await server.WaitForCompleteAnalysisAsync(CancellationToken.None);
271+
272+
args.diagnostics.Should().ContainSingle()
273+
.Which.message.Should().Be(Resources.ErrorRelativeImportNoPackage);
274+
}
275+
276+
[ServerTestMethod(LatestAvailable3X = true, TestSpecificRootUri = true), Priority(0)]
277+
public async Task Diagnostics_RelativeImportInWorkingDirectory(Server server) {
278+
var modulePath = "module.py";
279+
var moduleCode = "TEST = 0";
280+
var appPath = "app.py";
281+
var appCode = "from .module import *";
282+
var args = new PublishDiagnosticsEventArgs();
283+
server.OnPublishDiagnostics += (o, e) => args = e;
284+
285+
await server.OpenDocumentAndGetUriAsync(modulePath, moduleCode);
286+
await server.OpenDocumentAndGetUriAsync(appPath, appCode);
287+
await server.WaitForCompleteAnalysisAsync(CancellationToken.None);
288+
289+
args.diagnostics.Should().BeEmpty();
290+
}
291+
292+
[ServerTestMethod(LatestAvailable3X = true, TestSpecificRootUri = true), Priority(0)]
293+
[AddTestSpecificSearchPath("src")]
294+
public async Task Diagnostics_InvalidRelativeImportInWorkingDirectory(Server server) {
295+
var modulePath = "src/module.py";
296+
var moduleCode = "TEST = 0";
297+
var appPath = "app.py";
298+
var appCode = "from .module import TEST";
299+
var args = new PublishDiagnosticsEventArgs();
300+
server.OnPublishDiagnostics += (o, e) => args = e;
301+
302+
await server.OpenDocumentAndGetUriAsync(modulePath, moduleCode);
303+
await server.OpenDocumentAndGetUriAsync(appPath, appCode);
304+
await server.WaitForCompleteAnalysisAsync(CancellationToken.None);
305+
306+
args.diagnostics.Should().ContainSingle()
307+
.Which.message.Should().Be(ErrorMessages.UnresolvedImport("module"));
308+
}
309+
310+
[ServerTestMethod(LatestAvailable3X = true), Priority(0)]
311+
public async Task Diagnostics_RelativeImportInNonRooted(Server server) {
312+
var modulePath = "module.py";
313+
var moduleCode = "TEST = 0";
314+
var appPath = "app.py";
315+
var appCode = "from .module import TEST";
316+
var args = new PublishDiagnosticsEventArgs();
317+
server.OnPublishDiagnostics += (o, e) => args = e;
318+
319+
await server.OpenDocumentAndGetUriAsync(modulePath, moduleCode);
320+
await server.OpenDocumentAndGetUriAsync(appPath, appCode);
321+
await server.WaitForCompleteAnalysisAsync(CancellationToken.None);
322+
323+
args.diagnostics.Should().BeEmpty();
324+
}
325+
326+
[ServerTestMethod(LatestAvailable3X = true, TestSpecificRootUri = true), Priority(0)]
327+
public async Task Diagnostics_RelativeImportInPackage(Server server) {
328+
var module2Path = "package/module2.py";
329+
var module2Code = "TEST = 0";
330+
var module1Path = "package/module1.py";
331+
var module1Code = "from .module2 import TEST";
332+
var initPath = "package/__init__.py";
333+
var args = new PublishDiagnosticsEventArgs();
334+
server.OnPublishDiagnostics += (o, e) => args = e;
335+
336+
await server.OpenDocumentAndGetUriAsync(initPath, string.Empty);
337+
await server.OpenDocumentAndGetUriAsync(module1Path, module1Code);
338+
await server.OpenDocumentAndGetUriAsync(module2Path, module2Code);
339+
await server.WaitForCompleteAnalysisAsync(CancellationToken.None);
340+
341+
args.diagnostics.Should().BeEmpty();
342+
}
343+
344+
[ServerTestMethod(LatestAvailable3X = true, TestSpecificRootUri = true), Priority(0)]
345+
public async Task Diagnostics_RelativeImportInPackage_ModuleWithNameOfFolderWithInitPy(Server server) {
346+
var initPyPath = "package/module/__init__.py";
347+
var module1Path = "package/module.py";
348+
var module1Code = "TEST = 1";
349+
var module2Path = "package/module/sub/module.py";
350+
var module2Code = "TEST = 2";
351+
var testPath = "package/test.py";
352+
var testCode = "from .module.sub.module import TEST";
353+
var args = new PublishDiagnosticsEventArgs();
354+
355+
var testUri = await server.OpenDocumentAndGetUriAsync(testPath, testCode);
356+
server.OnPublishDiagnostics += (o, e) => {
357+
if (e.uri == testUri) {
358+
args = e;
359+
}
360+
};
361+
362+
await server.OpenDocumentAndGetUriAsync(initPyPath, string.Empty);
363+
await server.OpenDocumentAndGetUriAsync(module1Path, module1Code);
364+
await server.OpenDocumentAndGetUriAsync(module2Path, module2Code);
365+
366+
await server.WaitForCompleteAnalysisAsync(CancellationToken.None);
367+
368+
args.diagnostics.Should().BeEmpty();
369+
}
370+
371+
[ServerTestMethod(LatestAvailable3X = true, TestSpecificRootUri = true), Priority(0)]
372+
public async Task Diagnostics_InvalidRelativeImportInPackage_ModuleWithNameOfFolder(Server server) {
373+
var module2Path = "package/module/submodule.py";
374+
var module2Code = "TEST = 2";
375+
var module1Path = "package/module.py";
376+
var module1Code = "TEST = 1";
377+
var testPath = "package/test.py";
378+
var testCode = "from .module.submodule import TEST";
379+
var args = new PublishDiagnosticsEventArgs();
380+
381+
var testUri = await server.OpenDocumentAndGetUriAsync(testPath, testCode);
382+
server.OnPublishDiagnostics += (o, e) => {
383+
if (e.uri == testUri) {
384+
args = e;
385+
}
386+
};
387+
388+
await server.OpenDocumentAndGetUriAsync(module1Path, module1Code);
389+
await server.OpenDocumentAndGetUriAsync(module2Path, module2Code);
390+
391+
await server.WaitForCompleteAnalysisAsync(CancellationToken.None);
392+
393+
args.diagnostics.Should().ContainSingle()
394+
.Which.message.Should().Be(ErrorMessages.UnresolvedImport("package.module.submodule"));
395+
}
396+
254397
[ServerTestMethod(LatestAvailable3X = true), Priority(0)]
255398
public async Task Completions_SysModuleChain(Server server) {
256399
var content1 = @"import module2.mod as mod

0 commit comments

Comments
 (0)