Skip to content

Commit cf3c5e6

Browse files
committed
Generic/UpperCaseConstantName: skip when local define() is in scope
When a file declares a top-level `function define(...)` or imports one via `use function ...\define;` (including group use and `as` aliases), unqualified `define(...)` calls may resolve to that function instead of the global one. The sniff should bow out in those cases. Fully qualified `\define(...)` calls remain checked as before because they tokenize as T_NAME_FULLY_QUALIFIED and unambiguously refer to the global function. Fixes #733
1 parent c3eb74d commit cf3c5e6

6 files changed

Lines changed: 216 additions & 4 deletions

File tree

src/Standards/Generic/Sniffs/NamingConventions/UpperCaseConstantNameSniff.php

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,17 @@ public function process(File $phpcsFile, int $stackPtr)
103103
return;
104104
}
105105

106+
// If the file declares or imports a function named "define",
107+
// any unqualified `define()` call may resolve to that function
108+
// instead of the global one, so the sniff should bow out.
109+
// Fully qualified `\define(...)` calls are unaffected as they
110+
// come in as T_NAME_FULLY_QUALIFIED tokens and are handled below.
111+
if ($tokens[$stackPtr]['code'] === T_STRING
112+
&& $this->fileHasCustomDefine($phpcsFile) === true
113+
) {
114+
return;
115+
}
116+
106117
// If the next non-whitespace token after this token
107118
// is not an opening parenthesis then it is not a function call.
108119
$openBracket = $phpcsFile->findNext(Tokens::EMPTY_TOKENS, ($stackPtr + 1), null, true);
@@ -146,4 +157,142 @@ public function process(File $phpcsFile, int $stackPtr)
146157
$phpcsFile->recordMetric($constPtr, 'Constant name case', 'upper');
147158
}
148159
}
160+
161+
162+
/**
163+
* Determine whether a file declares or imports a function named "define".
164+
*
165+
* Checks for top-level `function define(...)` declarations and
166+
* `use function ...\define;` (or aliased) imports. The result is cached
167+
* per file path to avoid rescanning on every visited token.
168+
*
169+
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
170+
*
171+
* @return bool
172+
*/
173+
private function fileHasCustomDefine(File $phpcsFile)
174+
{
175+
static $cache = [];
176+
177+
$fileKey = $phpcsFile->getFilename();
178+
if (isset($cache[$fileKey]) === true) {
179+
return $cache[$fileKey];
180+
}
181+
182+
$tokens = $phpcsFile->getTokens();
183+
$result = false;
184+
185+
for ($i = 0; $i < $phpcsFile->numTokens; $i++) {
186+
$code = $tokens[$i]['code'];
187+
188+
// Top-level `function define(...)` declaration.
189+
if ($code === T_FUNCTION && empty($tokens[$i]['conditions']) === true) {
190+
$namePtr = $phpcsFile->findNext(Tokens::EMPTY_TOKENS, ($i + 1), null, true);
191+
if ($namePtr !== false
192+
&& $tokens[$namePtr]['code'] === T_STRING
193+
&& strtolower($tokens[$namePtr]['content']) === 'define'
194+
) {
195+
$result = true;
196+
break;
197+
}
198+
199+
continue;
200+
}
201+
202+
// `use function ...define;` import (only top-level use statements).
203+
if ($code === T_USE && empty($tokens[$i]['conditions']) === true) {
204+
$next = $phpcsFile->findNext(Tokens::EMPTY_TOKENS, ($i + 1), null, true);
205+
if ($next === false || $tokens[$next]['code'] !== T_STRING
206+
|| strtolower($tokens[$next]['content']) !== 'function'
207+
) {
208+
continue;
209+
}
210+
211+
$end = $phpcsFile->findNext([T_SEMICOLON, T_OPEN_USE_GROUP], ($next + 1));
212+
if ($end === false) {
213+
continue;
214+
}
215+
216+
if ($this->useListImportsDefine($phpcsFile, $next, $end) === true) {
217+
$result = true;
218+
break;
219+
}
220+
221+
// Group use statement: walk the body looking for a `define` import.
222+
if ($tokens[$end]['code'] === T_OPEN_USE_GROUP) {
223+
$groupEnd = $phpcsFile->findNext(T_CLOSE_USE_GROUP, ($end + 1));
224+
if ($groupEnd !== false
225+
&& $this->useListImportsDefine($phpcsFile, $end, $groupEnd) === true
226+
) {
227+
$result = true;
228+
break;
229+
}
230+
}
231+
}
232+
}
233+
234+
$cache[$fileKey] = $result;
235+
236+
return $result;
237+
}
238+
239+
240+
/**
241+
* Inspect a `use function` import list for a `define` import.
242+
*
243+
* Handles plain imports (`use function Foo\define;`), aliases away from
244+
* `define` (`use function Foo\define as something;` — does NOT count) and
245+
* aliases to `define` (`use function Foo\bar as define;` — counts).
246+
*
247+
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
248+
* @param int $start Position to start scanning after.
249+
* @param int $end Position to stop scanning at.
250+
*
251+
* @return bool
252+
*/
253+
private function useListImportsDefine(File $phpcsFile, int $start, int $end)
254+
{
255+
$tokens = $phpcsFile->getTokens();
256+
257+
for ($j = ($start + 1); $j < $end; $j++) {
258+
$code = $tokens[$j]['code'];
259+
260+
// Explicit alias: `... as define`.
261+
if ($code === T_AS) {
262+
$aliasPtr = $phpcsFile->findNext(Tokens::EMPTY_TOKENS, ($j + 1), $end, true);
263+
if ($aliasPtr !== false
264+
&& $tokens[$aliasPtr]['code'] === T_STRING
265+
&& strtolower($tokens[$aliasPtr]['content']) === 'define'
266+
) {
267+
return true;
268+
}
269+
270+
continue;
271+
}
272+
273+
if ($code !== T_STRING
274+
&& $code !== T_NAME_QUALIFIED
275+
&& $code !== T_NAME_FULLY_QUALIFIED
276+
) {
277+
continue;
278+
}
279+
280+
$segments = explode('\\', $tokens[$j]['content']);
281+
$last = strtolower(end($segments));
282+
if ($last !== 'define') {
283+
continue;
284+
}
285+
286+
// Skip if the next non-empty token is `as` — the import is renamed
287+
// and is therefore not in scope as `define`.
288+
$after = $phpcsFile->findNext(Tokens::EMPTY_TOKENS, ($j + 1), $end, true);
289+
if ($after !== false && $tokens[$after]['code'] === T_AS) {
290+
continue;
291+
}
292+
293+
return true;
294+
}
295+
296+
return false;
297+
}
149298
}

src/Standards/Generic/Tests/NamingConventions/UpperCaseConstantNameUnitTest.1.inc

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,6 @@ define(condition() ? 'name1' : 'name2', 'sniff should bow out');
7979

8080
$callable = define(...);
8181

82-
// Valid if outside the global namespace. Sniff should bow out.
83-
function define($param) {}
84-
8582
class MyClass {
8683
public function define($param) {}
8784
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
namespace Foo;
4+
5+
// These are calls to the namespaced function `Foo\define()`,
6+
// which is declared further down in the file.
7+
define('name', 'value');
8+
namespace\define('name', 'value');
9+
10+
// These are calls to other namespaced functions, never the global one.
11+
Sub\define('name', 'value');
12+
\My\Other\NS\define('name', 'value');
13+
14+
// Calls to the global function via the leading backslash MUST still be checked.
15+
\define('VALID_NAME', true);
16+
17+
// Lowercase here SHOULD be flagged: the leading backslash forces global resolution.
18+
\define('lowercase_global', 'value');
19+
20+
function define($name, $value) {
21+
// Do something.
22+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php
2+
3+
namespace Foo;
4+
5+
use function Bar\define;
6+
use function Bar\{otherFunc, define as renamedDefine};
7+
8+
// This is a call to the imported namespaced function, not the global one.
9+
define('name', 'value');
10+
11+
// Lowercase here SHOULD be flagged: the leading backslash forces global resolution.
12+
\define('lowercase_global', 'value');
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
namespace Foo;
4+
5+
// `define` is aliased AWAY: the local symbol is `something`, not `define`.
6+
use function Bar\define as something;
7+
8+
define('name', 'value');

src/Standards/Generic/Tests/NamingConventions/UpperCaseConstantNameUnitTest.php

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,31 @@ public function getErrorList($testFile = '')
4949
51 => 1,
5050
71 => 1,
5151
73 => 1,
52-
94 => 1,
52+
91 => 1,
53+
];
54+
55+
case 'UpperCaseConstantNameUnitTest.6.inc':
56+
return [
57+
// Only the fully qualified `\define()` call should be flagged;
58+
// the unqualified `define()` calls may resolve to the local
59+
// `Foo\define` function declared further down in the file.
60+
18 => 1,
61+
];
62+
63+
case 'UpperCaseConstantNameUnitTest.7.inc':
64+
return [
65+
// `use function Bar\define;` brings a `define` symbol into scope,
66+
// so unqualified `define()` calls must not be flagged. Only the
67+
// fully qualified `\define()` call is.
68+
12 => 1,
69+
];
70+
71+
case 'UpperCaseConstantNameUnitTest.8.inc':
72+
return [
73+
// `use function Bar\define as something;` aliases AWAY from
74+
// `define`, so the local `define` symbol is unaffected and
75+
// unqualified `define()` calls should still be flagged.
76+
8 => 1,
5377
];
5478

5579
default:

0 commit comments

Comments
 (0)