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

Feat: php-cs-fixer #154

Merged
merged 8 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
85 changes: 85 additions & 0 deletions .php-cs-fixer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
<?php

use PhpCsFixer\Config;
use PhpCsFixer\Finder;

/**
* Read excluded paths from .gitignore
*
* @param string $dir
*
* @return string[]
*/
function getGitignorePaths(string $dir): array
{
$gitignoreFile = $dir . '/.gitignore';
$paths = [];

if (!file_exists($gitignoreFile)) {
return $paths;
}

$lines = file($gitignoreFile, FILE_IGNORE_NEW_LINES | FILE_SKIP_EMPTY_LINES);
if (!empty($lines)) {
foreach ($lines as $line) {
// Ignore comments and empty lines
if (strpos($line, '#') === 0 || trim($line) === '') {
continue;
}
// Add relative paths
$paths[] = trim($line);
}
}

return $paths;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

According to the documentation:

Default finder ignores DIR . "/vendor" dir, “hidden” paths (ones starting with a dot) and VCS paths (e.g. .git), and filter only for *.php files.

What kind of files do you want to exclude with this function ? Most of the time, the defaut exclusions will do the job, and manually adding a specific dir is probably simplier than maintaining this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I initially intended to create a generic configuration that could be identical across each plugin, allowing for simple copy-and-paste. However, it might not be optimal, so I will remove it.


$projectDir = __DIR__;
$finder = Finder::create()
->in($projectDir)
->name('*.php');

// Exclude paths from .gitignore
$gitignorePaths = getGitignorePaths($projectDir);
foreach ($gitignorePaths as $path) {
$finder->notPath($path);
}

$config = new Config();

$mandatoryRules = [
'@PSR12' => true,
'@PER-CS2.0' => true,
AdrienClairembault marked this conversation as resolved.
Show resolved Hide resolved
];

$optionalRules = [
'array_indentation' => true,
'array_syntax' => ['syntax' => 'short'],
'binary_operator_spaces' => ['default' => 'align_single_space_minimal'],
'blank_line_after_namespace' => true,
'blank_line_after_opening_tag' => true,
'blank_line_before_statement' => ['statements' => ['return']],
'braces' => ['allow_single_line_closure' => true],
'cast_spaces' => ['space' => 'single'],
'class_attributes_separation' => ['elements' => ['method' => 'one']],
'concat_space' => ['spacing' => 'one'],
'declare_equal_normalize' => ['space' => 'single'],
'function_typehint_space' => true,
'lowercase_cast' => true,
'no_whitespace_in_blank_line' => true,
'single_blank_line_at_eof' => true,
'single_quote' => true,
'space_after_semicolon' => true,
'trailing_comma_in_multiline' => ['elements' => ['arrays']],
'trim_array_spaces' => true,
'unary_operator_spaces' => true,
'whitespace_after_comma_in_array' => true,
AdrienClairembault marked this conversation as resolved.
Show resolved Hide resolved
];
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not against some strict rules, but I guess that almost every PR will require an additional commit to fix CS violations.

Did you talk about it with the other developers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, but that's also the goal of this PR. Since NEWS doesn't have any other PRs in progress, it's being used as a test case ;)


$isFix = in_array('fix', $_SERVER['argv'], true);
$rules = $isFix ? array_merge($mandatoryRules, $optionalRules) : $mandatoryRules;

return $config
->setRules($rules)
->setFinder($finder)
->setUsingCache(false);
4 changes: 2 additions & 2 deletions ajax/display_alerts.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
*/

$AJAX_INCLUDE = 1;
include("../../../inc/includes.php");
header("Content-Type: text/html; charset=UTF-8");
include('../../../inc/includes.php');
header('Content-Type: text/html; charset=UTF-8');
Html::header_nocache();
Session::checkLoginUser();

Expand Down
4 changes: 2 additions & 2 deletions ajax/hide_alert.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
*/

$AJAX_INCLUDE = 1;
include("../../../inc/includes.php");
header("Content-Type: text/html; charset=UTF-8");
include('../../../inc/includes.php');
header('Content-Type: text/html; charset=UTF-8');
Html::header_nocache();
Session::checkLoginUser();

Expand Down
26 changes: 13 additions & 13 deletions ajax/targets.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,22 @@
*/

$AJAX_INCLUDE = 1;
include("../../../inc/includes.php");
header("Content-Type: text/html; charset=UTF-8");
include('../../../inc/includes.php');
header('Content-Type: text/html; charset=UTF-8');
Html::header_nocache();

Session::checkLoginUser();

if (isset($_POST['type']) && !empty($_POST['type'])) {
echo "<table class='tab_format'>";
echo "<tr>";
echo "<td>";
echo '<tr>';
echo '<td>';
switch ($_POST['type']) {
case 'User':
User::dropdown(['name' => 'items_id',
'right' => 'all',
'entity' => $_POST['entities_id'],
'entity_sons' => $_POST['is_recursive'],
User::dropdown(['name' => 'items_id',
'right' => 'all',
'entity' => $_POST['entities_id'],
'entity_sons' => $_POST['is_recursive'],
]);
break;

Expand All @@ -53,14 +53,14 @@
break;

case 'Profile':
Profile::dropdown(['name' => 'items_id',
'toadd' => [-1 => __('All', 'news')]
Profile::dropdown(['name' => 'items_id',
'toadd' => [-1 => __('All', 'news')],
]);
break;
}
echo "</td>";
echo '</td>';
echo "<td><input type='submit' name='addvisibility' value=\"" . _sx('button', 'Add', 'news') . "\"
class='submit'></td>";
echo "</tr>";
echo "</table>";
echo '</tr>';
echo '</table>';
}
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"php": ">=7.4"
},
"require-dev": {
"friendsofphp/php-cs-fixer": "^3.64",
"glpi-project/tools": "^0.7.3",
"php-parallel-lint/php-parallel-lint": "^1.4",
"phpstan/phpstan": "^1.12",
Expand Down
Loading
Loading