Skip to content
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

How am I supposed to fix STAN-0103 diagnostic? #550

Open
Bodigrim opened this issue Dec 19, 2023 · 5 comments
Open

How am I supposed to fix STAN-0103 diagnostic? #550

Bodigrim opened this issue Dec 19, 2023 · 5 comments

Comments

@Bodigrim
Copy link

I have a call of length xs in the middle of a function, which Stan is unhappy with because of STAN-0103: "Usage of the length function that hangs on infinite lists".

How am I supposed to act on this? Stan offers two suggestions:

  • "Don't use length if you expect your function to work with infinite lists" - nope, I bloody well need the length of the list.
  • "Use the slist library for fast and safe functions on infinite lists" - but it does not seem that Slist.size (Slist.slist xs) is any safer than length xs in case if xs happens to be infinite! Am I supposed to make a non-local refactoring, probably migrate the entire module to Slist? This is likely to be unfeasible or prohibitively expensive in the majority of real-world cases.

I personally do not think that STAN-0103 is worth to complain about, but that's fine, we can agree to disagree. I don't mind opinionated diagnostics, I do mind unactionable ones: if a policy mandates to maintain a clean Stan report, it's virtually impossible to do so.

@tomjaguarpaw
Copy link
Collaborator

tomjaguarpaw commented Dec 20, 2023

As far as I can tell the intention of this diagnostic is to encourage users never to use length on a list. That's also consistent with the Kowainik ecosystem which uses slist pervasively, including in stan itself. If it was only suggesting a non-local transformation then it would be debatable whether that is too much for a linting suggestion (cf. suggesting rewriting to NonEmpty if you attempt to use head). However, I'm skeptical whether slist is much safer. For example, concat on Slist has the same problem as length on lists.

stan is configurable so anyone who doesn't like this particular diagnostic can disable it for their codebase. I don't know whether it's it should be disabled by default but would be interested in hearing the opinion of other stan users.

(N.B. I'm a maintainer of the Kowainik ecosystem in the sense that I volunteer to keep it building as newer GHCs are released. I didn't author any of the ecosystem and I don't have much spare bandwidth to make any changes beyond keeping it building. I'm not sure if I have moral authority to change the defaults of stan. I'd have to check with Veronika if it comes to that.)

@Bodigrim
Copy link
Author

stan is configurable so anyone who doesn't like this particular diagnostic can disable it for their codebase. I don't know whether it's it should be disabled by default but would be interested in hearing the opinion of other stan users.

IMO the entire STAN-010X range, prohibiting reverse, isSuffixOf, length, genericLength, sum and product, is not a good default.

It's probably fine for applications to use slist extensively, but it's almost never an option for a library. E. g., in my case I have a function

foo :: FilePath -> IO ()
foo fn = if length fn > 255 then do_this else do_that

I cannot reasonably change the type of foo (clients expect to pass FilePath) and replacing length fn with Slist.size (Slist.slist xs) does not make anything safer.


The thing is that HLS 2.5.0.0 enabled hls-stan-plugin by default to all users. I can only imagine confusion of newcomers, who are writing their first HelloWorld.hs and suddenly told that their textbook length, reverse and sum are no good. The fact that HLS currently ignores .stan.toml (haskell/haskell-language-server#3904) makes it even worse.

If stan is to maintain its current defaults, I'll probably argue that hls-stan-plugin should be disabled by default. CC @0rphee.

@tomjaguarpaw
Copy link
Collaborator

If stan is to maintain its current defaults, I'll probably argue that hls-stan-plugin should be disabled by default. CC @0rphee.

Your overall preference regarding STAN-010X is a reasonable one, but if this issue is motivated by stan's presence in HLS then the discussion belongs on the HLS issue tracker. There's no reason that HLS's stan needs to have the same defaults as the command line tool (although perhaps this repo could help by incorporating HLS's default config file).

@noughtmare
Copy link

I have a function

foo :: FilePath -> IO ()
foo fn = if length fn > 255 then do_this else do_that

You can make it lazier like this:

foo :: FilePath -> IO ()
foo fn = if null (drop 255 fn) then do_that else do_this

@Bodigrim
Copy link
Author

That's a good point and it would be a much better suggestion, thanks.

I wonder if a lazy compareLength :: [a] -> Int -> Ordering would be a good addition to base. There is prior art of similar functions in bytestring and text.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants