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

Use dafny: Uris for standard library files #4832

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
28 changes: 22 additions & 6 deletions Source/DafnyCore/DafnyFile.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Reflection;
using System.Reflection.Metadata;
using System.Reflection.PortableExecutable;
Expand All @@ -16,13 +18,30 @@ public class DafnyFile {
public string BaseName { get; private set; }
public bool IsPreverified { get; set; }
public bool IsPrecompiled { get; set; }
public bool IsPrerefined { get; private set; }
Copy link
Member

Choose a reason for hiding this comment

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

Off topic but when did this appear and what was it used for?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was needed when .doo files still contained resolved code instead of just parsed code.

Copy link
Member

Choose a reason for hiding this comment

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

Okay good, I figured it was related but wasn't sure if was actually in sync before.

public Func<TextReader> GetContent { get; set; }
public Uri Uri { get; }
public Uri Uri { get; private set; }
[CanBeNull] public IToken Origin { get; }

private static readonly Dictionary<Uri, Uri> ExternallyVisibleEmbeddedFiles = new();

public static Uri ExposeInternalUri(string externalName, Uri internalUri) {
var externalUri = new Uri("dafny:" + externalName);
Copy link
Member

Choose a reason for hiding this comment

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

Why not just use $"dafny:{internalUri}"? That would be simpler and leaves the door open for other schemes. AFAICT attaching a fake Dafny file name doesn't help the IDE but perhaps I'm missing something.

Copy link
Member Author

@keyboardDrummer keyboardDrummer Dec 5, 2023

Choose a reason for hiding this comment

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

Because I didn't want to expose the implementation details of internalUri. Is there are a downside to having this mapping, except for it being slightly more work?

Copy link
Member

Choose a reason for hiding this comment

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

No real objection to the complexity/work, but a little worried about exposing "DafnyStandardLibraries.dfy" when that string/file name isn't actually meaningful. Eventually I expect the IDE will be able to actually open the doo file, and we shouldn't hide the fact that the source is coming from a doo file (especially to be clear it's read-only)

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I added the dot dfy at the end so the IDE can recognize it's Dafny code and provide ide tooling on it.

I think from the user perspective, opening a dotdfy file to view the sources inside a dotdoo file, seems good

Copy link
Member

Choose a reason for hiding this comment

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

Okay, I can live with it :)

Copy link
Member Author

@keyboardDrummer keyboardDrummer Dec 6, 2023

Choose a reason for hiding this comment

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

We can still change the part after the dafny: in a follow-up PR, so let's not get hung up on this

I think, as part of adding .doo files, we should have added IDE navigation for .doo files as well, and then we would have already solved this. I think it would help with correctly designing .doo files as well. For example, we could have considered not making dotdoo a compressed file and instead having it be a single text file, so the IDE can open it directly, without the need for a dafny doo/library source command. Not saying we should have gone for that option, but at least we would have had a reason to consider it.

Thinking about dotdoo support in the IDE more, and given the way dotdoo files are currently structured, the dafny doo/library source command should support both on-disk files and files internal to the CLI, so it does need to take a URI, and then the dafny: uris exposed to the IDE should probably contain a URI after the dafny:. The IDE will chop off the dafny: part and pass the rest to dafny doo source

Copy link
Member

Choose a reason for hiding this comment

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

For example, we could have considered not making dotdoo a compressed file and instead having it be a single text file, so the IDE can open it directly, without the need for a dafny doo/library source command.

I definitely intended to iterate on the file format as we added features, and hence why the manifest has a doo_file_version property. :) But I don't think I'd want to make it a single text file, since then the manifest information would have to be embedded in comments or something like that, which would make it a bit tougher to read robustly and harder to hide that implementation detail from users.

Thinking about dotdoo support in the IDE more, and given the way dotdoo files are currently structured, the dafny doo/library source command should support both on-disk files and files internal to the CLI, so it does need to take a URI, and then the dafny: uris exposed to the IDE should probably contain a URI after the dafny:. The IDE will chop off the dafny: part and pass the rest to dafny doo source

Yup, this is one of the reasons I thought $"dafny:{internalUri}" would be smart, but you expressed it better than I managed at the time. :)

ExternallyVisibleEmbeddedFiles[externalUri] = internalUri;
return externalUri;
}

public static DafnyFile CreateAndValidate(ErrorReporter reporter, IFileSystem fileSystem,
DafnyOptions options, Uri uri, IToken origin) {

var embeddedFile = ExternallyVisibleEmbeddedFiles.GetValueOrDefault(uri);
if (embeddedFile != null) {
var result = CreateAndValidate(reporter, fileSystem, options, embeddedFile, origin);
if (result != null) {
result.Uri = uri;
}
return result;
}

var filePath = uri.LocalPath;

origin ??= Token.NoToken;
Expand All @@ -32,7 +51,6 @@ public static DafnyFile CreateAndValidate(ErrorReporter reporter, IFileSystem fi
Func<TextReader> getContent = null;
bool isPreverified;
bool isPrecompiled;
var isPrerefined = false;
var extension = ".dfy";
if (uri.IsFile) {
extension = Path.GetExtension(uri.LocalPath).ToLower();
Expand Down Expand Up @@ -98,7 +116,7 @@ public static DafnyFile CreateAndValidate(ErrorReporter reporter, IFileSystem fi
dooFile = DooFile.Read(filePath);
}

if (!dooFile.Validate(reporter, filePathForErrors, options, options.CurrentCommand, origin)) {
if (!dooFile.Validate(reporter, filePathForErrors, options, origin)) {
return null;
}

Expand All @@ -109,7 +127,6 @@ public static DafnyFile CreateAndValidate(ErrorReporter reporter, IFileSystem fi
// the DooFile class should encapsulate the serialization logic better
// and expose a Program instead of the program text.
getContent = () => new StringReader(dooFile.ProgramText);
isPrerefined = true;
} else if (extension == ".dll") {
isPreverified = true;
// Technically only for C#, this is for backwards compatability
Expand All @@ -127,7 +144,6 @@ public static DafnyFile CreateAndValidate(ErrorReporter reporter, IFileSystem fi
return new DafnyFile(extension, canonicalPath, baseName, getContent, uri, origin) {
IsPrecompiled = isPrecompiled,
IsPreverified = isPreverified,
IsPrerefined = isPrerefined
};
}

Expand Down
15 changes: 12 additions & 3 deletions Source/DafnyCore/DafnyMain.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,18 @@
namespace Microsoft.Dafny {

public class DafnyMain {
public static readonly string StandardLibrariesDooUriBase = "dllresource://DafnyPipeline/DafnyStandardLibraries";
public static readonly Uri StandardLibrariesDooUri = new("dllresource://DafnyPipeline/DafnyStandardLibraries.doo");
public static readonly Uri StandardLibrariesArithmeticDooUri = new("dllresource://DafnyPipeline/DafnyStandardLibraries-arithmetic.doo");
public static readonly Dictionary<string, Uri> standardLibrariesDooUriTarget = new();
public static readonly Uri StandardLibrariesDooUri = DafnyFile.ExposeInternalUri("DafnyStandardLibraries.dfy",
new("dllresource://DafnyPipeline/DafnyStandardLibraries.doo"));
public static readonly Uri StandardLibrariesArithmeticDooUri = DafnyFile.ExposeInternalUri("DafnyStandardLibraries-arithmetic.dfy",
new("dllresource://DafnyPipeline/DafnyStandardLibraries-arithmetic.doo"));

static DafnyMain() {
foreach (var target in new[] { "cs", "java", "go", "py", "js", "notarget" }) {
standardLibrariesDooUriTarget[target] = DafnyFile.ExposeInternalUri($"DafnyStandardLibraries-{target}.dfy",
new($"dllresource://DafnyPipeline/DafnyStandardLibraries-{target}.doo"));
}
}

public static void MaybePrintProgram(Program program, string filename, bool afterResolver) {
if (filename == null) {
Expand Down
1 change: 0 additions & 1 deletion Source/DafnyCore/DafnyOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ public class DafnyOptions : Bpl.CommandLineOptions {
public IList<Uri> CliRootSourceUris = new List<Uri>();

public DafnyProject DafnyProject { get; set; }
public Command CurrentCommand { get; set; }

public static void ParseDefaultFunctionOpacity(Option<CommonOptionBag.DefaultFunctionOpacityOptions> option, Bpl.CommandLineParseState ps, DafnyOptions options) {
if (ps.ConfirmArgumentCount(1)) {
Expand Down
9 changes: 4 additions & 5 deletions Source/DafnyCore/DooFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,10 @@ public DooFile(Program dafnyProgram) {
private DooFile() {
}

public bool Validate(ErrorReporter reporter, string filePath, DafnyOptions options, Command currentCommand,
IToken origin) {
if (currentCommand == null) {
public bool Validate(ErrorReporter reporter, string filePath, DafnyOptions options, IToken origin) {
if (!options.UsingNewCli) {
reporter.Error(MessageSource.Project, origin,
$"Cannot load {filePath}: .doo files cannot be used with the legacy CLI");
$"cannot load {filePath}: .doo files cannot be used with the legacy CLI");
return false;
}

Expand All @@ -131,7 +130,7 @@ public bool Validate(ErrorReporter reporter, string filePath, DafnyOptions optio
}

var success = true;
var relevantOptions = currentCommand.Options.ToHashSet();
var relevantOptions = options.Options.OptionArguments.Keys.ToHashSet();
Copy link
Member

Choose a reason for hiding this comment

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

Is this still correct, given the comment immediately below? i.e. is OptionArguments.Keys the set of valid parameters for the current command, or just the options explicitly provided on the CLI or in project files?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is still correct, yes

foreach (var (option, check) in OptionChecks) {
// It's important to only look at the options the current command uses,
// because other options won't be initialized to the correct default value.
Expand Down
2 changes: 1 addition & 1 deletion Source/DafnyCore/Generic/Reporting.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ protected override bool MessageCore(MessageSource source, ErrorLevel level, stri

Options.OutputWriter.Write(errorLine);

if (Options.Get(DafnyConsolePrinter.ShowSnippets) && tok != Token.NoToken) {
if (Options.Get(DafnyConsolePrinter.ShowSnippets) && tok.Uri != null) {
TextWriter tw = new StringWriter();
new DafnyConsolePrinter(Options).WriteSourceCodeSnippet(tok.ToRange(), tw);
Options.OutputWriter.Write(tw.ToString());
Expand Down
9 changes: 3 additions & 6 deletions Source/DafnyCore/Rewriters/PrecedenceLinter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,18 @@
// SPDX-License-Identifier: MIT
//
//-----------------------------------------------------------------------------
using System.Collections.Generic;

using System.Linq;
using System;
using System.Diagnostics.Contracts;
using JetBrains.Annotations;
using Microsoft.Boogie;
using static Microsoft.Dafny.RewriterErrors;

namespace Microsoft.Dafny {

public class PrecedenceLinter : IRewriter {
private CompilationData compilation;
private readonly CompilationData compilation;
// Don't perform linting on doo files in general, since the source has already been processed.
internal override void PostResolve(ModuleDefinition moduleDefinition) {
if (moduleDefinition.tok.Uri != null && moduleDefinition.tok.Uri.LocalPath.EndsWith(".doo")) {
if (moduleDefinition.tok.Uri != null && !moduleDefinition.ShouldVerify(compilation)) {
Copy link
Member

Choose a reason for hiding this comment

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

For the record I think this is a step backwards in terms of accuracy: not linting doo files is more about the fact that the content is read-only to consumers and not their responsibility to lint, nothing to do with verification.

I don't want to block this change on it, but perhaps we could at least add a comment in the next related PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm wondering whether ShouldVerify should be renamed to ShouldCheck.

The reasoning behind not verifying a file seem to apply to other correctness checks as well.

return;
}
foreach (var topLevelDecl in moduleDefinition.TopLevelDecls.OfType<TopLevelDeclWithMembers>()) {
Expand Down
3 changes: 1 addition & 2 deletions Source/DafnyDriver/Commands/DafnyCli.cs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ async Task Handle(InvocationContext context) {
}
}

dafnyOptions.CurrentCommand = command;
dafnyOptions.ApplyDefaultOptionsWithoutSettingsDefault();
dafnyOptions.UsingNewCli = true;
context.ExitCode = await continuation(dafnyOptions, context);
Expand Down Expand Up @@ -490,7 +489,7 @@ public static ExitValue GetDafnyFiles(DafnyOptions options,
var reporter = new ConsoleErrorReporter(options);
if (options.CompilerName is null or "cs" or "java" or "go" or "py" or "js") {
var targetName = options.CompilerName ?? "notarget";
var stdlibDooUri = new Uri($"{DafnyMain.StandardLibrariesDooUriBase}-{targetName}.doo");
var stdlibDooUri = DafnyMain.standardLibrariesDooUriTarget[targetName];
options.CliRootSourceUris.Add(stdlibDooUri);
dafnyFiles.Add(DafnyFile.CreateAndValidate(reporter, OnDiskFileSystem.Instance, options, stdlibDooUri, Token.Cli));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
<ItemGroup>
<ProjectReference Include="..\DafnyCore.Test\DafnyCore.Test.csproj" />
<ProjectReference Include="..\DafnyLanguageServer\DafnyLanguageServer.csproj" />
<ProjectReference Include="..\DafnyPipeline\DafnyPipeline.csproj" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ private Action<LanguageServerOptions> GetServerOptionsAction(Action<DafnyOptions
var dafnyOptions = DafnyOptions.Create(output);
dafnyOptions.Set(ProjectManager.UpdateThrottling, 0);
modifyOptions?.Invoke(dafnyOptions);
Microsoft.Dafny.LanguageServer.LanguageServer.ConfigureDafnyOptionsForServer(dafnyOptions);
dafnyOptions.UsingNewCli = true;
LanguageServer.ConfigureDafnyOptionsForServer(dafnyOptions);
ApplyDefaultOptionValues(dafnyOptions);
return options => {
options.WithDafnyLanguageServer(() => { });
Expand Down
18 changes: 0 additions & 18 deletions Source/DafnyLanguageServer.Test/Lookup/GoToDefinitionTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -104,24 +104,6 @@ requires Err?
await AssertPositionsLineUpWithRanges(source);
}

/// <summary>
/// Given <paramref name="source"/> with N positions, for each K from 0 to N exclusive,
/// assert that a RequestDefinition at position K
/// returns either the Kth range, or the range with key K (as a string).
/// </summary>
private async Task AssertPositionsLineUpWithRanges(string source) {
MarkupTestFile.GetPositionsAndNamedRanges(source, out var cleanSource,
out var positions, out var ranges);

var documentItem = await CreateOpenAndWaitForResolve(cleanSource);
for (var index = 0; index < positions.Count; index++) {
var position = positions[index];
var range = ranges.ContainsKey(string.Empty) ? ranges[string.Empty][index] : ranges[index.ToString()].Single();
var result = (await RequestDefinition(documentItem, position)).Single();
Assert.Equal(range, result.Location!.Range);
}
}

[Fact]
public async Task StaticFunctionCall() {
var source = @"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -425,4 +425,22 @@ protected async Task<TextDocumentItem> GetDocumentItem(string source, string fil
await client.OpenDocumentAndWaitAsync(documentItem, CancellationToken);
return documentItem;
}

/// <summary>
/// Given <paramref name="source"/> with N positions, for each K from 0 to N exclusive,
/// assert that a RequestDefinition at position K
/// returns either the Kth range, or the range with key K (as a string).
/// </summary>
protected async Task AssertPositionsLineUpWithRanges(string source, string filePath = null) {
MarkupTestFile.GetPositionsAndNamedRanges(source, out var cleanSource,
out var positions, out var ranges);

var documentItem = await CreateOpenAndWaitForResolve(cleanSource, filePath);
for (var index = 0; index < positions.Count; index++) {
var position = positions[index];
var range = ranges.ContainsKey(string.Empty) ? ranges[string.Empty][index] : ranges[index.ToString()].Single();
var result = (await RequestDefinition(documentItem, position)).Single();
Assert.Equal(range, result.Location!.Range);
}
}
}
37 changes: 0 additions & 37 deletions Source/DafnyLanguageServer.Test/Various/StandardLibrary.cs

This file was deleted.

70 changes: 70 additions & 0 deletions Source/DafnyLanguageServer.Test/Various/StandardLibraryTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
using System;
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.Dafny.LanguageServer.IntegrationTest.Util;
using Microsoft.Extensions.Logging;
using OmniSharp.Extensions.LanguageServer.Protocol.Models;
using Xunit;
using Xunit.Abstractions;

namespace Microsoft.Dafny.LanguageServer.IntegrationTest.Various;

public class StandardLibraryTest : ClientBasedLanguageServerTest {
[Fact]
public async Task CanUseWrappers() {
var source = @"
import opened DafnyStdLibs.Wrappers

const triggerSemicolonWarning := 3;

method Foo() returns (s: Option<int>) {
return Some(3);
}".TrimStart();

var projectSource = @"
[options]
standard-libraries = true";

var withoutStandardLibraries = CreateAndOpenTestDocument(source);
var diagnostics1 = await GetLastDiagnostics(withoutStandardLibraries, DiagnosticSeverity.Error);
Assert.Single(diagnostics1);

var directory = Path.GetTempFileName();
CreateAndOpenTestDocument(projectSource, Path.Combine(directory, DafnyProject.FileName));
var document = CreateAndOpenTestDocument(source, Path.Combine(directory, "document.dfy"));
var diagnostics2 = await GetLastDiagnostics(document);
Assert.Single(diagnostics2);
Assert.Equal(DiagnosticSeverity.Warning, diagnostics2[0].Severity);
}

[Fact]
public async Task GotoDefinition() {
var source = @"
import opened DafnyStdLibs.Wrappers

method Foo() returns (s: ><Option<int>) {
return Some(3);
}".TrimStart();

var projectSource = @"
[options]
standard-libraries = true";

var directory = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
Directory.CreateDirectory(directory);
await File.WriteAllTextAsync(Path.Combine(directory, DafnyProject.FileName), projectSource);

MarkupTestFile.GetPositionsAndNamedRanges(source, out var cleanSource,
out var positions, out var ranges);

var filePath = Path.Combine(directory, "StandardLibraryGotoDefinition.dfy");
var documentItem = CreateAndOpenTestDocument(cleanSource, filePath);
await AssertNoDiagnosticsAreComing(CancellationToken);
var result = await RequestDefinition(documentItem, positions[0]);
Assert.Equal(new Uri("dafny:DafnyStandardLibraries.dfy"), result.Single().Location.Uri);
}

public StandardLibraryTest(ITestOutputHelper output, LogLevel dafnyLogLevel = LogLevel.Information) : base(output, dafnyLogLevel) {
}
}
15 changes: 1 addition & 14 deletions Source/DafnyLanguageServer/Language/DafnyLangParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,7 @@ public Program Parse(Compilation compilation, CancellationToken cancellationToke

var beforeParsing = DateTime.Now;
try {
var rootFiles = compilation.RootFiles!;
List<DafnyFile> dafnyFiles = new();
foreach (var rootFile in rootFiles) {
try {
dafnyFiles.Add(rootFile);
if (logger.IsEnabled(LogLevel.Trace)) {
logger.LogTrace($"Parsing file with uri {rootFile.Uri} and content\n{rootFile.GetContent().ReadToEnd()}");
}
} catch (IOException) {
logger.LogError($"Tried to parse file {rootFile} that could not be found");
}
}

return programParser.ParseFiles(compilation.Project.ProjectName, dafnyFiles, compilation.Reporter, cancellationToken);
return programParser.ParseFiles(compilation.Project.ProjectName, compilation.RootFiles, compilation.Reporter, cancellationToken);
}
finally {
telemetryPublisher.PublishTime("Parse", compilation.Project.Uri.ToString(), DateTime.Now - beforeParsing);
Expand Down
1 change: 0 additions & 1 deletion Source/DafnyLanguageServer/LanguageServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ related locations

public static void ConfigureDafnyOptionsForServer(DafnyOptions dafnyOptions) {
dafnyOptions.Set(DafnyConsolePrinter.ShowSnippets, true);

dafnyOptions.PrintIncludesMode = DafnyOptions.IncludesModes.None;

// TODO This may be subject to change. See Microsoft.Boogie.Counterexample
Expand Down
2 changes: 1 addition & 1 deletion Source/DafnyLanguageServer/Workspace/Compilation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ private IReadOnlyList<DafnyFile> DetermineRootFiles() {
if (Options.Get(CommonOptionBag.UseStandardLibraries)) {
if (Options.CompilerName is null or "cs" or "java" or "go" or "py" or "js") {
var targetName = Options.CompilerName ?? "notarget";
var stdlibDooUri = new Uri($"{DafnyMain.StandardLibrariesDooUriBase}-{targetName}.doo");
var stdlibDooUri = DafnyMain.standardLibrariesDooUriTarget[targetName];
Options.CliRootSourceUris.Add(stdlibDooUri);
result.Add(DafnyFile.CreateAndValidate(errorReporter, OnDiskFileSystem.Instance, Options, stdlibDooUri, Project.StartingToken));
}
Expand Down
Loading