fix(upgrade): check for other extensions in flarum.extensions

src/steps/upgrade/twopointoh/base.ts

@@ -304,6 +304,12 @@ export abstract class BaseUpgradeStep implements Step<FlarumProviders> {
       // eslint-disable-next-line no-await-in-loop
       code = await this.updateCode(file, code, result.updated);
 
+      // Later replacements read the parsed code: parse it again after a text
+      // change, or one that returns a syntax tree would undo this change.
+      if (typeof result.updated === 'string') {
+        advanced = this.advancedContent(file, code);
+      }
+
       if (result.imports?.length || (result.updated && typeof result.updated !== 'string' && 'program' in result.updated)) {
         try {
           advanced = parseCode(code);

src/steps/upgrade/twopointoh/frontend/admin-extender.ts

@@ -7,6 +7,7 @@ import * as t from '@babel/types';
 import { parse } from 'recast/parsers/babel-ts';
 import { BaseUpgradeStep, GitCommit, Replacement } from '../base';
 import { formatCode } from '../../../../utils/ast';
+import { extensionId } from '../../../../utils/composer';
 
 const TODO = '@TODO: register this with Extend.Admin in admin/extend instead';
 const DOCS = 'https://docs.flarum.org/2.x/extend/admin';
@@ -462,10 +463,7 @@ export default class AdminExtender extends BaseUpgradeStep {
   /** The extension's ID, as core makes it from the package name. */
   protected extensionId(): string | null {
     try {
-      const name: string = JSON.parse(fs.readFileSync(path.join(this.root, 'composer.json'), 'utf8')).name;
-      const [vendor, pkg] = name.includes('/') ? name.split('/', 2) : [name, name];
-
-      return `${vendor}-${pkg.replace(/flarum-ext-/g, '').replace(/flarum-/g, '')}`;
+      return extensionId(JSON.parse(fs.readFileSync(path.join(this.root, 'composer.json'), 'utf8')).name);
     } catch {
       return null;
     }

src/steps/upgrade/twopointoh/frontend/misc.ts

@@ -2,7 +2,48 @@ import { BaseUpgradeStep, GitCommit, ImportChange, Replacement } from '../base';
 import chalk from 'chalk';
 import * as t from '@babel/types';
 import traverse from '@babel/traverse';
+import fs from 'fs';
+import path from 'path';
+import { parse } from 'recast/parsers/babel-ts';
 import { getFunctionName } from '../../../../utils/ast';
+import { extensionId } from '../../../../utils/composer';
+
+/** The bundled extensions' IDs, which are their initializers' names in 2.0. */
+const BUNDLED = new Set([
+  'flarum-akismet',
+  'flarum-approval',
+  'flarum-audit',
+  'flarum-bbcode',
+  'flarum-deck',
+  'flarum-embed',
+  'flarum-emoji',
+  'flarum-extension-manager',
+  'flarum-flags',
+  'flarum-gdpr',
+  'flarum-lang-english',
+  'flarum-likes',
+  'flarum-lock',
+  'flarum-markdown',
+  'flarum-mentions',
+  'flarum-messages',
+  'flarum-nicknames',
+  'flarum-pusher',
+  'flarum-realtime',
+  'flarum-statistics',
+  'flarum-sticky',
+  'flarum-subscriptions',
+  'flarum-suspend',
+  'flarum-tags',
+]);
+
+/** The 1.x names of the bundled initializers 2.0 renamed (flarum/framework#4003). */
+const RENAMED: Record<string, string> = {
+  lock: 'flarum-lock',
+  subscriptions: 'flarum-subscriptions',
+  'flarum/nicknames': 'flarum-nicknames',
+};
+
+const CHECK_TODO = "@TODO: check whether the extension is enabled with '<its extension ID>' in flarum.extensions instead";
 
 export default class MiscFrontendChanges extends BaseUpgradeStep {
   type = 'Miscellaneous frontend changes.';
@@ -387,19 +428,124 @@ export default class MiscFrontendChanges extends BaseUpgradeStep {
     };
   }
 
+  /**
+   * `app.initializers.has(name)` becomes `'id' in flarum.extensions`, where the
+   * upgrade knows the ID for certain: a bundled extension's, or one the
+   * extension's composer.json names. An initializer is not always named after
+   * its extension, so a check it can't be sure of gets a @TODO instead.
+   */
   private updateInitializers(): Replacement {
     return (_file, code) => {
-      if (!code.includes('this.currentTag')) return null;
+      if (!/initializers\s*\.\s*has\s*\(/.test(code)) return null;
+
+      const edits: Array<{ start: number; end: number; text: string }> = [];
+      const flagged = new Set<number>();
+
+      traverse(parse(code) as t.File, {
+        CallExpression: (p) => {
+          const id = this.checkedExtension(p.node);
+
+          if (id === undefined) return;
+
+          if (id) {
+            edits.push({ start: p.node.start!, end: p.node.end!, text: `('${id}' in flarum.extensions)` });
+
+            return;
+          }
+
+          // Above the statement, where a comment can't land in JSX or a string.
+          const statement = p.getStatementParent();
+
+          if (!statement || statement.node.leadingComments?.some((comment) => comment.value.includes(CHECK_TODO))) return;
+
+          const lineStart = code.lastIndexOf('\n', statement.node.start! - 1) + 1;
+
+          if (flagged.has(lineStart)) return;
+
+          flagged.add(lineStart);
+
+          const indent = /^[\t ]*/.exec(code.slice(lineStart))![0];
+          const docs = 'https://docs.flarum.org/2.x/extend/extending-extensions';
+
+          edits.push({
+            start: lineStart,
+            end: lineStart,
+            text: `${indent}// ${CHECK_TODO} (${docs}). An initializer is not always named after its extension, and some were renamed in 2.0.\n`,
+          });
+        },
+      });
+
+      if (edits.length === 0) return null;
 
       return {
-        updated: code
-          .replace(/initializers\.has\(["']lock["']\)/g, "initializers.has('flarum-lock')")
-          .replace(/initializers\.has\(["']subscriptions["']\)/g, "initializers.has('flarum-subscriptions')")
-          .replace(/initializers\.has\(["']flarum\/nicknames["']\)/g, "initializers.add('flarum-nicknames')"),
+        updated: edits
+          .sort((a, b) => b.start - a.start)
+          .reduce((result, edit) => result.slice(0, edit.start) + edit.text + result.slice(edit.end), code),
       };
     };
   }
 
+  /**
+   * For `app.initializers.has('name')`: the ID of the extension it checks for,
+   * or null when the upgrade can't be sure of it. Undefined for anything else.
+   */
+  private checkedExtension(call: t.CallExpression): string | null | undefined {
+    const callee = call.callee;
+
+    if (!t.isMemberExpression(callee) || callee.computed || !t.isIdentifier(callee.property, { name: 'has' })) return undefined;
+
+    const initializers = callee.object;
+
+    if (!t.isMemberExpression(initializers) || initializers.computed || !t.isIdentifier(initializers.property, { name: 'initializers' })) {
+      return undefined;
+    }
+
+    const owner = initializers.object;
+
+    if (!t.isIdentifier(owner, { name: 'app' }) && !(t.isMemberExpression(owner) && t.isIdentifier(owner.property, { name: 'app' }))) {
+      return undefined;
+    }
+
+    const [name] = call.arguments;
+
+    if (call.arguments.length !== 1 || !t.isStringLiteral(name)) return undefined;
+
+    return RENAMED[name.value] ?? (BUNDLED.has(name.value) ? name.value : this.namedExtensions().get(name.value) ?? null);
+  }
+
+  private named: Map<string, string> | null = null;
+
+  /**
+   * The extensions the composer.json names, by package name and by ID.
+   */
+  private namedExtensions(): Map<string, string> {
+    if (this.named) return this.named;
+
+    this.named = new Map();
+
+    let composer: Record<string, any> = {};
+
+    try {
+      composer = JSON.parse(fs.readFileSync(path.join(this.root, 'composer.json'), 'utf8'));
+    } catch {
+      return this.named;
+    }
+
+    const packages = [
+      ...Object.keys(composer.require ?? {}),
+      ...Object.keys(composer['require-dev'] ?? {}),
+      ...Object.keys(composer.suggest ?? {}),
+      ...(composer.extra?.['flarum-extension']?.['optional-dependencies'] ?? []),
+    ].filter((name: unknown): name is string => typeof name === 'string' && name.includes('/'));
+
+    for (const name of packages) {
+      this.named.set(name, extensionId(name));
+      this.named.set(extensionId(name), extensionId(name));
+    }
+
+    return this.named;
+  }
+
   private updateUploadImageButton(): Replacement {
     return (file, code) => {
       if (!code.includes('UploadImageButton')) return null;

src/utils/composer.ts

@@ -70,3 +70,13 @@ export type ComposerJsonSchema = {
 export function getComposerJson(fs: Store, paths: Paths): ComposerJsonSchema {
   return create(fs).readJSON(paths.package('composer.json')) as ComposerJsonSchema;
 }
+
+/**
+ * An extension's ID, from its package name, as core makes it: `vendor-package`,
+ * without a `flarum-ext-` or `flarum-` in the package's part.
+ */
+export function extensionId(packageName: string): string {
+  const [vendor, pkg] = packageName.includes('/') ? packageName.split('/', 2) : [packageName, packageName];
+
+  return `${vendor}-${pkg.replace(/flarum-ext-/g, '').replace(/flarum-/g, '')}`;
+}

test/steps/upgrade/initializer-checks.test.ts

@@ -0,0 +1,167 @@
+import fs from 'fs';
+import os from 'os';
+import path from 'path';
+import MiscFrontendChanges from '../../../src/steps/upgrade/twopointoh/frontend/misc';
+import { formatCode } from '../../../src/utils/ast';
+
+/**
+ * In 1.x, code checked whether another extension was enabled with
+ * app.initializers.has(name). Initializer names are not always the extension's
+ * ID, and some of core's changed in 2.0, so the upgrade checks the extension's
+ * ID in flarum.extensions instead, where it knows the ID for certain.
+ */
+
+function extension(composer: Record<string, unknown>): string {
+  const root = fs.mkdtempSync(path.join(os.tmpdir(), 'initializer-checks-'));
+
+  fs.writeFileSync(path.join(root, 'composer.json'), JSON.stringify({ name: 'acme/flarum-interstellar', ...composer }));
+
+  return root;
+}
+
+async function upgrade(code: string, composer: Record<string, unknown> = {}, file = 'js/src/forum/index.ts'): Promise<string> {
+  const step = new MiscFrontendChanges({} as any, extension(composer), 1, 1, false);
+  const result = await step.applyReplacements(file, code, step.advancedContent(file, code));
+
+  return result.updated as string;
+}
+
+const format = (code: string) => formatCode(code);
+
+const todo =
+  "// @TODO: check whether the extension is enabled with '<its extension ID>' in flarum.extensions instead (https://docs.flarum.org/2.x/extend/extending-extensions). An initializer is not always named after its extension, and some were renamed in 2.0.";
+
+describe('checking whether another extension is enabled', () => {
+  it("uses a bundled extension's ID", async () => {
+    // As fof/sitemap's 1.x forum has it.
+    expect(
+      await upgrade(`import app from 'flarum/forum/app';
+
+if (app.initializers.has('flarum-tags')) {
+  console.log('tags');
+}
+`)
+    ).toBe(
+      await format(`import app from 'flarum/forum/app';
+
+if ('flarum-tags' in flarum.extensions) {
+  console.log('tags');
+}
+`)
+    );
+  });
+
+  it('uses the IDs of the bundled extensions whose initializers 2.0 renamed', async () => {
+    // fof/username-request's 1.x forum checks 'flarum/nicknames', which 2.0
+    // renamed 'flarum-nicknames', so the check is false in 2.0.
+    expect(
+      await upgrade(`const nicknames = app.initializers.has('flarum/nicknames');
+const lock = app.initializers.has('lock');
+const subscriptions = app.initializers.has("subscriptions");
+`)
+    ).toBe(
+      await format(`const nicknames = 'flarum-nicknames' in flarum.extensions;
+const lock = 'flarum-lock' in flarum.extensions;
+const subscriptions = 'flarum-subscriptions' in flarum.extensions;
+`)
+    );
+  });
+
+  it('uses the ID of an extension the composer.json names, by its ID or its package name', async () => {
+    // As ianm/follow-users' and fof/drafts' 1.x forums have it.
+    const composer = {
+      require: { 'flarum/core': '^1.8', 'fof/follow-tags': '*' },
+      'require-dev': { 'clarkwinkelmann/flarum-ext-author-change': '*' },
+      suggest: { 'fof/user-directory': 'Lists users' },
+      extra: { 'flarum-extension': { 'optional-dependencies': ['fof/byobu'] } },
+    };
+
+    expect(
+      await upgrade(
+        `const tags = app.initializers.has('fof/follow-tags');
+const directory = app.initializers.has('fof-user-directory');
+const byobu = app.initializers.has('fof-byobu');
+const author = app.initializers.has('clarkwinkelmann-author-change');
+`,
+        composer
+      )
+    ).toBe(
+      await format(`const tags = 'fof-follow-tags' in flarum.extensions;
+const directory = 'fof-user-directory' in flarum.extensions;
+const byobu = 'fof-byobu' in flarum.extensions;
+const author = 'clarkwinkelmann-author-change' in flarum.extensions;
+`)
+    );
+  });
+
+  it('keeps what the check meant, whatever it is part of', async () => {
+    expect(
+      await upgrade(`const shown = !app.initializers.has('flarum-tags') && app.initializers.has('flarum-sticky') ? 1 : 2;
+`)
+    ).toBe(
+      await format(`const shown = !('flarum-tags' in flarum.extensions) && 'flarum-sticky' in flarum.extensions ? 1 : 2;
+`)
+    );
+  });
+
+  it('leaves a check on an extension whose ID it cannot know, with a @TODO above its statement', async () => {
+    // As datlechin/flarum-scroll-buttons' 1.x forum has it: neither extension
+    // is in its composer.json. The @TODO goes above the statement, not inside
+    // the JSX, where it would show.
+    const code = `export default function view() {
+  return (
+    <div>
+      {app.initializers.has('acpl/mobile-tab') && app.initializers.has('itnt-uitab') ? <span /> : null}
+    </div>
+  );
+}
+`;
+
+    expect(await upgrade(code, {}, 'js/src/forum/view.tsx')).toBe(
+      await format(`export default function view() {
+  ${todo}
+  return (
+    <div>
+      {app.initializers.has('acpl/mobile-tab') && app.initializers.has('itnt-uitab') ? <span /> : null}
+    </div>
+  );
+}
+`)
+    );
+  });
+
+  it("keeps its changes, and the misc step's other ones, when a later change rewrites the syntax tree", async () => {
+    // As fof/impersonate's 1.x ImpersonateModal has it: the modal becomes a
+    // FormModal by rewriting the syntax tree, after the text changes ran.
+    const code = await upgrade(`import Modal from 'flarum/common/components/Modal';
+
+export default class ImpersonateModal extends Modal {
+  oninit(vnode) {
+    super.oninit(vnode);
+
+    this.reasonEnabled = app.initializers.has('flarum-tags');
+    this.tag = this.currentTag;
+  }
+
+  content() {
+    return <input className="FormControl" />;
+  }
+}
+`);
+
+    expect(code).toContain('extends FormModal');
+    expect(code).toContain("this.reasonEnabled = 'flarum-tags' in flarum.extensions;");
+    expect(code).toContain('this.tag = app.currentTag;');
+  });
+
+  it('changes nothing the second time', async () => {
+    const once = await upgrade(`if (app.initializers.has('flarum-tags') || app.initializers.has('acme-other')) {
+  console.log('tags');
+}
+`);
+
+    expect(once).toContain("'flarum-tags' in flarum.extensions");
+    expect(once.match(/@TODO/g)).toHaveLength(1);
+    expect(await upgrade(once)).toBe(once);
+  });
+});