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

Diagnostics are shown for all files in explicitly defined Dafny projects #2329

Closed
keyboardDrummer opened this issue Jun 30, 2022 · 2 comments · Fixed by #3902
Closed

Diagnostics are shown for all files in explicitly defined Dafny projects #2329

keyboardDrummer opened this issue Jun 30, 2022 · 2 comments · Fixed by #3902
Assignees
Labels
kind: enhancement Enhancements, feature requests, etc. These are NOT bugs, but ways we can improve Dafny part: language server Support for LSP in Dafny (server part; client is in ide-vscode repo) status: needs-approval Issue that needs approval from Dafny team members before moving to designed

Comments

@keyboardDrummer
Copy link
Member

keyboardDrummer commented Jun 30, 2022

Change

Enable customers to define which .dfy files belong together as part of a Dafny 'project'. When any file in a project is opened, show diagnostics for all files in that projects.

Which files a project contains can be done by specifying a root folder. All .dfy files under the root folder are part of the project. The root folder is specified by placing a dafny.toml file in that folder. The dafny.toml file can be empty and its contents are ignored.

The Dafny IDE should watch for file changes that could affect which Dafny projects the currently opened files are in, such as the removal or addition of dafny.toml files. In this case offering to restart the LSP server with the new file contents is fine.

Related issue: dafny-lang/ide-vscode#82

Not in scope

  • Using the contents of the Dafny project file
  • Allow configuring file filters to select which files within a root folder are part a project.
  • Enable specifying the Dafny version a project is built.
  • Enable configuring all sorts of project related information such as author, version etc.

Prerequisites

Implementation suggestions

  • When the language server starts processing a file, which is done in DocumentManagerDatabase, traverse up the file's path to see if a dafny.toml is present. If such a file is found, use this file's Uri to lookup a DafnyProject (currently named DocumentManager). If no such file is found, use the file's Uri for the lookup instead. If no DafnyProject is found, create one, either using the previously found project file or using an implicit project file.
  • Compilation needs to take a ProjectFile as input instead of a DocumentTextBuffer textBuffer, and the information from that is used to change which DafnyFiles are passed to ProgramParser.ParseFiles. This will also set the IsPreverified property on those DafnyFiles so that library files are not verified.
  • IdeState needs to be updated so it can hold diagnostics for more than one file. Document.ToIdeState and its overrides need to be updated to fill in the extra information.
  • Document will no longer hold a DocumentTextBuffer, but instead a Dictionary<Uri, DocumentTextBuffer>, and no longer a single VerificationTree, but one for each file.
  • DocumentManager (which we'll rename to DafnyProject), will have its fields updated to reflect that is handles multiple files List<Position> ChangedVerifiables { get; set; } = new(); and List<Range> ChangedRanges { get; set; } = new(); need the Position and Range to include a Uri as well, and the code that uses these fields to determine the order in which verification tasks are evaluated, is updated to use the new types.
@keyboardDrummer keyboardDrummer added kind: enhancement Enhancements, feature requests, etc. These are NOT bugs, but ways we can improve Dafny part: language server Support for LSP in Dafny (server part; client is in ide-vscode repo) status: needs-approval Issue that needs approval from Dafny team members before moving to designed labels Jun 30, 2022
@keyboardDrummer keyboardDrummer added this to the Project aware IDE milestone Jun 30, 2022
@cpitclaudel
Copy link
Member

such as the removal or addition of dafny.toml files.

I wonder how IDEs deal with this. Does Rider get confused if you remove a csproj, for example?
I think we don't need to be too smart about this; we could simply offer to restart the server in that case.

@keyboardDrummer
Copy link
Member Author

keyboardDrummer commented Jul 5, 2022

such as the removal or addition of dafny.toml files.

I wonder how IDEs deal with this. Does Rider get confused if you remove a csproj, for example? I think we don't need to be too smart about this; we could simply offer to restart the server in that case.

Some IDEs require explicitly opening a project file before they provide their full functionality. The LSP protocol however doesn't have such as open step. An LSP server is expected to respond to file system changes that affect the semantics of files that are open in the LSP client.

An example is opening a .ts file without a tsconfig.json being present. The TypeScript server starts immediately when you open the .ts file, but once you add a tsconfig.json next to it, the typechecking is restarted.

@keyboardDrummer keyboardDrummer self-assigned this Jun 27, 2023
keyboardDrummer added a commit that referenced this issue Jul 18, 2023
Fixes #2329

Depends on #4236,
#4237 and
#4238

### Changes

- Since this feature is still a work in progress, the server takes an
option `--project-mode` that is hidden, will be removed in the future,
and is currently off by default. If the option is not turned on, the IDE
attempts to behave as it did, even when project files are used.
- When using a Dafny project file, include directives are not needed for
files included by the project.
- When any file in a Dafny project is open in the IDE, all diagnostics
for that project are displayed, and any change in any file in the
project will update the diagnostics for all files in it.

### Testing
- Test that existing tests for hover and gutter icons still pass in the
presence of project files when _not_ using `--project-mode`, since some
of the features of these aren't currently supported when using
`--project-mode`
- Add a suite of tests that relate to explicit project files and
multiple source files
- What happens when a file is changed and that affects diagnostics in a
file that uses it
  - What happens when the project file is changed
  - What happens when a project file is introduced
- Test that go to definition works for files referenced through an
explicit project, without any include directives
- Test that ghost diagnostics work when an explicit project is used
- Add two tests that for now are skipped, since the behavior is not yet
implemented, that test behavior when two project files both reference
the same file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Enhancements, feature requests, etc. These are NOT bugs, but ways we can improve Dafny part: language server Support for LSP in Dafny (server part; client is in ide-vscode repo) status: needs-approval Issue that needs approval from Dafny team members before moving to designed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants