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

Commit 8a5b808

Browse files
heejaechangheejaechang
authored
some clean ups (#1596)
* simple clean up removed unused parameter, added name argument and etc * more clean up removed unused usings and other benign ones. * more simple clean ups named arguments, removing unncessary stuff. * addressed PR feedback removed named arguments. * PR feedback
1 parent 9f86f68 commit 8a5b808

14 files changed

+63
-47
lines changed

‎src/Analysis/Ast/Impl/Analyzer/AnalysisModuleKey.cs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,13 @@ namespace Microsoft.Python.Analysis.Analyzer {
2828
public bool IsTypeshed { get; }
2929
public bool IsNonUserAsDocument { get; }
3030

31-
public AnalysisModuleKey(IPythonModule module) {
32-
Name = module.Name;
33-
FilePath = module.ModuleType == ModuleType.CompiledBuiltin ? null : module.FilePath;
34-
IsTypeshed = module is StubPythonModule stub && stub.IsTypeshed;
35-
IsNonUserAsDocument = (module.IsNonUserFile() || module.IsCompiled()) && module is IDocument document && document.IsOpen;
36-
}
31+
public AnalysisModuleKey(IPythonModule module) : this(
32+
module.Name,
33+
module.ModuleType == ModuleType.CompiledBuiltin ? null : module.FilePath,
34+
IsTypeshedModule(module),
35+
IsNonUserAsDocumentModule(module)) { }
3736

38-
public AnalysisModuleKey(string name, string filePath, bool isTypeshed)
37+
public AnalysisModuleKey(string name, string filePath, bool isTypeshed)
3938
: this(name, filePath, isTypeshed, false) { }
4039

4140
private AnalysisModuleKey(string name, string filePath, bool isTypeshed, bool isNonUserAsDocument) {
@@ -73,5 +72,10 @@ public void Deconstruct(out string moduleName, out string filePath, out bool isT
7372
}
7473

7574
public override string ToString() => $"{Name}({FilePath})";
75+
76+
private static bool IsTypeshedModule(IPythonModule module)
77+
=> module is StubPythonModule stubPythonModule && stubPythonModule.IsTypeshed;
78+
private static bool IsNonUserAsDocumentModule(IPythonModule module)
79+
=> (module.IsNonUserFile() || module.IsCompiled()) && module is IDocument document && document.IsOpen;
7680
}
7781
}

‎src/Analysis/Ast/Impl/Analyzer/ProgressReporter.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@
2020

2121
namespace Microsoft.Python.Analysis.Analyzer {
2222
internal sealed class ProgressReporter : IProgressReporter, IDisposable {
23-
private const int _initialDelay = 50;
24-
private const int _reportingInterval = 100;
23+
private readonly static TimeSpan InitialDelay = TimeSpan.FromMilliseconds(50);
24+
private readonly static TimeSpan ReportingInterval = TimeSpan.FromMilliseconds(100);
2525

2626
private readonly IProgressService _progressService;
2727
private readonly object _lock = new object();
@@ -44,15 +44,15 @@ public void Dispose() {
4444
public void ReportRemaining(int count) {
4545
lock (_lock) {
4646
if (count == 0) {
47-
EndReport();
47+
EndReport();
4848
return;
4949
}
5050

5151
if (!_running) {
5252
// Delay reporting a bit in case the analysis is short in order to reduce UI flicker.
5353
_running = true;
5454
_reportTimer?.Dispose();
55-
_reportTimer = new Timer(OnReportTimer, null, _initialDelay, _reportingInterval);
55+
_reportTimer = new Timer(OnReportTimer, null, InitialDelay, ReportingInterval);
5656
}
5757
_lastReportedCount = count;
5858
}

‎src/Analysis/Ast/Impl/Analyzer/PythonAnalyzer.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ private PythonAnalyzerSession CreateSession(in IDependencyChainWalker<AnalysisMo
329329
_forceGCOnNextSession = false;
330330
}
331331

332-
return new PythonAnalyzerSession(_services, _progress, _analysisCompleteEvent, _startNextSession, _disposeToken.CancellationToken, walker, _version, entry, forceGC: forceGC);
332+
return new PythonAnalyzerSession(_services, _progress, _startNextSession, _disposeToken.CancellationToken, walker, _version, entry, forceGC: forceGC);
333333
}
334334

335335
private void LoadMissingDocuments(IPythonInterpreter interpreter, ImmutableArray<AnalysisModuleKey> missingKeys) {

‎src/Analysis/Ast/Impl/Analyzer/PythonAnalyzerEntry.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
using System.Threading.Tasks;
2222
using Microsoft.Python.Analysis.Modules;
2323
using Microsoft.Python.Analysis.Types;
24+
using Microsoft.Python.Core;
2425
using Microsoft.Python.Core.Collections;
2526
using Microsoft.Python.Parsing.Ast;
2627

@@ -93,7 +94,7 @@ public PythonAnalyzerEntry(EmptyAnalysis emptyAnalysis) {
9394
}
9495

9596
public Task<IDocumentAnalysis> GetAnalysisAsync(CancellationToken cancellationToken)
96-
=> _analysisTcs.Task.ContinueWith(t => t.GetAwaiter().GetResult(), cancellationToken);
97+
=> _analysisTcs.Task.WaitAsync(cancellationToken);
9798

9899
public bool CanUpdateAnalysis(int version, out IPythonModule module, out PythonAst ast, out IDocumentAnalysis currentAnalysis) {
99100
lock (_syncObj) {
@@ -271,7 +272,5 @@ private void UpdateAnalysisTcs(int analysisVersion) {
271272
_analysisTcs = new TaskCompletionSource<IDocumentAnalysis>(TaskCreationOptions.RunContinuationsAsynchronously);
272273
}
273274
}
274-
275-
276275
}
277276
}

‎src/Analysis/Ast/Impl/Analyzer/PythonAnalyzerSession.cs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ internal sealed class PythonAnalyzerSession {
4343
private readonly Action<Task> _startNextSession;
4444
private readonly CancellationToken _analyzerCancellationToken;
4545
private readonly IServiceManager _services;
46-
private readonly AsyncManualResetEvent _analysisCompleteEvent;
4746
private readonly IDiagnosticsService _diagnosticsService;
4847
private readonly IProgressReporter _progress;
4948
private readonly IPythonAnalyzer _analyzer;
@@ -68,7 +67,6 @@ public bool IsCompleted {
6867

6968
public PythonAnalyzerSession(IServiceManager services,
7069
IProgressReporter progress,
71-
AsyncManualResetEvent analysisCompleteEvent,
7270
Action<Task> startNextSession,
7371
CancellationToken analyzerCancellationToken,
7472
IDependencyChainWalker<AnalysisModuleKey, PythonAnalyzerEntry> walker,
@@ -77,7 +75,6 @@ public PythonAnalyzerSession(IServiceManager services,
7775
bool forceGC = false) {
7876

7977
_services = services;
80-
_analysisCompleteEvent = analysisCompleteEvent;
8178
_startNextSession = startNextSession;
8279
_analyzerCancellationToken = analyzerCancellationToken;
8380
Version = version;

‎src/Analysis/Ast/Impl/Caching/CacheFolders.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
using Microsoft.Python.Core.OS;
2424

2525
namespace Microsoft.Python.Analysis.Caching {
26-
internal sealed class CacheFolderService: ICacheFolderService {
26+
internal sealed class CacheFolderService : ICacheFolderService {
2727
public CacheFolderService(IServiceContainer services, string cacheRootFolder) {
2828
CacheFolder = cacheRootFolder ?? GetCacheFolder(services);
2929
}
@@ -34,7 +34,7 @@ public string GetFileNameFromContent(string content) {
3434
// File name depends on the content so we can distinguish between different versions.
3535
using (var hash = SHA256.Create()) {
3636
return Convert
37-
.ToBase64String(hash.ComputeHash(new UTF8Encoding(false).GetBytes(content)))
37+
.ToBase64String(hash.ComputeHash(new UTF8Encoding(encoderShouldEmitUTF8Identifier: false).GetBytes(content)))
3838
.Replace('/', '_').Replace('+', '-');
3939
}
4040
}
@@ -47,18 +47,18 @@ private static string GetCacheFolder(IServiceContainer services) {
4747
var localAppData = Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData);
4848
var plsSubfolder = $"Microsoft{Path.DirectorySeparatorChar}Python Language Server";
4949
var defaultCachePath = Path.Combine(localAppData, plsSubfolder);
50-
50+
5151
string cachePath = null;
5252
try {
5353
const string homeVarName = "HOME";
5454
var homeFolderPath = Environment.GetEnvironmentVariable(homeVarName);
5555

56-
if(platform.IsWindows) {
56+
if (platform.IsWindows) {
5757
cachePath = defaultCachePath;
5858
}
5959

6060
if (platform.IsMac) {
61-
if (CheckVariableSet(homeVarName, homeFolderPath, logger)
61+
if (CheckVariableSet(homeVarName, homeFolderPath, logger)
6262
&& CheckPathRooted(homeVarName, homeFolderPath, logger)
6363
&& !string.IsNullOrWhiteSpace(homeFolderPath)) {
6464
cachePath = Path.Combine(homeFolderPath, "Library/Caches", plsSubfolder);

‎src/Analysis/Ast/Impl/Diagnostics/DiagnosticsSeverityMap.cs

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,18 +25,17 @@ public DiagnosticsSeverityMap() { }
2525

2626
public DiagnosticsSeverityMap(string[] errors, string[] warnings, string[] information, string[] disabled) {
2727
_map.Clear();
28+
2829
// disabled > error > warning > information
29-
foreach (var x in information.MaybeEnumerate()) {
30-
_map[x] = Severity.Information;
31-
}
32-
foreach (var x in warnings.MaybeEnumerate()) {
33-
_map[x] = Severity.Warning;
34-
}
35-
foreach (var x in errors.MaybeEnumerate()) {
36-
_map[x] = Severity.Error;
37-
}
38-
foreach (var x in disabled.MaybeEnumerate()) {
39-
_map[x] = Severity.Suppressed;
30+
PopulateMap(information, Severity.Information);
31+
PopulateMap(warnings, Severity.Warning);
32+
PopulateMap(errors, Severity.Error);
33+
PopulateMap(disabled, Severity.Suppressed);
34+
35+
void PopulateMap(string[] codes, Severity severity) {
36+
foreach (var code in codes.MaybeEnumerate()) {
37+
_map[code] = severity;
38+
}
4039
}
4140
}
4241
public Severity GetEffectiveSeverity(string code, Severity defaultSeverity)

‎src/Analysis/Ast/Impl/Modules/Definitions/IModuleManagement.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
// permissions and limitations under the License.
1515

1616
using System;
17-
using System.Collections.Generic;
1817
using Microsoft.Python.Analysis.Caching;
1918
using Microsoft.Python.Analysis.Core.Interpreter;
2019
using Microsoft.Python.Analysis.Types;

‎src/Analysis/Ast/Impl/Modules/Resolution/ModuleResolutionBase.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ protected ModuleResolutionBase(string root, IServiceContainer services) {
6262

6363
protected abstract IPythonModule CreateModule(string name);
6464

65-
public IPythonModule GetImportedModule(string name)
65+
public IPythonModule GetImportedModule(string name)
6666
=> Modules.TryGetValue(name, out var moduleRef) ? moduleRef.Value : Interpreter.ModuleResolution.GetSpecializedModule(name);
6767

6868
public IPythonModule GetOrLoadModule(string name) {
@@ -104,7 +104,7 @@ public ModulePath FindModule(string filePath) {
104104
protected void ReloadModulePaths(in IEnumerable<string> rootPaths) {
105105
foreach (var root in rootPaths) {
106106
foreach (var moduleFile in PathUtils.EnumerateFiles(FileSystem, root)) {
107-
PathResolver.TryAddModulePath(moduleFile.FullName, moduleFile.Length, false, out _);
107+
PathResolver.TryAddModulePath(moduleFile.FullName, moduleFile.Length, allowNonRooted: false, out _);
108108
}
109109

110110
if (PathUtils.TryGetZipFilePath(root, out var zipFilePath, out var _) && File.Exists(zipFilePath)) {
@@ -113,7 +113,7 @@ protected void ReloadModulePaths(in IEnumerable<string> rootPaths) {
113113
PathResolver.TryAddModulePath(
114114
Path.Combine(zipFilePath,
115115
PathUtils.NormalizePath(moduleFile.FullName)),
116-
moduleFile.Length, false, out _
116+
moduleFile.Length, allowNonRooted: false, out _
117117
);
118118
}
119119
}

‎src/Analysis/Core/Impl/Interpreter/InterpreterConfiguration.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,7 @@ internal InterpreterConfiguration(
3131
string libPath = null,
3232
string sitePackagesPath = null,
3333
InterpreterArchitecture architecture = default,
34-
Version version = null
35-
) {
34+
Version version = null) {
3635
InterpreterPath = interpreterPath;
3736
PathEnvironmentVariable = pathVar;
3837
Architecture = architecture ?? InterpreterArchitecture.Unknown;

‎src/Core/Impl/Collections/ImmutableArray.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,10 @@ public ImmutableArray<T> Add(T item) {
9191
var newItems = _items;
9292
var newRef = _ref;
9393

94+
// _ref indicates whether the array "_items" can be re-used when creating new ImmutableArray.
95+
// this is an optimization to reduce array allocation while new elements are kept added at the end of the list
96+
// this is an alternative design compared to Builder model
97+
// (https://docs.microsoft.com/en-us/dotnet/api/system.collections.immutable.immutablearray-1.builder)
9498
if (Interlocked.CompareExchange(ref newRef.Count, newCount, _count) != _count || newCount > _items.Length) {
9599
var capacity = GetCapacity(newCount);
96100
newItems = new T[capacity];

‎src/Core/Impl/Extensions/TaskExtensions.cs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,16 @@ private static void DoNotWaitSynchronizationContextContinuation(Task task, objec
105105
/// </summary>
106106
public static T WaitAndUnwrapExceptions<T>(this Task<T> task) => task.GetAwaiter().GetResult();
107107

108+
/// <summary>
109+
/// Attach new <see cref="CancellationToken" /> to the given task.
110+
///
111+
/// this allows caller to have its own cancellation without aborting underlying work.
112+
///
113+
/// if <paramref name="task"/> uses different cancellation token than one given <paramref name="cancellationToken"/>
114+
/// it will throw <see cref="AggregateException" /> instead of <see cref="OperationCanceledException" /> and
115+
/// Task will be set to faulted rather than cancelled.
116+
/// </summary>
108117
public static Task<T> WaitAsync<T>(this Task<T> task, CancellationToken cancellationToken)
109-
=> task.ContinueWith(t => t.GetAwaiter().GetResult(), cancellationToken, TaskContinuationOptions.None, TaskScheduler.Default);
118+
=> task.ContinueWith(t => t.WaitAndUnwrapExceptions(), cancellationToken, TaskContinuationOptions.None, TaskScheduler.Default);
110119
}
111120
}

‎src/Core/Impl/Services/IdleTimeService.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,15 @@
1919

2020
namespace Microsoft.Python.Core.Services {
2121
public sealed class IdleTimeService : IIdleTimeService, IIdleTimeTracker, IDisposable {
22+
private static readonly TimeSpan InitialDelay = TimeSpan.FromMilliseconds(50);
23+
private static readonly TimeSpan Interval = TimeSpan.FromMilliseconds(50);
24+
private static readonly TimeSpan IdleInterval = TimeSpan.FromMilliseconds(100);
25+
2226
private Timer _timer;
2327
private DateTime _lastActivityTime;
2428

2529
public IdleTimeService() {
26-
_timer = new Timer(OnTimer, this, 50, 50);
30+
_timer = new Timer(OnTimer, this, InitialDelay, Interval);
2731
NotifyUserActivity();
2832
}
2933

@@ -32,11 +36,12 @@ public IdleTimeService() {
3236
public void Dispose() {
3337
_timer?.Dispose();
3438
_timer = null;
39+
3540
Closing?.Invoke(this, EventArgs.Empty);
3641
}
3742

3843
private void OnTimer(object state) {
39-
if ((DateTime.Now - _lastActivityTime).TotalMilliseconds >= 100 && _timer != null) {
44+
if (_timer != null && (DateTime.Now - _lastActivityTime) >= IdleInterval) {
4045
Idle?.Invoke(this, EventArgs.Empty);
4146
}
4247
}

‎src/Core/Impl/Threading/SingleThreadSynchronizationContext.cs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020

2121
namespace Microsoft.Python.Core.Threading {
2222
public class SingleThreadSynchronizationContext : SynchronizationContext, IDisposable {
23-
private readonly ConcurrentQueue<Tuple<SendOrPostCallback, object>> _queue = new ConcurrentQueue<Tuple<SendOrPostCallback, object>>();
23+
private readonly ConcurrentQueue<(SendOrPostCallback callback, object state)> _queue = new ConcurrentQueue<(SendOrPostCallback, object)>();
2424
private readonly ManualResetEventSlim _workAvailable = new ManualResetEventSlim(false);
2525
private readonly CancellationTokenSource _cts = new CancellationTokenSource();
2626

@@ -29,7 +29,7 @@ public SingleThreadSynchronizationContext() {
2929
}
3030

3131
public override void Post(SendOrPostCallback d, object state) {
32-
_queue.Enqueue(new Tuple<SendOrPostCallback, object>(d, state));
32+
_queue.Enqueue((d, state));
3333
_workAvailable.Set();
3434
}
3535

@@ -41,9 +41,10 @@ private void QueueWorker() {
4141
if (_cts.IsCancellationRequested) {
4242
break;
4343
}
44-
while (_queue.TryDequeue(out var t)) {
45-
t.Item1(t.Item2);
44+
while (_queue.TryDequeue(out var entry)) {
45+
entry.callback(entry.state);
4646
}
47+
4748
_workAvailable.Reset();
4849
}
4950
}

0 commit comments

Comments
 (0)