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

Print context of violations #45

Merged
merged 2 commits into from
Sep 24, 2024
Merged

Print context of violations #45

merged 2 commits into from
Sep 24, 2024

Conversation

ZedThree
Copy link
Member

Example:

warning: test.f90:90:3: P011 prefer 'real(real64)' to 'double precision' (see 'iso_fortran_env')
   |
89 |   ! Should trigger for use of 'double precision'
90 |   double precision function double_prec(x)
   |   ^ P011
91 |     double precision, intent(in) :: x
92 |     double_prec = 2 * x
   |

warning: test.f90:91:5: P011 prefer 'real(real64)' to 'double precision' (see 'iso_fortran_env')
   |
89 |   ! Should trigger for use of 'double precision'
90 |   double precision function double_prec(x)
91 |     double precision, intent(in) :: x
   |     ^ P011
92 |     double_prec = 2 * x
93 |   end function
   |

warning: test.f90:97:1: M001 subroutine not contained within (sub)module or program
   |
96 | ! This function should trigger for missing enclosing module
97 | subroutine triple(x)
   |  ^ M001
98 |   integer, intent(inout) :: x
99 |   x = x * 3
   |

A couple of annoying things:

  • Something really weird going on here, where I can't get it to
    put the annotation in the first column: it's either in column 2
    or the end of the previous line. But does appear to be right
    for other columns!
  • Some annoyance here: we have to have some level prefix to our
    message. Might be fixed in future version of annotate-snippets
    -- or we use an earlier version with more control.
  • Also, we could use .origin(path) to get the filename and
    line:col automatically, but see above about off-by-one error

Copy link
Collaborator

@LiamPattinson LiamPattinson left a comment

Choose a reason for hiding this comment

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

Amazing, this looks so much better.

I think I've figured out a fix to the weird off-by-one error, but I haven't tested it enough to be 100% sure.

src/lib.rs Outdated
let snippet = Level::Warning.title(&message_line).snippet(
Snippet::source(&content_slice)
.line_start(start_index)
.annotation(Level::Error.span(label_offset..label_offset).label(code)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
.annotation(Level::Error.span(label_offset..label_offset).label(code)),
.annotation(Level::Error.span(label_offset..label_offset.saturating_add(1)).label(code)),

src/lib.rs Outdated
// put the annotation in the first column: it's either in column 2
// or the end of the previous line. But does appear to be right
// for other columns!
let label_offset = offset_up_to_line + col.saturating_sub(1).max(1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let label_offset = offset_up_to_line + col.saturating_sub(1).max(1);
let label_offset = offset_up_to_line + col.saturating_sub(1);

@LiamPattinson
Copy link
Collaborator

As an aside, it might be nice to introduce a --concise mode that doesn't include the snippet, as it seems to be taking about 4 times as long when applied to a project I'm using for simple benchmarking (although 2 seconds to produce over 10000 annotated code snippets is still impressively fast as far as I'm concerned).

@LiamPattinson LiamPattinson merged commit 475271d into main Sep 24, 2024
22 checks passed
@ZedThree ZedThree deleted the print-context branch September 30, 2024 15:31
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