guibou / PyF

Haskell QuasiQuoter for String Formatting
BSD 3-Clause "New" or "Revised" License
66 stars 13 forks source link

Context during error when parsing haskell expression #16

Closed guibou closed 3 years ago

guibou commented 5 years ago

The following:

[f| ..... {this is a malformed "haskell expression}

Will leads to an error, such as Improperly terminated string pointing at the closing curly brace. However, the error may be that the user forgot to double the opening curly brace.

PyF may give more context in this case.

guibou commented 5 years ago

This is really a parser issue. haskell-src-exts or GHC internal parser (if we implement #9) may improve in the future but there is nothing we can really do with it.

See upstream issue here:

https://gitlab.haskell.org/ghc/ghc/issues/16955

guibou commented 3 years ago

61 now uses the GHC Api, but simply returns "Error", we should use the error from the GHC parser itself. I already have code for this:

diff --git a/src/PyF/Internal/Parser.hs b/src/PyF/Internal/Parser.hs
index e434886..3fc6f8d 100644
--- a/src/PyF/Internal/Parser.hs
+++ b/src/PyF/Internal/Parser.hs
@@ -32,9 +32,19 @@ import HsExpr as Expr
 import HsExtension as Ext
 #endif

+#if MIN_VERSION_ghc(9,0,0)
+import GHC.Utils.Error (pprErrMsgBagWithLoc)
+import GHC.Utils.Outputable (hcat, showSDoc)
+#else
+import ErrUtils (pprErrMsgBagWithLoc)
+import GhcPlugins (showSDoc)
+import Outputable (hcat)
+#endif
+
 import qualified PyF.Internal.ParserEx as ParseExp
+import Debug.Trace (traceShow)

-parseExpression :: String -> DynFlags -> Either (Int, Int) (HsExpr GhcPs)
+parseExpression :: String -> DynFlags -> Either (Int, Int, String) (HsExpr GhcPs)
 parseExpression s dynFlags =
   case ParseExp.parseExpression s dynFlags of
     POk _ locatedExpr ->
@@ -42,13 +52,18 @@ parseExpression s dynFlags =
        in Right
             expr
 #if MIN_VERSION_ghc(9,0,0)
-    PFailed PState{loc=SrcLoc.psRealLoc -> srcLoc} ->
+    PFailed PState{loc=SrcLoc.psRealLoc -> srcLoc, messages=msgs} ->
 #elif MIN_VERSION_ghc(8,10,0)
-    PFailed PState{loc=srcLoc} ->
+    PFailed PState{loc=srcLoc, messages=msgs} ->
 #else
     -- TODO: check for pattern failure
-    PFailed _ (SrcLoc.srcSpanEnd -> SrcLoc.RealSrcLoc srcLoc) _ ->
+    PFailed msgs (SrcLoc.srcSpanEnd -> SrcLoc.RealSrcLoc srcLoc) _ ->
 #endif
             let line = SrcLoc.srcLocLine srcLoc
                 col = SrcLoc.srcLocCol srcLoc
-             in Left (line, col)
+                -- TODO: do not ignore "warnMessages"
+                -- I have no idea what they can be
+                (warnMessages, errorMessages) = msgs dynFlags
+                err = showSDoc dynFlags (hcat $ pprErrMsgBagWithLoc errorMessages)
+                warns = showSDoc dynFlags (hcat $ pprErrMsgBagWithLoc warnMessages)
+             in traceShow warns $ Left (line, col, err)
diff --git a/src/PyF/Internal/PythonSyntax.hs b/src/PyF/Internal/PythonSyntax.hs
index dd22336..4f032ab 100644
--- a/src/PyF/Internal/PythonSyntax.hs
+++ b/src/PyF/Internal/PythonSyntax.hs
@@ -231,9 +231,8 @@ evalExpr exts exprParser = do
   case ParseExp.parseExpression s dynFlags of
     Right expr ->
       pure (toExp dynFlags (applyFixities (preludeFixities ++ baseFixities) expr))
-    Left (line, col) -> do
-      let err = "Parse error"
-          linesBefore = take (line - 1) (lines s)
+    Left (line, col, err) -> do
+      let linesBefore = take (line - 1) (lines s)
           currentOffset = length (unlines linesBefore) + col - 2
       setOffset (offset + currentOffset)
       fancyFailure (Set.singleton (ErrorFail err))

Will add it once #61 is merged.

In the meantime, I took the time to work on the upstream ticket and GHC should have better parsing error in the future once my PR will be finished.