Skip to content

Commit 20991fc

Browse files
committed
unified: Support short-hand returns
1 parent 1b0de7e commit 20991fc

4 files changed

Lines changed: 32 additions & 1 deletion

File tree

‎unified/ql/lib/codeql/unified/internal/FacadeAst.qll‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,4 +388,14 @@ module Unified {
388388
/** Gets the number of elements in this array literal. */
389389
int getNumberOfElements() { result = count(this.getAnElement()) }
390390
}
391+
392+
/** Declaration of a getter accessor associated with a variable or field. */
393+
final class GetterDeclaration extends AccessorDeclaration {
394+
GetterDeclaration() { this.getAccessorKind().getValue() = "get" }
395+
}
396+
397+
/** Declaration of a setter accessor associated with a variable or field. */
398+
final class SetterDeclaration extends AccessorDeclaration {
399+
SetterDeclaration() { this.getAccessorKind().getValue() = "set" }
400+
}
391401
}

‎unified/ql/lib/codeql/unified/internal/dataflow/DataFlowPluginSwift.qll‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,5 +67,22 @@ private class SwiftDataFlowPlugin extends DataFlowPlugin {
6767
step.taint() and
6868
node2.isResultValue(call)
6969
)
70+
or
71+
// Short-hand return from a function body, e.g. `func foo() -> String { "42" }`
72+
// This happens when the body contains a single expression and this expression resolves to a value.
73+
// We don't detect precisely whether the expression resolves to a value, but we restrict to cases
74+
// where a spurious flow edge would be harmless.
75+
exists(Callable callable, Block body |
76+
(
77+
callable instanceof FunctionExpr or
78+
callable instanceof FunctionDeclaration or
79+
callable instanceof GetterDeclaration
80+
) and
81+
body = callable.getBody() and
82+
not exists(body.getStmt(1)) and // Check that only one statement exists
83+
node1.isResultValue(body.getStmt(0)) and
84+
step.value() and
85+
node2.isReturnNode(callable)
86+
)
7087
}
7188
}

‎unified/ql/test/library-tests/dataflow/short-return.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,5 @@ func sink(_ s: Any) -> String { return "" }
55

66
func t1() {
77
func getValue() -> String { source("t1.1") }
8-
sink(getValue()) // $ MISSING: hasValueFlow=t1.1
8+
sink(getValue()) // $ hasValueFlow=t1.1
99
}

‎unified/ql/test/library-tests/dataflow/test.expected‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ edges
227227
| implicit-self.swift:85:9:85:14 | [incoming] ... .x | implicit-self.swift:85:9:85:12 | [post] self [x] | provenance | |
228228
| implicit-self.swift:85:29:85:43 | source(...) | implicit-self.swift:85:9:85:14 | [incoming] ... .x | provenance | |
229229
| implicit-self.swift:87:14:87:17 | self [x] | implicit-self.swift:87:14:87:19 | ... .x | provenance | |
230+
| short-return.swift:7:33:7:46 | source(...) | short-return.swift:8:10:8:19 | getValue(...) | provenance | |
230231
| test.swift:17:10:17:23 | source(...) | test.swift:17:10:17:32 | ... + ... | provenance | |
231232
| test.swift:18:19:18:32 | source(...) | test.swift:18:10:18:32 | ... + ... | provenance | |
232233
| test.swift:20:13:20:26 | source(...) | test.swift:20:10:20:33 | StringInterpolationExpr | provenance | |
@@ -653,6 +654,8 @@ nodes
653654
| implicit-self.swift:86:14:86:14 | x | semmle.label | x |
654655
| implicit-self.swift:87:14:87:17 | self [x] | semmle.label | self [x] |
655656
| implicit-self.swift:87:14:87:19 | ... .x | semmle.label | ... .x |
657+
| short-return.swift:7:33:7:46 | source(...) | semmle.label | source(...) |
658+
| short-return.swift:8:10:8:19 | getValue(...) | semmle.label | getValue(...) |
656659
| test.swift:9:10:9:21 | source(...) | semmle.label | source(...) |
657660
| test.swift:17:10:17:23 | source(...) | semmle.label | source(...) |
658661
| test.swift:17:10:17:32 | ... + ... | semmle.label | ... + ... |
@@ -926,6 +929,7 @@ testFailures
926929
| implicit-self.swift:80:14:80:19 | ... .x | implicit-self.swift:78:24:78:38 | source(...) | implicit-self.swift:80:14:80:19 | ... .x | $@ | implicit-self.swift:78:24:78:38 | source(...) | source(...) |
927930
| implicit-self.swift:86:14:86:14 | x | implicit-self.swift:85:29:85:43 | source(...) | implicit-self.swift:86:14:86:14 | x | $@ | implicit-self.swift:85:29:85:43 | source(...) | source(...) |
928931
| implicit-self.swift:87:14:87:19 | ... .x | implicit-self.swift:85:29:85:43 | source(...) | implicit-self.swift:87:14:87:19 | ... .x | $@ | implicit-self.swift:85:29:85:43 | source(...) | source(...) |
932+
| short-return.swift:8:10:8:19 | getValue(...) | short-return.swift:7:33:7:46 | source(...) | short-return.swift:8:10:8:19 | getValue(...) | $@ | short-return.swift:7:33:7:46 | source(...) | source(...) |
929933
| test.swift:9:10:9:21 | source(...) | test.swift:9:10:9:21 | source(...) | test.swift:9:10:9:21 | source(...) | $@ | test.swift:9:10:9:21 | source(...) | source(...) |
930934
| test.swift:17:10:17:32 | ... + ... | test.swift:17:10:17:23 | source(...) | test.swift:17:10:17:32 | ... + ... | $@ | test.swift:17:10:17:23 | source(...) | source(...) |
931935
| test.swift:18:10:18:32 | ... + ... | test.swift:18:19:18:32 | source(...) | test.swift:18:10:18:32 | ... + ... | $@ | test.swift:18:19:18:32 | source(...) | source(...) |

0 commit comments

Comments
 (0)