Skip to content

Commit 3b635d9

Browse files
gmazzapgmazzap
committed
Fix bug in LineLengthSniff
and changed default max length to 100, excluding leading indent.
1 parent 20092b4 commit 3b635d9

File tree

3 files changed

+148
-127
lines changed

3 files changed

+148
-127
lines changed

‎CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
## Not released
44
- Fix bug in `NoAccessorsSniff` and allow for a few method names related to PHP core interfaces.
55
- Exclude `ArrayAccess` methods from `ReturnTypeDeclarationSniff` and `ArgumentTypeDeclarationSniff`.
6+
- Fix bug in `LineLengthSniff` which affected edge cases.
7+
- Changed default `LineLengthSniff` max length to 100, excluding leading indent.
68

79
## 0.7.2
810
- Fix bug in `ReturnTypeDeclarationSniff` which caused wrong return type detection.

‎Inpsyde/Sniffs/CodeQuality/LineLengthSniff.php

Lines changed: 119 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ class LineLengthSniff implements Sniff
6767
*
6868
* @var integer
6969
*/
70-
public $lineLimit = 120;
70+
public $lineLimit = 100;
7171

7272
/**
7373
* @inheritdoc
@@ -82,167 +82,168 @@ public function register()
8282
*/
8383
public function process(File $file, $position)
8484
{
85-
$tokens = $file->getTokens();
86-
$start = max(1, $position);
87-
for ($i = $start; $i < $file->numTokens; $i++) {
88-
if ($tokens[$i]['column'] === 1) {
89-
$this->checkLineLength($file, $tokens, $i);
90-
}
85+
$longLinesData = $this->collectLongLinesData($file, max(1, $position));
86+
if (!$longLinesData) {
87+
return $file->numTokens + 1;
9188
}
9289

93-
$this->checkLineLength($file, $tokens, $i);
90+
// phpcs:disable VariableAnalysis
91+
foreach ($longLinesData as $lineNum => list($length, $start, $end)) {
92+
if ($this->shouldIgnoreLine($file, $start, $end)) {
93+
continue;
94+
}
95+
96+
$file->addWarning(
97+
sprintf(
98+
'Line %d exceeds %s characters; contains %s characters.',
99+
$lineNum,
100+
$this->lineLimit,
101+
$length
102+
),
103+
$end,
104+
'TooLong'
105+
);
106+
}
107+
// phpcs:enable
94108

95-
return ($file->numTokens + 1);
109+
return $file->numTokens + 1;
96110
}
97111

98112
/**
99113
* @param File $file
100-
* @param array $tokens
101-
* @param int $position
114+
* @param int $start
115+
* @return array
102116
*/
103-
protected function checkLineLength(File $file, array $tokens, int $position)
117+
protected function collectLongLinesData(File $file, int $start): array
104118
{
105-
// The passed token is the first on the line.
106-
$position--;
107-
108-
if ($tokens[$position]['column'] === 1
109-
&& $tokens[$position]['length'] === 0
110-
) {
111-
// Blank line.
112-
return;
113-
}
119+
$tokens = $file->getTokens();
120+
$lines = $counted = [];
121+
$lastLine = null;
122+
for ($i = $start; $i < $file->numTokens; $i++) {
123+
if ($tokens[$i]['line'] === $lastLine) {
124+
$lines[$lastLine] .= $tokens[$i]['content'];
125+
continue;
126+
}
114127

115-
if ($tokens[$position]['column'] !== 1
116-
&& $tokens[$position]['content'] === $file->eolChar
117-
) {
118-
$position--;
119-
}
128+
if ($lastLine && array_key_exists($lastLine, $counted)) {
129+
$counted[$lastLine][0] = strlen(ltrim($lines[$lastLine]));
130+
$counted[$lastLine][2] = $i - 1;
131+
}
120132

121-
$length = $tokens[$position]['column'] + $tokens[$position]['length'] - 1;
133+
$lastLine = $tokens[$i]['line'];
134+
$lines[$lastLine] = $tokens[$i]['content'];
135+
$counted[$lastLine] = [0, $i, -1];
136+
}
122137

123-
if ($length <= $this->lineLimit
124-
|| $this->shouldIgnoreLine($tokens, $file, $position)
125-
) {
126-
return;
138+
if ($lastLine && array_key_exists($lastLine, $counted) && $counted[$lastLine][2] === -1) {
139+
$counted[$lastLine][0] = strlen(ltrim($lines[$lastLine]));
140+
$counted[$lastLine][2] = $i - 1;
127141
}
128142

129-
$file->addWarning(
130-
'Line exceeds %s characters; contains %s characters',
131-
$position,
132-
'TooLong',
133-
[$this->lineLimit, $length]
143+
return array_filter(
144+
$counted,
145+
function (array $line): bool {
146+
return $line[0] > $this->lineLimit && $line[1] > 0 && $line[2] > 0;
147+
}
134148
);
135149
}
136150

137151
/**
138152
* Don't warn for lines that exceeds limit, but either are part of
139-
* translations function first argument or cointain single words that alone
153+
* translations function first argument or contain single words that alone
140154
* are longer that line limit (e.g. long URLs).
141155
*
142-
* @param array $tokens
143156
* @param File $file
144-
* @param int $position
157+
* @param int $start
158+
* @param int $end
145159
* @return bool
146160
*/
147-
private function shouldIgnoreLine(
148-
array $tokens,
149-
File $file,
150-
int $position
151-
): bool {
152-
153-
$line = $tokens[$position]['line'];
154-
$index = $position;
155-
156-
while ($index > 0) {
157-
if ($this->isI18nFunction($tokens, $index, $file)
158-
|| $this->containLongWords($tokens, $index)
159-
) {
160-
return true;
161-
}
162-
if ($tokens[$index]['line'] !== $line) {
163-
break;
164-
}
165-
$index--;
166-
}
167-
168-
return false;
161+
private function shouldIgnoreLine(File $file, int $start, int $end): bool
162+
{
163+
return
164+
$this->containLongWords($file, $start, $end)
165+
|| $this->isI18nFunction($file, $start, $end);
169166
}
170167

171168
/**
172-
* @param array $tokens
173-
* @param int $position
169+
* @param File $file
170+
* @param int $start
171+
* @param int $end
174172
* @return bool
175173
*/
176-
private function containLongWords(
177-
array $tokens,
178-
int $position
179-
): bool {
180-
181-
$string = $tokens[$position] ?? null;
182-
if (!$string
183-
|| !in_array($string['code'], self::STRING_TYPES, true)
184-
|| strlen($string['content']) < $this->lineLimit
185-
) {
186-
return false;
187-
}
174+
private function containLongWords(File $file, int $start, int $end): bool
175+
{
176+
$tokens = $file->getTokens();
188177

189-
$words = array_filter(preg_split('~[ ,.;:?!¿¡]~', $string['content']));
190-
foreach ($words as $word) {
191-
if (strlen($word) >= $this->lineLimit) {
192-
return true;
178+
for ($i = $start; $i <= $end; $i++) {
179+
if (!in_array($tokens[$i]['code'], self::STRING_TYPES, true)) {
180+
continue;
193181
}
182+
183+
$words = array_filter(preg_split('~[\s+]~', $tokens[$i]['content']));
184+
if (count($words) > 2) {
185+
return false;
186+
}
187+
188+
return (bool)array_filter(
189+
array_map('strlen', $words),
190+
function (int $len): bool {
191+
return ($len + 3) > $this->lineLimit;
192+
}
193+
);
194194
}
195195

196196
return false;
197197
}
198198

199199
/**
200-
* @param array $tokens
201-
* @param int $position
202200
* @param File $file
201+
* @param int $start
202+
* @param int $end
203203
* @return bool
204204
*/
205-
private function isI18nFunction(
206-
array $tokens,
207-
int $position,
208-
File $file
209-
): bool {
210-
211-
$string = $tokens[$position] ?? null;
212-
if (!$string
213-
|| $string['code'] !== T_CONSTANT_ENCAPSED_STRING
214-
|| strlen($string['content']) < $this->lineLimit
215-
) {
216-
return false;
217-
}
205+
private function isI18nFunction(File $file, int $start, int $end): bool
206+
{
207+
$tokens = $file->getTokens();
218208

219-
$previousPos = $file->findPrevious(
220-
self::IGNORE_TYPES,
221-
$position - 1,
222-
null,
223-
true,
224-
null,
225-
true
226-
);
209+
for ($i = $start; $i <= $end; $i++) {
210+
if ($tokens[$i]['code'] !== T_CONSTANT_ENCAPSED_STRING
211+
|| (strlen($tokens[$i]['content']) + 3) < $this->lineLimit
212+
) {
213+
continue;
214+
}
227215

228-
$previous = $previousPos ? $tokens[$previousPos] ?? null : null;
216+
$previousPos = $file->findPrevious(
217+
self::IGNORE_TYPES,
218+
$i - 1,
219+
null,
220+
true,
221+
null,
222+
true
223+
);
224+
225+
$previous = $previousPos ? $tokens[$previousPos] ?? null : null;
226+
if (!$previous
227+
|| $previous['code'] !== T_STRING
228+
|| !in_array($previous['content'], self::I18N_FUNCTIONS, true)
229+
) {
230+
continue;
231+
}
229232

230-
if (!$previous
231-
|| $previous['code'] !== T_STRING
232-
|| !in_array($previous['content'], self::I18N_FUNCTIONS, true)
233-
) {
234-
return false;
235-
}
233+
$next = $file->findNext(
234+
[T_WHITESPACE],
235+
$previousPos + 1,
236+
null,
237+
true,
238+
null,
239+
true
240+
);
236241

237-
$next = $file->findNext(
238-
[T_WHITESPACE],
239-
$previousPos + 1,
240-
null,
241-
true,
242-
null,
243-
true
244-
);
242+
if ($tokens[$next]['code'] === T_OPEN_PARENTHESIS) {
243+
return true;
244+
}
245+
}
245246

246-
return $tokens[$next]['code'] === T_OPEN_PARENTHESIS;
247+
return false;
247248
}
248249
}

‎tests/fixtures/line-length.php

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
<?php
22
// @phpcsSniff CodeQuality.LineLength
33

4-
$a = 'a long line like next
5-
http://example.com/?7f4a87f2df8a8e8b8febd6631589e062=7f4a87f2df8a8e8b8febd6631589e0627f4a87f2df8a8e8b8febd6631589e0627f4a87f2df8a8e8b8febd6631589e062
6-
does not trigger errors because it is a single word.
7-
';
8-
94
// @phpcsWarningOnNextLine
105
$b = '7f4a87f2df8a8e8b8febd6631589e062 7f4a87f2df8a8e8b8febd6631589e062 7f4a87f2df8a8e8b8febd6631589e062 7f4a87f2df8a8e8b8febd6631589e062
116
a long line like previous does trigger errors!.
127
';
138

9+
$a = 'a long line like next
10+
http://example.com/?7f4a87f2df8a8e8b8febd6631589e062=7f4a87f2df8a8e8b8febd6631589e0627f4a87f2df8a8e8b8febd6631589e0627f4a87f2df8a8e8b8febd6631589e062
11+
does not trigger errors because it is a single word.
12+
';
13+
1414
__(
1515
'This line does not trigger error because it is the first argument of a translation string that cannot be split or WordPress style will complain.',
1616
'textdomain'
@@ -26,7 +26,8 @@
2626
'textdomain'
2727
);
2828

29-
function meh() {
29+
function meh()
30+
{
3031

3132
_ex(
3233
'This line does not trigger error because it is the first argument of a translation string that cannot be split or WordPress style will complain.',
@@ -40,9 +41,11 @@ function meh() {
4041
);
4142
}
4243

43-
class FooBarBazMeh {
44+
class FooBarBazMeh
45+
{
4446

45-
function meh() {
47+
function meh()
48+
{
4649

4750
esc_attr_x(
4851
'This line does not trigger error because it is the first argument of a translation string that cannot be split or WordPress style will complain.',
@@ -55,4 +58,19 @@ function meh() {
5558
'foo bar'
5659
);
5760
}
58-
}
61+
62+
public function render()
63+
{
64+
?>
65+
<p class="description">
66+
<?php
67+
esc_html_e(
68+
'Lorem ipsum dolor sit amet, consectetur adipiscing el. Morbi egestas, purus non luctus semper, ligula ante venenatis',
69+
'ipsum'
70+
);
71+
?>
72+
</p>
73+
<?php
74+
}
75+
}
76+

0 commit comments

Comments
 (0)