Skip to content

Commit 5c11636

Browse files
keithfanchermichaelpjmergify[bot]
authored
Use relative file paths for HIE files and Stan's config maps (#4023)
* Use relative file paths for HIE files and Stan's config maps Stan expects relative paths. Without this change, file names won't map correctly to their associated language extension data, which means no enabled extensions will be detected. This causes annoying false positives with, e.g., the `StrictData` extension. (See issue #3174.) * Un-exclude Stan diagnostics related to `StrictData` We specifically want to test this diagnostic, so we need it to fire. * Add tests to ensure the Stan plugin detects a module's language extensions Includes test cases for both `LANGUAGE` pragmas and extensions enabled in a project's `.cabal` file. * Tighten up Stan plugin language extension test cases These changes ensure that the tests will fail given bad mappings in either the `cabalExtensionsMap` OR the `checksMap`. Either of these could cause bad behavior as seen in issue #3174. * Use correct extension/file mappings even in the case of a config fiasco The Stan plugin will still operate as expected even if we can't load a config -- it will simply default to showing all inspections. * Remove a slew of unused imports * Use OS-agnostic path separators in tests * Run `stylish-haskell` * Ensure `hs-source-dirs` in test cabal files don't contain path separators Related to (what I assume is) a bug in Stan, or its `extensions` library. Regardless of OS, the `hs-source-dirs` field is prepended as-is to the module name to create the file paths used in the cabal extensions map. This means the maps won't work in Windows if your cabal file contains `/` path separators. Working around the limitation here to ensure tests work on all platforms. --------- Co-authored-by: Michael Peyton Jones <me@michaelpj.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
1 parent 4b95e55 commit 5c11636

File tree

8 files changed

+111
-95
lines changed

8 files changed

+111
-95
lines changed

‎plugins/hls-stan-plugin/src/Ide/Plugin/Stan.hs

+57-77
Original file line numberDiff line numberDiff line change
@@ -2,68 +2,47 @@
22
{-# LANGUAGE PatternSynonyms #-}
33
module Ide.Plugin.Stan (descriptor, Log) where
44

5-
import Compat.HieTypes (HieASTs, HieFile (..))
6-
import Control.DeepSeq (NFData)
7-
import Control.Monad (void, when)
8-
import Control.Monad.IO.Class (liftIO)
9-
import Control.Monad.Trans.Maybe (MaybeT (MaybeT), runMaybeT)
10-
import Data.Default
11-
import Data.Foldable (toList)
12-
import Data.Hashable (Hashable)
13-
import qualified Data.HashMap.Strict as HM
14-
import Data.HashSet (HashSet)
15-
import qualified Data.HashSet as HS
16-
import qualified Data.Map as Map
17-
import Data.Maybe (fromJust, mapMaybe,
18-
maybeToList)
19-
import Data.String (IsString (fromString))
20-
import qualified Data.Text as T
5+
import Compat.HieTypes (HieFile (..))
6+
import Control.DeepSeq (NFData)
7+
import Control.Monad (void)
8+
import Control.Monad.IO.Class (liftIO)
9+
import Data.Foldable (toList)
10+
import Data.Hashable (Hashable)
11+
import qualified Data.HashMap.Strict as HM
12+
import Data.Maybe (mapMaybe)
13+
import qualified Data.Text as T
2114
import Development.IDE
22-
import Development.IDE.Core.Rules (getHieFile,
23-
getSourceFileSource)
24-
import Development.IDE.Core.RuleTypes (HieAstResult (..))
25-
import qualified Development.IDE.Core.Shake as Shake
26-
import Development.IDE.GHC.Compat (HieASTs (HieASTs),
27-
HieFile (hie_hs_file),
28-
RealSrcSpan (..), mkHieFile',
29-
mkRealSrcLoc, mkRealSrcSpan,
30-
runHsc, srcSpanEndCol,
31-
srcSpanEndLine,
32-
srcSpanStartCol,
33-
srcSpanStartLine, tcg_exports)
34-
import Development.IDE.GHC.Error (realSrcSpanToRange)
35-
import GHC.Generics (Generic)
36-
import Ide.Plugin.Config (PluginConfig (..))
37-
import Ide.Types (PluginDescriptor (..),
38-
PluginId, configHasDiagnostics,
39-
configInitialGenericConfig,
40-
defaultConfigDescriptor,
41-
defaultPluginDescriptor)
42-
import qualified Language.LSP.Protocol.Types as LSP
43-
import Stan (createCabalExtensionsMap,
44-
getStanConfig)
45-
import Stan.Analysis (Analysis (..), runAnalysis)
46-
import Stan.Category (Category (..))
47-
import Stan.Cli (StanArgs (..))
48-
import Stan.Config (Config, ConfigP (..),
49-
applyConfig, defaultConfig)
50-
import Stan.Config.Pretty (ConfigAction, configToTriples,
51-
prettyConfigAction,
52-
prettyConfigCli)
53-
import Stan.Core.Id (Id (..))
54-
import Stan.EnvVars (EnvVars (..), envVarsToText)
55-
import Stan.Inspection (Inspection (..))
56-
import Stan.Inspection.All (inspectionsIds, inspectionsMap)
57-
import Stan.Observation (Observation (..))
58-
import Stan.Report.Settings (OutputSettings (..),
59-
ToggleSolution (..),
60-
Verbosity (..))
61-
import Stan.Toml (usedTomlFiles)
62-
import System.Directory (makeRelativeToCurrentDirectory)
63-
import Trial (Fatality, Trial (..), fiasco,
64-
pattern FiascoL,
65-
pattern ResultL, prettyTrial,
66-
prettyTrialWith)
15+
import Development.IDE.Core.Rules (getHieFile)
16+
import qualified Development.IDE.Core.Shake as Shake
17+
import GHC.Generics (Generic)
18+
import Ide.Plugin.Config (PluginConfig (..))
19+
import Ide.Types (PluginDescriptor (..), PluginId,
20+
configHasDiagnostics,
21+
configInitialGenericConfig,
22+
defaultConfigDescriptor,
23+
defaultPluginDescriptor)
24+
import qualified Language.LSP.Protocol.Types as LSP
25+
import Stan (createCabalExtensionsMap,
26+
getStanConfig)
27+
import Stan.Analysis (Analysis (..), runAnalysis)
28+
import Stan.Category (Category (..))
29+
import Stan.Cli (StanArgs (..))
30+
import Stan.Config (Config, ConfigP (..), applyConfig)
31+
import Stan.Config.Pretty (prettyConfigCli)
32+
import Stan.Core.Id (Id (..))
33+
import Stan.EnvVars (EnvVars (..), envVarsToText)
34+
import Stan.Inspection (Inspection (..))
35+
import Stan.Inspection.All (inspectionsIds, inspectionsMap)
36+
import Stan.Observation (Observation (..))
37+
import Stan.Report.Settings (OutputSettings (..),
38+
ToggleSolution (..),
39+
Verbosity (..))
40+
import Stan.Toml (usedTomlFiles)
41+
import System.Directory (makeRelativeToCurrentDirectory)
42+
import Trial (Fatality, Trial (..), fiasco,
43+
pattern FiascoL, pattern ResultL,
44+
prettyTrial, prettyTrialWith)
45+
6746
descriptor :: Recorder (WithPriority Log) -> PluginId -> PluginDescriptor IdeState
6847
descriptor recorder plId = (defaultPluginDescriptor plId desc)
6948
{ pluginRules = rules recorder plId
@@ -164,24 +143,25 @@ rules recorder plId = do
164143
logWith recorder Debug (LogDebugStanEnvVars env)
165144
seTomlFiles <- liftIO $ usedTomlFiles useDefConfig (stanArgsConfigFile stanArgs)
166145

167-
(cabalExtensionsMap, checksMap, confIgnored) <- case configTrial of
146+
-- Note that Stan works in terms of relative paths, but the HIE come in as absolute. Without
147+
-- making its path relative, the file name(s) won't line up with the associated Map keys.
148+
relativeHsFilePath <- liftIO $ makeRelativeToCurrentDirectory $ fromNormalizedFilePath file
149+
let hieRelative = hie{hie_hs_file=relativeHsFilePath}
150+
151+
(checksMap, ignoredObservations) <- case configTrial of
168152
FiascoL es -> do
169153
logWith recorder Development.IDE.Warning (LogWarnConf es)
170-
pure (Map.empty,
171-
HM.fromList [(LSP.fromNormalizedFilePath file, inspectionsIds)],
172-
[])
173-
ResultL warnings stanConfig -> do
174-
let currentHSAbs = fromNormalizedFilePath file -- hie_hs_file hie
175-
currentHSRel <- liftIO $ makeRelativeToCurrentDirectory currentHSAbs
176-
cabalExtensionsMap <- liftIO $ createCabalExtensionsMap isLoud (stanArgsCabalFilePath stanArgs) [hie]
177-
178-
-- Files (keys) in checksMap need to have an absolute path for the analysis, but applyConfig needs to receive relative
179-
-- filepaths to apply the config, because the toml config has relative paths. Stan itself seems to work only in terms of relative paths.
180-
let checksMap = HM.mapKeys (const currentHSAbs) $ applyConfig [currentHSRel] stanConfig
181-
182-
let analysis = runAnalysis cabalExtensionsMap checksMap (configIgnored stanConfig) [hie]
183-
pure (cabalExtensionsMap, checksMap, configIgnored stanConfig)
184-
let analysis = runAnalysis cabalExtensionsMap checksMap confIgnored [hie]
154+
-- If we can't read the config file, default to using all inspections:
155+
let allInspections = HM.fromList [(relativeHsFilePath, inspectionsIds)]
156+
pure (allInspections, [])
157+
ResultL _warnings stanConfig -> do
158+
-- HashMap of *relative* file paths to info about enabled checks for those file paths.
159+
let checksMap = applyConfig [relativeHsFilePath] stanConfig
160+
pure (checksMap, configIgnored stanConfig)
161+
162+
-- A Map from *relative* file paths (just one, in this case) to language extension info:
163+
cabalExtensionsMap <- liftIO $ createCabalExtensionsMap isLoud (stanArgsCabalFilePath stanArgs) [hieRelative]
164+
let analysis = runAnalysis cabalExtensionsMap checksMap ignoredObservations [hieRelative]
185165
return (analysisToDiagnostics file analysis, Just ())
186166
else return ([], Nothing)
187167

‎plugins/hls-stan-plugin/test/Main.hs

+18-6
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,7 @@ module Main
44
where
55

66
import Control.Lens ((^.))
7-
import Control.Monad (void)
8-
import Data.List (find)
9-
import Data.Text (Text)
107
import qualified Data.Text as T
11-
import qualified Data.Text.IO as T
128
import qualified Ide.Plugin.Stan as Stan
139
import Ide.Types
1410
import qualified Language.LSP.Protocol.Lens as L
@@ -36,14 +32,30 @@ tests =
3632
return ()
3733
, testCase "ignores diagnostics from .stan.toml" $
3834
runStanSession "" $ do
39-
doc <- openDoc "dir/configTest.hs" "haskell"
35+
doc <- openDoc ("dir" </> "configTest.hs") "haskell"
4036
diags <- waitForDiagnosticsFromSource doc "stan"
4137
liftIO $ length diags @?= 0
4238
return ()
39+
, testCase "respects LANGUAGE pragmas in the source file" $
40+
runStanSession "" $ do
41+
doc <- openDoc ("extensions-language-pragma" </> "LanguagePragmaTest.hs") "haskell"
42+
diags <- waitForDiagnosticsFromSource doc "stan"
43+
-- We must include at least one valid diagnostic in our test file to avoid
44+
-- the false-positive case where Stan finds no analyses to perform due to a
45+
-- bad mapping, which would also lead to zero diagnostics being returned.
46+
liftIO $ length diags @?= 1
47+
return ()
48+
, testCase "respects language extensions defined in the .cabal file" $
49+
runStanSession "" $ do
50+
doc <- openDoc ("extensions-cabal-file" </> "CabalFileTest.hs") "haskell"
51+
diags <- waitForDiagnosticsFromSource doc "stan"
52+
-- We need at least one valid diagnostic here too, for the same reason as above.
53+
liftIO $ length diags @?= 1
54+
return ()
4355
]
4456

4557
testDir :: FilePath
46-
testDir = "plugins/hls-stan-plugin/test/testdata"
58+
testDir = "plugins" </> "hls-stan-plugin" </> "test" </> "testdata"
4759

4860
stanPlugin :: PluginTestDescriptor Stan.Log
4961
stanPlugin = mkPluginTestDescriptor enabledStanDescriptor "stan"

‎plugins/hls-stan-plugin/test/testdata/.stan.toml

-10
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,5 @@
11
# See https://github.com/kowainik/stan/issues/531
22
# Unix
3-
[[check]]
4-
type = "Exclude"
5-
id = "STAN-0206"
6-
scope = "all"
7-
83
[[check]]
94
type = "Exclude"
105
id = "STAN-0103"
@@ -16,11 +11,6 @@ id = "STAN-0212"
1611
directory = "dir/"
1712

1813
# Windows
19-
[[check]]
20-
type = "Exclude"
21-
id = "STAN-0206"
22-
scope = "all"
23-
2414
[[check]]
2515
type = "Exclude"
2616
id = "STAN-0103"

‎plugins/hls-stan-plugin/test/testdata/dir/configTest.hs

-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
data A = A Int Int
2-
31
a = length [1..]
42

53
b = undefined

‎plugins/hls-stan-plugin/test/testdata/extensions-cabal-file/CabalFileTest.hs

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
module CabalFileTest () where
2+
3+
-- With `StrictData` enabled in the `.cabal` file, Stan shouldn't complain here:
4+
data A = A Int Int
5+
6+
-- ...but it should still complain here!
7+
kewlFunc = undefined

‎plugins/hls-stan-plugin/test/testdata/extensions-cabal-file/cabal-file-test.cabal

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
cabal-version: 3.0
2+
name: cabal-file-test
3+
version: 0.0.0.0
4+
5+
library
6+
exposed-modules: CabalFileTest
7+
hs-source-dirs: extensions-cabal-file
8+
-- Specifically, we're testing that Stan respects the following extension definition:
9+
default-extensions: StrictData

‎plugins/hls-stan-plugin/test/testdata/extensions-language-pragma/LanguagePragmaTest.hs

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{-# LANGUAGE StrictData #-}
2+
3+
module LanguagePragmaTest () where
4+
5+
-- With the above `StrictData` language pragma, Stan shouldn't complain here:
6+
data A = A Int Int
7+
8+
-- ...but it should still complain here!
9+
kewlFunc = undefined

‎plugins/hls-stan-plugin/test/testdata/extensions-language-pragma/language-pragma-test.cabal

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
cabal-version: 3.0
2+
name: language-pragma-test
3+
version: 0.0.0.0
4+
5+
-- Without at least a minimal valid `.cabal` file, Stan won't bother building its
6+
-- map of language extensions. This means it also won't detect LANGUAGE pragmas
7+
-- without this file.
8+
9+
library
10+
exposed-modules: LanguagePragmaTest
11+
hs-source-dirs: extensions-language-pragma

0 commit comments

Comments
 (0)