Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cradle dependencies to session loading errors #3779

Merged
merged 6 commits into from
Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ghcide/ghcide.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ library
Development.IDE.LSP.Outline
Development.IDE.LSP.Server
Development.IDE.Session
Development.IDE.Session.Diagnostics
Development.IDE.Spans.Common
Development.IDE.Spans.Documentation
Development.IDE.Spans.AtPoint
Expand Down
72 changes: 2 additions & 70 deletions ghcide/session-loader/Development/IDE/Session.hs
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,12 @@
import Data.Bifunctor
import qualified Data.ByteString.Base16 as B16
import qualified Data.ByteString.Char8 as B
import Data.Char (isLower)
import Data.Default
import Data.Either.Extra
import Data.Function
import Data.Hashable hiding (hash)
import qualified Data.HashMap.Strict as HM
import Data.List
import Data.List.Extra (dropPrefix, split)
import qualified Data.Map.Strict as Map
import Data.Maybe
import Data.Proxy
Expand Down Expand Up @@ -69,7 +67,6 @@
import Development.IDE.Types.Options
import GHC.Check
import qualified HIE.Bios as HieBios
import qualified HIE.Bios.Cradle as HieBios
import HIE.Bios.Environment hiding (getCacheDir)
import HIE.Bios.Types hiding (Log)
import qualified HIE.Bios.Types as HieBios
Expand Down Expand Up @@ -103,6 +100,7 @@
import qualified Data.HashSet as Set
import Database.SQLite.Simple
import Development.IDE.Core.Tracing (withTrace)
import Development.IDE.Session.Diagnostics (renderCradleError)
import Development.IDE.Types.Shake (WithHieDb)
import HieDb.Create
import HieDb.Types
Expand Down Expand Up @@ -618,7 +616,7 @@
void $ modifyVar' fileToFlags $
Map.insert hieYaml (HM.fromList (concatMap toFlagsMap all_targets))
void $ modifyVar' filesMap $
flip HM.union (HM.fromList (zip (map fst $ concatMap toFlagsMap all_targets) (repeat hieYaml)))

Check warning on line 619 in ghcide/session-loader/Development/IDE/Session.hs

View workflow job for this annotation

GitHub Actions / Hlint check run

Warning in loadSessionWithOptions in module Development.IDE.Session: Use map ▫︎ Found: "zip (map fst $ concatMap toFlagsMap all_targets) (repeat hieYaml)" ▫︎ Perhaps: "map (, hieYaml) (map fst $ concatMap toFlagsMap all_targets)" ▫︎ Note: may require `{-# LANGUAGE TupleSections #-}` adding to the top of the file

void $ extendKnownTargets all_targets

Expand Down Expand Up @@ -685,7 +683,7 @@
Left err -> do
dep_info <- getDependencyInfo (maybeToList hieYaml)
let ncfp = toNormalizedFilePath' cfp
let res = (map (renderCradleError cradle ncfp) err, Nothing)
let res = (map (\err' -> renderCradleError err' cradle ncfp) err, Nothing)
void $ modifyVar' fileToFlags $
Map.insertWith HM.union hieYaml (HM.singleton ncfp (res, dep_info))
void $ modifyVar' filesMap $ HM.insert ncfp hieYaml
Expand Down Expand Up @@ -822,7 +820,7 @@
-> IO ( [TargetDetails], (IdeResult HscEnvEq, DependencyInfo))
newComponentCache recorder exts cradlePath cfp hsc_env uids ci = do
let df = componentDynFlags ci
hscEnv' <-

Check warning on line 823 in ghcide/session-loader/Development/IDE/Session.hs

View workflow job for this annotation

GitHub Actions / Hlint check run

Suggestion in newComponentCache in module Development.IDE.Session: Use let ▫︎ Found: "hscEnv' <- pure\n $ hscSetFlags\n df hsc_env {hsc_IC = (hsc_IC hsc_env) {ic_dflags = df}}" ▫︎ Perhaps: "let hscEnv'\n = hscSetFlags\n df hsc_env {hsc_IC = (hsc_IC hsc_env) {ic_dflags = df}}"
#if MIN_VERSION_ghc(9,3,0)
-- Set up a multi component session with the other units on GHC 9.4
Compat.initUnits (map snd uids) (hscSetFlags df hsc_env)
Expand Down Expand Up @@ -924,72 +922,6 @@
& maybe id setHieDir hieCacheDir
& maybe id setODir oCacheDir


renderCradleError :: Cradle a -> NormalizedFilePath -> CradleError -> FileDiagnostic
renderCradleError cradle nfp (CradleError _ _ec ms) =
ideErrorWithSource (Just "cradle") (Just DiagnosticSeverity_Error) nfp $ T.unlines $ map T.pack userFriendlyMessage
where

userFriendlyMessage :: [String]
userFriendlyMessage
| HieBios.isCabalCradle cradle = fromMaybe ms fileMissingMessage
| otherwise = ms

fileMissingMessage :: Maybe [String]
fileMissingMessage =
multiCradleErrMessage <$> parseMultiCradleErr ms

-- | Information included in Multi Cradle error messages
data MultiCradleErr = MultiCradleErr
{ mcPwd :: FilePath
, mcFilePath :: FilePath
, mcPrefixes :: [(FilePath, String)]
} deriving (Show)

-- | Attempt to parse a multi-cradle message
parseMultiCradleErr :: [String] -> Maybe MultiCradleErr
parseMultiCradleErr ms = do
_ <- lineAfter "Multi Cradle: "
wd <- lineAfter "pwd: "
fp <- lineAfter "filepath: "
ps <- prefixes
pure $ MultiCradleErr wd fp ps

where
lineAfter :: String -> Maybe String
lineAfter pre = listToMaybe $ mapMaybe (stripPrefix pre) ms

prefixes :: Maybe [(FilePath, String)]
prefixes = do
pure $ mapMaybe tuple ms

tuple :: String -> Maybe (String, String)
tuple line = do
line' <- surround '(' line ')'
[f, s] <- pure $ split (==',') line'
pure (f, s)

-- extracts the string surrounded by required characters
surround :: Char -> String -> Char -> Maybe String
surround start s end = do
guard (listToMaybe s == Just start)
guard (listToMaybe (reverse s) == Just end)
pure $ drop 1 $ take (length s - 1) s

multiCradleErrMessage :: MultiCradleErr -> [String]
multiCradleErrMessage e =
[ "Loading the module '" <> moduleFileName <> "' failed. It may not be listed in your .cabal file!"
, "Perhaps you need to add `"<> moduleName <> "` to other-modules or exposed-modules."
, "For more information, visit: https://cabal.readthedocs.io/en/3.4/developing-packages.html#modules-included-in-the-package"
, ""
] <> map prefix (mcPrefixes e)
where
localFilePath f = dropWhile (==pathSeparator) $ dropPrefix (mcPwd e) f
moduleFileName = localFilePath $ mcFilePath e
moduleName = intercalate "." $ map dropExtension $ dropWhile isSourceFolder $ splitDirectories moduleFileName
isSourceFolder p = all isLower $ take 1 p
prefix (f, r) = f <> " - " <> r

-- See Note [Multi Cradle Dependency Info]
type DependencyInfo = Map.Map FilePath (Maybe UTCTime)
type HieMap = Map.Map (Maybe FilePath) (HscEnv, [RawComponentInfo])
Expand Down
106 changes: 106 additions & 0 deletions ghcide/session-loader/Development/IDE/Session/Diagnostics.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
{-# LANGUAGE DeriveAnyClass #-}

Check warning on line 1 in ghcide/session-loader/Development/IDE/Session/Diagnostics.hs

View workflow job for this annotation

GitHub Actions / Hlint check run

Warning in module Development.IDE.Session.Diagnostics: Use module export list ▫︎ Found: "module Development.IDE.Session.Diagnostics where" ▫︎ Perhaps: "module Development.IDE.Session.Diagnostics (\n module Development.IDE.Session.Diagnostics\n ) where" ▫︎ Note: an explicit list is usually better
{-# LANGUAGE DeriveGeneric #-}

module Development.IDE.Session.Diagnostics where
import Control.Applicative
import Control.Monad
import qualified Data.Aeson as Aeson
import Data.List
import Data.List.Extra (split)
import Data.Maybe
import qualified Data.Text as T
import Development.IDE.Types.Diagnostics
import Development.IDE.Types.Location
import GHC.Generics
import qualified HIE.Bios.Cradle as HieBios
import HIE.Bios.Types hiding (Log)
import System.FilePath

data CradleErrorDetails =
CradleErrorDetails
{ cabalProjectFiles :: [FilePath]
-- ^ files related to the cradle error
-- i.e. .cabal, cabal.project, etc.
} deriving (Show, Eq, Ord, Read, Generic, Aeson.ToJSON, Aeson.FromJSON)

{- | Takes a cradle error, the corresponding cradle and the file path where
the cradle error occurred (of the file we attempted to load).
Depicts the cradle error in a user-friendly way.
-}
renderCradleError :: CradleError -> Cradle a -> NormalizedFilePath -> FileDiagnostic
renderCradleError (CradleError deps _ec ms) cradle nfp
| HieBios.isCabalCradle cradle =
let (fp, showDiag, diag) = ideErrorWithSource (Just "cradle") (Just DiagnosticSeverity_Error) nfp $ T.unlines $ map T.pack userFriendlyMessage in
(fp, showDiag, diag{_data_ = Just $ Aeson.toJSON CradleErrorDetails{cabalProjectFiles=absDeps}})
| otherwise = ideErrorWithSource (Just "cradle") (Just DiagnosticSeverity_Error) nfp $ T.unlines $ map T.pack userFriendlyMessage
where
absDeps = fmap (cradleRootDir cradle </>) deps
userFriendlyMessage :: [String]
userFriendlyMessage
| HieBios.isCabalCradle cradle = fromMaybe ms $ fileMissingMessage <|> mkUnknownModuleMessage
| otherwise = ms

mkUnknownModuleMessage :: Maybe [String]
mkUnknownModuleMessage
| any (isInfixOf "Failed extracting script block:") ms =
Just $ unknownModuleMessage (fromNormalizedFilePath nfp)
| otherwise = Nothing

fileMissingMessage :: Maybe [String]
fileMissingMessage =
multiCradleErrMessage <$> parseMultiCradleErr ms

-- | Information included in Multi Cradle error messages
data MultiCradleErr = MultiCradleErr
{ mcPwd :: FilePath
, mcFilePath :: FilePath
, mcPrefixes :: [(FilePath, String)]
} deriving (Show)

-- | Attempt to parse a multi-cradle message
fendor marked this conversation as resolved.
Show resolved Hide resolved
parseMultiCradleErr :: [String] -> Maybe MultiCradleErr
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remind me again why we aren't just doing structured errors from hie-bios? 😅

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reason :) Ill put it on my long term list of things to do after the release is finished.

parseMultiCradleErr ms = do
_ <- lineAfter "Multi Cradle: "
wd <- lineAfter "pwd: "
fp <- lineAfter "filepath: "
ps <- prefixes
pure $ MultiCradleErr wd fp ps

where
lineAfter :: String -> Maybe String
lineAfter pre = listToMaybe $ mapMaybe (stripPrefix pre) ms

prefixes :: Maybe [(FilePath, String)]
prefixes = do
pure $ mapMaybe tuple ms

tuple :: String -> Maybe (String, String)
tuple line = do
line' <- surround '(' line ')'
[f, s] <- pure $ split (==',') line'
pure (f, s)

-- extracts the string surrounded by required characters
surround :: Char -> String -> Char -> Maybe String
surround start s end = do
guard (listToMaybe s == Just start)
guard (listToMaybe (reverse s) == Just end)
pure $ drop 1 $ take (length s - 1) s

Check warning on line 88 in ghcide/session-loader/Development/IDE/Session/Diagnostics.hs

View workflow job for this annotation

GitHub Actions / Hlint check run

Warning in parseMultiCradleErr in module Development.IDE.Session.Diagnostics: Use drop1 ▫︎ Found: "drop 1" ▫︎ Perhaps: "drop1"

multiCradleErrMessage :: MultiCradleErr -> [String]
multiCradleErrMessage e =
unknownModuleMessage (mcFilePath e)
<> [""]
<> map prefix (mcPrefixes e)
where
prefix (f, r) = f <> " - " <> r

unknownModuleMessage :: String -> [String]
unknownModuleMessage moduleFileName =
[ "Loading the module '" <> moduleFileName <> "' failed."
, ""
, "It may not be listed in your .cabal file!"
, "Perhaps you need to add `"<> dropExtension (takeFileName moduleFileName) <> "` to other-modules or exposed-modules."
, ""
, "For more information, visit: https://cabal.readthedocs.io/en/3.4/developing-packages.html#modules-included-in-the-package"
]
50 changes: 20 additions & 30 deletions test/functional/FunctionalBadProject.hs
Original file line number Diff line number Diff line change
Expand Up @@ -2,37 +2,27 @@

module FunctionalBadProject (tests) where

-- import Control.Lens hiding (List)
-- import Control.Monad.IO.Class
-- import qualified Data.Text as T
-- import Language.LSP.Test hiding (message)
-- import Language.LSP.Types as LSP
-- import Language.LSP.Types.Lens as LSP hiding (contents, error )
import Control.Lens
import qualified Data.Text as T
import qualified Language.LSP.Protocol.Lens as L
import Test.Hls
import Test.Hls.Command


-- ---------------------------------------------------------------------
-- TODO: Currently this can not succeed, since such an error is thrown in "runActionWithContext" which
-- can produce diagnostics at the moment. Needs more investigation
-- TODO: @fendor: Add issue link here
--
tests :: TestTree
tests = testGroup "behaviour on malformed projects" [
testCase "no test executed" $ True @?= True
tests = testGroup "behaviour on malformed projects"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is much more sophisticated than what I was thinking (although I imagine we'll want it anyway). I was just thinking of a unit test for the string matching function!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that mean we don't want it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IDK, potentially good to have both?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the tests are there and we shouldn't have them forever, any way. Ideally cabal gives us better error messages and hie-bios doesn't demand the parsing. Should be good enough, imo.

[ testCase "Missing module diagnostic" $ do
runSession hlsCommand fullCaps "test/testdata/missingModuleTest/missingModule/" $ do
doc <- openDoc "src/MyLib.hs" "haskell"
[diag] <- waitForDiagnosticsFrom doc
liftIO $ assertBool "missing module name" $ "MyLib" `T.isInfixOf` (diag ^. L.message)
liftIO $ assertBool "module missing context" $ "may not be listed" `T.isInfixOf` (diag ^. L.message)
, testCase "Missing module diagnostic - no matching prefix" $ do
runSession hlsCommand fullCaps "test/testdata/missingModuleTest/noPrefixMatch/" $ do
doc <- openDoc "app/Other.hs" "haskell"
[diag] <- waitForDiagnosticsFrom doc
liftIO $ assertBool "missing module name" $
"Other" `T.isInfixOf` (diag ^. L.message)
liftIO $ assertBool "hie-bios message" $
"Cabal {component = Just \"exe:testExe\"}" `T.isInfixOf` (diag ^. L.message)
]

-- testCase "deals with cabal file with unsatisfiable dependency" $
-- runSession hlsCommandExamplePlugin codeActionSupportCaps "test/testdata/badProjects/cabal" $ do
-- _doc <- openDoc "Foo.hs" "haskell"

-- diags@(d:_) <- waitForDiagnosticsSource "bios"
-- -- liftIO $ show diags @?= ""
-- -- liftIO $ putStrLn $ show diags
-- -- liftIO $ putStrLn "a"
-- liftIO $ do
-- length diags @?= 1
-- d ^. range @?= Range (Position 0 0) (Position 1 0)
-- d ^. severity @?= (Just DsError)
-- d ^. code @?= Nothing
-- d ^. source @?= Just "bios"
-- d ^. message @?=
-- (T.pack "readCreateProcess: stack \"build\" \"--only-configure\" \".\" (exit 1): failed\n")
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
packages: ./
2 changes: 2 additions & 0 deletions test/testdata/missingModuleTest/missingModule/hie.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
cradle:
cabal:
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
cabal-version: 3.4
name: missingModule
version: 0.1.0.0
build-type: Simple

library
hs-source-dirs: ./src/
exposed-modules:
build-depends: base
default-language: Haskell2010
5 changes: 5 additions & 0 deletions test/testdata/missingModuleTest/missingModule/src/MyLib.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module MyLib where

someFunc :: IO ()
someFunc = do
putStrLn "someFunc"
4 changes: 4 additions & 0 deletions test/testdata/missingModuleTest/noPrefixMatch/app/Main.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

main :: IO ()
main = do
putStrLn "someFunc"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module Other where
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
packages: ./
4 changes: 4 additions & 0 deletions test/testdata/missingModuleTest/noPrefixMatch/hie.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
cradle:
cabal:
- path: ./app/Main.hs
component: exe:testExe
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
cabal-version: 3.4
name: noPrefixMatch
version: 0.1.0.0
build-type: Simple

executable testExe
main-is: Main.hs
hs-source-dirs: app
build-depends: base
Loading