Incorrect traversal for AppSpine

[purefunctor]

There seems to be a bug in the traversal for AppSpine, which was introduced by #51. Here's a small reproduction:

module Main where

import Prelude

import Effect (Effect)
import Effect.Console (log)
import PureScript.CST (RecoveredParserResult(..), parseModule)
import PureScript.CST.Traversal (defaultVisitorM, topDownTraversal, traverseModule)
import PureScript.CST.Types (Module)

moduleSource ∷ String
moduleSource =
  """
module Main where

x = f a
"""

onExpr :: Module Void -> Effect (Module Void)
onExpr = traverseModule $ topDownTraversal $ defaultVisitorM
  { onExpr = \e -> do
      log "Hey!"
      pure e
  }

main ∷ Effect Unit
main = case parseModule moduleSource of
  ParseSucceeded m → do
    _ <- onExpr m
    pure unit
  _ →
    log "Failed."

Expected output

Hey! is printed three times, for f a, f, and then a respectively.

Likely issue

The issue seems to stem from the use of traverseExpr rather than k.onExpr, as seen on this diff

More specifically, ExprApp was originally:

  ExprApp expr args -> ExprApp <$> k.onExpr expr <*> traverse k.onExpr args

Then it was changed to:

  ExprApp expr args -> ExprApp <$> k.onExpr expr <*> traverse (traverseExprAppSpine k) args
  -- ...

traverseExprAppSpine
  :: forall e f r
   . Applicative f
  => { | OnBinder (Rewrite e f) + OnExpr (Rewrite e f) + OnType (Rewrite e f) + r }
  -> Rewrite e f (AppSpine Expr)
traverseExprAppSpine k = case _ of
  AppType tok ty -> AppType tok <$> traverseType k ty
  AppTerm expr -> AppTerm <$> traverseExpr k expr  -- not k.onExpr anymore

(Shouldn't traverseType be k.onType as well?)

[natefaubion]

Yeah, that seems wrong!