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

Obsolete verification diagnostics prevent seeing resolution errors #317

Closed
MikaelMayer opened this issue Dec 21, 2022 · 0 comments · Fixed by dafny-lang/dafny#3282
Closed
Labels
hovering Issues related to hovering symbols

Comments

@MikaelMayer
Copy link
Member

Enter the following code in VSCode

predicate Ok(a: int) {
  a >= 0 &&
  a >= 1 &&
  a >= 2 &&
  a >= 3 &&
  a >= 4 &&
  a >= 5 &&
  a >= 6 &&
  a >= 7 &&
  a >= 8
}

function NeedsOk(i: int): int
  requires Ok(i)

lemma ok() {
  assert NeedsOk(0) == 1;
}

If one hovers over the call NeedsOk(), it says as expected that the first 5 preconditions don't hold, and it does not know for the remaining one. Ok.
Now, introduce a resolution error by changing NeedsOk(0) into NeedsOk(0, 1).
Hover over the call, you get this:
image
which is the same set of verification error and no clue about the resolution error. You'd have to scroll to the end. But if you do this naively, you'd see no resolution error
image
If you want to see the resolution error, you'd have to hover over the parenthesis (yes, very subtle) on which the resolution error is reported.
image

Of course, someone looking at the diagnostics window would immediately spot the resolution error and its message, but for users like me who don't like to be distracted by switching window, this recurring scenario above is really frustrating.
It ought to be that:

  • The resolution error should not be reported on the token itself, but on the same kinds of ranges as of the verification ranges.
  • When verification errors are obsolete, they should be written as such in the hover message. Currently, there is no difference and this is misleading.
  • Parse and resolution errors should always be on the top of the hover message
  • Assertions verified in the same batch should take much less space on the hovering window(the resource usage, for example, is the same because it's the assertion batch resource usage)
  • The function precondition that might not hold should be visible in the error message. Otherwise that's impossible to guess!
@MikaelMayer MikaelMayer changed the title Obsolete verifications diagnostics prevent seeing resolution errors Obsolete verification diagnostics prevent seeing resolution errors Dec 21, 2022
@MikaelMayer MikaelMayer added the hovering Issues related to hovering symbols label Jan 4, 2023
keyboardDrummer pushed a commit to dafny-lang/dafny that referenced this issue Jan 6, 2023
Fixes #3281 
Fixes dafny-lang/ide-vscode#335
Fixes dafny-lang/ide-vscode#317
Fixes dafny-lang/ide-vscode#210

Example code:
```dafny
predicate P(i: int) {
  i <= 0
}

predicate Q(i: int, j: int) {
  i == j || -i == j
}

function method Toast(i: int): int
  requires P(i)

method Test(i: int) returns (j: nat)
  ensures Q(i, j)
{
  if i < 0 {
    return -i;
  } else {
    return Toast(i);
  }
}
```

| Before: | After |
| --------|-------|
| Hovering Q(i,j)
![image](https://user-images.githubusercontent.com/3601079/209727765-e3c9c32b-dd27-4707-893b-8534b9ec0698.png)
|
![image](https://user-images.githubusercontent.com/3601079/209727590-2f833693-4b9d-44cf-afa8-fe43388e0c3d.png)
|
| Hovering the second return
![image](https://user-images.githubusercontent.com/3601079/209727748-5d291a36-7334-4b9c-8c08-c68718fd50f9.png)
|
![image](https://user-images.githubusercontent.com/3601079/209727565-19b8a1c6-0f78-41a3-ad85-7e0cbc1538c8.png)
|
| Hovering the call to Toast()
![image](https://user-images.githubusercontent.com/3601079/209727700-2e66f345-dd1f-47c5-92ce-cea811d2b8ce.png)
|
![image](https://user-images.githubusercontent.com/3601079/209727556-cbfe43c5-9fe2-49c9-a107-7111ef42b00c.png)
|
| Also, on the diagnostics side (and it's clickable!)
![image](https://user-images.githubusercontent.com/3601079/209844964-be5a31df-dddd-421a-937f-39a26fa8ce76.png)
|
![image](https://user-images.githubusercontent.com/3601079/209845060-09dd8fb5-5cd8-4e92-8302-beb93a837412.png)
|
| The resolver error appears after all obsolete verification
error<br>![image](https://user-images.githubusercontent.com/3601079/209727781-b3d88e9b-8bad-4e3b-b8fc-e9119837629e.png)
|
![image](https://user-images.githubusercontent.com/3601079/209728682-206bf18a-dfd8-40c5-a10d-8b5a4ccdd4c1.png)
|

I added language server tests and tested it in VSCode, and it works as
expected.

<!-- Are you moving a large amount of code? Read CONTRIBUTING.md to
learn how to do that while maintaining git history -->

<small>By submitting this pull request, I confirm that my contribution
is made under the terms of the [MIT
license](https://github.com/dafny-lang/dafny/blob/master/LICENSE.txt).</small>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hovering Issues related to hovering symbols
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant