dotnet/fsharp

FS-1093 op_Implicit 3395 warnings missing for constructors and potentially inconsistent

Opened this issue · 4 comments

Take the following example. Compiled with <OtherFlags>$(OtherFlags) --warnon:3395</OtherFlags> it produces no warnings on the constructor.

open System

// Minimized copy for demonstration of Microsoft.Extensions.Primitives.StringValues
type StringValues(v: string) =
    static member op_Implicit(values: StringValues): string = ""
    
type Foo(_v: Nullable<int>) =
    member val Test: Nullable<int> = 0 with get,set // Warning for 3391 as I would have interpreted it.
    member val StringValue: string = "" with get,set
    
    static member Create() =
      let value = 1
      let instance = Foo(value, Test = value) // No warning on either the argument or the property setter?
      instance.Test <- value // No warning for 3395 and 3391?
      instance.StringValue <- StringValues("1") // Warning for 3395
      
      let mutable test: Nullable<int> = value // Warning for 3391 as I would have interpreted it.
      
      // Warning for 3391 but it's syntactically identical to the line that had no warning and one that had 3395???
      test <- 1 

I was trying to add these warning yesterday and ran into this. The number has changed to 3395!

I fixed the RFC here: fsharp/fslang-design#686

I also couldn't find documentation on the xml syntax for --warnon but <WarnOn>FS3389; FS3395</WarnOn> seems to work.

@charlesroddie thanks! I'll update the issue as I still think it's somewhat inconsistent when it triggers for surface syntax.

As for the syntax you probably want <WarnOn>$(WarnOn); 3395</WarnOn> or old style <OtherFlags>$(OtherFlags) --warnon:3395</OtherFlags>

For a bit of an explainer why you'd want that syntax: Charles' version overwrites those properties, so any modifications by any other part of the build would be lost. Nino's version persists the old value of the property, making the change additive overall.

I also didn't get the FS prefixed versions to work, additionally I just created this issue over at JetBrains/resharper-fsharp#379