Skip to content

Fix #10569 FN: duplicateExpression with multiple strings compared#4087

Merged
danmar merged 8 commits into
cppcheck-opensource:mainfrom
chrchr-github:chr_Fix10569
May 9, 2022
Merged

Fix #10569 FN: duplicateExpression with multiple strings compared#4087
danmar merged 8 commits into
cppcheck-opensource:mainfrom
chrchr-github:chr_Fix10569

Conversation

@chrchr-github

Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread test/testautovariables.cpp

@danmar danmar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hmm.. the message is not ideal.

Comment thread test/testother.cpp Outdated
" (a == \"y\") ||\n"
" (a == \"42\")) {}\n"
"}\n");
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:4]: (style) Same expression on both sides of '||'.\n"

@danmar danmar May 8, 2022

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This message is not technically 100% true. It would be nice if we could rephraze the message somehow.

Duplicate expressions 'a == "42"' in condition ?

Comment thread lib/checkother.cpp Outdated

const std::string& op = opTok ? opTok->str() : "&&";
std::string msg = "Same expression on both sides of \'" + op + "\'";
std::string msg = "Same expression " + (hasMultipleExpr ? "found multiple times in chain of \'" + op + "\' operators" : "on both sides of \'" + op + "\'");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Better! I suggest that the expression is written also:

"Same expression " + (hasMultipleExpr ? ("'" + expr1 + "' found multiple time in chain...

@danmar danmar merged commit 54f832a into cppcheck-opensource:main May 9, 2022
Sign up for free to 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