fsprojects / FSharpLint

Lint tool for F#
https://fsprojects.github.io/FSharpLint/
MIT License
300 stars 73 forks source link

Structs: DUs and active patterns that have no fields or only simple sturcts should also be structs #705

Closed Thorium closed 3 months ago

Thorium commented 3 months ago

I'm stealing the great idea from @nojaf and adding it here as possible development as well. :-)

From FSharp.Core 6.0.1 onwards (F# 6):

A DU without any fields should be a struct:

[<Struct>]
type MyType =
| A
| B

Enums are fine, and this is a struct already:

type MyType =
| A = 1
| B = 2

Enums with simple fields like only boolean, int, Guid or struct could be also recommended to be structs, but that should be configurable, because that needs the name for the field, and changes may be a breaking change for existing API:

[<Struct>]
type MyType =
| A of a: int 
| B of b: struct(bool*bool)

Partial active pattern that returns empty (), boolean, guid or int, should also be marked as [<return: Struct>] and use ValueOption instead of Option:

[<return: Struct>]
let (|MyThing|_|) x =
   if x then ValueSome()
   else ValueNone

This comes from conversation in here: https://github.com/fsprojects/FSharp.Formatting/pull/906

knocte commented 3 months ago

So this would be a rule whose purpose is optimization? (speed? memory?)

Thorium commented 3 months ago

Yes, both. Similarly like testing IEnumerable isEmpty is optimization over "length > 0".

Thorium commented 3 months ago

I'm closing this because this is already done in ionide-analyzers project. Over time that will already improve general F# code-bases on a level appropriate, there is no point of doing double amount of work implementing it here.