Repository navigation
Checker: check record spreads once - #20867
auduchinok wants to merge 3 commits into
Conversation
✅ Release notes checked
|
13a9f7a to
38f8254
Compare
| /// | ||
| /// Used when determining if a structural type supports structural comparison. | ||
| let private GetStructuralElementsOfTyconDefn (cenv: cenv) env tpenv (MutRecDefnsPhase1DataForTycon(_, synTyconRepr, _, _, _, _)) tycon = | ||
| let cenv = { cenv with tcSink = TcResultsSink.NoSink } |
There was a problem hiding this comment.
I did not know about this trick. Thank you for fixing this.
There was a problem hiding this comment.
In this particular case it was about the duplicate field types reporting. The fields themselves are not reported due to the spread expansion changes.
| [<Fact>] | ||
| let ``Record spread 02 - Expression`` () = | ||
| let _, checkResults = | ||
| getParseAndCheckResultsPreview """ | ||
| type R1 = { A: int; B: int } | ||
| let r = { A = 1; B = 2 } | ||
| let _ = { ...r; B = 2 } | ||
| """ | ||
| symbolUses checkResults | ||
| |> shouldEqual | ||
| [ "int", ((2, 15), (2, 18)), [ "type" ] | ||
| "int", ((2, 23), (2, 26)), [ "type" ] | ||
| "field A", ((2, 12), (2, 13)), [ "defn" ] | ||
| "field B", ((2, 20), (2, 21)), [ "defn" ] | ||
| "R1", ((2, 5), (2, 7)), [ "defn" ] | ||
| "field A", ((3, 10), (3, 11)), [] | ||
| "field B", ((3, 17), (3, 18)), [] | ||
| "val r", ((3, 4), (3, 5)), [ "defn" ] | ||
| "val r", ((4, 13), (4, 14)), [] | ||
| "field B", ((4, 16), (4, 17)), [] | ||
| "Test", ((1, 0), (1, 0)), [ "defn" ] ] |
There was a problem hiding this comment.
If I'm understanding this right, before this PR, field A would show as having a usage on line 4 via the spread in { ...r; B = 2}, but now it does not. Is that correct?
Perhaps that was a misunderstanding on my part as to what symbol usage means from a tooling standpoint. I believe I was thinking about it from the layman's "find all references" point of view, where field A is "used" in the spread even though the symbol is not actually uttered.
There was a problem hiding this comment.
Yes, that is correct. We've been trying hard to remove anything not coming from the actual sources in this API.
I hope it doesn't break "find all references" in VS or Ionide and there are tests covering it.
Fixes #20676.
GetAllSymbolUsesInFile