Skip to content

Python: Fix a bunch of QL warnings #8336

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 9 commits into from
Mar 9, 2022

Conversation

tausbn
Copy link
Contributor

@tausbn tausbn commented Mar 4, 2022

Does what it says on the tin. Most of the remaining warnings fall into a few classes:

  • Alerts relating to deprecated classes (primarily stuff relating to the old data-flow library). I can fix these, but it didn't seem like much of a priority.
  • Uses of db-types outside of lib. Some of these should probably just be deleted (SVN support, anyone?), but may need a formal deprecation first.

All of the changes in this PR should be fairly benign, however.


As this doesn't change the API, I don't think a change note is required.

tausbn added 4 commits March 4, 2022 12:14
For the most part, these boil down to "some global property holds, and
so this relation contains all instances of class `X`". The fix is to
explicitly build the cartesian product (which we were already building
implicitly anyway) by adding `and exists(var)` to the disjunct that did
not mention `var`.

Note that these cartesian products are always with singletons on one
side, and so should be unproblematic.
These are all implied by the return type of the other side of the
equality.
@tausbn tausbn added the no-change-note-required This PR does not need a change note label Mar 4, 2022
@github-actions github-actions bot added the Python label Mar 4, 2022
@tausbn tausbn force-pushed the python-fix-a-bunch-of-ql-warnings branch from e7e57fe to a41f3cc Compare March 4, 2022 15:41
@tausbn tausbn force-pushed the python-fix-a-bunch-of-ql-warnings branch from c61d340 to 524d17e Compare March 7, 2022 13:24
@tausbn tausbn force-pushed the python-fix-a-bunch-of-ql-warnings branch from 524d17e to 5a8ba6a Compare March 7, 2022 19:00
@tausbn tausbn marked this pull request as ready for review March 7, 2022 20:32
@tausbn tausbn requested review from a team as code owners March 7, 2022 20:32
michaelnebel
michaelnebel previously approved these changes Mar 8, 2022
Copy link
Contributor

@michaelnebel michaelnebel left a comment

Choose a reason for hiding this comment

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

C#: LGTM

atorralba
atorralba previously approved these changes Mar 8, 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.

Java 👍

Copy link
Contributor

@yoff yoff left a comment

Choose a reason for hiding this comment

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

Thank you for doing this!

Co-authored-by: yoff <lerchedahl@gmail.com>
@tausbn tausbn dismissed stale reviews from atorralba and michaelnebel via 063a8bb March 8, 2022 14:20
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.

C++ 👍

Copy link
Contributor

@yoff yoff left a comment

Choose a reason for hiding this comment

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

Happy to merge this now.
The Field only used in CharPred-warnings are interesting, but can be tackled later.

Copy link
Contributor

@alexrford alexrford left a comment

Choose a reason for hiding this comment

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

Ruby 👍

@tausbn tausbn merged commit 7b877fb into github:main Mar 9, 2022
@tausbn tausbn deleted the python-fix-a-bunch-of-ql-warnings branch March 9, 2022 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants