Skip to content

Commit

Permalink
Fix the formatter for no curly "if" statement with a line comment [ap…
Browse files Browse the repository at this point in the history
…acheGH-7185]

- apache#7185
- Don't add the `WHITESPACE_BETWEEN_LINE_COMMENTS` token
- Add unit tests

Example:
```php
if (true)
    $example = 1; // comment1

// comment2
```

Before:
```php
if (true)
    $example = 1; // comment1

new line is added here
// comment2
```

After:
```php
if (true)
    $example = 1; // comment1

// comment2
```
  • Loading branch information
junichi11 committed Jun 7, 2024
1 parent 49f5521 commit db78ca9
Show file tree
Hide file tree
Showing 12 changed files with 322 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -2659,7 +2659,16 @@ private void addFormatToken(List<FormatToken> tokens) {
}
tokens.add(new FormatToken(FormatToken.Kind.WHITESPACE_INDENT, newOffset, "\n" + ts.token().text().toString()));
if (ts.moveNext() && ts.token().id() == PHPTokenId.PHP_LINE_COMMENT) {
tokens.add(new FormatToken(FormatToken.Kind.WHITESPACE_BETWEEN_LINE_COMMENTS, ts.offset()));
ASTNode parent = path.get(0);
if (!(parent instanceof IfStatement) || isCurly) {
// GH-7185 don't add this to an "if" statement without curly braces
// e.g.
// if (true)
// $example = 1; // comment1
//
// // comment2
tokens.add(new FormatToken(FormatToken.Kind.WHITESPACE_BETWEEN_LINE_COMMENTS, ts.offset()));
}
}
// #268710 for adding/checking the PHP_LINE_COMMENT token later
ts.movePrevious();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?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 (1)
$test = 0; // test

// test
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?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 (1)
$test = 0; // test

// test
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?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 (1)
$test = 0; // test

// test

// test
$test;
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?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 (1)
$test = 0; // test

// test
// test
$test;
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?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 (1)
$test = 0; // test

// test


function test(): void {
return;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?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 (1)
$test = 0; // test

// test


function test(): void {
return;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?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 (1)
$test = 0; // test
else
$else = 0; // test

// test

// test
function test(): void {
return;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?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 (1)
$test = 0; // test
else
$else = 0; // test

// test
// test
function test(): void {
return;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?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 (1) {
$test = 0; // test
// test
// test
} else {
$test = 0; // test

// test
// test

// test
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?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 (1) {
$test = 0; // test
// test
// test
} else {
$test = 0; // test
// test
// test
// test
}
Original file line number Diff line number Diff line change
Expand Up @@ -1139,4 +1139,29 @@ public void testTypedClassConstants_02() throws Exception {
reformatFileContents("testfiles/formatting/php83/typedClassConstants_02.php", options, false, true);
}

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

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

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

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

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

}

0 comments on commit db78ca9

Please sign in to comment.