Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Files/TestDoubles: support modern PHP, check paths case-sensitively and other improvements #344

Merged
merged 15 commits into from
Nov 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 3 additions & 32 deletions Yoast/Docs/Files/TestDoublesStandard.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,40 +5,11 @@
>
<standard>
<![CDATA[
Double/Mock test helper classes should be in their own file and placed in a dedicated test doubles sub-directory.
The name of the dedicated sub-directory is configurable.
Double/Mock test helper OO structures (classes/interfaces/traits/enums) should be in their own file and placed in a dedicated test fixtures sub-directory.

The name of the dedicated sub-directory is configurable.
]]>
</standard>
<code_comparison>
<code title="Valid: Double class in separate file in double sub-directory.">
<![CDATA[
<!-- Doubles file in double sub-directory -->
<?php
class Test_Double extends Original_Class {
// Code.
}

<!-- Unit test file -->
<?php
class Test_Original_Class extends TestCase {
// Test code.
}
]]>
</code>
<code title="Invalid: Having both the double class as well as the test class in the same file in the test directory.">
<![CDATA[
<!-- Unit test file -->
<?php
class Test_Double extends Original_Class {
// Code.
}

class Test_Original_Class extends TestCase {
// Test code.
}
]]>
</code>
</code_comparison>
<standard>
<![CDATA[
Double/Mock test helper classes should have "Double" or "Mock" in their class name.
Expand Down
202 changes: 76 additions & 126 deletions Yoast/Sniffs/Files/TestDoublesSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,20 @@

use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Sniffs\Sniff;
use PHP_CodeSniffer\Util\Tokens;
use PHPCSUtils\Utils\ObjectDeclarations;
use PHPCSUtils\Utils\TextStrings;
use YoastCS\Yoast\Utils\PathHelper;
use YoastCS\Yoast\Utils\PathValidationHelper;

/**
* Check that all mock/doubles classes are in their own file and in a `doubles` directory.
* Check that all mock/doubles OO structures are in a dedicated directory for test fixtures.
*
* Additionally, checks that all classes in the `doubles` directory/directories
* have `Double` or `Mock` in the class name.
* Additionally, checks that all OO structures in this fixtures directory/directories
* have `Double` or `Mock` in their name.
*
* @since 1.0.0
* @since 3.0.0 This sniff will no longer check for multiple OO object declarations within one file.
*/
final class TestDoublesSniff implements Sniff {

Expand All @@ -22,42 +27,34 @@ final class TestDoublesSniff implements Sniff {
* The paths should be relative to the root/basepath of the project and can be
* customized from within a custom ruleset.
*
* Preferably only one path is provided per project, but in exceptional circumstances
* multiple paths can be allowed.
*
* The new PHPCS 3.4.0 array `extend` feature can be used to add to this list.
* To overrule the list, just set the property.
* {@link https://github.com/squizlabs/PHP_CodeSniffer/pull/2154}
*
* @since 1.0.0
* @since 1.1.0 The property type has changed from string to array.
* Use of this property with a string value has been deprecated.
* @since 3.0.0 The default value has changed to an empty array.
* This property will now always need to be set from within a ruleset.
*
* @var string[]
* @var array<string>
*/
public $doubles_path = [
'/tests/doubles',
];
public $doubles_path = [];

/**
* Validated absolute target paths for test double/mock classes or an empty array
* Validated absolute target paths for test fixture directories or an empty array
* if the intended target directory/directories don't exist.
*
* @var string[]
* @var array<string, string>
*/
protected $target_paths;
private $target_paths;

/**
* Returns an array of tokens this test wants to listen for.
*
* @return array<int|string>
*/
public function register() {
return [
\T_CLASS,
\T_INTERFACE,
\T_TRAIT,
];
$targets = Tokens::$ooScopeTokens;
unset( $targets[ \T_ANON_CLASS ] );

return $targets;
}

/**
Expand All @@ -71,10 +68,10 @@ public function register() {
*/
public function process( File $phpcsFile, $stackPtr ) {
// Stripping potential quotes to ensure `stdin_path` passed by IDEs does not include quotes.
$file = \preg_replace( '`^([\'"])(.*)\1$`Ds', '$2', $phpcsFile->getFileName() );
$file = TextStrings::stripQuotes( $phpcsFile->getFileName() );

if ( $file === 'STDIN' ) {
return;
return; // @codeCoverageIgnore
}

if ( ! isset( $phpcsFile->config->basepath ) ) {
Expand All @@ -98,30 +95,10 @@ public function process( File $phpcsFile, $stackPtr ) {
return ( $phpcsFile->numTokens + 1 );
}

/*
* BC-compatibility for when the property was still a string.
*
* {@internal This should be removed in YoastCS 2.0.0.}}
*/
if ( \is_string( $this->doubles_path ) ) {
$this->doubles_path = (array) $this->doubles_path;
}

$tokens = $phpcsFile->getTokens();
$base_path = $this->normalize_directory_separators( $phpcsFile->config->basepath );
$base_path = \rtrim( $base_path, '/' ) . '/'; // Make sure the base_path ends in a single slash.

if ( ! isset( $this->target_paths ) || \defined( 'PHP_CODESNIFFER_IN_TESTS' ) ) {
$this->target_paths = [];

foreach ( $this->doubles_path as $doubles_path ) {
$target_path = $base_path;
$target_path .= \trim( $this->normalize_directory_separators( $doubles_path ), '/' ) . '/';

if ( \file_exists( $target_path ) && \is_dir( $target_path ) ) {
$this->target_paths[] = \strtolower( $target_path );
}
}
$this->target_paths = PathValidationHelper::relative_to_absolute( $phpcsFile, $this->doubles_path );
$this->target_paths = \array_filter( $this->target_paths, 'file_exists' );
$this->target_paths = \array_filter( $this->target_paths, 'is_dir' );
}

$object_name = ObjectDeclarations::getName( $phpcsFile, $stackPtr );
Expand All @@ -134,99 +111,72 @@ public function process( File $phpcsFile, $stackPtr ) {
$name_contains_double_or_mock = true;
}

$tokens = $phpcsFile->getTokens();
if ( empty( $this->target_paths ) === true ) {
if ( $name_contains_double_or_mock === true ) {
// No valid target paths found.
$data = [
$phpcsFile->config->basepath,
];

if ( \count( $this->doubles_path ) === 1 ) {
$data[] = 'directory';
$data[] = \implode( '', $this->doubles_path );
}
else {
$all_paths = \implode( '", "', $this->doubles_path );
$all_paths = \substr_replace( $all_paths, ' and', \strrpos( $all_paths, ',' ), 1 );

$data[] = 'directories';
$data[] = $all_paths;
}

$phpcsFile->addError(
'Double/Mock test helper class detected, but no test doubles sub-%2$s found in "%1$s". Expected: "%3$s". Please create the sub-%2$s.',
$stackPtr,
'NoDoublesDirectory',
$data
);
}
}
else {
$path_to_file = $this->normalize_directory_separators( $file );
$is_double_dir = false;

foreach ( $this->target_paths as $target_path ) {
if ( \stripos( $path_to_file, $target_path ) !== false ) {
$is_double_dir = true;
break;
}
if ( $name_contains_double_or_mock === false ) {
return;
}

// Mock/Double class found, but no valid target paths found.
$data = [
$tokens[ $stackPtr ]['content'],
$object_name,
$phpcsFile->config->basepath,
];

if ( $name_contains_double_or_mock === true && $is_double_dir === false ) {
$phpcsFile->addError(
'Double/Mock test helper classes should be placed in a dedicated test doubles sub-directory. Found %s: %s',
$stackPtr,
'WrongDirectory',
$data
);
if ( \count( $this->doubles_path ) === 1 ) {
$data[] = 'directory';
$data[] = \implode( '', $this->doubles_path );
}
elseif ( $name_contains_double_or_mock === false && $is_double_dir === true ) {
$phpcsFile->addError(
'Double/Mock test helper classes should contain "Double" or "Mock" in the class name. Found %s: %s',
$stackPtr,
'InvalidClassName',
$data
);
else {
$all_paths = \implode( '", "', $this->doubles_path );
$all_paths = \substr_replace( $all_paths, ' and', \strrpos( $all_paths, ',' ), 1 );

$data[] = 'directories';
$data[] = $all_paths;
}

$phpcsFile->addError(
'Double/Mock test helper %1$s detected, but no test fixtures sub-%3$s found in "%2$s". Expected: "%4$s". Please create the sub-%3$s.',
$stackPtr,
'NoDoublesDirectory',
$data
);

return;
}

if ( $name_contains_double_or_mock === true ) {
$more_objects_in_file = $phpcsFile->findNext( $this->register(), ( $stackPtr + 1 ) );
if ( $more_objects_in_file === false ) {
$more_objects_in_file = $phpcsFile->findPrevious( $this->register(), ( $stackPtr - 1 ) );
}
$path_to_file = PathHelper::normalize_absolute_path( $file );
$is_double_dir = false;

if ( $more_objects_in_file !== false ) {
$data = [
$tokens[ $stackPtr ]['content'],
$object_name,
$tokens[ $more_objects_in_file ]['content'],
ObjectDeclarations::getName( $phpcsFile, $more_objects_in_file ),
];

$phpcsFile->addError(
'Double/Mock test helper classes should be in their own file. Found %1$s: %2$s and %3$s: %4$s',
$stackPtr,
'OneObjectPerFile',
$data
);
foreach ( $this->target_paths as $target_path ) {
if ( PathHelper::path_starts_with( $path_to_file, $target_path ) === true ) {
$is_double_dir = true;
break;
}
}
}

/**
* Normalize all directory separators to be a forward slash.
*
* @param string $path Path to normalize.
*
* @return string
*/
private function normalize_directory_separators( $path ) {
return \strtr( $path, '\\', '/' );
$data = [
$tokens[ $stackPtr ]['content'],
$object_name,
];

if ( $name_contains_double_or_mock === true && $is_double_dir === false ) {
$phpcsFile->addError(
'Double/Mock test helpers should be placed in a dedicated test fixtures sub-directory. Found %s: %s',
$stackPtr,
'WrongDirectory',
$data
);
return;
}

if ( $name_contains_double_or_mock === false && $is_double_dir === true ) {
$phpcsFile->addError(
'Double/Mock test helpers should contain "Double" or "Mock" in the class name. Found %s: %s',
$stackPtr,
'InvalidClassName',
$data
);
}
}
}
Loading