loplugin:salbool: Warn about uses of sal_False/True
...that can generally be rewritten as false/true, and sometimes could hide errors, see e.g. <5be5f00fe16b0e255b31fbaba5f119773d1cd071> "So this is apparently about right-to-left levels, not a boolean flag". Change-Id: Ib39a936a632c2aab206f24c346252e31dcbb98f3
This commit is contained in:
@@ -150,7 +150,11 @@ public:
|
||||
|
||||
bool VisitValueDecl(ValueDecl const * decl);
|
||||
|
||||
bool TraverseStaticAssertDecl(StaticAssertDecl * decl);
|
||||
|
||||
private:
|
||||
bool isFromCIncludeFile(SourceLocation spellingLocation) const;
|
||||
|
||||
bool isInSpecialMainFile(SourceLocation spellingLocation) const;
|
||||
|
||||
bool rewrite(SourceLocation location);
|
||||
@@ -276,6 +280,43 @@ bool SalBool::VisitCStyleCastExpr(CStyleCastExpr * expr) {
|
||||
StringRef name { Lexer::getImmediateMacroName(
|
||||
loc, compiler.getSourceManager(), compiler.getLangOpts()) };
|
||||
if (name == "sal_False" || name == "sal_True") {
|
||||
auto callLoc = compiler.getSourceManager().getSpellingLoc(
|
||||
compiler.getSourceManager().getImmediateMacroCallerLoc(
|
||||
loc));
|
||||
if (!isFromCIncludeFile(callLoc)) {
|
||||
SourceLocation argLoc;
|
||||
if (compiler.getSourceManager().isMacroArgExpansion(
|
||||
expr->getLocStart(), &argLoc)
|
||||
//TODO: check its the complete (first) arg to the macro
|
||||
&& (Lexer::getImmediateMacroName(
|
||||
argLoc, compiler.getSourceManager(),
|
||||
compiler.getLangOpts())
|
||||
== "CPPUNIT_ASSERT_EQUAL"))
|
||||
{
|
||||
// Ignore sal_False/True that are directly used as
|
||||
// arguments to CPPUNIT_ASSERT_EQUAL:
|
||||
return true;
|
||||
}
|
||||
bool b = name == "sal_True";
|
||||
if (rewriter != nullptr) {
|
||||
unsigned n = Lexer::MeasureTokenLength(
|
||||
callLoc, compiler.getSourceManager(),
|
||||
compiler.getLangOpts());
|
||||
if (StringRef(
|
||||
compiler.getSourceManager().getCharacterData(
|
||||
callLoc),
|
||||
n)
|
||||
== name)
|
||||
{
|
||||
return replaceText(
|
||||
callLoc, n, b ? "true" : "false");
|
||||
}
|
||||
}
|
||||
report(
|
||||
DiagnosticsEngine::Warning,
|
||||
"use '%select{false|true}0' instead of '%1'", callLoc)
|
||||
<< b << name << expr->getSourceRange();
|
||||
}
|
||||
return true;
|
||||
}
|
||||
}
|
||||
@@ -578,6 +619,26 @@ bool SalBool::VisitValueDecl(ValueDecl const * decl) {
|
||||
return true;
|
||||
}
|
||||
|
||||
bool SalBool::TraverseStaticAssertDecl(StaticAssertDecl * decl) {
|
||||
// Ignore special code like
|
||||
//
|
||||
// static_cast<sal_Bool>(true) == sal_True
|
||||
//
|
||||
// inside static_assert in cppu/source/uno/check.cxx:
|
||||
return
|
||||
(compiler.getSourceManager().getFilename(decl->getLocation())
|
||||
== SRCDIR "/cppu/source/uno/check.cxx")
|
||||
|| RecursiveASTVisitor::TraverseStaticAssertDecl(decl);
|
||||
}
|
||||
|
||||
bool SalBool::isFromCIncludeFile(SourceLocation spellingLocation) const {
|
||||
return !compat::isInMainFile(compiler.getSourceManager(), spellingLocation)
|
||||
&& (StringRef(
|
||||
compiler.getSourceManager().getPresumedLoc(spellingLocation)
|
||||
.getFilename())
|
||||
.endswith(".h"));
|
||||
}
|
||||
|
||||
bool SalBool::isInSpecialMainFile(SourceLocation spellingLocation) const {
|
||||
if (!compat::isInMainFile(compiler.getSourceManager(), spellingLocation)) {
|
||||
return false;
|
||||
|
Reference in New Issue
Block a user