Emit special warning when = is used and user might want to use <-

src/fsharp/CompileOps.fs

@@ -142,7 +142,10 @@ let GetRangeOfError(err:PhasedError) =
       | TypeIsImplicitlyAbstract m 
       | RequiredButNotSpecified (_,_,_,_,m) 
       | FunctionValueUnexpected (_,_,m)
-      | UnitTypeExpected (_,_,_,m )
+      | UnitTypeExpected (_,_,m)
+      | UnitTypeExpectedWithEquality (_,_,m)
+      | UnitTypeExpectedWithPossiblePropertySetter (_,_,_,m)
+      | UnitTypeExpectedWithPossibleAssignment (_,_,_,_,m)
       | UseOfAddressOfOperator m 
       | DeprecatedThreadStaticBindingWarning(m) 
       | NonUniqueInferredAbstractSlot (_,_,_,_,_,m) 
@@ -266,6 +269,9 @@ let GetErrorNumber(err:PhasedError) =
       | UnionPatternsBindDifferentNames _  -> 18
       | UnionCaseWrongArguments  _ -> 19
       | UnitTypeExpected _  -> 20
+      | UnitTypeExpectedWithEquality _  -> 20
+      | UnitTypeExpectedWithPossiblePropertySetter _  -> 20
+      | UnitTypeExpectedWithPossibleAssignment _  -> 20
       | RecursiveUseCheckedAtRuntime  _ -> 21
       | LetRecEvaluatedOutOfOrder  _ -> 22
       | NameClash _ -> 23
@@ -532,8 +538,11 @@ let UseOfAddressOfOperatorE() = DeclareResourceString("UseOfAddressOfOperator","
 let DefensiveCopyWarningE() = DeclareResourceString("DefensiveCopyWarning","%s")
 let DeprecatedThreadStaticBindingWarningE() = DeclareResourceString("DeprecatedThreadStaticBindingWarning","")
 let FunctionValueUnexpectedE() = DeclareResourceString("FunctionValueUnexpected","%s")
-let UnitTypeExpected1E() = DeclareResourceString("UnitTypeExpected1","")
-let UnitTypeExpected2E() = DeclareResourceString("UnitTypeExpected2","%s")
+let UnitTypeExpectedE() = DeclareResourceString("UnitTypeExpected","")
+let UnitTypeExpectedWithEqualityE() = DeclareResourceString("UnitTypeExpectedWithEquality","")
+let UnitTypeExpectedWithPossiblePropertySetterE() = DeclareResourceString("UnitTypeExpectedWithPossiblePropertySetter","%s")
+let UnitTypeExpectedWithPossibleAssignmentE() = DeclareResourceString("UnitTypeExpectedWithPossibleAssignment","%s")
+let UnitTypeExpectedWithPossibleAssignmentToMutableE() = DeclareResourceString("UnitTypeExpectedWithPossibleAssignmentToMutable","%s")
 let RecursiveUseCheckedAtRuntimeE() = DeclareResourceString("RecursiveUseCheckedAtRuntime","")
 let LetRecUnsound1E() = DeclareResourceString("LetRecUnsound1","%s")
 let LetRecUnsound2E() = DeclareResourceString("LetRecUnsound2","%s%s")
@@ -1195,16 +1204,25 @@ let OutputPhasedErrorR (os:System.Text.StringBuilder) (err:PhasedError) =
       | DeprecatedThreadStaticBindingWarning(_) -> 
           os.Append(DeprecatedThreadStaticBindingWarningE().Format) |> ignore
       | FunctionValueUnexpected (denv,ty,_) ->
-          // REVIEW: consider if we need to show _cxs (the type parameter constrants)
+          // REVIEW: consider if we need to show _cxs (the type parameter constraints)
           let _, ty, _cxs = PrettyTypes.PrettifyTypes1 denv.g ty
           os.Append(FunctionValueUnexpectedE().Format (NicePrint.stringOfTy denv ty)) |> ignore
-      | UnitTypeExpected (denv,ty,perhapsProp,_) ->
-          // REVIEW: consider if we need to show _cxs (the type parameter constrants)
-          let _, ty, _cxs = PrettyTypes.PrettifyTypes1 denv.g ty
-          if perhapsProp then 
-            os.Append(UnitTypeExpected2E().Format (NicePrint.stringOfTy denv ty)) |> ignore
-          else
-            os.Append(UnitTypeExpected1E().Format) |> ignore
+      | UnitTypeExpected (_,_,_) ->
+          let warningText = UnitTypeExpectedE().Format
+          os.Append warningText |> ignore
+      | UnitTypeExpectedWithEquality (_) ->
+          let warningText = UnitTypeExpectedWithEqualityE().Format
+          os.Append warningText |> ignore
+      | UnitTypeExpectedWithPossiblePropertySetter (_,_,bindingName,_) ->
+          let warningText = UnitTypeExpectedWithPossiblePropertySetterE().Format bindingName
+          os.Append warningText |> ignore
+      | UnitTypeExpectedWithPossibleAssignment (_,_,isAlreadyMutable,bindingName,_) ->
+          let warningText = 
+            if isAlreadyMutable then
+                UnitTypeExpectedWithPossibleAssignmentToMutableE().Format bindingName
+            else
+                UnitTypeExpectedWithPossibleAssignmentE().Format bindingName
+          os.Append warningText |> ignore
       | RecursiveUseCheckedAtRuntime  _ -> 
           os.Append(RecursiveUseCheckedAtRuntimeE().Format) |> ignore
       | LetRecUnsound (_,[v],_) ->  

src/fsharp/FSStrings.resx

@@ -897,11 +897,20 @@
   <data name="FunctionValueUnexpected" xml:space="preserve">
     <value>This expression is a function value, i.e. is missing arguments. Its type is {0}.</value>
   </data>
-  <data name="UnitTypeExpected1" xml:space="preserve">
+  <data name="UnitTypeExpected" xml:space="preserve">
     <value>The result of this expression is implicitly ignored. Consider using 'ignore' to discard this value explicitly, e.g. 'expr |> ignore', or 'let' to bind the result to a name, e.g. 'let result = expr'.</value>
   </data>
-  <data name="UnitTypeExpected2" xml:space="preserve">
-    <value>This expression should have type 'unit', but has type '{0}'. If assigning to a property use the syntax 'obj.Prop &lt;- expr'.</value>
+  <data name="UnitTypeExpectedWithEquality" xml:space="preserve">
+    <value>The result of this equality expression is implicitly discarded. Consider using 'let' to bind the result to a name, e.g. 'let result = expression'.</value>
+  </data>
+  <data name="UnitTypeExpectedWithPossiblePropertySetter" xml:space="preserve">
+    <value>The result of this equality expression is implicitly discarded. Consider using 'let' to bind the result to a name, e.g. 'let result = expression'. If you intended to set a value to a property, then use the '&lt;-' operator e.g. '{0} &lt;- expression'.</value>
+  </data>
+  <data name="UnitTypeExpectedWithPossibleAssignment" xml:space="preserve">
+    <value>The result of this equality expression is implicitly discarded. Consider using 'let' to bind the result to a name, e.g. 'let result = expression'. If you intended to mutate a value, then mark the value 'mutable' and use the '&lt;-' operator e.g. '{0} &lt;- expression'.</value>
+  </data>
+  <data name="UnitTypeExpectedWithPossibleAssignmentToMutable" xml:space="preserve">
+    <value>The result of this equality expression is implicitly discarded. Consider using 'let' to bind the result to a name, e.g. 'let result = expression'. If you intended to mutate a value, then use the '&lt;-' operator e.g. '{0} &lt;- expression'.</value>
   </data>
   <data name="RecursiveUseCheckedAtRuntime" xml:space="preserve">
     <value>This recursive use will be checked for initialization-soundness at runtime. This warning is usually harmless, and may be suppressed by using '#nowarn "21"' or '--nowarn:21'.</value>
@@ -1083,4 +1092,4 @@
   <data name="TargetInvocationExceptionWrapper" xml:space="preserve">
     <value>internal error: {0}</value>
   </data>
-</root>
+</root>

src/fsharp/TypeChecker.fs

@@ -83,7 +83,10 @@ exception FieldsFromDifferentTypes of DisplayEnv * RecdFieldRef * RecdFieldRef *
 exception FieldGivenTwice of DisplayEnv * Tast.RecdFieldRef * range
 exception MissingFields of string list * range
 exception FunctionValueUnexpected of DisplayEnv * TType * range
-exception UnitTypeExpected of DisplayEnv * TType * bool * range
+exception UnitTypeExpected of DisplayEnv * TType * range
+exception UnitTypeExpectedWithEquality of DisplayEnv * TType * range
+exception UnitTypeExpectedWithPossibleAssignment of DisplayEnv * TType * bool * string * range
+exception UnitTypeExpectedWithPossiblePropertySetter of DisplayEnv * TType * string * range
 exception UnionPatternsBindDifferentNames of range
 exception VarBoundTwice of Ident
 exception ValueRestriction of DisplayEnv * bool * Val * Typar * range
@@ -725,13 +728,29 @@ let UnifyUnitType cenv denv m ty exprOpt =
         let resultTy = NewInferenceType ()
         if AddCxTypeEqualsTypeUndoIfFailed denv cenv.css m ty (domainTy --> resultTy) then 
             warning (FunctionValueUnexpected(denv,ty,m))
-        else
-            let perhapsProp = 
-                typeEquiv cenv.g cenv.g.bool_ty ty &&
+        else        
+            if not (typeEquiv cenv.g cenv.g.bool_ty ty) then 
+                warning (UnitTypeExpected (denv,ty,m)) 
+            else
                 match exprOpt with 
-                | Some(Expr.App(Expr.Val(vf,_,_),_,_,[__],_)) when vf.LogicalName = opNameEquals -> true
-                | _ -> false
-            warning (UnitTypeExpected (denv,ty,perhapsProp,m)) 
+                | Some(Expr.App(Expr.Val(vf,_,_),_,_,exprs,_)) when vf.LogicalName = opNameEquals ->
+                    match exprs with 
+                    | Expr.App(Expr.Val(prop,_,_),_,_,Expr.Val(vf,_,_) :: _,_) :: _ ->
+                        let isProperty = 
+                            match prop.MemberInfo with 
+                            | None -> false 
+                            | Some memInfo -> memInfo.MemberFlags.MemberKind = MemberKind.PropertyGet
+
+                        if isProperty then
+                            warning (UnitTypeExpectedWithPossiblePropertySetter (denv,ty,vf.LogicalName,m))
+                        else
+                            warning (UnitTypeExpectedWithEquality (denv,ty,m))
+                    | Expr.Op(_,_,Expr.Val(vf,_,_) :: _,_) :: _ -> 
+                        warning (UnitTypeExpectedWithPossiblePropertySetter (denv,ty,vf.LogicalName,m))
+                    | Expr.Val(vf,_,_) :: _ -> 
+                        warning (UnitTypeExpectedWithPossibleAssignment (denv,ty,vf.IsMutable,vf.LogicalName,m))
+                    | _ -> warning (UnitTypeExpectedWithEquality (denv,ty,m))
+                | _ -> warning (UnitTypeExpected (denv,ty,m)) 
         false
     else
         true
@@ -2249,7 +2268,8 @@ module GeneralizationHelpers =
         | Some memberFlags -> 
             match memberFlags.MemberKind with 
             // can't infer extra polymorphism for properties 
-            | MemberKind.PropertyGet  | MemberKind.PropertySet  -> 
+            | MemberKind.PropertyGet 
+            | MemberKind.PropertySet -> 
                  if not (isNil declaredTypars) then 
                      errorR(Error(FSComp.SR.tcPropertyRequiresExplicitTypeParameters(),m))
             | MemberKind.Constructor -> 

src/fsharp/TypeChecker.fsi

@@ -75,7 +75,10 @@ exception UnionCaseWrongNumberOfArgs of DisplayEnv * int * int * range
 exception FieldsFromDifferentTypes of DisplayEnv * RecdFieldRef * RecdFieldRef * range
 exception FieldGivenTwice of DisplayEnv * RecdFieldRef * range
 exception MissingFields of string list * range
-exception UnitTypeExpected of DisplayEnv * TType * bool * range
+exception UnitTypeExpected of DisplayEnv * TType * range
+exception UnitTypeExpectedWithEquality of DisplayEnv * TType * range
+exception UnitTypeExpectedWithPossiblePropertySetter of DisplayEnv * TType * string * range
+exception UnitTypeExpectedWithPossibleAssignment of DisplayEnv * TType * bool * string * range
 exception FunctionValueUnexpected of DisplayEnv * TType * range
 exception UnionPatternsBindDifferentNames of range
 exception VarBoundTwice of Ast.Ident

tests/fsharpqa/Source/Conformance/Expressions/DataExpressions/QueryExpressions/E_WhereRequiresParens01.fs

@@ -1,7 +1,7 @@
 // #Conformance #DataExpressions #Query
 // Where expressions require parenthesis
+//<Expects status="error" span="(8,21-8,22)" id="FS3145">This is not a known query operator. Query operators are identifiers such as 'select', 'where', 'sortBy', 'thenBy', 'groupBy', 'groupValBy', 'join', 'groupJoin', 'sumBy' and 'averageBy', defined using corresponding methods on the 'QueryBuilder' type.</Expects>
 //<Expects status="error" span="(8,9-8,14)" id="FS3095">'where' is not used correctly\. This is a custom operation in this query or computation expression\.$</Expects>
-//<Expects status="error" span="(8,9-8,24)" id="FS0020">The result of this expression is implicitly ignored\. Consider using 'ignore' to discard this value explicitly, e\.g\. 'expr \|> ignore', or 'let' to bind the result to a name, e\.g\. 'let result = expr'.$</Expects>
 let query =
     query {
         for i in [1..10] do

tests/fsharpqa/Source/Warnings/WarnIfImplicitlyDiscarded.fs

@@ -0,0 +1,11 @@
+// #Warnings
+//<Expects status="Warning" span="(8,5)" id="FS0020">The result of this equality expression is implicitly discarded. Consider using 'let' to bind the result to a name, e.g. 'let result = expression'.</Expects>
+
+let x = 10
+let y = 20
+
+let changeX() =
+    y * x = 20
+    y = 30
+
+exit 0

tests/fsharpqa/Source/Warnings/WarnIfPossibleAssignment.fs

@@ -0,0 +1,11 @@
+// #Warnings
+//<Expects status="Warning" span="(8,5)" id="FS0020">If you intended to mutate a value, then mark the value 'mutable' and use the '<-' operator e.g. 'x <- expression'.</Expects>
+
+let x = 10
+let y = "hello"
+
+let changeX() =
+    x = 20
+    y = "test"
+
+exit 0

tests/fsharpqa/Source/Warnings/WarnIfPossibleAssignmentToMutable.fs

@@ -0,0 +1,11 @@
+// #Warnings
+//<Expects status="Warning" span="(8,5)" id="FS0020">If you intended to mutate a value, then use the '<-' operator e.g. 'x <- expression'.</Expects>
+
+let mutable x = 10
+let y = "hello"
+
+let changeX() =
+    x = 20
+    y = "test"
+
+exit 0

tests/fsharpqa/Source/Warnings/WarnIfPossibleDotNetPropertySetter.fs

@@ -0,0 +1,13 @@
+// #Warnings
+//<Expects status="Warning" span="(10,5)" id="FS0020">If you intended to set a value to a property, then use the '<-' operator e.g. 'x <- expression'</Expects>
+
+open System
+
+let x = System.Timers.Timer()
+let y = "hello"
+
+let changeProperty() =
+    x.Enabled = true
+    y = "test"
+
+exit 0

tests/fsharpqa/Source/Warnings/WarnIfPossiblePropertySetter.fs

@@ -0,0 +1,15 @@
+// #Warnings
+//<Expects status="Warning" span="(12,5)" id="FS0020">If you intended to set a value to a property, then use the '<-' operator e.g. 'x <- expression'</Expects>
+
+type MyClass(property1 : int) =
+    member val Property1 = property1
+    member val Property2 = "" with get, set
+
+let x = MyClass(1)
+let y = "hello"
+
+let changeProperty() =
+    x.Property1 = 20
+    y = "test"
+
+exit 0

tests/fsharpqa/Source/Warnings/env.lst

@@ -30,3 +30,8 @@
 	SOURCE=RuntimeTypeTestInPattern2.fs # RuntimeTypeTestInPattern2.fs
 	SOURCE=WarnIfExpressionResultUnused.fs # WarnIfExpressionResultUnused.fs
 	SOURCE=Repro1548.fs SCFLAGS="-r:Repro1548.dll"				# Repro1548.fs
+	SOURCE=WarnIfPossibleAssignment.fs
+	SOURCE=WarnIfPossibleAssignmentToMutable.fs
+	SOURCE=WarnIfPossibleDotNetPropertySetter.fs
+	SOURCE=WarnIfImplicitlyDiscarded.fs
+	SOURCE=WarnIfPossiblePropertySetter.fs