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

[cli] Fix html output of CodeChecker parse #3524

Merged

Conversation

csordasmarton
Copy link
Contributor

Closes #3510

@csordasmarton csordasmarton added CLI 💻 Related to the command-line interface, such as the cmd, store, etc. commands bugfix 🔨 labels Nov 29, 2021
@csordasmarton csordasmarton added this to the release 6.18.1 milestone Nov 29, 2021
@vChavezB
Copy link

I can confirm that the patch works :) My repository is private where I use codechecker so unfortunately I cannot share the results.
Will be waiting for the next push to the master. Regards

@whisperity
Copy link
Contributor

Preliminary tests suggest that this is indeed okay. I'm doing a full run of the analysis in the CI with CodeChecker built from the request branch, stay tuned! 🎉

self,
reports: List[Report]
):
) -> Tuple[HTMLReports, FileSources]:
""" Get html reports from the given reporst.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
""" Get html reports from the given reporst.
""" Get HTML reports from the given reports.

#include "f.h"

int b() {
foo();
Copy link
Contributor

Choose a reason for hiding this comment

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

return foo();? †

<key>check_name</key>
<string>clang-diagnostic-return-type</string>
<key>description</key>
<string>non-void function does not return a value</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

† So this warning disappears from one of the TUs?

if re.search("<td file=", line):
report_count += 1

self.assertEqual(report_count, 6)
Copy link
Contributor

Choose a reason for hiding this comment

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

† And then it should be 5, right?

(I want to go for an odd number, so any chance of this mystical duplication issue coming up later would clearly be different, because it would end up the counts being even.)

Copy link
Contributor

Choose a reason for hiding this comment

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

This became 4 now, which is also an even number. 🙁
The idea was that only 1 out of the 2 TUs will have a return X;, the other won't, so we get (for the project) 1 "non-void function does not return a value" warning. Previously we had 2, now we have 0, the parity is the same, so we still can't catch a "duplication" mistake.

@whisperity
Copy link
Contributor

Full CI run from my side confirms the output structure (and the size) of the report directory is sensible again, thank you.
With the comments above about the tests, LGTM.

@csordasmarton csordasmarton force-pushed the fix_html_output_for_includes branch from 58825b1 to 8038db8 Compare December 6, 2021 07:56
if re.search("<td file=", line):
report_count += 1

self.assertEqual(report_count, 4)
Copy link
Contributor

Choose a reason for hiding this comment

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

This became 4 now, which is also an even number. 🙁
The idea was that only 1 out of the 2 TUs will have a return X;, the other won't, so we get (for the project) 1 "non-void function does not return a value" warning. Previously we had 2, now we have 0, the parity is the same, so we still can't catch a "duplication" mistake.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@whisperity Fixed.

@csordasmarton csordasmarton force-pushed the fix_html_output_for_includes branch from 8038db8 to 0b77928 Compare December 6, 2021 11:11
whisperity
whisperity approved these changes Dec 6, 2021
@bruntib bruntib merged commit be40466 into Ericsson:master Dec 6, 2021
@csordasmarton csordasmarton deleted the fix_html_output_for_includes branch January 28, 2022 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix 🔨 CLI 💻 Related to the command-line interface, such as the cmd, store, etc. commands
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicated bug report in html parser
4 participants