Repository navigation
Conversation
Tests are split into three categories: local, static, and instance. They follow mostly the same structure.
Accessors previously had a 'type' field. Now, it has a return_type instead, which is only set for getters. For set, didSet, and willSet, we insert a synthetic newValue/oldValue parameter, which also carries the type previously stored in accessor_declaration.type.
Modifiers like 'static' did not get preserved when extracting accessors, making most of the tests fail for static getters/setters. All those tests now pass.
This does the minimal change needed to get things working, but it's not pretty. A later commit will refactor this to have less ad-hoc logic here.
Now that all data flow nodes are in Expr, we get more noise if we don't restrict this
This was already handled in the AST mapping, but no QL test was added initially
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved accessor resolution, state propagation, and capture-ordering gaps can cause valid data flows to be missed.
5 open issues
What changed in this PR
Adds getter and setter modeling to Unified’s data-flow analysis, with Swift extraction support and regression fixtures.
Changes:
- Extracts accessor return types, parameters, and property modifiers.
- Models accessor calls, receiver flow, and captured-variable synchronization.
- Adds local, instance, and static accessor tests and updates expected results.
| File | Description |
|---|---|
unified/ql/test/library-tests/dataflow/test.expected |
Updates accessor and closure-flow expectations. |
unified/ql/test/library-tests/dataflow/accessors_static.swift |
Adds static accessor cases. |
unified/ql/test/library-tests/dataflow/accessors_local.swift |
Adds local accessor and capture cases. |
unified/ql/test/library-tests/dataflow/accessors_instance.swift |
Adds instance accessor and receiver-flow cases. |
unified/ql/lib/unified.dbscheme |
Renames the accessor return-type relation. |
unified/ql/lib/codeql/unified/internal/typeinference/Type.qll |
Exposes the class represented by a type. |
unified/ql/lib/codeql/unified/internal/StaticNameBinding.qll |
Unifies qualified and unqualified member access. |
unified/ql/lib/codeql/unified/internal/NameBindingPlugin.qll |
Recognizes instance accessors. |
unified/ql/lib/codeql/unified/internal/LocalNameBinding.qll |
Identifies computed local variables. |
unified/ql/lib/codeql/unified/internal/ExprPositions.qll |
Uses the renamed accessor return-type API. |
unified/ql/lib/codeql/unified/internal/dataflow/Step.qll |
Adds call-graph-only flow steps. |
unified/ql/lib/codeql/unified/internal/dataflow/LocalSsa.qll |
Excludes computed locals from stored-variable tracking. |
unified/ql/lib/codeql/unified/internal/dataflow/DataFlowNode.qll |
Extends receiver APIs and accessor control-flow positions. |
unified/ql/lib/codeql/unified/internal/dataflow/DataFlowInstantiation.qll |
Connects getter results and setter arguments. |
unified/ql/lib/codeql/unified/internal/dataflow/DataFlowGraph.qll |
Adds accessor receiver and callable edges. |
unified/ql/lib/codeql/unified/internal/dataflow/DataFlowCall.qll |
Represents implicit getter and setter calls. |
unified/ql/lib/codeql/unified/internal/dataflow/CaptureSsa.qll |
Revises closure alias and escape tracking. |
unified/ql/lib/codeql/unified/internal/dataflow/CallGraph.qll |
Resolves accessor call targets. |
unified/ql/lib/codeql/unified/internal/Ast.qll |
Updates generated accessor return-type APIs. |
unified/ql/consistency-queries/DataFlowConsistency.ql |
Exempts setter inputs from post-update checks. |
unified/extractor/tests/corpus/swift/variables/property-with-willset-and-didset-observers.output |
Records synthesized observer parameters. |
unified/extractor/tests/corpus/swift/types/protocol-with-read-only-and-read-write-property-requirements.output |
Updates protocol accessor expectations. |
unified/extractor/tests/corpus/swift/types/property-with-getter-and-setter.output |
Records getter return types and setter parameters. |
unified/extractor/tests/corpus/swift/types/computed-property.output |
Updates computed-property return-type output. |
unified/extractor/tests/corpus/swift/types/binding-modifier-does-not-leak-into-accessor-body.output |
Updates the accessor return-type field. |
unified/extractor/src/languages/swift/swift.rs |
Extracts accessor parameters, return types, and modifiers. |
unified/extractor/ast_types.yml |
Defines the accessor return-type field. |
Review effort: Balanced
| exists(MemberAccessExpr m, N::NamespaceNode namespace | | ||
| access = m and | ||
| // TODO: Make type inference own accessor-resolution. It also needs it internally. | ||
| namespace.isInstanceMemberNamespace(T::inferType(m.getBase()).asClass()) and |
Comment on lines
+94
to
+96
| node.isResultValue(any(Argument arg).getValue()) | ||
| or | ||
| node.isImplicitArgument(_, _, _) |
Comment on lines
+18
to
+19
| TSetterCall(Expr expr) { | ||
| isInAssignmentContext(expr, _) and |
Comment on lines
+145
to
+147
| node1.isReceiverPostUpdateEx(call) and | ||
| step.value() and | ||
| node2.isPostUpdate(expr.getBase()) |
Comment on lines
+359
to
+370
| ( | ||
| access = call.asExplicitCall() | ||
| or | ||
| access = call.asGetterCall() | ||
| or | ||
| access = call.asSetterCall() | ||
| ) and | ||
| ( | ||
| isPost = false and cfgNode.injects(access) | ||
| or | ||
| isPost = true and cfgNode.isAfter(access) | ||
| ) |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.