Skip to content

[Feature] Diff output configurable #482

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

Merged
merged 5 commits into from
May 13, 2021
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
6 changes: 3 additions & 3 deletions .github/workflows/frameworks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ jobs:
continue-on-error: ${{ env.allow_failure == 'true' }}
- name: Launch PHPInsights
working-directory: ./project
run: php vendor/bin/phpinsights -n --ansi
run: php vendor/bin/phpinsights -n --disable-security-check --ansi
continue-on-error: ${{ env.allow_failure == 'true' }}
- name: Launch PHPInsights Fixer
working-directory: ./project
Expand Down Expand Up @@ -131,9 +131,9 @@ jobs:
continue-on-error: ${{ env.allow_failure == 'true' }}
- name: Launch PHPInsights
working-directory: ./project
run: php artisan insights --ansi
run: php artisan insights -n --disable-security-check --ansi
continue-on-error: ${{ env.allow_failure == 'true' }}
- name: Launch PHPInsights Fixer
working-directory: ./project
run: php artisan insights --ansi --fix
run: php artisan insights -n --ansi --fix --disable-security-check
continue-on-error: ${{ env.allow_failure == 'true' }}
15 changes: 15 additions & 0 deletions docs/get-started.md
Original file line number Diff line number Diff line change
Expand Up @@ -178,3 +178,18 @@ composer bin phpinsights require nunomaduro/phpinsights

Between 2 analysis, issues are cached.
PHPInsights is smart enough to invalidate cache when it detect changes in your code, but you may completly flush cache before analysis by adding `--flush-cache` flag.

## Configure diff <Badge text="^2.0"/>

Some insights display a diff output.
If you want more context on the diff, configure it in the `phpinsights.php` file:

```php
<?php

return [
// ...
'diff_context' => 3,
// ...
];
```
13 changes: 12 additions & 1 deletion src/Application/Adapters/Laravel/Commands/InsightsCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use NunoMaduro\PhpInsights\Application\Console\Definitions\AnalyseDefinition;
use NunoMaduro\PhpInsights\Domain\Configuration;
use NunoMaduro\PhpInsights\Domain\Container;
use NunoMaduro\PhpInsights\Domain\Exceptions\InvalidConfiguration;
use NunoMaduro\PhpInsights\Domain\Kernel;
use NunoMaduro\PhpInsights\Domain\Reflection;
use RuntimeException;
Expand Down Expand Up @@ -47,7 +48,17 @@ public function handle(): int
* @noRector Rector\CodeQuality\Rector\Identical\SimplifyBoolIdenticalTrueRector
*/
$configuration['fix'] = $this->hasOption('fix') && (bool) $this->option('fix') === true;
$configuration = ConfigResolver::resolve($configuration, $this->input);
try {
$configuration = ConfigResolver::resolve($configuration, $this->input);
} catch (InvalidConfiguration $exception) {
$this->output->writeln([
'',
' <bg=red;options=bold> Invalid configuration </>',
' <fg=red>•</> <options=bold>' . $exception->getMessage() . '</>',
'',
]);
return 1;
}

$container = Container::make();
if (! $container instanceof \League\Container\Container) {
Expand Down
4 changes: 3 additions & 1 deletion src/Application/Console/Formatters/Console.php
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,8 @@ private function parseDetailMessage(Details $detail): string
$detailString = '';

foreach (explode(PHP_EOL, $detail->getMessage()) as $line) {
$detailString .= PHP_EOL;

if (mb_strpos($line, '-') === 0) {
$hasColor = true;
$detailString .= '<fg=red>';
Expand All @@ -637,7 +639,7 @@ private function parseDetailMessage(Details $detail): string
$detailString .= '<fg=green>';
}

$detailString .= OutputFormatter::escape($line) . PHP_EOL;
$detailString .= OutputFormatter::escape($line);

if ($hasColor) {
$hasColor = false;
Expand Down
15 changes: 14 additions & 1 deletion src/Application/Injectors/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
use NunoMaduro\PhpInsights\Application\Console\Definitions\DefinitionBinder;
use NunoMaduro\PhpInsights\Domain\Configuration as DomainConfiguration;
use NunoMaduro\PhpInsights\Domain\Container;
use NunoMaduro\PhpInsights\Domain\Exceptions\InvalidConfiguration;
use Psr\SimpleCache\CacheInterface;
use Symfony\Component\Console\Input\ArgvInput;
use Symfony\Component\Console\Output\ConsoleOutput;

/**
* @internal
Expand Down Expand Up @@ -52,7 +54,18 @@ public function __invoke(): array

$config['fix'] = $fixOption || $input->getFirstArgument() === 'fix';

return ConfigResolver::resolve($config, $input);
try {
return ConfigResolver::resolve($config, $input);
} catch (InvalidConfiguration $exception) {
(new ConsoleOutput())->getErrorOutput()
->writeln([
'',
' <bg=red;options=bold> Invalid configuration </>',
' <fg=red>•</> <options=bold>' . $exception->getMessage() . '</>',
'',
]);
die(1);
}
},
];
}
Expand Down
17 changes: 16 additions & 1 deletion src/Domain/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ final class Configuration

private int $threads;

private int $diffContext;

/**
* Configuration constructor.
*
Expand Down Expand Up @@ -237,6 +239,11 @@ public function getNumberOfThreads(): int
return $this->threads;
}

public function getDiffContext(): int
{
return $this->diffContext;
}

/**
* @param array<string, string|int|array|null> $config
*/
Expand All @@ -253,6 +260,7 @@ private function resolveConfig(array $config): void
'remove' => [],
'config' => [],
'fix' => false,
'diff_context' => 1,
]);

$resolver->setDefined('ide');
Expand All @@ -266,9 +274,15 @@ private function resolveConfig(array $config): void
$resolver->setAllowedValues('config', $this->validateConfigInsights());
$resolver->setAllowedValues('requirements', $this->validateRequirements());
$resolver->setAllowedTypes('threads', ['null', 'int']);
$resolver->setAllowedTypes('diff_context', 'int');
$resolver->setAllowedValues('diff_context', static fn ($value) => $value >= 0);
$resolver->setAllowedValues('threads', static fn ($value) => $value === null || $value >= 1);

$config = $resolver->resolve($config);
try {
$config = $resolver->resolve($config);
} catch (\Throwable $throwable) {
throw new InvalidConfiguration($throwable->getMessage(), $throwable->getCode(), $throwable);
}

$this->preset = $config['preset'];

Expand All @@ -286,6 +300,7 @@ private function resolveConfig(array $config): void
$this->config = $config['config'];
$this->requirements = $config['requirements'];
$this->fix = $config['fix'];
$this->diffContext = $config['diff_context'];

if (array_key_exists('ide', $config)
&& is_string($config['ide'])
Expand Down
4 changes: 3 additions & 1 deletion src/Domain/Differ.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ final class Differ implements DifferInterface

public function __construct()
{
$diffContext = Container::make()->get(Configuration::class)->getDiffContext();

$outputBuilder = new StrictUnifiedDiffOutputBuilder([
'collapseRanges' => true,
'commonLineThreshold' => 1,
'contextLines' => 0,
'contextLines' => $diffContext,
'fromFile' => '',
'toFile' => '',
]);
Expand Down
2 changes: 1 addition & 1 deletion src/Domain/Insights/FixerDecorator.php
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public function addDetails(Details $details): void

public function addDiff(string $file, string $diff): void
{
$diff = substr($diff, 8);
$diff = trim(substr($diff, 8));

$this->errors[] = Details::make()->setFile($file)->setDiff($diff)->setMessage($diff);
}
Expand Down
4 changes: 1 addition & 3 deletions tests/Application/ConfigResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputDefinition;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException;
use Tests\Fakes\FakeInput;

final class ConfigResolverTest extends TestCase
Expand All @@ -27,7 +26,6 @@ final class ConfigResolverTest extends TestCase
public function setUp(): void
{
parent::setUp();

$this->baseFixturePath = dirname(__DIR__) . DIRECTORY_SEPARATOR .
'Fixtures' . DIRECTORY_SEPARATOR . 'ConfigResolver' . DIRECTORY_SEPARATOR;
}
Expand Down Expand Up @@ -124,7 +122,7 @@ public function testResolvedConfigIsCorrectlyMerged(): void

public function testUnknownPresetThrowException(): void
{
$this->expectException(InvalidOptionsException::class);
$this->expectException(InvalidConfiguration::class);

$config = ['preset' => 'UnknownPreset'];

Expand Down
2 changes: 1 addition & 1 deletion tests/Domain/ConfigurationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public function testDefineNullForThreads(): void
*/
public function testExceptionOnInvalidSetThread($invalid): void
{
$this->expectException(\Symfony\Component\OptionsResolver\Exception\InvalidOptionsException::class);
$this->expectException(InvalidConfiguration::class);
$this->expectExceptionMessage('The option "threads" with value');
new Configuration(['threads' => $invalid]);
}
Expand Down