Commit 978ca74e authored by novadenizen's avatar novadenizen
Browse files

Added TODO: eradicateNoParse comments to all uses of bare 'read'

When 'read' is used on invalid input, it raises a "no parse" exception that is
typically very unhelpful to the user.  It is generally more desirable to either
handle the error or produce a more helpful error message.

This actually happens sometimes.  When a user creates a detailed-0.9 test
suite, and that test suite crashes or is killed so it can't write its result
log, the bare read in Distribution.Simple.Test.LibV09.runTest fails and cabal
fails with "no parse".

I could have just written a patch to fix that, but it seemed like a better idea
to find all uses of bare read and see what we can do for all of them.

I have grepped through the repository and I think I've found all uses of bare
'read' that might raise the dreaded 'no parse' exception.  The plan is to
separately patch each of them by either replacing with an alternate Read
function (readMaybe is the most obvious candidate), proving the read will
always succeed, or establishing that producing a 'no parse' exception is the
most desirable behavior.
parent a5e9544e
......@@ -366,7 +366,7 @@ type PathTemplateEnv = [(PathTemplateVariable, PathTemplate)]
-- | Convert a 'FilePath' to a 'PathTemplate' including any template vars.
--
toPathTemplate :: FilePath -> PathTemplate
toPathTemplate = PathTemplate . read
toPathTemplate = PathTemplate . read -- TODO: eradicateNoParse
-- | Convert back to a path, any remaining vars are included
--
......
......@@ -37,7 +37,7 @@ classify ('#':s) = case tokens s of
&& length file >= 2
&& head file == '"'
&& last file == '"'
-> Line (read line) (tail (init file))
-> Line (read line) (tail (init file)) -- TODO:eradicateNoParse
_ -> CPP s
where tokens = unfoldr $ \str -> case lex str of
(t@(_:_), str'):_ -> Just (t, str')
......
......@@ -116,7 +116,7 @@ runTest pkg_descr lbi clbi flags suite = do
(testSuiteName l) (testLogs l)
-- Generate TestSuiteLog from executable exit code and a machine-
-- readable test log
suiteLog <- fmap ((\l -> l { logFile = finalLogName l }) . read)
suiteLog <- fmap ((\l -> l { logFile = finalLogName l }) . read) -- TODO: eradicateNoParse
$ readFile tempLog
-- Write summary notice to log file indicating start of test suite
......@@ -210,7 +210,7 @@ simpleTestStub m = unlines
-- of detectable errors when Cabal is compiled.
stubMain :: IO [Test] -> IO ()
stubMain tests = do
(f, n) <- fmap read getContents
(f, n) <- fmap read getContents -- TODO: eradicateNoParse
dir <- getCurrentDirectory
results <- (tests >>= stubRunTests) `CE.catch` errHandler
setCurrentDirectory dir
......
......@@ -80,7 +80,8 @@ instance Text Int where
-- | Parser for non-negative integers.
parseNat :: Parse.ReadP r Int
parseNat = read `fmap` Parse.munch1 isDigit
parseNat = read `fmap` Parse.munch1 isDigit -- TODO: eradicateNoParse
instance Text Version where
disp (Version branch _tags) -- Death to version tags!!
......
......@@ -896,7 +896,7 @@ instance Text VersionRange where
if firstDigit == '0'
then return 0
else do rest <- Parse.munch isDigit
return (read (firstDigit : rest))
return (read (firstDigit : rest)) -- TODO: eradicateNoParse
parseRangeOp (s,f) = Parse.string s >> Parse.skipSpaces >> fmap f parse
rangeOps = [ ("<", EarlierVersion),
......
......@@ -82,7 +82,7 @@ data PackageType = Library | Executable
instance Text PackageType where
disp = Disp.text . show
parse = Parse.choice $ map (fmap read . Parse.string . show) [Library, Executable]
parse = Parse.choice $ map (fmap read . Parse.string . show) [Library, Executable] -- TODO: eradicateNoParse
instance Monoid InitFlags where
mempty = gmempty
......@@ -114,5 +114,5 @@ data Category
instance Text Category where
disp = Disp.text . show
parse = Parse.choice $ map (fmap read . Parse.string . show) [Codec .. ]
parse = Parse.choice $ map (fmap read . Parse.string . show) [Codec .. ] -- TODO: eradicateNoParse
......@@ -164,7 +164,7 @@ report verbosity repoCtxt mUsername mPassword = do
contents <- getDirectoryContents srcDir
forM_ (filter (\c -> takeExtension c ==".log") contents) $ \logFile ->
do inp <- readFile (srcDir </> logFile)
let (reportStr, buildLog) = read inp :: (String,String)
let (reportStr, buildLog) = read inp :: (String,String) -- TODO: eradicateNoParse
case BuildReport.parse reportStr of
Left errs -> warn verbosity $ "Errors: " ++ errs -- FIXME
Right report' ->
......
......@@ -1293,7 +1293,7 @@ userConfigAction ucflags extraArgs globalFlags = do
win32SelfUpgradeAction :: Win32SelfUpgradeFlags -> [String] -> Action
win32SelfUpgradeAction selfUpgradeFlags (pid:path:_extraArgs) _globalFlags = do
let verbosity = fromFlag (win32SelfUpgradeVerbosity selfUpgradeFlags)
Win32SelfUpgrade.deleteOldExeFile verbosity (read pid) path
Win32SelfUpgrade.deleteOldExeFile verbosity (read pid) path -- TODO: eradicateNoParse
win32SelfUpgradeAction _ _ _ = return ()
-- | Used as an entry point when cabal-install needs to invoke itself
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment