Skip to content

Commit

Permalink
Dedicated CLI option --generate-baseline with improved DX
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Mar 22, 2020
1 parent be8ff32 commit f4a9d5f
Show file tree
Hide file tree
Showing 15 changed files with 490 additions and 17 deletions.
36 changes: 36 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -446,3 +446,39 @@ jobs:
../bin/phpstan analyse -l 8 src tests && \
php bin/compile && \
../tmp/phpstan.phar
generate-baseline:
name: "Generate baseline"

runs-on: "ubuntu-latest"

strategy:
matrix:
php-version:
- "7.4"

steps:
- name: "Checkout"
uses: "actions/[email protected]"

- name: "Install PHP"
uses: "shivammathur/[email protected]"
with:
coverage: "none"
php-version: "${{ matrix.php-version }}"

- name: "Cache dependencies"
uses: "actions/[email protected]"
with:
path: "~/.composer/cache"
key: "php-${{ matrix.php-version }}-composer-${{ hashFiles('**/composer.json') }}"

This comment has been minimized.

Copy link
@bendavies

bendavies Mar 23, 2020

Contributor

this should be composer.json instead of **/composer.json.
**/composer.json will hash on all additional composer.json files in the vendor directory on save.

This comment has been minimized.

Copy link
@ondrejmirtes

ondrejmirtes Mar 23, 2020

Author Member

I think it's fine. It's similar to the official example (https://github.com/actions/cache/blob/master/examples.md#php---composer) with the difference that you can' use composer.lock if it's not commited.

This comment has been minimized.

Copy link
@bendavies

bendavies Mar 23, 2020

Contributor

it is sort of fine as it works yes, but the issue and behavior i describe still exists.
FYI, ** used to be required which is why it's in all the examples. it's not needed any more so hashFiles('composer.json') will work.

This comment has been minimized.

Copy link
@ondrejmirtes

ondrejmirtes Mar 23, 2020

Author Member

What I don't like about actions/cache is that they don't save the new cache if the key already exists in the storage, but they could push the updated files there. So it's not ideal from my PoV either.

This comment has been minimized.

Copy link
@bendavies

bendavies Mar 23, 2020

Contributor

indeed. immutable by design.
you are better off caching on github.sha here to fix that.
php-${{ matrix.php-version }}-composer-${{ github.sha }

This comment has been minimized.

Copy link
@ondrejmirtes

ondrejmirtes Mar 23, 2020

Author Member

That would basically be like having no cache at all, right?

This comment has been minimized.

Copy link
@bendavies

bendavies Mar 23, 2020

Contributor

no, it would fallback to your restore-keys: php-${{ matrix.php-version }}-composer- and hit every time.

see https://help.github.com/en/actions/configuring-and-managing-workflows/caching-dependencies-to-speed-up-workflows#matching-a-cache-key

restore-keys: "php-${{ matrix.php-version }}-composer-"

- name: "Install dependencies"
run: "composer update --no-interaction --no-progress --no-suggest"

- name: "Generate baseline"
run: |
cp phpstan-baseline.neon phpstan-baseline-orig.neon && \
vendor/bin/phing phpstan-generate-baseline && \
diff phpstan-baseline.neon phpstan-baseline-orig.neon
42 changes: 42 additions & 0 deletions build.xml
Original file line number Diff line number Diff line change
Expand Up @@ -385,4 +385,46 @@
</exec>
</target>

<target name="phpstan-generate-baseline">
<property name="phpstan.config" value="build/phpstan-generated.neon"/>
<touch file="${phpstan.config}"/>
<append
destFile="${phpstan.config}"
text="includes: [ phpstan.neon"
append="false"
></append>
<if>
<equals arg1="${isPHP74}" arg2="true" />
<then>
<append
destFile="${phpstan.config}"
text=", ignore-gte-php7.4-errors.neon"
></append>
</then>
</if>
<append
destFile="${phpstan.config}"
text=" ]"
></append>
<exec
executable="php"
logoutput="true"
passthru="true"
checkreturn="true"
>
<arg value="-d"/>
<arg value="memory_limit=512M"/>
<arg path="bin/phpstan"/>
<arg value="analyse"/>
<arg value="-c"/>
<arg path="${phpstan.config}"/>
<arg value="-l"/>
<arg value="8"/>
<arg path="build/PHPStan"/>
<arg path="src"/>
<arg path="tests"/>
<arg value="--generate-baseline"/>
</exec>
</target>

</project>
3 changes: 0 additions & 3 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@


parameters:
ignoreErrors:
-
Expand Down Expand Up @@ -197,4 +195,3 @@ parameters:
count: 1
path: tests/PHPStan/Node/FileNodeTest.php


118 changes: 118 additions & 0 deletions src/Command/AnalyseCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,19 @@

namespace PHPStan\Command;

use PHPStan\Command\ErrorFormatter\BaselineNeonErrorFormatter;
use PHPStan\Command\ErrorFormatter\ErrorFormatter;
use PHPStan\Command\Symfony\SymfonyOutput;
use PHPStan\Command\Symfony\SymfonyStyle;
use PHPStan\File\ParentDirectoryRelativePathHelper;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Input\StringInput;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Output\StreamOutput;
use function file_put_contents;
use function stream_get_contents;

class AnalyseCommand extends \Symfony\Component\Console\Command\Command
{
Expand Down Expand Up @@ -44,6 +52,7 @@ protected function configure(): void
new InputOption('debug', null, InputOption::VALUE_NONE, 'Show debug information - which file is analysed, do not catch internal errors'),
new InputOption('autoload-file', 'a', InputOption::VALUE_REQUIRED, 'Project\'s additional autoload file path'),
new InputOption('error-format', null, InputOption::VALUE_REQUIRED, 'Format in which to print the result of the analysis', 'table'),
new InputOption('generate-baseline', null, InputOption::VALUE_OPTIONAL, 'Path to a file where the baseline should be saved', false),
new InputOption('memory-limit', null, InputOption::VALUE_REQUIRED, 'Memory limit for analysis'),
new InputOption('xdebug', null, InputOption::VALUE_NONE, 'Allow running with XDebug for debugging purposes'),
]);
Expand Down Expand Up @@ -79,6 +88,14 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$pathsFile = $input->getOption('paths-file');
$allowXdebug = $input->getOption('xdebug');

/** @var string|false|null $generateBaselineFile */
$generateBaselineFile = $input->getOption('generate-baseline');
if ($generateBaselineFile === false) {
$generateBaselineFile = null;
} elseif ($generateBaselineFile === null) {
$generateBaselineFile = 'phpstan-baseline.neon';
}

if (
!is_array($paths)
|| (!is_string($memoryLimit) && $memoryLimit !== null)
Expand All @@ -101,6 +118,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$autoloadFile,
$this->composerAutoloaderProjectPaths,
$configuration,
$generateBaselineFile,
$level,
$allowXdebug,
true
Expand Down Expand Up @@ -129,6 +147,19 @@ protected function execute(InputInterface $input, OutputInterface $output): int
return 1;
}

if ($errorFormat === 'baselineNeon') {
$errorOutput = $inceptionResult->getErrorOutput();
$errorOutput->writeLineFormatted('⚠️ You\'re using an obsolete option <fg=cyan>--error-format baselineNeon</>. ⚠️️');
$errorOutput->writeLineFormatted('');
$errorOutput->writeLineFormatted(' There\'s a new and much better option <fg=cyan>--generate-baseline</>. Here are the advantages:');
$errorOutput->writeLineFormatted(' 1) The current baseline file does not have to be commented-out');
$errorOutput->writeLineFormatted(' nor emptied when generating the new baseline. It\'s excluded automatically.');
$errorOutput->writeLineFormatted(' 2) Output no longer has to be redirected to a file, PHPStan saves the baseline');
$errorOutput->writeLineFormatted(' to a specified path (defaults to <fg=cyan>phpstan-baseline.neon</>).');
$errorOutput->writeLineFormatted(' 3) Baseline contains correct relative paths if saved to a subdirectory.');
$errorOutput->writeLineFormatted('');
}

/** @var ErrorFormatter $errorFormatter */
$errorFormatter = $container->getService($errorFormatterServiceName);

Expand All @@ -140,6 +171,21 @@ protected function execute(InputInterface $input, OutputInterface $output): int
throw new \PHPStan\ShouldNotHappenException();
}

$generateBaselineFile = $inceptionResult->getGenerateBaselineFile();
if ($generateBaselineFile !== null) {
$baselineExtension = pathinfo($generateBaselineFile, PATHINFO_EXTENSION);
if ($baselineExtension === '') {
$inceptionResult->getStdOutput()->getStyle()->error(sprintf('Baseline filename must have an extension, %s provided instead.', pathinfo($generateBaselineFile, PATHINFO_BASENAME)));
return $inceptionResult->handleReturn(1);
}

if ($baselineExtension !== 'neon') {
$inceptionResult->getStdOutput()->getStyle()->error(sprintf('Baseline filename extension must be .neon, .%s was used instead.', $baselineExtension));

return $inceptionResult->handleReturn(1);
}
}

$analysisResult = $application->analyse(
$inceptionResult->getFiles(),
$inceptionResult->isOnlyFiles(),
Expand All @@ -151,9 +197,81 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$input
);

if ($generateBaselineFile !== null) {
if (!$analysisResult->hasErrors()) {
$inceptionResult->getStdOutput()->getStyle()->error('No errors were found during the analysis. Baseline could not be generated.');

return $inceptionResult->handleReturn(1);
}

$baselineFileDirectory = dirname($generateBaselineFile);
$baselineErrorFormatter = new BaselineNeonErrorFormatter(new ParentDirectoryRelativePathHelper($baselineFileDirectory));

$streamOutput = $this->createStreamOutput();
$errorConsoleStyle = new ErrorsConsoleStyle(new StringInput(''), $streamOutput);
$output = new SymfonyOutput($streamOutput, new SymfonyStyle($errorConsoleStyle));
$baselineErrorFormatter->formatErrors($analysisResult, $output);

$stream = $streamOutput->getStream();
rewind($stream);
$baselineContents = stream_get_contents($stream);
if ($baselineContents === false) {
throw new \PHPStan\ShouldNotHappenException();
}

if (!is_dir($baselineFileDirectory)) {
$mkdirResult = @mkdir($baselineFileDirectory, 0644, true);
if ($mkdirResult === false) {
$inceptionResult->getStdOutput()->writeLineFormatted(sprintf('Failed to create directory "%s".', $baselineFileDirectory));

return $inceptionResult->handleReturn(1);
}
}

$writeResult = @file_put_contents($generateBaselineFile, $baselineContents);
if ($writeResult === false) {
$inceptionResult->getStdOutput()->writeLineFormatted(sprintf('Failed to write the baseline to file "%s".', $generateBaselineFile));

return $inceptionResult->handleReturn(1);
}

$errorsCount = 0;
$unignorableCount = 0;
foreach ($analysisResult->getFileSpecificErrors() as $fileSpecificError) {
if (!$fileSpecificError->canBeIgnored()) {
$unignorableCount++;
continue;
}

$errorsCount++;
}

$message = sprintf('Baseline generated with %d %s.', $errorsCount, $errorsCount === 1 ? 'error' : 'errors');

if (
$unignorableCount === 0
&& count($analysisResult->getNotFileSpecificErrors()) === 0
) {
$inceptionResult->getStdOutput()->getStyle()->success($message);
} else {
$inceptionResult->getStdOutput()->getStyle()->warning($message . "\nSome errors could not be put into baseline. Re-run PHPStan and fix them.");
}

return $inceptionResult->handleReturn(0);
}

return $inceptionResult->handleReturn(
$errorFormatter->formatErrors($analysisResult, $inceptionResult->getStdOutput())
);
}

private function createStreamOutput(): StreamOutput
{
$resource = fopen('php://memory', 'w', false);
if ($resource === false) {
throw new \PHPStan\ShouldNotHappenException();
}
return new StreamOutput($resource);
}

}
1 change: 1 addition & 0 deletions src/Command/ClearResultCacheCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$autoloadFile,
$this->composerAutoloaderProjectPaths,
$configuration,
null,
'0',
false,
false
Expand Down
40 changes: 34 additions & 6 deletions src/Command/CommandHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public static function begin(
?string $autoloadFile,
array $composerAutoloaderProjectPaths,
?string $projectConfigFile,
?string $generateBaselineFile,
?string $level,
bool $allowXdebug,
bool $manageMemoryLimitFile = true
Expand Down Expand Up @@ -95,6 +96,11 @@ public static function begin(
} else {
$projectConfigFile = $currentWorkingDirectoryFileHelper->absolutizePath($projectConfigFile);
}

if ($generateBaselineFile !== null) {
$generateBaselineFile = $currentWorkingDirectoryFileHelper->normalizePath($currentWorkingDirectoryFileHelper->absolutizePath($generateBaselineFile));
}

$defaultLevelUsed = false;
if ($projectConfigFile === null && $level === null) {
$level = self::DEFAULT_LEVEL;
Expand Down Expand Up @@ -138,8 +144,10 @@ public static function begin(
}

$loader = (new LoaderFactory(
$currentWorkingDirectoryFileHelper,
$containerFactory->getRootDirectory(),
$containerFactory->getCurrentWorkingDirectory()
$containerFactory->getCurrentWorkingDirectory(),
$generateBaselineFile
))->createLoader();

try {
Expand Down Expand Up @@ -217,8 +225,21 @@ public static function begin(
}
}

if ($projectConfigFile !== null) {
$allCustomConfigFiles = self::getConfigFiles(
$currentWorkingDirectoryFileHelper,
new NeonAdapter(),
new PhpAdapter(),
$projectConfigFile,
$loaderParameters,
$generateBaselineFile
);
} else {
$allCustomConfigFiles = [];
}

try {
$container = $containerFactory->create($tmpDir, $additionalConfigFiles, $paths, $composerAutoloaderProjectPaths, $analysedPathsFromConfig, $projectConfigFile !== null ? self::getConfigFiles(new NeonAdapter(), new PhpAdapter(), $projectConfigFile, $loaderParameters) : [], $level ?? self::DEFAULT_LEVEL);
$container = $containerFactory->create($tmpDir, $additionalConfigFiles, $paths, $composerAutoloaderProjectPaths, $analysedPathsFromConfig, $allCustomConfigFiles, $level ?? self::DEFAULT_LEVEL, $generateBaselineFile);
} catch (\Nette\DI\InvalidConfigurationException | \Nette\Utils\AssertionException $e) {
$errorOutput->writeLineFormatted('<error>Invalid configuration:</error>');
$errorOutput->writeLineFormatted($e->getMessage());
Expand Down Expand Up @@ -348,7 +369,8 @@ public static function begin(
$container,
$defaultLevelUsed,
$memoryLimitFile,
$projectConfigFile
$projectConfigFile,
$generateBaselineFile
);
}

Expand Down Expand Up @@ -386,7 +408,7 @@ private static function detectDuplicateIncludedFiles(
$phpAdapter = new PhpAdapter();
$allConfigFiles = [];
foreach ($configFiles as $configFile) {
$allConfigFiles = array_merge($allConfigFiles, self::getConfigFiles($neonAdapter, $phpAdapter, $configFile, $loaderParameters));
$allConfigFiles = array_merge($allConfigFiles, self::getConfigFiles($fileHelper, $neonAdapter, $phpAdapter, $configFile, $loaderParameters, null));
}

$normalized = array_map(static function (string $file) use ($fileHelper): string {
Expand Down Expand Up @@ -416,15 +438,21 @@ private static function detectDuplicateIncludedFiles(
* @param \Nette\DI\Config\Adapters\PhpAdapter $phpAdapter
* @param string $configFile
* @param array<string, string> $loaderParameters
* @param string|null $generateBaselineFile
* @return string[]
*/
private static function getConfigFiles(
FileHelper $fileHelper,
NeonAdapter $neonAdapter,
PhpAdapter $phpAdapter,
string $configFile,
array $loaderParameters
array $loaderParameters,
?string $generateBaselineFile
): array
{
if ($generateBaselineFile === $fileHelper->normalizePath($configFile)) {
return [];
}
if (!is_file($configFile) || !is_readable($configFile)) {
return [];
}
Expand All @@ -440,7 +468,7 @@ private static function getConfigFiles(
$includes = Helpers::expand($data['includes'], $loaderParameters);
foreach ($includes as $include) {
$include = self::expandIncludedFile($include, $configFile);
$allConfigFiles = array_merge($allConfigFiles, self::getConfigFiles($neonAdapter, $phpAdapter, $include, $loaderParameters));
$allConfigFiles = array_merge($allConfigFiles, self::getConfigFiles($fileHelper, $neonAdapter, $phpAdapter, $include, $loaderParameters, $generateBaselineFile));
}
}

Expand Down
1 change: 1 addition & 0 deletions src/Command/DumpDependenciesCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$autoloadFile,
$this->composerAutoloaderProjectPaths,
$configurationFile,
null,
'0', // irrelevant but prevents an error when a config file is passed
$allowXdebug,
true
Expand Down
Loading

0 comments on commit f4a9d5f

Please sign in to comment.