-
Notifications
You must be signed in to change notification settings - Fork 133
Multiple analysis fixes #1297
Multiple analysis fixes #1297
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,39 +15,56 @@ | |
|
||
using System; | ||
using System.Diagnostics; | ||
using Microsoft.Python.Analysis.Documents; | ||
using Microsoft.Python.Analysis.Modules; | ||
using Microsoft.Python.Analysis.Types; | ||
using Microsoft.Python.Core; | ||
|
||
namespace Microsoft.Python.Analysis.Analyzer { | ||
[DebuggerDisplay("{Name} : {FilePath}")] | ||
internal struct AnalysisModuleKey : IEquatable<AnalysisModuleKey> { | ||
internal readonly struct AnalysisModuleKey : IEquatable<AnalysisModuleKey> { | ||
private enum KeyType { Default, Typeshed, LibraryAsDocument } | ||
|
||
private readonly KeyType _type; | ||
public string Name { get; } | ||
public string FilePath { get; } | ||
public bool IsTypeshed { get; } | ||
public bool IsTypeshed => _type == KeyType.Typeshed; | ||
public bool IsLibraryAsDocument => _type == KeyType.LibraryAsDocument; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a change for the #1294. Opened library module may be part of the loop, so with |
||
|
||
public AnalysisModuleKey(IPythonModule module) { | ||
Name = module.Name; | ||
FilePath = module.ModuleType == ModuleType.CompiledBuiltin ? null : module.FilePath; | ||
IsTypeshed = module is StubPythonModule stub && stub.IsTypeshed; | ||
_type = module is StubPythonModule stub && stub.IsTypeshed | ||
? KeyType.Typeshed | ||
: module.ModuleType == ModuleType.Library && module is IDocument document && document.IsOpen | ||
? KeyType.LibraryAsDocument | ||
: KeyType.Default; | ||
} | ||
|
||
public AnalysisModuleKey(string name, string filePath, bool isTypeshed) { | ||
Name = name; | ||
FilePath = filePath; | ||
IsTypeshed = isTypeshed; | ||
_type = isTypeshed ? KeyType.Typeshed : KeyType.Default; | ||
} | ||
|
||
private AnalysisModuleKey(string name, string filePath, KeyType type) { | ||
Name = name; | ||
FilePath = filePath; | ||
_type = type; | ||
} | ||
|
||
public AnalysisModuleKey GetLibraryAsDocumentKey() => new AnalysisModuleKey(Name, FilePath, KeyType.LibraryAsDocument); | ||
|
||
public bool Equals(AnalysisModuleKey other) | ||
=> Name.EqualsOrdinal(other.Name) && FilePath.PathEquals(other.FilePath) && IsTypeshed == other.IsTypeshed; | ||
=> Name.EqualsOrdinal(other.Name) && FilePath.PathEquals(other.FilePath) && _type == other._type; | ||
|
||
public override bool Equals(object obj) => obj is AnalysisModuleKey other && Equals(other); | ||
|
||
public override int GetHashCode() { | ||
unchecked { | ||
var hashCode = (Name != null ? Name.GetHashCode() : 0); | ||
hashCode = (hashCode * 397) ^ (FilePath != null ? FilePath.GetPathHashCode() : 0); | ||
hashCode = (hashCode * 397) ^ IsTypeshed.GetHashCode(); | ||
hashCode = (hashCode * 397) ^ _type.GetHashCode(); | ||
return hashCode; | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,14 +20,15 @@ | |
using Microsoft.Python.Analysis.Diagnostics; | ||
using Microsoft.Python.Analysis.Types; | ||
using Microsoft.Python.Core.Collections; | ||
using Microsoft.Python.Parsing.Ast; | ||
|
||
namespace Microsoft.Python.Analysis.Analyzer { | ||
public interface IPythonAnalyzer { | ||
Task WaitForCompleteAnalysisAsync(CancellationToken cancellationToken = default); | ||
/// <summary> | ||
/// Schedules module for analysis. Module will be scheduled if version of AST is greater than the one used to get previous analysis | ||
/// </summary> | ||
void EnqueueDocumentForAnalysis(IPythonModule module, int version); | ||
void EnqueueDocumentForAnalysis(IPythonModule module, PythonAst ast, int version); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There were several cases when There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't this indicate some other race condition, if the module's Ast is changing during an analysis? When would this occur? When the user modifies their code? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any change to the file, including opening/closing it. I'm not sure that having separate Parse/Analyze queue is still valuable for us, cause we don't have analysis on demand anymore - we use a graph, but that would be a separate change that I don't want to make until database support is in master. |
||
|
||
/// <summary> | ||
/// Schedules module for analysis for its existing AST, but with new dependencies. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,19 +56,22 @@ private void HandleImport(ModuleName moduleImportExpression, NameExpression asNa | |
// import_module('fob.oar.baz') | ||
var importNames = ImmutableArray<string>.Empty; | ||
var variableModule = default(PythonVariableModule); | ||
var importedNamesCount = 0; | ||
foreach (var nameExpression in moduleImportExpression.Names) { | ||
importNames = importNames.Add(nameExpression.Name); | ||
var imports = ModuleResolution.CurrentPathResolver.GetImportsFromAbsoluteName(Module.FilePath, importNames, forceAbsolute); | ||
if (!HandleImportSearchResult(imports, variableModule, asNameExpression, moduleImportExpression, out variableModule)) { | ||
return; | ||
break; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This handles a case when |
||
} | ||
|
||
importedNamesCount++; | ||
} | ||
|
||
// "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)) { | ||
if (!string.IsNullOrEmpty(asNameExpression?.Name) && importedNamesCount == moduleImportExpression.Names.Count) { | ||
Eval.DeclareVariable(asNameExpression.Name, variableModule, VariableSource.Import, asNameExpression); | ||
} else if (importNames.Count > 0 && !string.IsNullOrEmpty(importNames[0]) && _variableModules.TryGetValue(importNames[0], out variableModule)) { | ||
} else if (importedNamesCount > 0 && !string.IsNullOrEmpty(importNames[0]) && _variableModules.TryGetValue(importNames[0], out variableModule)) { | ||
var firstName = moduleImportExpression.Names[0]; | ||
Eval.DeclareVariable(importNames[0], variableModule, VariableSource.Import, firstName); | ||
} | ||
|
@@ -87,7 +90,7 @@ private bool HandleImportSearchResult(in IImportSearchResult imports, in PythonV | |
case RelativeImportBeyondTopLevel importBeyondTopLevel: | ||
var message = Resources.ErrorRelativeImportBeyondTopLevel.FormatInvariant(importBeyondTopLevel.RelativeImportName); | ||
Eval.ReportDiagnostics(Eval.Module.Uri, | ||
new DiagnosticsEntry(message, location.GetLocation(Eval.Module).Span, ErrorCodes.UnresolvedImport, Severity.Warning, DiagnosticSource.Analysis)); | ||
new DiagnosticsEntry(message, location.GetLocation(Eval).Span, ErrorCodes.UnresolvedImport, Severity.Warning, DiagnosticSource.Analysis)); | ||
variableModule = default; | ||
return false; | ||
case ImportNotFound importNotFound: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AnalysisState
has never been used