Skip to content

Conversation

@geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Nov 13, 2023

Update all the inline dataflow tests to:

  1. use the new InlineFlowTest.qll library added by @MathiasVP in Swift: Add variable-capture flow #14577 . Until now this was only used in the dataflow/capture test. This change reduces code duplication as much of the code is very similar between the different tests.
  2. as a result, we now support source labels (see below).

Source labels are a feature where we can label sources explicitly with a string in tests, rather than using their line number to identify them. This makes maintaining tests a lot less painful when line numbers inevitably change.

I wanted this change to be fully backwards compatible as far as the existing test cases were concerned (i.e. the existing .swift code with line numbers as labels). Not only does that make this PR smaller, is makes it much less likely to conflict with other changes to the tests are are going on continuously. I had to add a couple of compatibility features to InlineFlowTest.qll to accomplish this - in particular, custom inline tags (such as tainted=), and using line numbers as a fallback source label (rather than ""). We lose the (unused) ability to have unlabelled sources.

I've updated a minority of test cases to use source labels, as a proof of concept. We can gradually migrate the rest as and when we need to (i.e. when we would otherwise have to renumber them).

@geoffw0 geoffw0 added no-change-note-required This PR does not need a change note Swift labels Nov 13, 2023
@geoffw0 geoffw0 requested a review from a team as a code owner November 13, 2023 12:45
@geoffw0 geoffw0 requested a review from MathiasVP November 13, 2023 12:47
Copy link
Contributor

@MathiasVP MathiasVP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@geoffw0 geoffw0 merged commit 6f56a65 into github:main Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note Swift

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants