qmlformat: Fix ScriptFormatter for blocks/statements comments

Component.onCompleted: {
    if(true) /* true */ {
        // the true clause
    } else {
        // the else clause
    }
}

was formatted to :

Component.onCompleted: {
   if (true /* true */)
   // the true clause
   {} else
   // the else clause
   {}
}

Add a new parameter to outWithComments that allows to change the
indentation. The use cases are:
```
{
    // 1) some comment attached to '{'
    ...
    // 2) some comment attached to '}'
}
```
For 1), IncreaseIndentation prints '{' and then increase the
indentation before printing any post comment.

For 2), DecreaseIndentation prints the pre comments, decrease the
indentation and then proceed with '}' and potential post comments.

This allows to print the comments attached to `{}` tokens with the
correct indentation.

Also adapt a test to the new comment behavior on blocks.

Task-number: QTCREATORBUG-33333
Task-number: QTBUG-123386
Change-Id: If8dd483a520c3bd25e161f3cec05530c1460bb80
Initial-patch-by: Xavier BESSON <developer@xavi-b.fr>
Reviewed-by: Olivier De Cannière <olivier.decanniere@qt.io>
(cherry picked from commit a50af19873)
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
This commit is contained in:
Sami Shalayel 2025-08-27 14:51:46 +02:00
parent 54e3197024
commit 4b5b1540c5
6 changed files with 128 additions and 13 deletions

View File

@ -384,6 +384,15 @@ public:
}
using VisitAll::visit;
bool visit(Block *block) override
{
// blocks can have comments after their `{` and before their `}`. preVisit already handles
// comments before `{` and after `}`.
addSourceLocations(block, block->lbraceToken);
addSourceLocations(block, block->rbraceToken);
return true;
}
bool visit(CaseClause *caseClause) override
{
// special case: case clauses can have comments attached to their `:` token
@ -400,6 +409,15 @@ public:
return true;
}
bool visit(DoWhileStatement *doWhileStatement) override
{
// do while statements can have comments attached to their `()` token. preVisit already
// handles comments before `do` and after the doWhile statement.
addSourceLocations(doWhileStatement, doWhileStatement->lparenToken);
addSourceLocations(doWhileStatement, doWhileStatement->rparenToken);
return true;
}
bool visit(FormalParameterList *list) override
{
if (!list->commaToken.isValid())
@ -412,6 +430,15 @@ public:
return true;
}
bool visit(ForStatement *forStatement) override
{
// for statements can have comments attached to their `()` token. preVisit already
// handles comments before `for` and after the doWhile statement.
addSourceLocations(forStatement, forStatement->lparenToken);
addSourceLocations(forStatement, forStatement->rparenToken);
return true;
}
bool visit(FunctionExpression *fExpr) override
{
addSourceLocations(fExpr, fExpr->identifierToken);
@ -427,6 +454,15 @@ public:
return visit(static_cast<FunctionExpression *>(fExpr));
}
bool visit(WhileStatement *whileStatement) override
{
// while statements can have comments attached to their `()` token. preVisit already
// handles comments before `while` and after the doWhile statement.
addSourceLocations(whileStatement, whileStatement->lparenToken);
addSourceLocations(whileStatement, whileStatement->rparenToken);
return true;
}
QMap<qsizetype, ElementRef> starts;
QMap<qsizetype, ElementRef> ends;
};

View File

@ -459,14 +459,33 @@ bool ScriptFormatter::visit(ConditionalExpression *ast)
bool ScriptFormatter::visit(Block *ast)
{
// write comments manually because we need to indent before writing a potential post comment
const CommentedElement *c =
comments->commentForNode(ast, CommentAnchor::from(ast->lbraceToken));
if (c)
c->writePre(lw);
out(ast->lbraceToken);
const int indent = lw.increaseIndent();
if (c)
c->writePost(lw);
if (ast->statements) {
++expressionDepth;
lnAcceptIndented(ast->statements);
newLine();
lw.lineWriter.ensureNewline();
accept(ast->statements);
lw.lineWriter.ensureNewline();
--expressionDepth;
}
// write comments manually because we need to write a potential pre-comment before decreasing
// the indentation
c = comments->commentForNode(ast, CommentAnchor::from(ast->rbraceToken));
if (c)
c->writePre(lw);
lw.decreaseIndent(1, indent);
out(ast->rbraceToken);
if (c)
c->writePost(lw);
return false;
}
@ -548,9 +567,9 @@ bool ScriptFormatter::visit(DoWhileStatement *ast)
acceptBlockOrIndented(ast->statement, true);
out(ast->whileToken);
lw.lineWriter.ensureSpace();
out(ast->lparenToken);
outWithComments(ast->lparenToken, ast);
accept(ast->expression);
out(ast->rparenToken);
outWithComments(ast->rparenToken, ast);
return false;
}
@ -558,9 +577,9 @@ bool ScriptFormatter::visit(WhileStatement *ast)
{
out(ast->whileToken);
lw.lineWriter.ensureSpace();
out(ast->lparenToken);
outWithComments(ast->lparenToken, ast);
accept(ast->expression);
out(ast->rparenToken);
outWithComments(ast->rparenToken, ast);
acceptBlockOrIndented(ast->statement);
return false;
}
@ -569,7 +588,7 @@ bool ScriptFormatter::visit(ForStatement *ast)
{
out(ast->forToken);
lw.lineWriter.ensureSpace();
out(ast->lparenToken);
outWithComments(ast->lparenToken, ast);
if (ast->initialiser) {
accept(ast->initialiser);
} else if (ast->declarations) {
@ -594,7 +613,7 @@ bool ScriptFormatter::visit(ForStatement *ast)
out(u";"); // ast->secondSemicolonToken
lw.lineWriter.ensureSpace();
accept(ast->expression);
out(ast->rparenToken);
outWithComments(ast->rparenToken, ast);
acceptBlockOrIndented(ast->statement);
return false;
}

View File

@ -63,8 +63,8 @@ protected:
if (c)
c->writePost(lw);
}
inline void newLine(quint32 count = 1) { lw.ensureNewline(count); }
inline void newLine(quint32 count = 1) { lw.ensureNewline(count); }
inline void accept(AST::Node *node) { AST::Node::accept(node, this); }
void lnAcceptIndented(AST::Node *node);
bool acceptBlockOrIndented(AST::Node *ast, bool finishWithSpaceOrNewline = false);

View File

@ -11,8 +11,7 @@ Item {
} // post method
// binding comment
a: {
// header
a: {// header
// before x()
// before x()

View File

@ -11,8 +11,7 @@ Item {
} // post method
// binding comment
a: {
// header
a: {// header
// before x()
// before x()

View File

@ -781,6 +781,68 @@ private slots:
QTest::newRow("AroundArrowFunction2")
<< u"const x = /*1*/(/*2*/a/*3*/, /*4*/b/*5*/)/*6*/ => /*7*/42/*8*/"_s
<< u"const x = /*1*/(/*2*/a/*3*/, /*4*/b/*5*/)/*6*/ => /*7*/42/*8*/"_s;
QTest::newRow("while") << u"while (false) /* while */ { // while\n }"_s
<< u"while (false) /* while */ { // while\n }"_s;
QTest::newRow("while2") << u"while (false) /* while */ { let b = 0; }"_s
<< u"while (false) /* while */ {\nlet b = 0;\n}"_s;
QTest::newRow("while3") << u"while /* while */ (false) /* while */ { }"_s
<< u"while /* while */ (false) /* while */ {}"_s;
QTest::newRow("while4") << u"while (false /* false */) /* while */ { } // after while"_s
<< u"while (false /* false */) /* while */ {} // after while"_s;
QTest::newRow("for") << u"for (let i = 0; i < 0; i++) /* for */ { }"_s
<< u"for (let i = 0; i < 0; i++) /* for */ {}"_s;
QTest::newRow("do-while") << uR"(do /* do */ {
} while (false); // test
// before if
if (true) {
// the true clause
} // after if block
else {
})"_s << uR"(do /* do */ {} while (false) // test
// before if
if (true) {
// the true clause
} // after if block
else {})"_s;
QTest::newRow("if-else-blocks-1") << uR"(if(true) {
} // after if block
else {
// else comment
} // after else block)"_s << uR"(if (true) {} // after if block
else {
// else comment
} // after else block)"_s;
QTest::newRow("if-else-blocks2") << uR"(if (true) {
// the true clause
let a = 10;
// another comment
} else {
// the else clause
// another comment
})"_s << uR"(if (true) {
// the true clause
let a = 10;
// another comment
} else {
// the else clause
// another comment
})"_s;
QTest::newRow("blocks") << uR"(if (false) /*1*/ { /*2*/ // 3
/*4*/
//4
}//5)"_s << uR"(if (false) /*1*/ { /*2*/ // 3
/*4*/
//4
}//5)"_s;
}
void comments()