Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

Commit

Permalink
Implement declaration of submodules in module members (microsoft#1565)
Browse files Browse the repository at this point in the history 
* Add submodules to module members

* Add test

* Test updates

* Wait for complete analysis

* Different way of filtering

* Formatting

* Remove unused

* Move filtering to completions
Declare submodule as member in case of import A.B inside A

* Add another test

* using

* De-duplicate

* Remove unused

* Revert to initial GetMemberNames/GetMember on module

* Prefer submodule over variable

* Use child modules instead

* Extra checks

* Fix child module naming

* Prefer submodules to variables.

* Prefer submodules to variables in import star
  • Loading branch information
Mikhail Arkhipov authored Sep 26, 2019
1 parent 9cd4df3 commit c250f5b
Show file tree
Hide file tree
Showing 20 changed files with 369 additions and 177 deletions.
58 changes: 33 additions & 25 deletions src/Analysis/Ast/Impl/Analyzer/Handlers/FromImportHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ private void AssignVariables(FromImportStatement node, IImportSearchResult impor
// TODO: warn this is not a good style per
// TODO: https://docs.python.org/3/faq/programming.html#what-are-the-best-practices-for-using-import-in-a-module
// TODO: warn this is invalid if not in the global scope.
HandleModuleImportStar(variableModule, imports is ImplicitPackageImport, node.StartIndex);
HandleModuleImportStar(variableModule, imports, node.StartIndex, names[0]);
return;
}

Expand All @@ -69,43 +69,51 @@ private void AssignVariables(FromImportStatement node, IImportSearchResult impor
var nameExpression = asNames[i] ?? names[i];
var variableName = nameExpression?.Name ?? memberName;
if (!string.IsNullOrEmpty(variableName)) {
var variable = variableModule.Analysis?.GlobalScope?.Variables[memberName];
var exported = variable ?? variableModule.GetMember(memberName);
var value = exported ?? GetValueFromImports(variableModule, imports as IImportChildrenSource, memberName);
// Do not allow imported variables to override local declarations
Eval.DeclareVariable(variableName, value, VariableSource.Import, nameExpression, CanOverwriteVariable(variableName, node.StartIndex));
DeclareVariable(variableModule, memberName, imports, variableName, node.StartIndex, nameExpression);
}
}
}
}

private void HandleModuleImportStar(PythonVariableModule variableModule, bool isImplicitPackage, int importPosition) {
private void HandleModuleImportStar(PythonVariableModule variableModule, IImportSearchResult imports, int importPosition, NameExpression nameExpression) {
if (variableModule.Module == Module) {
// from self import * won't define any new members
return;
}

// If __all__ is present, take it, otherwise declare all members from the module that do not begin with an underscore.
var memberNames = isImplicitPackage
var memberNames = imports is ImplicitPackageImport
? variableModule.GetMemberNames()
: variableModule.Analysis.StarImportMemberNames ?? variableModule.GetMemberNames().Where(s => !s.StartsWithOrdinal("_"));

foreach (var memberName in memberNames) {
var member = variableModule.GetMember(memberName);
if (member == null) {
Log?.Log(TraceEventType.Verbose, $"Undefined import: {variableModule.Name}, {memberName}");
} else if (member.MemberType == PythonMemberType.Unknown) {
Log?.Log(TraceEventType.Verbose, $"Unknown import: {variableModule.Name}, {memberName}");
}

member = member ?? Eval.UnknownType;
if (member is IPythonModule m) {
ModuleResolution.GetOrLoadModule(m.Name);
}
DeclareVariable(variableModule, memberName, imports, memberName, importPosition, nameExpression);
}
}

var variable = variableModule.Analysis?.GlobalScope?.Variables[memberName];
// Do not allow imported variables to override local declarations
Eval.DeclareVariable(memberName, variable ?? member, VariableSource.Import, Eval.DefaultLocation, CanOverwriteVariable(memberName, importPosition));
/// <summary>
/// Determines value of the variable and declares it. Value depends if source module has submodule
/// that is named the same as the variable and/or it has internal variables named same as the submodule.
/// </summary>
/// <example>'from a.b import c' when 'c' is both submodule of 'b' and a variable declared inside 'b'.</example>
/// <param name="variableModule">Source module of the variable such as 'a.b' in 'from a.b import c as d'.</param>
/// <param name="memberName">Module member name such as 'c' in 'from a.b import c as d'.</param>
/// <param name="imports">Import search result.</param>
/// <param name="variableName">Name of the variable to declare, such as 'd' in 'from a.b import c as d'.</param>
/// <param name="importPosition">Position of the import statement.</param>
/// <param name="nameExpression">Name expression of the variable.</param>
private void DeclareVariable(PythonVariableModule variableModule, string memberName, IImportSearchResult imports, string variableName, int importPosition, Node nameExpression) {
// First try imports since child modules should win, i.e. in 'from a.b import c'
// 'c' should be a submodule if 'b' has one, even if 'b' also declares 'c = 1'.
var value = GetValueFromImports(variableModule, imports as IImportChildrenSource, memberName);
// Now try exported
value = value ?? variableModule.GetMember(memberName);
// If nothing is exported, variables are still accessible.
value = value ?? variableModule.Analysis?.GlobalScope?.Variables[memberName]?.Value ?? Eval.UnknownType;
// Do not allow imported variables to override local declarations
Eval.DeclareVariable(variableName, value, VariableSource.Import, nameExpression, CanOverwriteVariable(variableName, importPosition));
// Make sure module is loaded and analyzed.
if (value is IPythonModule m) {
ModuleResolution.GetOrLoadModule(m.Name);
}
}

Expand All @@ -131,7 +139,7 @@ private bool CanOverwriteVariable(string name, int importPosition) {

private IMember GetValueFromImports(PythonVariableModule parentModule, IImportChildrenSource childrenSource, string memberName) {
if (childrenSource == null || !childrenSource.TryGetChildImport(memberName, out var childImport)) {
return Interpreter.UnknownType;
return null;
}

switch (childImport) {
Expand All @@ -141,7 +149,7 @@ private IMember GetValueFromImports(PythonVariableModule parentModule, IImportCh
case ImplicitPackageImport packageImport:
return GetOrCreateVariableModule(packageImport.FullName, parentModule, memberName);
default:
return Interpreter.UnknownType;
return null;
}
}

Expand Down
48 changes: 34 additions & 14 deletions src/Analysis/Ast/Impl/Analyzer/Handlers/ImportHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,27 +56,47 @@ private void HandleImport(ModuleName moduleImportExpression, NameExpression asNa
// import_module('fob.oar.baz')
var importNames = ImmutableArray<string>.Empty;
var lastModule = default(PythonVariableModule);
var firstModule = default(PythonVariableModule);
foreach (var nameExpression in moduleImportExpression.Names) {
var resolvedModules = new (string name, PythonVariableModule module)[moduleImportExpression.Names.Count];
for (var i = 0; i < moduleImportExpression.Names.Count; i++) {
var nameExpression = moduleImportExpression.Names[i];
importNames = importNames.Add(nameExpression.Name);
var imports = ModuleResolution.CurrentPathResolver.GetImportsFromAbsoluteName(Module.FilePath, importNames, forceAbsolute);
if (!HandleImportSearchResult(imports, lastModule, asNameExpression, moduleImportExpression, out lastModule)) {
lastModule = default;
break;
}

if (firstModule == null) {
firstModule = lastModule;
}
resolvedModules[i] = (nameExpression.Name, lastModule);
}

// "import fob.oar.baz as baz" is handled as baz = import_module('fob.oar.baz')
// "import fob.oar.baz" is handled as fob = import_module('fob')
if (!string.IsNullOrEmpty(asNameExpression?.Name) && lastModule != null) {
Eval.DeclareVariable(asNameExpression.Name, lastModule, VariableSource.Import, asNameExpression);
} else if (firstModule != null && !string.IsNullOrEmpty(importNames[0])) {
var firstName = moduleImportExpression.Names[0];
Eval.DeclareVariable(importNames[0], firstModule, VariableSource.Import, firstName);
return;
}

var firstModule = resolvedModules.Length > 0 ? resolvedModules[0].module : null;
var secondModule = resolvedModules.Length > 1 ? resolvedModules[1].module : null;

// "import fob.oar.baz" when 'fob' is THIS module handled by declaring 'oar' as member.
// Consider pandas that has 'import pandas.testing' in __init__.py and 'testing'
// is available as member. See also https://github.com/microsoft/python-language-server/issues/1395
if (firstModule?.Module == Eval.Module && importNames.Count > 1 && !string.IsNullOrEmpty(importNames[1]) && secondModule != null) {
Eval.DeclareVariable(importNames[0], firstModule, VariableSource.Import, moduleImportExpression.Names[0]);
Eval.DeclareVariable(importNames[1], secondModule, VariableSource.Import, moduleImportExpression.Names[1]);
} else {
// "import fob.oar.baz" is handled as fob = import_module('fob')
if (firstModule != null && !string.IsNullOrEmpty(importNames[0])) {
Eval.DeclareVariable(importNames[0], firstModule, VariableSource.Import, moduleImportExpression.Names[0]);
}
}

// import a.b.c.d => declares a, b in the current module, c in b, d in c.
for (var i = 1; i < resolvedModules.Length - 1; i++) {
var (childName, childModule) = resolvedModules[i + 1];
if (!string.IsNullOrEmpty(childName) && childModule != null) {
var parent = resolvedModules[i].module;
parent?.AddChildModule(childName, childModule);
}
}
}

Expand All @@ -92,7 +112,7 @@ private bool HandleImportSearchResult(in IImportSearchResult imports, in PythonV
return TryGetPackageFromImport(packageImport, parent, out variableModule);
case RelativeImportBeyondTopLevel importBeyondTopLevel:
var message = Resources.ErrorRelativeImportBeyondTopLevel.FormatInvariant(importBeyondTopLevel.RelativeImportName);
Eval.ReportDiagnostics(Eval.Module.Uri,
Eval.ReportDiagnostics(Eval.Module.Uri,
new DiagnosticsEntry(message, location.GetLocation(Eval).Span, ErrorCodes.UnresolvedImport, Severity.Warning, DiagnosticSource.Analysis));
variableModule = default;
return false;
Expand Down Expand Up @@ -157,7 +177,7 @@ private bool TryGetModulePossibleImport(PossibleModuleImport possibleModuleImpor
return false;
}
}

return true;
}

Expand All @@ -170,8 +190,8 @@ private void MakeUnresolvedImport(string variableName, string moduleName, Node l
if (!string.IsNullOrEmpty(variableName)) {
Eval.DeclareVariable(variableName, new SentinelModule(moduleName, Eval.Services), VariableSource.Import, location);
}
Eval.ReportDiagnostics(Eval.Module.Uri,
new DiagnosticsEntry(Resources.ErrorUnresolvedImport.FormatInvariant(moduleName),
Eval.ReportDiagnostics(Eval.Module.Uri,
new DiagnosticsEntry(Resources.ErrorUnresolvedImport.FormatInvariant(moduleName),
Eval.GetLocationInfo(location).Span, ErrorCodes.UnresolvedImport, Severity.Warning, DiagnosticSource.Analysis));
}

Expand Down
17 changes: 11 additions & 6 deletions src/Analysis/Ast/Impl/Extensions/PythonModuleExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@
// See the Apache Version 2.0 License for specific language governing
// permissions and limitations under the License.

using System.Collections.Generic;
using System.Linq;
using Microsoft.Python.Analysis.Core.DependencyResolution;
using Microsoft.Python.Analysis.Modules;
using Microsoft.Python.Analysis.Types;
using Microsoft.Python.Core;
using Microsoft.Python.Core.Text;
using Microsoft.Python.Parsing.Ast;

Expand All @@ -39,12 +43,12 @@ internal static void AddAstNode(this IPythonModule module, object o, Node n)
/// </summary>
/// <param name="lineNum">The line number</param>
internal static string GetLine(this IPythonModule module, int lineNum) {
string content = module.Analysis?.Document?.Content;
var content = module.Analysis?.Document?.Content;
if (string.IsNullOrEmpty(content)) {
return string.Empty;
}

SourceLocation source = new SourceLocation(lineNum, 1);
var source = new SourceLocation(lineNum, 1);
var start = module.GetAst().LocationToIndex(source);
var end = start;

Expand All @@ -58,17 +62,18 @@ internal static string GetLine(this IPythonModule module, int lineNum) {
/// </summary>
/// <param name="lineNum">The line number</param>
internal static string GetComment(this IPythonModule module, int lineNum) {
string line = module.GetLine(lineNum);
var line = module.GetLine(lineNum);

int commentPos = line.IndexOf('#');
var commentPos = line.IndexOf('#');
if (commentPos < 0) {
return string.Empty;
}

return line.Substring(commentPos + 1).Trim('\t', ' ');
}

internal static bool IsNonUserFile(this IPythonModule module) => module.ModuleType.IsNonUserFile();
internal static bool IsCompiled(this IPythonModule module) => module.ModuleType.IsCompiled();
public static bool IsNonUserFile(this IPythonModule module) => module.ModuleType.IsNonUserFile();
public static bool IsCompiled(this IPythonModule module) => module.ModuleType.IsCompiled();
public static bool IsTypingModule(this IPythonModule module) => module?.ModuleType == ModuleType.Specialized && module.Name == "typing";
}
}
21 changes: 11 additions & 10 deletions src/Analysis/Ast/Impl/Modules/PythonModule.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ public virtual string Documentation {

#region IMemberContainer
public virtual IMember GetMember(string name) => GlobalScope.Variables[name]?.Value;

public virtual IEnumerable<string> GetMemberNames() {
// drop imported modules and typing.
return GlobalScope.Variables
Expand All @@ -166,26 +167,26 @@ public virtual IEnumerable<string> GetMemberNames() {
if (v.Value is IPythonInstance) {
return true;
}

var valueType = v.Value?.GetPythonType();
if (valueType is PythonModule) {
return false; // Do not re-export modules.
}
if (valueType is IPythonFunctionType f && f.IsLambda()) {
return false;
switch (valueType) {
case PythonModule _:
case IPythonFunctionType f when f.IsLambda():
return false; // Do not re-export modules.
}

if (this is TypingModule) {
return true; // Let typing module behave normally.
}

// Do not re-export types from typing. However, do export variables
// assigned with types from typing. Example:
// from typing import Any # do NOT export Any
// x = Union[int, str] # DO export x
if (valueType?.DeclaringModule is TypingModule && v.Name == valueType.Name) {
return false;
}
return true;
return !(valueType?.DeclaringModule is TypingModule) || v.Name != valueType.Name;
})
.Select(v => v.Name);
.Select(v => v.Name)
.ToArray();
}
#endregion

Expand Down
4 changes: 2 additions & 2 deletions src/Analysis/Ast/Impl/Modules/PythonVariableModule.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ public PythonVariableModule(IPythonModule module): base(module) {

public void AddChildModule(string memberName, PythonVariableModule module) => _children[memberName] = module;

public IMember GetMember(string name) => Module?.GetMember(name) ?? (_children.TryGetValue(name, out var module) ? module : default);
public IEnumerable<string> GetMemberNames() => Module != null ? Module.GetMemberNames().Concat(_children.Keys) : _children.Keys;
public IMember GetMember(string name) => _children.TryGetValue(name, out var module) ? module : Module?.GetMember(name);
public IEnumerable<string> GetMemberNames() => Module != null ? Module.GetMemberNames().Concat(_children.Keys).Distinct() : _children.Keys;

public IMember Call(IPythonInstance instance, string memberName, IArgumentSet args) => GetMember(memberName);
public IMember Index(IPythonInstance instance, IArgumentSet args) => Interpreter.UnknownType;
Expand Down
18 changes: 16 additions & 2 deletions src/Analysis/Ast/Test/FluentAssertions/MemberAssertions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,20 @@ public AndWhichConstraint<MemberAssertions, IPythonType> HaveBase(string name, s
public AndWhichConstraint<MemberAssertions, PythonFunctionType> HaveMethod(string name, string because = "", params object[] reasonArgs)
=> HaveMember<PythonFunctionType>(name, because, reasonArgs).OfMemberType(PythonMemberType.Method);

public void HaveMemberName(string name, string because = "", params object[] reasonArgs) {
NotBeNull();

var t = Subject.GetPythonType();
var mc = (IMemberContainer)t;
Execute.Assertion.ForCondition(mc != null)
.BecauseOf(because, reasonArgs)
.FailWith($"Expected {GetName(t)} to be a member container{{reason}}.");

Execute.Assertion.ForCondition(mc.GetMemberNames().Contains(name))
.BecauseOf(because, reasonArgs)
.FailWith($"Expected {GetName(t)} to have a member named '{name}'{{reason}}.");
}

public AndWhichConstraint<MemberAssertions, TMember> HaveMember<TMember>(string name,
string because = "", params object[] reasonArgs)
where TMember : class, IMember {
Expand Down Expand Up @@ -125,7 +139,7 @@ public void HaveSameMembersAs(IMember other) {

Debug.Assert(missingNames.Length == 0);
missingNames.Should().BeEmpty("Subject has missing names: ", missingNames);

Debug.Assert(extraNames.Length == 0);
extraNames.Should().BeEmpty("Subject has extra names: ", extraNames);

Expand All @@ -147,7 +161,7 @@ public void HaveSameMembersAs(IMember other) {
var otherClass = otherMemberType as IPythonClassType;
otherClass.Should().NotBeNull();

if(subjectClass is IGenericType gt) {
if (subjectClass is IGenericType gt) {
otherClass.Should().BeAssignableTo<IGenericType>();
otherClass.IsGeneric.Should().Be(gt.IsGeneric, $"Class name: {subjectClass.Name}");
}
Expand Down
Loading

0 comments on commit c250f5b

Please sign in to comment.