Skip to content

Swift: Modernize the encryption queries #12764

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 6, 2023
Merged

Conversation

geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Apr 5, 2023

Modernize the 8 Swift encryption queries. By 'modernize', I mean split into three files with standard layouts, extendable sinks etc in the usual way. Conversion to DataFlow::ConfigSig is a separate process and has in fact already been done for these queries.

I haven't yet added CSV extension points. That will be follow-up as, from past experience, it sometimes involves a bit more work and testing on individual queries.

I also haven't unified the various 'constant' sources some of these queries define - there's a separate issue for that at some point. And I haven't dealt with IntLiteralSource from swift/insufficient-hash-iterations, as I want to redesign it a bit before making it extendable / public. I'm creating an issue for that as well.

@geoffw0 geoffw0 added no-change-note-required This PR does not need a change note Swift labels Apr 5, 2023
@geoffw0 geoffw0 requested a review from a team as a code owner April 5, 2023 10:22
@geoffw0
Copy link
Contributor Author

geoffw0 commented Apr 5, 2023

I believe we're currently ignoring QLdoc check failures for *Config::isSource/1, *Config::isSink/1, *Config::isBarrier/1 and *Config::isAdditionalFlowStep/2.

@jketema
Copy link
Contributor

jketema commented Apr 5, 2023

I believe we're currently ignoring QLdoc check failures for *Config::isSource/1, *Config::isSink/1, *Config::isBarrier/1 and *Config::isAdditionalFlowStep/2.

Correct those should be considered as being inherited form the signature, but currently this is not picked up upon.

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 3baba70 into github:main Apr 6, 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.

3 participants