Bug in comment parsing in route definitions

[parsonsmatt]

Comments mess up the parsing of routes. Consider this block:

/nest/ NestR !NestingAttr:

  /spaces      SpacedR   GET !NonNested

  /nest2 Nest2:
    /           GetPostR  GET POST
    /get        Get2      GET
    /post       Post2         POST
--    /#Int       Delete2            DELETE
  /nest3 Nest3:
    /get        Get3      GET
    /post       Post3         POST
--    /#Int       Delete3            DELETE

The code generator should produce this as a ResourceParent with a leaf child SpacedR and two ResourceParent children, one for Nest3 and Nest2 . Instead, the comment starting at beginning-of-line causes this to parse Nest3 inside of Route Hierarchy.

It is likely that fixing this may cause significant downstream bugs and behavioral problems by changing the structure of the generated routes to match what is written in the file. As an example, the current actual URL structure of the Route Hierarchy will accept /nest3/get and /nest/nest2/get. Existing clients will be using this, and since all the typed route stuff is type-checking things fine, this will Work, even if it is misleading to what people intended.

I would expect people to notice bugsin the case like /admin/#Int AdminR:, since the YesodDispatch would not generate routes that actually have the Int parameter - but still.

[parsonsmatt]

Hypothesis: if we just "fix" this behavior, then we will almost certainly break route generation for people. It should be a compiler error if anyone is actually using the constructors. For instance, the constructor Nest3 will go from being a constructor on Route Hierarchy to a constructor on NestR. This will break any code that pattern matches on the routes to introspect on them, and also any test code that uses yesod-test or hspec-yesod and touches that route.

So, for us to break downstream in a way that doesn't cause compilation break, a user must:

1. Have a nested route, with a preceeding comment, on the wrong indentation level,
2. Do not have any tests that exercise the route
3. Do not have any Route site pattern matches that exercise the route.

This indicates a strong lack of discipline on the part of the developer, and I'd also expect them to not be compiling with -Werror, so it is also likely that simply using a warning in this case would still break downstream.

parseRoutes operates in TemplateHaskell, so we can report warnings and errors. However, these warnings won't have a specific warning category (feature request here), and so can't be disabled without actually fixing the problem (or modifying the source).

To be maximally safe, I think we need to by default, reportError if we see this. This will force developers to acknowledge the problem. We can then provide a few "fixes":

1. A different parseRoutes that maintains the old behavior, so users don't have to change anything.
2. A different parseRoutes that has the fixed behavior.
3. Instruct users on how to modify the comments to not potentially break the route generation.

Once the reportError has been out in the wild for a while, we can put the "fixed" behavior in parseRoutes default function.

Potentially we can configure this, too, but the form of a quasiquoter [parseRoutes| ... |] makes configuration tricky- you can't write, ie, [parseRoutes options| ... |]. $(parseRoutesFromFile opts "filename") works, of course, but that's, eh.