dotnet / dotnet/fsharp

FS3388 (implicit conversion warning) rarely gets triggered when it should, using `--warnon: 3388`

Open
#14,163 5 comments 5 reactions 1 assignee Claimed by @abelbraaksma View on GitHub
Area-Diagnostics Feature Request
Dominant language
F#
Stars
4.3k
Forks
876
Avg merge
4d 22h
Merged PRs (30d)
144

Description

Part of F# 6.0 was [more leniency towards implicit conversions](https://github.com/fsharp/fslang-design/blob/main/FSharp-6.0/FS-1093-additional-conversions.md#warnings-for-type-directed-conversions).

While there have been arguments both in favor and against it, the workaround for people wanting the "old behavior", warning FS3388, along with a few others, was introduced. Explicitly, it states:

> `FS3388`: Type-directed conversion by subtyping (e.g. `string --> obj`). This warning is OFF by default.

However, even after adding `warnon: 3388` to the compiler options, this warning is rarely raised, among which some of the most critical scenarios.

## Repro steps

### Example 1: boxing should always trigger this warning

The following code does not trigger the warning, whether on or off:

```f#
let a: obj list = [1; 2] // boxing, should always warn, or raise, at the very least trigger FS3388
let b: obj list = ["a"; "b"] // cast, should trigger FS3388
```

### Example 2: previously failing-to-compile code should now warn

This code comes from [this StackOverflow question, which, at the time, did not compile](https://stackoverflow.com/questions/13985733/fsharp-and-upcasting-to-interfaces-seems-redundant), because an upcast was necessary.

Code that didn't compile previously, should, imo, always raise this warning currently.

```f#
type A() =
member x.A = "A"

type B() =
inherit A()
member x.B = "B"

let f (g: _ -> A) = g ()

let a = f (fun () -> A()) // works
let b = f (fun () -> B()) // should raise FS3388, but doesn't. Used to fail on older compilers
```

### Example 3: most notorious scenarios with `Task`

It has happened to @natalie-o-perret recently, and last week to myself as well: accidentally returning a nested task, where the implementation expects a `Task`, not a `Task<'T>`, and a nested task then _never gets executed_. This can lead to _very_ subtle bugs, and F# can, and I believe, should help with this.

This now gets auto-upcast, and _does not trigger FS3388_ when it's enabled:

```f#
type IFoo =
abstract member Execute: unit -> Task

type Foo() =
interface IFoo with
member x.Execute() =
let doSomething () = task { return () }
task { return doSomething () } // should trigger FS3388, as Task> gets upcast to Task
```

## Expected behavior

We introduced the new warning, so I'd expect the warning to be triggered. I should note that I tested this with different warning levels, and all with `--warnon: 3388`.

## Actual behavior

In many cases the warning is not raised.

## Known workarounds

None. Unless you code differently. For instance, take the last example. If you code it with an extra assignment, the warning _does trigger_. However, the whole point of the warning is to warn _against_ such inadvertent uses.

This triggers the warning correctly:

```f#
type IFoo =
abstract member Execute: unit -> Task

type Foo() =
interface IFoo with
member x.Execute() =
let doSomething () = task { return () }
let x = task { return doSomething () }
x // due to the extra assignment, warning FS3388 is triggered
```

This also triggers the warning correctly:

```f#
let b: obj = "test" // correctly triggers FS3388
```

## Related information

Tested both on F# 6.0 and latest F# 7.0 Preview (thanks @gusty!).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.