Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
6a46da4
unified: Add tests with accessors
asgerf Oct 23, 2026
6270fad
unified: Allow accessors to be seen as instance members
asgerf Oct 23, 2026
3fcfb68
unified: Add DataFlowCalls for getter and setter calls
asgerf Oct 23, 2026
61a102c
unified: Resolve calls statically
asgerf Oct 23, 2026
f1f4d84
unified: Add out-node from getter calls
asgerf Oct 23, 2026
82487f8
unified: Synthesize setter parameters and return type in accessors
asgerf Oct 23, 2026
5dec1dc
unified: Pass the incoming value into setter calls
asgerf Oct 23, 2026
c7c80a7
unified: Ensure 'static' modifier is preserved for accessors
asgerf Oct 23, 2026
32979b2
unified: Add MemberAccess as union of MemberAccessExpr and Unqualifie…
asgerf Oct 23, 2026
364e982
unified: Also resolve accessors on instances
asgerf Oct 23, 2026
9da580f
unified: Make accessors introduce LocalVariables too
asgerf Oct 23, 2026
34de910
unified: Handle capturing local getters/setters
asgerf Oct 24, 2026
5c9942f
unified: Propagate receiver argument into accessor calls
asgerf Oct 24, 2026
b348803
unified: Allow self-return via receiver parameter
asgerf Oct 24, 2026
edc465e
unified: Add more capture tests
asgerf Oct 24, 2026
6444d0b
unified: Add test with chain of accessors
asgerf Oct 25, 2026
4bd9473
unified: Split up hasControlFlowPosition()
asgerf Oct 26, 2026
a808fde
unified: Associate CFG nodes with implicit getter/setter arguments
asgerf Oct 26, 2026
3f7735f
unified: Use hasControlFlowPosition in CaptureSsa
asgerf Oct 26, 2026
d0d1363
unified: Move ad-hoc aliasing logic into a step
asgerf Oct 26, 2026
e3883da
unified: Add basic escape
asgerf Oct 26, 2026
09fb8f1
unified: Update nodes/edges in test output
asgerf Oct 26, 2026
b084467
unified: Tolerate missing post-update for RHS
asgerf Oct 26, 2026
1cc99d5
unified: Add test for explicitly-named setter parameter
asgerf Oct 26, 2026
612c241
Update isEscape predicate to include incoming values
asgerf Oct 27, 2026
ac8adee
unified: Add test for receiver updates from getter reads
asgerf Oct 27, 2026
6741896
unified: Simple fix for getter calls missing post update node
asgerf Oct 27, 2026
4a510f7
unified: More precise CFG node for implicit setter arguments
asgerf Oct 27, 2026
ed70ef4
unified: Support setter calls from writebacks
asgerf Oct 27, 2026
fcf39ec
unified: Add a hook for writebacks
asgerf Oct 27, 2026
3769c46
unified: Test that writebacks after a mutating call works
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
126 changes: 87 additions & 39 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::OutputId>,
/// 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 getter return type and its `newValue`/`oldValue`
/// parameter type, when available.
property_type: Option<yeast::OutputId>,
/// 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::OutputId>,
/// 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,35 +401,74 @@ 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.
rule!(
(accessorDecl accessorSpecifier: @@spec body: _? @@body)
=>
(accessor_declaration
modifier: {
if body.is_some() {
ctx.outer_modifiers.clone()
} else {
Vec::new()
//
// `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
parameters: (accessorParameters name: @@param_name)?
body: _? @@body)
=>
accessor_declaration {
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})))
}
}
modifier: {chained_modifier(&mut ctx)}
name_node: {
ctx.property_name_node
.ok_or("accessor outside property context")?
}
type: {ctx.property_type}
accessor_kind: (accessor_kind #{spec})
body: {ctx.reset(); body}
location: {spec}
location: {body})
(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 return_ty = if spec_text == "get" {
ctx.property_type
} else {
None
};
let body = match body {
Some(block) => {
ctx.reset();
ctx.translate(block)?.into_iter().next()
}
None => None,
};
tree!((accessor_declaration
modifier: {modifiers}
modifier: {chained}
name_node: {name}
return_type: {return_ty}
accessor_kind: (accessor_kind #{spec})
parameter: {param}
body: {body}
location: {spec}
location: {body}))
}
),
// ---- Variables ----
// The individual bindings of a `variableDecl`. The binding modifier and
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 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.(BuilderNode).isSetterValueArgument(_)
}

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
Loading
Loading