Skip to content

Swift: Add some summary queries. #10903

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 5 commits into from
Oct 25, 2022
Merged

Swift: Add some summary queries. #10903

merged 5 commits into from
Oct 25, 2022

Conversation

geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Oct 20, 2022

I wrote these queries to assess the progress we're making identifying flow sources and sensitive expressions in real world Swift snapshots. I intend to track these things fairly often in the near-mid future, so it would be convenient to have the queries in the public repo. I don't think there's any harm in letting curious users see them as well.

It might be nice to have LinesOfCode.ql and LinesOfUserCode.ql summary queries as well as all (?) other languages do, but I don't think we extract the data for them yet.

@geoffw0 geoffw0 added no-change-note-required This PR does not need a change note Swift labels Oct 20, 2022
@geoffw0 geoffw0 requested a review from a team as a code owner October 20, 2022 11:49
Copy link
Contributor

@jketema jketema left a comment

Choose a reason for hiding this comment

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

Mostly LGTM. Just one description I do not fully understand.

@geoffw0
Copy link
Contributor Author

geoffw0 commented Oct 21, 2022

In addition to your suggested change I've restricted the count of "Expressions" to ones with locations in code now. It turns out there's quite a lot of locationless stuff that was exaggerating the size of smaller projects (and in turn making me wonder why we weren't finding many results, taint sources etc in them).

atorralba
atorralba previously approved these changes Oct 24, 2022
Copy link
Contributor

@atorralba atorralba left a comment

Choose a reason for hiding this comment

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

This LGTM. I guess summary queries don't need a docs review.

@geoffw0
Copy link
Contributor Author

geoffw0 commented Oct 25, 2022

I guess summary queries don't need a docs review.

As far as I can tell summary queries don't usually get full .qhelp documentation, just an explanatory @description. They're not really as visible as our main 'problem' queries so this seems appropriate to me. Happy to add full docs, or get the @descriptions reviewed by the doc team if you would prefer.

@atorralba
Copy link
Contributor

atorralba commented Oct 25, 2022

Happy to add full docs, or get the @descriptions reviewed by the doc team if you would prefer.

No, let's keep it consistent with the rest of summary queries. Looks good as it is.

@geoffw0 geoffw0 merged commit 3d025ea into github:main Oct 25, 2022
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.

4 participants