Skip to content
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

C++: Clearer messages for the format args queries #11149

Merged
merged 4 commits into from Nov 8, 2022

Conversation

geoffw0
Copy link
Contributor

@geoffw0 geoffw0 commented Nov 7, 2022

Makes the messages from the cpp/too-many-format-arguments and cpp/wrong-number-format-arguments queries a bit clearer in cases where macros are hiding the formatting function call in question. Motivated by a real case that took a bit of investigation before anyone realized it was a TP.

No change note required because results should be the same apart from clarification in the messages.

@geoffw0 geoffw0 added C++ no-change-note-required This PR does not need a change note labels Nov 7, 2022
@geoffw0 geoffw0 requested a review from a team as a code owner Nov 7, 2022
jketema
jketema approved these changes Nov 7, 2022
Copy link
Contributor

@jketema jketema left a comment

LGTM. It does seem some internal test needs updating (or we need to move it, if possible).

I don't think we need a DCA run, as we're only changing the messages used during reporting?

@geoffw0
Copy link
Contributor Author

geoffw0 commented Nov 8, 2022

Agree on both points. I'll update the internal test...

@geoffw0 geoffw0 added the depends on internal PR This PR should only be merged in sync with an internal Semmle PR label Nov 8, 2022
@geoffw0
Copy link
Contributor Author

geoffw0 commented Nov 8, 2022

I've created an internal PR to update the internal test.

@jketema jketema merged commit 6a5f37b into github:main Nov 8, 2022
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ depends on internal PR This PR should only be merged in sync with an internal Semmle PR no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants