Merge pull request #3 from krakphp/exit-on-changes

Fail on Changes

Author: ragboyjr

README.md

@@ -240,6 +240,25 @@ The generate option allows you to control which generators get used for the spec
 
 The list of generator names you can use are: `constructor,from-validated-array,to-array,getters,withers`
 
+### Continuous Integration Setup
+
+If you are committing your struct gen changes directly in the repo, then you'll want to make sure that struct-gen was properly run and tested before any commit is merged into the mainline branch.
+
+In that case, you'll want to use the `--fail-on-changes` flag while running struct gen during your CI testing pipeline. This will fail with exit code 1 if any changes are detected which in most CI pipelines will fail the build ensuring that the developer submitting the patch has tested with the latest changes and didn't modify any of the generated files.
+
+Example:
+```yaml
+- composer struct-gen:generate --fail-on-changes
+```
+
+If you are generating the structs into an external file via the `generatedPath` option and are ignoring that file in your CVS, then you'll want to make sure to run struct-gen **before you run composer install**.
+
+```yaml
+- composer struct-gen:generate -vv
+# some point later
+- composer install --no-dev -o
+```
+
 ## Why use static generation?
 
 Most libraries for DTOs or structs are not IDE friendly and give access to helpful methods for a struct via runtime magic and reflection. Not only is there a slight performance hit with these methods, it's difficult to get typesafe while also working well with ides and static analysis tools.

src/Command/GenerateStructsCommand.php

@@ -11,10 +11,10 @@
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Finder\Finder;
 use function Krak\StructGen\{
+    detectChangesInGeneratedStructsForFiles,
     generateStructsExternallyForFiles,
     generateStructsForFiles,
-    traversePhpFiles
-};
+    traversePhpFiles};
 
 final class GenerateStructsCommand extends BaseCommand
 {
@@ -30,23 +30,27 @@ protected function configure() {
             ->setDescription('Generate struct info for any matching classes')
             ->addArgument('paths', InputArgument::OPTIONAL | InputArgument::IS_ARRAY, 'Path globs to search and list directories', $this->config->paths())
             ->addOption('generated-path', 'g', InputOption::VALUE_REQUIRED, 'If set, the file path to generate the structs into', $this->config->generatedPath())
+            ->addOption('fail-on-changes', 'f', InputOption::VALUE_NONE, 'Ensures no changes are detected on generated structs by exiting with a failure if changes are detected.')
             ->addOption('basic-traverse-files', 'b', InputOption::VALUE_NONE, 'By default, this command will use symfony finder to traverse the list of paths which support glob patterns. If you do not want to include symfony finder, you can use the basic traverse files feature.');
     }
 
     protected function execute(InputInterface $input, OutputInterface $output) {
         $paths = $input->getArgument('paths') ?: $this->config->paths();
         $generatedPath = $input->getOption('generated-path');
         $basicTraverseFiles = $input->getOption('basic-traverse-files');
+        $failOnChanges = $input->getOption('fail-on-changes');
         $files = $basicTraverseFiles ? traversePhpFiles($paths) : (function() use ($paths) {
             $finder = new Finder();
             $finder->files()->name('*.php')->in($paths);
             return $finder;
         })();
 
         if ($generatedPath) {
-            generateStructsExternallyForFiles($files, $generatedPath, new ConsoleLogger($output));
+            $res = generateStructsExternallyForFiles($files, $generatedPath, new ConsoleLogger($output));
         } else {
-            generateStructsForFiles($files, new ConsoleLogger($output));
+            $res = generateStructsForFiles($files, new ConsoleLogger($output));
         }
+
+        return $failOnChanges && $res->hasChanges() ? 1 : 0;
     }
 }

src/struct-gen.php

@@ -23,27 +23,43 @@ function traversePhpFiles(iterable $paths): iterable {
     }
 }
 
+final class GeneratedStructsForFilesResult {
+    private $hasChanges;
+
+    public function __construct(bool $hasChanges) {
+        $this->hasChanges = $hasChanges;
+    }
+
+    public function hasChanges(): bool {
+        return $this->hasChanges;
+    }
+}
+
 /**
  * @param \SplFileInfo[] $files
  * @param ?callable(string): string $generateStruct
  */
-function generateStructsForFiles(iterable $files, LoggerInterface $logger = null, ?callable $generateStruct = null): void {
+function generateStructsForFiles(iterable $files, LoggerInterface $logger = null, ?callable $generateStruct = null): GeneratedStructsForFilesResult {
     $logger = $logger ?: new NullLogger();
     $generateStruct = $generateStruct ?: new GenerateStruct();
+    $hasChanges = false;
     foreach ($files as $file) {
         $logger->info('Generate Structs for File: ' . $file->getPathname());
         $original = file_get_contents($file->getPathname());
         $updated = $generateStruct(GenerateStructArgs::inline($original))->code();
-        if ($original === $updated) {
+        if (trim($original) === trim($updated)) {
             $logger->debug('No changes detected.');
         } else {
             $logger->info("New Struct Info: \n". $updated);
             file_put_contents($file->getPathname(), $updated);
+            $hasChanges = true;
         }
     }
+
+    return new GeneratedStructsForFilesResult($hasChanges);
 }
 
-function generateStructsExternallyForFiles(iterable $files, string $generatedFilePath, LoggerInterface $logger = null, ?callable $generateStruct = null): void {
+function generateStructsExternallyForFiles(iterable $files, string $generatedFilePath, LoggerInterface $logger = null, ?callable $generateStruct = null): GeneratedStructsForFilesResult {
     $logger = $logger ?: new NullLogger();
     $generateStruct = $generateStruct ?: new GenerateStruct();
     $ast = [];
@@ -53,6 +69,20 @@ function generateStructsExternallyForFiles(iterable $files, string $generatedFil
         $ast = array_merge($ast, $generateStruct(GenerateStructArgs::external($original))->ast());
     }
 
+    $newContents = (new \PhpParser\PrettyPrinter\Standard())->prettyPrintFile($ast);
+
+    // perform md5 hash to try and keep memory down so we don't need to load both strings in memory
+    $currentHash = file_exists($generatedFilePath) ? md5_file($generatedFilePath) : '';
+    $newHash = md5($newContents);
+
+    if ($currentHash !== $newHash) {
+        $logger->debug("Changes detected: old hash = $currentHash, new hash = $newHash");
+    } else {
+        $logger->debug("No changes detected: hash $currentHash");
+    }
+
     $logger->info('Writing generated structs into: ' . $generatedFilePath);
-    file_put_contents($generatedFilePath, (new \PhpParser\PrettyPrinter\Standard())->prettyPrintFile($ast));
+    file_put_contents($generatedFilePath, $newContents);
+
+    return new GeneratedStructsForFilesResult($currentHash !== $newHash);
 }

test/Feature/StructGenTest.php

@@ -74,10 +74,9 @@ public function provide_for_can_generate_struct_files_externally() {
 
     /** @test */
     public function can_generate_structs_for_files() {
-        $tplPath = __DIR__ . '/Fixtures/generated-from-paths-template.php';
+        $this->given_generated_from_paths_is_initialized_with('template');
         $expectedPath = __DIR__ . '/Fixtures/generated-from-paths-expected.php';
-        copy($tplPath, self::GENERATED_FILE_PATH);
-        generateStructsForFiles(traversePhpFiles([__DIR__ . '/Fixtures/generated-from-paths.php']));
+        generateStructsForFiles(traversePhpFiles([self::GENERATED_FILE_PATH]));
         $this->assertEquals(
             trim(file_get_contents($expectedPath)),
             trim(file_get_contents(self::GENERATED_FILE_PATH))
@@ -98,6 +97,40 @@ public function can_generate_structs_externally_for_files() {
         );
     }
 
+    /**
+     * @depends can_generate_structs_externally_for_files
+     * @test
+     */
+    public function generating_structs_inline_for_files_can_detect_changes() {
+        // set of files is different, should result in changed output
+        $res = generateStructsExternallyForFiles(traversePhpFiles([
+            __DIR__ . '/Fixtures/generate-struct-externally-for-files-test-cases/Acme.php',
+            __DIR__ . '/Fixtures/generate-struct-externally-for-files-test-cases/GlobalClass.php',
+        ]), __DIR__ . '/Fixtures/generate-struct-externally-for-files-test-cases/.generated.php');
+        $this->assertEquals(true, $res->hasChanges());
+    }
+    
+    /**
+     * @dataProvider provide_for_generating_structs_externally_for_files_can_detect_changes
+     * @test
+     */
+    public function generating_structs_externally_for_files_can_detect_changes(string $type, bool $hasChanges) {
+        $this->given_generated_from_paths_is_initialized_with($type);
+        $res = generateStructsForFiles(traversePhpFiles([self::GENERATED_FILE_PATH]));
+        $this->assertEquals($hasChanges, $res->hasChanges());
+    }
+
+    public function provide_for_generating_structs_externally_for_files_can_detect_changes() {
+        yield 'has-changes' => ['template', true];
+        yield 'no-changes' => ['expected', false];
+    }
+
+    /** can be template or expected */
+    private function given_generated_from_paths_is_initialized_with(string $type = 'template') {
+        $tplPath = __DIR__ . "/Fixtures/generated-from-paths-{$type}.php";
+        copy($tplPath, self::GENERATED_FILE_PATH);
+    }
+
     /** @test */
     public function files_without_any_matching_classes_are_left_untouched() {
         $genStruct = new GenerateStruct();