Skip to content

include GUI test sources in selfchecks - #5687

Draft
firewave wants to merge 8 commits into
cppcheck-opensource:mainfrom
firewave:selfcheck-gui
Draft

include GUI test sources in selfchecks#5687
firewave wants to merge 8 commits into
cppcheck-opensource:mainfrom
firewave:selfcheck-gui

Conversation

@firewave

Copy link
Copy Markdown
Collaborator

No description provided.

@firewavefirewave changed the title removed unnecessary file pattern for GUI sources in selfcheck - GUI tests are now includedinclude GUI test sources in selfcheckNov 20, 2023
@firewave

This comment was marked as resolved.

@firewave
firewave marked this pull request as ready for review December 1, 2023 15:07
@firewave

firewave commented Dec 1, 2023

Copy link
Copy Markdown
CollaboratorAuthor

I added those as there's a passedByValue false negative in testcppchecklibrary. Also more dogfooding is always good.

I did suppress the naming-privateMemberVariable warnings since they would have caused almost all lines in the source to be changed.

The uninitMemberVar are not of interest since the classes are simply mocked.

danmar
danmar previously approved these changes Dec 1, 2023
@firewave
firewave marked this pull request as draft December 4, 2023 16:30
#include "addoninfo.h"
#include "importproject.h"
// cppcheck-suppress-begin uninitMemberVar
#include "platform.h"

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

I need to suppress all the warnings from an include. I would expect this to work but it doesn't as it was probably not intended to be used that way. I will file a ticket about it.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

I filed https://trac.cppcheck.net/ticket/14219 about being able to suppress the uninitMemberVar warnings per implementation

@firewave
firewaveforce-pushed the selfcheck-gui branch 5 times, most recently from 13d7da2 to 3c8cfd2CompareOctober 22, 2025 20:40
@firewave
firewaveforce-pushed the selfcheck-gui branch 2 times, most recently from ce278c3 to 881f529CompareNovember 12, 2025 12:29
@firewavefirewave changed the title include GUI test sources in selfcheckinclude GUI test sources in selfchecksNov 12, 2025
@firewave
firewave marked this pull request as ready for review November 12, 2025 12:33
@firewave
firewaveforce-pushed the selfcheck-gui branch 4 times, most recently from dabe5ed to 145b119CompareFebruary 16, 2026 05:32
@firewave
firewave marked this pull request as draft February 24, 2026 06:30
@sonarqubecloud

Copy link
Copy Markdown

@firewave

Copy link
Copy Markdown
CollaboratorAuthor

@chrchr-github Looks like we have false positives when a function is not implemented:

gui/projectfile.h:345:61: style:inconclusive: Function 'addSuppression' argument 1 names different: declaration 'suppression' definition '<unnamed>'. [funcArgNamesDifferentUnnamed]
void addSuppression(const SuppressionList::Suppression &suppression);
^

@chrchr-github

Copy link
Copy Markdown
Collaborator

@chrchr-github Looks like we have false positives when a function is not implemented:

But it is implemented here: https://github.com/firewave/cppcheck/blob/70e4ea3b850d73f4ed120d54a2b3c637edc79653/gui/projectfile.cpp#L787
Still looks like some kind of FP though.

@firewave

Copy link
Copy Markdown
CollaboratorAuthor

But it is implemented here:

The GUI tests mock these classes and only implement what is necessary to get them to link.

@firewave

Copy link
Copy Markdown
CollaboratorAuthor

So we have drive-by detection of https://trac.cppcheck.net/ticket/10670 now - so hopefully we should be able to package that into a separate check.

@chrchr-github

Copy link
Copy Markdown
Collaborator

But it is implemented here:

The GUI tests mock these classes and only implement what is necessary to get them to link.

There is no warning for this, so not sure what`s going on:

struct S {
void f(int i);
};

@firewave

Copy link
Copy Markdown
CollaboratorAuthor

There is no warning for this, so not sure what`s going on:

I will have a look tomorrow.

@firewave

Copy link
Copy Markdown
CollaboratorAuthor

The warnings originate from gui/test/resultstree/testresultstree.cpp which mocks the function calls. So we lack the information in the warning about the location of the unnamed parameter.

And in this case it is in intentional since the parameters are not being used (I possibly pointed out this case in the PR which introduced these warnings).

These warnings also cannot be properly suppressed because they are reported at the declaration and thus the annotation needs to be added in the header. This is another case where we need to report them on the implementation instead. This also makes sense in the case that there might be various implementations of the functions which might not all experience the same issue.

@firewave

Copy link
Copy Markdown
CollaboratorAuthor

These warnings also cannot be properly suppressed because they are reported at the declaration and thus the annotation needs to be added in the header. This is another case where we need to report them on the implementation instead. This also makes sense in the case that there might be various implementations of the functions which might not all experience the same issue.

I filed https://trac.cppcheck.net/ticket/14760 about it.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@firewave@chrchr-github@danmar