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

fix: Added code to catch resolver exceptions if any. #2080

Merged
merged 13 commits into from
May 10, 2022
Merged
Show file tree
Hide file tree
Changes from 10 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
2 changes: 2 additions & 0 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
- feat: The new `older` modifier on arguments to predicates indicates that a predicate ensures the allocatedness of some of its arguments. This allows more functions to show that their quantifiers do not depend on the allocation state. For more information, see the reference manual section on `older`. (https://github.com/dafny-lang/dafny/pull/1936)
- fix: Fix `(!new)` checks (https://github.com/dafny-lang/dafny/issues/1419)
- fix: multiset keyword no longer crashes the parser (https://github.com/dafny-lang/dafny/pull/2079)
- fix: Prevent the language server from crashing and not responding on resolution or ghost diagnostics errors (https://github.com/dafny-lang/dafny/pull/2080)


# 3.5.0

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
using System.Collections.Generic;
using System.Threading;
using Microsoft.Boogie;
using Microsoft.Dafny.LanguageServer.Language;
using Microsoft.Dafny.LanguageServer.Language.Symbols;
using Microsoft.Dafny.LanguageServer.Workspace;
using Microsoft.Extensions.Logging;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Moq;

namespace Microsoft.Dafny.LanguageServer.IntegrationTest.Unit;

[TestClass]
public class DafnyLangSymbolResolverTest {
private DafnyLangSymbolResolver dafnyLangSymbolResolver;

[TestInitialize]
public void SetUp() {
var loggerFactory = new Mock<ILoggerFactory>();
dafnyLangSymbolResolver = new DafnyLangSymbolResolver(
loggerFactory.Object.CreateLogger<DafnyLangSymbolResolver>()
);
}

class CollectingErrorReporter : BatchErrorReporter {
public Dictionary<ErrorLevel, List<ErrorMessage>> GetErrors() {
return this.AllMessages;
}
}

class DummyModuleDecl : LiteralModuleDecl {
public DummyModuleDecl() : base(
new DefaultModuleDecl(), null) {
}
public override object Dereference() {
return this;
}
}

[TestMethod]
public void EnsureResilienceAgainstErrors() {
// Builtins is null to trigger an error.
Copy link
Member

@keyboardDrummer keyboardDrummer May 6, 2022

Choose a reason for hiding this comment

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

It seems like this test relies on some badly defined API of DafnyPipeline.csproj, that it requires Builtins to be not null, to trigger the exception. I think it might be nicer if the test uses a more explicit API to trigger a failure, like mocking something from DafnyPipeline.csproj and letting the mock throw an Exception, but I imagine DafnyPipeline.csproj is not setup for being mocked.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes you identified this test correctly.
I don't see either how to mock the DafnyPipeline, so I took the route of the minimum working test for now.

var reporter = new CollectingErrorReporter();
var program = new Dafny.Program("dummy", new DummyModuleDecl(), null, reporter);
dafnyLangSymbolResolver.ResolveSymbols(null!, program, CancellationToken.None);
Assert.AreEqual(1, reporter.ErrorCount);
Assert.AreEqual(1, reporter.GetErrors()[ErrorLevel.Error].Count);
var expected = "Dafny encountered an error. Please report it at ";
Assert.AreEqual(expected, reporter.GetErrors()[ErrorLevel.Error][0].message.Substring(0, expected.Length));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using IntervalTree;
using Microsoft.Boogie;
using Microsoft.Dafny.LanguageServer.Language;
using Microsoft.Dafny.LanguageServer.Language.Symbols;
using Microsoft.Dafny.LanguageServer.Workspace;
using Microsoft.Dafny.LanguageServer.Workspace.Notifications;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Moq;
using OmniSharp.Extensions.LanguageServer.Protocol.Models;

namespace Microsoft.Dafny.LanguageServer.IntegrationTest.Unit;

[TestClass]
public class GhostStateDiagnosticCollectorTest {
private GhostStateDiagnosticCollector ghostStateDiagnosticCollector;

class DummyOptions : IOptions<GhostOptions> {
public GhostOptions Value { get; set; }
}

class DummyLogger : ILogger<GhostStateDiagnosticCollector> {
public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter) {
// Do nothing
}

public bool IsEnabled(LogLevel logLevel) {
return true;
}

public IDisposable BeginScope<TState>(TState state) {
return null;
}
}

[TestInitialize]
public void SetUp() {
var loggerFactory = new Mock<ILoggerFactory>();
var options = new DummyOptions {
Value = new Mock<GhostOptions>().Object
};
options.Value.MarkStatements = true;
ghostStateDiagnosticCollector = new GhostStateDiagnosticCollector(
options,
new DummyLogger());
}

class CollectingErrorReporter : BatchErrorReporter {
public Dictionary<ErrorLevel, List<ErrorMessage>> GetErrors() {
return this.AllMessages;
}
}

class DummyModuleDecl : LiteralModuleDecl {
public DummyModuleDecl() : base(
new DefaultModuleDecl(), null) {
}
public override object Dereference() {
return this;
}
}

[TestMethod]
public void EnsureResilienceAgainstErrors() {
// Builtins is null to trigger an error.
var reporter = new CollectingErrorReporter();
var program = new Dafny.Program("dummy", new DummyModuleDecl(), null, reporter);
var ghostDiagnostics = ghostStateDiagnosticCollector.GetGhostStateDiagnostics(
new SymbolTable(null!, new CompilationUnit(program), null!, null!, new IntervalTree<Position, ILocalizableSymbol>(), true)
, CancellationToken.None);
Assert.AreEqual(0, ghostDiagnostics.Count());
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
using Microsoft.Boogie;
using System;
using Microsoft.Boogie;
using Microsoft.Dafny.LanguageServer.Language.Symbols;
using Microsoft.Dafny.LanguageServer.Util;
using Microsoft.Extensions.Options;
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using Microsoft.Extensions.Logging;
using Range = OmniSharp.Extensions.LanguageServer.Protocol.Models.Range;

namespace Microsoft.Dafny.LanguageServer.Language {
/// <summary>
Expand All @@ -20,18 +23,25 @@ public class GhostStateDiagnosticCollector : IGhostStateDiagnosticCollector {
private const string GhostStatementMessage = "Ghost statement";

private readonly GhostOptions options;

public GhostStateDiagnosticCollector(IOptions<GhostOptions> options) {
private readonly ILogger<GhostStateDiagnosticCollector> logger;
public GhostStateDiagnosticCollector(IOptions<GhostOptions> options, ILogger<GhostStateDiagnosticCollector> logger) {
this.logger = logger;
this.options = options.Value;
}

public IEnumerable<Diagnostic> GetGhostStateDiagnostics(SymbolTable symbolTable, CancellationToken cancellationToken) {
if (!options.MarkStatements) {
return Enumerable.Empty<Diagnostic>();
}
var visitor = new GhostStateSyntaxTreeVisitor(symbolTable.CompilationUnit.Program, cancellationToken);
visitor.Visit(symbolTable.CompilationUnit.Program);
return visitor.GhostDiagnostics;

try {
var visitor = new GhostStateSyntaxTreeVisitor(symbolTable.CompilationUnit.Program, cancellationToken);
visitor.Visit(symbolTable.CompilationUnit.Program);
return visitor.GhostDiagnostics;
} catch (Exception e) {
logger.LogDebug(e, "encountered an exception while getting ghost state diagnostics of {Name}", symbolTable.CompilationUnit.Name);
Copy link
Member

Choose a reason for hiding this comment

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

This will not be visible to users, right? Aren't these logs only written to disk in a somewhat hidden location?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes they are written next to DafnyLanguageServer.dll, and only if an option is activated.
But this kind of error can happen if the language server recovered from problematic resolving, and leaving the program in a state that the Ghost Diagnostic Collector is not assuming, so that's why it's less important.

return new Diagnostic[] { };
}
}

private class GhostStateSyntaxTreeVisitor : SyntaxTreeVisitor {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ public CompilationUnit ResolveSymbols(TextDocumentItem textDocument, Dafny.Progr
// because it must not be null.
return new CompilationUnit(program);
}
} catch (Exception e) {
program.reporter.Error(MessageSource.Resolver, program.GetFirstTopLevelToken(), $"Dafny encountered an error. Please report it at <https://github.com/dafny-lang/dafny/issues>:\n{e}");
return new CompilationUnit(program);
}
finally {
resolverMutex.Release();
Expand Down