Skip to content

Commit 5734e47

Browse files
rdmarsh2rdmarsh2
authored
Merge pull request github#14014 from rdmarsh2/rdmarsh2/swift/keypath-force-steps
Swift: flow through keypath optional components
2 parents afe1e9c + e94781f commit 5734e47

File tree

5 files changed

+108
-1
lines changed

5 files changed

+108
-1
lines changed

‎swift/ql/lib/change-notes/2023-08-21-keypath-optionals.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
5+
* Flow through optional chaining and forced unwrapping in keypaths is now supported by the data flow library.

‎swift/ql/lib/codeql/swift/dataflow/internal/DataFlowPrivate.qll

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,14 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
727727
c.isSingleton(any(Content::ArrayContent ac))
728728
)
729729
or
730+
// creation of an optional via implicit wrapping keypath component
731+
exists(KeyPathComponent component |
732+
component.isOptionalWrapping() and
733+
node1.(KeyPathComponentNodeImpl).getComponent() = component and
734+
node2.(KeyPathReturnNodeImpl).getKeyPathExpr() = component.getKeyPathExpr() and
735+
c instanceof OptionalSomeContentSet
736+
)
737+
or
730738
FlowSummaryImpl::Private::Steps::summaryStoreStep(node1.(FlowSummaryNode).getSummaryNode(), c,
731739
node2.(FlowSummaryNode).getSummaryNode())
732740
}
@@ -789,6 +797,13 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
789797
or
790798
c.isSingleton(any(Content::ArrayContent ac)) and
791799
component.isSubscript()
800+
or
801+
c instanceof OptionalSomeContentSet and
802+
(
803+
component.isOptionalForcing()
804+
or
805+
component.isOptionalChaining()
806+
)
792807
)
793808
|
794809
// the next node is either the next element in the chain

‎swift/ql/test/library-tests/dataflow/dataflow/DataFlow.expected

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
edges
2+
| file://:0:0:0:0 | KeyPathComponent | test.swift:663:13:663:29 | exit #keyPath(...) [some:0] |
23
| file://:0:0:0:0 | self [a, x] | file://:0:0:0:0 | .a [x] |
34
| file://:0:0:0:0 | self [str] | file://:0:0:0:0 | .str |
45
| file://:0:0:0:0 | self [v2, some:0] | file://:0:0:0:0 | .v2 [some:0] |
@@ -333,6 +334,23 @@ edges
333334
| test.swift:648:20:648:22 | KeyPathComponent [Array element] | test.swift:648:13:648:22 | exit #keyPath(...) |
334335
| test.swift:649:15:649:15 | array [Array element] | test.swift:648:13:648:22 | enter #keyPath(...) [Array element] |
335336
| test.swift:649:15:649:15 | array [Array element] | test.swift:649:15:649:31 | \\...[...] |
337+
| test.swift:655:8:655:12 | s [some:0, x] | test.swift:656:14:656:14 | s [some:0, x] |
338+
| test.swift:656:5:656:5 | [post] self [s, some:0, x] | test.swift:655:3:657:3 | self[return] [s, some:0, x] |
339+
| test.swift:656:14:656:14 | s [some:0, x] | test.swift:656:5:656:5 | [post] self [s, some:0, x] |
340+
| test.swift:661:13:661:26 | call to S.init(x:) [x] | test.swift:662:29:662:29 | s [x] |
341+
| test.swift:661:18:661:25 | call to source() | test.swift:617:8:617:11 | x |
342+
| test.swift:661:18:661:25 | call to source() | test.swift:661:13:661:26 | call to S.init(x:) [x] |
343+
| test.swift:662:14:662:30 | call to S2_Optional.init(s:) [s, some:0, x] | test.swift:664:15:664:15 | s2 [s, some:0, x] |
344+
| test.swift:662:29:662:29 | s [some:0, x] | test.swift:655:8:655:12 | s [some:0, x] |
345+
| test.swift:662:29:662:29 | s [some:0, x] | test.swift:662:14:662:30 | call to S2_Optional.init(s:) [s, some:0, x] |
346+
| test.swift:662:29:662:29 | s [x] | test.swift:662:29:662:29 | s [some:0, x] |
347+
| test.swift:663:13:663:29 | enter #keyPath(...) [s, some:0, x] | test.swift:663:26:663:26 | KeyPathComponent [s, some:0, x] |
348+
| test.swift:663:26:663:26 | KeyPathComponent [s, some:0, x] | test.swift:663:27:663:27 | KeyPathComponent [some:0, x] |
349+
| test.swift:663:27:663:27 | KeyPathComponent [some:0, x] | test.swift:663:29:663:29 | KeyPathComponent [x] |
350+
| test.swift:663:29:663:29 | KeyPathComponent [x] | file://:0:0:0:0 | KeyPathComponent |
351+
| test.swift:664:15:664:15 | s2 [s, some:0, x] | test.swift:663:13:663:29 | enter #keyPath(...) [s, some:0, x] |
352+
| test.swift:664:15:664:15 | s2 [s, some:0, x] | test.swift:664:15:664:28 | \\...[...] [some:0] |
353+
| test.swift:664:15:664:28 | \\...[...] [some:0] | test.swift:664:15:664:29 | ...! |
336354
| test.swift:668:13:668:20 | call to source() | test.swift:676:15:676:15 | y |
337355
| test.swift:678:9:678:16 | call to source() | test.swift:680:11:680:11 | x |
338356
| test.swift:678:9:678:16 | call to source() | test.swift:681:15:681:15 | x |
@@ -404,6 +422,19 @@ edges
404422
| test.swift:756:15:756:19 | .v2 [some:0] | test.swift:756:15:756:21 | ...! |
405423
| test.swift:757:15:757:15 | mo1 [v3] | test.swift:732:9:732:9 | self [v3] |
406424
| test.swift:757:15:757:15 | mo1 [v3] | test.swift:757:15:757:19 | .v3 |
425+
| test.swift:764:13:764:26 | call to S.init(x:) [x] | test.swift:765:29:765:29 | s [x] |
426+
| test.swift:764:18:764:25 | call to source() | test.swift:617:8:617:11 | x |
427+
| test.swift:764:18:764:25 | call to source() | test.swift:764:13:764:26 | call to S.init(x:) [x] |
428+
| test.swift:765:14:765:30 | call to S2_Optional.init(s:) [s, some:0, x] | test.swift:767:15:767:15 | s2 [s, some:0, x] |
429+
| test.swift:765:29:765:29 | s [some:0, x] | test.swift:655:8:655:12 | s [some:0, x] |
430+
| test.swift:765:29:765:29 | s [some:0, x] | test.swift:765:14:765:30 | call to S2_Optional.init(s:) [s, some:0, x] |
431+
| test.swift:765:29:765:29 | s [x] | test.swift:765:29:765:29 | s [some:0, x] |
432+
| test.swift:766:13:766:29 | enter #keyPath(...) [s, some:0, x] | test.swift:766:26:766:26 | KeyPathComponent [s, some:0, x] |
433+
| test.swift:766:26:766:26 | KeyPathComponent [s, some:0, x] | test.swift:766:26:766:26 | KeyPathComponent [some:0, x] |
434+
| test.swift:766:26:766:26 | KeyPathComponent [some:0, x] | test.swift:766:29:766:29 | KeyPathComponent [x] |
435+
| test.swift:766:29:766:29 | KeyPathComponent [x] | test.swift:766:13:766:29 | exit #keyPath(...) |
436+
| test.swift:767:15:767:15 | s2 [s, some:0, x] | test.swift:766:13:766:29 | enter #keyPath(...) [s, some:0, x] |
437+
| test.swift:767:15:767:15 | s2 [s, some:0, x] | test.swift:767:15:767:28 | \\...[...] |
407438
nodes
408439
| file://:0:0:0:0 | .a [x] | semmle.label | .a [x] |
409440
| file://:0:0:0:0 | .str | semmle.label | .str |
@@ -412,6 +443,7 @@ nodes
412443
| file://:0:0:0:0 | .v3 | semmle.label | .v3 |
413444
| file://:0:0:0:0 | .x | semmle.label | .x |
414445
| file://:0:0:0:0 | .x [some:0] | semmle.label | .x [some:0] |
446+
| file://:0:0:0:0 | KeyPathComponent | semmle.label | KeyPathComponent |
415447
| file://:0:0:0:0 | [post] self [v2, some:0] | semmle.label | [post] self [v2, some:0] |
416448
| file://:0:0:0:0 | [post] self [v2] | semmle.label | [post] self [v2] |
417449
| file://:0:0:0:0 | [post] self [v3] | semmle.label | [post] self [v3] |
@@ -771,6 +803,23 @@ nodes
771803
| test.swift:648:20:648:22 | KeyPathComponent [Array element] | semmle.label | KeyPathComponent [Array element] |
772804
| test.swift:649:15:649:15 | array [Array element] | semmle.label | array [Array element] |
773805
| test.swift:649:15:649:31 | \\...[...] | semmle.label | \\...[...] |
806+
| test.swift:655:3:657:3 | self[return] [s, some:0, x] | semmle.label | self[return] [s, some:0, x] |
807+
| test.swift:655:8:655:12 | s [some:0, x] | semmle.label | s [some:0, x] |
808+
| test.swift:656:5:656:5 | [post] self [s, some:0, x] | semmle.label | [post] self [s, some:0, x] |
809+
| test.swift:656:14:656:14 | s [some:0, x] | semmle.label | s [some:0, x] |
810+
| test.swift:661:13:661:26 | call to S.init(x:) [x] | semmle.label | call to S.init(x:) [x] |
811+
| test.swift:661:18:661:25 | call to source() | semmle.label | call to source() |
812+
| test.swift:662:14:662:30 | call to S2_Optional.init(s:) [s, some:0, x] | semmle.label | call to S2_Optional.init(s:) [s, some:0, x] |
813+
| test.swift:662:29:662:29 | s [some:0, x] | semmle.label | s [some:0, x] |
814+
| test.swift:662:29:662:29 | s [x] | semmle.label | s [x] |
815+
| test.swift:663:13:663:29 | enter #keyPath(...) [s, some:0, x] | semmle.label | enter #keyPath(...) [s, some:0, x] |
816+
| test.swift:663:13:663:29 | exit #keyPath(...) [some:0] | semmle.label | exit #keyPath(...) [some:0] |
817+
| test.swift:663:26:663:26 | KeyPathComponent [s, some:0, x] | semmle.label | KeyPathComponent [s, some:0, x] |
818+
| test.swift:663:27:663:27 | KeyPathComponent [some:0, x] | semmle.label | KeyPathComponent [some:0, x] |
819+
| test.swift:663:29:663:29 | KeyPathComponent [x] | semmle.label | KeyPathComponent [x] |
820+
| test.swift:664:15:664:15 | s2 [s, some:0, x] | semmle.label | s2 [s, some:0, x] |
821+
| test.swift:664:15:664:28 | \\...[...] [some:0] | semmle.label | \\...[...] [some:0] |
822+
| test.swift:664:15:664:29 | ...! | semmle.label | ...! |
774823
| test.swift:668:13:668:20 | call to source() | semmle.label | call to source() |
775824
| test.swift:676:15:676:15 | y | semmle.label | y |
776825
| test.swift:678:9:678:16 | call to source() | semmle.label | call to source() |
@@ -849,6 +898,18 @@ nodes
849898
| test.swift:756:15:756:21 | ...! | semmle.label | ...! |
850899
| test.swift:757:15:757:15 | mo1 [v3] | semmle.label | mo1 [v3] |
851900
| test.swift:757:15:757:19 | .v3 | semmle.label | .v3 |
901+
| test.swift:764:13:764:26 | call to S.init(x:) [x] | semmle.label | call to S.init(x:) [x] |
902+
| test.swift:764:18:764:25 | call to source() | semmle.label | call to source() |
903+
| test.swift:765:14:765:30 | call to S2_Optional.init(s:) [s, some:0, x] | semmle.label | call to S2_Optional.init(s:) [s, some:0, x] |
904+
| test.swift:765:29:765:29 | s [some:0, x] | semmle.label | s [some:0, x] |
905+
| test.swift:765:29:765:29 | s [x] | semmle.label | s [x] |
906+
| test.swift:766:13:766:29 | enter #keyPath(...) [s, some:0, x] | semmle.label | enter #keyPath(...) [s, some:0, x] |
907+
| test.swift:766:13:766:29 | exit #keyPath(...) | semmle.label | exit #keyPath(...) |
908+
| test.swift:766:26:766:26 | KeyPathComponent [s, some:0, x] | semmle.label | KeyPathComponent [s, some:0, x] |
909+
| test.swift:766:26:766:26 | KeyPathComponent [some:0, x] | semmle.label | KeyPathComponent [some:0, x] |
910+
| test.swift:766:29:766:29 | KeyPathComponent [x] | semmle.label | KeyPathComponent [x] |
911+
| test.swift:767:15:767:15 | s2 [s, some:0, x] | semmle.label | s2 [s, some:0, x] |
912+
| test.swift:767:15:767:28 | \\...[...] | semmle.label | \\...[...] |
852913
subpaths
853914
| test.swift:75:22:75:22 | x | test.swift:65:16:65:28 | arg1 | test.swift:65:1:70:1 | arg2[return] | test.swift:75:32:75:32 | [post] y |
854915
| test.swift:114:19:114:19 | arg | test.swift:109:9:109:14 | arg | test.swift:110:12:110:12 | arg | test.swift:114:12:114:22 | call to ... |
@@ -891,12 +952,18 @@ subpaths
891952
| test.swift:641:18:641:18 | s [x] | test.swift:634:8:634:11 | s [x] | test.swift:634:3:636:3 | self[return] [s, x] | test.swift:641:12:641:19 | call to S2.init(s:) [s, x] |
892953
| test.swift:643:13:643:13 | s2 [s, x] | test.swift:642:11:642:17 | enter #keyPath(...) [s, x] | test.swift:642:11:642:17 | exit #keyPath(...) | test.swift:643:13:643:26 | \\...[...] |
893954
| test.swift:649:15:649:15 | array [Array element] | test.swift:648:13:648:22 | enter #keyPath(...) [Array element] | test.swift:648:13:648:22 | exit #keyPath(...) | test.swift:649:15:649:31 | \\...[...] |
955+
| test.swift:661:18:661:25 | call to source() | test.swift:617:8:617:11 | x | test.swift:617:3:619:3 | self[return] [x] | test.swift:661:13:661:26 | call to S.init(x:) [x] |
956+
| test.swift:662:29:662:29 | s [some:0, x] | test.swift:655:8:655:12 | s [some:0, x] | test.swift:655:3:657:3 | self[return] [s, some:0, x] | test.swift:662:14:662:30 | call to S2_Optional.init(s:) [s, some:0, x] |
957+
| test.swift:664:15:664:15 | s2 [s, some:0, x] | test.swift:663:13:663:29 | enter #keyPath(...) [s, some:0, x] | test.swift:663:13:663:29 | exit #keyPath(...) [some:0] | test.swift:664:15:664:28 | \\...[...] [some:0] |
894958
| test.swift:746:14:746:21 | call to source() | test.swift:731:9:731:9 | value | file://:0:0:0:0 | [post] self [v2] | test.swift:746:5:746:5 | [post] mo1 [v2] |
895959
| test.swift:746:14:746:21 | call to source() [some:0] | test.swift:731:9:731:9 | value [some:0] | file://:0:0:0:0 | [post] self [v2, some:0] | test.swift:746:5:746:5 | [post] mo1 [v2, some:0] |
896960
| test.swift:747:14:747:21 | call to source() | test.swift:732:9:732:9 | value | file://:0:0:0:0 | [post] self [v3] | test.swift:747:5:747:5 | [post] mo1 [v3] |
897961
| test.swift:756:15:756:15 | mo1 [v2, some:0] | test.swift:731:9:731:9 | self [v2, some:0] | file://:0:0:0:0 | .v2 [some:0] | test.swift:756:15:756:19 | .v2 [some:0] |
898962
| test.swift:756:15:756:15 | mo1 [v2] | test.swift:731:9:731:9 | self [v2] | file://:0:0:0:0 | .v2 | test.swift:756:15:756:19 | .v2 |
899963
| test.swift:757:15:757:15 | mo1 [v3] | test.swift:732:9:732:9 | self [v3] | file://:0:0:0:0 | .v3 | test.swift:757:15:757:19 | .v3 |
964+
| test.swift:764:18:764:25 | call to source() | test.swift:617:8:617:11 | x | test.swift:617:3:619:3 | self[return] [x] | test.swift:764:13:764:26 | call to S.init(x:) [x] |
965+
| test.swift:765:29:765:29 | s [some:0, x] | test.swift:655:8:655:12 | s [some:0, x] | test.swift:655:3:657:3 | self[return] [s, some:0, x] | test.swift:765:14:765:30 | call to S2_Optional.init(s:) [s, some:0, x] |
966+
| test.swift:767:15:767:15 | s2 [s, some:0, x] | test.swift:766:13:766:29 | enter #keyPath(...) [s, some:0, x] | test.swift:766:13:766:29 | exit #keyPath(...) | test.swift:767:15:767:28 | \\...[...] |
900967
#select
901968
| test.swift:7:15:7:15 | t1 | test.swift:6:19:6:26 | call to source() | test.swift:7:15:7:15 | t1 | result |
902969
| test.swift:9:15:9:15 | t1 | test.swift:6:19:6:26 | call to source() | test.swift:9:15:9:15 | t1 | result |
@@ -980,6 +1047,7 @@ subpaths
9801047
| test.swift:628:13:628:32 | \\...[...] | test.swift:623:16:623:23 | call to source() | test.swift:628:13:628:32 | \\...[...] | result |
9811048
| test.swift:643:13:643:26 | \\...[...] | test.swift:640:16:640:23 | call to source() | test.swift:643:13:643:26 | \\...[...] | result |
9821049
| test.swift:649:15:649:31 | \\...[...] | test.swift:647:18:647:25 | call to source() | test.swift:649:15:649:31 | \\...[...] | result |
1050+
| test.swift:664:15:664:29 | ...! | test.swift:661:18:661:25 | call to source() | test.swift:664:15:664:29 | ...! | result |
9831051
| test.swift:676:15:676:15 | y | test.swift:668:13:668:20 | call to source() | test.swift:676:15:676:15 | y | result |
9841052
| test.swift:681:15:681:15 | x | test.swift:678:9:678:16 | call to source() | test.swift:681:15:681:15 | x | result |
9851053
| test.swift:682:15:682:15 | y | test.swift:678:9:678:16 | call to source() | test.swift:682:15:682:15 | y | result |
@@ -996,3 +1064,4 @@ subpaths
9961064
| test.swift:754:15:754:15 | v3 | test.swift:744:10:744:17 | call to source() | test.swift:754:15:754:15 | v3 | result |
9971065
| test.swift:756:15:756:21 | ...! | test.swift:746:14:746:21 | call to source() | test.swift:756:15:756:21 | ...! | result |
9981066
| test.swift:757:15:757:19 | .v3 | test.swift:747:14:747:21 | call to source() | test.swift:757:15:757:19 | .v3 | result |
1067+
| test.swift:767:15:767:28 | \\...[...] | test.swift:764:18:764:25 | call to source() | test.swift:767:15:767:28 | \\...[...] | result |

‎swift/ql/test/library-tests/dataflow/dataflow/LocalFlow.expected

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,7 @@
746746
| test.swift:663:9:663:9 | f | test.swift:663:9:663:9 | SSA def(f) |
747747
| test.swift:663:13:663:29 | #keyPath(...) | test.swift:663:9:663:9 | f |
748748
| test.swift:663:13:663:29 | enter #keyPath(...) | test.swift:663:26:663:26 | KeyPathComponent |
749+
| test.swift:664:15:664:28 | \\...[...] | test.swift:664:15:664:29 | ...! |
749750
| test.swift:668:9:668:9 | SSA def(x) | test.swift:672:9:672:9 | x |
750751
| test.swift:668:9:668:9 | x | test.swift:668:9:668:9 | SSA def(x) |
751752
| test.swift:668:13:668:20 | call to source() | test.swift:668:9:668:9 | x |
@@ -913,3 +914,13 @@
913914
| test.swift:759:15:759:15 | mo2 | test.swift:760:15:760:15 | mo2 |
914915
| test.swift:759:15:759:20 | .v2 | test.swift:759:15:759:22 | ...! |
915916
| test.swift:760:15:760:15 | mo2 | test.swift:760:15:760:18 | ...! |
917+
| test.swift:764:9:764:9 | SSA def(s) | test.swift:765:29:765:29 | s |
918+
| test.swift:764:9:764:9 | s | test.swift:764:9:764:9 | SSA def(s) |
919+
| test.swift:764:13:764:26 | call to S.init(x:) | test.swift:764:9:764:9 | s |
920+
| test.swift:765:9:765:9 | SSA def(s2) | test.swift:767:15:767:15 | s2 |
921+
| test.swift:765:9:765:9 | s2 | test.swift:765:9:765:9 | SSA def(s2) |
922+
| test.swift:765:14:765:30 | call to S2_Optional.init(s:) | test.swift:765:9:765:9 | s2 |
923+
| test.swift:766:9:766:9 | SSA def(f) | test.swift:767:27:767:27 | f |
924+
| test.swift:766:9:766:9 | f | test.swift:766:9:766:9 | SSA def(f) |
925+
| test.swift:766:13:766:29 | #keyPath(...) | test.swift:766:9:766:9 | f |
926+
| test.swift:766:13:766:29 | enter #keyPath(...) | test.swift:766:26:766:26 | KeyPathComponent |

‎swift/ql/test/library-tests/dataflow/dataflow/test.swift

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,7 @@ func testOptionalKeyPath() {
661661
let s = S(x: source())
662662
let s2 = S2_Optional(s: s)
663663
let f = \S2_Optional.s?.x
664-
sink(opt: s2[keyPath: f]) // $ MISSING: flow=661
664+
sink(arg: s2[keyPath: f]!) // $ flow=661
665665
}
666666

667667
func testSwap() {
@@ -759,3 +759,10 @@ func testWriteOptional() {
759759
sink(arg: mo2!.v2!) // $ MISSING:flow=749
760760
sink(arg: mo2!.v3) // $ MISSING:flow=750
761761
}
762+
763+
func testOptionalKeyPathForce() {
764+
let s = S(x: source())
765+
let s2 = S2_Optional(s: s)
766+
let f = \S2_Optional.s!.x
767+
sink(arg: s2[keyPath: f]) // $ flow=764
768+
}

0 commit comments

Comments
 (0)