ndmitchell / hlint

Haskell source code suggestions
Other
1.46k stars 195 forks source link

Avoid isJust/fromJust #156

Open ndmitchell opened 9 years ago

ndmitchell commented 9 years ago

From https://code.google.com/p/ndmitchell/issues/detail?id=238 by @np:

In most situations code like:

if null lst then ... else ... head l ... tail l ...

could be replaced by a case

case lst of [] -> ... ; x : xs -> ... x ... xs ...

Same thing with isJust + fromJust, but maybe could be suggested instead of a case.

@ndmitchell replies:

if isNothing x then 0 else fromJust x + 3

Should suggest:

maybe 0 (+3) x

This relies on having a nice looking functional in the else. Alternatively could have patterns with context, i.e.

if isNothing x then y else _f x ==> maybe x _f x

where _f is the lambda that needs inserting to get it working.

amigalemming commented 8 years ago

I have the line

warn "fromJust" = fromJust ==> fromMaybe (error "location of call")

in my local hlint config. I prefer total functions and thus would always replace fromJust by a complete case analysis.

amigalemming commented 8 years ago

maybe 0 (+3) x = Foldable.sum $ fmap (+3) x

ndmitchell commented 8 years ago

I prefer total programs, and would generally rather the compiler proved that for me, rather than trying to approximate it with complete case analysis - but I appreciate many people do go that route. The fromJust to fromMaybe pattern is only useful for implementations of Haskell which don't have stack handling, which I think the latest GHC will have (at least for fromJust).

I see the Foldable.sum version as more complex than the maybe, and also more special purpose, since it only works for numerical operations. Was that just one example, or a suggestion of a hint?

ysangkok commented 2 years ago

Just found a case of isJust $ a <$> b and I was wondering whether HLint had an option for that. I guess that isJust . fmap is a special case of what is being suggested here.

ndmitchell commented 2 years ago

@ysangkok - I don't think that's a special case of this - there actually already is a hint for what you want - https://github.com/ndmitchell/hlint/blob/master/data/hlint.yaml#L623 - just no variant with <$>. I'd happily accept a patch to add that.