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.

Sign up for GitHub

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

Jump to bottom

msgs: more decision making in handleReport #948

Merged
merged 3 commits into from
Oct 8, 2023
Merged

msgs: more decision making in handleReport #948

merged 3 commits into from
Oct 8, 2023

Commits on Oct 7, 2023

  1. msgs: more decision making in handleReport 

    Summary
=======

* fix hints/warnings promoted to errors always counting towards the
  maximum error count, even if the hint/warnings is currently disabled
* fix disabled warnings and hints being reported by `nimsuggest chk`

Details
=======

Processing of a `Report` previously worked as follows:
1. `handleReport` pre-processes the report
2. the report is passed to the report hook
3. the hook outputs the report if it determines that the report should
   be rendered and displayed
4. `handleReport` determines the error handling strategy
5. error handling is performed

This has two problems:
1. the responsibility of deciding whether a report should be output is
   left to the hook, which also means that the hook needs access to all
   state necessary to make the decision
2. error handling always takes place, even if the report is disabled

Report handling is split up, and the responsibilities are shifted:
1. if a report is disabled (`isEnabled` returns false), `handleReport`
   is a no-op
2. deciding writability is now the responsibility of `handleReport`
   (i.e., the compiler), with the report hook only invoked if the
   report is eligible for being displayed
3. computing writability no longer takes into account whether the
   report is enabled (since writeability is only decided *after* it is
   know that a report is enabled)
4. `handleReport` now handles the "forcefully enabled" case, by
   temporarily overriding the `writeln` hook with something that always
   echoes the message (this mirrors the previous behaviour of the
   report hook implementations)

In short, more responsibility regarding reports shift into a single
place (`handleReport`), with the report hook now only being invoked for
reports that should be displayed. Centralizing `Report` handling is a
step towards eventually removing/replacing `Report`s.

Since the the report hook used by `nimsuggest` doesn't consider
writeability (`writabilityKind`), this fixes all produced warnings being
reported by `chk`, even those that are disabled.

Finally, disabled reports no longer triggering error handling fixes
disabled warnings promoted to errors being able to abort compilation.
    @zerbina
    zerbina committed Oct 7, 2023
    Configuration menu
    Copy the full SHA
    3d8c251 View commit details
    Browse the repository at this point in the history

  2. tests: update tchk1.nim 

    The LineTooLong hints are no longer reported, as it should be.
    @zerbina
    zerbina committed Oct 7, 2023
    Configuration menu
    Copy the full SHA
    74a15cb View commit details
    Browse the repository at this point in the history

  3. tests: add a regression test for warnings/hints

    @zerbina
    zerbina committed Oct 7, 2023
    Configuration menu
    Copy the full SHA
    a91ebfe View commit details
    Browse the repository at this point in the history