Fix comment in empty export list

[brandonchinn178]

Resolves #906

[brandonchinn178]

@mrkkrp is it possible to get this PR reviewed any time soon?

[amesgen]

@mrkkrp and I actually talked about this some time ago, but I forgot to comment here; sorry for the delay!

The changes are well-separated, but due to the heuristic nature of comment placing, the changes in espc. the last commit are rather ad-hoc, and introduce (albeit relatively isolated) complexity that seems somewhat disproprionate compared to the low severity of the issue they are resolving, as you note in #906.

Therefore, I played a bit around to see if there maybe is an easier way to fix the issue, and I think the core issue for this (and related bugs) is that when a specific region with a list of items (like a module exports list, but also a record field declaration, see the following example) is empty, there is no chance for a comment to be printed inside because there is no "last" element before which it could be printed. Concretely, this means that the comments in this example "escape" their enclosing brackets:

module Foo
  ( -- a
  )
where

data Test = Test {
  -- b
  }

Hence, an idea would be to use located with bogus spans corresponding to the opening and closing bracket. I implemented this here, and it indeed seems to work in principle (and might even be adapted to work e.g. with the record example above): it passes all tests, but there are some idempotency issues in the hackageTests.

I might have some time this week to see if the idempotency issue of this approach can be resolved relatively easily, but also feel free to have a look, as you might have considered something similar when working on this PR, but discarded it (or other approaches).

[brandonchinn178]

That does look much better 😃 I do like some of the commits in this branch that clean up some of the comments code (e.g. the popComment commit is much more readable IMO), so maybe I can put those up in a separate cleanup PR. But 👍 to your approach

[brandonchinn178]

@amesgen any update on your separate approach?

[amesgen]

@amesgen any update on your separate approach?

Sorry, I finally opened #1013. I agree that some commits in this PR would be nice to incorporate just as generic code cleanup, feel free to open a PR for those!