Skip to content

Commit 10a7be4

Browse files
klausiklausi
authored
fix(ScopeClosingBrace): Exclude sniff from PHP template files and fix PHP error (#3204177)
1 parent 1a59890 commit 10a7be4

File tree

7 files changed

+218
-27
lines changed

7 files changed

+218
-27
lines changed

‎coder_sniffer/Drupal/Sniffs/WhiteSpace/ScopeClosingBraceSniff.php

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -103,30 +103,30 @@ public function process(File $phpcsFile, $stackPtr)
103103
}
104104

105105
// Check that the closing brace is on it's own line.
106-
$lastContent = $phpcsFile->findPrevious(
107-
[
108-
T_WHITESPACE,
109-
T_INLINE_HTML,
110-
T_OPEN_TAG,
111-
],
112-
($scopeEnd - 1),
113-
$scopeStart,
114-
true
115-
);
106+
for ($lastContent = ($scopeEnd - 1); $lastContent > $scopeStart; $lastContent--) {
107+
if ($tokens[$lastContent]['code'] === T_WHITESPACE || $tokens[$lastContent]['code'] === T_OPEN_TAG) {
108+
continue;
109+
}
110+
111+
if ($tokens[$lastContent]['code'] === T_INLINE_HTML
112+
&& ltrim($tokens[$lastContent]['content']) === ''
113+
) {
114+
continue;
115+
}
116+
117+
break;
118+
}
116119

117120
if ($tokens[$lastContent]['line'] === $tokens[$scopeEnd]['line']) {
118121
// Only allow empty classes and methods.
119-
if (($tokens[$tokens[$scopeEnd]['scope_condition']]['code'] !== T_CLASS
120-
&& $tokens[$tokens[$scopeEnd]['scope_condition']]['code'] !== T_INTERFACE
121-
&& in_array(T_CLASS, $tokens[$scopeEnd]['conditions']) === false
122-
&& in_array(T_INTERFACE, $tokens[$scopeEnd]['conditions']) === false)
123-
|| $tokens[$lastContent]['code'] !== T_OPEN_CURLY_BRACKET
124-
) {
125-
$error = 'Closing brace must be on a line by itself';
126-
$fix = $phpcsFile->addFixableError($error, $scopeEnd, 'Line');
127-
if ($fix === true) {
128-
$phpcsFile->fixer->addNewlineBefore($scopeEnd);
129-
}
122+
if ($tokens[$lastContent]['code'] === T_OPEN_CURLY_BRACKET) {
123+
return;
124+
}
125+
126+
$error = 'Closing brace must be on a line by itself';
127+
$fix = $phpcsFile->addFixableError($error, $scopeEnd, 'Line');
128+
if ($fix === true) {
129+
$phpcsFile->fixer->addNewlineBefore($scopeEnd);
130130
}
131131

132132
return;
@@ -170,7 +170,7 @@ public function process(File $phpcsFile, $stackPtr)
170170
$fix = $phpcsFile->addFixableError($error, $scopeEnd, 'BreakIndent', $data);
171171
}
172172
} else {
173-
$expectedIndent = ($startColumn - 1);
173+
$expectedIndent = max(0, ($startColumn - 1));
174174
if ($braceIndent !== $expectedIndent) {
175175
$error = 'Closing brace indented incorrectly; expected %s spaces, found %s';
176176
$data = [

‎coder_sniffer/Drupal/ruleset.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@
4343
the HTML -->
4444
<exclude-pattern>*.tpl.php</exclude-pattern>
4545
</rule>
46+
<rule ref="Drupal.WhiteSpace.ScopeClosingBrace">
47+
<!-- Do not run this sniff on template files, as the indentation might follow
48+
the HTML -->
49+
<exclude-pattern>*.tpl.php</exclude-pattern>
50+
</rule>
4651

4752
<rule ref="Generic.CodeAnalysis.UselessOverridingMethod" />
4853
<rule ref="Generic.Files.ByteOrderMark" />

‎tests/Drupal/WhiteSpace/ScopeClosingBraceUnitTest.inc

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,41 @@ function test2() {
2727
*/
2828
function test3() {
2929
return 7; }
30+
31+
/**
32+
* Defines an archiver attribute object.
33+
*
34+
* Plugin Namespace: Plugin\Archiver.
35+
*
36+
* For a working example, see \Drupal\system\Plugin\Archiver\Zip
37+
*
38+
* @see \Drupal\Core\Archiver\ArchiverManager
39+
* @see \Drupal\Core\Archiver\ArchiverInterface
40+
* @see plugin_api
41+
* @see hook_archiver_info_alter()
42+
*/
43+
#[\Attribute(\Attribute::TARGET_CLASS)]
44+
class Archiver extends Plugin {
45+
46+
/**
47+
* Constructs an archiver plugin attribute object.
48+
*
49+
* @param string $id
50+
* The archiver plugin ID.
51+
* @param \Drupal\Core\StringTranslation\TranslatableMarkup|null $title
52+
* The human-readable name of the archiver plugin.
53+
* @param \Drupal\Core\StringTranslation\TranslatableMarkup|null $description
54+
* The description of the archiver plugin.
55+
* @param array $extensions
56+
* An array of valid extensions for this archiver.
57+
* @param class-string|null $deriver
58+
* (optional) The deriver class.
59+
*/
60+
public function __construct(
61+
public readonly string $id,
62+
public readonly ?TranslatableMarkup $title = NULL,
63+
public readonly ?TranslatableMarkup $description = NULL,
64+
public readonly array $extensions = [],
65+
public readonly ?string $deriver = NULL) {}
66+
67+
}

‎tests/Drupal/WhiteSpace/ScopeClosingBraceUnitTest.inc.fixed

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,42 @@ function test2() {
2828
function test3() {
2929
return 7;
3030
}
31+
32+
/**
33+
* Defines an archiver attribute object.
34+
*
35+
* Plugin Namespace: Plugin\Archiver.
36+
*
37+
* For a working example, see \Drupal\system\Plugin\Archiver\Zip
38+
*
39+
* @see \Drupal\Core\Archiver\ArchiverManager
40+
* @see \Drupal\Core\Archiver\ArchiverInterface
41+
* @see plugin_api
42+
* @see hook_archiver_info_alter()
43+
*/
44+
#[\Attribute(\Attribute::TARGET_CLASS)]
45+
class Archiver extends Plugin {
46+
47+
/**
48+
* Constructs an archiver plugin attribute object.
49+
*
50+
* @param string $id
51+
* The archiver plugin ID.
52+
* @param \Drupal\Core\StringTranslation\TranslatableMarkup|null $title
53+
* The human-readable name of the archiver plugin.
54+
* @param \Drupal\Core\StringTranslation\TranslatableMarkup|null $description
55+
* The description of the archiver plugin.
56+
* @param array $extensions
57+
* An array of valid extensions for this archiver.
58+
* @param class-string|null $deriver
59+
* (optional) The deriver class.
60+
*/
61+
public function __construct(
62+
public readonly string $id,
63+
public readonly ?TranslatableMarkup $title = NULL,
64+
public readonly ?TranslatableMarkup $description = NULL,
65+
public readonly array $extensions = [],
66+
public readonly ?string $deriver = NULL,
67+
) {}
68+
69+
}

‎tests/Drupal/WhiteSpace/ScopeClosingBraceUnitTest.php

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,16 @@ class ScopeClosingBraceUnitTest extends CoderSniffUnitTest
2020
*/
2121
protected function getErrorList(string $testFile): array
2222
{
23-
return [
24-
16 => 1,
25-
23 => 1,
26-
29 => 1,
27-
];
23+
switch ($testFile) {
24+
case 'ScopeClosingBraceUnitTest.inc':
25+
return [
26+
16 => 1,
27+
23 => 1,
28+
29 => 1,
29+
];
30+
}
31+
32+
return [];
2833

2934
}//end getErrorList()
3035

@@ -46,4 +51,20 @@ protected function getWarningList(string $testFile): array
4651
}//end getWarningList()
4752

4853

54+
/**
55+
* Skip this test on PHP versions lower than 8 because the syntax is not allowed there.
56+
*
57+
* @return bool
58+
*/
59+
protected function shouldSkipTest()
60+
{
61+
if (version_compare(PHP_VERSION, '8.0.0') < 0) {
62+
return true;
63+
}
64+
65+
return false;
66+
67+
}//end shouldSkipTest()
68+
69+
4970
}//end class

‎tests/Drupal/WhiteSpace/ScopeClosingBraceUnitTest.tpl.php

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?php
2+
3+
/**
4+
* @file
5+
* Template error test.
6+
*
7+
* This sniff should not run on template files, since the closing brace can be
8+
* used with content on the line before. We just have this test case to
9+
* demonstrate there are no fatal errors when running the fixer.
10+
*/
11+
12+
?>
13+
<table <?php if ($classes) { print 'class="'. $classes . '" '; } ?><?php print $attributes; ?>>
14+
<?php if (!empty($title) || !empty($caption)) : ?>
15+
<caption><?php print $caption . $title; ?></caption>
16+
<?php endif; ?>
17+
<?php if (!empty($header)) : ?>
18+
<thead>
19+
<tr>
20+
<?php foreach ($header as $field => $label): ?>
21+
<th <?php if ($header_classes[$field]) { print 'class="'. $header_classes[$field] . '" '; } ?> scope="col">
22+
<?php print $label; ?>
23+
</th>
24+
<?php endforeach; ?>
25+
</tr>
26+
</thead>
27+
<?php endif; ?>
28+
<tbody>
29+
<?php foreach ($rows as $row_count => $row): ?>
30+
<tr <?php if ($row_classes[$row_count]) { print 'class="' . implode(' ', $row_classes[$row_count]) .'"'; } ?>>
31+
<?php foreach ($row as $field => $content): ?>
32+
<td <?php if ($field_classes[$field][$row_count]) { print 'class="'. $field_classes[$field][$row_count] . '" '; } ?><?php print drupal_attributes($field_attributes[$field][$row_count]); ?>>
33+
<?php print $content; ?>
34+
</td>
35+
<?php endforeach; ?>
36+
</tr>
37+
<tr <?php if ($row_classes[$row_count]) { print 'class="details ' . implode(' ', $row_classes[$row_count]) .'"'; } ?>>
38+
<td colspan="<?php print count($row); ?>">
39+
<?php print $descriptions[$row_count]; ?>
40+
</td>
41+
</tr>
42+
<?php endforeach; ?>
43+
</tbody>
44+
</table>

‎tests/Drupal/WhiteSpace/ScopeClosingBraceUnitTest.tpl.php.fixed

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?php
2+
3+
/**
4+
* @file
5+
* Template error test.
6+
*
7+
* This sniff should not run on template files, since the closing brace can be
8+
* used with content on the line before. We just have this test case to
9+
* demonstrate there are no fatal errors when running the fixer.
10+
*/
11+
12+
?>
13+
<table <?php if ($classes) { print 'class="'. $classes . '" '; } ?><?php print $attributes; ?>>
14+
<?php if (!empty($title) || !empty($caption)) : ?>
15+
<caption><?php print $caption . $title; ?></caption>
16+
<?php endif; ?>
17+
<?php if (!empty($header)) : ?>
18+
<thead>
19+
<tr>
20+
<?php foreach ($header as $field => $label): ?>
21+
<th <?php if ($header_classes[$field]) { print 'class="'. $header_classes[$field] . '" '; } ?> scope="col">
22+
<?php print $label; ?>
23+
</th>
24+
<?php endforeach; ?>
25+
</tr>
26+
</thead>
27+
<?php endif; ?>
28+
<tbody>
29+
<?php foreach ($rows as $row_count => $row): ?>
30+
<tr <?php if ($row_classes[$row_count]) { print 'class="' . implode(' ', $row_classes[$row_count]) .'"'; } ?>>
31+
<?php foreach ($row as $field => $content): ?>
32+
<td <?php if ($field_classes[$field][$row_count]) { print 'class="'. $field_classes[$field][$row_count] . '" '; } ?><?php print drupal_attributes($field_attributes[$field][$row_count]); ?>>
33+
<?php print $content; ?>
34+
</td>
35+
<?php endforeach; ?>
36+
</tr>
37+
<tr <?php if ($row_classes[$row_count]) { print 'class="details ' . implode(' ', $row_classes[$row_count]) .'"'; } ?>>
38+
<td colspan="<?php print count($row); ?>">
39+
<?php print $descriptions[$row_count]; ?>
40+
</td>
41+
</tr>
42+
<?php endforeach; ?>
43+
</tbody>
44+
</table>

0 commit comments

Comments
 (0)