qmlls: find definitions of function parameters

Function parameter definitions in the QQmlJSScope were lacking their
source location, such that asking the definition of a parameter always
returned the location of the first parameter.

Fix it by writing the proper Location in the QQmlJSScope. This requires
adding the location information to BoundName(s).
Also added some tests.

Task-number: QTBUG-111409
Change-Id: Ieb6155f120ca24e899af4b3824cab561788d008b
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Semih Yavuz <semih.yavuz@qt.io>

5 files changed, 49 insertions, 20 deletions

diff --git a/src/qml/parser/qqmljsast.cpp b/src/qml/parser/qqmljsast.cpp
index e3ed876473..347a2945f3 100644
--- a/src/qml/parser/qqmljsast.cpp
+++ b/src/qml/parser/qqmljsast.cpp
@@ -3,6 +3,7 @@
 
 #include <QString>
 #include <QLocale>
+#include "common/qqmljssourcelocation_p.h"
 #include "qqmljsast_p.h"
 
 #include "qqmljsastvisitor_p.h"
@@ -1015,13 +1016,9 @@ BoundNames FormalParameterList::formals() const
                 // change the name of the earlier argument to enforce the lookup semantics from the spec
                 formals[duplicateIndex].id += QLatin1String("#") + QString::number(i);
             }
-            formals += {
-                    name,
-                    it->element->typeAnnotation,
-                    it->element->isInjectedSignalParameter
-                        ? BoundName::Injected
-                        : BoundName::Declared
-            };
+            formals += { name, it->element->firstSourceLocation(), it->element->typeAnnotation,
+                         it->element->isInjectedSignalParameter ? BoundName::Injected
+                                                                : BoundName::Declared };
         }
         ++i;
     }
@@ -1423,8 +1420,8 @@ void PatternElement::boundNames(BoundNames *names)
         else if (PatternPropertyList *p = propertyList())
             p->boundNames(names);
     } else {
-        names->append({bindingIdentifier.toString(), typeAnnotation,
-                       isInjectedSignalParameter ? BoundName::Injected : BoundName::Declared});
+        names->append({ bindingIdentifier.toString(), firstSourceLocation(), typeAnnotation,
+                        isInjectedSignalParameter ? BoundName::Injected : BoundName::Declared });
     }
 }
 
diff --git a/src/qml/parser/qqmljsast_p.h b/src/qml/parser/qqmljsast_p.h
index 0511d8db91..5f718c4c62 100644
--- a/src/qml/parser/qqmljsast_p.h
+++ b/src/qml/parser/qqmljsast_p.h
@@ -844,9 +844,11 @@ struct QML_PARSER_EXPORT BoundName
     };
 
     QString id;
+    QQmlJS::SourceLocation location;
     QTaggedPointer<TypeAnnotation, Type> typeAnnotation;
-    BoundName(const QString &id, TypeAnnotation *typeAnnotation, Type type = Declared)
-        : id(id), typeAnnotation(typeAnnotation, type)
+    BoundName(const QString &id, const QQmlJS::SourceLocation &location,
+              TypeAnnotation *typeAnnotation, Type type = Declared)
+        : id(id), location(location), typeAnnotation(typeAnnotation, type)
     {}
     BoundName() = default;
 
diff --git a/src/qmlcompiler/qqmljsimportvisitor.cpp b/src/qmlcompiler/qqmljsimportvisitor.cpp
index ab991c038a..edf489bdc0 100644
--- a/src/qmlcompiler/qqmljsimportvisitor.cpp
+++ b/src/qmlcompiler/qqmljsimportvisitor.cpp
@@ -2468,7 +2468,7 @@ bool QQmlJSImportVisitor::visit(QQmlJS::AST::FormalParameterList *fpl)
                 typeName = type->toString();
         m_currentScope->insertJSIdentifier(boundName.id,
                                            { QQmlJSScope::JavaScriptIdentifier::Parameter,
-                                             fpl->firstSourceLocation(), typeName, false });
+                                             boundName.location, typeName, false });
     }
     return true;
 }
@@ -2704,7 +2704,7 @@ bool QQmlJSImportVisitor::visit(QQmlJS::AST::PatternElement *element)
                     { (element->scope == QQmlJS::AST::VariableScope::Var)
                               ? QQmlJSScope::JavaScriptIdentifier::FunctionScoped
                               : QQmlJSScope::JavaScriptIdentifier::LexicalScoped,
-                      element->firstSourceLocation(), typeName,
+                      name.location, typeName,
                       element->scope == QQmlJS::AST::VariableScope::Const });
         }
     }
diff --git a/tests/auto/qmlls/utils/data/JSDefinitions.qml b/tests/auto/qmlls/utils/data/JSDefinitions.qml
index 34d44df8f4..0827bc036c 100644
--- a/tests/auto/qmlls/utils/data/JSDefinitions.qml
+++ b/tests/auto/qmlls/utils/data/JSDefinitions.qml
@@ -27,4 +27,15 @@ Item {
         }
         f(scoped, i);
     }
+
+    Rectangle {
+        id: nested
+
+        property int i
+
+        function f() {
+            let x = i, y = nested.i, z = rootId.i;
+        }
+
+    }
 }
diff --git a/tests/auto/qmlls/utils/tst_qmlls_utils.cpp b/tests/auto/qmlls/utils/tst_qmlls_utils.cpp
index e271f095b7..9fd5f47184 100644
--- a/tests/auto/qmlls/utils/tst_qmlls_utils.cpp
+++ b/tests/auto/qmlls/utils/tst_qmlls_utils.cpp
@@ -879,6 +879,8 @@ void tst_qmlls_utils::findDefinitionFromLocation_data()
 
     QTest::addRow("JSIdentifierX")
             << JSDefinitionsQml << 14 << 11 << JSDefinitionsQml << 13 << 13 << strlen("x");
+    QTest::addRow("JSIdentifierX2")
+            << JSDefinitionsQml << 15 << 11 << JSDefinitionsQml << 13 << 13 << strlen("x");
     QTest::addRow("propertyI") << JSDefinitionsQml << 14 << 14 << JSDefinitionsQml << 9
                                << positionAfterOneIndent << strlen("property int i");
     QTest::addRow("qualifiedPropertyI") << JSDefinitionsQml << 15 << 21 << JSDefinitionsQml << 9
@@ -888,8 +890,14 @@ void tst_qmlls_utils::findDefinitionFromLocation_data()
 
     QTest::addRow("parameterA") << JSDefinitionsQml << 10 << 16 << noResultExpected << -1 << -1
                                 << size_t{};
+    QTest::addRow("parameterAUsage")
+            << JSDefinitionsQml << 10 << 39 << JSDefinitionsQml << -1 << 16 << strlen("a");
+
     QTest::addRow("parameterB") << JSDefinitionsQml << 10 << 28 << noResultExpected << -1 << -1
                                 << size_t{};
+    QTest::addRow("parameterBUsage")
+            << JSDefinitionsQml << 10 << 86 << JSDefinitionsQml << -1 << 28 << strlen("b");
+
     QTest::addRow("comment") << JSDefinitionsQml << 10 << 21 << noResultExpected << -1 << -1
                              << size_t{};
 
@@ -904,6 +912,17 @@ void tst_qmlls_utils::findDefinitionFromLocation_data()
                              << positionAfterOneIndent << strlen("property int i");
     QTest::addRow("scopedI") << JSDefinitionsQml << 25 << 27 << JSDefinitionsQml << 24 << 32
                              << strlen("i");
+
+    QTest::addRow("shadowingProperty") << JSDefinitionsQml << 37 << 21 << JSDefinitionsQml << 34
+                                       << 9 << strlen("property int i");
+    QTest::addRow("shadowingQualifiedProperty") << JSDefinitionsQml << 37 << 35 << JSDefinitionsQml
+                                                << 34 << 9 << strlen("property int i");
+    QTest::addRow("shadowedProperty") << JSDefinitionsQml << 37 << 49 << JSDefinitionsQml << 9
+                                      << positionAfterOneIndent << strlen("property int i");
+    QTest::addRow("parentId") << JSDefinitionsQml << 37 << 44 << JSDefinitionsQml << 6 << 1
+                              << strlen("Item");
+    QTest::addRow("currentId") << JSDefinitionsQml << 37 << 30 << JSDefinitionsQml << 31
+                               << positionAfterOneIndent << strlen("Rectangle");
 }
 
 void tst_qmlls_utils::findDefinitionFromLocation()
@@ -938,22 +957,22 @@ void tst_qmlls_utils::findDefinitionFromLocation()
 
     QCOMPARE(locations.size(), 1);
 
-    auto type = QQmlLSUtils::findDefinitionOf(locations.front().domItem);
+    auto definition = QQmlLSUtils::findDefinitionOf(locations.front().domItem);
 
     // if expectedFilePath is empty, we probably just want to make sure that it does
     // not crash
     if (expectedFilePath == noResultExpected) {
-        QVERIFY(!type);
+        QVERIFY(!definition);
         return;
     }
 
-    QVERIFY(type);
+    QVERIFY(definition);
 
-    QCOMPARE(type->filename, expectedFilePath);
+    QCOMPARE(definition->filename, expectedFilePath);
 
-    QCOMPARE(type->location.startLine, quint32(expectedLine));
-    QCOMPARE(type->location.startColumn, quint32(expectedCharacter));
-    QCOMPARE(type->location.length, quint32(expectedLength));
+    QCOMPARE(definition->location.startLine, quint32(expectedLine));
+    QCOMPARE(definition->location.startColumn, quint32(expectedCharacter));
+    QCOMPARE(definition->location.length, quint32(expectedLength));
 }
 
 QTEST_MAIN(tst_qmlls_utils)