From ffcbbecf271d23830dbe0515586044ffc05f2fc2 Mon Sep 17 00:00:00 2001
From: Semih Yavuz <semih.yavuz@qt.io>
Date: Sat, 6 May 2023 17:06:17 +0200
Subject: [PATCH] reformatter: Do not remove type annotations

Type annotations were being removed by the reformat action. Write out
the relavant annotations in ast to fix it.

Add exclusion mechanism in tst_qml_reformatter test since it performs
line by line comparison which doesn't fit all reformatting cases.
Introduce char-by-char data tests.

Fixes: QTCREATORBUG-29061
Change-Id: Ia52b51e6d7d938bdec325c4f426b11c722f85f8e
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
---
 src/libs/qmljs/qmljsreformatter.cpp           |  8 +++
 .../auto/qml/reformatter/tst_reformatter.cpp  | 53 ++++++++++++++++++-
 .../reformatter/typeAnnotations.formatted.qml | 18 +++++++
 .../auto/qml/reformatter/typeAnnotations.qml  | 17 ++++++
 4 files changed, 95 insertions(+), 1 deletion(-)
 create mode 100644 tests/auto/qml/reformatter/typeAnnotations.formatted.qml
 create mode 100644 tests/auto/qml/reformatter/typeAnnotations.qml

diff --git a/src/libs/qmljs/qmljsreformatter.cpp b/src/libs/qmljs/qmljsreformatter.cpp
index d2891451e95..008968afc46 100644
--- a/src/libs/qmljs/qmljsreformatter.cpp
+++ b/src/libs/qmljs/qmljsreformatter.cpp
@@ -1282,6 +1282,10 @@ protected:
         out(ast->rparenToken);
         if (ast->isArrowFunction && !ast->formals)
             out("()");
+        if (ast->typeAnnotation) {
+            out(": ");
+            out(ast->typeAnnotation->type->toString());
+        }
         out(" ");
         if (ast->isArrowFunction)
             out("=> ");
@@ -1406,6 +1410,10 @@ protected:
     {
         for (FormalParameterList *it = ast; it; it = it->next) {
             accept(it->element);
+            if (it->element->typeAnnotation) {
+                out(": ");
+                out(it->element->typeAnnotation->type->toString());
+            }
             if (it->next)
                 out(", ");
         }
diff --git a/tests/auto/qml/reformatter/tst_reformatter.cpp b/tests/auto/qml/reformatter/tst_reformatter.cpp
index 36c8f2a83e8..748b8b81263 100644
--- a/tests/auto/qml/reformatter/tst_reformatter.cpp
+++ b/tests/auto/qml/reformatter/tst_reformatter.cpp
@@ -28,6 +28,11 @@ public:
 private slots:
     void test();
     void test_data();
+
+    void reformatter_data();
+    void reformatter();
+
+private:
 };
 
 tst_Reformatter::tst_Reformatter()
@@ -41,10 +46,18 @@ void tst_Reformatter::test_data()
 {
     QTest::addColumn<QString>("path");
 
+    // This test performs line-by-line comparison and fails if reformatting
+    // makes a change inline, for example whitespace removal. We omit
+    // those files in this test.
+    QSet<QString> excludedFiles;
+    excludedFiles << QString::fromLatin1(TESTSRCDIR) + QDir::separator() + "typeAnnotations.qml";
+    excludedFiles << QString::fromLatin1(TESTSRCDIR) + QDir::separator() + "typeAnnotations.formatted.qml";
+
     QDirIterator it(TESTSRCDIR, QStringList() << QLatin1String("*.qml") << QLatin1String("*.js"), QDir::Files);
     while (it.hasNext()) {
         const QString fileName = it.next();
-        QTest::newRow(fileName.toLatin1()) << it.filePath();
+        if (!excludedFiles.contains(fileName))
+            QTest::newRow(fileName.toLatin1()) << it.filePath();
     }
 }
 
@@ -84,6 +97,44 @@ void tst_Reformatter::test()
     QCOMPARE(sourceLines.size(), newLines.size());
 }
 
+void tst_Reformatter::reformatter_data()
+{
+    QTest::addColumn<QString>("filePath");
+    QTest::addColumn<QString>("formattedFilePath");
+
+    QTest::newRow("typeAnnotations")
+        << QString::fromLatin1(TESTSRCDIR) + QDir::separator() + "typeAnnotations.qml"
+        << QString::fromLatin1(TESTSRCDIR) + QDir::separator() + "typeAnnotations.formatted.qml";
+}
+
+void tst_Reformatter::reformatter()
+{
+    QFETCH(QString, filePath);
+    QFETCH(QString, formattedFilePath);
+
+    Utils::FilePath fPath = Utils::FilePath::fromString(filePath);
+    Document::MutablePtr doc
+        = Document::create(fPath, ModelManagerInterface::guessLanguageOfFile(fPath));
+
+    QString fileContent;
+    {
+        QFile file(filePath);
+        QVERIFY(file.open(QFile::ReadOnly | QFile::Text));
+        fileContent = QString::fromUtf8(file.readAll());
+    }
+    doc->setSource(fileContent);
+    doc->parse();
+    QString expected;
+    {
+        QFile file(formattedFilePath);
+        QVERIFY(file.open(QFile::ReadOnly | QFile::Text));
+        expected = QString::fromUtf8(file.readAll());
+    }
+
+    QString formatted = reformat(doc);
+    QCOMPARE(formatted, expected);
+}
+
 QTEST_GUILESS_MAIN(tst_Reformatter);
 
 #include "tst_reformatter.moc"
diff --git a/tests/auto/qml/reformatter/typeAnnotations.formatted.qml b/tests/auto/qml/reformatter/typeAnnotations.formatted.qml
new file mode 100644
index 00000000000..208aa25cd9a
--- /dev/null
+++ b/tests/auto/qml/reformatter/typeAnnotations.formatted.qml
@@ -0,0 +1,18 @@
+
+// Copyright (C) 2023 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
+import QtQuick
+
+Text {
+    function aaa(t: int, k: double): int {
+        return 42
+    }
+
+    function bbb(aaa): int {
+        return 42
+    }
+
+    function abc(cba: int) {
+        return 42
+    }
+}
diff --git a/tests/auto/qml/reformatter/typeAnnotations.qml b/tests/auto/qml/reformatter/typeAnnotations.qml
new file mode 100644
index 00000000000..af0d93b3de7
--- /dev/null
+++ b/tests/auto/qml/reformatter/typeAnnotations.qml
@@ -0,0 +1,17 @@
+// Copyright (C) 2023 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
+
+import QtQuick
+
+Text {
+    function aaa (t : int, k       : double) : int {
+ return 42;
+    }
+
+    function bbb(aaa)
+    : int {return 42}
+
+    function abc (cba : int) {
+   return 42;
+    }
+}