fslang-suggestions icon indicating copy to clipboard operation
fslang-suggestions copied to clipboard

on by default warning for partially applied function piped into `ignore`

Open smoothdeveloper opened this issue 2 years ago • 5 comments

In scenarios where the result of a function is discarded through ignore, and the function is modified to take an extra parameter, it is very difficult to get useful type checking.

let myFunction a b c = 
  printfn $"{a} + {b} + {c}"
  a + b + c

myFunction 1 2 |> ignore

I suggest the above code raises an on-by-default warning:

this expression is a partially applied function, if you intend to ignore it, add an explicit type argument to the ignore call, if you intend the function to be called, add the missing arguments.

to fix the warning, if I intend to ignore the partially applied function:

myFunction 1 2 |> ignore<int -> int>

The warning would not go away by using a whole discard:

myFunction 1 2 |> ignore<_> // same warning
myFunction 1 2 |> ignore<_ -> _> // this is ok

The existing way of approaching this problem in F# is

[<RequiresExplicitTypeArguments>]
let inline ignore<'a> = ignore<'a>

but it is possible to circumvent with ignore<_>.

Pros and Cons

The advantages of making this adjustment to F# are making refactor of functions safer by default.

The disadvantages of making this adjustment to F# are

  • a breaking change (if we make the warning on by default)
  • making ignore a special case function.
  • suited for analyzer rather than incurring perf hit at compile time

Extra information

**Estimated cost (XS, S, M, L, XL, XXL): S

Affidavit (please submit!)

Please tick these items by placing a cross in the box:

  • [x] This is not a question (e.g. like one you might ask on StackOverflow) and I have searched StackOverflow for discussions of this issue
  • [x] This is a language change and not purely a tooling change (e.g. compiler bug, editor support, warning/error messages, new warning, non-breaking optimisation) belonging to the compiler and tooling repository
  • [x] This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it
  • [x] I have searched both open and closed suggestions on this site and believe this is not a duplicate

Please tick all that apply:

  • [x] This is not a breaking change to the F# language design
  • [x] I or my company would be willing to help implement and/or test this

smoothdeveloper avatar Oct 08 '23 17:10 smoothdeveloper

Probably should be an analyzer

vzarytovskii avatar Oct 09 '23 13:10 vzarytovskii

Some arguments against this:

  • It's probabilistic - a function could do something and return a function which is ignored. Unless there is a special way, beyond the type signature, of detecting a function which does nothing and returns a function.
  • The RequiresExplicitTypeArguments behaviour of allowing <_> seems like a bug (or if not an incompleteness requiring some other attrubute like RequiresHighlyExplicitTypeArguments) and if this is fixed there will be a type-signature based approach to this. RequireExplicitTypeArguments and ignore would go particularly well together.
  • The use case is functional kool-aid. Where people write curried functions just because it's kool. There is no natural "most-varying" input and instead all the function inputs are intended to be given, so there is no intention to actually curry them and when this is done it's a mistake as in this issue.

charlesroddie avatar Oct 10 '23 22:10 charlesroddie

It's probabilistic - a function could do something and return a function which is ignored. Unless there is a special way, beyond the type signature, of detecting a function which does nothing and returns a function.

Would it be ok to say, putting ignore<_ -> _> (adjusted to correct arrity, going generic on arity here seems like risky business, in practice) is a low price to pay for making this edge case a bit explicit in such code?

This contention feels to me like the edge case of purposedly ignoring a returned function, through calling a function for it's side effect: edge case².

The use case is functional kool-aid. Where people write curried functions just because it's kool.

https://github.com/smoothdeveloper/visualfsharp/commit/136a875e50420990b994432a3af6adee1050041f the compiler codebase "drowned in the kool aid" already, we can't backtrack on this. Using this terminology sounds dogmatic.

I spent a bit of time to eventually come to see and fix the error. I faced the same in the past, and it didn't had to do with "kool aid", it had to do with a hiccup one may face dealing with F# code and partial application in non trivial refactorings.

The edge case you mention above is also "kool-aid"y, if we take it like this, if kool aid is partial application, you shouldn't encourage returning partially applied function just for side effect of the function which returns such value.

The RequiresExplicitTypeArguments behaviour of allowing <_> seems like a bug (or if not an incompleteness requiring some other attrubute like RequiresHighlyExplicitTypeArguments)

the doc for the attribute:

Adding this attribute to a type, value or member requires that uses of the construct must explicitly instantiate any generic type parameters.

To me, once the brackets are there, with the right arity, the type parameters are instanciated (through type inference with the discard _).

For more completeness, about obscure attributes to become highly explicit, we can always make new suggestions 🙂.

smoothdeveloper avatar Oct 10 '23 23:10 smoothdeveloper

To me, once the brackets are there, with the right arity, the type parameters are instanciated (through type inference with the discard _).

For more completeness, about obscure attributes to become highly explicit, we can always make new suggestions 🙂. Would it be ok to say, putting ignore<_ -> _> (adjusted to correct arrity, going generic on arity here seems like risky business, in practice) is a low price to pay for making this edge case a bit explicit in such code?

I suppose anything that makes RequireExplicitTypeArguments go in the direction of requiring explicitness is good, and _ -> _ is more explicit than _.

smoothdeveloper/visualfsharp@136a875 the compiler codebase "drowned in the kool aid" already, we can't backtrack on this. Using this terminology sounds dogmatic.

The codebase is using all sorts of informality which leads to these dangers. ignore reduces the danger only a small amount. The code could, instead of using ignore, have set x to the output of the fully applied function and used it to output string x somewhere. The same problem would have happened.

It's a cultural problem, partly to do with currying where currying is not intended, and partly to do with only allowing the smallest commits so the codebase can't get improved.

Some work added isFsx to member x.ExecuteTestCase assemblyPath (deps: string[]) isFsx =. At that point it should have been realized that the original code is not suitable for currying (either because it's a member or because the deps is not going to have many uses across data for the same assemblyPath. As part of adding isFsx this should therefore be converted to a non-curried method, and any failure to have inFsx as an input by callers would be a compile error.

I spent a bit of time to eventually come to see and fix the error. I faced the same in the past, and it didn't had to do with "kool aid", it had to do with a hiccup one may face dealing with F# code and partial application in non trivial refactorings.

An example where it happens in a valid case would be useful. To me "adding an extra parameter" and "currying" would be extremely hard to find a realistic example for.

The edge case you mention above is also "kool-aid"y, if we take it like this, if kool aid is partial application, you shouldn't encourage returning partially applied function just for side effect of the function which returns such value.

I agree it's an edge case but could be valid. You could take an input, process it in some way, and use it to generate a useful function.

charlesroddie avatar Oct 11 '23 08:10 charlesroddie

An example where it happens in a valid case would be useful.

We have differing concept of valid, mine probably applies to more realworld codebase if you don't find the example I provided as valid.

At that point it should have been realized

Taking your stance, it should have been "realized" before, extending some code generally doesn't involve backtracking design decisions.

Overall, it is common practice to add arguments to functions or even curried methods, and usage of ignore of the return value of such call should raise a warning if the ignore call doesn't specify the function type with the correct arrity, at the very least.

@charlesroddie, if you feel it shouldn't, please downvote the issue, but as I read you, your subconscious probably want to upvote it, for sake of improving the practical things done with F# code, even if it fails to your idealistic standard of "validity" of code that such code can even exist.

A part of you wants to upvote it, and it needs to make the other part of you to not cling to ideals that won't apply to most of real world F# code anyways :)

smoothdeveloper avatar Oct 27 '23 16:10 smoothdeveloper