Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
7a89b0b
unified: Add tests with accessors
asgerf Oct 23, 2026
4ee78d4
unified: Allow accessors to be seen as instance members
asgerf Oct 23, 2026
68ab96a
unified: Add DataFlowCalls for getter and setter calls
asgerf Oct 23, 2026
34bc913
unified: Resolve calls statically
asgerf Oct 23, 2026
a36d0da
unified: Add out-node from getter calls
asgerf Oct 23, 2026
dceea31
unified: Synthesize setter parameters and return type in accessors
asgerf Oct 23, 2026
f777b69
unified: Pass the incoming value into setter calls
asgerf Oct 23, 2026
db88e9c
unified: Ensure 'static' modifier is preserved for accessors
asgerf Oct 23, 2026
f42b4e7
unified: Add MemberAccess as union of MemberAccessExpr and Unqualifie…
asgerf Oct 23, 2026
e8afe95
unified: Also resolve accessors on instances
asgerf Oct 23, 2026
70c8a94
unified: Make accessors introduce LocalVariables too
asgerf Oct 23, 2026
e294170
unified: Handle capturing local getters/setters
asgerf Oct 24, 2026
2da2d62
unified: Propagate receiver argument into accessor calls
asgerf Oct 24, 2026
2d23257
unified: Allow self-return via receiver parameter
asgerf Oct 24, 2026
e3e5c89
unified: Add more capture tests
asgerf Oct 24, 2026
f84f31d
unified: Add test with chain of accessors
asgerf Oct 25, 2026
59126d9
unified: Split up hasControlFlowPosition()
asgerf Oct 26, 2026
e4a8eab
unified: Associate CFG nodes with implicit getter/setter arguments
asgerf Oct 26, 2026
2ab5b57
unified: Use hasControlFlowPosition in CaptureSsa
asgerf Oct 26, 2026
62d6a59
unified: Move ad-hoc aliasing logic into a step
asgerf Oct 26, 2026
7fcf818
unified: Add basic escape
asgerf Oct 26, 2026
e496672
unified: Update nodes/edges in test output
asgerf Oct 26, 2026
693189c
unified: Tolerate missing post-update for RHS
asgerf Oct 26, 2026
c522a9b
unified: Add test for explicitly-named setter parameter
asgerf Oct 26, 2026
c225984
Update isEscape predicate to include incoming values
asgerf Oct 27, 2026
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
2 changes: 1 addition & 1 deletion unified/extractor/ast_types.yml
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ named:
name_node: identifier
accessor_kind: accessor_kind
parameter*: parameter
type?: expr
return_type?: expr
body?: block

# "get", "set", or a language-specific kind like "didSet"
Expand Down
92 changes: 65 additions & 27 deletions unified/extractor/src/languages/swift/swift.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ struct SwiftContext {
/// `variableDecl` rule before translating the accessor block; read by the
/// inner `accessorDecl` rules to name each `accessor_declaration`.
property_name_node: Option<yeast::Id>,
/// Translated type node for the property type. Set (for computed
/// properties) by the accessor-bearing `variableDecl` rule; read by the
/// inner `accessorDecl` rules. Left `None` for stored properties with
/// observers, so their `willSet`/`didSet` accessors carry no type.
/// Translated type node for the property type. Set by the
/// accessor-bearing `variableDecl` rule; read by the inner `accessorDecl`
/// rules for both the accessor type and its `newValue`/`oldValue`
/// parameter type.
property_type: Option<yeast::Id>,
/// Translated outer modifiers to attach to each child of a flattening
/// outer rule — e.g. the `let`/`var` binding modifier on each
/// `patternBinding` of a `variableDecl`, or the binding modifier on each
/// accessor of a property.
/// outer rule — e.g. the `let`/`var` binding modifier, attributes and
/// modifiers (`static`, `public`, …) on each `patternBinding` of a
/// `variableDecl`, or on each accessor of a property.
outer_modifiers: Vec<yeast::Id>,
/// True when the current child of a flattening outer rule is not
/// the first one — its inner rule should emit a
Expand Down Expand Up @@ -325,6 +325,8 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
// block.
rule!(
(variableDecl
attributes: _* @attrs
modifiers: _* @mods
bindingSpecifier: @@spec
bindings: (patternBinding
pattern: (identifierPattern identifier: @@name)
Expand All @@ -333,8 +335,10 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
=>
(accessor_declaration
modifier: (modifier #{spec})
modifier: {attrs}
modifier: {mods}
name_node: (identifier #{name})
type: {ty}
return_type: {ty}
accessor_kind: {ctx.literal_at_start_of("accessor_kind", "get", body)}
body: {body})
),
Expand All @@ -345,16 +349,18 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
// * With an initializer (`var x: T = e { willSet {…} didSet {…} }`) it is
// a *stored* property with observers: emit the backing
// `variable_declaration` first, then one `accessor_declaration` per
// observer (observers carry no type).
// observer.
// * Without an initializer (`var v: T { get set }`, incl. protocol
// requirements) it is a *computed* property: no backing variable; the
// type is published so the get/set accessors carry it.
// requirements) it is a *computed* property: no backing variable.
//
// In both cases the first emitted declaration is unchained and every
// In both cases the type is published so every accessor carries it, and
// the first emitted declaration is unchained and every
// subsequent one is tagged `chained_declaration` (the `!result.is_empty()`
// test). Must precede the plain `variableDecl` rules.
rule!(
(variableDecl
attributes: _* @attrs
modifiers: _* @mods
bindingSpecifier: @@spec
bindings: (patternBinding
pattern: (identifierPattern identifier: @@name)
Expand All @@ -363,8 +369,14 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
accessorBlock: (accessorBlock accessors: (accessorDecl)+ @@accessors)))
=>
member* {
ctx.outer_modifiers = vec![tree!((modifier #{spec}))];
// The binding (`let`/`var`) leads, then attributes then modifiers,
// matching the plain `variableDecl` rule.
ctx.outer_modifiers = std::iter::once(tree!((modifier #{spec})))
.chain(attrs)
.chain(mods)
.collect();
ctx.property_name_node = Some(tree!((identifier #{name})));
ctx.property_type = Some(ty);
let mut result = Vec::new();
if let Some(val) = val {
// Stored property with observers: the initializer is not part
Expand All @@ -380,9 +392,6 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
type: {ty}
value: {val})
));
} else {
// Computed property: the accessors carry the type.
ctx.property_type = Some(ty);
}
for acc in accessors.into_iter() {
ctx.is_chained = !result.is_empty();
Expand All @@ -392,28 +401,56 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
}
),
// Each `accessorDecl` becomes an `accessor_declaration`, reading the
// property name/type and the binding/chained modifiers from `ctx`. The
// property name/type and the modifiers from `ctx`. The
// accessor kind comes straight from the specifier keyword
// (`get`/`set`/`willSet`/`didSet`). The body is optional: a body-bearing
// accessor (a computed getter/setter, or a `willSet`/`didSet` observer)
// carries the binding modifier and a translated body, whereas a bodyless
// one (a protocol requirement) carries neither. The property context is
// has a translated body, whereas a bodyless one (a protocol requirement)
// has none. The property context is
// read out *before* translating the body, which resets `ctx` so the
// accessor's context does not leak into the body subtree.
//
// `set` and `willSet` accessors get a parameter holding the incoming
// value, and `didSet` gets one holding the previous value. It is named
// explicitly (`set(v)`) or else implicitly `newValue`/`oldValue`.
rule!(
(accessorDecl accessorSpecifier: @@spec body: _? @@body)
(accessorDecl
accessorSpecifier: @@spec
parameters: (accessorParameters name: @@param_name)?
body: _? @@body)
=>
accessor_declaration {
let binding = if body.is_some() {
ctx.outer_modifiers.clone()
} else {
Vec::new()
let spec_text = ctx.source_text(spec);
let implicit_param_name = match spec_text.as_str() {
"set" | "willSet" => Some("newValue"),
"didSet" => Some("oldValue"),
_ => None,
};
let param_ty = ctx.property_type;
let param = match (param_name, implicit_param_name) {
(Some(n), _) => {
Some(tree!((parameter pattern: (identifier #{n}) type: {param_ty})))
}
(None, Some(n)) => {
let n = n.to_string();
Some(tree!((parameter
modifier: (modifier "generated")
pattern: (identifier #{n} location: {spec})
type: {param_ty}
location: {spec})))
}
(None, None) => None,
};
let modifiers = ctx.outer_modifiers.clone();
let chained = chained_modifier(&mut ctx);
let name = ctx
.property_name_node
.ok_or("accessor outside property context")?;
let ty = ctx.property_type;
let return_ty = if spec_text == "get" {
ctx.property_type
} else {
None
};
let body = match body {
Some(block) => {
ctx.reset();
Expand All @@ -422,11 +459,12 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
None => None,
};
tree!((accessor_declaration
modifier: {binding}
modifier: {modifiers}
modifier: {chained}
name_node: {name}
type: {ty}
return_type: {return_ty}
accessor_kind: (accessor_kind #{spec})
parameter: {param}
body: {body}
location: {spec}
location: {body}))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ top_level source="var p: Int {\n ⟨body⟩\n}"
modifier: modifier "var" source="var" (external)
name_node: identifier "p" source="p" (external)
accessor_kind: accessor_kind "get" source="get"
type: identifier "Int" source="Int" (external)
return_type: identifier "Int" source="Int" (external)
body:
block source="{\n ⟨stmt⟩\n }"
stmt:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,11 @@ top_level source="⟨body⟩"
modifier: modifier "var" source="var"
pattern: identifier "h" source="h"
type: identifier "Double" source="Double"
accessor_declaration source="⟨modifier⟩ ⟨name_node⟩: ⟨type⟩ ⟨body⟩"
accessor_declaration source="⟨modifier⟩ ⟨name_node⟩: ⟨return_type⟩ ⟨body⟩"
modifier: modifier "var" source="var"
name_node: identifier "area" source="area"
accessor_kind: accessor_kind "get" source=""
type: identifier "Double" source="Double"
return_type: identifier "Double" source="Double"
body:
block source="{\n ⟨stmt⟩\n }"
stmt:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,19 +120,23 @@ top_level source="⟨body⟩"
modifier: modifier "var" source="var" (external)
name_node: identifier "v" source="v" (external)
accessor_kind: accessor_kind "get" source="get"
type: identifier "Int" source="Int" (external)
return_type: identifier "Int" source="Int" (external)
body:
block source="{ ⟨stmt⟩ }"
stmt:
return_expr source="return ⟨value⟩"
value: identifier "_v" source="_v"
accessor_declaration source="⟨accessor_kind⟩ ⟨body⟩"
accessor_declaration source="⟨accessor_kind⟩⟨parameter⟩ ⟨body⟩"
modifier:
modifier "var" source="var" (external)
modifier "chained_declaration" source=""
name_node: identifier "v" source="v" (external)
accessor_kind: accessor_kind "set" source="set"
type: identifier "Int" source="Int" (external)
parameter:
parameter source="⟨pattern⟩"
modifier: modifier "generated" source=""
type: identifier "Int" source="Int" (external)
pattern: identifier "newValue" source="set"
body:
block source="{ ⟨stmt⟩ }"
stmt:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,19 +111,28 @@ top_level source="⟨body⟩"
name_node: identifier "P" source="P"
member:
accessor_declaration source="⟨accessor_kind⟩"
modifier: modifier "var" source="var" (external)
name_node: identifier "foo" source="foo" (external)
accessor_kind: accessor_kind "get" source="get"
type: identifier "Int" source="Int" (external)
return_type: identifier "Int" source="Int" (external)
accessor_declaration source="⟨accessor_kind⟩"
modifier: modifier "var" source="var" (external)
name_node: identifier "bar" source="bar" (external)
accessor_kind: accessor_kind "get" source="get"
type: identifier "String" source="String" (external)
accessor_declaration source="⟨accessor_kind⟩"
modifier: modifier "chained_declaration" source=""
return_type: identifier "String" source="String" (external)
accessor_declaration source="⟨accessor_kind⟩⟨parameter⟩"
modifier:
modifier "var" source="var" (external)
modifier "chained_declaration" source=""
name_node: identifier "bar" source="bar" (external)
accessor_kind: accessor_kind "set" source="set"
type: identifier "String" source="String" (external)
parameter:
parameter source="⟨pattern⟩"
modifier: modifier "generated" source=""
type: identifier "String" source="String" (external)
pattern: identifier "newValue" source="set"
accessor_declaration source="⟨accessor_kind⟩"
modifier: modifier "var" source="var" (external)
name_node: identifier "count" source="count" (external)
accessor_kind: accessor_kind "get" source="get"
type: identifier "Int" source="Int" (external)
return_type: identifier "Int" source="Int" (external)
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,17 @@ top_level source="⟨body⟩"
pattern: identifier "x" source="x"
type: identifier "Int" source="Int"
value: int_literal "0" source="0"
accessor_declaration source="⟨accessor_kind⟩ ⟨body⟩"
accessor_declaration source="⟨accessor_kind⟩⟨parameter⟩ ⟨body⟩"
modifier:
modifier "var" source="var" (external)
modifier "chained_declaration" source=""
name_node: identifier "x" source="x" (external)
accessor_kind: accessor_kind "willSet" source="willSet"
parameter:
parameter source="⟨pattern⟩"
modifier: modifier "generated" source=""
type: identifier "Int" source="Int" (external)
pattern: identifier "newValue" source="willSet"
body:
block source="{ ⟨stmt⟩ }"
stmt:
Expand All @@ -124,12 +129,17 @@ top_level source="⟨body⟩"
argument:
argument source="⟨value⟩"
value: identifier "newValue" source="newValue"
accessor_declaration source="⟨accessor_kind⟩ ⟨body⟩"
accessor_declaration source="⟨accessor_kind⟩⟨parameter⟩ ⟨body⟩"
modifier:
modifier "var" source="var" (external)
modifier "chained_declaration" source=""
name_node: identifier "x" source="x" (external)
accessor_kind: accessor_kind "didSet" source="didSet"
parameter:
parameter source="⟨pattern⟩"
modifier: modifier "generated" source=""
type: identifier "Int" source="Int" (external)
pattern: identifier "oldValue" source="didSet"
body:
block source="{ ⟨stmt⟩ }"
stmt:
Expand Down
4 changes: 4 additions & 0 deletions unified/ql/consistency-queries/DataFlowConsistency.ql
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ private import codeql.dataflow.internal.DataFlowImplConsistency
module ConsistencyInput implements InputSig<Location, DataFlowInput> {
predicate argHasPostUpdateExclude(DataFlowInput::ArgumentNode n) {
not exists(n.getBasicBlock()) // ignore unreachable data flow nodes
or
// The RHS value passed into a setter call technically needs a post-update node
// because it's an argument to a call, but this rarely matters in practice.
n.isIncomingValue(any(DataFlowCall c).asSetterCall())
}

predicate reverseReadExclude(DataFlow::Node n) {
Expand Down
10 changes: 6 additions & 4 deletions unified/ql/lib/codeql/unified/internal/Ast.qll
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,10 @@ module Unified {
/** Gets the node corresponding to the field `parameter`. */
final override F::Parameter getAParameter() { result = this.getParameter(_) }

/** Gets the node corresponding to the field `type`. */
final F::Expr getType() { unified_accessor_declaration_type(this, result) }
/** Gets the node corresponding to the field `return_type`. */
final override F::Expr getReturnType() {
unified_accessor_declaration_return_type(this, result)
}

/** Gets a field or child node of this node. */
final override F::AstNode getAFieldOrChild() {
Expand All @@ -133,7 +135,7 @@ module Unified {
unified_accessor_declaration_modifier(this, _, result) or
unified_accessor_declaration_def(this, _, result) or
unified_accessor_declaration_parameter(this, _, result) or
unified_accessor_declaration_type(this, result)
unified_accessor_declaration_return_type(this, result)
}
}

Expand Down Expand Up @@ -1624,7 +1626,7 @@ module Unified {
or
result = node.(AccessorDeclaration).getParameter(i) and name = "getParameter"
or
result = node.(AccessorDeclaration).getType() and i = -1 and name = "getType"
result = node.(AccessorDeclaration).getReturnType() and i = -1 and name = "getReturnType"
or
result = node.(Argument).getModifier(i) and name = "getModifier"
or
Expand Down
2 changes: 1 addition & 1 deletion unified/ql/lib/codeql/unified/internal/ExprPositions.qll
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ predicate isInTypeContext(Expr expr) {
or
expr = any(FunctionExpr n).getReturnType()
or
expr = any(AccessorDeclaration n).getType()
expr = any(AccessorDeclaration n).getReturnType()
or
expr = any(Parameter n).getType()
or
Expand Down
20 changes: 20 additions & 0 deletions unified/ql/lib/codeql/unified/internal/LocalNameBinding.qll
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,7 @@ module Public {
|
decl instanceof VariableDeclaration or
decl instanceof FunctionDeclaration or // treat functions as values
decl instanceof AccessorDeclaration or
decl instanceof Parameter or
decl instanceof ForEachStmt or
decl instanceof PatternGuardExpr or
Expand Down Expand Up @@ -446,6 +447,25 @@ module Public {
}
}

private AccessorDeclaration getAnAccessor(LocalVariable v) {
v.getABinding() = result.getNameNode() and
(result instanceof GetterDeclaration or result instanceof SetterDeclaration)
}

/** A local variable defined by a getter/setter pair, which does not store a value on its own. */
final class ComputedLocalVariable extends LocalVariable {
ComputedLocalVariable() { exists(getAnAccessor(this)) }

/** Gets the getter or setter for this computed variable. */
AccessorDeclaration getAnAccessor() { result = getAnAccessor(this) }

/** Gets the getter for this computed variable. */
GetterDeclaration getGetterDeclaration() { result = getAnAccessor(this) }

/** Gets the setter for this computed variable. */
SetterDeclaration getSetterDeclaration() { result = getAnAccessor(this) }
}

/** An access to a locally-declared name. */
class LocalNameAccess extends PotentialLocalNameAccess {
LocalNameAccess() { not this instanceof UnqualifiedMemberAccess }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ predicate isInstanceMember(Member member) {
(
member instanceof VariableDeclaration or
member instanceof FunctionDeclaration or
member instanceof AccessorDeclaration or
member instanceof ConstructorDeclaration
) and
exists(ClassLikeDeclaration cls | cls.getAMember() = member |
Expand Down
Loading
Loading