haskell / haskell-language-server

Official haskell ide support via language server (LSP). Successor of ghcide & haskell-ide-engine.
Apache License 2.0
2.61k stars 351 forks source link

StylishHaskell shifting the working dir, this could lead to race condition #4234

Open soulomoon opened 1 month ago

soulomoon commented 1 month ago

StylishHaskell shifting the working dir, it could make other part of hls depending on cwd result in error in some race condition.

https://github.com/haskell/haskell-language-server/blob/61fd5c464842448dfebe91d3e01de425eacf07b7/plugins/hls-stylish-haskell-plugin/src/Ide/Plugin/StylishHaskell.hs#L81

Such as Hlint, Stan, Template haskel, session loader etc. We should not shifting the working dir after initilization.

soulomoon commented 1 month ago

4231 Could alleviate the some potential race condition. But there are still a lot of place we could not clean up

soulomoon commented 1 month ago

I am wondering if there is an alternaive function than loadConfig that does not depend on cwd to load the stylish config.

michaelpj commented 1 month ago

We could ask upstream to expose a version that takes the directory as an argument.

soulomoon commented 1 month ago

Yep, I've craete an issue there

fendor commented 5 days ago

This is quite bad :sweat_smile: Perhaps we can copy-paste the loadConfig code from stylish-haskell and fix it ourselves until upstream fixed it?

jhrcek commented 5 days ago

Seems like stylish's loadConfig already supports specifying directory as argument (Maybe FilePath). We just need to pass Just instead of Nothing, or am I missing something?

jhrcek commented 5 days ago

Ok, the Maybe FilePath is the path to the stylish config file itselt, not to a "starting directory" as I imagined. So this will require some upstream changes after all :disappointed: