Skip to content

Commit 73875a7

Browse files
committed
- Introduce Collection usage: Import Illuminate\Support\Collection for better type hinting and usage.
- Update analizeCommittedFiles method: Refactor the method to better describe its purpose and work with a Collection of ChangedFile objects. - Add validateConfigPath method: Implement validation for configuration file paths to ensure they exist. - Refactor suggestAutoFixOrExit: Separate the autofix logic into a new method autoFixFiles for better readability and testability. - Update PintPreCommitHook: Simplify the conditional logic when validating the config path. - Add test datasets: Introduce new datasets for testing cases when ESLint autofixer does not fix files completely. - Add new test: Create a test to ensure the commit fails when ESLint autofixer is unable to fix all files.
1 parent 6ebc93d commit 73875a7

File tree

6 files changed

+84
-18
lines changed

6 files changed

+84
-18
lines changed

src/Console/Commands/Hooks/BaseCodeAnalyzerPreCommitHook.php

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Igorsgm\GitHooks\Git\ChangedFiles;
1010
use Igorsgm\GitHooks\Traits\ProcessHelper;
1111
use Illuminate\Console\Command;
12+
use Illuminate\Support\Collection;
1213
use Symfony\Component\Console\Terminal;
1314

1415
abstract class BaseCodeAnalyzerPreCommitHook
@@ -93,14 +94,15 @@ public function handleCommittedFiles(ChangedFiles $files, Closure $next)
9394
}
9495

9596
/**
96-
* Analyzes the committed files and checks if they are properly formatted.
97+
* Analyzes an array of ChangedFile objects and checks whether each file can be analyzed,
98+
* whether it is properly formatted according to the configured analyzer, and collects
99+
* paths of any files that are not properly formatted.
97100
*
98-
* @param mixed $commitFiles The files to analyze.
101+
* @param ChangedFile[]|Collection $commitFiles The files to analyze.
99102
* @return $this
100103
*/
101104
protected function analizeCommittedFiles($commitFiles)
102105
{
103-
/** @var ChangedFile $file */
104106
foreach ($commitFiles as $file) {
105107
if (! $this->canFileBeAnalyzed($file)) {
106108
continue;
@@ -181,6 +183,14 @@ protected function validateAnalyzerInstallation()
181183
throw new HookFailException();
182184
}
183185

186+
/**
187+
* Validates the given configuration path.
188+
*
189+
* @param string $path The path to the configuration file.
190+
* @return $this This instance for method chaining.
191+
*
192+
* @throws HookFailException If the configuration file does not exist.
193+
*/
184194
protected function validateConfigPath($path)
185195
{
186196
if (file_exists($path)) {
@@ -199,27 +209,50 @@ protected function validateConfigPath($path)
199209

200210
/**
201211
* Suggests attempting to automatically fix the incorrectly formatted files or exit.
202-
*
203-
* @return void
212+
* If any files cannot be fixed, throws a HookFailException to cancel the commit.
204213
*
205214
* @throws HookFailException
206215
*/
207-
protected function suggestAutoFixOrExit()
216+
protected function suggestAutoFixOrExit(): bool
208217
{
209218
$hasFixerCommand = ! empty($this->fixerCommand());
210219

211220
if (Terminal::hasSttyAvailable() && $hasFixerCommand &&
212-
$this->command->confirm('Would you like to attempt to correct files automagically?')
221+
$this->command->confirm('Would you like to attempt to correct files automagically?') &&
222+
$this->autoFixFiles()
213223
) {
214-
$errorFilesString = implode(' ', $this->filesBadlyFormattedPaths);
215-
216-
$this->runCommands([
217-
$this->fixerCommand().' '.$errorFilesString,
218-
'git add '.$errorFilesString,
219-
]);
220-
} else {
221-
throw new HookFailException();
224+
return true;
225+
}
226+
227+
throw new HookFailException();
228+
}
229+
230+
/**
231+
* Automatically fixes any files in the `$filesBadlyFormattedPaths` array using the
232+
* configured fixer command. For each fixed file, adds it to Git and removes its path
233+
* from the `$filesBadlyFormattedPaths` array.
234+
*
235+
*
236+
* @throws HookFailException if any files cannot be fixed.
237+
*/
238+
private function autoFixFiles(): bool
239+
{
240+
foreach ($this->filesBadlyFormattedPaths as $key => $filePath) {
241+
$wasProperlyFixed = $this->runCommands($this->fixerCommand().' '.$filePath)->isSuccessful();
242+
243+
if ($wasProperlyFixed) {
244+
$this->runCommands('git add '.$filePath);
245+
unset($this->filesBadlyFormattedPaths[$key]);
246+
247+
continue;
248+
}
249+
250+
$this->command->getOutput()->writeln(
251+
sprintf('<fg=red> %s Autofix Failed:</> %s', $this->getName(), $filePath)
252+
);
222253
}
254+
255+
return empty($this->filesBadlyFormattedPaths);
223256
}
224257

225258
/**

src/Console/Commands/Hooks/PintPreCommitHook.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,7 @@ protected function configParam(): string
6161
{
6262
$pintConfigFile = config('git-hooks.code_analyzers.laravel_pint.config');
6363

64-
if (! empty($pintConfigFile)) {
65-
$this->validateConfigPath($pintConfigFile);
66-
64+
if (! empty($pintConfigFile) && $this->validateConfigPath($pintConfigFile)) {
6765
return '--config '.trim($pintConfigFile, '/');
6866
}
6967

tests/Datasets/GitLogsDataset.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,3 +36,9 @@
3636
'AM temp/fixable-js-file.js',
3737
]),
3838
]);
39+
40+
dataset('listOfNonFixableJSFiles', [
41+
'List of Fixable Files' => implode(PHP_EOL, [
42+
'AM temp/not-fully-fixable-js-file.js',
43+
]),
44+
]);

tests/Datasets/PreCommitHooksDataset.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,9 @@
7979
$nonExistentPath = [
8080
'path' => 'nonexistent/path',
8181
'phpcs_path' => 'nonexistent/path',
82+
'preset' => null,
8283
'config' => __DIR__.'/../Fixtures/pintFixture.json',
84+
8385
];
8486

8587
dataset('codeAnalyzersList', [

tests/Features/Commands/Hooks/ESLintPreCommitHookTest.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,27 @@ function ($eslintConfiguration, $listOfFixableJSFiles) {
4747
->assertExitCode(1);
4848
})->with('eslintConfiguration', 'listOfFixableJSFiles');
4949

50+
test('Fails commit when ESLint autofixer does not fix the files completely',
51+
function ($eslintConfiguration, $listOfFixableNonJSFiles) {
52+
$this->config->set('git-hooks.code_analyzers.eslint', $eslintConfiguration);
53+
$this->config->set('git-hooks.pre-commit', [
54+
ESLintPreCommitHook::class,
55+
]);
56+
57+
$this->makeTempFile('not-fully-fixable-js-file.js',
58+
file_get_contents(__DIR__.'/../../../Fixtures/not-fully-fixable-js-file.js')
59+
);
60+
61+
GitHooks::shouldReceive('isMergeInProgress')->andReturn(false);
62+
GitHooks::shouldReceive('getListOfChangedFiles')->andReturn($listOfFixableNonJSFiles);
63+
64+
$this->artisan('git-hooks:pre-commit')
65+
->expectsOutputToContain('ESLint Failed')
66+
->expectsOutputToContain('COMMIT FAILED')
67+
->expectsConfirmation('Would you like to attempt to correct files automagically?', 'yes')
68+
->expectsOutputToContain('ESLint Autofix Failed');
69+
})->with('eslintConfiguration', 'listOfNonFixableJSFiles');
70+
5071
test('Commit passes when ESLint fixes the files', function ($eslintConfiguration, $listOfFixableJSFiles) {
5172
$this->config->set('git-hooks.code_analyzers.eslint', $eslintConfiguration);
5273
$this->config->set('git-hooks.pre-commit', [
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
function test()
2+
3+
{
4+
return false;
5+
6+
}

0 commit comments

Comments
 (0)