Skip to content

Commit 00aebad

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

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
@@ -187,16 +187,11 @@ public IImportSearchResult GetImportsFromRelativePath(in string modulePath, in i
187187
return default;
188188
}
189189

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

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

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

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

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

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

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

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

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

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

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

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

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

‎src/Analysis/Engine/Impl/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)