fsharp / fslang-suggestions

The place to make suggestions, discuss and vote on F# language and core library features
346 stars 21 forks source link

Warn when an auto-property getter is confused with a default value #549

Closed TMVector closed 2 years ago

TMVector commented 7 years ago

I propose the compiler should produce a warning when it is likely the programmer has confused the inline getter syntax for setting the intial value.

For example, consider the following class:

type TestType() =
  // Syntax of fields and immutable auto-properties:
  let field1 = 0
  member this.prop1 = 0 // Different behaviour to `let`
  member val prop2 = 0 // This acheives the same effect as `let` but for a property

  // This difference only really matters when the property type has mutable state:
  member this.dict2 = new Dictionary<int,string>() // Returns new instance every use
  member val dict = new Dictionary<int,string>() // Initialises property, as intended

An unexperienced fsharper (like me) could very easily confuse member _ with member val _, and the result will be very confusing behaviour. When the programmer attempts to add an entry to dict2, they will be frustrated to find nothing works.

I think the easiest/cleanest solution is for the compiler to issue a warning when this syntax is used, and the getter body is an object creation expression not of record or tuple type.

type Type1() =
  member this.dict = new Dictionary<int,string>() // WARN
  member this.dict2 = Dictionary<int,string>() // WARN

I tentatively propose the following warning message:

  member this.dict = new Dictionary<int,string>()
--^^^^^^^^^^^^^^^^
warning:
  Did you mean to provide a getter for 'dict'? This will return a new instance each use.
  If you are trying to initialise a property, use 'member val dict = ...' instead.

Pros and Cons

The advantages of making this adjustment to F# are:

The disadvantages of making this adjustment to F# are:

Extra informtion

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

Affadavit (must be submitted)

Please tick this by placing a cross in the box:

Please tick all that apply:

dsyme commented 7 years ago

I think the place for this is FSHarpLint. FSharpLint warnings are active by default in editors such as VSCode/Ionide so have a strong affect on the usage of the langugae

dsyme commented 2 years ago

Closing as I don't see a general criteria for this in the language. Linting is possible