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

Delete some dead or deprecated settings #2481

Merged
merged 5 commits into from Dec 14, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 0 additions & 5 deletions docs/configuration.md
Expand Up @@ -41,11 +41,6 @@ This option obviously would not make sense for language servers for other langua
Here is a list of the additional settings currently supported by `haskell-language-server`, along with their setting key (you may not need to know this) and default:

- Formatting provider (`haskell.formattingProvider`, default `ormolu`): what formatter to use; one of `floskell`, `ormolu`, `fourmolu`, `stylish-haskell`, or `brittany` (if compiled with the brittany plugin).
- Format on imports (`haskell.formatOnImportOn`, default true): whether to format after adding an import.
- Diagnostics on change (`haskell.diagnosticsOnChange`, default true): (currently unused).
- Completion snippets (`haskell.completionSnippetsOn`, default true): whether to support completion snippets. *Deprecated* as it is equivalent to `haskell.plugin.ghcide-completions.config.snippetsOn`.
pepeiborra marked this conversation as resolved.
Show resolved Hide resolved
- Liquid Haskell (`haskell.liquidOn`, default false): whether to enable Liquid Haskell support (currently unused until the Liquid Haskell support is functional again, see <https://github.com/haskell/haskell-language-server/issues/367>).
- Hlint (`haskell.hlintOn`, default true): whether to enable Hlint support. *Deprecated* as it is equivalent to `haskell.plugin.hlint.globalOn`
- Max completions (`haskell.maxCompletions`, default 40): maximum number of completions sent to the LSP client.
- Check project (`haskell.checkProject`, default true): whether to typecheck the entire project on load. As it is activated by default could drive to bad perfomance in large projects.
- Check parents (`haskell.checkParents`, default `CheckOnSaveAndClose`): when to typecheck reverse dependencies of a file; one of `NeverCheck`, `CheckOnClose`, `CheckOnSaveAndClose`, or `AlwaysCheck`.
Expand Down
1 change: 0 additions & 1 deletion haskell-language-server.cabal
Expand Up @@ -451,7 +451,6 @@ test-suite func-test
Format
FunctionalBadProject
FunctionalCodeAction
FunctionalLiquid
HieBios
Highlight
Progress
Expand Down
26 changes: 5 additions & 21 deletions hls-plugin-api/src/Ide/Plugin/Config.hs
Expand Up @@ -47,25 +47,17 @@ data CheckParents
-- will be surprises relating to config options being ignored, initially though.
data Config =
Config
{ checkParents :: CheckParents
, checkProject :: !Bool
, hlintOn :: !Bool
, diagnosticsOnChange :: !Bool
, liquidOn :: !Bool
, formatOnImportOn :: !Bool
, formattingProvider :: !T.Text
, maxCompletions :: !Int
, plugins :: !(Map.Map T.Text PluginConfig)
{ checkParents :: CheckParents
, checkProject :: !Bool
, formattingProvider :: !T.Text
, maxCompletions :: !Int
, plugins :: !(Map.Map T.Text PluginConfig)
} deriving (Show,Eq)

instance Default Config where
def = Config
{ checkParents = CheckOnSave
, checkProject = True
, hlintOn = True
, diagnosticsOnChange = True
, liquidOn = False
, formatOnImportOn = True
-- , formattingProvider = "brittany"
, formattingProvider = "ormolu"
-- , formattingProvider = "floskell"
Expand All @@ -85,10 +77,6 @@ parseConfig defValue = A.withObject "Config" $ \v -> do
Just s -> flip (A.withObject "Config.settings") s $ \o -> Config
<$> (o .:? "checkParents" <|> v .:? "checkParents") .!= checkParents defValue
<*> (o .:? "checkProject" <|> v .:? "checkProject") .!= checkProject defValue
<*> o .:? "hlintOn" .!= hlintOn defValue
<*> o .:? "diagnosticsOnChange" .!= diagnosticsOnChange defValue
<*> o .:? "liquidOn" .!= liquidOn defValue
<*> o .:? "formatOnImportOn" .!= formatOnImportOn defValue
<*> o .:? "formattingProvider" .!= formattingProvider defValue
<*> o .:? "maxCompletions" .!= maxCompletions defValue
<*> o .:? "plugin" .!= plugins defValue
Expand All @@ -99,10 +87,6 @@ instance A.ToJSON Config where
where
r = object [ "checkParents" .= checkParents
, "checkProject" .= checkProject
, "hlintOn" .= hlintOn
, "diagnosticsOnChange" .= diagnosticsOnChange
, "liquidOn" .= liquidOn
, "formatOnImportOn" .= formatOnImportOn
, "formattingProvider" .= formattingProvider
, "maxCompletions" .= maxCompletions
, "plugin" .= plugins
Expand Down
5 changes: 2 additions & 3 deletions plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs
Expand Up @@ -167,9 +167,8 @@ rules :: PluginId -> Rules ()
rules plugin = do
define $ \GetHlintDiagnostics file -> do
config <- getClientConfigAction def
let pluginConfig = configForPlugin config plugin
let hlintOn' = hlintOn config && plcGlobalOn pluginConfig && plcDiagnosticsOn pluginConfig
ideas <- if hlintOn' then getIdeas file else return (Right [])
let hlintOn = pluginEnabledConfig plcDiagnosticsOn plugin config
ideas <- if hlintOn then getIdeas file else return (Right [])
Copy link
Member

Choose a reason for hiding this comment

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

is not the deactivation handled automatically and generically by hls-plugin-api?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have no idea how this works 😅 If I do that then the test that changing the config works starts timing out 🤷

Copy link
Member

@jneira jneira Dec 14, 2021

Choose a reason for hiding this comment

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

hmm ok i think we should change noHlintDiagnostics for

noHlintDiagnostics :: Assertion
noHlintDiagnostics  = expectNoMoreDiagnostics 3 doc "hlint"

to avoid the parser hang on some diagnostic which never is emitted (not sure why it works keeping the if but 🤷 )

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you mind if I leave that as a future improvement, and leave this PR as just getting rid of the old settings?

return (diagnostics file ideas, Just ())

defineNoFile $ \GetHlintSettings -> do
Expand Down
43 changes: 15 additions & 28 deletions plugins/hls-hlint-plugin/test/Main.hs
Expand Up @@ -12,8 +12,7 @@ import Data.List (find)
import qualified Data.Map as Map
import Data.Maybe (fromJust, isJust)
import qualified Data.Text as T
import Ide.Plugin.Config (Config (..), PluginConfig (..),
hlintOn)
import Ide.Plugin.Config (Config (..), PluginConfig (..))
import qualified Ide.Plugin.Config as Plugin
import qualified Ide.Plugin.Hlint as HLint
import qualified Language.LSP.Types.Lens as L
Expand Down Expand Up @@ -251,40 +250,24 @@ suggestionsTests =
, "a = id @Int 1"
]


configTests :: TestTree
configTests = testGroup "hlint plugin config" [

testCase "changing hlintOn configuration enables or disables hlint diagnostics" $ runHlintSession "" $ do
let config = def { hlintOn = True }
sendConfigurationChanged (toJSON config)

doc <- openDoc "Base.hs" "haskell"
testHlintDiagnostics doc

let config' = def { hlintOn = False }
sendConfigurationChanged (toJSON config')

diags' <- waitForDiagnosticsFrom doc

liftIO $ noHlintDiagnostics diags'

, testCase "changing hlint plugin configuration enables or disables hlint diagnostics" $ runHlintSession "" $ do
let config = def { hlintOn = True }
sendConfigurationChanged (toJSON config)
testCase "changing hlint plugin configuration enables or disables hlint diagnostics" $ runHlintSession "" $ do
enableHlint

doc <- openDoc "Base.hs" "haskell"
testHlintDiagnostics doc

let config' = pluginGlobalOn config "hlint" False
sendConfigurationChanged (toJSON config')
disableHlint

diags' <- waitForDiagnosticsFrom doc

liftIO $ noHlintDiagnostics diags'

, testCase "adding hlint flags to plugin configuration removes hlint diagnostics" $ runHlintSession "" $ do
let config = def { hlintOn = True }
sendConfigurationChanged (toJSON config)
enableHlint

doc <- openDoc "Base.hs" "haskell"
testHlintDiagnostics doc
Expand All @@ -297,8 +280,7 @@ configTests = testGroup "hlint plugin config" [
liftIO $ noHlintDiagnostics diags'

, testCase "adding hlint flags to plugin configuration adds hlint diagnostics" $ runHlintSession "" $ do
let config = def { hlintOn = True }
sendConfigurationChanged (toJSON config)
enableHlint

doc <- openDoc "Generalise.hs" "haskell"

Expand Down Expand Up @@ -341,14 +323,19 @@ pluginGlobalOn config pid state = config'
hlintConfigWithFlags :: [T.Text] -> Config
hlintConfigWithFlags flags =
def
{ hlintOn = True
, Plugin.plugins = Map.fromList [("hlint",
def { Plugin.plcConfig = unObject $ object ["flags" .= flags] }
{ Plugin.plugins = Map.fromList [("hlint",
def { Plugin.plcGlobalOn = True, Plugin.plcConfig = unObject $ object ["flags" .= flags] }
)] }
where
unObject (Object obj) = obj
unObject _ = undefined

enableHlint :: Session ()
enableHlint = sendConfigurationChanged $ toJSON $ def { Plugin.plugins = Map.fromList [ ("hlint", def { Plugin.plcGlobalOn = True }) ] }

disableHlint :: Session ()
disableHlint = sendConfigurationChanged $ toJSON $ def { Plugin.plugins = Map.fromList [ ("hlint", def { Plugin.plcGlobalOn = False }) ] }

-- We have two main code paths in the plugin depending on how hlint interacts with ghc:
-- * One when hlint uses ghc-lib (all ghc versions but the last version supported by hlint)
-- * Another one when hlint uses directly ghc (only one version, which not have to be the last version supported by ghcide)
Expand Down
25 changes: 0 additions & 25 deletions test/functional/Diagnostic.hs
Expand Up @@ -3,9 +3,6 @@
module Diagnostic (tests) where

import Control.Lens hiding (List)
import Data.Aeson (toJSON)
import qualified Data.Default
import Ide.Plugin.Config
import qualified Language.LSP.Types.Lens as LSP
import Test.Hls
import Test.Hls.Command
Expand All @@ -15,7 +12,6 @@ import Test.Hls.Command
tests :: TestTree
tests = testGroup "diagnostics providers" [
basicTests
, saveTests
, warningTests
]

Expand All @@ -41,24 +37,3 @@ warningTests = testGroup "Warnings are warnings" [
liftIO $ diag ^. LSP.severity @?= Just DsWarning
]

saveTests :: TestTree
saveTests = testGroup "only diagnostics on save" [
ignoreTestBecause "diagnosticsOnChange parameter is not supported right now" $ testCase "Respects diagnosticsOnChange setting" $
runSession hlsCommandExamplePlugin codeActionSupportCaps "test/testdata" $ do
let config = Data.Default.def { diagnosticsOnChange = False } :: Config
sendConfigurationChanged (toJSON config)
doc <- openDoc "Hover.hs" "haskell"
diags <- waitForDiagnosticsFrom doc

liftIO $ do
length diags @?= 0

let te = TextEdit (Range (Position 0 0) (Position 0 13)) ""
_ <- applyEdit doc te
skipManyTill loggingNotification noDiagnostics

sendNotification STextDocumentDidSave (DidSaveTextDocumentParams doc Nothing)
diags2 <- waitForDiagnosticsFrom doc
liftIO $
length diags2 @?= 1
]
31 changes: 0 additions & 31 deletions test/functional/FunctionalLiquid.hs

This file was deleted.

2 changes: 0 additions & 2 deletions test/functional/Main.hs
Expand Up @@ -9,7 +9,6 @@ import Diagnostic
import Format
import FunctionalBadProject
import FunctionalCodeAction
import FunctionalLiquid
import HieBios
import Highlight
import Progress
Expand All @@ -31,7 +30,6 @@ main = defaultTestRunner
, ignoreInEnv [HostOS Windows, GhcVer GHC90] "Tests gets stuck in ci" $ Format.tests
, FunctionalBadProject.tests
, FunctionalCodeAction.tests
, FunctionalLiquid.tests
, HieBios.tests
, Highlight.tests
, ignoreInEnv [HostOS Windows, GhcVer GHC90] "Tests gets stuck in ci" $ Progress.tests
Expand Down
11 changes: 1 addition & 10 deletions test/functional/Progress.hs
Expand Up @@ -9,11 +9,10 @@ module Progress (tests) where

import Control.Lens hiding ((.=))
import Data.Aeson (Value, decode, encode, object,
toJSON, (.=))
(.=))
import Data.List (delete)
import Data.Maybe (fromJust)
import Data.Text (Text, pack)
import Ide.Plugin.Config
import Language.LSP.Types.Capabilities
import qualified Language.LSP.Types.Lens as L
import System.FilePath ((</>))
Expand Down Expand Up @@ -52,14 +51,6 @@ tests =
expectProgressReports ["Setting up testdata (for Format.hs)", "Processing", "Indexing"]
_ <- sendRequest STextDocumentFormatting $ DocumentFormattingParams Nothing doc (FormattingOptions 2 True Nothing Nothing Nothing)
expectProgressReports ["Formatting Format.hs"]
, ignoreTestBecause "no liquid Haskell support" $
testCase "liquid haskell plugin sends progress notifications" $ do
runSession hlsCommand progressCaps "test/testdata" $ do
doc <- openDoc "liquid/Evens.hs" "haskell"
let config = def{liquidOn = True, hlintOn = False}
sendConfigurationChanged (toJSON config)
sendNotification STextDocumentDidSave (DidSaveTextDocumentParams doc Nothing)
expectProgressReports ["Running Liquid Haskell on Evens.hs"]
]

formatLspConfig :: Value -> Value
Expand Down