cabal icon indicating copy to clipboard operation
cabal copied to clipboard

Reimplement `cabal check`

Open ffaf1 opened this issue 3 years ago • 5 comments

Where are we now

cabal check is a tool to check the correctness of a .cabal file and more generally to provide additional useful output (warnings, especially related to uploading the package to Hackage).

As now cabal check is an imprecise tool: simple example (#7423), where -O2 gets a warning even when it is passed under a (cabal) flag.

Where we want to be

A number of these errors are caused by the way the checker operates today. Immediately noticeable: inefficiency (we go through pkg numerous times), lack of scoping awareness.

The correct way to tackle the problem is to traverse the AST.

Steps to achieve the goal

  • [X] write tests to document how we would like cabal check to behave. We should scoop as much as possible from the issues in the issue tracker. The resulting modifications will have the shape of a number of small pull requests, easy to parse and easily mergeable
  • [ ] rewrite checkPackage to pattern match on GenericPackageDescription (instead of using accessors). This buys us the peace of mind that if GenericPackageDescription is modified (fields added), then a compilation error will gently point the user to add relevant tests. This should a slightly bigger modification and maybe not uncontroversial (accessors are handier than pattern matching a big type).
  • [X] tipify cabal check errors (as now they are Strings). Again a propaedeutic task, this should be relatively self contained and uncontroversial .
  • [ ] write the traversing function. First flesh out the types (get comments), then implement it. The eventual pull request will be probably bigger and slightly more difficult to comment on; this is were having written a lot of tests will come handy. This could be done as a third party tool first (disadvantages: being less close to Cabal codebase means more to rewrite).

This issue will be useful to document development and gather comments.

ffaf1 avatar Jun 13 '22 08:06 ffaf1

First batch of tests here: #8248 There is quite some work to do.

ffaf1 avatar Jun 22 '22 15:06 ffaf1

With #8248 and #8250 merged (105 tests in total) the “write a testsuite” box is checked. We are now reasonably sure reimplementation won’t break current behaviour; we are now reasonably sure in the future we will be warned if a check becomes redundant (e.g. because of improved parsing).

Next step: convert String errors to typed ones.

ffaf1 avatar Jul 01 '22 15:07 ffaf1

With #8269 merged (structured errors), half of the work is hopefully done.

Next steps: study GenericPackageDescription, see which checks are reimplementable by just pattern matching on the AST, see which ones are not. Most likely we will need to introduce/check scoping (how to do it? pass some kind of state around?).

Once ready: write a short implementation proposal.

ffaf1 avatar Jul 21 '22 04:07 ffaf1

@ffaf1: Great work so far! Please remember to submit your Contributor Midterm Evaluation to GSoc by tomorrow!

andreasabel avatar Jul 28 '22 18:07 andreasabel

Thanks for the reminder, will do!

ffaf1 avatar Jul 28 '22 18:07 ffaf1