Fix parser warnings order

[omarjatoi]

See #11269 for more information. This PR changes the parse results to append warnings so they're displayed in-order.

---

This PR modifies behaviour or interface

Include the following checklist in your PR:

• Patches conform to the coding conventions.
• Any changes that could be relevant to users have been recorded in the changelog.
  • Is the change significant? If so, remember to add significance: significant in the changelog file.
• The documentation has been updated, if necessary.
• Manual QA notes have been included.
• Tests have been added. (NOTE: I updated the ordering in existing test files)

[geekosaur]

Fine aside from the missing field in the changelog.

[omarjatoi]

Thanks! Let me get that fixed.

[geekosaur]

I do wonder if just doing a reverse at the end would be better, but on the other hand if they have enough warnings for that to make a significant difference then they have bigger problems than warning collection. 😁

[omarjatoi]

Yeah I thought of doing that at first, and figured it maybe made sense to just return the warnings in the correct order instead of having to reverse them. But I do not feel strongly, if that's preferred I'd be happy to make that change.

[geekosaur]

No, I don't think it's really necessary. The downside of xs ++ [x] is that it has to traverse xs, but again if they have that many warnings that this shows, they have bigger problems.

[omarjatoi]

Just wondering if I need to be looking at those 'Validate' CI failures more closely. At least some of the failures seem to be flakes. Let me know if there's anything else I can look into to request a second approval to merge!

[geekosaur]

The first one I opened has a newly reversed warning.

          --- encoding-0.8.cabal:1:1: The field "name" is specified more than once at positions 1:1, 2:1
              encoding-0.8.cabal:4:22: Packages with 'cabal-version: 1.12' or later should specify a specific version of the Cabal spec of the form 'cabal-version: x.y'. Use 'cabal-version: 1.12'.
          +++ encoding-0.8.cabal:1:1: The field "name" is specified more than once at positions 1:1, 2:1

[omarjatoi]

The first one I opened has a newly reversed warning.

          --- encoding-0.8.cabal:1:1: The field "name" is specified more than once at positions 1:1, 2:1

              encoding-0.8.cabal:4:22: Packages with 'cabal-version: 1.12' or later should specify a specific version of the Cabal spec of the form 'cabal-version: x.y'. Use 'cabal-version: 1.12'.

          +++ encoding-0.8.cabal:1:1: The field "name" is specified more than once at positions 1:1, 2:1

Ah okay, thanks -- I'll take a look at these

[philderbeast]

@omarjatoi could you please rebase this on master?

[omarjatoi]

I've rebased, but after investigating some of the errors it seems that we parse the cabal-version first, and then the rest of the fields, so this approach still fails when there's a warning for the cabal-version field, e.g. in encoding-0.8.cabal.format

I created another branch with an approach that sorts by position (see: omarjatoi@8caeb97) which gets us what we want in all cases, I'm open to feedback on whether or not that approach makes sense. If yes, I'm happy to apply that in this PR.

EDIT: I've pushed 98697f1 to this branch to let CI run, if this looks good I'll squash my commits and clean up the PR description.

[omarjatoi]

Ok that did give us a successful CI run, I've squashed the changes into a single commit now. @geekosaur, I re-requested a review since I've changed the implementation since you approved, please take a look.

Thanks!

[zlonast]

Hi @omarjatoi, CI is a bit bad, can you rebase?

[omarjatoi]

Hi @omarjatoi, CI is a bit bad, can you rebase?

done, thanks for the heads up!