-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Cleanup how we compute and report file-content-load issues. #77880
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
Cleanup how we compute and report file-content-load issues. #77880
Conversation
| public static bool IsWorkspaceDiagnosticAnalyzer(this DiagnosticAnalyzer analyzer) | ||
| => analyzer is DocumentDiagnosticAnalyzer | ||
| || analyzer is ProjectDiagnosticAnalyzer | ||
| || analyzer == FileContentLoadAnalyzer.Instance |
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.
FileContentLoadAnalyzer is just a DocumentDiagnosticAnalyzer now. so we don't need this special check.
| var document = textDocument as Document; | ||
| RoslynDebug.Assert(document != null || kind == AnalysisKind.Syntax, "We only support syntactic analysis for non-source documents"); | ||
|
|
||
| var loadDiagnostic = await textDocument.State.GetLoadDiagnosticAsync(cancellationToken).ConfigureAwait(false); |
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.
computation and returning of this diagnostic is pushed into FileContentLoadAnalyzer itself, instead of having to special case it here.
| foreach (var document in project.Documents) | ||
| { | ||
| // don't analyze documents whose content failed to load | ||
| if (failedDocuments == null || !failedDocuments.Contains(document)) |
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.
we don't need to track of the content failed to load. if it failed to load, it's just an empty string, which doesn't report diagnostics anyways.
|
|
||
| private sealed class HostAnalyzerInfo | ||
| { | ||
| private const int FileContentLoadAnalyzerPriority = -4; |
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.
moved into the FileContentloadAnalyzer.
| FileContentLoadAnalyzer _ => FileContentLoadAnalyzerPriority, | ||
| GeneratorDiagnosticsPlaceholderAnalyzer _ => GeneratorDiagnosticsPlaceholderAnalyzerPriority, | ||
| DocumentDiagnosticAnalyzer analyzer => Math.Max(0, analyzer.Priority), | ||
| DocumentDiagnosticAnalyzer analyzer => analyzer.Priority, |
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.
removed as we do just want to pass the priority through. Note that FileContentLoad is still pri -4, and the rest of the doc analyzers (which we own 100%) are all just at 50 and never change.
|
@JoeRobich ptal. |
No description provided.