Skip to content
This repository was archived by the owner on Oct 7, 2020. It is now read-only.

[WIP] stylish-haskell plugin #1618

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
+110 −14
Open

[WIP] stylish-haskell plugin #1618

Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
39afabc
First draft of the Stylish (Haskell) plugin
lukasz-golebiewski Jan 25, 2020
577519f
Add stylish formatter provider to HSImportSpec (failing)
EncodePanda Jan 25, 2020
50f0a65
Extract formatLspConfig in FormatSpec
EncodePanda Jan 26, 2020
4566617
Add first working draft of the stylish plugin
lukasz-golebiewski Jan 30, 2020
e631aa9
Update stack.yaml
lukasz-golebiewski Jan 31, 2020
a64db52
Update stack.yaml
lukasz-golebiewski Jan 31, 2020
3bb0de8
Add stylish-haskell version to *.cabal
lukasz-golebiewski Feb 2, 2020
9754065
Add stylish-haskell to stack*.yaml
lukasz-golebiewski Feb 2, 2020
87ca5fd
Force HsYaml versions
lukasz-golebiewski Feb 2, 2020
ea10da5
Add HsYaml to .cabal
lukasz-golebiewski Feb 2, 2020
1ef9650
Update cabal index snapshot
Avi-D-coder Feb 2, 2020
c0e5559
Merge branch 'master' into stylish-support
Avi-D-coder Feb 2, 2020
d143052
Add stylish with deps to stack yaml 8.8.2
lukasz-golebiewski Feb 4, 2020
9e59e29
Add cabal-3.0.0.0
lukasz-golebiewski Feb 4, 2020
d8f274c
Merge branch 'master' of github.com:haskell/haskell-ide-engine into s…
lukasz-golebiewski Feb 4, 2020
26f9e87
Revert "Add cabal-3.0.0.0"
lukasz-golebiewski Feb 4, 2020
6f91f83
Add dependency on vector to stack-8.4.4
lukasz-golebiewski Feb 5, 2020
a1a6f75
Add dependency on Cabal-3.0.0.0
lukasz-golebiewski Feb 5, 2020
15e816d
Set cabal to 2.4.1.0
lukasz-golebiewski Feb 5, 2020
5988529
Downgrade cabal to 2.2.0.1
lukasz-golebiewski Feb 8, 2020
df7cce7
Merge branch 'master' of github.com:haskell/haskell-ide-engine into s…
lukasz-golebiewski Feb 8, 2020
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
2 changes: 2 additions & 0 deletions app/MainHie.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import Haskell.Ide.Engine.Plugin.Liquid
import Haskell.Ide.Engine.Plugin.Ormolu
import Haskell.Ide.Engine.Plugin.Package
import Haskell.Ide.Engine.Plugin.Pragmas
import Haskell.Ide.Engine.Plugin.Stylish (stylishDescriptor)

-- ---------------------------------------------------------------------

Expand All @@ -66,6 +67,7 @@ plugins includeExamples = pluginDescToIdePlugins allPlugins
, genericDescriptor "generic"
, ghcmodDescriptor "ghcmod"
, ormoluDescriptor "ormolu"
, stylishDescriptor "stylish"
]
examplePlugins =
[example2Descriptor "eg2"
Expand Down
2 changes: 1 addition & 1 deletion cabal.project
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ constraints:

write-ghc-environment-files: never

index-state: 2020-02-02T17:43:42Z
index-state: 2020-02-02T18:20:00Z
4 changes: 4 additions & 0 deletions haskell-ide-engine.cabal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ library
Haskell.Ide.Engine.Plugin.Package
Haskell.Ide.Engine.Plugin.Package.Compat
Haskell.Ide.Engine.Plugin.Pragmas
Haskell.Ide.Engine.Plugin.Stylish
Haskell.Ide.Engine.Plugin.Generic
Haskell.Ide.Engine.Plugin.GhcMod
Haskell.Ide.Engine.Scheduler
Expand Down Expand Up @@ -80,6 +81,8 @@ library
, hoogle >= 5.0.13
, hsimport
, hslogger
, HsYAML >= 0.2.1.0
, HsYAML-aeson >= 0.2.0.0
, lifted-async
, lens >= 4.15.2
, monoid-subclasses > 0.4
Expand All @@ -90,6 +93,7 @@ library
, safe
, sorted-list >= 0.2.1.0
, stm
, stylish-haskell >= 0.10.0.0
, syb
, tagsoup
, text
Expand Down
43 changes: 43 additions & 0 deletions src/Haskell/Ide/Engine/Plugin/Stylish.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
{-# LANGUAGE OverloadedStrings #-}
module Haskell.Ide.Engine.Plugin.Stylish where

import Control.Monad.IO.Class (liftIO)
import Data.Aeson (Value (Null))
import Data.List (intercalate)
import qualified Data.Text as T
import Haskell.Ide.Engine.MonadTypes
import Haskell.Ide.Engine.PluginUtils (fullRange, pluginGetFile)
import Language.Haskell.Stylish (ConfigPath (..), format)


stylishDescriptor :: PluginId -> PluginDescriptor
stylishDescriptor plId = PluginDescriptor
{ pluginId = plId
, pluginName = "Stylish"
, pluginDesc = "Stylish is a tool to format source code."
, pluginCommands = []
, pluginCodeActionProvider = Nothing
, pluginDiagnosticProvider = Nothing
, pluginHoverProvider = Nothing
, pluginSymbolProvider = Nothing
, pluginFormattingProvider = Just provider
}

provider :: FormattingProvider
provider contents uri typ _ =
case typ of
FormatRange _ ->
return $ IdeResultFail (IdeError PluginError (T.pack "Selection formatting for Stylish is not currently supported.") Null)
FormatText -> pluginGetFile "stylish:" uri $ \file -> do
res <- liftIO $ runStylish Nothing file contents
case res of
Left err -> return $ IdeResultFail
(IdeError PluginError
(T.pack $ "stylish: " ++ err)
Null
)
Right new -> return $ IdeResultOk [TextEdit (fullRange contents) (T.pack $ ((intercalate "\n" new) <> "\n"))]


runStylish :: Maybe ConfigPath -> FilePath -> T.Text -> IO (Either String [String])
runStylish config file contents = format config (Just file) (T.unpack contents)
3 changes: 3 additions & 0 deletions stack-8.4.2.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ extra-deps:
- hoogle-5.0.17.11
- hsimport-0.11.0
- hslogger-1.3.1.0
- HsYAML-0.2.1.0
- HsYAML-aeson-0.2.0.0
- invariant-0.5.3
- lens-4.18.1
- libyaml-0.1.1.0
Expand All @@ -50,6 +52,7 @@ extra-deps:
- rope-utf16-splay-0.3.1.0
- simple-sendfile-0.2.30 # for network and network-bsd
- socks-0.6.1 # for network and network-bsd
- stylish-haskell-0.10.0.0
- syz-0.2.0.0
- type-equality-1
- unix-compat-0.5.2
Expand Down
3 changes: 3 additions & 0 deletions stack-8.4.3.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ extra-deps:
- hspec-2.7.1
- hspec-core-2.7.1
- hspec-discover-2.7.1
- HsYAML-0.2.1.0
- HsYAML-aeson-0.2.0.0
- indexed-profunctors-0.1
- invariant-0.5.3
- lens-4.18.1
Expand All @@ -66,6 +68,7 @@ extra-deps:
- singleton-bool-0.1.5
- socks-0.6.1 # for network and network-bsd
- splitmix-0.0.3
- stylish-haskell-0.10.0.0
- tagged-0.8.6
- th-abstraction-0.3.1.0
- these-1.0.1
Expand Down
5 changes: 5 additions & 0 deletions stack-8.4.4.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ extra-deps:
- bifunctors-5.5.6
- brittany-0.12.1.1
- bytestring-trie-0.2.5.0
- Cabal-2.4.1.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

This does not seem correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Stack recommended to set cabal version explicitly in https://circleci.com/gh/haskell/haskell-ide-engine/13311
If this is incorrect then I don't think I know how to proceed with this

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this compile? Because, iirc, stack snapshots are bundled with cabal-install and Cabal versions, iirc, setting a custom Cabal version for a snapshot may not succeed. If it compiles, everything is fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not yet, I'm slowly bisecting towards the cabal version satisfying every dependency

Copy link
Member Author

Choose a reason for hiding this comment

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

And back to stack suggesting Cabal-3.0.0.0. Looks like stylish 0.10.0.0 can't be used here and below because of conflicting dependencies :-(

- cabal-helper-1.0.0.0
- cabal-plan-0.5.0.0
- connection-0.3.1 # for network and network-bsd
Expand All @@ -35,6 +36,8 @@ extra-deps:
- hoogle-5.0.17.11
- hsimport-0.11.0
- hslogger-1.3.1.0
- HsYAML-0.2.1.0
- HsYAML-aeson-0.2.0.0
- invariant-0.5.3
- lens-4.18.1
- libyaml-0.1.1.0
Expand All @@ -50,6 +53,7 @@ extra-deps:
- rope-utf16-splay-0.3.1.0
- simple-sendfile-0.2.30 # for network and network-bsd
- socks-0.6.1 # for network and network-bsd
- stylish-haskell-0.10.0.0
- syz-0.2.0.0
- unix-compat-0.5.2
- unordered-containers-0.2.10.0
Expand All @@ -61,6 +65,7 @@ extra-deps:
- temporary-1.2.1.1
- time-compat-1.9.2.2
- time-manager-0.0.0 # for http2
- vector-0.12.1.2
- warp-3.2.28 # for network and network-bsd
- wai-3.2.2.1 # for network and network-bsd

Expand Down
3 changes: 3 additions & 0 deletions stack-8.6.4.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,16 @@ extra-deps:
- hlint-2.2.10
- hoogle-5.0.17.11
- hsimport-0.11.0
- HsYAML-0.2.1.0
- HsYAML-aeson-0.2.0.0
- lsp-test-0.10.0.0
- monad-dijkstra-0.1.1.2@rev:1
- monad-memo-0.4.1
- multistate-0.8.0.1
- ormolu-0.0.3.1
- parser-combinators-1.2.1
- rope-utf16-splay-0.3.1.0
- stylish-haskell-0.10.0.0
- syz-0.2.0.0
- temporary-1.2.1.1
- time-compat-1.9.2.2
Expand Down
3 changes: 3 additions & 0 deletions stack-8.6.5.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ extra-deps:
- hlint-2.2.10
- hoogle-5.0.17.11
- hsimport-0.11.0
- HsYAML-0.2.1.0
- HsYAML-aeson-0.2.0.0
- indexed-profunctors-0.1
- lsp-test-0.10.0.0
- monad-dijkstra-0.1.1.2
Expand All @@ -36,6 +38,7 @@ extra-deps:
- ormolu-0.0.3.1
- parser-combinators-1.2.1
- semialign-1.1
- stylish-haskell-0.10.0.0
- temporary-1.2.1.1
- topograph-1
flags:
Expand Down
3 changes: 3 additions & 0 deletions stack-8.8.1.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,13 @@ extra-deps:
- hlint-2.2.10
- hoogle-5.0.17.11
- hsimport-0.11.0
- HsYAML-0.2.1.0
- HsYAML-aeson-0.2.0.0
- ilist-0.3.1.0
- monad-dijkstra-0.1.1.2
- ormolu-0.0.3.1
- semigroups-0.18.5
- stylish-haskell-0.10.0.0
- temporary-1.2.1.1

flags:
Expand Down
3 changes: 3 additions & 0 deletions stack-8.8.2.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,13 @@ extra-deps:
- hlint-2.2.10
- hoogle-5.0.17.11
- hsimport-0.11.0
- HsYAML-0.2.1.0
- HsYAML-aeson-0.2.0.0
- ilist-0.3.1.0
- monad-dijkstra-0.1.1.2
- ormolu-0.0.3.1
- semigroups-0.18.5
- stylish-haskell-0.10.0.0
- temporary-1.2.1.1

flags:
Expand Down
3 changes: 3 additions & 0 deletions stack.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,12 @@ extra-deps:
- hlint-2.2.10
- hoogle-5.0.17.11
- hsimport-0.11.0
- HsYAML-0.2.1.0
- HsYAML-aeson-0.2.0.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

You'll likely also need to add these to each stack.yaml and to haskell-ide-engine.cabal

Copy link
Member Author

Choose a reason for hiding this comment

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

This doesn't seem to be working :-(

Copy link
Member

Choose a reason for hiding this comment

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

I have to note that HsYAML is using GPL and the project licensing would be affected (not sure if it already is using libs with gpl)

Copy link
Member

Choose a reason for hiding this comment

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

Personally i am fine with linking a gpl lib but the project will have two yaml libs and maybe we should use only one (not in this pr)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm also okay with GPL, however stylish-haskell just made the switch, we could ask them to switch back haskell/stylish-haskell@498d676

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure "fine" is the right wording, since we are not GPL, we do not really have the rights to using it, right? On the other hand, I doubt, too, that someone will sue us.

Copy link
Member

Choose a reason for hiding this comment

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

Hey, I wasn't aware that HsYAML was GPL-licensed. I don't think it's a problem -- it's fine for a BSD3 application to depend on a GPL-licensed library. It does not affect the code of haskell-ide-engine or stylish-haskell, those remain BSD3-licensed. However, the compiled binary must adhere to the GPL license (which seems fine in this case). This is similar to e.g. the fact that GHC links against libgmp by default.

But, if this worries you, I am happy to add a flag to stylish-haskell that makes it pick the C-based yaml as a dependency rather than HsYAML?

Copy link
Member

@jneira jneira Feb 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Afaiu we can use it without changing our licenses files but the license for hie artifacts linking HsYaml becomes auto gpl. So anybody could issue someone who doesnt respect the gpl resdistributing hie.

Copy link
Collaborator

Choose a reason for hiding this comment

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

IANAL, if you argue that this is indeed just fine, I have nothing to say against it :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah. Assuming this is correct, this sounds totally reasonable.

- ilist-0.3.1.0
- monad-dijkstra-0.1.1.2
- ormolu-0.0.3.1
- stylish-haskell-0.10.0.0
- semigroups-0.18.5
- temporary-1.2.1.1

Expand Down
11 changes: 5 additions & 6 deletions test/functional/FormatSpec.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@ spec = do
documentContents doc >>= liftIO . (`shouldBe` formattedRangeTabSize5)

describe "formatting provider" $ do
let formatLspConfig provider =
object [ "languageServerHaskell" .= object ["formattingProvider" .= (provider :: Value)] ]
formatConfig provider = defaultConfig { lspConfig = Just (formatLspConfig provider) }
let formatConfig provider = defaultConfig { lspConfig = Just (formatLspConfig provider) }

it "respects none" $ runSessionWithConfig (formatConfig "none") hieCommand fullCaps "test/testdata" $ do
doc <- openDoc "Format.hs" "haskell"
Expand Down Expand Up @@ -93,9 +91,7 @@ spec = do
"foo x y = do\n print x\n return 42\n"]

describe "ormolu" $ do
let formatLspConfig provider =
object [ "languageServerHaskell" .= object ["formattingProvider" .= (provider :: Value)] ]


it "formats correctly" $ runSession hieCommand fullCaps "test/testdata" $ do
sendNotification WorkspaceDidChangeConfiguration (DidChangeConfigurationParams (formatLspConfig "ormolu"))
doc <- openDoc "Format.hs" "haskell"
Expand All @@ -107,6 +103,9 @@ spec = do
GHC86 -> formatted
_ -> liftIO $ docContent `shouldBe` unchangedOrmolu

formatLspConfig :: Value -> Value
formatLspConfig provider =
object [ "languageServerHaskell" .= object ["formattingProvider" .= provider] ]

formattedDocTabSize2 :: T.Text
formattedDocTabSize2 =
Expand Down
38 changes: 30 additions & 8 deletions test/unit/HsImportSpec.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@
module HsImportSpec where

import Control.Monad.IO.Class
import qualified Data.Text as T
import qualified Data.HashMap.Strict as Map
import qualified Data.HashMap.Strict as Map
import qualified Data.Text as T
import qualified Haskell.Ide.Engine.Config as Config
import Haskell.Ide.Engine.MonadTypes
import Haskell.Ide.Engine.PluginUtils
import Haskell.Ide.Engine.Plugin.HsImport
import qualified Haskell.Ide.Engine.Config as Config
import qualified Haskell.Ide.Engine.Plugin.Brittany as Brittany
import qualified Haskell.Ide.Engine.Plugin.Ormolu as Ormolu
import qualified Haskell.Ide.Engine.Plugin.Floskell as Floskell
import Haskell.Ide.Engine.Plugin.HsImport
import qualified Haskell.Ide.Engine.Plugin.Ormolu as Ormolu
import qualified Haskell.Ide.Engine.Plugin.Stylish as Stylish
import Haskell.Ide.Engine.PluginUtils
import System.Directory
import System.FilePath
import Test.Hspec
Expand All @@ -31,6 +32,7 @@ testPlugins = pluginDescToIdePlugins
[ Brittany.brittanyDescriptor "brittany"
, Floskell.floskellDescriptor "floskell"
, Ormolu.ormoluDescriptor "ormolu"
, Stylish.stylishDescriptor "stylish"
]

codeActionImportList :: FilePath
Expand Down Expand Up @@ -126,7 +128,27 @@ hsImportSpec = do
]
_ -> it "is NOP formatter" $
pendingWith "Ormolu only supported by GHC >= 8.6. Need to restore this."

describe "formats with stylish" $ hsImportSpecRunner "stylish"
[ -- Expected output for simple format.
[ TextEdit (Range (toPos (2, 1)) (toPos (2, 1))) "import Control.Monad\n"
]
, [ TextEdit (Range (toPos (2, 1)) (toPos (2, 1))) "import Control.Monad (when)\n"
]
, [ TextEdit (Range (toPos (2, 1)) (toPos (2, 1))) "import Data.Maybe (Maybe)\n"
]
, [ TextEdit (Range (toPos (2, 1)) (toPos (2, 1))) "import Data.Maybe (Maybe (..))\n"
]
, [ TextEdit (Range (toPos (2, 1)) (toPos (2, 1))) "import Data.Maybe (Maybe (Nothing))\n"
]
, [ TextEdit (Range (toPos (2, 1)) (toPos (2, 1))) "import Data.Function (($))\n"
]
, [ TextEdit (Range (toPos (2, 1)) (toPos (2, 32))) "import System.IO (IO, hPutStrLn)"
]
, [ TextEdit (Range (toPos (3, 1)) (toPos (3, 99))) $
"import Data.List (cons, find, head, init, last, length, null, reverse,\n" <>
" tail, uncons, union, (\\\\))"
]
]
-- ---------------------------------------------------------------------
-- Parameterized HsImport Spec.
-- ---------------------------------------------------------------------
Expand Down Expand Up @@ -196,4 +218,4 @@ expectHsImportResult formatterName fp uri expectedChanges act = do
IdeResultOk (WorkspaceEdit (Just changes) _) <- runSingle' (setFormatter formatterName) testPlugins fp act
case Map.lookup uri changes of
Just (List val) -> val `shouldBe` expectedChanges
Nothing -> fail "No Change found"
Nothing -> fail "No Change found"