Skip to content

Commit c1cf608

Browse files
committed
Squiz/ClassFileName: inverse the error message
As things were, the `Squiz.Classes.ClassFileName` sniff would recommend for a class (OO structure) to be called the same as the file. However, file names can contain characters which are not allowed in PHP class names, so this advise could end up being impossible to action for names not complying with the PHP requirements for symbol names. This commit changes the error message to advise to change the file name instead of the class name, which should make the error actionable in all cases. Includes tests. While the tests would not _fail_ without this change, the tests can be used to verify the difference in the error messages on the command-line. Before this change, the errors would read like so: ``` FILE: src/Standards/Squiz/Tests/Classes/ClassFileName Spaces In FilenameUnitTest.inc ------------------------------------------------------------------------------------------------------------------------------------------------ FOUND 1 ERROR AFFECTING 1 LINE ------------------------------------------------------------------------------------------------------------------------------------------------ 7 | ERROR | Class name doesn't match filename; expected "class ClassFileName Spaces In FilenameUnitTest" (Squiz.Classes.ClassFileName.NoMatch) ------------------------------------------------------------------------------------------------------------------------------------------------ FILE: src/Standards/Squiz/Tests/Classes/ClassFileName-Dashes-In-FilenameUnitTest.inc ------------------------------------------------------------------------------------------------------------------------------------------------ FOUND 1 ERROR AFFECTING 1 LINE ------------------------------------------------------------------------------------------------------------------------------------------------ 7 | ERROR | Class name doesn't match filename; expected "class ClassFileName-Dashes-In-FilenameUnitTest" (Squiz.Classes.ClassFileName.NoMatch) ------------------------------------------------------------------------------------------------------------------------------------------------ ``` With this change, the errors will read: ``` FILE: src/Standards/Squiz/Tests/Classes/ClassFileName Spaces In FilenameUnitTest.inc ----------------------------------------------------------------------------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE ----------------------------------------------------------------------------------------------------------------------------------------------------- 7 | ERROR | Filename doesn't match class name; expected file name "ClassFileNameSpacesInFilenameUnitTest.inc" (Squiz.Classes.ClassFileName.NoMatch) ----------------------------------------------------------------------------------------------------------------------------------------------------- FILE: src/Standards/Squiz/Tests/Classes/ClassFileName-Dashes-In-FilenameUnitTest.inc ----------------------------------------------------------------------------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE ----------------------------------------------------------------------------------------------------------------------------------------------------- 7 | ERROR | Filename doesn't match class name; expected file name "ClassFileNameDashesInFilenameUnitTest.inc" (Squiz.Classes.ClassFileName.NoMatch) ----------------------------------------------------------------------------------------------------------------------------------------------------- ``` Side-note: while the `strrpos($fullPath, '.')` _could_ return `false`, this is not something we need to be concerned about in this sniff as PHPCS doesn't examine files without a file extension (at this time), so there should always be a `.` in the file name.
1 parent a0b5353 commit c1cf608

File tree

4 files changed

+29
-5
lines changed

4 files changed

+29
-5
lines changed

src/Standards/Squiz/Sniffs/Classes/ClassFileNameSniff.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,9 @@ public function process(File $phpcsFile, $stackPtr)
4848
return $phpcsFile->numTokens;
4949
}
5050

51-
$fullPath = basename($filename);
52-
$fileName = substr($fullPath, 0, strrpos($fullPath, '.'));
51+
$fullPath = basename($filename);
52+
$fileName = substr($fullPath, 0, strrpos($fullPath, '.'));
53+
$extension = substr($fullPath, (strrpos($fullPath, '.') + 1));
5354

5455
$tokens = $phpcsFile->getTokens();
5556
$ooName = $phpcsFile->getDeclarationName($stackPtr);
@@ -59,11 +60,10 @@ public function process(File $phpcsFile, $stackPtr)
5960
}
6061

6162
if ($ooName !== $fileName) {
62-
$error = '%s name doesn\'t match filename; expected "%s %s"';
63+
$error = 'Filename doesn\'t match %s name; expected file name "%s"';
6364
$data = [
64-
ucfirst($tokens[$stackPtr]['content']),
6565
$tokens[$stackPtr]['content'],
66-
$fileName,
66+
$ooName.'.'.$extension,
6767
];
6868
$phpcsFile->addError($error, $stackPtr, 'NoMatch', $data);
6969
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
// Test to demonstrate that file names should follow class names, not the other way around.
4+
// Making the class name comply with the file name would result in a parse error for this file,
5+
// as the file name doesn't comply with the naming conventions for PHP symbol names.
6+
7+
class ClassFileNameSpacesInFilenameUnitTest {}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
// Test to demonstrate that file names should follow class names, not the other way around.
4+
// Making the class name comply with the file name would result in a parse error for this file,
5+
// as the file name doesn't comply with the naming conventions for PHP symbol names.
6+
7+
class ClassFileNameDashesInFilenameUnitTest

src/Standards/Squiz/Tests/Classes/ClassFileNameUnitTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,16 @@ public function getErrorList($testFile='')
111111
6 => 1,
112112
];
113113

114+
case 'ClassFileName Spaces In FilenameUnitTest.inc':
115+
return [
116+
7 => 1,
117+
];
118+
119+
case 'ClassFileName-Dashes-In-FilenameUnitTest.inc':
120+
return [
121+
7 => 1,
122+
];
123+
114124
default:
115125
return [];
116126
}//end switch

0 commit comments

Comments
 (0)