Use -fno-show-error-context from GHC 9.8

[dsaenztagarro]

Resolves #4281

Before the change, with GHC 9.8.1, this is how error messages look in Visual Studio Code:

After the change, this is how it looks now:

Notes

• b67871e fixes following specs in this build

  See fixed specs

  

[michaelpj]

Looks great!

[michaelpj]

So it did work without changing the diagnostic printing options?

[michaelpj]

The failures look weird, I've restarted them just to see.

[dsaenztagarro]

So it did work without changing the diagnostic printing options?

@michaelpj exactly!, no need for changing diagnostic printing options.

It was completely my fault because I was using #if !MIN_VERSION_ghc(9,8,0) instead of #if MIN_VERSION_ghc(9,8,0), and so the changes were not applying, and the HLS code was not really recompiled.

[michaelpj]

Okay, so we have several problems.

Some of the tests are failing and they actually rely on matching against the error context! Argh! See https://github.com/haskell/haskell-language-server/blob/master/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs#L810

I think that actually doesn't need to match on the error context and can be rewritten to just ignore the extra bits.

I'm not sure what's going on with the hole message, we should investigate what if anything is different about the message that GHC gives us and see if we can adapt the regexes to avoid the problem: https://github.com/haskell/haskell-language-server/blob/master/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/Plugins/FillHole.hs#L14

[dsaenztagarro]

@michaelpj I debugged specifically the following failing spec regarding filling infix type hole:

cabal test hls-refactor-plugin-tests --test-option="-p /filling infix type hole uses prefix notation/"

And when looking to the relevant lines you suggested:

haskell-language-server/plugins/hls-refactor-plugin/src/Development/IDE/Plugin/Plugins/FillHole.hs

Lines 18 to 20 in 7b8e2e5

	let isInfixHole = _message =~ addBackticks holeName :: Bool in
	map (proposeHoleFit holeName False isInfixHole) holeFits
	++ map (proposeHoleFit holeName True isInfixHole) refFits

I have found, while debugging, that comparing _message GHC payload before/after the changes, after the changes, the following part, that is used specifically in the regex comparison, it is lost:

8226 In the expression: a1 `_` a2
  In an equation for 8216test8217: test a1 a2 = a1 `_` a2

Looking at the rest of the payload, I can't see how we can still evaluate whether it is an infix hole. See below the full _message payload before/after the changes.

Before the change

8226 Found hole: _ :: A -> A -> A
8226 In the expression: a1 `_` a2
  In an equation for 8216test8217: test a1 a2 = a1 `_` a2
8226 Relevant bindings include
    a2 :: A
      (bound at /private/var/folders/ln/tvkm8dnn5pd8gz7hcpv9y_7m0000gn/T/extra-dir-28739387754/Test.hs:6:9)
    a1 :: A
      (bound at /private/var/folders/ln/tvkm8dnn5pd8gz7hcpv9y_7m0000gn/T/extra-dir-28739387754/Test.hs:6:6)
    test :: A -> A -> A
      (bound at /private/var/folders/ln/tvkm8dnn5pd8gz7hcpv9y_7m0000gn/T/extra-dir-28739387754/Test.hs:6:1)
  Valid hole fits include
    test
    foo
    asTypeOf
    const
    pure
    return
    seq
  Valid refinement hole fits include
    curry _
    ($) _
    ($!) _
    const _
    flip _
    pure _
    return _
    id _
    head _
    last _
    (Some refinement holefits suppressed; use -fmax-refinement-hole-fits=N or -fno-max-refinement-hole-fits)

After the change

8226 Found hole: _ :: A -> A -> A
8226 Relevant bindings include
    a2 :: A
 (bound at /private/var/folders/ln/tvkm8dnn5pd8gz7hcpv9y_7m0000gn/T/extra-dir-17030982144/Test.hs:6:9)
    a1 :: A
      (bound at /private/var/folders/ln/tvkm8dnn5pd8gz7hcpv9y_7m0000gn/T/extra-dir-17030982144/Test.hs:6:6)
    test :: A -> A -> A
      (bound at /private/var/folders/ln/tvkm8dnn5pd8gz7hcpv9y_7m0000gn/T/extra-dir-17030982144/Test.hs:6:1)
  Valid hole fits include
    test
    foo
    asTypeOf
    const
    pure
    return
    seq
  Valid refinement hole fits include
    curry _
    ($) _
    ($!) _
    const _
    flip _
    pure _
    return _
    id _
    head _
    last _
    (Some refinement hole fits suppressed; use -fmax-refinement-hole-fits=N or -fno-max-refinement-hole-fits)

[dsaenztagarro]

Quick-update^tm: first of all thank you very much for all the support during the ZuriHac 2024,.. It would have been much harder to start being involved in the project without that initial push. Now, just some picks regarding the current state and my plans for the following weeks:

• I wrongly initially created a Pull request instead of a Draft request, so from now on, to avoid unnecessary noise to the Codeowners and the rest of maintainers, I will try to push changes only when reasonable sure that the changes will make it for different GHC versions locally.
  Update: I converted the PR this morning to Draft, but Codeowner may keep receiving notifications.
• I assume that I luck right now of the knowledge that I need to complete the job, so for now, I'm trying to get a deeper knowledge of ghc-exactprint and other libraries used in the project, so I have the tools to at least approach fixing the current failing specs with a good understanding of what I'm doing.
• Unless I get stack, I won't raise my hand, but I will leave a comment periodically to set the current status (let's say, once a month, unless you have any objection).
• If you consider this is urgent or needs to be managed faster, don't hesitate to ping me and I'm open to discuss all options.

[michaelpj]

Don't worry about pushing more, that's not a problem. And if you can point out where you're still getting stuck then maybe we can help!

[dsaenztagarro]

ℹ️ Outdated! see last comment

Quick update^tm: I got to fix all specs related to hls-refactor-plugin, but now hls-class-plugin is returning errors in the CI, and I don't have a clear a idea of the patch needed.

High level, I've gone through the plugin code, and for the moment I can't see the relation between setting the new session flag and the specs failing (I verified removing just that piece of code fixes the specs).

I've started with the following spec:

That spec is defined here, and following the code, I can see here that is calling to getAllCodeActions :: TextDocumentIdentifier -> Session [Command |? CodeAction]. Since this function seems to be defined in lsp-test package, looking into the docs, for comparison, I have run the spec while recording the LSP logs generated, using the following command:

LSP_TEST_LOG_MESSAGES=1 LSP_TEST_LOG_STDERR=1 cabal test hls-class-plugin-tests --test-options="-p \"Produces addMinimalMethodPlaceholders code actions for one instance\""

Logs generated:

• test_session_with_flag.log
• test_session_without_flag.log

Looking into the relevant parts, I can't see a difference in the initialize request or in the request associated to the getAllCodeActions call, despite of the different response:

See request/response for getAllCodeActions without flag

--> {
    "id": 1,
    "jsonrpc": "2.0",
    "method": "textDocument/codeAction",
    "params": {
        "context": {
            "diagnostics": [
                {
                    "code": "-Wmissing-methods",
                    "message": "⢠No explicit implementation for\n    either â==â or â/=â\n⢠In the instance declaration for âEq Xâ",
                    "range": {
                        "end": {
                            "character": 13,
                            "line": 4
                        },
                        "start": {
                            "character": 9,
                            "line": 4
                        }
                    },
                    "relatedInformation": [
                        {
                            "location": {
                                "range": {
                                    "end": {
                                        "character": 13,
                                        "line": 4
                                    },
                                    "start": {
                                        "character": 9,
                                        "line": 4
                                    }
                                },
                                "uri": "file:///Users/dsaenz/Code/haskell-language-server/plugins/hls-class-plugin/test/testdata/T1.hs"
                            },
                            "message": "TypeCheck"
                        }
                    ],
                    "severity": 2,
                    "source": "typecheck",
                    "tags": []
                }
            ]
        },
        "range": {
            "end": {
                "character": 13,
                "line": 4
            },
            "start": {
                "character": 9,
                "line": 4
            }
        },
        "textDocument": {
            "uri": "file:///Users/dsaenz/Code/haskell-language-server/plugins/hls-class-plugin/test/testdata/T1.hs"
        }
    }
}
<-- {
    "id": 1,
    "jsonrpc": "2.0",
    "result": [
        {
            "command": {
                "arguments": [
                    {
                        "methodGroup": [
                            [
                                "==",
                                "(==) :: X -> X -> Bool"
                            ]
                        ],
                        "range": {
                            "end": {
                                "character": 13,
                                "line": 4
                            },
                            "start": {
                                "character": 9,
                                "line": 4
                            }
                        },
                        "verTxtDocId": {
                            "uri": "file:///Users/dsaenz/Code/haskell-language-server/plugins/hls-class-plugin/test/testdata/T1.hs",
                            "version": 0
                        },
                        "withSig": false
                    }
                ],
                "command": "33088:class:classplugin.codeaction",
                "title": "Add placeholders for '=='"
            },
            "diagnostics": [],
            "kind": "quickfix",
            "title": "Add placeholders for '=='"
        },
        {
            "command": {
                "arguments": [
                    {
                        "methodGroup": [
                            [
                                "==",
                                "(==) :: X -> X -> Bool"
                            ]
                        ],
                        "range": {
                            "end": {
                                "character": 13,
                                "line": 4
                            },
                            "start": {
                                "character": 9,
                                "line": 4
                            }
                        },
                        "verTxtDocId": {
                            "uri": "file:///Users/dsaenz/Code/haskell-language-server/plugins/hls-class-plugin/test/testdata/T1.hs",
                            "version": 0
                        },
                        "withSig": true
                    }
                ],
                "command": "33088:class:classplugin.codeaction",
                "title": "Add placeholders for '==' with signature(s)"
            },
            "diagnostics": [],
            "kind": "quickfix",
            "title": "Add placeholders for '==' with signature(s)"
        },
        {
            "command": {
                "arguments": [
                    {
                        "methodGroup": [
                            [
                                "/=",
                                "(/=) :: X -> X -> Bool"
                            ]
                        ],
                        "range": {
                            "end": {
                                "character": 13,
                                "line": 4
                            },
                            "start": {
                                "character": 9,
                                "line": 4
                            }
                        },
                        "verTxtDocId": {
                            "uri": "file:///Users/dsaenz/Code/haskell-language-server/plugins/hls-class-plugin/test/testdata/T1.hs",
                            "version": 0
                        },
                        "withSig": false
                    }
                ],
                "command": "33088:class:classplugin.codeaction",
                "title": "Add placeholders for '/='"
            },
            "diagnostics": [],
            "kind": "quickfix",
            "title": "Add placeholders for '/='"
        },
        {
            "command": {
                "arguments": [
                    {
                        "methodGroup": [
                            [
                                "/=",
                                "(/=) :: X -> X -> Bool"
                            ]
                        ],
                        "range": {
                            "end": {
                                "character": 13,
                                "line": 4
                            },
                            "start": {
                                "character": 9,
                                "line": 4
                            }
                        },
                        "verTxtDocId": {
                            "uri": "file:///Users/dsaenz/Code/haskell-language-server/plugins/hls-class-plugin/test/testdata/T1.hs",
                            "version": 0
                        },
                        "withSig": true
                    }
                ],
                "command": "33088:class:classplugin.codeaction",
                "title": "Add placeholders for '/=' with signature(s)"
            },
            "diagnostics": [],
            "kind": "quickfix",
            "title": "Add placeholders for '/=' with signature(s)"
        },
        {
            "command": {
                "arguments": [
                    {
                        "methodGroup": [
                            [
                                "==",
                                "(==) :: X -> X -> Bool"
                            ],
                            [
                                "/=",
                                "(/=) :: X -> X -> Bool"
                            ]
                        ],
                        "range": {
                            "end": {
                                "character": 13,
                                "line": 4
                            },
                            "start": {
                                "character": 9,
                                "line": 4
                            }
                        },
                        "verTxtDocId": {
                            "uri": "file:///Users/dsaenz/Code/haskell-language-server/plugins/hls-class-plugin/test/testdata/T1.hs",
                            "version": 0
                        },
                        "withSig": false
                    }
                ],
                "command": "33088:class:classplugin.codeaction",
                "title": "Add placeholders for all missing methods"
            },
            "diagnostics": [],
            "kind": "quickfix",
            "title": "Add placeholders for all missing methods"
        },
        {
            "command": {
                "arguments": [
                    {
                        "methodGroup": [
                            [
                                "==",
                                "(==) :: X -> X -> Bool"
                            ],
                            [
                                "/=",
                                "(/=) :: X -> X -> Bool"
                            ]
                        ],
                        "range": {
                            "end": {
                                "character": 13,
                                "line": 4
                            },
                            "start": {
                                "character": 9,
                                "line": 4
                            }
                        },
                        "verTxtDocId": {
                            "uri": "file:///Users/dsaenz/Code/haskell-language-server/plugins/hls-class-plugin/test/testdata/T1.hs",
                            "version": 0
                        },
                        "withSig": true
                    }
                ],
                "command": "33088:class:classplugin.codeaction",
                "title": "Add placeholders for all missing methods with signature(s)"
            },
            "diagnostics": [],
            "kind": "quickfix",
            "title": "Add placeholders for all missing methods with signature(s)"
        }
    ]
}

See request/response for getAllCodeActions with flag

--> {
    "id": 1,
    "jsonrpc": "2.0",
    "method": "textDocument/codeAction",
    "params": {
        "context": {
            "diagnostics": [
                {
                    "code": "-Wmissing-methods",
                    "message": "No explicit implementation for\n  either â==â or â/=â",
                    "range": {
                        "end": {
                            "character": 13,
                            "line": 4
                        },
                        "start": {
                            "character": 9,
                            "line": 4
                        }
                    },
                    "relatedInformation": [
                        {
                            "location": {
                                "range": {
                                    "end": {
                                        "character": 13,
                                        "line": 4
                                    },
                                    "start": {
                                        "character": 9,
                                        "line": 4
                                    }
                                },
                                "uri": "file:///Users/dsaenz/Code/haskell-language-server/plugins/hls-class-plugin/test/testdata/T1.hs"
                            },
                            "message": "TypeCheck"
                        }
                    ],
                    "severity": 2,
                    "source": "typecheck",
                    "tags": []
                }
            ]
        },
        "range": {
            "end": {
                "character": 13,
                "line": 4
            },
            "start": {
                "character": 9,
                "line": 4
            }
        },
        "textDocument": {
            "uri": "file:///Users/dsaenz/Code/haskell-language-server/plugins/hls-class-plugin/test/testdata/T1.hs"
        }
    }
}
<-- {
    "id": 1,
    "jsonrpc": "2.0",
    "result": []
}

I will continue digging deeper into the failing specs, but of course, any kind of clue will be welcomed!

[dsaenztagarro]

ℹ️ Outdated! see last comment

Quick update^tm: ok, I guess the problem is here, with that ad-hoc diagnostics messages implementation, reviewing in detail 👀

[dsaenztagarro]

Quick update^tm: @michaelpj @fendor I've progressed until hls-change-type-signature-plugin, and this plugin is basically failing because with the new flag, the part for In an equation for ‘(.+)’ is not present anymore as part of the diagnostic message, and we need that matching group to get the declaration name to be updated.

haskell-language-server/plugins/hls-change-type-signature-plugin/src/Ide/Plugin/ChangeTypeSignature.hs

Lines 96 to 103 in 13e5795

	-- | 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 the following example used in the specs failing, without the new flag, you can see that the diagnostic message returns correctly the declaration name is fullSig.

See example

So, which should be the correct approach to get the declaration name now?