Skip to content

Migrate change-type-signature-plugin to use structured diagnostics #4632

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

Merged
merged 4 commits into from
Jun 23, 2025
Merged
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
43 changes: 42 additions & 1 deletion ghcide/src/Development/IDE/GHC/Compat/Error.hs
Original file line number Diff line number Diff line change
@@ -17,16 +17,24 @@ module Development.IDE.GHC.Compat.Error (
DriverMessage (..),
-- * General Diagnostics
Diagnostic(..),
-- * Prisms for error selection
-- * Prisms and lenses for error selection
_TcRnMessage,
_TcRnMessageWithCtx,
_GhcPsMessage,
_GhcDsMessage,
_GhcDriverMessage,
_TcRnMissingSignature,
_TcRnSolverReport,
_TcRnMessageWithInfo,
reportContextL,
reportContentL,
_MismatchMessage,
_TypeEqMismatchActual,
_TypeEqMismatchExpected,
) where

import Control.Lens
import Development.IDE.GHC.Compat (Type)
import GHC.Driver.Errors.Types
import GHC.HsToCore.Errors.Types
import GHC.Tc.Errors.Types
@@ -82,3 +90,36 @@ msgEnvelopeErrorL :: Lens' (MsgEnvelope e) e
msgEnvelopeErrorL = lens errMsgDiagnostic (\envelope e -> envelope { errMsgDiagnostic = e } )

makePrisms ''TcRnMessage

makeLensesWith
(lensRules & lensField .~ mappingNamer (pure . (++ "L")))
''SolverReportWithCtxt

-- | Focus 'MismatchMsg' from 'TcSolverReportMsg'. Currently, 'MismatchMsg' can be
-- extracted from 'CannotUnifyVariable' and 'Mismatch' constructors.
_MismatchMessage :: Traversal' TcSolverReportMsg MismatchMsg
_MismatchMessage focus (Mismatch msg t a c) = (\msg' -> Mismatch msg' t a c) <$> focus msg
_MismatchMessage focus (CannotUnifyVariable msg a) = flip CannotUnifyVariable a <$> focus msg
_MismatchMessage _ report = pure report

-- | Focus 'teq_mismatch_expected' from 'TypeEqMismatch'.
_TypeEqMismatchExpected :: Traversal' MismatchMsg Type
#if MIN_VERSION_ghc(9,12,0)
_TypeEqMismatchExpected focus mismatch@(TypeEqMismatch _ _ _ expected _ _ _) =
(\expected' -> mismatch { teq_mismatch_expected = expected' }) <$> focus expected
#else
_TypeEqMismatchExpected focus mismatch@(TypeEqMismatch _ _ _ _ expected _ _ _) =
(\expected' -> mismatch { teq_mismatch_expected = expected' }) <$> focus expected
#endif
_TypeEqMismatchExpected _ mismatch = pure mismatch

-- | Focus 'teq_mismatch_actual' from 'TypeEqMismatch'.
_TypeEqMismatchActual :: Traversal' MismatchMsg Type
#if MIN_VERSION_ghc(9,12,0)
_TypeEqMismatchActual focus mismatch@(TypeEqMismatch _ _ _ _ actual _ _) =
(\actual' -> mismatch { teq_mismatch_actual = actual' }) <$> focus actual
#else
_TypeEqMismatchActual focus mismatch@(TypeEqMismatch _ _ _ _ _ actual _ _) =
(\actual' -> mismatch { teq_mismatch_expected = actual' }) <$> focus actual
#endif
_TypeEqMismatchActual _ mismatch = pure mismatch
3 changes: 3 additions & 0 deletions haskell-language-server.cabal
Original file line number Diff line number Diff line change
@@ -1173,12 +1173,14 @@ library hls-change-type-signature-plugin
build-depends:
, ghcide == 2.11.0.0
, hls-plugin-api == 2.11.0.0
, lens
, lsp-types
, regex-tdfa
, syb
, text
, transformers
, containers
, ghc
default-extensions:
DataKinds
ExplicitNamespaces
@@ -1196,6 +1198,7 @@ test-suite hls-change-type-signature-plugin-tests
build-depends:
, filepath
, haskell-language-server:hls-change-type-signature-plugin
, hls-plugin-api
, hls-test-utils == 2.11.0.0
, regex-tdfa
, text
Original file line number Diff line number Diff line change
@@ -1,47 +1,93 @@
{-# LANGUAGE CPP #-}
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE ViewPatterns #-}
-- | An HLS plugin to provide code actions to change type signatures
module Ide.Plugin.ChangeTypeSignature (descriptor
-- * For Unit Tests
, Log(..)
, errorMessageRegexes
) where

import Control.Monad (guard)
import Control.Monad.IO.Class (MonadIO)
import Control.Monad.Trans.Except (ExceptT)
import Data.Foldable (asum)
import qualified Data.Map as Map
import Data.Maybe (mapMaybe)
import Data.Text (Text)
import qualified Data.Text as T
import Development.IDE (realSrcSpanToRange)
import Control.Lens
import Control.Monad (guard)
import Control.Monad.IO.Class (MonadIO)
import Control.Monad.Trans.Class (MonadTrans (lift))
import Control.Monad.Trans.Except (ExceptT (..))
import Control.Monad.Trans.Maybe (MaybeT (..), hoistMaybe)
import Data.Foldable (asum)
import qualified Data.Map as Map
import Data.Maybe (catMaybes)
import Data.Text (Text)
import qualified Data.Text as T
import Development.IDE (FileDiagnostic,
IdeState (..), Pretty (..),
Priority (..), Recorder,
WithPriority,
fdLspDiagnosticL,
fdStructuredMessageL,
logWith, realSrcSpanToRange)
import Development.IDE.Core.PluginUtils
import Development.IDE.Core.RuleTypes (GetParsedModule (GetParsedModule))
import Development.IDE.Core.Service (IdeState)
import Development.IDE.GHC.Compat
import Development.IDE.GHC.Util (printOutputable)
import Generics.SYB (extQ, something)
import Ide.Plugin.Error (PluginError,
getNormalizedFilePathE)
import Ide.Types (PluginDescriptor (..),
PluginId (PluginId),
PluginMethodHandler,
defaultPluginDescriptor,
mkPluginHandler)
import Development.IDE.Core.RuleTypes (GetParsedModule (GetParsedModule))
import Development.IDE.GHC.Compat hiding (vcat)
import Development.IDE.GHC.Compat.Error (_MismatchMessage,
_TcRnMessageWithCtx,
_TcRnMessageWithInfo,
_TcRnSolverReport,
_TypeEqMismatchActual,
_TypeEqMismatchExpected,
msgEnvelopeErrorL,
reportContentL)
import Development.IDE.GHC.Util (printOutputable)
import Development.IDE.Types.Diagnostics (_SomeStructuredMessage)
import Generics.SYB (extQ, something)
import GHC.Tc.Errors.Types (ErrInfo (..),
TcRnMessageDetailed (..))
import qualified Ide.Logger as Logger
import Ide.Plugin.Error (PluginError,
getNormalizedFilePathE)
import Ide.Types (Config, HandlerM,
PluginDescriptor (..),
PluginId (PluginId),
PluginMethodHandler,
defaultPluginDescriptor,
mkPluginHandler)
import Language.LSP.Protocol.Message
import Language.LSP.Protocol.Types
import Text.Regex.TDFA ((=~))

descriptor :: PluginId -> PluginDescriptor IdeState
descriptor plId = (defaultPluginDescriptor plId "Provides a code action to change the type signature of a binding if it is wrong")
{ pluginHandlers = mkPluginHandler SMethod_TextDocumentCodeAction (codeActionHandler plId) }

codeActionHandler :: PluginId -> PluginMethodHandler IdeState 'Method_TextDocumentCodeAction
codeActionHandler plId ideState _ CodeActionParams {_textDocument = TextDocumentIdentifier uri, _context = CodeActionContext diags _ _} = do
nfp <- getNormalizedFilePathE uri
decls <- getDecls plId ideState nfp
let actions = mapMaybe (generateAction plId uri decls) diags
pure $ InL actions
import Text.Regex.TDFA ((=~))

data Log
= LogErrInfoCtxt ErrInfo
| LogFindSigLocFailure DeclName

instance Pretty Log where
pretty = \case
LogErrInfoCtxt (ErrInfo ctxt suppl) ->
Logger.vcat [fromSDoc ctxt, fromSDoc suppl]
LogFindSigLocFailure name ->
pretty ("Lookup signature location failure: " <> name)
where
fromSDoc = pretty . printOutputable

descriptor :: Recorder (WithPriority Log) -> PluginId -> PluginDescriptor IdeState
descriptor recorder plId =
(defaultPluginDescriptor plId "Provides a code action to change the type signature of a binding if it is wrong")
{ pluginHandlers = mkPluginHandler SMethod_TextDocumentCodeAction (codeActionHandler recorder plId)
}

codeActionHandler
:: Recorder (WithPriority Log)
-> PluginId
-> PluginMethodHandler IdeState 'Method_TextDocumentCodeAction
codeActionHandler recorder plId ideState _ CodeActionParams{_textDocument, _range} = do
let TextDocumentIdentifier uri = _textDocument
nfp <- getNormalizedFilePathE uri
decls <- getDecls plId ideState nfp

activeDiagnosticsInRange (shakeExtras ideState) nfp _range >>= \case
Nothing -> pure (InL [])
Just fileDiags -> do
actions <- lift $ mapM (generateAction recorder plId uri decls) fileDiags
pure (InL (catMaybes actions))

getDecls :: MonadIO m => PluginId -> IdeState -> NormalizedFilePath -> ExceptT PluginError m [LHsDecl GhcPs]
getDecls (PluginId changeTypeSignatureId) state =
@@ -67,39 +113,74 @@ data ChangeSignature = ChangeSignature {
-- | the location of the declaration signature
, declSrcSpan :: RealSrcSpan
-- | the diagnostic to solve
, diagnostic :: Diagnostic
, diagnostic :: FileDiagnostic
}

-- | Create a CodeAction from a Diagnostic
generateAction :: PluginId -> Uri -> [LHsDecl GhcPs] -> Diagnostic -> Maybe (Command |? CodeAction)
generateAction plId uri decls diag = changeSigToCodeAction plId uri <$> diagnosticToChangeSig decls diag
generateAction
:: Recorder (WithPriority Log)
-> PluginId
-> Uri
-> [LHsDecl GhcPs]
-> FileDiagnostic
-> HandlerM Config (Maybe (Command |? CodeAction))
generateAction recorder plId uri decls fileDiag = do
changeSig <- diagnosticToChangeSig recorder decls fileDiag
pure $
changeSigToCodeAction plId uri <$> changeSig

-- | Convert a diagnostic into a ChangeSignature and add the proper SrcSpan
diagnosticToChangeSig :: [LHsDecl GhcPs] -> Diagnostic -> Maybe ChangeSignature
diagnosticToChangeSig decls diagnostic = do
-- regex match on the GHC Error Message
(expectedType, actualType, declName) <- matchingDiagnostic diagnostic
-- Find the definition and it's location
declSrcSpan <- findSigLocOfStringDecl decls expectedType (T.unpack declName)
pure $ ChangeSignature{..}

diagnosticToChangeSig
:: Recorder (WithPriority Log)
-> [LHsDecl GhcPs]
-> FileDiagnostic
-> HandlerM Config (Maybe ChangeSignature)
diagnosticToChangeSig recorder decls diagnostic = runMaybeT $ do
-- Extract expected, actual, and extra error info
(expectedType, actualType, errInfo) <- hoistMaybe $ do
msg <- diagnostic ^. fdStructuredMessageL ^? _SomeStructuredMessage
tcRnMsg <- msg ^. msgEnvelopeErrorL ^? _TcRnMessageWithCtx
(_, TcRnMessageDetailed errInfo tcRnMsg') <- tcRnMsg ^? _TcRnMessageWithInfo
solverReport <- tcRnMsg' ^? _TcRnSolverReport . _1 . reportContentL
mismatch <- solverReport ^? _MismatchMessage
expectedType <- mismatch ^? _TypeEqMismatchExpected
actualType <- mismatch ^? _TypeEqMismatchActual

pure (showType expectedType, showType actualType, errInfo)

logWith recorder Debug (LogErrInfoCtxt errInfo)

-- Extract the declName from the extra error text
declName <- hoistMaybe (matchingDiagnostic errInfo)

-- Look up location of declName. If it fails, log it
declSrcSpan <-
case findSigLocOfStringDecl decls expectedType (T.unpack declName) of
Just x -> pure x
Nothing -> do
logWith recorder Debug (LogFindSigLocFailure declName)
hoistMaybe Nothing

pure ChangeSignature{..}
where
showType :: Type -> Text
showType = T.pack . showSDocUnsafe . pprTidiedType

-- | If a diagnostic has the proper message create a ChangeSignature from it
matchingDiagnostic :: Diagnostic -> Maybe (ExpectedSig, ActualSig, DeclName)
matchingDiagnostic Diagnostic{_message} = asum $ map (unwrapMatch . (=~) _message) errorMessageRegexes
matchingDiagnostic :: ErrInfo -> Maybe DeclName
matchingDiagnostic ErrInfo{errInfoContext} =
Comment on lines +170 to +171
Copy link
Collaborator

Choose a reason for hiding this comment

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

We still need that? Should we open a GHC issue for adding the DeclName context to the error message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We still need that?

Sadly, I think we do

Should we open a GHC issue for adding the DeclName context to the error message?

That would be awesome

asum $ map (unwrapMatch . (=~) errInfoTxt) errorMessageRegexes
where
unwrapMatch :: (Text, Text, Text, [Text]) -> Maybe (ExpectedSig, ActualSig, DeclName)
-- due to using (.|\n) in regex we have to drop the erroneous, but necessary ("." doesn't match newlines), match
unwrapMatch (_, _, _, [expect, actual, _, name]) = Just (expect, actual, name)
unwrapMatch _ = Nothing
unwrapMatch :: (Text, Text, Text, [Text]) -> Maybe DeclName
unwrapMatch (_, _, _, [name]) = Just name
unwrapMatch _ = Nothing

errInfoTxt = printOutputable errInfoContext

-- | List of regexes that match various Error Messages
errorMessageRegexes :: [Text]
errorMessageRegexes = [ -- be sure to add new Error Messages Regexes at the bottom to not fail any existing tests
"Expected type: (.+)\n +Actual type: (.+)\n(.|\n)+In an equation for ‘(.+)’"
, "Couldn't match expected type ‘(.+)’ with actual type ‘(.+)’\n(.|\n)+In an equation for ‘(.+)’"
-- GHC >9.2 version of the first error regex
, "Expected: (.+)\n +Actual: (.+)\n(.|\n)+In an equation for ‘(.+)’"
"In an equation for ‘(.+)’:"
]

-- | Given a String with the name of a declaration, GHC's "Expected Type", find the declaration that matches
@@ -147,7 +228,7 @@ changeSigToCodeAction :: PluginId -> Uri -> ChangeSignature -> Command |? CodeAc
changeSigToCodeAction (PluginId changeTypeSignatureId) uri ChangeSignature{..} =
InR CodeAction { _title = mkChangeSigTitle declName actualType
, _kind = Just (CodeActionKind_Custom ("quickfix." <> changeTypeSignatureId))
, _diagnostics = Just [diagnostic]
, _diagnostics = Just [diagnostic ^. fdLspDiagnosticL ]
, _isPreferred = Nothing
, _disabled = Nothing
, _edit = Just $ mkChangeSigEdit uri declSrcSpan (mkNewSignature declName actualType)
Loading

Unchanged files with check annotations Beta

{ source_version = ver
, old_value = m_old
, get_file_version = use GetModificationTime_{missingFileDiagnostics = False}
, get_linkable_hashes = \fs -> map (snd . fromJust . hirCoreFp) <$> uses_ GetModIface fs

Check warning on line 805 in ghcide/src/Development/IDE/Core/Rules.hs

GitHub Actions / Hlint check run

Suggestion in getModIfaceFromDiskRule in module Development.IDE.Core.Rules: Use fmap ▫︎ Found: "\\ fs -> map (snd . fromJust . hirCoreFp) <$> uses_ GetModIface fs" ▫︎ Perhaps: "fmap (map (snd . fromJust . hirCoreFp)) . uses_ GetModIface"
, get_module_graph = useWithSeparateFingerprintRule_ GetModuleGraphTransDepsFingerprints GetModuleGraph f
, regenerate = regenerateHiFile session f ms
}
Just fileDiags -> do
pure $ Just $ filter diagRangeOverlaps fileDiags
where
diagRangeOverlaps = \fileDiag ->

Check warning on line 219 in ghcide/src/Development/IDE/Core/PluginUtils.hs

GitHub Actions / Hlint check run

Warning in activeDiagnosticsInRangeMT in module Development.IDE.Core.PluginUtils: Redundant lambda ▫︎ Found: "diagRangeOverlaps\n = \\ fileDiag\n -> rangesOverlap range (fileDiag ^. fdLspDiagnosticL . LSP.range)" ▫︎ Perhaps: "diagRangeOverlaps fileDiag\n = rangesOverlap range (fileDiag ^. fdLspDiagnosticL . LSP.range)"
rangesOverlap range (fileDiag ^. fdLspDiagnosticL . LSP.range)
-- | Just like 'activeDiagnosticsInRangeMT'. See the docs of 'activeDiagnosticsInRangeMT' for details.
import Data.Time (UTCTime (..))
import Data.Tuple.Extra (dupe)
import Debug.Trace
import Development.IDE.Core.FileStore (resetInterfaceStore)

Check warning on line 73 in ghcide/src/Development/IDE/Core/Compile.hs

GitHub Actions / Hlint check run

Warning in module Development.IDE.Core.Compile: Use fewer imports ▫︎ Found: "import Development.IDE.Core.FileStore ( resetInterfaceStore )\nimport Development.IDE.Core.FileStore ( shareFilePath )\n" ▫︎ Perhaps: "import Development.IDE.Core.FileStore\n ( resetInterfaceStore, shareFilePath )\n"
import Development.IDE.Core.Preprocessor
import Development.IDE.Core.ProgressReporting (progressUpdate)
import Development.IDE.Core.RuleTypes
tcs = tcg_tcs ts :: [TyCon]
hie_asts = GHC.enrichHie all_binds (tmrRenamed tcm) top_ev_binds insts tcs
pure $ Just $

Check warning on line 826 in ghcide/src/Development/IDE/Core/Compile.hs

GitHub Actions / Hlint check run

Suggestion in generateHieAsts in module Development.IDE.Core.Compile: Redundant $ ▫︎ Found: "Just $ hie_asts" ▫︎ Perhaps: "Just hie_asts"
#if MIN_VERSION_ghc(9,11,0)
hie_asts (tcg_type_env ts)
#else
convImport (L _ i) = (
(ideclPkgQual i)

Check warning on line 1106 in ghcide/src/Development/IDE/Core/Compile.hs

GitHub Actions / Hlint check run

Suggestion in getModSummaryFromImports in module Development.IDE.Core.Compile: Redundant bracket ▫︎ Found: "((ideclPkgQual i), reLoc $ ideclName i)" ▫︎ Perhaps: "(ideclPkgQual i, reLoc $ ideclName i)"
, reLoc $ ideclName i)
msrImports = implicit_imports ++ imps
{-# LANGUAGE DeriveAnyClass #-}

Check warning on line 1 in ghcide/session-loader/Development/IDE/Session/Diagnostics.hs

GitHub Actions / Hlint check run

Warning in module Development.IDE.Session.Diagnostics: Use module export list ▫︎ Found: "module Development.IDE.Session.Diagnostics where" ▫︎ Perhaps: "module Development.IDE.Session.Diagnostics (\n module Development.IDE.Session.Diagnostics\n ) where" ▫︎ Note: an explicit list is usually better
module Development.IDE.Session.Diagnostics where
import Control.Applicative
surround start s end = do
guard (listToMaybe s == Just start)
guard (listToMaybe (reverse s) == Just end)
pure $ drop 1 $ take (length s - 1) s

Check warning on line 92 in ghcide/session-loader/Development/IDE/Session/Diagnostics.hs

GitHub Actions / Hlint check run

Warning in parseMultiCradleErr in module Development.IDE.Session.Diagnostics: Use drop1 ▫︎ Found: "drop 1" ▫︎ Perhaps: "drop1"
multiCradleErrMessage :: MultiCradleErr -> [String]
multiCradleErrMessage e =
[] -> error $ "GHC version could not be parsed: " <> version
((runTime, _):_)
| compileTime == runTime -> do
atomicModifyIORef' cradle_files (\xs -> (cfp:xs,()))

Check warning on line 630 in ghcide/session-loader/Development/IDE/Session.hs

GitHub Actions / Hlint check run

Warning in loadSessionWithOptions in module Development.IDE.Session: Use atomicModifyIORef'_ ▫︎ Found: "atomicModifyIORef' cradle_files (\\ xs -> (cfp : xs, ()))" ▫︎ Perhaps: "atomicModifyIORef'_ cradle_files ((:) cfp)"
session (hieYaml, toNormalizedFilePath' cfp, opts, libDir)
| otherwise -> return (([renderPackageSetupException cfp GhcVersionMismatch{..}], Nothing),[])
-- Failure case, either a cradle error or the none cradle
x <- map errMsgDiagnostic closure_errs
DriverHomePackagesNotClosed us <- pure x
pure us
isBad ci = (homeUnitId_ (componentDynFlags ci)) `OS.member` bad_units

Check warning on line 897 in ghcide/session-loader/Development/IDE/Session.hs

GitHub Actions / Hlint check run

Suggestion in newComponentCache in module Development.IDE.Session: Redundant bracket ▫︎ Found: "(homeUnitId_ (componentDynFlags ci)) `OS.member` bad_units" ▫︎ Perhaps: "homeUnitId_ (componentDynFlags ci) `OS.member` bad_units"
-- Whenever we spin up a session on Linux, dynamically load libm.so.6
-- in. We need this in case the binary is statically linked, in which
-- case the interactive session will fail when trying to load
{-# LANGUAGE CPP #-}

Check warning on line 1 in exe/Wrapper.hs

GitHub Actions / Hlint check run

Warning in module Main: Use module export list ▫︎ Found: "module Main where" ▫︎ Perhaps: "module Main (\n module Main\n ) where" ▫︎ Note: an explicit list is usually better
{-# LANGUAGE DataKinds #-}
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE OverloadedStrings #-}