loplugin:stringconstant: handle OStringBuffer::append

Change-Id: I283da52c0ee2b63c19e31e9a61ab24997c037a6a

7 files changed, 139 insertions, 44 deletions

diff --git a/codemaker/source/codemaker/global.cxx b/codemaker/source/codemaker/global.cxx
index c3dc266a655d..177230982905 100644
--- a/codemaker/source/codemaker/global.cxx
+++ b/codemaker/source/codemaker/global.cxx
@@ -99,7 +99,7 @@ OString createFileNameFromType( const OString& destination,
         fileNameBuf.append(destination.getStr(), destination.getLength());
 
     if (bWithSeparator)
-        fileNameBuf.append("/", 1);
+        fileNameBuf.append("/");
 
     fileNameBuf.append(type.getStr(), type.getLength());
     fileNameBuf.append(postfix.getStr(), postfix.getLength());
diff --git a/compilerplugins/clang/stringconstant.cxx b/compilerplugins/clang/stringconstant.cxx
index 259cee7a2116..cab25bbf71cf 100644
--- a/compilerplugins/clang/stringconstant.cxx
+++ b/compilerplugins/clang/stringconstant.cxx
@@ -106,7 +106,7 @@ private:
     std::string describeChangeKind(ChangeKind kind);
 
     bool isStringConstant(
-        Expr const * expr, unsigned * size, bool * nonAscii,
+        Expr const * expr, unsigned * size, bool * nonArray, bool * nonAscii,
         bool * embeddedNuls, bool * terminatingNul);
 
     bool isZero(Expr const * expr);
@@ -477,11 +477,13 @@ bool StringConstant::VisitCallExpr(CallExpr const * expr) {
         && fdecl->getNumParams() == 1)
     {
         unsigned n;
+        bool nonArray;
         bool non;
         bool emb;
         bool trm;
         if (!isStringConstant(
-                expr->getArg(0)->IgnoreParenImpCasts(), &n, &non, &emb, &trm))
+                expr->getArg(0)->IgnoreParenImpCasts(), &n, &nonArray, &non,
+                &emb, &trm))
         {
             return true;
         }
@@ -516,12 +518,13 @@ bool StringConstant::VisitCallExpr(CallExpr const * expr) {
     {
         for (unsigned i = 0; i != 2; ++i) {
             unsigned n;
+            bool nonArray;
             bool non;
             bool emb;
             bool trm;
             if (!isStringConstant(
-                    expr->getArg(i)->IgnoreParenImpCasts(), &n, &non, &emb,
-                    &trm))
+                    expr->getArg(i)->IgnoreParenImpCasts(), &n, &nonArray, &non,
+                    &emb, &trm))
             {
                 continue;
             }
@@ -560,12 +563,13 @@ bool StringConstant::VisitCallExpr(CallExpr const * expr) {
     {
         for (unsigned i = 0; i != 2; ++i) {
             unsigned n;
+            bool nonArray;
             bool non;
             bool emb;
             bool trm;
             if (!isStringConstant(
-                    expr->getArg(i)->IgnoreParenImpCasts(), &n, &non, &emb,
-                    &trm))
+                    expr->getArg(i)->IgnoreParenImpCasts(), &n, &nonArray, &non,
+                    &emb, &trm))
             {
                 continue;
             }
@@ -603,11 +607,13 @@ bool StringConstant::VisitCallExpr(CallExpr const * expr) {
         && fdecl->getNumParams() == 1)
     {
         unsigned n;
+        bool nonArray;
         bool non;
         bool emb;
         bool trm;
         if (!isStringConstant(
-                expr->getArg(1)->IgnoreParenImpCasts(), &n, &non, &emb, &trm))
+                expr->getArg(1)->IgnoreParenImpCasts(), &n, &nonArray, &non,
+                &emb, &trm))
         {
             return true;
         }
@@ -658,6 +664,78 @@ bool StringConstant::VisitCallExpr(CallExpr const * expr) {
             TreatEmpty::Error);
         return true;
     }
+    if (dc.Function("append").Class("OStringBuffer").Namespace("rtl")
+        .GlobalNamespace())
+    {
+        switch (fdecl->getNumParams()) {
+        case 1:
+            {
+                // char const * const s = "foo"; b.append(s) ->
+                // char const s[] = "foo"; b.append(s):
+                unsigned n;
+                bool nonArray;
+                bool non;
+                bool emb;
+                bool trm;
+                if (!isStringConstant(
+                        expr->getArg(0)->IgnoreParenImpCasts(), &n, &nonArray,
+                        &non, &emb, &trm))
+                {
+                    return true;
+                }
+                if (non || emb) {
+                    return true;
+                }
+                if (!trm) {
+                    report(
+                        DiagnosticsEngine::Warning,
+                        ("call of %0 with string constant argument lacking a"
+                         " terminating NUL"),
+                        getMemberLocation(expr))
+                        << fdecl->getQualifiedNameAsString()
+                        << expr->getSourceRange();
+                    return true;
+                }
+                std::string repl;
+                checkEmpty(expr, fdecl, TreatEmpty::Error, n, &repl);
+                if (nonArray) {
+                    report(
+                        DiagnosticsEngine::Warning,
+                        ("in call of %0 with non-array string constant"
+                         " argument, change that argument into an array"),
+                        getMemberLocation(expr))
+                        << fdecl->getQualifiedNameAsString()
+                        << expr->getSourceRange();
+                    return true;
+                }
+                return true;
+            }
+        case 2:
+            {
+                // b.append("foo", 3) -> b.append("foo"):
+                std::string file(
+                    compiler.getSourceManager().getFilename(
+                        compiler.getSourceManager().getSpellingLoc(
+                            expr->getLocStart())));
+                if (file
+                    == SRCDIR "/sal/qa/OStringBuffer/rtl_OStringBuffer.cxx")
+                {
+                    return true;
+                }
+                handleCharLen(
+                    expr, 0, 1, fdecl, "rtl::OStringBuffer::append",
+                    TreatEmpty::Error);
+                return true;
+            }
+        default:
+            return true;
+        }
+    }
+    if ((dc.Function("append").Class("OStringBuffer").Namespace("rtl")
+         .GlobalNamespace())
+        && fdecl->getNumParams() == 2)
+    {
+    }
     return true;
 }
 
@@ -696,11 +774,13 @@ bool StringConstant::VisitCXXConstructExpr(CXXConstructExpr const * expr) {
                     simplify = false;
                 } else {
                     unsigned n;
+                    bool nonArray;
                     bool non;
                     bool emb;
                     bool trm;
                     if (!isStringConstant(
-                            arg->IgnoreParenImpCasts(), &n, &non, &emb, &trm))
+                            arg->IgnoreParenImpCasts(), &n, &nonArray, &non,
+                            &emb, &trm))
                     {
                         return true;
                     }
@@ -742,12 +822,13 @@ bool StringConstant::VisitCXXConstructExpr(CXXConstructExpr const * expr) {
         case 4:
             {
                 unsigned n;
+                bool nonArray;
                 bool non;
                 bool emb;
                 bool trm;
                 if (!isStringConstant(
-                        expr->getArg(0)->IgnoreParenImpCasts(), &n, &non, &emb,
-                        &trm))
+                        expr->getArg(0)->IgnoreParenImpCasts(), &n, &nonArray,
+                        &non, &emb, &trm))
                 {
                     return true;
                 }
@@ -995,11 +1076,12 @@ std::string StringConstant::describeChangeKind(ChangeKind kind) {
 }
 
 bool StringConstant::isStringConstant(
-    Expr const * expr, unsigned * size, bool * nonAscii, bool * embeddedNuls,
-    bool * terminatingNul)
+    Expr const * expr, unsigned * size, bool * nonArray, bool * nonAscii,
+    bool * embeddedNuls, bool * terminatingNul)
 {
     assert(expr != nullptr);
     assert(size != nullptr);
+    assert(nonArray != nullptr);
     assert(nonAscii != nullptr);
     assert(embeddedNuls != nullptr);
     assert(terminatingNul != nullptr);
@@ -1030,12 +1112,16 @@ bool StringConstant::isStringConstant(
             }
         }
     }
-    if (!(loplugin::TypeCheck(t).Pointer().Const().Char()
-          || (t->isConstantArrayType()
-              && (loplugin::TypeCheck(
-                      t->getAsArrayTypeUnsafe()->getElementType())
-                  .Char()))))
+    bool isPtr;
+    if (loplugin::TypeCheck(t).Pointer().Const().Char()) {
+        isPtr = true;
+    } else if (t->isConstantArrayType()
+               && (loplugin::TypeCheck(
+                       t->getAsArrayTypeUnsafe()->getElementType())
+                   .Char()))
     {
+        isPtr = false;
+    } else {
         return false;
     }
     clang::StringLiteral const * lit = dyn_cast<clang::StringLiteral>(expr);
@@ -1055,6 +1141,7 @@ bool StringConstant::isStringConstant(
             }
         }
         *size = n;
+        *nonArray = isPtr;
         *nonAscii = non;
         *embeddedNuls = emb;
         *terminatingNul = true;
@@ -1077,7 +1164,7 @@ bool StringConstant::isStringConstant(
             Expr const * e2 = e->IgnoreParenImpCasts();
             if (e2 != e) {
                 return isStringConstant(
-                    e2, size, nonAscii, embeddedNuls, terminatingNul);
+                    e2, size, nonArray, nonAscii, embeddedNuls, terminatingNul);
             }
             //TODO: string literals are represented as recursive LValues???
             llvm::APInt n
@@ -1086,6 +1173,7 @@ bool StringConstant::isStringConstant(
             --n;
             assert(n.ule(std::numeric_limits<unsigned>::max()));
             *size = static_cast<unsigned>(n.getLimitedValue());
+            *nonArray = isPtr || *nonArray;
             *nonAscii = false; //TODO
             *embeddedNuls = false; //TODO
             *terminatingNul = true;
@@ -1118,6 +1206,7 @@ bool StringConstant::isStringConstant(
             }
             bool trm = e.getInt() == 0;
             *size = trm ? n - 1 : n;
+            *nonArray = isPtr;
             *nonAscii = non;
             *embeddedNuls = emb;
             *terminatingNul = trm;
@@ -1353,11 +1442,13 @@ void StringConstant::handleChar(
     char const * rewriteFrom, char const * rewriteTo)
 {
     unsigned n;
+    bool nonArray;
     bool non;
     bool emb;
     bool trm;
     if (!isStringConstant(
-            expr->getArg(arg)->IgnoreParenImpCasts(), &n, &non, &emb, &trm))
+            expr->getArg(arg)->IgnoreParenImpCasts(), &n, &nonArray, &non, &emb,
+            &trm))
     {
         return;
     }
@@ -1411,11 +1502,13 @@ void StringConstant::handleCharLen(
     // that at the level of non-expanded macros instead, but I have not found
     // out how to do that yet anyway):
     unsigned n;
+    bool nonArray;
     bool non;
     bool emb;
     bool trm;
     if (!(isStringConstant(
-              expr->getArg(arg1)->IgnoreParenImpCasts(), &n, &non, &emb, &trm)
+              expr->getArg(arg1)->IgnoreParenImpCasts(), &n, &nonArray, &non,
+              &emb, &trm)
           && trm))
     {
         return;
@@ -1437,12 +1530,13 @@ void StringConstant::handleCharLen(
             return;
         }
         unsigned n2;
+        bool nonArray2;
         bool non2;
         bool emb2;
         bool trm2;
         if (!(isStringConstant(
-                  subs->getBase()->IgnoreParenImpCasts(), &n2, &non2, &emb2,
-                  &trm2)
+                  subs->getBase()->IgnoreParenImpCasts(), &n2, &nonArray2,
+                  &non2, &emb2, &trm2)
               && n2 == n && non2 == non && emb2 == emb && trm2 == trm
                   //TODO: same strings
               && subs->getIdx()->EvaluateAsInt(res, compiler.getASTContext())
@@ -1527,11 +1621,13 @@ void StringConstant::handleOUStringCtor(
         return;
     }
     unsigned n;
+    bool nonArray;
     bool non;
     bool emb;
     bool trm;
     if (!isStringConstant(
-            e3->getArg(0)->IgnoreParenImpCasts(), &n, &non, &emb, &trm))
+            e3->getArg(0)->IgnoreParenImpCasts(), &n, &nonArray, &non, &emb,
+            &trm))
     {
         return;
     }
diff --git a/sal/qa/rtl/oustringbuffer/test_oustringbuffer_utf32.cxx b/sal/qa/rtl/oustringbuffer/test_oustringbuffer_utf32.cxx
index d47fd5f0efc0..e71614641ce0 100644
--- a/sal/qa/rtl/oustringbuffer/test_oustringbuffer_utf32.cxx
+++ b/sal/qa/rtl/oustringbuffer/test_oustringbuffer_utf32.cxx
@@ -49,7 +49,7 @@ namespace {
 void appendString(rtl::OStringBuffer & buffer, rtl::OUString const & string) {
     buffer.append('"');
     for (int i = 0; i < string.getLength(); ++i) {
-        buffer.append(RTL_CONSTASCII_STRINGPARAM("\\u"));
+        buffer.append("\\u");
         sal_Unicode c = string[i];
         if (c < 0x1000) {
             buffer.append('0');
@@ -72,7 +72,7 @@ void createMessage(
 {
     message.setLength(0);
     appendString(message, string1);
-    message.append(RTL_CONSTASCII_STRINGPARAM(" vs. "));
+    message.append(" vs. ");
     appendString(message, string2);
 }
 
diff --git a/sal/qa/rtl/strings/test_oustring_convert.cxx b/sal/qa/rtl/strings/test_oustring_convert.cxx
index 639d9b744555..f92ce9378301 100644
--- a/sal/qa/rtl/strings/test_oustring_convert.cxx
+++ b/sal/qa/rtl/strings/test_oustring_convert.cxx
@@ -63,28 +63,28 @@ void testConvertToString(TestConvertToString const & rTest)
                                                  rTest.nFlags));
 
     rtl::OStringBuffer aPrefix;
-    aPrefix.append(RTL_CONSTASCII_STRINGPARAM("{"));
+    aPrefix.append("{");
     for (sal_Int32 i = 0; i < rTest.nLength; ++i)
     {
-        aPrefix.append(RTL_CONSTASCII_STRINGPARAM("U+"));
+        aPrefix.append("U+");
         aPrefix.append(static_cast< sal_Int32 >(rTest.aSource[i]), 16);
         if (i + 1 < rTest.nLength)
-            aPrefix.append(RTL_CONSTASCII_STRINGPARAM(","));
+            aPrefix.append(",");
     }
-    aPrefix.append(RTL_CONSTASCII_STRINGPARAM("}, "));
+    aPrefix.append("}, ");
     aPrefix.append(static_cast< sal_Int32 >(rTest.nEncoding));
-    aPrefix.append(RTL_CONSTASCII_STRINGPARAM(", 0x"));
+    aPrefix.append(", 0x");
     aPrefix.append(static_cast< sal_Int32 >(rTest.nFlags), 16);
-    aPrefix.append(RTL_CONSTASCII_STRINGPARAM(" -> "));
+    aPrefix.append(" -> ");
 
     if (bSuccess)
     {
         if (rTest.pStrict == nullptr || !aStrict.equals(rTest.pStrict))
         {
             rtl::OStringBuffer aMessage(aPrefix);
-            aMessage.append(RTL_CONSTASCII_STRINGPARAM("strict = \""));
+            aMessage.append("strict = \"");
             aMessage.append(aStrict);
-            aMessage.append(RTL_CONSTASCII_STRINGPARAM("\""));
+            aMessage.append("\"");
             CPPUNIT_ASSERT_MESSAGE(aMessage.getStr(), false);
         }
     }
@@ -93,22 +93,22 @@ void testConvertToString(TestConvertToString const & rTest)
         if (!aStrict.equals(rtl::OString(RTL_CONSTASCII_STRINGPARAM("12345"))))
         {
             rtl::OStringBuffer aMessage(aPrefix);
-            aMessage.append(RTL_CONSTASCII_STRINGPARAM("modified output"));
+            aMessage.append("modified output");
             CPPUNIT_ASSERT_MESSAGE(aMessage.getStr(), false);
         }
         if (rTest.pStrict != nullptr)
         {
             rtl::OStringBuffer aMessage(aPrefix);
-            aMessage.append(RTL_CONSTASCII_STRINGPARAM("failed"));
+            aMessage.append("failed");
             CPPUNIT_ASSERT_MESSAGE(aMessage.getStr(), false);
         }
     }
     if (!aRelaxed.equals(rTest.pRelaxed))
     {
         rtl::OStringBuffer aMessage(aPrefix);
-        aMessage.append(RTL_CONSTASCII_STRINGPARAM("relaxed = \""));
+        aMessage.append("relaxed = \"");
         aMessage.append(aRelaxed);
-        aMessage.append(RTL_CONSTASCII_STRINGPARAM("\""));
+        aMessage.append("\"");
         CPPUNIT_ASSERT_MESSAGE(aMessage.getStr(), false);
     }
 }
diff --git a/sal/qa/rtl/strings/test_oustring_endswith.cxx b/sal/qa/rtl/strings/test_oustring_endswith.cxx
index 37d5c608c127..8912a4027027 100644
--- a/sal/qa/rtl/strings/test_oustring_endswith.cxx
+++ b/sal/qa/rtl/strings/test_oustring_endswith.cxx
@@ -96,10 +96,9 @@ void test::oustring::EndsWith::endsWith()
     for (size_t i = 0; i < SAL_N_ELEMENTS(data); ++i) {
         rtl::OStringBuffer msg;
         appendString(msg, rtl::OString(data[i].str1, data[i].str1Len));
-        msg.append(
-            RTL_CONSTASCII_STRINGPARAM(".endsWithIgnoreAsciiCaseAsciiL("));
+        msg.append(".endsWithIgnoreAsciiCaseAsciiL(");
         appendString(msg, rtl::OString(data[i].str2, data[i].str2Len));
-        msg.append(RTL_CONSTASCII_STRINGPARAM(") == "));
+        msg.append(") == ");
         msg.append(data[i].endsWith);
         CPPUNIT_ASSERT_EQUAL_MESSAGE(
             msg.getStr(),
diff --git a/sdext/source/pdfimport/pdfparse/pdfentries.cxx b/sdext/source/pdfimport/pdfparse/pdfentries.cxx
index 237609ef76fd..ab8a2b999347 100644
--- a/sdext/source/pdfimport/pdfparse/pdfentries.cxx
+++ b/sdext/source/pdfimport/pdfparse/pdfentries.cxx
@@ -433,7 +433,7 @@ bool PDFObjectRef::emit( EmitContext& rWriteContext ) const
     aBuf.append( sal_Int32( m_nNumber ) );
     aBuf.append( ' ' );
     aBuf.append( sal_Int32( m_nGeneration ) );
-    aBuf.append( " R", 2 );
+    aBuf.append( " R" );
     return rWriteContext.write( aBuf.getStr(), aBuf.getLength() );
 }
 
diff --git a/sw/source/filter/html/css1atr.cxx b/sw/source/filter/html/css1atr.cxx
index 08acd4222a15..3134a1c8265a 100644
--- a/sw/source/filter/html/css1atr.cxx
+++ b/sw/source/filter/html/css1atr.cxx
@@ -163,8 +163,8 @@ static Writer& OutCSS1_SwFormatLayoutSplit( Writer& rWrt, const SfxPoolItem& rHt
 namespace
 {
 
-const sal_Char* const sCSS1_rule_end      = " }";
-const sal_Char* const sCSS1_span_tag_end  = "\">";
+const sal_Char sCSS1_rule_end[] = " }";
+const sal_Char sCSS1_span_tag_end[] = "\">";
 const sal_Char cCSS1_style_opt_end  = '\"';
 
 const sal_Char* const sHTML_FTN_fontheight = "57%";