From 16d6ea6980553fb3e40dfc62eb6bf2a6870440d9 Mon Sep 17 00:00:00 2001 From: Jon Shen Date: Mon, 31 Jan 2022 21:14:59 -0500 Subject: [PATCH 1/4] fix hlint parsing file with disabled extension --- plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs | 11 +++++------ plugins/hls-hlint-plugin/test/Main.hs | 3 +-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs b/plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs index 7a73ccfb5b..80359c7602 100644 --- a/plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs +++ b/plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs @@ -284,16 +284,16 @@ getIdeas nfp = do Just <$> liftIO (parseModuleEx flags' fp contents') setExtensions flags = do - hlintExts <- getExtensions flags nfp + hlintExts <- getExtensions nfp debugm $ "hlint:getIdeas:setExtensions:" ++ show hlintExts return $ flags { enabledExtensions = hlintExts } -getExtensions :: ParseFlags -> NormalizedFilePath -> Action [Extension] -getExtensions pflags nfp = do +getExtensions :: NormalizedFilePath -> Action [Extension] +getExtensions nfp = do dflags <- getFlags let hscExts = EnumSet.toList (extensionFlags dflags) let hscExts' = mapMaybe (GhclibParserEx.readExtension . show) hscExts - let hlintExts = nub $ enabledExtensions pflags ++ hscExts' + let hlintExts = nub hscExts' return hlintExts where getFlags :: Action DynFlags getFlags = do @@ -538,8 +538,7 @@ applyHint ide nfp mhint = liftIO $ withSystemTempFile (takeFileName fp) $ \temp h -> do hClose h writeFileUTF8NoNewLineTranslation temp oldContent - (pflags, _, _) <- runAction' $ useNoFile_ GetHlintSettings - exts <- runAction' $ getExtensions pflags nfp + exts <- runAction' $ getExtensions nfp -- We have to reparse extensions to remove the invalid ones let (enabled, disabled, _invalid) = Refact.parseExtensions $ map show exts let refactExts = map show $ enabled ++ disabled diff --git a/plugins/hls-hlint-plugin/test/Main.hs b/plugins/hls-hlint-plugin/test/Main.hs index 69d92a28dc..ec873e3fdc 100644 --- a/plugins/hls-hlint-plugin/test/Main.hs +++ b/plugins/hls-hlint-plugin/test/Main.hs @@ -212,8 +212,7 @@ suggestionsTests = length diags @?= 1 unusedExt ^. L.code @?= Just (InR "refact:Unused LANGUAGE pragma") - , knownBrokenForHlintOnGhcLib "[#1279] hlint uses a fixed set of extensions" $ - testCase "hlint should not activate extensions like PatternSynonyms" $ runHlintSession "" $ do + , testCase "[#1279] hlint should not activate extensions like PatternSynonyms" $ runHlintSession "" $ do doc <- openDoc "PatternKeyword.hs" "haskell" waitForAllProgressDone From 84f576f806239d2058b0c9a72ed392ae322f36fd Mon Sep 17 00:00:00 2001 From: Jon Shen Date: Mon, 31 Jan 2022 23:10:38 -0500 Subject: [PATCH 2/4] maybe test is magically fixed? --- plugins/hls-hlint-plugin/test/Main.hs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/hls-hlint-plugin/test/Main.hs b/plugins/hls-hlint-plugin/test/Main.hs index ec873e3fdc..65242266c2 100644 --- a/plugins/hls-hlint-plugin/test/Main.hs +++ b/plugins/hls-hlint-plugin/test/Main.hs @@ -134,8 +134,7 @@ suggestionsTests = changeDoc doc [change'] testHlintDiagnostics doc - , knownBrokenForHlintOnGhcLib "hlint doesn't take in account cpp flag as ghc -D argument" $ - testCase "[#554] hlint diagnostics works with CPP via ghc -XCPP argument" $ runHlintSession "cpp" $ do + , testCase "[#554] hlint diagnostics works with CPP via ghc -XCPP argument" $ runHlintSession "cpp" $ do doc <- openDoc "CppCond.hs" "haskell" testHlintDiagnostics doc From 01874ecc342a47ad33e0fcf89ff298af0efe7b13 Mon Sep 17 00:00:00 2001 From: Jon Shen Date: Tue, 1 Feb 2022 15:15:16 -0500 Subject: [PATCH 3/4] remove useness nub, add comments to getExtensions, add Known differences section to hlint plugin readme --- plugins/hls-hlint-plugin/README.md | 6 ++++++ plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs | 12 +++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/plugins/hls-hlint-plugin/README.md b/plugins/hls-hlint-plugin/README.md index fa775a35ca..77b33c0769 100644 --- a/plugins/hls-hlint-plugin/README.md +++ b/plugins/hls-hlint-plugin/README.md @@ -5,3 +5,9 @@ This is typically done through an [HLint configuration file](https://github.com/ndmitchell/hlint#customizing-the-hints). You can also change the behavior of HLint by adding a list of flags to `haskell.plugin.hlint.config.flags` if your configuration is in a non-standard location or you want to change settings globally. + +## Known Differences from the `hlint` executable + +- The `hlint` executable by default turns on many extensions when parsing a file because it is not certain about the exact extensions that apply to the file (they may come from project files). This differs from HLS which uses only the extensions the file needs to parse the file. Hence it is possible for the `hlint` executable to report a parse error on a file, but the `hlint` plugin to work just fine on the same file. This does mean that the turning on/off of extensions in the hlint config may be ignored by the `hlint` plugin. +- Hlint restrictions do not work (yet). This [PR](https://github.com/ndmitchell/hlint/pull/1340) should enable that functionality, but this requires a newer version of hlint to be used in HLS. + diff --git a/plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs b/plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs index 80359c7602..f42703457e 100644 --- a/plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs +++ b/plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs @@ -288,12 +288,22 @@ getIdeas nfp = do debugm $ "hlint:getIdeas:setExtensions:" ++ show hlintExts return $ flags { enabledExtensions = hlintExts } +-- Gets extensions from ModSummary dynflags for the file. +-- Previously this would concatenate extensions from both hlint's parsedFlags +-- and the ModSummary dynflags. However using the parsedFlags extensions +-- can sometimes interfere with the hlint parsing of the file. +-- See https://github.com/haskell/haskell-language-server/issues/1279 +-- +-- Note: this is used when HLINT_ON_GHC_LIB is not defined. We seem to need +-- these extensions to construct dynflags to parse the file again. Therefore +-- using hlint default extensions doesn't seem to be a problem when +-- HLINT_ON_GHC_LIB is not defined because we don't parse the file again. getExtensions :: NormalizedFilePath -> Action [Extension] getExtensions nfp = do dflags <- getFlags let hscExts = EnumSet.toList (extensionFlags dflags) let hscExts' = mapMaybe (GhclibParserEx.readExtension . show) hscExts - let hlintExts = nub hscExts' + let hlintExts = hscExts' return hlintExts where getFlags :: Action DynFlags getFlags = do From c8c3f6473c6d740bfff54f55ef49f364ac05c041 Mon Sep 17 00:00:00 2001 From: Jon Shen Date: Tue, 1 Feb 2022 15:33:20 -0500 Subject: [PATCH 4/4] remove redundant import --- plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs b/plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs index f42703457e..7620003e33 100644 --- a/plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs +++ b/plugins/hls-hlint-plugin/src/Ide/Plugin/Hlint.hs @@ -55,7 +55,6 @@ import Development.IDE.Core.Shake (getDiagnost import qualified Refact.Apply as Refact #ifdef HLINT_ON_GHC_LIB -import Data.List (nub) import Development.IDE.GHC.Compat (BufSpan, DynFlags, WarningFlag (Opt_WarnUnrecognisedPragmas),