Skip to content
New issue

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

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

Already on GitHub? # to your account

Improve --cabal-default-extensions #783

Merged
merged 1 commit into from
Sep 23, 2021
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
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,15 @@
## Unreleased

* Improvements to `.cabal` file handling:
* When looking for a `.cabal` file, directories were previously
erroneously also considered. [Issue 781](
https://github.com/tweag/ormolu/issues/781).
* We now print a note if Ormolu was told to consider
`.cabal` files, but no suitable one could be found.
* Handle an empty `hs-source-dirs` correctly.
* Also consider modules which are only conditionally listed
in the `.cabal` file.

## Ormolu 0.3.0.0

* Data declarations with multiline kind signatures are now formatted
Expand Down
3 changes: 3 additions & 0 deletions expected-failures/Agda.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
Found .cabal file Agda.cabal, but it did not mention Setup.hs
Found .cabal file Agda.cabal, but it did not mention src/data/MAlonzo/src/MAlonzo/RTE.hs
Found .cabal file Agda.cabal, but it did not mention src/data/MAlonzo/src/MAlonzo/RTE/Float.hs
src/full/Agda/Syntax/Internal.hs
AST of input and AST of formatted code differ.
at src/full/Agda/Syntax/Internal.hs:640:5
Expand Down
1 change: 1 addition & 0 deletions expected-failures/esqueleto.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Found .cabal file esqueleto.cabal, but it did not mention Setup.hs
src/Database/Esqueleto/Internal/Internal.hs:405:1
The GHC parser (in Haddock mode) failed:
lexical error in string/character literal at character 's'
3 changes: 3 additions & 0 deletions expected-failures/graphql-engine.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
Could not find a .cabal file for contrib/metadata-types/generated/HasuraMetadataV2.hs
Found .cabal file server/graphql-engine.cabal, but it did not mention server/Setup.hs
Found .cabal file server/bench-wrk/wrk-websocket-server/wrk-websocket-server.cabal, but it did not mention server/bench-wrk/wrk-websocket-server/Setup.hs
server/src-lib/Hasura/Backends/BigQuery/Types.hs
@@ -509,22 +509,29 @@
| LessOrEqualOp
Expand Down
1 change: 1 addition & 0 deletions expected-failures/haxl.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Haxl/Core/DataCache.hs:54:49
The GHC parser (in Haddock mode) failed:
Not a data constructor: `!'
Found .cabal file haxl.cabal, but it did not mention Setup.hs
4 changes: 4 additions & 0 deletions expected-failures/hlint.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
Found .cabal file hlint.cabal, but it did not mention Setup.hs
Found .cabal file hlint.cabal, but it did not mention data/HLint_QuickCheck.hs
Found .cabal file hlint.cabal, but it did not mention data/HLint_TypeCheck.hs
Found .cabal file hlint.cabal, but it did not mention data/Test.hs
src/Extension.hs
@@ -17,7 +17,8 @@
UnboxedTuples,
Expand Down
1 change: 1 addition & 0 deletions expected-failures/idris.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Found .cabal file idris.cabal, but it did not mention Setup.hs
src/Idris/Parser.hs:1052:1
The GHC parser (in Haddock mode) failed:
parse error on input `@'
Expand Down
1 change: 1 addition & 0 deletions expected-failures/intero.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Found .cabal file intero.cabal, but it did not mention Setup.hs
src/InteractiveUI.hs
@@ -3746,6 +3746,7 @@
stdout
Expand Down
2 changes: 2 additions & 0 deletions expected-failures/pandoc.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Found .cabal file pandoc.cabal, but it did not mention Setup.hs
src/Text/Pandoc/Readers/Vimwiki.hs
@@ -618,7 +618,8 @@
<$ ( skipMany1 spaceChar
Expand All @@ -12,3 +13,4 @@ src/Text/Pandoc/Readers/Vimwiki.hs

Formatting is not idempotent.
Please, consider reporting the bug.
Found .cabal file pandoc.cabal, but it did not mention test/command/3510-src.hs
1 change: 1 addition & 0 deletions expected-failures/pipes.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Found .cabal file pipes.cabal, but it did not mention Setup.hs
src/Pipes/Core.hs
AST of input and AST of formatted code differ.
at src/Pipes/Core.hs:(128,1)-(151,2)
Expand Down
1 change: 1 addition & 0 deletions expected-failures/postgrest.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
Found .cabal file postgrest.cabal, but it did not mention Setup.hs
src/PostgREST/DbRequestBuilder.hs
@@ -148,12 +148,11 @@
-- /projects?select=clients(*)
Expand Down
4 changes: 2 additions & 2 deletions nix/ormolize/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,14 @@
cp "$hs_file" "''${hs_file}-original"
done

(ormolu --cabal-default-extensions --check-idempotence --mode inplace $hs_files || true) 2> log.txt
((ormolu --cabal-default-extensions --check-idempotence --mode inplace $hs_files; echo $? > exit_code) || true) 2> log.txt
'';
inherit doCheck;
checkPhase =
if expectedFailures == null
then ''
echo "No failures expected"
if [[ -s log.txt ]]; then exit 1; fi
if (( $(cat exit_code) != 0 )); then exit 1; fi
''
else ''
diff --ignore-blank-lines --color=always ${expectedFailures} log.txt
Expand Down
2 changes: 2 additions & 0 deletions ormolu.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -149,11 +149,13 @@ test-suite tests
build-depends:
base >=4.12 && <5.0,
containers >=0.5 && <0.7,
directory ^>=1.3,
filepath >=1.2 && <1.5,
hspec >=2.0 && <3.0,
ormolu,
path >=0.6 && <0.10,
path-io >=1.4.2 && <2.0,
temporary ^>=1.3,
text >=0.2 && <1.3

if flag(dev)
Expand Down
41 changes: 32 additions & 9 deletions src/Ormolu/Utils/Extensions.hs
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,19 @@ where
import Control.Exception
import Control.Monad.IO.Class
import qualified Data.ByteString as B
import Data.List (find)
import Data.Map.Lazy (Map)
import qualified Data.Map.Lazy as M
import Data.Maybe (maybeToList)
import qualified Distribution.ModuleName as ModuleName
import Distribution.PackageDescription
import Distribution.PackageDescription.Parsec
import qualified Distribution.Types.CondTree as CT
import Language.Haskell.Extension
import Ormolu.Config
import Ormolu.Exception
import System.Directory
import System.FilePath
import System.IO (hPutStrLn, stderr)
import System.IO.Error (isDoesNotExistError)

-- | Get a map from Haskell source file paths (without any extensions)
Expand All @@ -49,12 +50,17 @@ getExtensionsFromCabalFile cabalFile = liftIO $ do
buildMap extractFromBenchmark . snd <$> condBenchmarks
]
where
buildMap f a = let (files, exts) = f (condTreeData a) in M.fromList $ (,exts) <$> files
buildMap f a = let (files, exts) = f mergedA in M.fromList $ (,exts) <$> files
where
(mergedA, _) = CT.ignoreConditions a

extractFromBuildInfo extraModules BuildInfo {..} = (,exts) $ do
m <- extraModules ++ (ModuleName.toFilePath <$> otherModules)
(takeDirectory cabalFile </>) . (</> dropExtensions m) <$> hsSourceDirs
(takeDirectory cabalFile </>) <$> prependSrcDirs (dropExtensions m)
where
prependSrcDirs f
| null hsSourceDirs = [f]
| otherwise = (</> f) <$> hsSourceDirs
exts = maybe [] langExt defaultLanguage ++ fmap extToDynOption defaultExtensions
langExt =
pure . DynOption . \case
Expand Down Expand Up @@ -94,11 +100,19 @@ findCabalFile ::
m (Maybe FilePath)
findCabalFile p = liftIO $ do
let parentDir = takeDirectory p
ps <-
dirEntries <-
listDirectory parentDir `catch` \case
(isDoesNotExistError -> True) -> pure []
e -> throwIO e
case find ((== ".cabal") . takeExtension) ps of
let findDotCabal = \case
[] -> pure Nothing
e : es
| takeExtension e == ".cabal" ->
doesFileExist (parentDir </> e) >>= \case
True -> pure $ Just e
False -> findDotCabal es
_ : es -> findDotCabal es
findDotCabal dirEntries >>= \case
Just cabalFile -> pure . Just $ parentDir </> cabalFile
Nothing ->
if isDrive parentDir
Expand All @@ -114,9 +128,18 @@ getCabalExtensionDynOptions ::
m [DynOption]
getCabalExtensionDynOptions sourceFile' = liftIO $ do
sourceFile <- makeAbsolute sourceFile'
mCabalFile <- findCabalFile sourceFile
case mCabalFile of
findCabalFile sourceFile >>= \case
Just cabalFile -> do
extsByFile <- getExtensionsFromCabalFile cabalFile
pure $ M.findWithDefault [] (dropExtensions sourceFile) extsByFile
Nothing -> pure []
case M.lookup (dropExtensions sourceFile) extsByFile of
Just exts -> pure exts
Nothing -> do
relativeCabalFile <- makeRelativeToCurrentDirectory cabalFile
note $
"Found .cabal file "
<> relativeCabalFile
<> ", but it did not mention "
<> sourceFile'
Nothing -> note $ "Could not find a .cabal file for " <> sourceFile'
where
note msg = [] <$ hPutStrLn stderr msg
8 changes: 8 additions & 0 deletions tests/Ormolu/CabalExtensionsSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ module Ormolu.CabalExtensionsSpec (spec) where
import qualified Data.Map as M
import Ormolu.Config
import Ormolu.Utils.Extensions
import System.Directory
import System.FilePath
import System.IO.Temp (withSystemTempDirectory)
import Test.Hspec

spec :: Spec
Expand All @@ -23,5 +26,10 @@ spec = describe "Handle extensions from .cabal files" $ do
cabalFile `shouldBe` Just expectedCabalFile
findsOrmoluCabal "src/Ormolu/Config.hs" "./ormolu.cabal"
findsOrmoluCabal "a/b/c/d/e" "./ormolu.cabal"
it "do not consider directories as .cabal files" $
withSystemTempDirectory "" $ \dir -> do
createDirectory $ dir </> ".cabal"
cabalFile <- findCabalFile $ dir </> "foo/bar.hs"
cabalFile `shouldBe` Nothing
where
members as m = all (`M.member` m) as