shiboken2: Improve whitespaces formatting in injected code

- Trim newlines and dedent all snippets from XML
- Move the trailing newlines into the code snippet function.
- Do not indent preprocessor directives

Change-Id: Ic8c3745f0b88ee4ed27ac9cbaf376d71a50d9188
Reviewed-by: Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io>

8 files changed, 92 insertions, 33 deletions

diff --git a/sources/shiboken2/ApiExtractor/tests/testaddfunction.cpp b/sources/shiboken2/ApiExtractor/tests/testaddfunction.cpp
index c50084b8e..ca4af9a10 100644
--- a/sources/shiboken2/ApiExtractor/tests/testaddfunction.cpp
+++ b/sources/shiboken2/ApiExtractor/tests/testaddfunction.cpp
@@ -283,7 +283,7 @@ void TestAddFunction::testAddFunctionAtModuleLevel()
     QCOMPARE(mods.size(), 1);
     QVERIFY(mods.first().isCodeInjection());
     CodeSnip snip = mods.first().snips.first();
-    QCOMPARE(snip.code(), QLatin1String("custom_code();"));
+    QCOMPARE(snip.code().trimmed(), QLatin1String("custom_code();"));
 }
 
 void TestAddFunction::testAddFunctionWithVarargs()
diff --git a/sources/shiboken2/ApiExtractor/tests/testnestedtypes.cpp b/sources/shiboken2/ApiExtractor/tests/testnestedtypes.cpp
index 10194eb34..e61418467 100644
--- a/sources/shiboken2/ApiExtractor/tests/testnestedtypes.cpp
+++ b/sources/shiboken2/ApiExtractor/tests/testnestedtypes.cpp
@@ -69,7 +69,7 @@ void TestNestedTypes::testNestedTypesModifications()
     QCOMPARE(ins->functions().count(), 1);
     QCOMPARE(ins->typeEntry()->codeSnips().count(), 1);
     CodeSnip snip = ins->typeEntry()->codeSnips().first();
-    QCOMPARE(snip.code(), QLatin1String("custom_code1();"));
+    QCOMPARE(snip.code().trimmed(), QLatin1String("custom_code1();"));
 
     AbstractMetaFunction* addedFunc = ins->functions().first();
     QVERIFY(addedFunc->isUserAdded());
@@ -80,7 +80,7 @@ void TestNestedTypes::testNestedTypesModifications()
     QCOMPARE(addedFunc->modifications().size(), 1);
     QVERIFY(addedFunc->modifications().first().isCodeInjection());
     snip = addedFunc->modifications().first().snips.first();
-    QCOMPARE(snip.code(), QLatin1String("custom_code2();"));
+    QCOMPARE(snip.code().trimmed(), QLatin1String("custom_code2();"));
 
     const AbstractMetaClass *sc = AbstractMetaClass::findClass(classes, QLatin1String("OuterNamespace::InnerNamespace::SomeClass"));
     QVERIFY(ins);
diff --git a/sources/shiboken2/ApiExtractor/typesystem.cpp b/sources/shiboken2/ApiExtractor/typesystem.cpp
index 920da9e10..e6cf02cbd 100644
--- a/sources/shiboken2/ApiExtractor/typesystem.cpp
+++ b/sources/shiboken2/ApiExtractor/typesystem.cpp
@@ -34,6 +34,7 @@
 #include <QtCore/QSet>
 
 #include <algorithm>
+#include <limits>
 
 static QString strings_Object = QLatin1String("Object");
 static QString strings_String = QLatin1String("String");
@@ -212,7 +213,7 @@ QString TemplateInstance::expandCode() const
     if (!code.startsWith(QLatin1Char('\n')))
         result += QLatin1Char('\n');
     result += code;
-    result += QLatin1String("\n// TEMPLATE - ") + m_name + QLatin1String(" - END");
+    result += QLatin1String("\n// TEMPLATE - ") + m_name + QLatin1String(" - END\n");
     return result;
 }
 
@@ -226,6 +227,68 @@ QString CodeSnipAbstract::code() const
     return res;
 }
 
+void CodeSnipAbstract::addCode(const QString &code)
+{
+    codeList.append(CodeSnipFragment(fixSpaces(code)));
+}
+
+template <class String> // QString, QStringRef
+static inline int firstNonBlank(const String &s)
+{
+    const auto it = std::find_if(s.cbegin(), s.cend(),
+                                 [] (QChar c) { return !c.isSpace(); });
+    return int(it - s.cbegin());
+}
+
+template <class String> // QString, QStringRef
+static inline bool isEmpty(const String &s)
+{
+    return s.isEmpty()
+        || std::all_of(s.cbegin(), s.cend(),
+                       [] (QChar c) { return c.isSpace(); });
+}
+
+QString CodeSnipAbstract::dedent(const QString &code)
+{
+    if (code.isEmpty())
+        return code;
+    // Right trim if indent=0, or trim if single line
+    if (!code.at(0).isSpace() || !code.contains(QLatin1Char('\n')))
+        return code.trimmed();
+    const auto lines = code.splitRef(QLatin1Char('\n'));
+    int spacesToRemove = std::numeric_limits<int>::max();
+    for (const auto &line : lines) {
+        if (!isEmpty(line)) {
+            const int nonSpacePos = firstNonBlank(line);
+            if (nonSpacePos < spacesToRemove)
+                spacesToRemove = nonSpacePos;
+            if (spacesToRemove == 0)
+                return code;
+        }
+    }
+    QString result;
+    for (const auto &line : lines) {
+        if (!isEmpty(line) && spacesToRemove < line.size())
+            result += line.mid(spacesToRemove).toString();
+        result += QLatin1Char('\n');
+    }
+    return result;
+}
+
+QString CodeSnipAbstract::fixSpaces(QString code)
+{
+    code.remove(QLatin1Char('\r'));
+    // Check for XML <tag>\n<space>bla...
+    if (code.startsWith(QLatin1String("\n ")))
+        code.remove(0, 1);
+    while (!code.isEmpty() && code.back().isSpace())
+        code.chop(1);
+    code = dedent(code);
+    if (!code.isEmpty() && !code.endsWith(QLatin1Char('\n')))
+        code.append(QLatin1Char('\n'));
+    return code;
+}
+
 QString CodeSnipFragment::code() const
 {
     return m_instance ? m_instance->expandCode() : m_code;
@@ -1184,3 +1247,8 @@ TypeEntry *ObjectTypeEntry::clone() const
 }
 
 ObjectTypeEntry::ObjectTypeEntry(const ObjectTypeEntry &) = default;
+
+void DocModification::setCode(const QString &code)
+{
+    m_code = CodeSnipAbstract::fixSpaces(code);
+}
diff --git a/sources/shiboken2/ApiExtractor/typesystem.h b/sources/shiboken2/ApiExtractor/typesystem.h
index 6d2d4bb44..31907435b 100644
--- a/sources/shiboken2/ApiExtractor/typesystem.h
+++ b/sources/shiboken2/ApiExtractor/typesystem.h
@@ -115,7 +115,7 @@ class CodeSnipAbstract
 public:
     QString code() const;
 
-    void addCode(const QString &code) { codeList.append(CodeSnipFragment(code)); }
+    void addCode(const QString &code);
     void addCode(const QStringRef &code) { addCode(code.toString()); }
 
     void addTemplateInstance(TemplateInstance *ti)
@@ -124,6 +124,9 @@ public:
     }
 
     QVector<CodeSnipFragment> codeList;
+
+    static QString fixSpaces(QString code);
+    static QString dedent(const QString &code);
 };
 
 class CustomFunction : public CodeSnipAbstract
@@ -510,8 +513,8 @@ public:
     explicit DocModification(TypeSystem::DocModificationMode mode, const QString& signature) :
         m_signature(signature), m_mode(mode) {}
 
-    void setCode(const QString& code) { m_code = code; }
-    void setCode(const QStringRef& code) { m_code = code.toString(); }
+    void setCode(const QString& code);
+    void setCode(const QStringRef& code) { setCode(code.toString()); }
 
     QString code() const
     {
diff --git a/sources/shiboken2/ApiExtractor/typesystemparser.cpp b/sources/shiboken2/ApiExtractor/typesystemparser.cpp
index 9fdf81821..0ba94d94e 100644
--- a/sources/shiboken2/ApiExtractor/typesystemparser.cpp
+++ b/sources/shiboken2/ApiExtractor/typesystemparser.cpp
@@ -149,7 +149,7 @@ static QString extractSnippet(const QString &code, const QString &snippetLabel)
         } else if (useLine)
             result += line.toString() + QLatin1Char('\n');
     }
-    return result;
+    return CodeSnipAbstract::fixSpaces(result);
 }
 
 template <class EnumType, Qt::CaseSensitivity cs = Qt::CaseInsensitive>
@@ -2475,7 +2475,7 @@ bool TypeSystemParser::readFileSnippet(QXmlStreamAttributes *attributes, CodeSni
            "// START of custom code block [file: "
         << source << "]\n"
         << extractSnippet(QString::fromUtf8(codeFile.readAll()), snippetLabel)
-        << "\n// END of custom code block [file: " << source
+        << "// END of custom code block [file: " << source
         << "]\n// ========================================================================\n";
     snip->addCode(content);
     return true;
diff --git a/sources/shiboken2/generator/generator.cpp b/sources/shiboken2/generator/generator.cpp
index 88ba1f04f..4eabb3d2c 100644
--- a/sources/shiboken2/generator/generator.cpp
+++ b/sources/shiboken2/generator/generator.cpp
@@ -558,7 +558,9 @@ QTextStream &formatCode(QTextStream &s, const QString &code, Indentor &indentor)
     Q_ASSERT(emptyLine.isValid());
 
     for (QString line : lst) {
-        if (!line.isEmpty() && !emptyLine.match(line).hasMatch()) {
+        if (line.startsWith(QLatin1Char('#'))) {
+            s << line; // Do not indent preprocessor lines
+        } else if (!line.isEmpty() && !emptyLine.match(line).hasMatch()) {
             while (line.constEnd()->isSpace())
                 line.chop(1);
             int limit = 0;
diff --git a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
index 8e9883eaf..9bcece86d 100644
--- a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
+++ b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp
@@ -859,7 +859,6 @@ void CppGenerator::writeVirtualMethodNative(QTextStream &s,
         CodeSnipList snips = func->injectedCodeSnips();
         const AbstractMetaArgument *lastArg = func->arguments().isEmpty() ? nullptr : func->arguments().constLast();
         writeCodeSnips(s, snips, TypeSystem::CodeSnipPositionDeclaration, TypeSystem::NativeCode, func, lastArg);
-        s << Qt::endl;
     }
 
     // PYSIDE-803: Build a boolean cache for unused overrides.
@@ -913,7 +912,6 @@ void CppGenerator::writeVirtualMethodNative(QTextStream &s,
             snips = func->injectedCodeSnips();
             const AbstractMetaArgument *lastArg = func->arguments().isEmpty() ? nullptr : func->arguments().constLast();
             writeCodeSnips(s, snips, TypeSystem::CodeSnipPositionBeginning, TypeSystem::ShellCode, func, lastArg);
-            s << Qt::endl;
         }
 
         if (func->isAbstract()) {
@@ -1017,7 +1015,6 @@ void CppGenerator::writeVirtualMethodNative(QTextStream &s,
 
         const AbstractMetaArgument *lastArg = func->arguments().isEmpty() ? nullptr : func->arguments().constLast();
         writeCodeSnips(s, snips, TypeSystem::CodeSnipPositionBeginning, TypeSystem::NativeCode, func, lastArg);
-        s << Qt::endl;
     }
 
     if (!injectedCodeCallsPythonOverride(func)) {
@@ -1349,7 +1346,7 @@ void CppGenerator::writeConverterFunctions(QTextStream &s, const AbstractMetaCla
             << INDENT << "if (sbkType && Shiboken::ObjectType::hasSpecialCastFunction(sbkType)) {\n"
             << INDENT << "    typeName = typeNameOf(tCppIn);\n"
             << INDENT << "    changedTypeName = true;\n"
-            << INDENT << " }\n"
+            << INDENT << "}\n"
             << INDENT << "PyObject *result = Shiboken::Object::newObject(" << cpythonType
             <<           ", const_cast<void *>(cppIn), false, /* exactType */ changedTypeName, typeName);\n"
             << INDENT << "if (changedTypeName)\n"
@@ -3233,7 +3230,6 @@ void CppGenerator::writeMethodCall(QTextStream &s, const AbstractMetaFunction *f
         }
 
         writeCodeSnips(s, snips, TypeSystem::CodeSnipPositionBeginning, TypeSystem::TargetLangCode, func, lastArg);
-        s << Qt::endl;
     }
 
     writeConversionRule(s, func, TypeSystem::NativeCode);
@@ -3524,10 +3520,8 @@ void CppGenerator::writeMethodCall(QTextStream &s, const AbstractMetaFunction *f
         }
     }
 
-    if (func->hasInjectedCode() && !func->isConstructor()) {
-        s << Qt::endl;
+    if (func->hasInjectedCode() && !func->isConstructor())
         writeCodeSnips(s, snips, TypeSystem::CodeSnipPositionEnd, TypeSystem::TargetLangCode, func, lastArg);
-    }
 
     bool hasReturnPolicy = false;
 
@@ -5687,10 +5681,8 @@ bool CppGenerator::finishGeneration()
     const CodeSnipList snips = moduleEntry->codeSnips();
 
     // module inject-code native/beginning
-    if (!snips.isEmpty()) {
+    if (!snips.isEmpty())
         writeCodeSnips(s, snips, TypeSystem::CodeSnipPositionBeginning, TypeSystem::NativeCode);
-        s << Qt::endl;
-    }
 
     // cleanup staticMetaObject attribute
     if (usePySideExtensions()) {
@@ -5821,10 +5813,8 @@ bool CppGenerator::finishGeneration()
 
     ErrorCode errorCode(QLatin1String("SBK_MODULE_INIT_ERROR"));
     // module inject-code target/beginning
-    if (!snips.isEmpty()) {
+    if (!snips.isEmpty())
         writeCodeSnips(s, snips, TypeSystem::CodeSnipPositionBeginning, TypeSystem::TargetLangCode);
-        s << Qt::endl;
-    }
 
     for (const QString &requiredModule : requiredModules) {
         s << INDENT << "{\n";
@@ -5926,16 +5916,12 @@ bool CppGenerator::finishGeneration()
     s << INDENT << "}\n";
 
     // module inject-code target/end
-    if (!snips.isEmpty()) {
+    if (!snips.isEmpty())
         writeCodeSnips(s, snips, TypeSystem::CodeSnipPositionEnd, TypeSystem::TargetLangCode);
-        s << Qt::endl;
-    }
 
     // module inject-code native/end
-    if (!snips.isEmpty()) {
+    if (!snips.isEmpty())
         writeCodeSnips(s, snips, TypeSystem::CodeSnipPositionEnd, TypeSystem::NativeCode);
-        s << Qt::endl;
-    }
 
     if (usePySideExtensions()) {
         for (AbstractMetaEnum *metaEnum : qAsConst(globalEnums))
diff --git a/sources/shiboken2/generator/shiboken2/shibokengenerator.cpp b/sources/shiboken2/generator/shiboken2/shibokengenerator.cpp
index fe0d2765c..6abaef698 100644
--- a/sources/shiboken2/generator/shiboken2/shibokengenerator.cpp
+++ b/sources/shiboken2/generator/shiboken2/shibokengenerator.cpp
@@ -1755,7 +1755,7 @@ void ShibokenGenerator::writeClassCodeSnips(QTextStream &s,
     processClassCodeSnip(code, context);
     s << INDENT << "// Begin code injection\n";
     s << code;
-    s << INDENT << "// End of code injection\n";
+    s << INDENT << "// End of code injection\n\n";
 }
 
 void ShibokenGenerator::writeCodeSnips(QTextStream &s,
@@ -1769,7 +1769,7 @@ void ShibokenGenerator::writeCodeSnips(QTextStream &s,
     processCodeSnip(code);
     s << INDENT << "// Begin code injection\n";
     s << code;
-    s << INDENT << "// End of code injection\n";
+    s << INDENT << "// End of code injection\n\n";
 }
 
 void ShibokenGenerator::writeCodeSnips(QTextStream &s,
@@ -1996,7 +1996,7 @@ void ShibokenGenerator::writeCodeSnips(QTextStream &s,
     processCodeSnip(code);
     s << INDENT << "// Begin code injection\n";
     s << code;
-    s << INDENT << "// End of code injection\n";
+    s << INDENT << "// End of code injection\n\n";
 }
 
 // Returns true if the string is an expression,