Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] New rule to prefer non-mutable property initialization #535

Closed
knocte opened this issue Dec 27, 2021 · 0 comments · Fixed by #683
Closed

[Feature] New rule to prefer non-mutable property initialization #535

knocte opened this issue Dec 27, 2021 · 0 comments · Fixed by #683

Comments

@knocte
Copy link
Collaborator

knocte commented Dec 27, 2021

Description

New rule to prefer non-mutable property initialization.

Testcases

Code that should be flagged:

  1. With a write-only property:
type SomeClass() =
    let mutable myInternalValue = 1
    // A write-only property.
    member this.MyWriteOnlyProperty with set (value) = myInternalValue <- value

module Program =
    let someInstance = SomeClass()
    someInstance.MyWriteOnlyProperty <- 2
  1. With a read-write property:
type SomeClass() =
    let mutable myInternalValue = 1
    // A read-write property.
    member this.MyReadWriteProperty
        with get () = myInternalValue
        and set (value) = myInternalValue <- value

module Program =
    let someInstance = SomeClass()
    someInstance.MyReadWriteProperty <- 2
  1. With a C# class:
module Program =
    // SomeCSharpClass implementation lives in a referenced assembly
    let someInstance = SomeCSharpClass()
    someInstance.MyReadWriteProperty <- 2
  1. Same as 2 but with a static method call (static invocations shouldn't affect the flagging):
type SomeClass() =
    let mutable myInternalValue = 1
    static member SomeStaticMethod() =
        () // any code goes here
    // A read-write property.
    member this.MyReadWriteProperty
        with get () = myInternalValue
        and set (value) = myInternalValue <- value

module Program =
    let someInstance = SomeClass()
    SomeClass.SomeStaticMethod()
    someInstance.MyReadWriteProperty <- 2

Code that should not be flagged:

  1. Using non-mutable property initialization (with a write-only property):
type SomeClass() =
    let mutable myInternalValue = 1
    // A write-only property.
    member this.MyWriteOnlyProperty with set (value) = myInternalValue <- value

module Program =
    let someInstance = SomeClass(MyReadWriteProperty = 2)
    1. Using non-mutable property initialization (with a read-write property):
type SomeClass() =
    let mutable myInternalValue = 1
    // A read-write property.
    member this.MyReadWriteProperty
        with get () = myInternalValue
        and set (value) = myInternalValue <- value

module Program =
    let someInstance = SomeClass(MyReadWriteProperty = 2)
  1. Mutating the class after using it:
type SomeClass() =
    let mutable myInternalValue = 1
    member this.SomeMethod() =
        () // some code here
    // A read-write property.
    member this.MyReadWriteProperty
        with get () = myInternalValue
        and set (value) = myInternalValue <- value

module Program =
    let someInstance = SomeClass()
    someInstance.SomeMethod()
    someInstance.MyReadWriteProperty <- 2
  1. Modules, not classes:
module SomeModule =
    let mutable myInternalValue = 1

module Program =
    SomeModule.myInternalValue <- 2

Granted, using non-mutable property initialization generates the same CIL code than the flagged code, but it looks better because it doesn't have the <- operator.

janus added a commit to janus/FSharpLint that referenced this issue Jan 11, 2022
janus added a commit to janus/FSharpLint that referenced this issue Jan 11, 2022
janus added a commit to janus/FSharpLint that referenced this issue Jan 12, 2022
janus added a commit to janus/FSharpLint that referenced this issue Jan 12, 2022
janus added a commit to janus/FSharpLint that referenced this issue Jan 13, 2022
janus added a commit to janus/FSharpLint that referenced this issue May 12, 2022
janus added a commit to janus/FSharpLint that referenced this issue May 12, 2022
parhamsaremi pushed a commit to parhamsaremi/FSharpLint that referenced this issue Nov 21, 2022
knocte pushed a commit to knocte/FSharpLint that referenced this issue Dec 31, 2023
knocte pushed a commit to knocte/FSharpLint that referenced this issue Dec 31, 2023
knocte pushed a commit to knocte/FSharpLint that referenced this issue Dec 31, 2023
knocte pushed a commit to knocte/FSharpLint that referenced this issue Dec 31, 2023
webwarrior-ws pushed a commit to webwarrior-ws/FSharpLint that referenced this issue Jan 16, 2024
webwarrior-ws pushed a commit to webwarrior-ws/FSharpLint that referenced this issue Jan 16, 2024
webwarrior-ws pushed a commit to webwarrior-ws/FSharpLint that referenced this issue Jan 16, 2024
webwarrior-ws pushed a commit to webwarrior-ws/FSharpLint that referenced this issue Jan 16, 2024
webwarrior-ws pushed a commit to webwarrior-ws/FSharpLint that referenced this issue Jan 18, 2024
knocte added a commit that referenced this issue Jan 18, 2024
…on-rebased

* Add FavourNonMutablePropertyInitialization rule and tests for it.
* Apply suggestions of this rule to FSharpLint code.

Fixes #535
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment