Add actor-preferred-username-required lint rule

CHANGES.md

@@ -233,6 +233,13 @@ To be released.
         `endpoints.uploadMedia` is not built with
         `ctx.getMediaUploaderUri(identifier)`.
 
+ -  Added the `actor-preferred-username-required` lint rule, which warns
+    when an actor dispatcher's return value does not include a
+    `preferredUsername` property. [[#895], [#1022] by Jae-Hyuk-Jang\]
+
+[#895]: https://github.com/fedify-dev/fedify/issues/895
+[#1022]: https://github.com/fedify-dev/fedify/pull/1022
+
 ### @fedify/mysql
 
  -  Fixed the CommonJS MySQL adapter build so it no longer requires

changes.d/lint/actor-preferred-username-rule.md

@@ -0,0 +1,8 @@
+---
+links:
+  '#1022': https://github.com/fedify-dev/fedify/pull/1022
+  '#895': https://github.com/fedify-dev/fedify/issues/895
+---
+ -  Added the `actor-preferred-username-required` lint rule, which warns
+    when an actor dispatcher's return value does not include a
+    `preferredUsername` property. [[#895], [#1022] by Jae-Hyuk-Jang]

docs/manual/lint.md

@@ -557,6 +557,45 @@ federation
 
 [Object Integrity Proofs]: ./send.md#object-integrity-proofs
 
+### `actor-preferred-username-required`
+
+*This rule is introduced in Fedify 2.4.0.*
+
+Ensures actors have a `preferredUsername` property.
+
+**When this rule applies:**
+The actor dispatcher is configured with `setActorDispatcher()`, but the actor
+object doesn't include a `preferredUsername` property.
+
+**Why it matters:**
+Most fediverse software expects actors to expose a stable
+`preferredUsername`.  Omitting it tends to make remote display, search, and
+profile rendering worse, even though Fedify itself doesn't require it.
+
+~~~~ typescript twoslash
+// @noErrors: 2345
+import { createFederation } from "@fedify/fedify";
+import { Person } from "@fedify/vocab";
+const federation = createFederation<void>({ kv: null as any });
+// ---cut-before---
+// ❌ Bad: Missing preferredUsername property
+federation.setActorDispatcher("/users/{identifier}", (ctx, identifier) => {
+  return new Person({
+    id: ctx.getActorUri(identifier),
+    name: "John Doe",  // No preferredUsername!
+  });
+});
+
+// ✅ Good: Include preferredUsername property
+federation.setActorDispatcher("/users/{identifier}", (ctx, identifier) => {
+  return new Person({
+    id: ctx.getActorUri(identifier),
+    preferredUsername: identifier,
+    name: "John Doe",
+  });
+});
+~~~~
+
 ### `actor-inbox-property-required`
 
 Ensures `inbox` is defined when `setInboxListeners()` is configured.

packages/lint/src/index.ts

@@ -54,6 +54,9 @@ import {
 import {
   eslint as actorOutboxPropertyRequired,
 } from "./rules/actor-outbox-property-required.ts";
+import {
+  eslint as actorPreferredUsernameRequired,
+} from "./rules/actor-preferred-username-required.ts";
 import {
   eslint as actorPublicKeyRequired,
 } from "./rules/actor-public-key-required.ts";
@@ -110,6 +113,7 @@ const rules: Record<
   [RULE_IDS.actorUploadMediaPropertyMismatch]: actorUploadMediaPropertyMismatch,
   [RULE_IDS.actorPublicKeyRequired]: actorPublicKeyRequired,
   [RULE_IDS.actorAssertionMethodRequired]: actorAssertionMethodRequired,
+  [RULE_IDS.actorPreferredUsernameRequired]: actorPreferredUsernameRequired,
   [RULE_IDS.collectionFilteringNotImplemented]: collectionFiltering,
   [RULE_IDS.outboxListenerDeliveryRequired]: outboxListenerDeliveryRequired,
   [RULE_IDS.mediaUploaderObjectUriRequired]: mediaUploaderObjectUriRequired,

packages/lint/src/lib/const.ts

@@ -114,6 +114,13 @@ export const properties = {
     requiresIdentifier: true,
     isKeyProperty: true,
   },
+  preferredUsername: {
+    name: "preferredUsername",
+    path: ["preferredUsername"],
+    setter: "setActorDispatcher",
+    requiresIdentifier: false,
+    pluralName: "preferredUsernames",
+  },
 } as const satisfies Record<string, PropertyConfig>;
 
 /**
@@ -133,6 +140,7 @@ export const RULE_IDS = {
   actorUploadMediaPropertyRequired: "actor-upload-media-property-required",
   actorPublicKeyRequired: "actor-public-key-required",
   actorAssertionMethodRequired: "actor-assertion-method-required",
+  actorPreferredUsernameRequired: "actor-preferred-username-required",
 
   // Mismatch rules
   actorIdMismatch: "actor-id-mismatch",

packages/lint/src/lib/messages.ts

@@ -11,18 +11,24 @@ export const actorPropertyRequired = ({
   path,
   getter,
   requiresIdentifier = true,
-}: PropertyConfig): string =>
-  `When \`${setter}\` is configured, the \`${
-    path.join(".")
-  }\` property is recommended. Use \`${
-    getExpectedCall({
-      ctxName: "Context",
-      methodName: getter,
-      idName: "identifier",
-      path: path.join("."),
-      requiresIdentifier,
-    })
-  }\` for the \`${path.join(".")}\` property URI.`;
+}: PropertyConfig): string => {
+  const propertyPath = path.join(".");
+
+  const recommendation = getter == null
+    ? `Set the \`${propertyPath}\` property directly on the actor object.`
+    : `Use \`${
+      getExpectedCall({
+        ctxName: "Context",
+        methodName: getter,
+        idName: "identifier",
+        path: path.join("."),
+        requiresIdentifier,
+      })
+    }\` for the \`${propertyPath}\` property URI.`;
+
+  return `When \`${setter}\` is configured, the \`${propertyPath}\` ` +
+    `property is recommended. ${recommendation}`;
+};
 
 /**
  * Generates error message for *-mismatch rules.

packages/lint/src/lib/mismatch.ts

@@ -78,7 +78,7 @@ const getNameIfIdentifier = (node: Parameter): string | null =>
   node?.type === "Identifier" ? node.name : null;
 
 function createMismatchRule<Context = Deno.lint.RuleContext | Rule.RuleContext>(
-  config: PropertyConfig,
+  config: PropertyConfig & { getter: string },
   describe: (
     methodCallContext: MethodCallContext,
   ) => Context extends Deno.lint.RuleContext ? {
@@ -152,7 +152,7 @@ function createMismatchRule<Context = Deno.lint.RuleContext | Rule.RuleContext>(
  * @returns A Deno lint rule
  */
 export const createMismatchRuleDeno = (
-  config: PropertyConfig,
+  config: PropertyConfig & { getter: string },
 ): Deno.lint.Rule => ({
   create: createMismatchRule(
     config,
@@ -163,7 +163,7 @@ export const createMismatchRuleDeno = (
 });
 
 export const createMismatchRuleEslint = (
-  config: PropertyConfig,
+  config: PropertyConfig & { getter: string },
 ): Rule.RuleModule => ({
   meta: {
     type: "problem",

packages/lint/src/lib/property-checker.ts

@@ -126,14 +126,23 @@ const unwrapTypeScriptExpression = (node: Expression): Expression => {
 const isNullLiteral = (node: Expression): boolean =>
   node.type === "Literal" && node.value === null;
 
+const isTombstoneExpression = (node: Expression): boolean =>
+  node.type === "NewExpression" &&
+  node.callee.type === "Identifier" &&
+  node.callee.name === "Tombstone";
+
 // Check if both branches have the property
 const checkBranchWith =
   (propertyChecker: PropertyChecker) => (branch: Expression): boolean => {
     const expression = unwrapTypeScriptExpression(branch);
 
     // A null return means that no actor was found, so there is no actor object
-    // whose properties need to be checked.
-    if (isNullLiteral(expression)) return true;
+    // whose properties need to be checked. A Tombstone return means the actor
+    // was deleted, which ActorDispatcher explicitly permits, so it likewise
+    // has no actor properties to check.
+    if (isNullLiteral(expression) || isTombstoneExpression(expression)) {
+      return true;
+    }
 
     return pipe(
       expression,
@@ -215,6 +224,7 @@ export const createPropertySearcher = (propertyChecker: PropertyChecker) => {
         return checkAllReturnPaths(propertyChecker)(node);
 
       case "NewExpression":
+        if (isTombstoneExpression(node)) return true;
         return pipe(
           node,
           extractFirstObjectExpression,

packages/lint/src/lib/required.ts

@@ -77,7 +77,17 @@ function createRequiredRule<Context = Deno.lint.RuleContext | Rule.RuleContext>(
     const propertyChecker = createPropertyChecker(Boolean)(
       config.path,
     );
-    const propertySearcher = createPropertySearcher(propertyChecker);
+    const pluralPropertyChecker = config.pluralName == null ? null : (
+      createPropertyChecker(Boolean)([
+        ...config.path.slice(0, -1),
+        config.pluralName,
+      ])
+    );
+    const propertySearcher = createPropertySearcher(
+      pluralPropertyChecker == null
+        ? propertyChecker
+        : (node) => propertyChecker(node) || pluralPropertyChecker(node),
+    );
 
     return {
       VariableDeclarator: federationTracker.VariableDeclarator,