Additional code actions/quick fixes by friedbrice · Pull Request #164 · josephsumabat/static-ls

friedbrice · 2025-07-06T15:44:48Z

This PR introduces six new code actions/quick fixes:

Add required extension
Insert associated type
Insert cases
Insert fields
Insert missing methods
Use valid hole fit

These code actions work by parsing relevant information from GHC error messages.

* Add required extension * Insert associated type * Insert cases * Insert fields * Insert missing methods * Use valid hole fit

friedbrice · 2025-07-06T18:25:05Z

src/StaticLS/HIE/File.hs

-getHieFileFromPath :: (HasStaticEnv m, HasLogger m, MonadIO m, HasLogger m) => AbsPath -> MaybeT m HieFile
+getHieFileFromPath :: (HasStaticEnv m, HasLogger m, MonadIO m) => AbsPath -> MaybeT m HieFile


Drive-by improvement.

friedbrice · 2025-07-06T18:35:18Z

src/StaticLS/IDE/CodeActions.hs

+getCodeActions ::
+  LSP.TextDocumentIdentifier ->
+  LSP.CodeActionContext ->
+  AbsPath ->
+  LineCol ->
+  StaticLsM [LSP.CodeAction]
+getCodeActions tdi ctx path lineCol = do


My code actions were written to a TextDocumentIdentifier -> CodeActionContext -> StaticLsM [CodeAction] shape, whereas the others were written to conform to AbsPath -> LineCol -> StaticLsM [Assist]. I attempted to refactor mine to be more idiomatic with the existing code actions, but I ran into significant difficulty constructing Assist values when I needed to provide them a Data.Range.Range (from tree-sitter). A tree-sitter Range requires two Data.Pos.Pos (which is what I need to construct), each of which carries just a single Int representing a document as either a sequence of characters or a sequence of bytes---that part's not clear to me---whereas an LSP Position (which is what I have in scope) has line number and column. I doubt there even is a way to convert between the two kinds of positions, considering platform differences such as newline character sequences and such.

Since I didn't have the information at hand to construct a tree-sitter Pos, I changed codeActions to take the info I needed to construct my CodeActions, and pushed assistToCodeAction into the body of codeActions so that it returns CodeAction instead of Assist.

friedbrice · 2025-07-06T18:36:15Z

src/StaticLS/IDE/CodeActions.hs

+  let issues = S.fromList (mapMaybe Parse.actionableIssue ctx._diagnostics)
+  issueActions <- join <$> traverse (issueToActions tdi) (S.toList issues)


These are the new code actions. See issueToActions and the type ActionableIssue.

friedbrice · 2025-07-06T18:38:44Z

src/StaticLS/IDE/CodeActions.hs

+  let issues = S.fromList (mapMaybe Parse.actionableIssue ctx._diagnostics)
+  issueActions <- join <$> traverse (issueToActions tdi) (S.toList issues)
+  assistActions <-
+    traverse assistToCodeAction $


See discussion above for rationale on moving assistToCodeAction into the body of codeAction.

friedbrice · 2025-07-06T18:40:04Z

src/StaticLS/IDE/CodeActions/AddRequiredExtension.hs

+  LSP.Diagnostic ->
+  Parse.KnownExtension ->
+  LSP.CodeAction
+addRequiredExtension tdi diag (Parse.KnownExtension _ text) =


If GHC says "Perhaps you intended to enable TypeApplications" or something like that, this function will create a quick fix for it.

friedbrice · 2025-07-06T18:41:14Z

src/StaticLS/IDE/CodeActions/InsertAssociatedType.hs

+codeAction = insertAssociatedType
+
+insertAssociatedType :: LSP.TextDocumentIdentifier -> LSP.Diagnostic -> T.Text -> LSP.CodeAction
+insertAssociatedType tdi diag ty =


This code action is pretty superficial. Maybe we should remove it.

friedbrice · 2025-07-06T18:41:58Z

src/StaticLS/IDE/CodeActions/InsertCases.hs

+codeAction = insertCases
+
+insertCases :: LSP.TextDocumentIdentifier -> LSP.Diagnostic -> [T.Text] -> LSP.CodeAction
+insertCases tdi diag pats =


If a GHC error (or warning) tells you which cases are missing, this function will create a quick fix that will insert the missing cases.

friedbrice · 2025-07-06T18:42:57Z

src/StaticLS/IDE/CodeActions/InsertFields.hs

+  Maybe T.Text ->
+  [T.Text] ->
+  LSP.CodeAction
+insertFields tdi diag ctor existingFields missingFields =


If a GHC error (or warning) tells you which fields are missing, this code action will create a quick fix that will insert the missing fields for you. Works with both kinds of missing-fields error messages (i.e. the one for strict fields and the one for non-strict fields).

friedbrice · 2025-07-06T18:44:06Z

src/StaticLS/IDE/CodeActions/InsertMissingMethods.hs

+codeAction = insertMissingMethods
+
+insertMissingMethods :: LSP.TextDocumentIdentifier -> LSP.Diagnostic -> [T.Text] -> LSP.CodeAction
+insertMissingMethods tdi diag methods =


If a GHC error (or warning) says your class instance is missing methods, this function will create a quick fix that will insert the missing methods for you.

friedbrice · 2025-07-06T18:44:40Z

src/StaticLS/IDE/CodeActions/Parse.hs

+instance Eq (Ignored a) where (==) _ _ = True
+instance Ord (Ignored a) where compare _ _ = EQ
+
+data KnownExtension = KnownExtension {-# UNPACK #-} !Extension {-# UNPACK #-} !T.Text


I guess I should remove those pragmas. I don't remember why I even had them in there in the first place.

friedbrice · 2025-07-06T18:45:45Z

src/StaticLS/IDE/CodeActions/Parse.hs

+{-# NOINLINE knownExtensions #-}
+knownExtensions :: [KnownExtension]
+knownExtensions = (\ext -> KnownExtension ext $ T.pack $ show ext) <$> [minBound .. maxBound]


No-inline because we only want to allocate this once, and refer to it again and again during execution. That said, it's probably not a huge deal to rebuild it every time. It's at most (what?) 40 or so items.

friedbrice · 2025-07-06T18:47:02Z

src/StaticLS/IDE/CodeActions/Parse.hs

+knownExtensions :: [KnownExtension]
+knownExtensions = (\ext -> KnownExtension ext $ T.pack $ show ext) <$> [minBound .. maxBound]
+
+data ActionableIssue


An ActionableIssue is a defunctionalization of a method of creating one or more quick fixes. They are constructed by parsing GHC diagnostic messages, extracting the info relevant for making text edits in the relevant source file.

friedbrice · 2025-07-06T18:49:23Z

src/StaticLS/IDE/CodeActions/Parse.hs

+actionableIssue :: LSP.Diagnostic -> Maybe ActionableIssue
+actionableIssue diag =


The parsing code in this module follows similar code in HLS. They literally do the same asinine thing for their quick fixes. That is, instead of using structured data that GHC would hypothetically provided, they parse the human-readable error messages. Go Haskell!

friedbrice · 2025-07-06T18:50:31Z

src/StaticLS/IDE/CodeActions/UseValidHoleFit.hs

+codeAction = useValidHoleFit
+
+useValidHoleFit :: LSP.TextDocumentIdentifier -> LSP.Diagnostic -> T.Text -> LSP.CodeAction
+useValidHoleFit tdi diag sym =


When GHC suggests valid hole fits, this function will create quick fixes for each such hole fit. (This one is the million-dollar feature.)

friedbrice

I've annotated the PR with context intended to assist the reviewer.

josephsumabat · 2025-07-14T13:19:20Z

Thanks for the PR! I'll take a look some time this week/test it out :)

josephsumabat · 2025-07-18T23:32:04Z

Sorry I haven't gotten to this yet - I was a bit sick earlier this week so have been catching up on things but I will get this merged when I have the chance!

josephsumabat

Thank you for submitting this, these are some really nice features! I think the PR is pretty close to being merge-able but after testing I ran into a few issues

Some of the code actions don't actually show up because of the parsing for me for one reason or another - e.g. the _message field of diagnostics actually has the bars stripped on my machine e.g.

_message = "Pattern match(es) are non-exhaustive\nIn a case alternative:\n    Patterns of type \8216A _\nB \n C\n        ..."

This causes the messages relying on bars to parse to fail. I'm not sure if this behavior is platform dependent though.

Changing the nonExhaustivePatterns locally to not use the bar seemed to make it work for me.

The actual action doesn't insert anything (likely related to me not fully fixing the parsing)
I suspect that this issue

I would like 2 main changes to the PR before I merge

Fixing the functionality - if this is local to my machine we can try to figure this out together
I would like at the very least a parsing test suite for the actionable messages - This can be golden tests or just unit tests - the main thing I want is something where I can paste a diagnostic and easily see if it matches its expected output.

it's possible that the error messages change across ghc versions or flags and it would be much easier to see why something isn't working and iterate with such a suite

Additional code actions/quick fixes...

c463bc7

* Add required extension * Insert associated type * Insert cases * Insert fields * Insert missing methods * Use valid hole fit

friedbrice requested a review from josephsumabat July 6, 2025 15:44

Run code formatter

7c21118

friedbrice commented Jul 6, 2025

View reviewed changes

josephsumabat requested changes Jul 19, 2025

View reviewed changes

		getHieFileFromPath :: (HasStaticEnv m, HasLogger m, MonadIO m, HasLogger m) => AbsPath -> MaybeT m HieFile
		getHieFileFromPath :: (HasStaticEnv m, HasLogger m, MonadIO m) => AbsPath -> MaybeT m HieFile

		let issues = S.fromList (mapMaybe Parse.actionableIssue ctx._diagnostics)
		issueActions <- join <$> traverse (issueToActions tdi) (S.toList issues)

		actionableIssue :: LSP.Diagnostic -> Maybe ActionableIssue
		actionableIssue diag =

Conversation

friedbrice commented Jul 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

friedbrice Jul 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

friedbrice Jul 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

friedbrice Jul 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

friedbrice Jul 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

friedbrice left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josephsumabat commented Jul 14, 2025

Uh oh!

josephsumabat commented Jul 18, 2025

Uh oh!

josephsumabat left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

friedbrice Jul 6, 2025 •

edited

Loading

friedbrice Jul 6, 2025 •

edited

Loading

friedbrice Jul 6, 2025 •

edited

Loading

friedbrice Jul 6, 2025 •

edited

Loading

friedbrice left a comment •

edited

Loading

josephsumabat left a comment •

edited

Loading