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

Improve reporting of related locations #4929

Merged

Conversation

keyboardDrummer
Copy link
Member

@keyboardDrummer keyboardDrummer commented Jan 3, 2024

Description

Behavior

Make reporting of related locations more consistent, so related locations are always shown on a separate line. Example:

-git-issue-864y.dfy(6,9): Error: duplicate name of top-level declaration: A [Related location] git-issue-864y.dfy(7,8)
+git-issue-864y.dfy(6,9): Error: duplicate name of top-level declaration: A
+git-issue-864y.dfy(7,8): Related location

This style already occurred in many other locations, such as here:

CoPrefix.dfy(169,2): Error: a postcondition could not be proved on this return path
CoPrefix.dfy(168,14): Related location: this is the postcondition that could not be proved

and here:

CoPrefix.dfy(142,24): Error: assertion might not hold
CoPrefix.dfy(117,22): Related location

Refactoring

  • Move class LocalVariableDeclarationVisitor into its own file, without changes
  • Move class SymbolDeclarationResolver into its own file, without changes
  • Move class DiagnosticUtil into its own file, and rename to DiagnosticsUtil
  • Move class MapType into its own file, without changes

How has this been tested?

Expect files have been updated to match the new related location format.

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

@keyboardDrummer keyboardDrummer marked this pull request as ready for review January 3, 2024 15:45
@keyboardDrummer keyboardDrummer enabled auto-merge (squash) January 3, 2024 15:46
@keyboardDrummer keyboardDrummer changed the title Refactor reporting code Improve reporting of related locations Jan 3, 2024
MikaelMayer
MikaelMayer previously approved these changes Jan 9, 2024
Copy link
Member

@MikaelMayer MikaelMayer left a comment

Choose a reason for hiding this comment

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

Creating harmony between error reporting systems brings me happiness. Thanks!

}

public void WriteSourceCodeSnippet(IToken tok, TextWriter tw) {
string line = GetFileLine(tok.Uri, tok.line - 1);
if (line == null) {
Copy link
Member

Choose a reason for hiding this comment

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

Makes sense, thanks for doing that!

yield return SerializeRelated(tok, null, "Related location");
while (tok is NestedToken nestedToken) {
tok = nestedToken.Inner;
var message = nestedToken.Message != null ? "Related location: " + nestedToken.Message : "Related location";
Copy link
Member

Choose a reason for hiding this comment

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

Good job updating this! I don't think it is tested, is it?

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 is, in Source/IntegrationTests/bin/Test/cli/diagnosticsFormats.dfy

rwLock.EnterWriteLock();
try {
counts[dafnyDiagnostic.Level] = counts.GetValueOrDefault(dafnyDiagnostic.Level, 0) + 1;
if (messageSource != MessageSource.Verifier && messageSource != MessageSource.Compiler) {
if (dafnyDiagnostic.Source != MessageSource.Verifier && dafnyDiagnostic.Source != MessageSource.Compiler) {
Copy link
Member

Choose a reason for hiding this comment

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

I like dafnyDiagnostic more than message too


yield return new DafnyRelatedInformation(token, message);
if (inner != null) {
foreach (var nestedInformation in CreateDiagnosticRelatedInformationFor(inner, newMessage)) {
Copy link
Member

Choose a reason for hiding this comment

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

One CI warning to fix below

@keyboardDrummer keyboardDrummer merged commit 7d14cad into dafny-lang:master Jan 10, 2024
20 checks passed
@keyboardDrummer keyboardDrummer deleted the reportingRefactoring branch January 10, 2024 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants