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 source file to project parse errors and warnings #10644

Draft
wants to merge 32 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
745893a
Add ProjectParseResult
philderbeast Dec 13, 2024
5149950
Add test for import parse warnings
philderbeast Dec 14, 2024
a7387da
Remove added type sigs, use e for error type
philderbeast Dec 15, 2024
1d10854
Move ProjectParseResult into its own module
philderbeast Dec 15, 2024
5d7a767
Import qualified from Deprecated.ParseUtils
philderbeast Dec 15, 2024
36f04f5
Reverse warnings so they are in line number order
philderbeast Dec 15, 2024
f0b219d
Report parse result error in imported config
philderbeast Dec 15, 2024
43de9d8
Split project test into warning and error tests
philderbeast Dec 15, 2024
14fbf07
Add type synonyms for project parse
philderbeast Dec 16, 2024
6d5cf18
Extract function reportProjectParseWarnings
philderbeast Dec 16, 2024
8e73a77
Show the snippet that doesn't parse
philderbeast Dec 16, 2024
da9a444
Add if, elif and else test projects
philderbeast Dec 16, 2024
3887728
Fix else for elif typo
philderbeast Dec 16, 2024
a9bc878
Show provenance if not root
philderbeast Dec 16, 2024
926f089
Rerun expected output with provenance
philderbeast Dec 16, 2024
4d8d2fa
Redo ParseWarningProvenence with ordered output
philderbeast Dec 16, 2024
d7f57f0
Add ProjectParseError record
philderbeast Dec 16, 2024
b3780ed
Reword badly formed comment lines
philderbeast Dec 16, 2024
eb8dca5
Satisfy fix-whitespace
philderbeast Dec 16, 2024
187eb1f
Add changelog entry
philderbeast Dec 16, 2024
9f05ce2
Updated - indented expectation
philderbeast Dec 16, 2024
044b568
No snippet when modifying compiler under condition
philderbeast Dec 16, 2024
5013202
Only show custom message with snippet
philderbeast Dec 16, 2024
caebeec
Rerun expected output with source
philderbeast Dec 16, 2024
4932fd9
Use a Doc for the ReportParseResult message
philderbeast Dec 17, 2024
69ebe78
Update expected .out files
philderbeast Dec 17, 2024
120fe42
Use normalized path when recursing
philderbeast Dec 17, 2024
a468360
.
philderbeast Dec 17, 2024
db907f0
Consistent projectParse ... source
philderbeast Dec 17, 2024
5de0cfb
Consistent projectParse ... normSource
philderbeast Dec 17, 2024
76c4a3f
Use normalizeWindowsOutput
philderbeast Dec 17, 2024
d45311c
Use OS-specific splitPath before comparing projects
philderbeast Dec 17, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,16 @@ import Data.Coerce (coerce)
import Data.List.NonEmpty ((<|))
import Network.URI (parseURI, parseAbsoluteURI)
import System.Directory
import System.FilePath
import System.FilePath hiding (splitPath)
import qualified System.FilePath as FP (splitPath)
import qualified System.FilePath.Posix as Posix
import qualified System.FilePath.Windows as Windows
import qualified Data.List.NonEmpty as NE
import Distribution.Solver.Modular.Version (VR)
import Distribution.Pretty (prettyShow)
import Text.PrettyPrint
import Distribution.Simple.Utils (ordNub)
import Distribution.System

-- | Path to a configuration file, either a singleton project root, or a longer
-- list representing a path to an import. The path is a non-empty list that we
Expand All @@ -58,6 +62,14 @@ newtype ProjectConfigPath = ProjectConfigPath (NonEmpty FilePath)
-- The project itself, a single element root path, compared to any of the
-- configuration paths it imports, should always sort first. Comparing one
-- project root path against another is done lexically.
--
-- For comparison purposes, path separators are normalized to the @buildOS@
-- platform's path separator.
--
-- >>> let abFwd = ProjectConfigPath $ "a/b.config" :| []
-- >>> let abBwd = ProjectConfigPath $ "a\\b.config" :| []
-- >>> compare abFwd abBwd
-- EQ
instance Ord ProjectConfigPath where
compare pa@(ProjectConfigPath (NE.toList -> as)) pb@(ProjectConfigPath (NE.toList -> bs)) =
case (as, bs) of
Expand All @@ -66,7 +78,7 @@ instance Ord ProjectConfigPath where
-- this though, do a comparison anyway when both sides have length
-- 1. The root path, the project itself, should always be the first
-- path in a sorted listing.
([a], [b]) -> compare a b
([a], [b]) -> compare (splitPath a) (splitPath b)
([_], _) -> LT
(_, [_]) -> GT

Expand All @@ -80,6 +92,12 @@ instance Ord ProjectConfigPath where
P.<> compare (length aPaths) (length bPaths)
P.<> compare aPaths bPaths
where
splitPath = FP.splitPath . normSep where
normSep p =
if buildOS == Windows
then Windows.joinPath $ Windows.splitDirectories [if c == '/' then '\\' else c| c <- p]
else Posix.joinPath $ Posix.splitDirectories [if c == '\\' then '/' else c| c <- p]

aPaths = splitPath <$> as
bPaths = splitPath <$> bs
aImporters = snd $ unconsProjectConfigPath pa
Expand Down
1 change: 1 addition & 0 deletions cabal-install/cabal-install.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ library
-- this modules are moved from Cabal
-- they are needed for as long until cabal-install moves to parsec parser
Distribution.Deprecated.ParseUtils
Distribution.Deprecated.ProjectParseUtils
Distribution.Deprecated.ReadP
Distribution.Deprecated.ViewAsFieldDescr

Expand Down
17 changes: 9 additions & 8 deletions cabal-install/src/Distribution/Client/Errors.hs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ import Distribution.Pretty
import Distribution.Simple (VersionRange)
import Distribution.Simple.Utils
import Network.URI
import Text.PrettyPrint hiding (render, (<>))
import qualified Text.PrettyPrint as PP
import Text.Regex.Posix.ByteString (WrapError)

data CabalInstallException
Expand Down Expand Up @@ -112,7 +114,7 @@ data CabalInstallException
| ParseExtraLinesFailedErr String String
| ParseExtraLinesOkError [PWarning]
| FetchPackageErr
| ReportParseResult String FilePath String String
| ReportParseResult String FilePath String Doc
| ReportSourceRepoProblems String
| BenchActionException
| RenderBenchTargetProblem [String]
Expand Down Expand Up @@ -491,13 +493,12 @@ exceptionMessageCabalInstall e = case e of
ParseExtraLinesOkError ws -> unlines (map (showPWarning "Error parsing additional config lines") ws)
FetchPackageErr -> "fetchPackage: source repos not supported"
ReportParseResult filetype filename line msg ->
"Error parsing "
++ filetype
++ " "
++ filename
++ line
++ ":\n"
++ msg
PP.render $
vcat
-- NOTE: As given to us, the line number string is prefixed by a colon.
[ text "Error parsing" <+> text filetype <+> text filename PP.<> text line PP.<> colon
, nest 1 $ text "-" <+> msg
]
ReportSourceRepoProblems errorStr -> errorStr
BenchActionException ->
"The bench command does not support '--only-dependencies'. "
Expand Down
56 changes: 45 additions & 11 deletions cabal-install/src/Distribution/Client/ProjectConfig.hs
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,9 @@ module Distribution.Client.ProjectConfig
, maxNumFetchJobs
) where

import Distribution.Client.Compat.Prelude
import Text.PrettyPrint (nest, render, text, vcat)
import Distribution.Client.Compat.Prelude hiding (empty)
import Distribution.Simple.Utils (ordNub)
import Text.PrettyPrint (cat, colon, comma, empty, hsep, nest, quotes, render, text, vcat)
import Prelude ()

import Distribution.Client.Glob
Expand Down Expand Up @@ -135,10 +136,12 @@ import Distribution.Client.Utils
( determineNumJobs
)
import qualified Distribution.Deprecated.ParseUtils as OldParser
( ParseResult (..)
, locatedErrorMsg
( locatedErrorMsg
, showPWarning
)
import qualified Distribution.Deprecated.ProjectParseUtils as OldParser
( ProjectParseResult (..)
)
import Distribution.Fields
( PError
, PWarning
Expand Down Expand Up @@ -177,6 +180,7 @@ import Distribution.Simple.Utils
, info
, maybeExit
, notice
, noticeDoc
, rawSystemIOWithEnv
, warn
)
Expand Down Expand Up @@ -240,6 +244,7 @@ import System.IO
, withBinaryFile
)

import Distribution.Deprecated.ProjectParseUtils (ProjectParseError (..), ProjectParseWarning)
import Distribution.Solver.Types.ProjectConfigPath

----------------------------------------
Expand Down Expand Up @@ -874,16 +879,45 @@ readGlobalConfig verbosity configFileFlag = do
monitorFiles [monitorFileHashed configFile]
return (convertLegacyGlobalConfig config)

reportParseResult :: Verbosity -> String -> FilePath -> OldParser.ParseResult ProjectConfigSkeleton -> IO ProjectConfigSkeleton
reportParseResult verbosity _filetype filename (OldParser.ParseOk warnings x) = do
reportProjectParseWarnings :: Verbosity -> FilePath -> [ProjectParseWarning] -> IO ()
reportProjectParseWarnings verbosity projectFile warnings =
unless (null warnings) $
let msg = unlines (map (OldParser.showPWarning (intercalate ", " $ filename : (projectConfigPathRoot <$> projectSkeletonImports x))) warnings)
in warn verbosity msg
let msgs =
[ OldParser.showPWarning pFilename w
| (p, w) <- warnings
, let pFilename = fst $ unconsProjectConfigPath p
]
in noticeDoc verbosity $
vcat
[ (text "Warnings found while parsing the project file" <> comma) <+> (text (takeFileName projectFile) <> colon)
, cat [nest 1 $ text "-" <+> text m | m <- ordNub msgs]
]

reportParseResult :: Verbosity -> String -> FilePath -> OldParser.ProjectParseResult ProjectConfigSkeleton -> IO ProjectConfigSkeleton
reportParseResult verbosity _filetype projectFile (OldParser.ProjectParseOk warnings x) = do
reportProjectParseWarnings verbosity projectFile warnings
return x
reportParseResult verbosity filetype filename (OldParser.ParseFailed err) =
reportParseResult verbosity filetype projectFile (OldParser.ProjectParseFailed (ProjectParseError snippet rootOrImportee err)) = do
let (line, msg) = OldParser.locatedErrorMsg err
errLineNo = maybe "" (\n -> ':' : show n) line
in dieWithException verbosity $ ReportParseResult filetype filename errLineNo msg
let errLineNo = maybe "" (\n -> ':' : show n) line
let (sourceFile, provenance) =
maybe
(projectFile, empty)
( \p ->
( fst $ unconsProjectConfigPath p
, if isTopLevelConfigPath p then empty else docProjectConfigPath p
)
)
rootOrImportee
let doc = case snippet of
Nothing -> vcat (text <$> lines msg)
Just s ->
vcat
[ provenance
, text "Failed to parse" <+> quotes (text s) <+> (text "with error" <> colon)
, nest 2 $ hsep $ text <$> lines msg
]
dieWithException verbosity $ ReportParseResult filetype sourceFile errLineNo doc

---------------------------------------------
-- Finding packages in the project
Expand Down
73 changes: 46 additions & 27 deletions cabal-install/src/Distribution/Client/ProjectConfig/Legacy.hs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
{-# LANGUAGE ConstraintKinds #-}
{-# LANGUAGE DataKinds #-}
{-# LANGUAGE DeriveGeneric #-}
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE NamedFieldPuns #-}
{-# LANGUAGE RecordWildCards #-}
{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE TupleSections #-}
{-# LANGUAGE ViewPatterns #-}

-- | Project configuration, implementation in terms of legacy types.
Expand Down Expand Up @@ -159,6 +161,11 @@ import Distribution.Deprecated.ParseUtils
, syntaxError
)
import qualified Distribution.Deprecated.ParseUtils as ParseUtils
import Distribution.Deprecated.ProjectParseUtils
( ProjectParseResult (..)
, projectParse
, projectParseFail
)
import Distribution.Deprecated.ReadP
( ReadP
, (+++)
Expand All @@ -183,6 +190,7 @@ import Distribution.Utils.Path hiding
)

import qualified Data.ByteString.Char8 as BS
import Data.Functor ((<&>))
import qualified Data.Map as Map
import qualified Data.Set as Set
import Network.URI (URI (..), parseURI)
Expand Down Expand Up @@ -240,12 +248,15 @@ parseProject
-> Verbosity
-> ProjectConfigToParse
-- ^ The contents of the file to parse
-> IO (ParseResult ProjectConfigSkeleton)
parseProject rootPath cacheDir httpTransport verbosity configToParse = do
let (dir, projectFileName) = splitFileName rootPath
projectDir <- makeAbsolute dir
projectPath <- canonicalizeConfigPath projectDir (ProjectConfigPath $ projectFileName :| [])
parseProjectSkeleton cacheDir httpTransport verbosity projectDir projectPath configToParse
-> IO (ProjectParseResult ProjectConfigSkeleton)
parseProject rootPath cacheDir httpTransport verbosity configToParse =
do
let (dir, projectFileName) = splitFileName rootPath
projectDir <- makeAbsolute dir
projectPath <- canonicalizeConfigPath projectDir (ProjectConfigPath $ projectFileName :| [])
parseProjectSkeleton cacheDir httpTransport verbosity projectDir projectPath configToParse
-- NOTE: Reverse the warnings so they are in line number order.
<&> \case ProjectParseOk ws x -> ProjectParseOk (reverse ws) x; x -> x

parseProjectSkeleton
:: FilePath
Expand All @@ -257,57 +268,62 @@ parseProjectSkeleton
-- ^ The path of the file being parsed, either the root or an import
-> ProjectConfigToParse
-- ^ The contents of the file to parse
-> IO (ParseResult ProjectConfigSkeleton)
-> IO (ProjectParseResult ProjectConfigSkeleton)
parseProjectSkeleton cacheDir httpTransport verbosity projectDir source (ProjectConfigToParse bs) =
(sanityWalkPCS False =<<) <$> liftPR (go []) (ParseUtils.readFields bs)
(sanityWalkPCS False =<<) <$> liftPR source (go []) (ParseUtils.readFields bs)
where
go :: [ParseUtils.Field] -> [ParseUtils.Field] -> IO (ParseResult ProjectConfigSkeleton)
go :: [ParseUtils.Field] -> [ParseUtils.Field] -> IO (ProjectParseResult ProjectConfigSkeleton)
go acc (x : xs) = case x of
(ParseUtils.F _ "import" importLoc) -> do
let importLocPath = importLoc `consProjectConfigPath` source

-- Once we canonicalize the import path, we can check for cyclical imports
normSource <- canonicalizeConfigPath projectDir source
normLocPath <- canonicalizeConfigPath projectDir importLocPath

debug verbosity $ "\nimport path, normalized\n=======================\n" ++ render (docProjectConfigPath normLocPath)

if isCyclicConfigPath normLocPath
then pure . parseFail $ ParseUtils.FromString (render $ cyclicalImportMsg normLocPath) Nothing
then pure . projectParseFail Nothing (Just normSource) $ ParseUtils.FromString (render $ cyclicalImportMsg normLocPath) Nothing
else do
normSource <- canonicalizeConfigPath projectDir source
let fs = (\z -> CondNode z [normLocPath] mempty) <$> fieldsToConfig normSource (reverse acc)
res <- parseProjectSkeleton cacheDir httpTransport verbosity projectDir importLocPath . ProjectConfigToParse =<< fetchImportConfig normLocPath
rest <- go [] xs
pure . fmap mconcat . sequence $ [fs, res, rest]
pure . fmap mconcat . sequence $ [projectParse Nothing normSource fs, res, rest]
(ParseUtils.Section l "if" p xs') -> do
normSource <- canonicalizeConfigPath projectDir source
subpcs <- go [] xs'
let fs = singletonProjectConfigSkeleton <$> fieldsToConfig source (reverse acc)
(elseClauses, rest) <- parseElseClauses xs
let condNode =
(\c pcs e -> CondNode mempty mempty [CondBranch c pcs e])
<$>
-- we rewrap as as a section so the readFields lexer of the conditional parser doesn't get confused
adaptParseError l (parseConditionConfVarFromClause . BS.pack $ "if(" <> p <> ")")
( let s = "if(" <> p <> ")"
in projectParse (Just s) normSource (adaptParseError l (parseConditionConfVarFromClause $ BS.pack s))
)
<*> subpcs
<*> elseClauses
pure . fmap mconcat . sequence $ [fs, condNode, rest]
pure . fmap mconcat . sequence $ [projectParse Nothing normSource fs, condNode, rest]
_ -> go (x : acc) xs
go acc [] = do
normSource <- canonicalizeConfigPath projectDir source
pure . fmap singletonProjectConfigSkeleton . fieldsToConfig normSource $ reverse acc
pure . fmap singletonProjectConfigSkeleton . projectParse Nothing normSource . fieldsToConfig normSource $ reverse acc

parseElseClauses :: [ParseUtils.Field] -> IO (ParseResult (Maybe ProjectConfigSkeleton), ParseResult ProjectConfigSkeleton)
parseElseClauses :: [ParseUtils.Field] -> IO (ProjectParseResult (Maybe ProjectConfigSkeleton), ProjectParseResult ProjectConfigSkeleton)
parseElseClauses x = case x of
(ParseUtils.Section _l "else" _p xs' : xs) -> do
subpcs <- go [] xs'
rest <- go [] xs
pure (Just <$> subpcs, rest)
(ParseUtils.Section l "elif" p xs' : xs) -> do
normSource <- canonicalizeConfigPath projectDir source
subpcs <- go [] xs'
(elseClauses, rest) <- parseElseClauses xs
let condNode =
(\c pcs e -> CondNode mempty mempty [CondBranch c pcs e])
<$> adaptParseError l (parseConditionConfVarFromClause . BS.pack $ "else(" <> p <> ")")
<$> ( let s = "elif(" <> p <> ")"
in projectParse (Just s) normSource (adaptParseError l (parseConditionConfVarFromClause $ BS.pack s))
)
<*> subpcs
<*> elseClauses
pure (Just <$> condNode, rest)
Expand All @@ -326,15 +342,16 @@ parseProjectSkeleton cacheDir httpTransport verbosity projectDir source (Project
addProvenance :: ProjectConfigPath -> ProjectConfig -> ProjectConfig
addProvenance sourcePath x = x{projectConfigProvenance = Set.singleton $ Explicit sourcePath}

adaptParseError :: Show e => ParseUtils.LineNo -> Either e a -> ParseResult a
adaptParseError _ (Right x) = pure x
adaptParseError l (Left e) = parseFail $ ParseUtils.FromString (show e) (Just l)

liftPR :: (a -> IO (ParseResult b)) -> ParseResult a -> IO (ParseResult b)
liftPR f (ParseOk ws x) = addWarnings <$> f x
liftPR :: ProjectConfigPath -> (a -> IO (ProjectParseResult b)) -> ParseResult a -> IO (ProjectParseResult b)
liftPR p f (ParseOk ws x) = addWarnings <$> f x
where
addWarnings (ParseOk ws' x') = ParseOk (ws' ++ ws) x'
addWarnings (ProjectParseOk ws' x') = ProjectParseOk (ws' ++ ((p,) <$> ws)) x'
addWarnings x' = x'
liftPR _ (ParseFailed e) = pure $ ParseFailed e
liftPR p _ (ParseFailed e) = pure $ projectParseFail Nothing (Just p) e

fetchImportConfig :: ProjectConfigPath -> IO BS.ByteString
fetchImportConfig (ProjectConfigPath (pci :| _)) = do
Expand All @@ -357,12 +374,14 @@ parseProjectSkeleton cacheDir httpTransport verbosity projectDir source (Project
where
isSet f = f (projectConfigShared pc) /= NoFlag

sanityWalkPCS :: Bool -> ProjectConfigSkeleton -> ParseResult ProjectConfigSkeleton
sanityWalkPCS underConditional t@(CondNode d _c comps)
| underConditional && modifiesCompiler d = parseFail $ ParseUtils.FromString "Cannot set compiler in a conditional clause of a cabal project file" Nothing
| otherwise = mapM_ sanityWalkBranch comps >> pure t
sanityWalkPCS :: Bool -> ProjectConfigSkeleton -> ProjectParseResult ProjectConfigSkeleton
sanityWalkPCS underConditional t@(CondNode d (listToMaybe -> c) comps)
| underConditional && modifiesCompiler d =
projectParseFail Nothing c $ ParseUtils.FromString "Cannot set compiler in a conditional clause of a cabal project file" Nothing
| otherwise =
mapM_ sanityWalkBranch comps >> pure t

sanityWalkBranch :: CondBranch ConfVar [ProjectConfigPath] ProjectConfig -> ParseResult ()
sanityWalkBranch :: CondBranch ConfVar [ProjectConfigPath] ProjectConfig -> ProjectParseResult ()
sanityWalkBranch (CondBranch _c t f) = traverse_ (sanityWalkPCS True) f >> sanityWalkPCS True t >> pure ()

------------------------------------------------------------------
Expand Down
Loading
Loading