Skip to content

Commit

Permalink
Type check: recover in Set expressions when left side value is not mu…
Browse files Browse the repository at this point in the history
…table (dotnet#7682)

* Recover ValNoMutable and PropertyCannotBeSet errors

* Recover FieldNotMutable error

* Refactor tests

* Refactor tests

* Update baseline
  • Loading branch information
auduchinok authored and nosami committed Feb 22, 2021
1 parent 9d57f27 commit c47efcb
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 16 deletions.
3 changes: 2 additions & 1 deletion src/fsharp/MethodCalls.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1638,5 +1638,6 @@ let MethInfoChecks g amap isInstance tyargsOpt objArgs ad m (minfo: MethInfo) =
exception FieldNotMutable of DisplayEnv * Tast.RecdFieldRef * range

let CheckRecdFieldMutation m denv (rfinfo: RecdFieldInfo) =
if not rfinfo.RecdField.IsMutable then error (FieldNotMutable(denv, rfinfo.RecdFieldRef, m))
if not rfinfo.RecdField.IsMutable then
errorR (FieldNotMutable (denv, rfinfo.RecdFieldRef, m))

28 changes: 14 additions & 14 deletions src/fsharp/TypeChecker.fs
Original file line number Diff line number Diff line change
Expand Up @@ -9362,7 +9362,8 @@ and TcItemThen cenv overallTy env tpenv (item, mItem, rest, afterResolution) del
if isByrefTy g vty then
destByrefTy g vty
else
if not vref.IsMutable then error (ValNotMutable(env.DisplayEnv, vref, mStmt))
if not vref.IsMutable then
errorR (ValNotMutable (env.DisplayEnv, vref, mStmt))
vty
// Always allow subsumption on assignment to fields
let e2', tpenv = TcExprFlex cenv true false vty2 env tpenv e2
Expand Down Expand Up @@ -9415,15 +9416,15 @@ and TcItemThen cenv overallTy env tpenv (item, mItem, rest, afterResolution) del
if meths.IsEmpty then
let meths = pinfos |> GettersOfPropInfos
let isByrefMethReturnSetter = meths |> List.exists (function (_,Some pinfo) -> isByrefTy g (pinfo.GetPropertyType(cenv.amap,mItem)) | _ -> false)
if isByrefMethReturnSetter then
// x.P <- ... byref setter
if isNil meths then error (Error (FSComp.SR.tcPropertyIsNotReadable nm, mItem))
TcMethodApplicationThen cenv env overallTy None tpenv tyargsOpt [] mItem mItem nm ad NeverMutates true meths afterResolution NormalValUse args ExprAtomicFlag.Atomic delayed
else
error (Error (FSComp.SR.tcPropertyCannotBeSet1 nm, mItem))
if not isByrefMethReturnSetter then
errorR (Error (FSComp.SR.tcPropertyCannotBeSet1 nm, mItem))
// x.P <- ... byref setter
if isNil meths then error (Error (FSComp.SR.tcPropertyIsNotReadable nm, mItem))
TcMethodApplicationThen cenv env overallTy None tpenv tyargsOpt [] mItem mItem nm ad NeverMutates true meths afterResolution NormalValUse args ExprAtomicFlag.Atomic delayed
else
let args = if pinfo.IsIndexer then args else []
if isNil meths then error (Error (FSComp.SR.tcPropertyCannotBeSet1 nm, mItem))
if isNil meths then
errorR (Error (FSComp.SR.tcPropertyCannotBeSet1 nm, mItem))
// Note: static calls never mutate a struct object argument
TcMethodApplicationThen cenv env overallTy None tpenv tyargsOpt [] mStmt mItem nm ad NeverMutates true meths afterResolution NormalValUse (args@[e2]) ExprAtomicFlag.NonAtomic otherDelayed
| _ ->
Expand Down Expand Up @@ -9600,12 +9601,11 @@ and TcLookupThen cenv overallTy env tpenv mObjExpr objExpr objExprTy longId dela
if meths.IsEmpty then
let meths = pinfos |> GettersOfPropInfos
let isByrefMethReturnSetter = meths |> List.exists (function (_,Some pinfo) -> isByrefTy cenv.g (pinfo.GetPropertyType(cenv.amap,mItem)) | _ -> false)
if isByrefMethReturnSetter then
// x.P <- ... byref setter
if isNil meths then error (Error (FSComp.SR.tcPropertyIsNotReadable nm, mItem))
TcMethodApplicationThen cenv env overallTy None tpenv tyargsOpt objArgs mExprAndItem mItem nm ad PossiblyMutates true meths afterResolution NormalValUse args atomicFlag delayed
else
error (Error (FSComp.SR.tcPropertyCannotBeSet1 nm, mItem))
if not isByrefMethReturnSetter then
errorR (Error (FSComp.SR.tcPropertyCannotBeSet1 nm, mItem))
// x.P <- ... byref setter
if isNil meths then error (Error (FSComp.SR.tcPropertyIsNotReadable nm, mItem))
TcMethodApplicationThen cenv env overallTy None tpenv tyargsOpt objArgs mExprAndItem mItem nm ad PossiblyMutates true meths afterResolution NormalValUse args atomicFlag delayed
else
let args = if pinfo.IsIndexer then args else []
let mut = (if isStructTy cenv.g (tyOfExpr cenv.g objExpr) then DefinitelyMutates else PossiblyMutates)
Expand Down
2 changes: 1 addition & 1 deletion tests/fsharp/core/span/test3.bsl
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ test3.fsx(47,21,47,27): typecheck error FS0027: This value is not mutable. Consi
test3.fsx(46,26,46,27): typecheck error FS0193: Type constraint mismatch. The type
'Span<int>'
is not compatible with type
'seq<'a>'
'seq<int>'

29 changes: 29 additions & 0 deletions tests/service/Common.fs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ open System.IO
open System.Collections.Generic
open FSharp.Compiler
open FSharp.Compiler.SourceCodeServices
open FsUnit

#if NETCOREAPP2_0
let readRefs (folder : string) (projectFile: string) =
Expand Down Expand Up @@ -304,6 +305,34 @@ let rec allSymbolsInEntities compGen (entities: IList<FSharpEntity>) =
yield! allSymbolsInEntities compGen e.NestedEntities ]


let getSymbolUses (source: string) =
let _, typeCheckResults = parseAndCheckScript("/home/user/Test.fsx", source)
typeCheckResults.GetAllUsesOfAllSymbolsInFile() |> Async.RunSynchronously

let getSymbols (source: string) =
getSymbolUses source
|> Array.map (fun symbolUse -> symbolUse.Symbol)


let getSymbolName (symbol: FSharpSymbol) =
match symbol with
| :? FSharpMemberOrFunctionOrValue as mfv -> Some mfv.LogicalName
| :? FSharpEntity as entity -> Some entity.LogicalName
| :? FSharpGenericParameter as parameter -> Some parameter.Name
| :? FSharpParameter as parameter -> parameter.Name
| :? FSharpStaticParameter as parameter -> Some parameter.Name
| :? FSharpActivePatternCase as case -> Some case.Name
| :? FSharpUnionCase as case -> Some case.Name
| _ -> None


let assertContainsSymbolWithName name source =
getSymbols source
|> Array.choose getSymbolName
|> Array.contains name
|> shouldEqual true


let coreLibAssemblyName =
#if NETCOREAPP2_0
"System.Runtime"
Expand Down
37 changes: 37 additions & 0 deletions tests/service/EditorTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1319,3 +1319,40 @@ let ``FSharpField.IsNameGenerated`` () =
"type U = Case of string * Item2: string * string * Name: string",
["Item1", true; "Item2", false; "Item3", true; "Name", false]]
|> List.iter (fun (source, expected) -> checkFields source |> shouldEqual expected)


[<Test>]
let ``ValNoMutable recovery`` () =
let source = """
let x = 1
x <-
let y = 1
y
"""
assertContainsSymbolWithName "y" source


[<Test>]
let ``PropertyCannotBeSet recovery`` () =
let source = """
type T =
static member P = 1
T.P <-
let y = 1
y
"""
assertContainsSymbolWithName "y" source


[<Test>]
let ``FieldNotMutable recovery`` () =
let source = """
type R =
{ F: int }
{ F = 1 }.F <-
let y = 1
y
"""
assertContainsSymbolWithName "y" source

0 comments on commit c47efcb

Please sign in to comment.