Add zizmor, improve performance

Author: Seldaek

.github/dependabot.yml

@@ -4,4 +4,6 @@ updates:
     directory: "/"
     schedule:
       interval: "weekly"
+    cooldown:
+      default-days: 7
     labels: []

.github/workflows/zizmor.yml

@@ -0,0 +1,33 @@
+name: GitHub Actions Security Analysis with zizmor 🌈
+
+on: # zizmor: ignore[concurrency-limits]
+    push:
+        branches:
+            - main
+        paths:
+            - '.github/**.yml'
+            - 'action.yml'
+    pull_request:
+        paths:
+            - '.github/**.yml'
+            - 'action.yml'
+
+permissions:
+    contents: read
+
+jobs:
+    zizmor:
+        name: Run zizmor 🌈
+        runs-on: ubuntu-latest
+        steps:
+            - name: Checkout repository
+              uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+              with:
+                  persist-credentials: false
+
+            - name: Run zizmor 🌈
+              uses: zizmorcore/zizmor-action@b1d7e1fb5de872772f31590499237e7cce841e8e # v0.5.3
+              with:
+                  advanced-security: false
+                  annotations: true
+                  persona: 'pedantic'

README.md

@@ -78,7 +78,7 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-  comment:
+  check-api-surface:
     runs-on: ubuntu-latest
     steps:
       - uses: composer/api-surface-check@main

action.yml

@@ -96,7 +96,9 @@ runs:
     - name: Fetch base ref
       if: steps.pr.outputs.number
       shell: bash
-      run: git fetch --no-tags --depth=1 origin "${{ steps.pr.outputs.base-ref }}:refs/remotes/origin/${{ steps.pr.outputs.base-ref }}"
+      env:
+        BASE_REF: ${{ steps.pr.outputs.base-ref }}
+      run: git fetch --no-tags --depth=1 origin "${BASE_REF}:refs/remotes/origin/${BASE_REF}"
 
     - name: Preflight scan
       id: preflight
@@ -107,8 +109,9 @@ runs:
         BASE_REF: origin/${{ steps.pr.outputs.base-ref }}
         PATHS_GLOB: ${{ inputs.paths }}
         OUTPUT_FILE: api-surface-preflight.txt
+        ACTION_PATH: ${{ github.action_path }}
       run: |
-        bash ${{ github.action_path }}/scripts/preflight.sh
+        bash "${ACTION_PATH}/scripts/preflight.sh"
         echo "should-run=$(cat api-surface-preflight.txt)" >> "$GITHUB_OUTPUT"
 
     - name: Setup PHP
@@ -143,7 +146,7 @@ runs:
           exit 0
         fi
         if [[ -d vendor ]]; then
-          echo "vendor-path=$(cd vendor && pwd)" >> "$GITHUB_OUTPUT"
+          echo "project-path=$(pwd)" >> "$GITHUB_OUTPUT"
         fi
 
     - name: Run analysis
@@ -155,7 +158,7 @@ runs:
         BASE_REF: origin/${{ steps.pr.outputs.base-ref }}
         PATHS_GLOB: ${{ inputs.paths }}
         SOURCE_ROOTS: ${{ inputs.source-roots }}
-        VENDOR_PATH: ${{ steps.project-deps.outputs.vendor-path }}
+        COMPOSER_PROJECT_PATH: ${{ steps.project-deps.outputs.project-path }}
         OUTPUT_DIR: ${{ inputs.output-dir }}
         INCLUDE_INTERNAL: ${{ inputs.include-internal }}
         TYPES: ${{ inputs.types }}
@@ -164,7 +167,8 @@ runs:
         SHOW_MODIFIED: ${{ inputs.show-modified }}
         COMMENT_MARKER: ${{ inputs.comment-marker }}
         HEADING: ${{ inputs.heading }}
-      run: bash ${{ github.action_path }}/scripts/detect.sh
+        ACTION_PATH: ${{ github.action_path }}
+      run: bash "${ACTION_PATH}/scripts/detect.sh"
 
     # Always run post-comment when we have a PR. With analysis skipped,
     # comment-body.txt is missing/empty and post-comment deletes any
@@ -177,4 +181,5 @@ runs:
         OUTPUT_DIR: ${{ inputs.output-dir }}
         COMMENT_MARKER: ${{ inputs.comment-marker }}
         PR_NUMBER: ${{ steps.pr.outputs.number }}
-      run: bash ${{ github.action_path }}/scripts/post-comment.sh
+        ACTION_PATH: ${{ github.action_path }}
+      run: bash "${ACTION_PATH}/scripts/post-comment.sh"

scripts/detect.sh

@@ -17,7 +17,7 @@ INCLUDE_INTERNAL="${INCLUDE_INTERNAL:-false}"
 TYPES="${TYPES:-class,interface,trait,enum,method,property,constant}"
 VISIBILITY="${VISIBILITY:-public,protected}"
 SHOW_REMOVED="${SHOW_REMOVED:-true}"
-SHOW_MODIFIED="${SHOW_MODIFIED:-false}"
+SHOW_MODIFIED="${SHOW_MODIFIED:-true}"
 COMMENT_MARKER="${COMMENT_MARKER:-<!-- api-surface-bot -->}"
 HEADING="${HEADING:-## API Surface Changes}"
 
@@ -58,16 +58,26 @@ fi
 echo "Analyzing ${#changed_files[@]} changed file(s)."
 files_csv=$(IFS=','; echo "${changed_files[*]}")
 
-# When install-dependencies is on, the action exports VENDOR_PATH as an
-# absolute path to the project's vendor/. We append it to the source roots so
-# better-reflection can walk through into vendor parents on both HEAD and BASE
-# snapshots (BASE uses HEAD's vendor since dependencies aren't in git).
+# When install-dependencies is on, the action exports COMPOSER_PROJECT_PATH
+# pointing at a directory containing composer.json + vendor/composer/installed.json.
+# Snapshotter then resolves vendor parents through composer's PSR-4 mappings
+# (O(1) per FQCN) instead of recursive directory walks.
+#
+# VENDOR_PATH is supported as a fallback for environments / tests where vendor/
+# is laid out without a real composer.json — it appends the directory to the
+# source roots so DirectoriesSourceLocator can find classes (slow on real
+# vendor trees; intended for synthetic fixtures).
 roots_for_snapshot=("${roots_array[@]}")
 if [[ -n "${VENDOR_PATH:-}" && -d "${VENDOR_PATH}" ]]; then
     roots_for_snapshot+=("${VENDOR_PATH}")
     echo "Including vendor path for resolution: ${VENDOR_PATH}"
 fi
 roots_csv=$(IFS=','; echo "${roots_for_snapshot[*]}")
+composer_arg=()
+if [[ -n "${COMPOSER_PROJECT_PATH:-}" && -d "${COMPOSER_PROJECT_PATH}" ]]; then
+    composer_arg=("--composer-path=${COMPOSER_PROJECT_PATH}")
+    echo "Using composer project path for vendor resolution: ${COMPOSER_PROJECT_PATH}"
+fi
 
 # Extract BASE source for parent/interface resolution and BASE-side reflection.
 base_tree="${OUTPUT_DIR}/.base-tree"
@@ -82,15 +92,17 @@ git archive "${BASE_REF}" -- "${roots_array[@]}" 2>/dev/null | tar -x -C "${base
 php "${SCRIPT_DIR}/snapshot.php" \
     --files="${files_csv}" \
     --roots="${roots_csv}" \
-    --output="${OUTPUT_DIR}/head-snapshot.json"
+    --output="${OUTPUT_DIR}/head-snapshot.json" \
+    "${composer_arg[@]}"
 
 # BASE snapshot — run from base-tree so relative paths match.
 (
     cd "${base_tree}"
     php "${SCRIPT_DIR}/snapshot.php" \
         --files="${files_csv}" \
         --roots="${roots_csv}" \
-        --output="${OLDPWD}/${OUTPUT_DIR}/base-snapshot.json"
+        --output="${OLDPWD}/${OUTPUT_DIR}/base-snapshot.json" \
+        "${composer_arg[@]}"
 )
 
 # Diff

scripts/snapshot.php

@@ -6,7 +6,7 @@
  * CLI wrapper around the Snapshotter.
  *
  * Usage:
- *   php snapshot.php --files=<comma-or-newline-list> --roots=<comma-or-newline-list> --output=<path>
+ *   php snapshot.php --files=<list> --roots=<list> --output=<path> [--composer-path=<dir>]
  */
 
 require __DIR__ . '/../vendor/autoload.php';
@@ -15,21 +15,24 @@
 
 $opts = parseArgs($argv);
 
-$snapshotter = new Snapshotter($opts['roots']);
+$snapshotter = new Snapshotter($opts['roots'], $opts['composer-path']);
 $records = $snapshotter->snapshot($opts['files']);
 
 file_put_contents($opts['output'], json_encode($records, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES) . "\n");
 
 function parseArgs(array $argv): array
 {
-    $opts = ['files' => [], 'roots' => [], 'output' => null];
+    $opts = ['files' => [], 'roots' => [], 'output' => null, 'composer-path' => null];
     foreach (array_slice($argv, 1) as $arg) {
         if (str_starts_with($arg, '--files=')) {
             $opts['files'] = splitList(substr($arg, 8));
         } elseif (str_starts_with($arg, '--roots=')) {
             $opts['roots'] = splitList(substr($arg, 8));
         } elseif (str_starts_with($arg, '--output=')) {
             $opts['output'] = substr($arg, 9);
+        } elseif (str_starts_with($arg, '--composer-path=')) {
+            $value = substr($arg, 16);
+            $opts['composer-path'] = $value !== '' ? $value : null;
         }
     }
     if ($opts['output'] === null) {

src/Snapshotter.php

@@ -30,8 +30,13 @@
 use Roave\BetterReflection\Reflector\Exception\IdentifierNotFound;
 use Roave\BetterReflection\Reflector\Reflector;
 use Roave\BetterReflection\SourceLocator\Type\AggregateSourceLocator;
+use Roave\BetterReflection\SourceLocator\Type\Composer\Factory\Exception\MissingComposerJson;
+use Roave\BetterReflection\SourceLocator\Type\Composer\Factory\Exception\MissingInstalledJson;
+use Roave\BetterReflection\SourceLocator\Type\Composer\Factory\MakeLocatorForComposerJsonAndInstalledJson;
 use Roave\BetterReflection\SourceLocator\Type\DirectoriesSourceLocator;
+use Roave\BetterReflection\SourceLocator\Type\MemoizingSourceLocator;
 use Roave\BetterReflection\SourceLocator\Type\PhpInternalSourceLocator;
+use Roave\BetterReflection\SourceLocator\Type\SingleFileSourceLocator;
 
 /**
  * Builds an array of API-surface symbol records for a set of files.
@@ -47,9 +52,11 @@ final class Snapshotter
     private Parser $parser;
 
     /**
-     * @param list<string> $sourceRoots Directories used to resolve parent classes / interfaces.
+     * @param list<string> $sourceRoots         Directories used to resolve parent classes / interfaces.
+     * @param string|null  $composerProjectPath Optional project root with composer.json + vendor/composer/installed.json. When set, parent / interface
+     *                                          lookups go through composer's PSR-4 mappings (O(1) per FQCN) instead of recursive directory scans.
      */
-    public function __construct(private array $sourceRoots)
+    public function __construct(private array $sourceRoots, private ?string $composerProjectPath = null)
     {
         $this->prettyPrinter = new PrettyPrinter();
         $this->parser = (new ParserFactory())->createForHostVersion();
@@ -73,7 +80,7 @@ public function snapshot(array $files): array
             return [];
         }
 
-        $reflector = $this->buildReflector();
+        $reflector = $this->buildReflector(array_keys($targetFiles));
         $records = [];
 
         // Enumerate target classes by parsing each target file directly with
@@ -150,21 +157,50 @@ private function extractClassFqcns(string $absPath): array
         return $fqcns;
     }
 
-    private function buildReflector(): Reflector
+    /**
+     * @param list<string> $targetAbsPaths Absolute paths of files we'll be snapshotting (added at the front of the
+     *                                     aggregate so target FQCN lookups don't fall through to vendor / src walks).
+     */
+    private function buildReflector(array $targetAbsPaths): Reflector
     {
         $br = new BetterReflection();
         $astLocator = $br->astLocator();
         $stubber = $br->sourceStubber();
 
         $locators = [];
+
+        // Target files first: lookups for classes declared in changed files
+        // resolve in O(1) without falling through to the slower DirectoriesSourceLocator.
+        foreach ($targetAbsPaths as $abs) {
+            if (is_file($abs)) {
+                $locators[] = new SingleFileSourceLocator($abs, $astLocator);
+            }
+        }
+
+        // Composer-aware locator: when a project with composer.json + installed.json
+        // is provided, all parent / interface lookups under PSR-4 / PSR-0 namespaces
+        // resolve in O(1) via prefix mapping. Without this, parent resolution into
+        // vendor walks every file (~thousands of file reads + AST parses per FQCN
+        // miss) — the dominant cost on real-world projects.
+        if ($this->composerProjectPath !== null && is_dir($this->composerProjectPath)) {
+            try {
+                $locators[] = (new MakeLocatorForComposerJsonAndInstalledJson())($this->composerProjectPath, $astLocator);
+            } catch (MissingComposerJson | MissingInstalledJson) {
+                // Fall through to source-roots only.
+            }
+        }
+
         foreach ($this->sourceRoots as $root) {
             if (is_dir($root)) {
                 $locators[] = new DirectoriesSourceLocator([$root], $astLocator);
             }
         }
         $locators[] = new PhpInternalSourceLocator($astLocator, $stubber);
 
-        return new DefaultReflector(new AggregateSourceLocator($locators));
+        // Memoize by FQCN: any parent / interface looked up more than once
+        // (which happens for every method / constant / property check on
+        // a class) reuses the cached result instead of re-walking locators.
+        return new DefaultReflector(new MemoizingSourceLocator(new AggregateSourceLocator($locators)));
     }
 
     /**

tests/DetectTest.php

@@ -139,7 +139,8 @@ public function testInterfaceImplementationIsNotReportedOnImplementer(): void
         ], 'implement Iface on Repo');
 
         $body = $this->runDetect();
-        $this->assertSame('', $body, 'Interface implementations should not be reported as new API surface.');
+        $this->assertStringNotContainsString('### New API Surface', $body, 'Interface implementations should not be reported as new API surface.');
+        $this->assertStringNotContainsString('L\\Repo::action', $body);
     }
 
     public function testRemovedPublicMethodIsReported(): void
@@ -219,7 +220,7 @@ public function testIncludeInternalFlagShowsInternalAdditions(): void
         $this->assertStringContainsString('L\\Foo::impl', $body);
     }
 
-    public function testModifiedSignatureRequiresFlag(): void
+    public function testModifiedSignatureFlag(): void
     {
         $this->commit([
             'src/Foo.php' => "<?php\nnamespace L;\nclass Foo {\n    public function mut(int \$x): void {}\n}\n",
@@ -228,15 +229,15 @@ public function testModifiedSignatureRequiresFlag(): void
             'src/Foo.php' => "<?php\nnamespace L;\nclass Foo {\n    public function mut(int \$x, int \$y): void {}\n}\n",
         ], 'add param');
 
-        // Default (show-modified=false) → empty body.
-        $this->assertSame('', $this->runDetect());
-
-        // With the flag → reported under Modified.
-        $body = $this->runDetect(['SHOW_MODIFIED' => 'true']);
+        // Default → reported under Modified.
+        $body = $this->runDetect();
         $this->assertStringContainsString('### Modified API Surface', $body);
         $this->assertStringContainsString('L\\Foo::mut', $body);
         $this->assertStringContainsString('was: `public function mut(int $x): void`', $body);
         $this->assertStringContainsString('now: `public function mut(int $x, int $y): void`', $body);
+
+        // Explicit opt-out → empty body.
+        $this->assertSame('', $this->runDetect(['SHOW_MODIFIED' => 'false']));
     }
 
     public function testVendorParentMethodIsNotReportedWhenVendorPathIsProvided(): void