diff options
author | Sami Shalayel <sami.shalayel@qt.io> | 2023-11-29 16:48:04 +0100 |
---|---|---|
committer | Sami Shalayel <sami.shalayel@qt.io> | 2023-12-05 17:50:35 +0100 |
commit | 99c8a040f6e2a8a8733e94fbd07986b4f5982211 (patch) | |
tree | e3d56d3a59c041e65a504bf36b18856dfcace7e7 | |
parent | 2313dc93733b6fa46dd0de5711b9f4053768552e (diff) |
qmlls: completions in variable declarations
Add the equal token sourcelocation into a ScriptPattern. This is
somewhat cumbersome because the parser has no direct access to it.
Instead, create a new ExpressionNode type called InitializerExpression:
it contains an ExpressionNode and an equaltoken, and is populated in the
parser for Initializer and Initializer_In rules. It also implements some
pure virtual methods to not be abstract, and has its own
Kind_InitializerExpression.
The PatternElement constructor extracts the location of the equaltoken
from the InitializerExpression in its constructor, and saves it in its
new member equaltoken.
Later on, the Dom constructor will be able to add the location of the
equal token to the Dom, such that qmlls's completion can decide whether
or not completion is required in variable declaration statements.
With this commit, qmlls will provide completions only after the above
mentioned equal token. The explanation is in a comment in qqmllsutils,
but the rough idea is that everything before the '=' is a variable name
(so it should not be in use yet, to avoid shadowing and confusing QML
programs) and that everything behind a '=' is a default value that can
be any arbitrary expression in JS. This default value can be a method
name, a property name, etc, so provide completion at this place.
Also takes care of completions inside of deconstructions nested inside
variable declarations.
Task-number: QTBUG-117445
Change-Id: Ie58ffda4de9636796a9a690537affef85ede398d
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
-rw-r--r-- | src/qml/parser/qqmljs.g | 4 | ||||
-rw-r--r-- | src/qml/parser/qqmljsast.cpp | 5 | ||||
-rw-r--r-- | src/qml/parser/qqmljsast_p.h | 58 | ||||
-rw-r--r-- | src/qmldom/qqmldomastcreator.cpp | 3 | ||||
-rw-r--r-- | src/qmlls/qqmllsutils.cpp | 80 | ||||
-rw-r--r-- | tests/auto/qmlls/utils/data/completions/variableDeclaration.qml | 36 | ||||
-rw-r--r-- | tests/auto/qmlls/utils/tst_qmlls_utils.cpp | 197 |
7 files changed, 376 insertions, 7 deletions
diff --git a/src/qml/parser/qqmljs.g b/src/qml/parser/qqmljs.g index 6d9d11800c..bc5955245a 100644 --- a/src/qml/parser/qqmljs.g +++ b/src/qml/parser/qqmljs.g @@ -2167,7 +2167,9 @@ Initializer: T_EQ AssignmentExpression; Initializer_In: T_EQ AssignmentExpression_In; /. case $rule_number: { - sym(1) = sym(2); + auto node = new (pool) AST::InitializerExpression(sym(2).Expression); + node->equalToken = loc(1); + sym(1).Expression = node; } break; ./ diff --git a/src/qml/parser/qqmljsast.cpp b/src/qml/parser/qqmljsast.cpp index 76448f6853..a45d4cbd1b 100644 --- a/src/qml/parser/qqmljsast.cpp +++ b/src/qml/parser/qqmljsast.cpp @@ -1394,6 +1394,11 @@ void TaggedTemplate::accept0(BaseVisitor *visitor) visitor->endVisit(this); } +void InitializerExpression::accept0(BaseVisitor *visitor) +{ + expression->accept0(visitor); +} + void PatternElement::accept0(BaseVisitor *visitor) { if (visitor->visit(this)) { diff --git a/src/qml/parser/qqmljsast_p.h b/src/qml/parser/qqmljsast_p.h index 73b5c5cd5e..2a76c371d0 100644 --- a/src/qml/parser/qqmljsast_p.h +++ b/src/qml/parser/qqmljsast_p.h @@ -149,6 +149,7 @@ public: Kind_ClassDeclaration, Kind_IdentifierExpression, Kind_IdentifierPropertyName, + Kind_InitializerExpression, Kind_ComputedPropertyName, Kind_IfStatement, Kind_LabelledStatement, @@ -873,6 +874,39 @@ struct BoundNames : public QVector<BoundName> } }; +/*! +\internal +This class is needed to pass the information about the equalToken in the parser, and is only needed +during AST construction. It behaves exactly like the expression it contains: that avoids changing +all the usages in qqmljs.g from ExpressionNode to InitializerExpression for every rule expecting a +InitializerOpt_In or InitializerOpt. +*/ +class QML_PARSER_EXPORT InitializerExpression : public ExpressionNode +{ +public: + QQMLJS_DECLARE_AST_NODE(InitializerExpression) + + InitializerExpression(ExpressionNode *e) : expression(e) { kind = K; } + + void accept0(BaseVisitor *visitor) override; + + SourceLocation firstSourceLocation() const override + { return equalToken; } + + SourceLocation lastSourceLocation() const override { return expression->lastSourceLocation(); } + + FunctionExpression *asFunctionDefinition() override + { + return expression->asFunctionDefinition(); + } + + ClassExpression *asClassDefinition() override { return expression->asClassDefinition(); } + + // attributes + ExpressionNode *expression; + SourceLocation equalToken; +}; + class QML_PARSER_EXPORT PatternElement : public Node { public: @@ -893,9 +927,28 @@ public: Binding, }; +private: + /*! + \internal + Hide InitializerExpression from the AST. InitializerExpression is only needed during parsing for + the AST construction, and it is not possible for the parser to directly embed the location of + equal tokens inside the PatternElement without the InitializerExpression. + */ + void unwrapInitializer() + { + if (auto unwrapped = AST::cast<InitializerExpression *>(initializer)) { + equalToken = unwrapped->equalToken; + initializer = unwrapped->expression; + } + } +public: + PatternElement(ExpressionNode *i = nullptr, Type t = Literal) : initializer(i), type(t) - { kind = K; } + { + kind = K; + unwrapInitializer(); + } PatternElement(QStringView n, TypeAnnotation *typeAnnotation = nullptr, ExpressionNode *i = nullptr, Type t = Binding) : bindingIdentifier(n), initializer(i), type(t) @@ -903,6 +956,7 @@ public: { Q_ASSERT(t >= RestElement); kind = K; + unwrapInitializer(); } PatternElement(Pattern *pattern, ExpressionNode *i = nullptr, Type t = Binding) @@ -910,6 +964,7 @@ public: { Q_ASSERT(t >= RestElement); kind = K; + unwrapInitializer(); } void accept0(BaseVisitor *visitor) override; @@ -933,6 +988,7 @@ public: // attributes SourceLocation identifierToken; + SourceLocation equalToken; QStringView bindingIdentifier; ExpressionNode *bindingTarget = nullptr; ExpressionNode *initializer = nullptr; diff --git a/src/qmldom/qqmldomastcreator.cpp b/src/qmldom/qqmldomastcreator.cpp index ceb2433fdf..bee43c5130 100644 --- a/src/qmldom/qqmldomastcreator.cpp +++ b/src/qmldom/qqmldomastcreator.cpp @@ -1585,6 +1585,9 @@ void QQmlDomAstCreator::endVisitHelper( AST::PatternElement *pe, const std::shared_ptr<ScriptElements::GenericScriptElement> ¤t) { + if (pe->equalToken.isValid()) + current->addLocation(FileLocationRegion::EqualTokenRegion, pe->equalToken); + if (pe->identifierToken.isValid() && !pe->bindingIdentifier.isEmpty()) { auto identifier = std::make_shared<ScriptElements::IdentifierExpression>(pe->identifierToken); diff --git a/src/qmlls/qqmllsutils.cpp b/src/qmlls/qqmllsutils.cpp index 88e1742bb5..703312cc40 100644 --- a/src/qmlls/qqmllsutils.cpp +++ b/src/qmlls/qqmllsutils.cpp @@ -3033,6 +3033,70 @@ static QList<CompletionItem> insideBinaryExpressionCompletion(const DomItem &cur return {}; } +/*! +\internal +Doing completion in variable declarations requires taking a look at all different cases: + +\list + \li Normal variable names, like \c{let helloWorld = 123;} + In this case, only autocomplete scriptexpressionidentifiers after the '=' token. + Do not propose existing names for the variable name, because the variable name needs to be + an identifier that is not used anywhere (to avoid shadowing and confusing code), + + \li Deconstructed arrays, like \c{let [ helloWorld, ] = [ 123, ];} + In this case, only autocomplete scriptexpressionidentifiers after the '=' token. + Do not propose already existing identifiers inside the left hand side array. + + \li Deconstructed arrays with initializers, like \c{let [ helloWorld = someVar, ] = [ 123, ];} + Note: this assigns the value of someVar to helloWorld if the right hand side's first element + is undefined or does not exist. + + In this case, only autocomplete scriptexpressionidentifiers after the '=' tokens. + Only propose already existing identifiers inside the left hand side array when behind a '=' + token. + + \li Deconstructed Objects, like \c{let { helloWorld, } = { helloWorld: 123, };} + In this case, only autocomplete scriptexpressionidentifiers after the '=' token. + Do not propose already existing identifiers inside the left hand side object. + + \li Deconstructed Objects with initializers, like \c{let { helloWorld = someVar, } = {};} + Note: this assigns the value of someVar to helloWorld if the right hand side's object does + not have a property called 'helloWorld'. + + In this case, only autocomplete scriptexpressionidentifiers after the '=' token. + Only propose already existing identifiers inside the left hand side object when behind a '=' + token. + + \li Finally, you are allowed to nest and combine all above possibilities together for all your + deconstruction needs, so the exact same completion needs to be done for + DomType::ScriptPatternElement too. + +\endlist +*/ +static QList<CompletionItem> insideScriptPattern(const DomItem ¤tItem, + const CompletionContextStrings &ctx) +{ + const auto regions = FileLocations::treeOf(currentItem)->info().regions; + + const QQmlJS::SourceLocation equal = regions[EqualTokenRegion]; + + if (!afterLocation(equal, ctx)) + return {}; + + // otherwise, only complete case and default + return QQmlLSUtils::scriptIdentifierCompletion(currentItem, ctx); +} + +/*! +\internal +See comment on insideScriptPattern(). +*/ +static QList<CompletionItem> insideVariableDeclarationEntry(const DomItem ¤tItem, + const CompletionContextStrings &ctx) +{ + return insideScriptPattern(currentItem, ctx); +} + static bool ctxBeforeStatement(const CompletionContextStrings &ctx, const DomItem ¤tItem, FileLocationRegion firstRegion) { @@ -3145,17 +3209,25 @@ QList<CompletionItem> QQmlLSUtils::completions(const DomItem ¤tItem, return insideDefaultClause(currentParent, ctx); case DomType::ScriptCaseBlock: return insideCaseBlock(currentParent, ctx); + case DomType::ScriptVariableDeclaration: + // not needed: thats a list of ScriptVariableDeclarationEntry, and those entries cannot + // be suggested because they all start with `{`, `[` or an identifier that should not be + // in use yet. + return {}; + case DomType::ScriptVariableDeclarationEntry: + return insideVariableDeclarationEntry(currentParent, ctx); + case DomType::ScriptProperty: + // fallthrough: a ScriptProperty is a ScriptPattern but inside a JS Object. It gets the + // same completions as a ScriptPattern. + case DomType::ScriptPattern: + return insideScriptPattern(currentParent, ctx); // TODO: Implement those statements. // In the meanwhile, suppress completions to avoid weird behaviors. - case DomType::ScriptVariableDeclaration: - case DomType::ScriptVariableDeclarationEntry: case DomType::ScriptArray: case DomType::ScriptObject: - case DomType::ScriptProperty: case DomType::ScriptElision: case DomType::ScriptArrayEntry: - case DomType::ScriptPattern: return {}; default: diff --git a/tests/auto/qmlls/utils/data/completions/variableDeclaration.qml b/tests/auto/qmlls/utils/data/completions/variableDeclaration.qml new file mode 100644 index 0000000000..2a32c2ff44 --- /dev/null +++ b/tests/auto/qmlls/utils/data/completions/variableDeclaration.qml @@ -0,0 +1,36 @@ +import QtQuick + +Item { + function data() { return 42; } + + function f(x) { + let letStatement = data(); + const constStatement = data(); + var varStatement = data(); // bad? + } + + function objects(x) { + let { deconstructed } = data(); + let { deconstructedAloneWithInitializer = 55 } = data(); + let { deconstructedWithInitializer1 = 55, deconstructedWithInitializer2 = 66 } = data(), { unused: deconstructedWithInitializer3 = 77, } = data() ; + } + + function arrays(x) { + let [ deconstructed ] = data(); + let [ deconstructedAloneWithInitializer = 55 ] = data(); + let [ deconstructedWithInitializer1 = 55, deconstructedWithInitializer2 = 66 ] = data(), [ deconstructedWithInitializer3 = 77, ] = data() ; + } + + function oneArrayingToRuleThemAll(x) { + let [ head, [headOfSecond = 44, secondOfSecond = 55], [_ = "useless", secondOfThird = g()], [ [ headOfHeadOfFourth = g() + 1] ] ] = data(); + } + + function needleInTheHarraystack(x) { + let [ head, [headOfSecond = 44, secondOfSecond = 55], [_ = "useless", secondOfThird = g(), { needle = "x" }], [ [ headOfHeadOfFourth = g() + 1] ] ] = data(); + } + + function arrayInTheObject(x) { + let { p: [first, second] } = { p: [1,2] }; + } + +} diff --git a/tests/auto/qmlls/utils/tst_qmlls_utils.cpp b/tests/auto/qmlls/utils/tst_qmlls_utils.cpp index 8eaf2cc54f..8503e3d320 100644 --- a/tests/auto/qmlls/utils/tst_qmlls_utils.cpp +++ b/tests/auto/qmlls/utils/tst_qmlls_utils.cpp @@ -2754,7 +2754,6 @@ void tst_qmlls_utils::completions_data() << QStringList{ propertyCompletion, letStatementCompletion, u"myProperty"_s, u"x"_s, u"f"_s } << None; - QTest::newRow("inDefaultAfterDefault") << testFile(u"completions/switchStatements.qml"_s) << 33 << 1 << ExpectedCompletions{ { u"x"_s, CompletionItemKind::Variable }, @@ -2765,6 +2764,202 @@ void tst_qmlls_utils::completions_data() { u"myProperty"_s, CompletionItemKind::Property } } << QStringList{ propertyCompletion, } << None; + + // variableDeclaration.qml tests for let/const/var statements + destructuring + + QTest::newRow("letStatement") << testFile(u"completions/variableDeclaration.qml"_s) << 7 << 13 + << ExpectedCompletions{} + << QStringList{ propertyCompletion, letStatementCompletion, + u"x"_s, u"data"_s } + << None; + + QTest::newRow("letStatement2") + << testFile(u"completions/variableDeclaration.qml"_s) << 7 << 26 + << ExpectedCompletions{} + << QStringList{ propertyCompletion, letStatementCompletion, u"x"_s, u"data"_s } << None; + + QTest::newRow("letStatementBehindEqual") + << testFile(u"completions/variableDeclaration.qml"_s) << 7 << 28 + << ExpectedCompletions{ {u"x"_s, CompletionItemKind::Variable}, + {u"data"_s, CompletionItemKind::Method}, + } + << QStringList{ propertyCompletion, letStatementCompletion } + << None; + + QTest::newRow("letStatementBehindEqual2") + << testFile(u"completions/variableDeclaration.qml"_s) << 7 << 33 + << ExpectedCompletions{ {u"x"_s, CompletionItemKind::Variable}, + {u"data"_s, CompletionItemKind::Method}, + } + << QStringList{ propertyCompletion, letStatementCompletion } + << None; + + QTest::newRow("constStatement") + << testFile(u"completions/variableDeclaration.qml"_s) << 8 << 19 + << ExpectedCompletions{} + << QStringList{ propertyCompletion, letStatementCompletion, u"x"_s, u"data"_s } << None; + + QTest::newRow("constStatementBehindEqual") + << testFile(u"completions/variableDeclaration.qml"_s) << 8 << 32 + << ExpectedCompletions{ {u"x"_s, CompletionItemKind::Variable}, + {u"data"_s, CompletionItemKind::Method}, + } + << QStringList{ propertyCompletion, letStatementCompletion } + << None; + + QTest::newRow("varStatement") << testFile(u"completions/variableDeclaration.qml"_s) << 9 << 17 + << ExpectedCompletions{} + << QStringList{ propertyCompletion, letStatementCompletion, + u"x"_s, u"data"_s } + << None; + + QTest::newRow("varStatementBehindEqual") + << testFile(u"completions/variableDeclaration.qml"_s) << 9 << 28 + << ExpectedCompletions{ {u"x"_s, CompletionItemKind::Variable}, + {u"data"_s, CompletionItemKind::Method}, + } + << QStringList{ propertyCompletion, letStatementCompletion } + << None; + + QTest::newRow("objectDeconstruction") + << testFile(u"completions/variableDeclaration.qml"_s) << 13 << 20 + << ExpectedCompletions{} + << QStringList{ propertyCompletion, letStatementCompletion, u"x"_s, u"data"_s } << None; + + QTest::newRow("objectDeconstructionAloneBehindEqual") + << testFile(u"completions/variableDeclaration.qml"_s) << 14 << 51 + << ExpectedCompletions{ {u"x"_s, CompletionItemKind::Variable}, + {u"data"_s, CompletionItemKind::Method}, + } + << QStringList{ propertyCompletion, letStatementCompletion } + << None; + + QTest::newRow("objectDeconstructionAloneBehindEqual2") + << testFile(u"completions/variableDeclaration.qml"_s) << 14 << 58 + << ExpectedCompletions{ {u"x"_s, CompletionItemKind::Variable}, + {u"data"_s, CompletionItemKind::Method}, + } + << QStringList{ propertyCompletion, letStatementCompletion } + << None; + + QTest::newRow("objectDeconstruction2BehindEqual") + << testFile(u"completions/variableDeclaration.qml"_s) << 15 << 83 + << ExpectedCompletions{ {u"x"_s, CompletionItemKind::Variable}, + {u"data"_s, CompletionItemKind::Method}, + } + << QStringList{ propertyCompletion, letStatementCompletion } + << None; + + QTest::newRow("objectDeconstruction2BehindEqual2") + << testFile(u"completions/variableDeclaration.qml"_s) << 15 << 90 + << ExpectedCompletions{ {u"x"_s, CompletionItemKind::Variable}, + {u"data"_s, CompletionItemKind::Method}, + } + << QStringList{ propertyCompletion, letStatementCompletion } + << None; + + QTest::newRow("objectDeconstruction3BehindEqual") + << testFile(u"completions/variableDeclaration.qml"_s) << 15 << 140 + << ExpectedCompletions{ {u"x"_s, CompletionItemKind::Variable}, + {u"data"_s, CompletionItemKind::Method}, + } + << QStringList{ propertyCompletion, letStatementCompletion } + << None; + + QTest::newRow("objectDeconstructionBehindComma") + << testFile(u"completions/variableDeclaration.qml"_s) << 15 << 143 + << ExpectedCompletions{} + << QStringList{ propertyCompletion, letStatementCompletion, u"x"_s, u"data"_s } + << None; + + QTest::newRow("objectDeconstructionBetweenObjects") + << testFile(u"completions/variableDeclaration.qml"_s) << 15 << 50 + << ExpectedCompletions{} + << QStringList{ propertyCompletion, letStatementCompletion, u"x"_s, u"data"_s } + << None; + + QTest::newRow("objectDeconstructionBetweenDeconstructions") + << testFile(u"completions/variableDeclaration.qml"_s) << 15 << 97 + << ExpectedCompletions{} + << QStringList{ propertyCompletion, letStatementCompletion, u"x"_s, u"data"_s } + << None; + + QTest::newRow("arrayDeconstructionAlone") + << testFile(u"completions/variableDeclaration.qml"_s) << 19 << 24 + << ExpectedCompletions{} + << QStringList{ propertyCompletion, letStatementCompletion, u"x"_s, u"data"_s } + << None; + + QTest::newRow("arrayDeconstructionAloneBehindEqual") + << testFile(u"completions/variableDeclaration.qml"_s) << 19 << 33 + << ExpectedCompletions{ {u"x"_s, CompletionItemKind::Variable}, + {u"data"_s, CompletionItemKind::Method}, + } + << QStringList{ propertyCompletion, letStatementCompletion } + << None; + + QTest::newRow("arrayDeconstruction2") + << testFile(u"completions/variableDeclaration.qml"_s) << 21 << 71 + << ExpectedCompletions{} + << QStringList{ propertyCompletion, letStatementCompletion, u"x"_s, u"data"_s } + << None; + + QTest::newRow("arrayDeconstruction2BehindEqual") + << testFile(u"completions/variableDeclaration.qml"_s) << 21 << 83 + << ExpectedCompletions{ {u"x"_s, CompletionItemKind::Variable}, + {u"data"_s, CompletionItemKind::Method}, + } + << QStringList{ propertyCompletion, letStatementCompletion } + << None; + QTest::newRow("arrayDeconstruction3") + << testFile(u"completions/variableDeclaration.qml"_s) << 21 << 125 + << ExpectedCompletions{} + << QStringList{ propertyCompletion, letStatementCompletion, u"x"_s, u"data"_s } + << None; + + QTest::newRow("arrayDeconstruction3BehindEqual") + << testFile(u"completions/variableDeclaration.qml"_s) << 21 << 139 + << ExpectedCompletions{ {u"x"_s, CompletionItemKind::Variable}, + {u"data"_s, CompletionItemKind::Method}, + } + << QStringList{ propertyCompletion, letStatementCompletion } + << None; + + QTest::newRow("arrayDeconstructionIn_Wildcard") + << testFile(u"completions/variableDeclaration.qml"_s) << 25 << 64 + << ExpectedCompletions{} + << QStringList{ propertyCompletion, letStatementCompletion, u"x"_s, u"data"_s } + << None; + + QTest::newRow("arrayDeconstructionBehind+") + << testFile(u"completions/variableDeclaration.qml"_s) << 25 << 132 + << ExpectedCompletions{ {u"x"_s, CompletionItemKind::Variable}, + {u"data"_s, CompletionItemKind::Method}, + } + << QStringList{ propertyCompletion, letStatementCompletion } + << None; + + QTest::newRow("objectDeconstructionForNeedle") + << testFile(u"completions/variableDeclaration.qml"_s) << 29 << 111 + << ExpectedCompletions{ {u"x"_s, CompletionItemKind::Variable}, + {u"data"_s, CompletionItemKind::Method}, + } + << QStringList{ propertyCompletion, letStatementCompletion } + << None; + + QTest::newRow("arrayInObjectDeconstructionInObjectInitializer") + << testFile(u"completions/variableDeclaration.qml"_s) << 33 << 44 + << ExpectedCompletions{ {u"x"_s, CompletionItemKind::Variable}, + {u"data"_s, CompletionItemKind::Method}, + } + << QStringList{ propertyCompletion, letStatementCompletion } + << None; + + QTest::newRow("arrayInObjectDeconstructionInObjectPropertyName") + << testFile(u"completions/variableDeclaration.qml"_s) << 33 << 26 + << ExpectedCompletions{} + << QStringList{ propertyCompletion, letStatementCompletion, u"x"_s, u"data"_s } + << None; } void tst_qmlls_utils::completions() |