-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove portions of solution crawler #72147
Changes from all commits
a535fdc
830e2b7
c4596d3
4c46f5c
8647ca8
4b5c783
7b45542
5fb5e96
ec03cb4
f6cabc2
17853a7
e3ec62d
0f3ad82
e818095
ef93f8d
30d76b3
709962e
9c11024
6e52853
12fc4bc
420c78c
ed9fb8c
4c5349c
a053be1
5d6d2b8
112d74b
93b93d6
15db9e3
8666516
582e695
1b28bee
f56eaa9
585a5b6
67060c5
2252204
a8e2c23
05ab279
f7f25cc
2767598
bd63986
0dc2b86
4204ff4
ae1739c
f8ca295
9661564
9e04fef
c287b3e
bde023f
3b327a0
2d7ac01
38bd0a1
d66d299
9036c09
78b323a
a1f85ba
bc88023
e1c6a04
83457e8
a1dee3c
1b2bc2b
4410145
76888cf
f74513b
ec1df0a
c0788ff
ff22e7c
ef6b8fa
90f1598
65c70ce
7a11784
207b22c
b79e96c
fcdff66
7ba8f61
4392802
825de4e
cd8f428
872c92e
e6dff02
a8682ee
075969a
0800057
211d963
ea39b88
e758b15
9b1cd8d
a1b8045
28cd397
88d00a9
ab895d9
bbd3f8f
b07bb3b
7886069
90a7a40
679cba6
19e44f3
879505f
3c2ad1a
7a753d4
4235f61
e4a0ec7
2e702fd
ffdaa85
0f74f3a
63183a7
198dfeb
9b013fc
55dc44b
af3d72f
47f87c9
7b35feb
7034e9e
2b1feb7
8a22cec
6ee97bd
e01ce89
e288593
971e5ae
21ecb35
15e39bc
8116f2f
3d989c0
fc3d1d3
3fff34c
c229cc8
02a6cb2
6ad16ed
a634166
f0a4e32
8625198
362a41c
ba5f0a6
bf3639d
f12a7d3
aad4cc3
beed166
bcf7ade
dc64a31
60e8c25
d11dad9
0cd3a33
8529020
8bb1727
ffce41b
ee46be3
8406b40
83db397
271dfe6
3736dbd
46974be
01fedd9
9e6fa99
bb72c10
e779dfa
d98087b
fee50ab
53c75fe
a0b59c3
bd0d760
a353904
7f5ce9f
5c178fc
ae88cc3
b79e41b
bf6a73e
1088b1e
9c55f85
29a9085
e26a4fa
69580e2
ccc064c
b180a57
7ecbd9a
d16f3fa
afa4b65
e8c5229
9215881
86c8455
7ab7f73
8e98c2a
1d735b3
2786a90
28ff753
48f07c5
99fbc9c
e4a8977
8df1119
f8ca81e
380ca90
8a0d58b
5d1b92c
f2acaad
ecad828
639a2a8
5d0afaf
fbc68c2
1bbcfc6
beeaf7a
292244a
a612d43
48281e1
9a4219a
201cc17
0ba50f4
b8a4252
d67d9b8
3919c90
d80f180
108ad64
2f8e4e3
615fc8e
26a63e2
50b59b6
5859b29
5bcb0c9
cd055f2
e023df2
6ed1ed6
07de218
6c00395
8bb165e
71e22da
6f8be3a
d5b3e94
908bab3
9e7686c
83b1946
5b866f3
d53b35d
02077d1
8018bfd
f9899b3
19799aa
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 |
---|---|---|
|
@@ -3,8 +3,6 @@ | |
// See the LICENSE file in the project root for more information. | ||
|
||
using Microsoft.CodeAnalysis.Host; | ||
using Microsoft.CodeAnalysis.Options; | ||
using Microsoft.CodeAnalysis.SolutionCrawler; | ||
using Microsoft.CodeAnalysis.Text; | ||
using Microsoft.VisualStudio.Text; | ||
using Roslyn.Utilities; | ||
|
@@ -16,22 +14,9 @@ internal partial class InteractiveWorkspace : Workspace | |
private SourceTextContainer? _openTextContainer; | ||
private DocumentId? _openDocumentId; | ||
|
||
internal InteractiveWorkspace(HostServices hostServices, IGlobalOptionService globalOptions) | ||
internal InteractiveWorkspace(HostServices hostServices) | ||
: base(hostServices, WorkspaceKind.Interactive) | ||
{ | ||
// register work coordinator for this workspace | ||
if (globalOptions.GetOption(SolutionCrawlerRegistrationService.EnableSolutionCrawler)) | ||
{ | ||
Services.GetRequiredService<ISolutionCrawlerRegistrationService>().Register(this); | ||
} | ||
} | ||
|
||
protected override void Dispose(bool finalize) | ||
{ | ||
// workspace is going away. unregister this workspace from work coordinator | ||
Services.GetRequiredService<ISolutionCrawlerRegistrationService>().Unregister(this, blockingShutdown: true); | ||
|
||
base.Dispose(finalize); | ||
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. no solution crawler, so workspaces are now decoupled from the deeply embedded concept. |
||
} | ||
|
||
public override bool CanOpenDocuments | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -701,12 +701,6 @@ private async ValueTask<IDifferenceViewerPreview<TDifferenceViewer>> CreateNewDi | |
rightWorkspace = null; | ||
}; | ||
|
||
if (_editorOptionsService.GlobalOptions.GetOption(SolutionCrawlerRegistrationService.EnableSolutionCrawler)) | ||
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. note: this was not a user visible option. We had it because i guess we just like options. Note: if this was ever something turned off it woudl break all diagnostics as well as unit testing. As an unsupported and undocumented internal option, it has just been removed. 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 was primarily added to allow us to turn off solution crawler in RPS. |
||
{ | ||
leftWorkspace?.Target.EnableSolutionCrawler(); | ||
rightWorkspace?.Target.EnableSolutionCrawler(); | ||
} | ||
|
||
return CreateDifferenceViewerPreview(diffViewer); | ||
} | ||
|
||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,11 @@ | |
|
||
namespace Microsoft.CodeAnalysis.LegacySolutionEvents; | ||
|
||
/// <summary> | ||
/// Event listener that hears about workspaces and exists solely to let unit testing continue to work using their own | ||
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.
Is it possible to move this type to test project? 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. Sorry. By "unit testing" I don't mean "our unit tests". I mean "the unit testing team". They have a crawler to discover tests. But it's extremely trimmed down, and owned by them. |
||
/// fork of solution crawler. Importantly, this is always active until the point that we can get unit testing to move | ||
/// to an entirely differently (ideally 'pull') model for test discovery. | ||
/// </summary> | ||
[ExportEventListener(WellKnownEventListeners.Workspace, WorkspaceKind.Host), Shared] | ||
internal sealed partial class HostLegacySolutionEventsWorkspaceEventListener : IEventListener<object> | ||
{ | ||
|
@@ -44,6 +49,8 @@ public HostLegacySolutionEventsWorkspaceEventListener( | |
|
||
public void StartListening(Workspace workspace, object? serviceOpt) | ||
{ | ||
// We only support this option to disable crawling in internal perf runs to lower noise. It is not exposed to | ||
// the user. | ||
if (_globalOptions.GetOption(SolutionCrawlerRegistrationService.EnableSolutionCrawler)) | ||
{ | ||
workspace.WorkspaceChanged += OnWorkspaceChanged; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,11 +59,6 @@ public async Task TestGetFirstDiagnosticWithFixAsync() | |
var fixService = new CodeFixService( | ||
diagnosticService, logger, fixers, SpecializedCollections.EmptyEnumerable<Lazy<IConfigurationFixProvider, CodeChangeProviderMetadata>>()); | ||
|
||
var incrementalAnalyzer = (IIncrementalAnalyzerProvider)diagnosticService; | ||
|
||
// register diagnostic engine to solution crawler | ||
var analyzer = incrementalAnalyzer.CreateIncrementalAnalyzer(workspace); | ||
|
||
var reference = new MockAnalyzerReference(); | ||
var project = workspace.CurrentSolution.Projects.Single().AddAnalyzerReference(reference); | ||
var document = project.Documents.Single(); | ||
|
@@ -302,8 +297,6 @@ private static async Task<ImmutableArray<CodeFixCollection>> GetAddedFixesAsync( | |
errorReportingService.OnError = message => errorReported = true; | ||
|
||
GetDocumentAndExtensionManager(tuple.analyzerService, workspace, out var document, out var extensionManager); | ||
var incrementalAnalyzer = (IIncrementalAnalyzerProvider)tuple.analyzerService; | ||
var analyzer = incrementalAnalyzer.CreateIncrementalAnalyzer(workspace); | ||
var reference = new MockAnalyzerReference(codefix, ImmutableArray.Create(diagnosticAnalyzer)); | ||
var project = workspace.CurrentSolution.Projects.Single().AddAnalyzerReference(reference); | ||
document = project.Documents.Single(); | ||
|
@@ -408,10 +401,8 @@ private static void GetDocumentAndExtensionManager( | |
MockAnalyzerReference? analyzerReference = null, | ||
TextDocumentKind documentKind = TextDocumentKind.Document) | ||
{ | ||
var incrementalAnalyzer = (IIncrementalAnalyzerProvider)diagnosticService; | ||
|
||
// register diagnostic engine to solution crawler | ||
diagnosticIncrementalAnalyzer = (DiagnosticIncrementalAnalyzer)incrementalAnalyzer.CreateIncrementalAnalyzer(workspace)!; | ||
diagnosticIncrementalAnalyzer = diagnosticService.CreateIncrementalAnalyzer(workspace)!; | ||
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. the core interfaces went away, so this became strongly typed. teh tests just test calling through teh actual DiagnosticService and DiagnosticIncrementalAnalyzers now. |
||
|
||
var reference = analyzerReference ?? new MockAnalyzerReference(); | ||
var project = workspace.CurrentSolution.Projects.Single().AddAnalyzerReference(reference); | ||
|
@@ -794,11 +785,6 @@ private static async Task<ImmutableArray<CodeFixCollection>> GetNuGetAndVsixCode | |
var fixService = new CodeFixService( | ||
diagnosticService, logger, vsixFixers, SpecializedCollections.EmptyEnumerable<Lazy<IConfigurationFixProvider, CodeChangeProviderMetadata>>()); | ||
|
||
var incrementalAnalyzer = (IIncrementalAnalyzerProvider)diagnosticService; | ||
|
||
// register diagnostic engine to solution crawler | ||
var analyzer = incrementalAnalyzer.CreateIncrementalAnalyzer(workspace); | ||
|
||
diagnosticAnalyzer ??= new MockAnalyzerReference.MockDiagnosticAnalyzer(); | ||
var analyzers = ImmutableArray.Create<DiagnosticAnalyzer>(diagnosticAnalyzer); | ||
var reference = new MockAnalyzerReference(nugetFixer, analyzers); | ||
|
@@ -1022,7 +1008,6 @@ public AdditionalFileFixerWithoutDocumentKindsAndExtensions() : base(nameof(Addi | |
[Theory, CombinatorialData] | ||
public async Task TestGetFixesWithDeprioritizedAnalyzerAsync( | ||
DeprioritizedAnalyzer.ActionKind actionKind, | ||
bool testWithCachedDiagnostics, | ||
bool diagnosticOnFixLineInPriorSnapshot, | ||
bool editOnFixLine, | ||
bool addNewLineWithEdit) | ||
|
@@ -1042,7 +1027,7 @@ public async Task TestGetFixesWithDeprioritizedAnalyzerAsync( | |
// of the heuristic where we compare intersecting diagnostics across document snapshots only if both | ||
// snapshots have the same number of lines. | ||
|
||
var expectDeprioritization = GetExpectDeprioritization(actionKind, testWithCachedDiagnostics, diagnosticOnFixLineInPriorSnapshot, addNewLineWithEdit); | ||
var expectDeprioritization = GetExpectDeprioritization(actionKind, diagnosticOnFixLineInPriorSnapshot, addNewLineWithEdit); | ||
|
||
var priorSnapshotFixLine = diagnosticOnFixLineInPriorSnapshot ? "int x1 = 0;" : "System.Console.WriteLine();"; | ||
var code = $@" | ||
|
@@ -1073,7 +1058,8 @@ void M() | |
// Trigger background analysis to ensure analyzer diagnostics are computed and cached. | ||
// We enable full solution analysis so the 'AnalyzeDocumentAsync' doesn't skip analysis based on whether the document is active/open. | ||
workspace.GlobalOptions.SetGlobalOption(SolutionCrawlerOptionsStorage.BackgroundAnalysisScopeOption, LanguageNames.CSharp, BackgroundAnalysisScope.FullSolution); | ||
await diagnosticIncrementalAnalyzer.AnalyzeDocumentAsync(sourceDocument, bodyOpt: null!, InvocationReasons.DocumentChanged, CancellationToken.None); | ||
|
||
await diagnosticIncrementalAnalyzer.ForceAnalyzeProjectAsync(sourceDocument.Project, CancellationToken.None); | ||
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. AnalzyeDocument went away, but we stilll have ForceAnalyzeProject (called when the user invokes "Run code analysis on project). So these tests could remain, just using that entrypoint to help simulate the behavior here. |
||
await VerifyCachedDiagnosticsAsync(sourceDocument, expectedCachedDiagnostic: diagnosticOnFixLineInPriorSnapshot, testSpan, diagnosticIncrementalAnalyzer); | ||
|
||
// Compute and apply code edit | ||
|
@@ -1103,12 +1089,9 @@ void M() | |
? root.DescendantNodes().OfType<CodeAnalysis.CSharp.Syntax.VariableDeclarationSyntax>().First().Span | ||
: root.DescendantNodes().OfType<CodeAnalysis.CSharp.Syntax.InvocationExpressionSyntax>().First().Span; | ||
|
||
if (testWithCachedDiagnostics) | ||
{ | ||
// Trigger background analysis to ensure analyzer diagnostics are computed and cached. | ||
await diagnosticIncrementalAnalyzer.AnalyzeDocumentAsync(sourceDocument, bodyOpt: null!, InvocationReasons.DocumentChanged, CancellationToken.None); | ||
CyrusNajmabadi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
await VerifyCachedDiagnosticsAsync(sourceDocument, expectedCachedDiagnostic: !expectedNoFixes, testSpan, diagnosticIncrementalAnalyzer); | ||
} | ||
await diagnosticIncrementalAnalyzer.GetDiagnosticsAsync( | ||
sourceDocument.Project.Solution, sourceDocument.Project.Id, sourceDocument.Id, includeSuppressedDiagnostics: true, includeNonLocalDocumentDiagnostics: true, CancellationToken.None); | ||
await diagnosticIncrementalAnalyzer.GetTestAccessor().TextDocumentOpenAsync(sourceDocument); | ||
|
||
var lowPriorityAnalyzers = new ConcurrentSet<DiagnosticAnalyzer>(); | ||
var priorityProvider = new SuggestedActionPriorityProvider(CodeActionRequestPriority.Default, lowPriorityAnalyzers); | ||
|
@@ -1144,31 +1127,25 @@ void M() | |
|
||
static bool GetExpectDeprioritization( | ||
DeprioritizedAnalyzer.ActionKind actionKind, | ||
bool testWithCachedDiagnostics, | ||
bool diagnosticOnFixLineInPriorSnapshot, | ||
bool addNewLineWithEdit) | ||
{ | ||
// We expect de-prioritization of analyzer from 'Normal' to 'Low' bucket only if following conditions are met: | ||
// 1. There are no cached diagnostics from background analysis, i.e. 'testWithCachedDiagnostics == false'. | ||
// 2. We have an expensive analyzer that registers SymbolStart/End or SemanticModel actions, both of which have a broad analysis scope. | ||
// 3. Either of the below is true: | ||
// 1. We have an expensive analyzer that registers SymbolStart/End or SemanticModel actions, both of which have a broad analysis scope. | ||
// 2. Either of the below is true: | ||
// a. We do not have an analyzer diagnostic reported in the prior document snapshot on the edited line OR | ||
// b. Number of lines in the prior document snapshot differs from number of lines in the current document snapshot, | ||
// i.e. we added a new line with the edit and 'addNewLineWithEdit = true'. | ||
|
||
// Condition 1 | ||
if (testWithCachedDiagnostics) | ||
return false; | ||
|
||
// Condition 2 | ||
if (actionKind is not (DeprioritizedAnalyzer.ActionKind.SymbolStartEnd or DeprioritizedAnalyzer.ActionKind.SemanticModel)) | ||
return false; | ||
|
||
// Condition 3(a) | ||
// Condition 2(a) | ||
if (!diagnosticOnFixLineInPriorSnapshot) | ||
return true; | ||
|
||
// Condition 3(b) | ||
// Condition 2(b) | ||
return addNewLineWithEdit; | ||
} | ||
|
||
|
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.
a lot of these calls were bogus. they were effectively trying to simulate the solution-crawler starting up and creating the incremental analyzer for a workspace.
we cannot depend on solution crawler for that, so now the underlying system ensures that the incremental analyzers are created on demand.