Skip to content

Commit

Permalink
Ensure no-cabal-file has equivalent behavior to cabal-file-not-mention
Browse files Browse the repository at this point in the history
  • Loading branch information
brandonchinn178 authored and mrkkrp committed Jun 2, 2023
1 parent b9a7333 commit 32bc6f4
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 125 deletions.
48 changes: 25 additions & 23 deletions app/Main.hs
Original file line number Diff line number Diff line change
Expand Up @@ -80,40 +80,40 @@ formatOne ::
formatOne ConfigFileOpts {..} mode reqSourceType rawConfig mpath =
withPrettyOrmoluExceptions (cfgColorMode rawConfig) $ do
let getCabalInfoForSourceFile' sourceFile = do
cabalSearchResult <- getCabalInfoForSourceFile sourceFile
let debugEnabled = cfgDebug rawConfig
case cabalSearchResult of
CabalNotFound -> do
getCabalInfoForSourceFile sourceFile >>= \case
Nothing -> do
when debugEnabled $
hPutStrLn stderr $
"Could not find a .cabal file for " <> sourceFile
return Nothing
CabalDidNotMention cabalInfo -> do
when debugEnabled $ do
relativeCabalFile <-
makeRelativeToCurrentDirectory (ciCabalFilePath cabalInfo)
hPutStrLn stderr $
"Found .cabal file "
<> relativeCabalFile
<> ", but it did not mention "
<> sourceFile
return (Just cabalInfo)
CabalFound cabalInfo -> return (Just cabalInfo)
return (Nothing, Nothing)
Just CabalInfo {..} -> do
mStanzaInfo <- lookupStanzaInfo sourceFile ciStanzaInfoMap
case mStanzaInfo of
Nothing | debugEnabled -> do
relativeCabalFile <- makeRelativeToCurrentDirectory ciCabalFilePath
hPutStrLn stderr $
"Found .cabal file "
<> relativeCabalFile
<> ", but it did not mention "
<> sourceFile
_ -> pure ()
return (Just ciPackageName, mStanzaInfo)
getDotOrmoluForSourceFile' sourceFile = do
if optDoNotUseDotOrmolu
then return Nothing
else Just <$> getDotOrmoluForSourceFile sourceFile
case FP.normalise <$> mpath of
-- input source = STDIN
Nothing -> do
mcabalInfo <- case (optStdinInputFile, optDoNotUseCabal) of
(_, True) -> return Nothing
(mPackageName, mStanzaInfo) <- case (optStdinInputFile, optDoNotUseCabal) of
(_, True) -> return (Nothing, Nothing)
(Nothing, False) -> throwIO OrmoluMissingStdinInputFile
(Just inputFile, False) -> getCabalInfoForSourceFile' inputFile
mdotOrmolu <- case optStdinInputFile of
Nothing -> return Nothing
Just inputFile -> getDotOrmoluForSourceFile' inputFile
config <- patchConfig Nothing mcabalInfo mdotOrmolu
config <- patchConfig Nothing mPackageName mStanzaInfo mdotOrmolu
case mode of
Stdout -> do
ormoluStdin config >>= TIO.putStr
Expand All @@ -133,15 +133,16 @@ formatOne ConfigFileOpts {..} mode reqSourceType rawConfig mpath =
handleDiff originalInput formattedInput stdinRepr
-- input source = a file
Just inputFile -> do
mcabalInfo <-
(mPackageName, mStanzaInfo) <-
if optDoNotUseCabal
then return Nothing
then return (Nothing, Nothing)
else getCabalInfoForSourceFile' inputFile
mdotOrmolu <- getDotOrmoluForSourceFile' inputFile
config <-
patchConfig
(Just (detectSourceType inputFile))
mcabalInfo
mPackageName
mStanzaInfo
mdotOrmolu
case mode of
Stdout -> do
Expand All @@ -162,7 +163,7 @@ formatOne ConfigFileOpts {..} mode reqSourceType rawConfig mpath =
ormolu config inputFile originalInput
handleDiff originalInput formattedInput inputFile
where
patchConfig mdetectedSourceType mcabalInfo mdotOrmolu = do
patchConfig mdetectedSourceType mPackageName mStanzaInfo mdotOrmolu = do
let sourceType =
fromMaybe
ModuleSource
Expand All @@ -172,7 +173,8 @@ formatOne ConfigFileOpts {..} mode reqSourceType rawConfig mpath =
return $
refineConfig
sourceType
mcabalInfo
mPackageName
mStanzaInfo
mfixityOverrides
mmoduleReexports
rawConfig
Expand Down
32 changes: 18 additions & 14 deletions src/Ormolu.hs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@ module Ormolu
DynOption (..),

-- * Cabal info
CabalUtils.CabalSearchResult (..),
CabalUtils.CabalInfo (..),
CabalUtils.StanzaInfo (..),
CabalUtils.defaultStanzaInfo,
CabalUtils.StanzaInfoMap,
CabalUtils.lookupStanzaInfo,
CabalUtils.getCabalInfoForSourceFile,

-- * Fixity overrides and module re-exports
Expand All @@ -46,6 +49,7 @@ import Data.Set qualified as Set
import Data.Text (Text)
import Data.Text qualified as T
import Debug.Trace
import Distribution.PackageDescription (PackageName)
import GHC.Driver.CmdLine qualified as GHC
import GHC.Types.SrcLoc
import Ormolu.Config
Expand Down Expand Up @@ -177,8 +181,10 @@ ormoluStdin cfg =
refineConfig ::
-- | Source type to use
SourceType ->
-- | Cabal info for the file, if available
Maybe CabalUtils.CabalInfo ->
-- | Name of the package, if available
Maybe PackageName ->
-- | Stanza information for the source file, if available
Maybe CabalUtils.StanzaInfo ->
-- | Fixity overrides, if available
Maybe FixityOverrides ->
-- | Module re-exports, if available
Expand All @@ -187,7 +193,7 @@ refineConfig ::
Config region ->
-- | Refined 'Config'
Config region
refineConfig sourceType mcabalInfo mfixityOverrides mreexports rawConfig =
refineConfig sourceType mPackageName mStanzaInfo mfixityOverrides mreexports rawConfig =
rawConfig
{ cfgDynOptions = cfgDynOptions rawConfig ++ dynOptsFromCabal,
cfgFixityOverrides =
Expand All @@ -212,16 +218,14 @@ refineConfig sourceType mcabalInfo mfixityOverrides mreexports rawConfig =
where
fixityOverrides = fromMaybe defaultFixityOverrides mfixityOverrides
reexports = fromMaybe defaultModuleReexports mreexports
(dynOptsFromCabal, depsFromCabal) =
case mcabalInfo of
Nothing ->
-- If no cabal info is provided, assume base as a dependency by
-- default.
([], defaultDependencies)
Just CabalUtils.CabalInfo {..} ->
-- It makes sense to take into account the operator info for the
-- package itself if we know it, as if it were its own dependency.
(ciDynOpts, Set.insert ciPackageName ciDependencies)
CabalUtils.StanzaInfo {..} = fromMaybe CabalUtils.defaultStanzaInfo mStanzaInfo
dynOptsFromCabal = siDynOpts
depsFromCabal =
case mPackageName of
Nothing -> siDependencies
-- It makes sense to take into account the operator info for the
-- package itself if we know it, as if it were its own dependency.
Just package -> Set.insert package siDependencies

----------------------------------------------------------------------------
-- Helpers
Expand Down
102 changes: 32 additions & 70 deletions src/Ormolu/Utils/Cabal.hs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@
{-# LANGUAGE RecordWildCards #-}

module Ormolu.Utils.Cabal
( CabalSearchResult (..),
CabalInfo (..),
( CabalInfo (..),
StanzaInfo (..),
defaultStanzaInfo,
StanzaInfoMap,
lookupStanzaInfo,
Extension (..),
getCabalInfoForSourceFile,
findCabalFile,
Expand Down Expand Up @@ -34,29 +37,14 @@ import System.Directory
import System.FilePath
import System.IO.Unsafe (unsafePerformIO)

-- | The result of searching for a @.cabal@ file.
--
-- @since 0.5.3.0
data CabalSearchResult
= -- | Cabal file could not be found
CabalNotFound
| -- | Cabal file was found, but it did not mention the source file in
-- question
CabalDidNotMention CabalInfo
| -- | Cabal file was found and it mentions the source file in question
CabalFound CabalInfo
deriving (Eq, Show)

-- | Cabal information of interest to Ormolu.
data CabalInfo = CabalInfo
{ -- | Package name
ciPackageName :: !PackageName,
-- | Extension and language settings in the form of 'DynOption's
ciDynOpts :: ![DynOption],
-- | Direct dependencies
ciDependencies :: !(Set PackageName),
-- | Absolute path to the cabal file
ciCabalFilePath :: !FilePath
ciCabalFilePath :: !FilePath,
-- | Stanza information for all source files mentioned in the cabal file
ciStanzaInfoMap :: !StanzaInfoMap
}
deriving (Eq, Show)

Expand All @@ -67,11 +55,18 @@ data StanzaInfo = StanzaInfo
-- | Direct dependencies
siDependencies :: !(Set PackageName)
}
deriving (Show)
deriving (Eq, Show)

defaultStanzaInfo :: StanzaInfo
defaultStanzaInfo =
StanzaInfo
{ siDynOpts = [],
siDependencies = defaultDependencies
}

-- | Map from source files (absolute path without extensions) to the corresponding stanza information.
newtype StanzaInfoMap = StanzaInfoMap (Map FilePath StanzaInfo)
deriving (Show)
deriving (Eq, Show)

-- | Look up the given source file in the 'StanzaInfoMap'.
lookupStanzaInfo :: FilePath -> StanzaInfoMap -> IO (Maybe StanzaInfo)
Expand All @@ -86,17 +81,9 @@ getCabalInfoForSourceFile ::
-- | Haskell source file
FilePath ->
-- | Extracted cabal info, if any
m CabalSearchResult
m (Maybe CabalInfo)
getCabalInfoForSourceFile sourceFile =
liftIO (findCabalFile sourceFile) >>= \case
Just cabalFile -> do
(mentioned, cabalInfo) <- parseCabalInfo cabalFile sourceFile
return
( if mentioned
then CabalFound cabalInfo
else CabalDidNotMention cabalInfo
)
Nothing -> return CabalNotFound
liftIO (findCabalFile sourceFile) >>= traverse parseCabalInfo

-- | Find the path to an appropriate @.cabal@ file for a Haskell source
-- file, if available.
Expand All @@ -109,16 +96,8 @@ findCabalFile ::
findCabalFile = findClosestFileSatisfying $ \x ->
takeExtension x == ".cabal"

-- | Parsed cabal file information to be shared across multiple source files.
data CachedCabalFile = CachedCabalFile
{ -- | Parsed generic package description.
genericPackageDescription :: GenericPackageDescription,
stanzaInfoMap :: StanzaInfoMap
}
deriving (Show)

-- | Cache ref that stores 'CachedCabalFile' per Cabal file.
cacheRef :: IORef (Map FilePath CachedCabalFile)
cacheRef :: IORef (Map FilePath CabalInfo)
cacheRef = unsafePerformIO $ newIORef M.empty
{-# NOINLINE cacheRef #-}

Expand All @@ -127,37 +106,20 @@ parseCabalInfo ::
(MonadIO m) =>
-- | Location of the .cabal file
FilePath ->
-- | Location of the source file we are formatting
FilePath ->
-- | Indication if the source file was mentioned in the Cabal file and the
-- extracted 'CabalInfo'
m (Bool, CabalInfo)
parseCabalInfo cabalFileAsGiven sourceFileAsGiven = liftIO $ do
m CabalInfo
parseCabalInfo cabalFileAsGiven = liftIO $ do
cabalFile <- makeAbsolute cabalFileAsGiven
CachedCabalFile {..} <- withIORefCache cacheRef cabalFile $ do
withIORefCache cacheRef cabalFile $ do
cabalFileBs <- B.readFile cabalFile
genericPackageDescription <-
whenLeft (snd . runParseResult $ parseGenericPackageDescription cabalFileBs) $
throwIO . OrmoluCabalFileParsingFailed cabalFile . snd
let stanzaInfoMap = toStanzaInfoMap cabalFile genericPackageDescription
pure CachedCabalFile {..}
(dynOpts, dependencies, mentioned) <-
lookupStanzaInfo sourceFileAsGiven stanzaInfoMap >>= \case
Nothing -> pure ([], defaultDependencies, False)
Just StanzaInfo{..} -> pure (siDynOpts, siDependencies, True)
let pdesc = packageDescription genericPackageDescription
return
( mentioned,
CabalInfo
{ ciPackageName = pkgName (package pdesc),
ciDynOpts = dynOpts,
ciDependencies = dependencies,
ciCabalFilePath = cabalFile
}
)
where
whenLeft :: (Applicative f) => Either e a -> (e -> f a) -> f a
whenLeft eitha ma = either ma pure eitha
case snd . runParseResult . parseGenericPackageDescription $ cabalFileBs of
Right genericPackageDescription ->
pure
CabalInfo
{ ciPackageName = pkgName . package . packageDescription $ genericPackageDescription,
ciCabalFilePath = cabalFile,
ciStanzaInfoMap = toStanzaInfoMap cabalFile genericPackageDescription
}
Left (_, e) -> throwIO $ OrmoluCabalFileParsingFailed cabalFile e

-- | Get a map from Haskell source file paths (without any extensions) to
-- the corresponding 'DynOption's and dependencies.
Expand Down
40 changes: 22 additions & 18 deletions tests/Ormolu/CabalInfoSpec.hs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE RecordWildCards #-}

module Ormolu.CabalInfoSpec (spec) where
Expand Down Expand Up @@ -32,35 +33,38 @@ spec = do
cabalFile `shouldBe` Nothing
describe "parseCabalInfo" $ do
it "extracts correct cabal info from ormolu.cabal for src/Ormolu/Config.hs" $ do
(mentioned, CabalInfo {..}) <- parseCabalInfo "ormolu.cabal" "src/Ormolu/Config.hs"
mentioned `shouldBe` True
CabalInfo {..} <- parseCabalInfo "ormolu.cabal"
unPackageName ciPackageName `shouldBe` "ormolu"
ciDynOpts `shouldBe` [DynOption "-XGHC2021"]
Set.map unPackageName ciDependencies `shouldBe` Set.fromList ["Cabal-syntax", "Diff", "MemoTrie", "ansi-terminal", "array", "base", "binary", "bytestring", "containers", "deepseq", "directory", "file-embed", "filepath", "ghc-lib-parser", "megaparsec", "mtl", "syb", "text"]
ciCabalFilePath `shouldSatisfy` isAbsolute
makeRelativeToCurrentDirectory ciCabalFilePath `shouldReturn` "ormolu.cabal"
StanzaInfo {..} <- lookupStanzaInfo' "src/Ormolu/Config.hs" ciStanzaInfoMap
siDynOpts `shouldBe` [DynOption "-XGHC2021"]
Set.map unPackageName siDependencies `shouldBe` Set.fromList ["Cabal-syntax", "Diff", "MemoTrie", "ansi-terminal", "array", "base", "binary", "bytestring", "containers", "deepseq", "directory", "file-embed", "filepath", "ghc-lib-parser", "megaparsec", "mtl", "syb", "text"]
it "extracts correct cabal info from ormolu.cabal for tests/Ormolu/PrinterSpec.hs" $ do
(mentioned, CabalInfo {..}) <- parseCabalInfo "ormolu.cabal" "tests/Ormolu/PrinterSpec.hs"
mentioned `shouldBe` True
CabalInfo {..} <- parseCabalInfo "ormolu.cabal"
unPackageName ciPackageName `shouldBe` "ormolu"
ciDynOpts `shouldBe` [DynOption "-XGHC2021"]
Set.map unPackageName ciDependencies `shouldBe` Set.fromList ["Cabal-syntax", "QuickCheck", "base", "containers", "directory", "filepath", "ghc-lib-parser", "hspec", "hspec-megaparsec", "megaparsec", "ormolu", "path", "path-io", "temporary", "text"]
ciCabalFilePath `shouldSatisfy` isAbsolute
makeRelativeToCurrentDirectory ciCabalFilePath `shouldReturn` "ormolu.cabal"
StanzaInfo {..} <- lookupStanzaInfo' "tests/Ormolu/PrinterSpec.hs" ciStanzaInfoMap
siDynOpts `shouldBe` [DynOption "-XGHC2021"]
Set.map unPackageName siDependencies `shouldBe` Set.fromList ["Cabal-syntax", "QuickCheck", "base", "containers", "directory", "filepath", "ghc-lib-parser", "hspec", "hspec-megaparsec", "megaparsec", "ormolu", "path", "path-io", "temporary", "text"]
it "handles correctly files that are not mentioned in ormolu.cabal" $ do
(mentioned, CabalInfo {..}) <- parseCabalInfo "ormolu.cabal" "src/FooBob.hs"
mentioned `shouldBe` False
CabalInfo {..} <- parseCabalInfo "ormolu.cabal"
unPackageName ciPackageName `shouldBe` "ormolu"
ciDynOpts `shouldBe` []
Set.map unPackageName ciDependencies `shouldBe` Set.fromList ["base"]
ciCabalFilePath `shouldSatisfy` isAbsolute
makeRelativeToCurrentDirectory ciCabalFilePath `shouldReturn` "ormolu.cabal"
mStanzaInfo <- lookupStanzaInfo "src/FooBob.hs" ciStanzaInfoMap
mStanzaInfo `shouldBe` Nothing
it "handles `hs-source-dirs: .`" $ do
(_, CabalInfo {..}) <- parseTestCabalInfo "Foo.hs"
ciDynOpts `shouldContain` [DynOption "-XImportQualifiedPost"]
CabalInfo {..} <- parseCabalInfo "data/cabal-tests/test.cabal"
StanzaInfo {..} <- lookupStanzaInfo' "data/cabal-tests/Foo.hs" ciStanzaInfoMap
siDynOpts `shouldContain` [DynOption "-XImportQualifiedPost"]
it "handles empty hs-source-dirs" $ do
(_, CabalInfo {..}) <- parseTestCabalInfo "Bar.hs"
ciDynOpts `shouldContain` [DynOption "-XImportQualifiedPost"]
CabalInfo {..} <- parseCabalInfo "data/cabal-tests/test.cabal"
StanzaInfo {..} <- lookupStanzaInfo' "data/cabal-tests/Bar.hs" ciStanzaInfoMap
siDynOpts `shouldContain` [DynOption "-XImportQualifiedPost"]
where
parseTestCabalInfo f =
parseCabalInfo "data/cabal-tests/test.cabal" ("data/cabal-tests" </> f)
lookupStanzaInfo' fp stanzaInfoMap =
lookupStanzaInfo fp stanzaInfoMap >>= \case
Nothing -> error $ "StanzaInfoMap did not contain: " ++ fp
Just info -> pure info

0 comments on commit 32bc6f4

Please sign in to comment.