Skip to content

Commit

Permalink
Merge pull request #4741 from junichi11/gh-4635-formatting-for-textua…
Browse files Browse the repository at this point in the history
…l-operators

Fix the formatting for the textual operators(`AND`, `OR`, `XOR`) #4635
  • Loading branch information
tmysik committed Oct 7, 2022
2 parents 36441f7 + 2dc1907 commit cafc4bb
Show file tree
Hide file tree
Showing 8 changed files with 226 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public enum Kind {
WHITESPACE_AROUND_INTERSECTION_TYPE_SEPARATOR,
WHITESPACE_AROUND_CONCAT_OP,
WHITESPACE_AROUND_UNARY_OP,
WHITESPACE_AROUND_TEXTUAL_OP, // AND, OR, XOR
WHITESPACE_BEFORE_BINARY_OP,
WHITESPACE_AFTER_BINARY_OP,
WHITESPACE_AROUND_TERNARY_OP,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1895,12 +1895,19 @@ public void visit(InfixExpression node) {
&& lastIndex < ts.index()) {
addFormatToken(formatTokens);
}
// don't add to AND, OR, and XOR (PHPTokenId.PHP_TEXTUAL_OPERATOR)
// see https://netbeans.org/bugzilla/show_bug.cgi?id=240274
if (ts.token().id() == PHPTokenId.PHP_TOKEN || ts.token().id() == PHPTokenId.PHP_OPERATOR) {
// don't add WHITESPACE_BEFORE_BINARY_OP and WHITESPACE_AFTER_BINARY_OP to AND, OR, and XOR (PHPTokenId.PHP_TEXTUAL_OPERATOR)
// e.g. copy($old,$new) or die("error"); -> copy($old,$new) ordie("error");
// see https://bz.apache.org/netbeans/show_bug.cgi?id=240274
if (ts.token().id() == PHPTokenId.PHP_TOKEN
|| ts.token().id() == PHPTokenId.PHP_OPERATOR) {
formatTokens.add(new FormatToken(whitespaceBefore, ts.offset()));
addFormatToken(formatTokens);
formatTokens.add(new FormatToken(whitespaceAfter, ts.offset() + ts.token().length()));
} else if (ts.token().id() == PHPTokenId.PHP_TEXTUAL_OPERATOR) {
// always add whitespace gh-4635
formatTokens.add(new FormatToken(FormatToken.Kind.WHITESPACE_AROUND_TEXTUAL_OP, ts.offset()));
addFormatToken(formatTokens);
formatTokens.add(new FormatToken(FormatToken.Kind.WHITESPACE_AROUND_TEXTUAL_OP, ts.offset() + ts.token().length()));
} else {
ts.movePrevious();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1083,6 +1083,9 @@ public void run() {
case WHITESPACE_AROUND_UNARY_OP:
countSpaces = docOptions.spaceAroundUnaryOps ? 1 : countSpaces;
break;
case WHITESPACE_AROUND_TEXTUAL_OP:
countSpaces = 1;
break;
case WHITESPACE_BEFORE_BINARY_OP:
if (docOptions.wrapAfterBinOps) {
countSpaces = docOptions.spaceAroundBinaryOps ? 1 : 0;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?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.
*/

if ($a AND !$b) { // test

}

if ($a OR !$b) { // test

}

if ($a XOR !$b) { // test

}

if ($a AND $b) {

}

if ($a OR $b) {

}

if ($a XOR $b) {

}

if ($a && !$b) {

}

if ($a || !$b) {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?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.
*/

if ($a AND !$b) { // test
}

if ($a OR !$b) { // test
}

if ($a XOR !$b) { // test
}

if ($a AND $b) {

}

if ($a OR $b) {

}

if ($a XOR $b) {

}

if ($a && !$b) {

}

if ($a || !$b) {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?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.
*/

if ($a AND !$b) { // test

}

if ($a OR !$b) { // test

}

if ($a XOR !$b) { // test

}

if ($a AND $b) {

}

if ($a OR $b) {

}

if ($a XOR $b) {

}

if ($a && !$b) {

}

if ($a || !$b) {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?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.
*/

if ($a AND !$b) { // test
}

if ($a OR !$b) { // test
}

if ($a XOR !$b) { // test
}

if ($a AND $b) {

}

if ($a OR $b) {

}

if ($a XOR $b) {

}

if ($a&&!$b) {

}

if ($a||!$b) {

}
Original file line number Diff line number Diff line change
Expand Up @@ -1025,6 +1025,18 @@ public void testIssue240274() throws Exception {
reformatFileContents("testfiles/formatting/spaces/issue240274.php", options);
}

public void testGH4635_01() throws Exception {
HashMap<String, Object> options = new HashMap<>(FmtOptions.getDefaults());
options.put(FmtOptions.SPACE_AROUND_BINARY_OPS, true);
reformatFileContents("testfiles/formatting/spaces/gh4635_01.php", options);
}

public void testGH4635_02() throws Exception {
HashMap<String, Object> options = new HashMap<>(FmtOptions.getDefaults());
options.put(FmtOptions.SPACE_AROUND_BINARY_OPS, false);
reformatFileContents("testfiles/formatting/spaces/gh4635_02.php", options);
}

public void testSpacesAroundReturnType01() throws Exception {
HashMap<String, Object> options = new HashMap<String, Object>(FmtOptions.getDefaults());
reformatFileContents("testfiles/formatting/spaces/spaceAroundReturnType01.php", options);
Expand Down

0 comments on commit cafc4bb

Please sign in to comment.