Skip to content
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

support semantic errors for script files in misc projects. #31134

Merged
merged 5 commits into from
Dec 6, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
464 changes: 464 additions & 0 deletions src/Features/Core/Portable/Diagnostics/AnalyzerHelper.cs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Composition;
using System.Diagnostics;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Options;
Expand All @@ -18,12 +18,14 @@ namespace Microsoft.CodeAnalysis.Diagnostics
[ExportIncrementalAnalyzerProvider(WellKnownSolutionCrawlerAnalyzers.Diagnostic, workspaceKinds: null)]
internal partial class DefaultDiagnosticAnalyzerService : IIncrementalAnalyzerProvider, IDiagnosticUpdateSource
{
private const int Syntax = 1;
private const int Semantic = 2;
private readonly IDiagnosticAnalyzerService _analyzerService;

[ImportingConstructor]
public DefaultDiagnosticAnalyzerService(IDiagnosticUpdateSourceRegistrationService registrationService)
public DefaultDiagnosticAnalyzerService(
IDiagnosticAnalyzerService analyzerService,
IDiagnosticUpdateSourceRegistrationService registrationService)
{
_analyzerService = analyzerService;
registrationService.Register(this);
}

Expand All @@ -34,14 +36,13 @@ public IIncrementalAnalyzer CreateIncrementalAnalyzer(Workspace workspace)
return null;
}

return new CompilerDiagnosticAnalyzer(this, workspace);
return new DefaultDiagnosticIncrementalAnalyzer(this, workspace);
}

public event EventHandler<DiagnosticsUpdatedArgs> DiagnosticsUpdated;

public bool SupportGetDiagnostics =>
// this only support push model, pull model will be provided by DiagnosticService by caching everything this one pushed
false;
// this only support push model, pull model will be provided by DiagnosticService by caching everything this one pushed
public bool SupportGetDiagnostics => false;

public ImmutableArray<DiagnosticData> GetDiagnostics(Workspace workspace, ProjectId projectId, DocumentId documentId, object id, bool includeSuppressedDiagnostics = false, CancellationToken cancellationToken = default)
{
Expand All @@ -54,12 +55,12 @@ internal void RaiseDiagnosticsUpdated(DiagnosticsUpdatedArgs state)
this.DiagnosticsUpdated?.Invoke(this, state);
}

private class CompilerDiagnosticAnalyzer : IIncrementalAnalyzer
private class DefaultDiagnosticIncrementalAnalyzer : IIncrementalAnalyzer
{
private readonly DefaultDiagnosticAnalyzerService _service;
private readonly Workspace _workspace;

public CompilerDiagnosticAnalyzer(DefaultDiagnosticAnalyzerService service, Workspace workspace)
public DefaultDiagnosticIncrementalAnalyzer(DefaultDiagnosticAnalyzerService service, Workspace workspace)
{
_service = service;
_workspace = workspace;
Expand All @@ -68,7 +69,8 @@ public CompilerDiagnosticAnalyzer(DefaultDiagnosticAnalyzerService service, Work
public bool NeedsReanalysisOnOptionChanged(object sender, OptionChangedEventArgs e)
{
if (e.Option == InternalRuntimeDiagnosticOptions.Syntax ||
e.Option == InternalRuntimeDiagnosticOptions.Semantic)
e.Option == InternalRuntimeDiagnosticOptions.Semantic ||
e.Option == InternalRuntimeDiagnosticOptions.ScriptSemantic)
{
return true;
}
Expand All @@ -78,53 +80,73 @@ public bool NeedsReanalysisOnOptionChanged(object sender, OptionChangedEventArgs

public async Task AnalyzeSyntaxAsync(Document document, InvocationReasons reasons, CancellationToken cancellationToken)
{
Debug.Assert(document.Project.Solution.Workspace == _workspace);

// right now, there is no way to observe diagnostics for closed file.
if (!_workspace.IsDocumentOpen(document.Id) ||
!_workspace.Options.GetOption(InternalRuntimeDiagnosticOptions.Syntax) ||
!document.SupportsSyntaxTree)
!_workspace.Options.GetOption(InternalRuntimeDiagnosticOptions.Syntax))
{
return;
}

var tree = await document.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(false);
var diagnostics = tree.GetDiagnostics(cancellationToken);

Debug.Assert(document.Project.Solution.Workspace == _workspace);

var diagnosticData = diagnostics == null ? ImmutableArray<DiagnosticData>.Empty : diagnostics.Select(d => DiagnosticData.Create(document, d)).ToImmutableArrayOrEmpty();

_service.RaiseDiagnosticsUpdated(
DiagnosticsUpdatedArgs.DiagnosticsCreated(new DefaultUpdateArgsId(_workspace.Kind, Syntax, document.Id),
_workspace, document.Project.Solution, document.Project.Id, document.Id, diagnosticData));
await AnalyzeForKind(document, AnalysisKind.Syntax, cancellationToken).ConfigureAwait(false);
}

public async Task AnalyzeDocumentAsync(Document document, SyntaxNode bodyOpt, InvocationReasons reasons, CancellationToken cancellationToken)
{
// right now, there is no way to observe diagnostics for closed file.
if (!_workspace.IsDocumentOpen(document.Id) ||
!_workspace.Options.GetOption(InternalRuntimeDiagnosticOptions.Semantic))
Debug.Assert(document.Project.Solution.Workspace == _workspace);

if (!IsSemanticAnalysisOn())
{
return;
}

var model = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
var diagnostics = model.GetMethodBodyDiagnostics(span: null, cancellationToken: cancellationToken).Concat(
model.GetDeclarationDiagnostics(span: null, cancellationToken: cancellationToken));
await AnalyzeForKind(document, AnalysisKind.Semantic, cancellationToken).ConfigureAwait(false);

Debug.Assert(document.Project.Solution.Workspace == _workspace);
bool IsSemanticAnalysisOn()
{
// right now, there is no way to observe diagnostics for closed file.
if (!_workspace.IsDocumentOpen(document.Id))
{
return false;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure i understand this. Why is semantic analysis not on for closed files? Isn't that what FSA is?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be answered?


if (_workspace.Options.GetOption(InternalRuntimeDiagnosticOptions.Semantic))
{
return true;
}

return _workspace.Options.GetOption(InternalRuntimeDiagnosticOptions.ScriptSemantic) && document.SourceCodeKind == SourceCodeKind.Script;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what are the implications of this for CSX?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be answered?

}
}

var diagnosticData = diagnostics == null ? ImmutableArray<DiagnosticData>.Empty : diagnostics.Select(d => DiagnosticData.Create(document, d)).ToImmutableArrayOrEmpty();
private async Task AnalyzeForKind(Document document, AnalysisKind kind, CancellationToken cancellationToken)
{
var diagnosticData = await _service._analyzerService.GetDiagnosticsAsync(document, GetAnalyzers(), kind, cancellationToken).ConfigureAwait(false);

_service.RaiseDiagnosticsUpdated(
DiagnosticsUpdatedArgs.DiagnosticsCreated(new DefaultUpdateArgsId(_workspace.Kind, Semantic, document.Id),
_workspace, document.Project.Solution, document.Project.Id, document.Id, diagnosticData));
DiagnosticsUpdatedArgs.DiagnosticsCreated(new DefaultUpdateArgsId(_workspace.Kind, kind, document.Id),
_workspace, document.Project.Solution, document.Project.Id, document.Id, diagnosticData.ToImmutableArrayOrEmpty()));

IEnumerable<DiagnosticAnalyzer> GetAnalyzers()
{
// C# or VB document that supports compiler
var compilerAnalyzer = _service._analyzerService.GetCompilerDiagnosticAnalyzer(document.Project.Language);
if (compilerAnalyzer != null)
{
return SpecializedCollections.SingletonEnumerable(compilerAnalyzer);
}

// document that doesn't support compiler diagnostics such as fsharp or typescript
return _service._analyzerService.GetDiagnosticAnalyzers(document.Project);
}
}

public void RemoveDocument(DocumentId documentId)
{
// a file is removed from misc project
Copy link
Member

@tmat tmat Dec 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// a file is removed from misc project [](start = 15, length = 39)

Is this comment still relevant? This shouldn't have any assumption of misc workspace, correct?

Copy link
Member

@tmat tmat Dec 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is ScriptSemantic missing here? If not I'd add a comment why.


In reply to: 238470136 [](ancestors = 238470136)

RaiseEmptyDiagnosticUpdated(Syntax, documentId);
RaiseEmptyDiagnosticUpdated(Semantic, documentId);
RaiseEmptyDiagnosticUpdated(AnalysisKind.Syntax, documentId);
RaiseEmptyDiagnosticUpdated(AnalysisKind.Semantic, documentId);
}

public Task DocumentResetAsync(Document document, CancellationToken cancellationToken)
Expand All @@ -139,7 +161,7 @@ public Task DocumentCloseAsync(Document document, CancellationToken cancellation
return DocumentResetAsync(document, cancellationToken);
}

private void RaiseEmptyDiagnosticUpdated(int kind, DocumentId documentId)
private void RaiseEmptyDiagnosticUpdated(AnalysisKind kind, DocumentId documentId)
{
_service.RaiseDiagnosticsUpdated(DiagnosticsUpdatedArgs.DiagnosticsRemoved(
new DefaultUpdateArgsId(_workspace.Kind, kind, documentId), _workspace, null, documentId.ProjectId, documentId));
Expand Down Expand Up @@ -168,7 +190,7 @@ private class DefaultUpdateArgsId : BuildToolId.Base<int, DocumentId>, ISupportL
{
private readonly string _workspaceKind;

public DefaultUpdateArgsId(string workspaceKind, int type, DocumentId documentId) : base(type, documentId)
public DefaultUpdateArgsId(string workspaceKind, AnalysisKind kind, DocumentId documentId) : base((int)kind, documentId)
{
_workspaceKind = workspaceKind;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,12 +262,12 @@ public bool ContainsDiagnostics(Workspace workspace, ProjectId projectId)
}

// virtual for testing purposes.
internal virtual Action<Exception, DiagnosticAnalyzer, Diagnostic> GetOnAnalyzerException(ProjectId projectId, DiagnosticLogAggregator diagnosticLogAggregator)
internal virtual Action<Exception, DiagnosticAnalyzer, Diagnostic> GetOnAnalyzerException(ProjectId projectId, DiagnosticLogAggregator logAggregatorOpt)
{
return (ex, analyzer, diagnostic) =>
{
// Log telemetry, if analyzer supports telemetry.
DiagnosticAnalyzerLogger.LogAnalyzerCrashCount(analyzer, ex, diagnosticLogAggregator, projectId);
DiagnosticAnalyzerLogger.LogAnalyzerCrashCount(analyzer, ex, logAggregatorOpt);

AnalyzerHelper.OnAnalyzerException_NoTelemetryLogging(ex, analyzer, diagnostic, _hostDiagnosticUpdateSource, projectId);
};
Expand Down
14 changes: 14 additions & 0 deletions src/Features/Core/Portable/Diagnostics/EngineV2/AnalysisKind.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace Microsoft.CodeAnalysis.Diagnostics
{
/// <summary>
/// enum for each analysis kind.
/// </summary>
internal enum AnalysisKind
{
Syntax,
Semantic,
NonLocal
}
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using System.Runtime.CompilerServices;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.ErrorReporting;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Diagnostics.EngineV2
Expand Down Expand Up @@ -70,79 +67,10 @@ public Task<CompilationWithAnalyzers> CreateAnalyzerDriverAsync(Project project,
return CreateAnalyzerDriverAsync(project, analyzers, includeSuppressedDiagnostics, cancellationToken);
}

public async Task<CompilationWithAnalyzers> CreateAnalyzerDriverAsync(
public Task<CompilationWithAnalyzers> CreateAnalyzerDriverAsync(
Project project, IEnumerable<DiagnosticAnalyzer> analyzers, bool includeSuppressedDiagnostics, CancellationToken cancellationToken)
{
if (!project.SupportsCompilation)
{
return null;
}

var compilation = await project.GetCompilationAsync(cancellationToken).ConfigureAwait(false);

// Create driver that holds onto compilation and associated analyzers
return CreateAnalyzerDriver(
project, compilation, analyzers, logAnalyzerExecutionTime: true, reportSuppressedDiagnostics: includeSuppressedDiagnostics);
}

private CompilationWithAnalyzers CreateAnalyzerDriver(
Project project,
Compilation compilation,
IEnumerable<DiagnosticAnalyzer> allAnalyzers,
bool logAnalyzerExecutionTime,
bool reportSuppressedDiagnostics)
{
var analyzers = allAnalyzers.Where(a => !a.IsWorkspaceDiagnosticAnalyzer()).ToImmutableArrayOrEmpty();

// PERF: there is no analyzers for this compilation.
// compilationWithAnalyzer will throw if it is created with no analyzers which is perf optimization.
if (analyzers.IsEmpty)
{
return null;
}

Contract.ThrowIfFalse(project.SupportsCompilation);
AssertCompilation(project, compilation);

var analysisOptions = GetAnalyzerOptions(project, logAnalyzerExecutionTime, reportSuppressedDiagnostics);

// Create driver that holds onto compilation and associated analyzers
return compilation.WithAnalyzers(analyzers, analysisOptions);
}

private CompilationWithAnalyzersOptions GetAnalyzerOptions(
Project project,
bool logAnalyzerExecutionTime,
bool reportSuppressedDiagnostics)
{
// in IDE, we always set concurrentAnalysis == false otherwise, we can get into thread starvation due to
// async being used with syncronous blocking concurrency.
return new CompilationWithAnalyzersOptions(
options: new WorkspaceAnalyzerOptions(project.AnalyzerOptions, project.Solution.Options, project.Solution),
onAnalyzerException: GetOnAnalyzerException(project.Id),
analyzerExceptionFilter: GetAnalyzerExceptionFilter(project),
concurrentAnalysis: false,
logAnalyzerExecutionTime: logAnalyzerExecutionTime,
reportSuppressedDiagnostics: reportSuppressedDiagnostics);
}

private Func<Exception, bool> GetAnalyzerExceptionFilter(Project project)
{
return ex =>
{
if (project.Solution.Workspace.Options.GetOption(InternalDiagnosticsOptions.CrashOnAnalyzerException))
{
// if option is on, crash the host to get crash dump.
FatalError.ReportUnlessCanceled(ex);
}

return true;
};
}

private Action<Exception, DiagnosticAnalyzer, Diagnostic> GetOnAnalyzerException(ProjectId projectId)
{
return _owner.Owner.GetOnAnalyzerException(projectId, _owner.DiagnosticLogAggregator);
return _owner.Owner.CreateAnalyzerDriverAsync(project, analyzers, includeSuppressedDiagnostics, _owner.DiagnosticLogAggregator, cancellationToken);
}

private void ResetAnalyzerDriverMap()
Expand All @@ -169,14 +97,6 @@ private void AssertAnalyzers(CompilationWithAnalyzers analyzerDriver, IEnumerabl
Contract.ThrowIfFalse(analyzerDriver.Analyzers.SetEquals(stateSets.Select(s => s.Analyzer).Where(a => !a.IsWorkspaceDiagnosticAnalyzer())));
}

[Conditional("DEBUG")]
private void AssertCompilation(Project project, Compilation compilation1)
{
// given compilation must be from given project.
Contract.ThrowIfFalse(project.TryGetCompilation(out var compilation2));
Contract.ThrowIfFalse(compilation1 == compilation2);
}

#region state changed
public void OnActiveDocumentChanged()
{
Expand Down
Loading