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

Look for badly named files #629

Open
wants to merge 9 commits into
base: trunk
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

59 changes: 58 additions & 1 deletion includes/Checker/Checks/Plugin_Repo/File_Type_Check.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@
if ( $this->flags & self::TYPE_APPLICATION ) {
$this->look_for_application_files( $result, $files );
}

// Check for badly named files.
$this->look_for_badly_named_files( $result, $files );
}

/**
Expand Down Expand Up @@ -244,6 +247,60 @@
}
}

/**
* Looks for application files and amends the given result with an error if found.
*
* @since 1.0.0
*
* @param Check_Result $result The check result to amend, including the plugin context to check.
* @param array $files List of absolute file paths.
*/
protected function look_for_badly_named_files( Check_Result $result, array $files ) {
$conflict_chars = '!@#$%^&*()+=[]{};:"\'<>,?/\\|`~';

foreach ( $files as $file ) {
$badly_name = false;
if ( preg_match( '/\s/', $file ) ) {
$badly_name = true;
}

if ( preg_match( '/[' . preg_quote( $conflict_chars, '/' ) . ']/', basename( $file ) ) ) {
$badly_name = true;
}

if ( $badly_name ) {
$this->add_result_error_for_file(
$result,
__( 'Badly named files are not permitted.', 'plugin-check' ),
'badly_named_files',
$file,
0,
0,
'',
8
);
}
}

// Duplicated names.
$files = array_map( 'basename', $files );
$files = array_map( 'strtolower', $files );
$duplicated_files = array_unique( array_diff_assoc( $files, array_unique( $files ) ) );

if ( ! empty( $duplicated_files ) ) {
$this->add_result_error_for_file(
$result,
__( 'Duplicated file names are not permitted.', 'plugin-check' ),
'duplicated_files',
implode( ', ', $duplicated_files ),
0,
0,
'',
8
);
}
}

/**
* Gets the description for the check.
*
Expand All @@ -254,7 +311,7 @@
* @return string Description.
*/
public function get_description(): string {
return __( 'Detects the usage of hidden and compressed files, VCS directories, and application files.', 'plugin-check' );
return __( 'Detects the usage of hidden and compressed files, VCS directories, application files and badly named files.', 'plugin-check' );

Check warning on line 314 in includes/Checker/Checks/Plugin_Repo/File_Type_Check.php

View check run for this annotation

Codecov / codecov/patch

includes/Checker/Checks/Plugin_Repo/File_Type_Check.php#L314

Added line #L314 was not covered by tests
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<?php
// File in a directory with a badly named file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php

// This file has a badly named file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php

// File for testing duplicate files in a plugin.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php

// File for testing duplicate files in a plugin.
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php
/**
* Plugin Name: Test Plugin File Type Badly Named Files Errors
* Plugin URI: https://github.com/wordpress/plugin-check
* Description: Some plugin description.
* Author: WordPress Review Team
* Author URI: https://make.wordpress.org/performance/
* License: GPLv2 or later
* License URI: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
* Text Domain: test-plugin-file-type-badly-named-files-errors
*
* @package test-plugin-file-type-badly-named-files-errors
*/

/**
* Plugin folder contains a file with a badly named file.
*/
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<?php

// This is an error file.
34 changes: 34 additions & 0 deletions tests/phpunit/tests/Checker/Checks/File_Type_Check_Tests.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,4 +98,38 @@ public function test_run_without_any_file_type_errors() {
$this->assertEmpty( $errors );
$this->assertSame( 0, $check_result->get_error_count() );
}

public function test_run_with_badly_named_errors() {
davidperezgar marked this conversation as resolved.
Show resolved Hide resolved
// Test plugin without any forbidden file types.
$check_context = new Check_Context( UNIT_TESTS_PLUGIN_DIR . 'test-plugin-file-type-badly-named-files-errors/load.php' );
$check_result = new Check_Result( $check_context );

$check = new File_Type_Check();
$check->run( $check_result );

$errors = $check_result->get_errors();

$this->assertNotEmpty( $errors );
$this->assertEquals( 4, $check_result->get_error_count() );

// Check for invalid name error.
$this->assertArrayHasKey( 0, $errors['plugin name.php'] );
$this->assertArrayHasKey( 0, $errors['plugin name.php'][0] );
$this->assertCount( 1, wp_list_filter( $errors['plugin name.php'][0][0], array( 'code' => 'badly_named_files' ) ) );

// Badly named directory check.
$this->assertArrayHasKey( 0, $errors['badly directory/file.php'] );
$this->assertArrayHasKey( 0, $errors['badly directory/file.php'][0] );
$this->assertCount( 1, wp_list_filter( $errors['badly directory/file.php'][0][0], array( 'code' => 'badly_named_files' ) ) );

// Badly named file with special chars.
$this->assertArrayHasKey( 0, $errors['badly|file%name!@#$%^&*()+=[]{};:"\'<>,?|`~.php'] );
$this->assertArrayHasKey( 0, $errors['badly|file%name!@#$%^&*()+=[]{};:"\'<>,?|`~.php'][0] );
$this->assertCount( 1, wp_list_filter( $errors['badly|file%name!@#$%^&*()+=[]{};:"\'<>,?|`~.php'][0][0], array( 'code' => 'badly_named_files' ) ) );

// Duplicated filenames.
$this->assertArrayHasKey( 0, $errors['class-filename.php'] );
$this->assertArrayHasKey( 0, $errors['class-filename.php'][0] );
$this->assertCount( 1, wp_list_filter( $errors['class-filename.php'][0][0], array( 'code' => 'duplicated_files' ) ) );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having same file name in separate folders should not be error I believe. From what I understand, duplicate files refers to like this:
filename.php and Filename.php. These two files in same location are treated differently based on operating system.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I have to force, I cannot create in my system two files like that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How could I do it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot create in my MacOS also. May be we need to rewrite test from an array of files names rather than actual filename.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that will be easiest

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can achieve that? some guide to make it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to achieve a positive outcome using things like mocking/overriding the ::get_files method to return a fake list of bad files.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any ideas @felixarntz ?

}
}