Skip to content

Commit

Permalink
Merge pull request apache#7487 from junichi11/php-gh-7172-incorrect-f…
Browse files Browse the repository at this point in the history
…ormatting-for-method-chaining

Fix formatting for method chaining [apacheGH-7172]
  • Loading branch information
junichi11 authored Jun 17, 2024
2 parents cac6e26 + c7f9f21 commit b4f3d08
Show file tree
Hide file tree
Showing 6 changed files with 868 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1779,10 +1779,8 @@ public void visit(FunctionInvocation node) {
}
scan(node.getFunctionName());
List<Expression> parameters = node.getParameters();
if (parameters != null && parameters.size() > 0) {
boolean addIndentation = !(path.get(1) instanceof ReturnStatement
|| path.get(1) instanceof Assignment
|| (path.size() > 2 && path.get(1) instanceof MethodInvocation && path.get(2) instanceof Assignment));
if (parameters != null && !parameters.isEmpty()) {
boolean addIndentation = addIndentToFunctionInvocation();
FormatToken.IndentToken indentToken = new FormatToken.IndentToken(node.getFunctionName().getEndOffset(), options.continualIndentSize);
if (addIndentation) {
formatTokens.add(indentToken);
Expand Down Expand Up @@ -1951,6 +1949,31 @@ private boolean removeIndentTokenForAnonymousClass(FunctionInvocation functionIn
return addIndent;
}

private boolean addIndentToFunctionInvocation() {
boolean addIndentation = !(path.get(1) instanceof ReturnStatement
|| path.get(1) instanceof Assignment
|| (path.size() > 2 && path.get(1) instanceof MethodInvocation && path.get(2) instanceof Assignment));
if (!addIndentation && path.size() > 1 && path.get(1) instanceof MethodInvocation) {
// GH-7172
// e.g.
// $example = (new Ex())
// ->method(
// $param
// );
MethodInvocation method = (MethodInvocation) path.get(1);
int start = method.getDispatcher().getEndOffset();
int end = method.getMethod().getStartOffset();
try {
if (document.getText(start, end - start).contains("\n")) { // NOI18N
addIndentation = true;
}
} catch (BadLocationException ex) {
LOGGER.log(Level.WARNING, "Invalid offset: {0}", ex.offsetRequested()); // NOI18N
}
}
return addIndentation;
}

@Override
public void visit(InfixExpression node) {
scan(node.getLeft());
Expand Down Expand Up @@ -2099,7 +2122,7 @@ public void visit(MethodInvocation node) {
if (document.getText(startText, endText - startText).contains("\n")) {
shift = true;
addAllUntilOffset(node.getStartOffset());
boolean addIndent = !(path.size() > 1 && (path.get(1) instanceof Assignment));
boolean addIndent = addIndentToMethodInvocation();

// anonymous classes
if (options.wrapMethodCallArgs != CodeStyle.WrapStyle.WRAP_ALWAYS) {
Expand Down Expand Up @@ -2135,6 +2158,33 @@ public void visit(MethodInvocation node) {
}
}

private boolean addIndentToMethodInvocation() {
// GH-7172
// e.g.
// $example = (new Ex())
// ->method(
// param: $param
// )
// ->field;
boolean addIndent = false;
if (path.size() > 1) {
ASTNode parent = path.get(1);
if (parent instanceof FieldAccess) {
int index = 1;
while (index < path.size()) {
index++;
parent = path.get(index);
if (parent instanceof FieldAccess) {
continue;
}
break;
}
}
addIndent = !(parent instanceof Assignment);
}
return addIndent;
}

@Override
public void visit(NamespaceDeclaration node) {
addAllUntilOffset(node.getStartOffset());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2025,12 +2025,23 @@ && countOfNewLines(formatTokens.get(index + 1).getOldText()) > 0) {
if (wasARule) {
if ((!indentRule || newLines == -1) && indentLine) {
boolean handlingSpecialCases = false;
boolean checkHandlingSpecialCases = true;
if (FormatToken.Kind.TEXT == formatToken.getId()
&& (")".equals(formatToken.getOldText()) || "]".equals(formatToken.getOldText()))) {
if (formatTokens.get(index - 1).getId() == FormatToken.Kind.WHITESPACE_WITHIN_METHOD_CALL_PARENS
&& formatTokens.get(index - 2).getId() == FormatToken.Kind.INDENT) {
// GH-7172
// e.g.
// $example = (new Ex())
// ->method(
// $param
// );
checkHandlingSpecialCases = false;
}
// tryin find out and handling cases when )) folows.
int hIndex = index + 1;
int hindent = indent;
if (hIndex < formatTokens.size()) {
if (hIndex < formatTokens.size() && checkHandlingSpecialCases) {
FormatToken token;
int lastIndent = 0;
boolean bracketsInLine = false;
Expand Down
263 changes: 263 additions & 0 deletions php/php.editor/test/unit/data/testfiles/formatting/issueGH7172.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,263 @@
<?php

/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

function gh7172($param): void {
$test1 = (new Example())
->method(
test: $param,
);

$test2 = (new Example())
->method(
test1: $param1,
test2: $param2,
)
->field;

$test3 = (new Example())
->method(
test1: $param1,
test2: $param2
)
->method2()
->field;

$test4 = (new Example())
->method(
test1: $param1,
)
->field
->field2;

$test5 = (new Example())
->method(
test: $param,
)
->method2();

$test6 = (new Example())
?->method(
test1: $param1,
test2: $param2,
);

$test7 = (new Example())
?->method(
test: $param,
)
?->field;

$test8 = (new Example())
?->method(
test: $param,
)
?->method2();

$test9 = (new Example())
?->method(
test: $param,
)
?->field
?->method2(
test: $param,
);
}

$test1 = $example
->method(
test: $param,
);

$test2 = $example
->method(
test: $param,
)
->field;

$test3 = $example
->method(
test1: $param1,
test2: $param2
)
->method2()
->field;

$test4 = $example
->method(
test1: $param1,
)
->field
->field2;

$test5 = $example
->method(
test: $param,
)
->method2();

$test6 = $example
?->method(
test1: $param1,
test2: $param2,
);

$test7 = $example
?->method(
test: $param,
)
?->field;

$test8 = $example
?->method(
test: $param,
)
?->method2();

$test9 = $example
?->method(
test: $param,
)
?->field
?->method2(
test: $param,
);

// formatted
function gh7172_2($param): void {
$test1 = (new Example())
->method(
test: $param,
);

$test2 = (new Example())
->method(
test1: $param1,
test2: $param2,
)
->field;

$test3 = (new Example())
->method(
test1: $param1,
test2: $param2
)
->method2()
->field;

$test4 = (new Example())
->method(
test1: $param1,
)
->field
->field2;

$test5 = (new Example())
->method(
test: $param,
)
->method2();

$test6 = (new Example())
?->method(
test1: $param1,
test2: $param2,
);

$test7 = (new Example())
?->method(
test: $param,
)
?->field;

$test8 = (new Example())
?->method(
test: $param,
)
?->method2();

$test9 = (new Example())
?->method(
test: $param,
)
?->field
?->method2(
test: $param,
);
}

$test1 = $example
->method(
test: $param,
);

$test2 = $example
->method(
test: $param,
)
->field;

$test3 = $example
->method(
test1: $param1,
test2: $param2
)
->method2()
->field;

$test4 = $example
->method(
test1: $param1,
)
->field
->field2;

$test5 = $example
->method(
test: $param,
)
->method2();

$test6 = $example
?->method(
test1: $param1,
test2: $param2,
);

$test7 = $example
?->method(
test: $param,
)
?->field;

$test8 = $example
?->method(
test: $param,
)
?->method2();

$test9 = $example
?->method(
test: $param,
)
?->field
?->method2(
test: $param,
);
Loading

0 comments on commit b4f3d08

Please sign in to comment.