Enable and mitigate readability-simplify-boolean-expr (#4897)

This commit is contained in:
chrchr-github 2023-03-17 13:51:55 +01:00 committed by GitHub
parent 173c84375c
commit 3ccd0505cd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 36 additions and 79 deletions

View File

@ -1,7 +1,9 @@
---
Checks: '*,-abseil-*,-altera-*,-android-*,-boost-*,-cert-*,-cppcoreguidelines-*,-darwin-*,-fuchsia-*,-google-*,-hicpp-*,-linuxkernel-*,-llvm-*,-llvmlibc-*,-mpi-*,-objc-*,-openmp-*,-zircon-*,google-explicit-constructor,-readability-braces-around-statements,-readability-magic-numbers,-bugprone-macro-parentheses,-readability-isolate-declaration,-readability-function-size,-modernize-use-trailing-return-type,-readability-implicit-bool-conversion,-readability-uppercase-literal-suffix,-modernize-use-auto,-readability-else-after-return,-modernize-use-default-member-init,-readability-redundant-member-init,-modernize-avoid-c-arrays,-modernize-use-equals-default,-readability-container-size-empty,-readability-simplify-boolean-expr,-bugprone-branch-clone,-bugprone-narrowing-conversions,-modernize-raw-string-literal,-readability-convert-member-functions-to-static,-modernize-loop-convert,-readability-const-return-type,-modernize-return-braced-init-list,-performance-inefficient-string-concatenation,-misc-throw-by-value-catch-by-reference,-readability-avoid-const-params-in-decls,-misc-non-private-member-variables-in-classes,-clang-analyzer-*,-bugprone-signed-char-misuse,-misc-no-recursion,-readability-use-anyofallof,-performance-no-automatic-move,-readability-function-cognitive-complexity,-readability-redundant-access-specifiers,-concurrency-mt-unsafe,-bugprone-easily-swappable-parameters,-readability-suspicious-call-argument,-readability-identifier-length,-readability-container-data-pointer,-bugprone-assignment-in-if-condition,-misc-const-correctness,-portability-std-allocator-const,-modernize-deprecated-ios-base-aliases,-bugprone-unchecked-optional-access,-modernize-replace-auto-ptr,-readability-identifier-naming,-portability-simd-intrinsics,-misc-use-anonymous-namespace'
Checks: '*,-abseil-*,-altera-*,-android-*,-boost-*,-cert-*,-cppcoreguidelines-*,-darwin-*,-fuchsia-*,-google-*,-hicpp-*,-linuxkernel-*,-llvm-*,-llvmlibc-*,-mpi-*,-objc-*,-openmp-*,-zircon-*,google-explicit-constructor,-readability-braces-around-statements,-readability-magic-numbers,-bugprone-macro-parentheses,-readability-isolate-declaration,-readability-function-size,-modernize-use-trailing-return-type,-readability-implicit-bool-conversion,-readability-uppercase-literal-suffix,-modernize-use-auto,-readability-else-after-return,-modernize-use-default-member-init,-readability-redundant-member-init,-modernize-avoid-c-arrays,-modernize-use-equals-default,-readability-container-size-empty,-bugprone-branch-clone,-bugprone-narrowing-conversions,-modernize-raw-string-literal,-readability-convert-member-functions-to-static,-modernize-loop-convert,-readability-const-return-type,-modernize-return-braced-init-list,-performance-inefficient-string-concatenation,-misc-throw-by-value-catch-by-reference,-readability-avoid-const-params-in-decls,-misc-non-private-member-variables-in-classes,-clang-analyzer-*,-bugprone-signed-char-misuse,-misc-no-recursion,-readability-use-anyofallof,-performance-no-automatic-move,-readability-function-cognitive-complexity,-readability-redundant-access-specifiers,-concurrency-mt-unsafe,-bugprone-easily-swappable-parameters,-readability-suspicious-call-argument,-readability-identifier-length,-readability-container-data-pointer,-bugprone-assignment-in-if-condition,-misc-const-correctness,-portability-std-allocator-const,-modernize-deprecated-ios-base-aliases,-bugprone-unchecked-optional-access,-modernize-replace-auto-ptr,-readability-identifier-naming,-portability-simd-intrinsics,-misc-use-anonymous-namespace'
WarningsAsErrors: '*'
HeaderFilterRegex: '(cli|gui|lib|oss-fuzz|test|triage)\/[a-z]+\.h'
CheckOptions:
- key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic
value: '1'
- key: readability-simplify-boolean-expr.SimplifyDeMorgan
value: '0'

View File

@ -49,7 +49,6 @@ We are not interesting in the size/complexity of a function.
`readability-magic-numbers`<br>
`readability-redundant-member-init`<br>
`readability-simplify-boolean-expr`<br>
These do not (always) increase readability.

View File

@ -365,12 +365,12 @@ static CppcheckLibraryData::Markup loadMarkup(QXmlStreamReader &xmlReader)
mandatoryAttibuteMissing(xmlReader, "ext");
}
if (xmlReader.attributes().hasAttribute("aftercode")) {
markup.afterCode = (xmlReader.attributes().value("aftercode") == QString("true")) ? true : false;
markup.afterCode = (xmlReader.attributes().value("aftercode") == QString("true"));
} else {
mandatoryAttibuteMissing(xmlReader, "aftercode");
}
if (xmlReader.attributes().hasAttribute("reporterrors")) {
markup.reportErrors = (xmlReader.attributes().value("reporterrors") == QString("true")) ? true : false;
markup.reportErrors = (xmlReader.attributes().value("reporterrors") == QString("true"));
} else {
mandatoryAttibuteMissing(xmlReader, "reporterrors");
}

View File

@ -147,10 +147,7 @@ Qt::CheckState SettingsDialog::boolToCheckState(bool yes)
bool SettingsDialog::checkStateToBool(Qt::CheckState state)
{
if (state == Qt::Checked) {
return true;
}
return false;
return state == Qt::Checked;
}

View File

@ -392,9 +392,7 @@ bool isVariableDecl(const Token* tok)
if (var->nameToken() == tok)
return true;
const Token * const varDeclEndToken = var->declEndToken();
if (Token::Match(varDeclEndToken, "; %var%") && varDeclEndToken->next() == tok)
return true;
return false;
return Token::Match(varDeclEndToken, "; %var%") && varDeclEndToken->next() == tok;
}
bool isStlStringType(const Token* tok)
@ -2002,11 +2000,7 @@ bool isUniqueExpression(const Token* tok)
return true;
const std::string freturnType = f.retType ? f.retType->name() : f.retDef->stringifyList(f.returnDefEnd());
if (f.argumentList.size() == fun->argumentList.size() && returnType == freturnType &&
f.name() != fun->name()) {
return false;
}
return true;
return f.argumentList.size() != fun->argumentList.size() || returnType != freturnType || f.name() == fun->name();
}))
return false;
} else if (tok->variable()) {

View File

@ -1111,12 +1111,10 @@ static bool isVLAIndex(const Token* tok)
return true;
if (tok->str() == "?") {
// this is a VLA index if both expressions around the ":" is VLA index
if (tok->astOperand2() &&
tok->astOperand2()->str() == ":" &&
isVLAIndex(tok->astOperand2()->astOperand1()) &&
isVLAIndex(tok->astOperand2()->astOperand2()))
return true;
return false;
return tok->astOperand2() &&
tok->astOperand2()->str() == ":" &&
isVLAIndex(tok->astOperand2()->astOperand1()) &&
isVLAIndex(tok->astOperand2()->astOperand2());
}
return isVLAIndex(tok->astOperand1()) || isVLAIndex(tok->astOperand2());
}

View File

@ -2894,7 +2894,7 @@ void CheckClass::checkCopyCtorAndEqOperator()
// This is disabled because of #8388
// The message must be clarified. How is the behaviour different?
// cppcheck-suppress unreachableCode - remove when code is enabled again
if ((true) || !mSettings->severity.isEnabled(Severity::warning))
if ((true) || !mSettings->severity.isEnabled(Severity::warning)) // NOLINT(readability-simplify-boolean-expr)
return;
for (const Scope * scope : mSymbolDatabase->classAndStructScopes) {

View File

@ -364,15 +364,15 @@ void CheckCondition::comparison()
if ((expr1->str() == "&" && (num1 & num2) != num2) ||
(expr1->str() == "|" && (num1 | num2) != num2)) {
const std::string& op(tok->str());
comparisonError(expr1, expr1->str(), num1, op, num2, op=="==" ? false : true);
comparisonError(expr1, expr1->str(), num1, op, num2, op != "==");
}
} else if (expr1->str() == "&") {
const bool or_equal = Token::Match(tok, ">=|<=");
const std::string& op(tok->str());
if ((Token::Match(tok, ">=|<")) && (num1 < num2)) {
comparisonError(expr1, expr1->str(), num1, op, num2, or_equal ? false : true);
comparisonError(expr1, expr1->str(), num1, op, num2, !or_equal);
} else if ((Token::Match(tok, "<=|>")) && (num1 <= num2)) {
comparisonError(expr1, expr1->str(), num1, op, num2, or_equal ? true : false);
comparisonError(expr1, expr1->str(), num1, op, num2, or_equal);
}
} else if (expr1->str() == "|") {
if ((expr1->astOperand1()->valueType()) &&
@ -382,11 +382,11 @@ void CheckCondition::comparison()
if ((Token::Match(tok, ">=|<")) && (num1 >= num2)) {
//"(a | 0x07) >= 7U" is always true for unsigned a
//"(a | 0x07) < 7U" is always false for unsigned a
comparisonError(expr1, expr1->str(), num1, op, num2, or_equal ? true : false);
comparisonError(expr1, expr1->str(), num1, op, num2, or_equal);
} else if ((Token::Match(tok, "<=|>")) && (num1 > num2)) {
//"(a | 0x08) <= 7U" is always false for unsigned a
//"(a | 0x07) > 6U" is always true for unsigned a
comparisonError(expr1, expr1->str(), num1, op, num2, or_equal ? false : true);
comparisonError(expr1, expr1->str(), num1, op, num2, !or_equal);
}
}
}
@ -1048,10 +1048,7 @@ static bool parseComparison(const Token *comp, bool &not1, std::string &op, std:
inconclusive = inconclusive || ((value)[0] == '\'' && !(op == "!=" || op == "=="));
// Only float and int values are currently handled
if (!MathLib::isInt(value) && !MathLib::isFloat(value) && (value)[0] != '\'')
return false;
return true;
return MathLib::isInt(value) || MathLib::isFloat(value) || (value[0] == '\'');
}
static std::string conditionString(bool not1, const Token *expr1, const std::string &op, const std::string &value1)

View File

@ -554,10 +554,7 @@ void CheckOther::redundantAssignmentInSwitchError(const Token *tok1, const Token
//---------------------------------------------------------------------------
static inline bool isFunctionOrBreakPattern(const Token *tok)
{
if (Token::Match(tok, "%name% (") || Token::Match(tok, "break|continue|return|exit|goto|throw"))
return true;
return false;
return Token::Match(tok, "%name% (") || Token::Match(tok, "break|continue|return|exit|goto|throw");
}
void CheckOther::checkRedundantAssignmentInSwitch()
@ -1097,7 +1094,7 @@ void CheckOther::variableScopeError(const Token *tok, const std::string &varname
void CheckOther::checkCommaSeparatedReturn()
{
// This is experimental for now. See #5076
if ((true) || !mSettings->severity.isEnabled(Severity::style))
if ((true) || !mSettings->severity.isEnabled(Severity::style)) // NOLINT(readability-simplify-boolean-expr)
return;
for (const Token *tok = mTokenizer->tokens(); tok; tok = tok->next()) {

View File

@ -702,9 +702,7 @@ static bool isSameIteratorContainerExpression(const Token* tok1,
ValueFlow::Value::LifetimeKind kind = ValueFlow::Value::LifetimeKind::Iterator)
{
if (isSameExpression(true, false, tok1, tok2, library, false, false)) {
if (astIsContainerOwned(tok1) && isTemporary(true, tok1, &library))
return false;
return true;
return !astIsContainerOwned(tok1) || !isTemporary(true, tok1, &library);
}
if (kind == ValueFlow::Value::LifetimeKind::Address) {
return isSameExpression(true, false, getAddressContainer(tok1), getAddressContainer(tok2), library, false, false);

View File

@ -1469,9 +1469,7 @@ bool CheckUninitVar::isMemberVariableUsage(const Token *tok, bool isPointer, All
if (Token::Match(tok, "%name% . %name%") && tok->strAt(2) == membervar && !(tok->tokAt(-2)->variable() && tok->tokAt(-2)->variable()->isReference())) {
const Token *parent = tok->next()->astParent();
if (parent && parent->isUnaryOp("&"))
return false;
return true;
return !parent || !parent->isUnaryOp("&");
} else if (!isPointer && !Token::simpleMatch(tok->astParent(), ".") && Token::Match(tok->previous(), "[(,] %name% [,)]") && isVariableUsage(tok, isPointer, alloc))
return true;
@ -1487,7 +1485,7 @@ bool CheckUninitVar::isMemberVariableUsage(const Token *tok, bool isPointer, All
return true;
// TODO: this used to be experimental - enable or remove see #5586
else if ((false) &&
else if ((false) && // NOLINT(readability-simplify-boolean-expr)
!isPointer &&
Token::Match(tok->tokAt(-2), "[(,] & %name% [,)]") &&
isVariableUsage(tok, isPointer, alloc))

View File

@ -825,9 +825,7 @@ struct ForwardTraversal {
}
static bool isUnevaluated(const Token* tok) {
if (Token::Match(tok->previous(), "sizeof|decltype ("))
return true;
return false;
return Token::Match(tok->previous(), "sizeof|decltype (");
}
static bool isFunctionCall(const Token* tok)

View File

@ -140,14 +140,10 @@ bool Path::isAbsolute(const std::string& path)
return false;
// On Windows, 'C:\foo\bar' is an absolute path, while 'C:foo\bar' is not
if (nativePath.compare(0, 2, "\\\\") == 0 || (std::isalpha(nativePath[0]) != 0 && nativePath.compare(1, 2, ":\\") == 0))
return true;
return nativePath.compare(0, 2, "\\\\") == 0 || (std::isalpha(nativePath[0]) != 0 && nativePath.compare(1, 2, ":\\") == 0);
#else
if (!nativePath.empty() && nativePath[0] == '/')
return true;
return !nativePath.empty() && nativePath[0] == '/';
#endif
return false;
}
std::string Path::getRelativePath(const std::string& absolutePath, const std::vector<std::string>& basePaths)

View File

@ -1754,7 +1754,7 @@ bool SymbolDatabase::isFunction(const Token *tok, const Scope* outerScope, const
// function returning function pointer? '... ( ... %name% ( ... ))( ... ) {'
// function returning reference to array '... ( & %name% ( ... ))[ ... ] {'
// TODO: Activate this again
if ((false) && tok->str() == "(" && tok->strAt(1) != "*" &&
if ((false) && tok->str() == "(" && tok->strAt(1) != "*" && // NOLINT(readability-simplify-boolean-expr)
(tok->link()->previous()->str() == ")" || Token::simpleMatch(tok->link()->tokAt(-2), ") const"))) {
const Token* tok2 = tok->link()->next();
if (tok2 && tok2->str() == "(" && Token::Match(tok2->link()->next(), "{|;|const|=")) {

View File

@ -1094,9 +1094,7 @@ public:
const Scope * parent = nestedIn;
while (outer != parent && parent)
parent = parent->nestedIn;
if (parent && parent == outer)
return true;
return false;
return parent && parent == outer;
}
static Function* nestedInFunction(const Scope* scope) {

View File

@ -1877,10 +1877,7 @@ void TemplateSimplifier::expandTemplate(
if (tok3->next() && tok3->next()->str()=="<") {
std::vector<const Token *> localTypeParametersInDeclaration;
getTemplateParametersInDeclaration(tok3->tokAt(2), localTypeParametersInDeclaration);
if (localTypeParametersInDeclaration.size() != typeParametersInDeclaration.size())
inTemplateDefinition = false; // Partial specialization
else
inTemplateDefinition = true;
inTemplateDefinition = localTypeParametersInDeclaration.size() == typeParametersInDeclaration.size(); // Partial specialization
} else {
inTemplateDefinition = false; // Only template instantiation
}

View File

@ -325,17 +325,9 @@ bool Tokenizer::duplicateTypedef(Token **tokPtr, const Token *name, const Token
return false;
}
} else if (tok->previous()->str() == "union") {
if (tok->next()->str() != ";") {
return true;
} else {
return false;
}
return tok->next()->str() != ";";
} else if (isCPP() && tok->previous()->str() == "class") {
if (tok->next()->str() != ";") {
return true;
} else {
return false;
}
return tok->next()->str() != ";";
}
if (tok)
tok = tok->previous();
@ -7367,7 +7359,7 @@ void Tokenizer::validate() const
if (tok->link() == nullptr)
cppcheckError(tok);
if (linkTokens.empty() == true)
if (linkTokens.empty())
cppcheckError(tok);
if (tok->link() != linkTokens.top())

View File

@ -621,9 +621,7 @@ static bool isQualifier(const Token* tok)
{
while (Token::Match(tok, "&|&&|*"))
tok = tok->next();
if (!Token::Match(tok, "{|;"))
return false;
return true;
return Token::Match(tok, "{|;");
}
static void compileUnaryOp(Token *&tok, AST_state& state, void (*f)(Token *&tok, AST_state& state))

View File

@ -8037,9 +8037,7 @@ bool ValueFlow::isContainerSizeChanged(const Token* tok, int indirect, const Set
case Library::Container::Action::CHANGE_INTERNAL:
break;
}
if (isContainerSizeChangedByFunction(tok, indirect, settings, depth))
return true;
return false;
return isContainerSizeChangedByFunction(tok, indirect, settings, depth);
}
static bool isContainerSizeChanged(nonneg int varId,