Merge pull request #1235 from taylorwood/result-ignored

Expand warning when expression result is ignored

src/fsharp/CompileOps.fs

@@ -531,7 +531,7 @@ let UseOfAddressOfOperatorE() = DeclareResourceString("UseOfAddressOfOperator","
 let DefensiveCopyWarningE() = DeclareResourceString("DefensiveCopyWarning","%s")
 let DeprecatedThreadStaticBindingWarningE() = DeclareResourceString("DeprecatedThreadStaticBindingWarning","")
 let FunctionValueUnexpectedE() = DeclareResourceString("FunctionValueUnexpected","%s")
-let UnitTypeExpected1E() = DeclareResourceString("UnitTypeExpected1","%s")
+let UnitTypeExpected1E() = DeclareResourceString("UnitTypeExpected1","")
 let UnitTypeExpected2E() = DeclareResourceString("UnitTypeExpected2","%s")
 let RecursiveUseCheckedAtRuntimeE() = DeclareResourceString("RecursiveUseCheckedAtRuntime","")
 let LetRecUnsound1E() = DeclareResourceString("LetRecUnsound1","%s")
@@ -1185,7 +1185,7 @@ let OutputPhasedErrorR (os:System.Text.StringBuilder) (err:PhasedError) =
           if perhapsProp then 
             os.Append(UnitTypeExpected2E().Format (NicePrint.stringOfTy denv ty)) |> ignore
           else
-            os.Append(UnitTypeExpected1E().Format (NicePrint.stringOfTy denv ty)) |> ignore
+            os.Append(UnitTypeExpected1E().Format) |> ignore
       | RecursiveUseCheckedAtRuntime  _ -> 
           os.Append(RecursiveUseCheckedAtRuntimeE().Format) |> ignore
       | LetRecUnsound (_,[v],_) ->  

src/fsharp/FSStrings.resx

@@ -892,7 +892,7 @@
     <value>This expression is a function value, i.e. is missing arguments. Its type is {0}.</value>
   </data>
   <data name="UnitTypeExpected1" xml:space="preserve">
-    <value>This expression should have type 'unit', but has type '{0}'. Use 'ignore' to discard the result of the expression, or 'let' to bind the result to a name.</value>
+    <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>

tests/fsharpqa/Source/Conformance/DeclarationElements/LetBindings/Basic/W_DoBindingsNotUnit01.fs

@@ -2,7 +2,7 @@
 #light
 
 // Verify warning when 'do-bindings' do not return unit.
-//<Expects id="FS0020" status="warning">This expression should have type 'unit', but has type 'int'</Expects>
+//<Expects id="FS0020" status="warning">The result of this expression is implicitly ignored</Expects>
 
 let square x = x * x
 

tests/fsharpqa/Source/Conformance/Expressions/BindingExpressions/Binding/in01.fs

@@ -5,7 +5,7 @@
 // Eventually, we will deprecated them - and the specs will be updated.
 //
 //<Expects status="error" span="(11,5-11,6)" id="FS0039">The value or constructor 'a' is not defined$</Expects>
-//<Expects status="error" span="(11,5-11,10)" id="FS0020">This expression should have type 'unit', but has type 'bool'\. Use 'ignore' to discard the result of the expression, or 'let' to bind the result to a name\.$</Expects>
+//<Expects status="error" span="(11,5-11,10)" 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>
 module A =
     let a = 3 in a + 1 |> ignore;;
     a > 4;;

tests/fsharpqa/Source/Conformance/Expressions/BindingExpressions/Binding/in02.fs

@@ -4,7 +4,7 @@
 // I'm adding these cases to make sure we do not accidentally change the behavior from version to version
 // Eventually, we will deprecated them - and the specs will be updated.
 //
-//<Expects status="error" span="(11,5-12,10)" id="FS0020">This expression should have type 'unit', but has type 'bool'\. Use 'ignore' to discard the result of the expression, or 'let' to bind the result to a name\.$</Expects>
+//<Expects status="error" span="(11,5-12,10)" 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>
 //
 
 module B =

tests/fsharpqa/Source/Conformance/Expressions/BindingExpressions/Binding/in03.fs

@@ -4,7 +4,7 @@
 // I'm adding these cases to make sure we do not accidentally change the behavior from version to version
 // Eventually, we will deprecated them - and the specs will be updated.
 //
-//<Expects status="error" span="(12,5-12,10)" id="FS0020">This expression should have type 'unit', but has type 'bool'\. Use 'ignore' to discard the result of the expression, or 'let' to bind the result to a name\.$</Expects>
+//<Expects status="error" span="(12,5-12,10)" 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>
 //
 module C =
     let a = 3 

tests/fsharpqa/Source/Conformance/Expressions/BindingExpressions/Binding/in04.fs

@@ -4,7 +4,7 @@
 // I'm adding these cases to make sure we do not accidentally change the behavior from version to version
 // Eventually, we will deprecated them - and the specs will be updated.
 //
-//<Expects status="error" span="(11,5-13,10)" id="FS0020">This expression should have type 'unit', but has type 'bool'\. Use 'ignore' to discard the result of the expression, or 'let' to bind the result to a name\.$</Expects>
+//<Expects status="error" span="(11,5-13,10)" 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>
 //
 
 module D =

tests/fsharpqa/Source/Conformance/Expressions/BindingExpressions/Binding/in05.fs

@@ -5,7 +5,7 @@
 // Eventually, we will deprecated them - and the specs will be updated.
 //<Expects status="error" span="(12,13-12,14)" id="FS0001">The type 'int' does not match the type 'unit'$</Expects>
 //<Expects status="error" span="(12,18-12,24)" id="FS0001">Type mismatch\. Expecting a.    ''a -> 'b'    .but given a.    ''a -> unit'    .The type 'int' does not match the type 'unit'$</Expects>
-//<Expects status="error" span="(10,5-13,10)" id="FS0020">This expression should have type 'unit', but has type 'bool'\. Use 'ignore' to discard the result of the expression, or 'let' to bind the result to a name\.$</Expects>
+//<Expects status="error" span="(10,5-13,10)" 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>
 module E = 
     let a = 3 in
         a + 1 |> ignore

tests/fsharpqa/Source/Conformance/Expressions/ControlFlowExpressions/TryFinally/W-TryFinallyNotUnit.fs

@@ -2,7 +2,7 @@
 #light
 
 // Verify warning if a finally block does not return 'unit'
-//<Expects id="FS0020" status="warning">This expression should have type 'unit', but has type 'bool'</Expects>
+//<Expects id="FS0020" status="warning">The result of this expression is implicitly ignored</Expects>
 
 let x : int =
     try

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,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">This expression should have type 'unit', but has type 'bool'\. Use 'ignore' to discard the result of the expression, or 'let' to bind the result to a name\.$</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/Conformance/PatternMatching/Named/E_ActivePatternUnconstrained01.fs

@@ -2,7 +2,7 @@
 // Regression test for FSHARP1.0:5590
 // This code used to compile, but fail peverification
 // Now, it just does not compile anymore telling the user to annotated it a bit.
-//<Expects status="warning" span="(14,1-14,5)" id="FS0020">This expression should have type 'unit', but has type 'int'\. Use 'ignore' to discard the result of the expression, or 'let' to bind the result to a name\.$</Expects>
+//<Expects status="warning" span="(14,1-14,5)" 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>
 //<Expects status="error" span="(7,6-7,16)" id="FS1210">Active pattern '\|A1\|A2\|A3\|' has a result type containing type variables that are not determined by the input\. The common cause is a when a result case is not mentioned, e\.g\. 'let \(\|A\|B\|\) \(x:int\) = A x'\. This can be fixed with a type constraint, e\.g\. 'let \(\|A\|B\|\) \(x:int\) : Choice<int,unit> = A x'$</Expects>
 let (|A1|A2|A3|) (inp:int) : Choice<unit,'T,'U> = 
     printfn "hello"

tests/fsharpqa/Source/Diagnostics/General/W_Multiline02.fs

@@ -1,7 +1,7 @@
 // #Regression #Diagnostics 
 // Regression test for FSHARP1.0:3596
 // Make sure that error spans correctly across multiple lines
-//<Expects status="warning" id="FS0020" span="(8,1-10,18)">This expression should have type 'unit'</Expects>
+//<Expects status="warning" id="FS0020" span="(8,1-10,18)">The result of this expression is implicitly ignored</Expects>
 #nowarn "988"
 let f g x = g x
 

tests/fsharpqa/Source/Diagnostics/NONTERM/quoteExpr01.fs

@@ -1,5 +1,5 @@
 // #Regression #Diagnostics 
 // Regression test for FSHARP1.0:2391, FSHARP1.0:1479
-//<Expects id="FS0020" span="(5,1-5,24)" status="warning">This expression should have type 'unit', but has type 'seq.Quotations.Var.'</Expects>
+//<Expects id="FS0020" span="(5,1-5,24)" status="warning">The result of this expression is implicitly ignored</Expects>
 #light "off"
 <@@ 1 @@>.GetFreeVars()

tests/fsharpqa/Source/Diagnostics/async/MissingIgnore.fs

@@ -1,5 +1,5 @@
 // #Regression #Diagnostics #Async 
 // Regression tests for FSHARP1.0:4394
-//<Expects status="error" span="(4,9-4,10)" id="FS0020">This expression should have type 'unit', but has type 'int'\. Use 'ignore' to discard the result of the expression, or 'let' to bind the result to a name\.$</Expects>
+//<Expects status="error" span="(4,9-4,10)" 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>
 async { 1;
         return 2 }  |> ignore

tests/fsharpqa/Source/Diagnostics/async/MissingReturnBangForLoop01.fs

@@ -1,5 +1,5 @@
 // #Regression #Diagnostics #Async 
 // Regression tests for FSHARP1.0:4394
 // common mistake: forgetting the return! For a loop
-//<Expects status="error" span="(5,39-5,45)" id="FS0020">This expression should have type 'unit', but has type 'Async<'a>'\. Use 'ignore' to discard the result of the expression, or 'let' to bind the result to a name\.$</Expects>
+//<Expects status="error" span="(5,39-5,45)" 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 rec loop() = async { let x = 1 in loop() }

tests/fsharpqa/Source/Diagnostics/async/MissingReturnBangForLoop02.fs

@@ -1,5 +1,5 @@
 // #Regression #Diagnostics #Async 
 // Regression tests for FSHARP1.0:4394
 // common mistake: forgetting the return! For a loop
-//<Expects status="error" span="(5,26-5,32)" id="FS0020">This expression should have type 'unit', but has type 'Async<'a>'\. Use 'ignore' to discard the result of the expression, or 'let' to bind the result to a name\.$</Expects>
+//<Expects status="error" span="(5,26-5,32)" 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 rec loop() = async { loop() }

tests/fsharpqa/Source/Diagnostics/async/MissingReturnBangForLoop03.fs

@@ -1,6 +1,6 @@
 // #Regression #Diagnostics #Async 
 // Regression tests for FSHARP1.0:4394
 // common mistake: forgetting the return! For a loop
-//<Expects status="error" span="(6,41-6,48)" id="FS0020">This expression should have type 'unit', but has type 'Async<'a>'\. Use 'ignore' to discard the result of the expression, or 'let' to bind the result to a name\.$</Expects>
+//<Expects status="error" span="(6,41-6,48)" 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>
 //<Expects status="error" span="(6,50-6,52)" id="FS0001">This expression was expected to have type.    'Async<'a>'    .but here has type.    'unit'</Expects>
 let rec loop2() = async.Delay(fun () -> loop2(); ()); 

tests/fsharpqa/Source/Diagnostics/async/MissingReturnBangForLoop04.fs

@@ -1,7 +1,7 @@
 // #Regression #Diagnostics #Async 
 // Regression tests for FSHARP1.0:4394
 // common mistake: forgetting the return! For a loop
-//<Expects status="error" span="(8,35-8,42)" id="FS0020">This expression should have type 'unit', but has type 'Async<'a>'\. Use 'ignore' to discard the result of the expression, or 'let' to bind the result to a name\.$</Expects>
+//<Expects status="error" span="(8,35-8,42)" 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>
 //<Expects status="error" span="(8,44-8,46)" id="FS0001">This expression was expected to have type.    'Async<'a>'    .but here has type.    'unit'</Expects>
 // Note: interestingly, this looks much better if a method call is not used
 let delay x = async.Delay x

tests/fsharpqa/Source/Misc/Parsing02.fs

@@ -1,10 +1,10 @@
 // #Regression #Misc 
 // Verify warnings associated with top level expressions getting discarded
 
-//<Expects id="FS0020" status="warning">This expression should have type 'unit', but has type '\( \^a -> unit\) \* int'</Expects>
-//<Expects id="FS0020" status="warning">This expression should have type 'unit', but has type '\('a \[\] -> unit \[\]\) \* string \[\]'</Expects>
+//<Expects id="FS0020" status="warning" span="(9,1)">The result of this expression is implicitly ignored</Expects>
+//<Expects id="FS0020" status="warning" span="(12,1)">The result of this expression is implicitly ignored</Expects>
 
-// Note the comma between printf "%A", this results in a tuple expr which probabaly wasn't intended.
+// Note the comma between printf "%A", this results in a tuple expr which probably wasn't intended.
 let arr = [|"Foo"; "Bar"|]
 printf "%d", arr.Length
 

tests/fsharpqa/Source/Warnings/WarnIfExpressionResultUnused.fs

@@ -0,0 +1,5 @@
+// #Warnings
+//<Expects status="warning" span="(4,1)" 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>
+
+1 + 2
+printfn "%d" 3 

tests/fsharpqa/Source/Warnings/env.lst

@@ -14,3 +14,4 @@
 	SOURCE=DowncastInsteadOfUpcast.fs # DowncastInsteadOfUpcast.fs
 	SOURCE=RuntimeTypeTestInPattern.fs # RuntimeTypeTestInPattern.fs
 	SOURCE=RuntimeTypeTestInPattern2.fs # RuntimeTypeTestInPattern2.fs
+	SOURCE=WarnIfExpressionResultUnused.fs # WarnIfExpressionResultUnused.fs