fix(upgrade): correct what a real 1.x extension's upgrade got wrong, and add --check

README.md

@@ -132,7 +132,7 @@ The CLI has different types of commands for different tasks:
 **Code Updates**: These commands help update extensions for newer versions of Flarum.
 
 - `flarum-cli update js-imports [PATH]`: Adds admin/forum/common namespaces to all JS imports from flarum core.
-- `flarum-cli upgrade 2.0 [PATH] [-n|--no-interaction] [--json] [--step <number>] [--force-steps]`: Upgrades a 1.x extension to Flarum 2.0, one step at a time, committing each step. Steps already committed are skipped, so running it again resumes where it stopped.
+- `flarum-cli upgrade 2.0 [PATH] [-n|--no-interaction] [--json] [--check] [--step <number>] [--force-steps]`: Upgrades a 1.x extension to Flarum 2.0, one step at a time, committing each step. Steps already committed are skipped, so running it again resumes where it stopped.
 
 To drive the upgrade from another program, such as an AI agent, pass `--json`. It asks nothing, and prints a single JSON object when it finishes. It exits non-zero when the upgrade didn't complete:
 
@@ -161,7 +161,9 @@ To drive the upgrade from another program, such as an AI agent, pass `--json`. I
 }
 ```
 
-Each step is `committed`, `no-changes`, `skipped` (committed by an earlier run), `failed` or `not-run`. A step that fails on some files lists them and makes no changes, so fix those files, commit, and run the command again. The `todos` are the `@TODO` comments the step's commit added, for what the upgrade could not change itself.
+Each step is `committed`, `no-changes`, `skipped` (committed by an earlier run), `failed` or `not-run`. A step that fails on some files lists them and makes no changes, so fix those files, commit, and run the command again. The `todos` are what the upgrade could not change itself: the `@TODO` comments the step's commit added, and work it can only point out, such as a file to add, which has a `line` of `null`. A `no-changes` step lists its `todos` too, when it has any.
+
+To see what the upgrade would change without changing anything, pass `--check`, such as for an extension already on Flarum 2.0. The steps run on the extension's last commit, in a temporary git worktree that is removed afterwards, and none is skipped. With `--json`, a step that would change something is `would-change`, with the `files` and `diff` of the change, and the report's `check` names the commit checked and whether changes not yet committed were left out.
 
 _And of course, you can always use the help command to see a list of all available commands with their descriptions:_
 

boilerplate/skeleton/extension/js/tsconfig.json

@@ -12,6 +12,9 @@
       "@types/**/*"
 ],
   "compilerOptions": {
+    // Only load the type packages the code imports. Others, such as the
+    // @types/node webpack brings along, use syntax TypeScript 4.9 can't read.
+    "types": [],
     // This will output typings to `dist-typings`
     "declarationDir": "./dist-typings",
     "baseUrl": ".",

boilerplate/skeleton/extension/js/tsconfig.test.json

@@ -3,6 +3,7 @@
     "include": ["tests/**/*"],
     "files": ["../../../node_modules/@flarum/jest-config/shims.d.ts"],
     "compilerOptions": {
+        "types": ["jest"],
         "strict": false,
         "noImplicitReturns": false,
         "noImplicitAny": false

php-subsystem/src/NodeUtil.php

@@ -55,18 +55,28 @@ public static function addUsesToNamespace(Node\Stmt\Namespace_ $node, array $use
             return true;
         }));
 
-        $lastUseStatement = null;
+        usort($uses, fn ($a, $b) => strcasecmp($a->uses[0]->name->toString(), $b->uses[0]->name->toString()));
 
-        foreach ($node->stmts as $index => $stmt) {
-            if ($stmt instanceof \PhpParser\Node\Stmt\Use_) {
-                $lastUseStatement = $index;
-            } elseif (! is_null($lastUseStatement)) {
-                break;
+        // Each goes where it sorts among the imports already there, as the
+        // Flarum StyleCI preset orders them, or after the last one.
+        foreach ($uses as $use) {
+            $lastUseStatement = null;
+            $position = null;
+
+            foreach ($node->stmts as $index => $stmt) {
+                if ($stmt instanceof \PhpParser\Node\Stmt\Use_) {
+                    $lastUseStatement = $index;
+
+                    if (is_null($position) && strcasecmp($stmt->uses[0]->name->toString(), $use->uses[0]->name->toString()) > 0) {
+                        $position = $index;
+                    }
+                } elseif (! is_null($lastUseStatement)) {
+                    break;
+                }
             }
-        }
 
-        if (! is_null($lastUseStatement)) {
-            array_splice($node->stmts, $lastUseStatement + 1, 0, $uses);
+            // Without any imports yet, it opens the namespace.
+            array_splice($node->stmts, $position ?? (is_null($lastUseStatement) ? 0 : $lastUseStatement + 1), 0, [$use]);
         }
     }
 

php-subsystem/src/Upgrade/TwoPointOh/JsonApi.php

@@ -135,9 +135,9 @@ function extenders(string $file, string $code, array $ast): ?ReplacementResult
 
         $traverser->addVisitor(new NodeVisitor\ParentConnectingVisitor());
 
+        // The TODO says what to write instead. Imports are left to whoever
+        // writes it: added now, they would go unused until then.
         $traverser->addVisitor(new class () extends \PhpParser\NodeVisitorAbstract {
-            public $hasExtenders = false;
-
             const LINK_TO_DOCS = 'https://docs.flarum.org/2.x/extend/api#extending-api-resources';
             const COMMENT = '// @TODO: Replace with the new implementation '.self::LINK_TO_DOCS;
 
@@ -157,28 +157,10 @@ public function enterNode(\PhpParser\Node $node)
                             if ($parent) {
                                 $parent->setAttribute('comments', [new \PhpParser\Comment\Doc(self::COMMENT)]);
                             }
-
-                            $this->hasExtenders = true;
                         }
                     }
                 }
             }
-
-            public function leaveNode(\PhpParser\Node $node)
-            {
-                if (! $this->hasExtenders) {
-                    return null;
-                }
-
-                if ($node instanceof \PhpParser\Node\Stmt\Namespace_) {
-                    NodeUtil::addUsesToNamespace($node, [
-                        'Flarum\Api\Context',
-                        'Flarum\Api\Endpoint',
-                        'Flarum\Api\Resource',
-                        'Flarum\Api\Schema',
-                    ]);
-                }
-            }
         });
 
         return new ReplacementResult($traverser->traverse($ast));

php-subsystem/src/Upgrade/TwoPointOh/Misc.php

@@ -2,6 +2,7 @@
 
 namespace Flarum\CliPhpSubsystem\Upgrade\TwoPointOh;
 
+use Flarum\CliPhpSubsystem\NodeUtil;
 use Flarum\CliPhpSubsystem\NodeVisitors\ChangeSignatures;
 use Flarum\CliPhpSubsystem\Upgrade\Replacement;
 use Flarum\CliPhpSubsystem\Upgrade\ReplacementResult;
@@ -198,7 +199,11 @@ public function enterNode(\PhpParser\Node $node)
                 if ($node instanceof \PhpParser\Node\Expr\MethodCall) {
                     if ($class = $this->getClass($node)) {
                         if ($class === 'Extend\Notification') {
-                            if ($node->name->name === 'type') {
+                            // 1.x's second argument is the serializer. 2.x's is the
+                            // drivers enabled by default, an array, which stays.
+                            $serializer = $node->args[1]->value ?? null;
+
+                            if ($node->name->name === 'type' && ($serializer instanceof \PhpParser\Node\Expr\ClassConstFetch || $serializer instanceof \PhpParser\Node\Scalar\String_)) {
                                 $args = $node->args;
                                 unset($args[1]);
                                 $node->args = array_values($args);
@@ -371,14 +376,13 @@ function alertable(string $file, string $code, array $ast, array $data): ?Replac
 
             public function enterNode(Node $node)
             {
-                // Does this class implement the Flarum\Notification\Blueprint\BlueprintInterface ?
+                // Does this class implement the Flarum\Notification\Blueprint\BlueprintInterface,
+                // and not yet the Flarum\Notification\AlertableInterface ?
                 if ($node instanceof Node\Stmt\Class_) {
-                    foreach ($node->implements as $interface) {
-                        if ($interface->getAttribute('resolvedName')->name === 'Flarum\Notification\Blueprint\BlueprintInterface') {
-                            $this->valid = true;
-                            break;
-                        }
-                    }
+                    $implements = array_map(fn ($interface) => $interface->getAttribute('resolvedName')->name, $node->implements);
+
+                    $this->valid = in_array('Flarum\Notification\Blueprint\BlueprintInterface', $implements)
+                        && ! in_array('Flarum\Notification\AlertableInterface', $implements);
                 }
             }
 
@@ -389,15 +393,7 @@ public function leaveNode(\PhpParser\Node $node)
                 }
 
                 if ($node instanceof Node\Stmt\Namespace_) {
-                    // Add the Flarum\Notification\AlertableInterface import
-                    $node->stmts = array_merge(
-                        [
-                            new Node\Stmt\Use_([
-                                new Node\Stmt\UseUse(new Node\Name('Flarum\Notification\AlertableInterface'))
-                            ])
-                        ],
-                        $node->stmts
-                    );
+                    NodeUtil::addUsesToNamespace($node, ['Flarum\Notification\AlertableInterface']);
                 }
 
                 if ($node instanceof \PhpParser\Node\Stmt\Class_) {

src/base-command.ts

@@ -60,7 +60,7 @@ export default abstract class BaseCommand extends Command {
     const { args } = await this.parse(this.constructor as any);
 
     this.args = args;
-    const path: string | undefined = args.path;
+    const path: string | undefined = this.stepsPath(args.path);
 
     const paths = new NodePaths({
       requestedDir: path,
@@ -72,6 +72,14 @@ export default abstract class BaseCommand extends Command {
     return steps.run(paths, this.genIO(), { php: phpProvider }, this.dry, args);
   }
 
+  /**
+   * Where the steps run, which is where the command was asked to unless a
+   * command runs them somewhere else.
+   */
+  protected stepsPath(path: string | undefined): string | undefined {
+    return path;
+  }
+
   async extRoot(path: string | undefined): Promise<string> {
     let extRoot: string;