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

[WIP] Nested Record Field Copy and Update Expression #4511

Closed
Closed
Show file tree
Hide file tree
Changes from 47 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
f2f23a0
no message
williamtetlow Mar 8, 2018
0f1a8fa
convert nested record fields to correct ast in TcRecdExpr - copyInfo …
williamtetlow Mar 10, 2018
e7520f9
WIP - TypeName.PropertyName now works for { person with Person.A = { …
williamtetlow Mar 13, 2018
a004c5a
got nested record field TypeName.PropertyName.PropertyName working
williamtetlow Mar 13, 2018
6980070
fix ambiguities between TypeName.PropertyName and PropertyName.Proper…
williamtetlow Mar 14, 2018
a974cd7
wip
williamtetlow Mar 16, 2018
9e54406
added Module/Namespace.TypeName.FieldName resolution. [WIP] need to a…
williamtetlow Mar 19, 2018
dc792d3
remove comments
williamtetlow Mar 19, 2018
3eb2b23
Merge pull request #1 from williamtetlow/nested-recd-module-res
williamtetlow Mar 19, 2018
a05aa22
fix error
AviAvni Mar 22, 2018
8ae92d2
Merge branch 'master' into nested-recd-update-recdexpr
AviAvni Mar 22, 2018
413672d
resolve nested module and namespace access
williamtetlow Mar 22, 2018
c160937
search whole list of module or namespace refs returned
williamtetlow Mar 27, 2018
ba2492a
Updated ModuleOrNamespace search to handle Namespace.Module.FieldName…
williamtetlow Mar 28, 2018
59975e0
search abbreviated type fields
williamtetlow Mar 28, 2018
85b43bb
[WIP] Add copy info so nested fields not referenced in nested express…
williamtetlow Apr 8, 2018
e4b3cc1
added basic conformance test for single update with varying levels of…
williamtetlow Apr 8, 2018
b8fc206
Merge branch 'master' of https://github.com/williamtetlow/visualfshar…
williamtetlow Apr 8, 2018
dc0bf4d
fix long ident range to be the total range of longident
williamtetlow Apr 9, 2018
79d141b
fix TypeName.Fieldname & NamespaceOrModule.FieldName copyInfo to only…
williamtetlow Apr 10, 2018
8597c01
first pass of multi updates
williamtetlow Apr 14, 2018
8911432
refactor module or namespace search
williamtetlow Apr 15, 2018
4edaee3
WIP refactoring code to fix ModuleName.TypeName.FieldName.FieldName
williamtetlow Apr 16, 2018
8fa2ec3
fix ModuleOrNamespace.TypeName.FieldName.FieldName
williamtetlow Apr 21, 2018
1ec29b6
simplify expanding AST
williamtetlow Apr 21, 2018
5f4bc72
refactoring
williamtetlow Apr 21, 2018
5b287f8
use active pattern throughout
williamtetlow Apr 21, 2018
5a17d31
more tidying up
williamtetlow Apr 21, 2018
9d45340
module and namespace test
williamtetlow Apr 21, 2018
295df45
Merge branch 'master' of https://github.com/williamtetlow/visualfshar…
williamtetlow Apr 21, 2018
0141e9e
Merge branch 'nested-recd-update-recdexpr' of https://github.com/will…
williamtetlow Apr 21, 2018
685ac8d
adding conformance tests
williamtetlow Jun 18, 2018
08416a4
Merge branch 'master' into nested-recd-update-recdexpr
williamtetlow Jun 18, 2018
990792c
Merge branch 'nested-recd-update-recdexpr' into nestd-recd-refactoring
williamtetlow Jun 18, 2018
efc1e19
fixing conformance tests
williamtetlow Jun 18, 2018
98d610f
Merge pull request #2 from williamtetlow/nestd-recd-refactoring
williamtetlow Jun 18, 2018
0d6c454
moving lid res to NameResolution module
williamtetlow Jun 19, 2018
45780fd
added conformance test for Type vs Field ambiguities
williamtetlow Jun 19, 2018
fdb0b01
moved resolution of lid representing nested field access to NameResol…
williamtetlow Jun 26, 2018
8498ba8
fixing issues from moving resolution code to NameResolution module
williamtetlow Jun 30, 2018
ab70071
Merge pull request #3 from williamtetlow/nestd-recd-refactoring
williamtetlow Jun 30, 2018
03f5b22
add missing exit to conformance tests
williamtetlow Jun 30, 2018
b5cb6af
Merge branch 'master' into nested-recd-update-recdexpr
williamtetlow Jun 30, 2018
33d7990
fixes to conformance tests
williamtetlow Jun 30, 2018
3a366af
make id used not the recd field reference
williamtetlow Jul 1, 2018
1207879
Check the same field is not declared twice in a nested field update
brokenprogrammer Sep 19, 2018
9f79a3b
Merge branch 'master' of https://github.com/williamtetlow/visualfshar…
williamtetlow Sep 19, 2018
35568d8
Merge branch 'master' into nested-recd-update-recdexpr
williamtetlow Jan 29, 2019
9dfc288
resolve issues from feedback
williamtetlow Jan 29, 2019
5af5fae
Merge branch 'nested-recd-update-recdexpr' of https://github.com/will…
williamtetlow Jan 29, 2019
a5c0b09
missed update to solution file
williamtetlow Jan 29, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 103 additions & 0 deletions src/fsharp/NameResolution.fs
Original file line number Diff line number Diff line change
Expand Up @@ -3137,6 +3137,109 @@ let ResolveFieldPrim sink (ncenv:NameResolver) nenv ad ty (mp,id:Ident) allField

[(resInfo,item)]

/// Resolve a long identifier representing a nested record field
let ResolveNestedField sink (ncenv:NameResolver) nenv ad ty (lid : Ident list) =
let typeNameResInfo = TypeNameResolutionInfo.Default
let g = ncenv.g

let lookupFld ty (id : Ident) =
let m = id.idRange
let otherRecdFlds ty =
let typeName = NicePrint.minimalStringOfType nenv.eDisplayEnv ty
[
for KeyValue(_, v) in nenv.eFieldLabels do
let fldOpt = v |> List.tryFind (fun r -> r.TyconRef.DisplayName = typeName)
match fldOpt with
| Some rfref -> yield rfref.RecdField.Id
| None -> ()
]

let lookup() =
let frefs =
try Map.find id.idText nenv.eFieldLabels |> success
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not Map.tryFind with match?

with :? KeyNotFoundException ->
// record label is unknown -> suggest related labels and give a hint to the user
raze (SuggestLabelsOfRelatedRecords g nenv id (otherRecdFlds ty))

// Eliminate duplicates arising from multiple 'open'
frefs
|?> ListSet.setify (fun fref1 fref2 -> tyconRefEq g fref1.TyconRef fref2.TyconRef)
|?> List.map (fun x -> FieldResolution(x,false))

if isAppTy g ty then
match ncenv.InfoReader.TryFindRecdOrClassFieldInfoOfType(id.idText,m,ty) with
| Some (RecdFieldInfo(_,rfref)) -> success [FieldResolution(rfref,false)]
| None ->
if isRecdTy g ty then
// record label doesn't belong to record type -> suggest other labels of same record
let suggestLabels() = SuggestOtherLabelsOfSameRecordType g nenv ty id (otherRecdFlds ty)
let typeName = NicePrint.minimalStringOfType nenv.eDisplayEnv ty
let errorText = FSComp.SR.nrRecordDoesNotContainSuchLabel(typeName,id.idText)
raze (ErrorWithSuggestions(errorText, m, id.idText, suggestLabels))
else
lookup()
else
lookup()

let access, flds =
match lid with
| [] -> [], []
| [id] -> [], lookupFld ty id |> ForceRaise
| id :: _ ->
let fldSearch () =
match lid with
| id :: rest ->
let fldSearch = lookupFld ty id
let fldSearch = fldSearch |?> List.map (fun (FieldResolution(rfref, dep)) -> FieldResolution(rfref, dep), rfref.FieldName, rfref.FormalType, rest)
fldSearch
| _ -> NoResultsOrUsefulErrors

let tyconSearch ad () =
match lid with
| tn :: id :: rest ->
let m = tn.idRange
let tcrefs = LookupTypeNameInEnvNoArity OpenQualified tn.idText nenv
if isNil tcrefs then NoResultsOrUsefulErrors else
let tcrefs = tcrefs |> List.map (fun tcref -> (ResolutionInfo.Empty,tcref))
let tyconSearch = ResolveLongIdentInTyconRefs ResultCollectionSettings.AllResults ncenv nenv LookupKind.RecdField 1 m ad id rest typeNameResInfo tn.idRange tcrefs
let tyconSearch = tyconSearch |?> List.choose (function (_, Item.RecdField(RecdFieldInfo(_,rfref)), rest) -> Some(FieldResolution(rfref,false), rfref.FieldName, rfref.FormalType, rest) | _ -> None)
tyconSearch
| _ -> NoResultsOrUsefulErrors

let moduleOrNsSearch ad () =
match lid with
| [] -> NoResultsOrUsefulErrors
| id :: rest ->
let m = id.idRange
let t = ResolveLongIndentAsModuleOrNamespaceThen sink ResultCollectionSettings.AtMostOneResult ncenv.amap m OpenQualified nenv ad id rest false
(ResolveFieldInModuleOrNamespace ncenv nenv ad)
t |?> List.map (fun (_, FieldResolution(rfref, dep), rest) -> (FieldResolution(rfref, dep), rfref.FieldName, rfref.FormalType, rest))

let item, fldIdText, fldTy, rest =
fldSearch () +++ moduleOrNsSearch ad +++ tyconSearch ad +++ moduleOrNsSearch AccessibleFromSomeFSharpCode +++ tyconSearch AccessibleFromSomeFSharpCode
|> AtMostOneResult id.idRange
|> ForceRaise

let idsBeforeField = lid |> List.takeWhile (fun id -> id.idText <> fldIdText)

match rest with
| [] -> idsBeforeField, [item]
| _ ->
let rec nestedFieldSearch flds ty =
function
| [] -> flds
| id :: rest ->
let resolved = lookupFld ty id |> ForceRaise
let fldTy =
match resolved with
| [FieldResolution(rfref, _)] -> rfref.FormalType
| _ -> ty
nestedFieldSearch (flds @ resolved) fldTy rest

idsBeforeField, item::(nestedFieldSearch [] fldTy rest)

access, flds |> List.map (fun (FieldResolution(rfref, _)) -> lid |> List.find (fun id -> id.idText = rfref.FieldName))

let ResolveField sink ncenv nenv ad ty (mp,id) allFields =
let res = ResolveFieldPrim sink ncenv nenv ad ty (mp,id) allFields
// Register the results of any field paths "Module.Type" in "Module.Type.field" as a name resolution. (Note, the path resolution
Expand Down
3 changes: 3 additions & 0 deletions src/fsharp/NameResolution.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,9 @@ val internal ResolveTypeLongIdentInTyconRef : TcResultsSink -> NameResol
/// Resolve a long identifier to a type definition
val internal ResolveTypeLongIdent : TcResultsSink -> NameResolver -> ItemOccurence -> FullyQualifiedFlag -> NameResolutionEnv -> AccessorDomain -> Ident list -> TypeNameResolutionStaticArgsInfo -> PermitDirectReferenceToGeneratedType -> ResultOrException<TyconRef>

/// Resolve a long identifier to a nested field
val internal ResolveNestedField : TcResultsSink -> NameResolver -> NameResolutionEnv -> AccessorDomain -> TType -> Ident list -> Ident list * Ident list

/// Resolve a long identifier to a field
val internal ResolveField : TcResultsSink -> NameResolver -> NameResolutionEnv -> AccessorDomain -> TType -> Ident list * Ident -> Ident list -> FieldResolution list

Expand Down
86 changes: 82 additions & 4 deletions src/fsharp/TypeChecker.fs
Original file line number Diff line number Diff line change
Expand Up @@ -6898,10 +6898,85 @@ and TcAssertExpr cenv overallTy env (m:range) tpenv x =
//-------------------------------------------------------------------------

and TcRecdExpr cenv overallTy env tpenv (inherits, optOrigExpr, flds, mWholeExpr) =
let buildForNestdFlds (lidwd : LongIdentWithDots) v =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think throwing a couple of chars saves a lot of space in this case. Why not buildForNestedFields?

let recdExprCopyInfo ids (optOrigExpr : (SynExpr * BlockSeparator) option) (id : Ident) =
let upToId origSepRng id lidwd =
let rec buildLid res (id : Ident) =
function
| [] -> res
| (h : Ident) :: t -> if h.idText = id.idText then h :: res else buildLid (h :: res) id t

let rec combineIds =
function
| [] | [_] -> []
| id1::id2::rest -> (id1, id2) :: (id2 :: rest |> combineIds)

let calcLidSeparatorRanges lid =
match lid with
| [] | [_] -> [origSepRng]
| _ :: t -> origSepRng :: List.map (fun (s : Ident, e : Ident) -> mkRange s.idRange.FileName s.idRange.End e.idRange.Start) t

let lid = buildLid [] id lidwd |> List.rev

(lid, lid |> combineIds |> calcLidSeparatorRanges)

let totalRange (origId : Ident) (id : Ident) =
mkRange origId.idRange.FileName origId.idRange.End id.idRange.Start

match optOrigExpr with
| Some (SynExpr.Ident origId, (sepRange, _)) ->
let lid, rng = upToId sepRange id (origId :: ids)
Some (SynExpr.LongIdent (false, LongIdentWithDots(lid, rng), None, totalRange origId id), (id.idRange, None)) // TODO: id.idRange should be the range of the next separator
| _ -> None


let rec synExprRecd copyInfo id flds =
Some(SynExpr.Record((None, (copyInfo id), [ match flds with
| [] -> yield ((LongIdentWithDots ([], []), true), v, None)
| [fldId] -> yield ((LongIdentWithDots ([fldId],[]), true), v, None)
| fldId :: rest ->
let nestedFld = synExprRecd copyInfo fldId
yield ((LongIdentWithDots ([fldId], []), true), nestedFld rest, None)], id.idRange)))

let access, flds = lidwd.Lid |> ResolveNestedField cenv.tcSink cenv.nameResolver env.eNameResEnv env.eAccessRights overallTy

let expanded =
[
match (access, flds) with
| [], [] -> ()
| accessIds, [] -> yield (accessIds |> List.frontAndBack), v
| accessIds, [fldId] -> yield ((accessIds@[fldId]) |> List.frontAndBack), v
| accessIds, fldId :: rest ->
yield (accessIds, fldId), synExprRecd (recdExprCopyInfo flds optOrigExpr) fldId rest
]

expanded

let reduceNstdUpdates flds =
let grpdByFld = flds |> List.groupBy (fun ((_, fld : Ident), _) -> fld.idText)
[
for (_, flds) in grpdByFld do
if (flds |> List.length < 2) then
yield! flds
else
let rec grpIfNstd res xs =
match xs with
| [] -> res
| x::[] -> x :: res
| x::y::ys -> match x, y with
| (lidwid, Some(SynExpr.Record (aBI, aCI, aFlds, aRng))), (_, Some(SynExpr.Record (_, _, bFlds, _))) ->
let combinedFlds = aFlds @ bFlds
let reducedRecd = (lidwid, Some(SynExpr.Record (aBI, aCI, combinedFlds, aRng)))
grpIfNstd (reducedRecd :: res) ys
| _ -> grpIfNstd (x :: res) (y :: ys)

yield! flds |> grpIfNstd []
]

let requiresCtor = (GetCtorShapeCounter env = 1) // Get special expression forms for constructors
let haveCtor = Option.isSome inherits


let optOrigExprInfo, tpenv =
match optOrigExpr with
| None -> None, tpenv
Expand All @@ -6915,7 +6990,7 @@ and TcRecdExpr cenv overallTy env tpenv (inherits, optOrigExpr, flds, mWholeExpr

let hasOrigExpr = optOrigExprInfo.IsSome

let fldsList =
let fldsList =
let flds =
[
// if we met at least one field that is not syntactically correct - raise ReportedError to transfer control to the recovery routine
Expand All @@ -6925,9 +7000,12 @@ and TcRecdExpr cenv overallTy env tpenv (inherits, optOrigExpr, flds, mWholeExpr
// we assume that parse errors were already reported
raise (ReportedError None)

yield (List.frontAndBack lidwd.Lid, v)
]

match lidwd.Lid with
| [] -> ()
| [id] -> yield (([], id), v)
| _ -> yield! buildForNestdFlds lidwd v
] |> reduceNstdUpdates

match flds with
| [] -> []
| _ ->
Expand Down
3 changes: 3 additions & 0 deletions src/fsharp/tast.fs
Original file line number Diff line number Diff line change
Expand Up @@ -3884,6 +3884,9 @@ and
/// Get the signature range of the record field
member x.SigRange = x.RecdField.SigRange

/// The type of the record field
member x.FormalType = x.RecdField.FormalType

member x.Index =
let (RFRef(tcref,id)) = x
try
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// #Regression #Conformance #TypesAndModules #Records
// Verify same field cannot be declared twice in a nested field update
//<Expects id="FS0668" status="error">The field 'A' appears twice in this record expression or pattern</Expects>
#light

type AnotherNestedRecTy = { A : int; }

type NestdRecTy = { B : string; C : AnotherNestedRecTy; }

type RecTy = { D : NestdRecTy; E : string option; }

let t1 = { D = { B = "t1"; C = { A = 1; } }; E = None; }

let t2 = { t1 with D.C.A = 3; D.C.A = 2}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// #Conformance #TypesAndModules #Records
#light

// Verify cloning and updating of fields using nested copy and update syntax

type AnotherNestedRecTy = { A : int; }

type NestdRecTy = { B : string; C : AnotherNestedRecTy; }

type RecTy = { D : NestdRecTy; E : string option; }

let t1 = { D = { B = "t1"; C = { A = 1; } }; E = None; }

let t2 = { t1 with D.B = "t2" }

let t3 = { t2 with D.C.A = 3 }

// Changed fields t1 to t2
if t1.D.B <> "t1" || t2.D.B <> "t2" then exit 1

// Fields cloned t1 to t2
if t1.E <> t2.E || t1.D.C <> t2.D.C then exit 1

// Changed fields t2 to t3
if t2.D.C.A <> 1 || t3.D.C.A <> 3 then exit 1

// Fields cloned t2 to t3
if t2.E <> t3.E || t2.D.B <> t3.D.B then exit 1

exit 0
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// #Conformance #TypesAndModules #Records
#light

// Verify cloning and updating of fields accessed through TypeName using nested copy and update syntax

type AnotherNestedRecTy = { A : int; }

type NestdRecTy = { B : string; C : AnotherNestedRecTy; }

type RecTy = { D : NestdRecTy; E : string option; }

let t1 = { D = { B = "t1"; C = { A = 1; } }; E = None; }

// TypeName.FieldName access
let t2 = { t1 with RecTy.D.B = "t2"; }

let t3 = { t2 with RecTy.D.B = "t3"; RecTy.D.C.A = 3; }

// Changed Fields t1 to t2
if t1.D.B <> "t1" || t2.D.B <> "t2" then exit 1

// Fields Cloned t1 to t2
if t2.D.C.A <> t1.D.C.A || t2.E <> t1.E then exit 1

// Changed Fields t2 to t3
if t3.D.B <> "t3" || t2.D.C.A <> 1 || t3.D.C.A <> 3 then exit 1

// Fields Cloned t2 to t3
if t3.E <> t2.E then exit 1

exit 0
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// #Conformance #TypesAndModules #Records
#light

// Verify cloning and updating of fields with ambiguities between TypeName and FieldName using nested copy and update syntax

type AnotherNestedRecTy = { A : int; }

type NestdRecTy = { B : string; AnotherNestedRecTy : AnotherNestedRecTy; }

type RecTy = { NestdRecTy : NestdRecTy; E : string option; }


let t1 = { RecTy.NestdRecTy = { B = "t1"; AnotherNestedRecTy = { A = 1; } }; E = None; }

// Ambiguous access
let t2 = { t1 with NestdRecTy.B = "t2" }

let t3 = { t2 with NestdRecTy.AnotherNestedRecTy.A = 3 }

// Changed Fields t1 to t2
if t1.NestdRecTy.B <> "t1" || t2.NestdRecTy.B <> "t2" then exit 1

// Fields Cloned t1 to t2
if t2.E <> t1.E || t2.NestdRecTy.AnotherNestedRecTy.A <> t1.NestdRecTy.AnotherNestedRecTy.A then exit 1

// Changed Fields t2 to t3
if t3.NestdRecTy.AnotherNestedRecTy.A <> 3 then exit 1

// Fields Cloned t2 to t3
if t3.E <> t2.E || t3.NestdRecTy.B <> t2.NestdRecTy.B then exit 1

exit 0
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// #Conformance #TypesAndModules #Records
#light

// Verify cloning and updating of fields accessed through ModuleName using nested copy and update syntax
module Test =
module M =
type AnotherNestedRecTy = { A : int; }

type NestdRecTy = { B : string; C : AnotherNestedRecTy; }

type RecTy = { D : NestdRecTy; E : string option; }


let t1 = { M.RecTy.D = { M.B = "t1"; M.C = { M.A = 1; } }; M.E = None; }

// Module.FieldName access
let t2 = { t1 with M.D.B = "t2"; M.D.C.A = 2; }

// Module.TypeName.FieldName access
let t3 = { t2 with M.RecTy.E = Some "t3"; M.RecTy.D.B = "t3"; }

// Changed Fields t1 to t2
if t1.D.B <> "t1" || t2.D.B <> "t2" || t2.D.C.A <> 2 || t1.D.C.A <> 1 then exit 1

// Fields Cloned t1 to t2
if t2.E <> t1.E then exit 1

// Changed Fields t2 to t3
if t2.E <> None || t3.E <> Some "t3" || t3.D.B <> "t3" then exit 1

// Fields Cloned t2 to t3
if t3.D.C <> t2.D.C then exit 1

exit 0
Loading