Add safeIdentifier and safeObjectProperty helpers

packages/protobuf-bench/README.md

@@ -10,5 +10,5 @@ server would usually do.
 
 | code generator      | bundle size             | minified               | compressed         |
 |---------------------|------------------------:|-----------------------:|-------------------:|
-| protobuf-es         | 86,799 b      | 36,976 b | 11,083 b |
+| protobuf-es         | 87,033 b      | 37,025 b | 11,104 b |
 | protobuf-javascript | 394,384 b  | 288,775 b | 54,825 b |

packages/protobuf-test/src/name.test.ts

@@ -0,0 +1,62 @@
+// Copyright 2021-2023 Buf Technologies, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import { codegenInfo } from "@bufbuild/protobuf";
+import { describe, expect, test } from "@jest/globals";
+
+describe("safeIdentifier", () => {
+  const { safeIdentifier } = codegenInfo;
+
+  test("sanitized reserved identifiers", () => {
+    expect(safeIdentifier("break")).toBe("break$");
+    expect(safeIdentifier("case")).toBe("case$");
+    expect(safeIdentifier("catch")).toBe("catch$");
+    expect(safeIdentifier("class")).toBe("class$");
+    expect(safeIdentifier("const")).toBe("const$");
+    expect(safeIdentifier("continue")).toBe("continue$");
+    expect(safeIdentifier("debugger")).toBe("debugger$");
+    expect(safeIdentifier("default")).toBe("default$");
+    expect(safeIdentifier("delete")).toBe("delete$");
+  });
+
+  test("does not modify other inputs which are not reserved identifiers", () => {
+    expect(safeIdentifier("constructor")).toBe("constructor");
+    expect(safeIdentifier("toString")).toBe("toString");
+    expect(safeIdentifier("toJSON")).toBe("toJSON");
+    expect(safeIdentifier("valueOf")).toBe("valueOf");
+  });
+});
+
+describe("safeObjectProperty", () => {
+  const { safeObjectProperty } = codegenInfo;
+
+  test("sanitizes reserved object property names", () => {
+    expect(safeObjectProperty("constructor")).toBe("constructor$");
+    expect(safeObjectProperty("toString")).toBe("toString$");
+    expect(safeObjectProperty("toJSON")).toBe("toJSON$");
+    expect(safeObjectProperty("valueOf")).toBe("valueOf$");
+  });
+
+  test("does not modify other inputs which are not reserved object properties", () => {
+    expect(safeObjectProperty("break")).toBe("break");
+    expect(safeObjectProperty("case")).toBe("case");
+    expect(safeObjectProperty("catch")).toBe("catch");
+    expect(safeObjectProperty("class")).toBe("class");
+    expect(safeObjectProperty("const")).toBe("const");
+    expect(safeObjectProperty("continue")).toBe("continue");
+    expect(safeObjectProperty("debugger")).toBe("debugger");
+    expect(safeObjectProperty("default")).toBe("default");
+    expect(safeObjectProperty("delete")).toBe("delete");
+  });
+});

packages/protobuf/src/codegen-info.ts

@@ -12,7 +12,11 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import { localName } from "./private/names.js";
+import {
+  localName,
+  safeIdentifier,
+  safeObjectProperty,
+} from "./private/names.js";
 import { getUnwrappedFieldType } from "./private/field-wrapper.js";
 import { scalarDefaultValue } from "./private/scalars.js";
 import { reifyWkt } from "./private/reify-wkt.js";
@@ -25,6 +29,8 @@ interface CodegenInfo {
   readonly wktSourceFiles: readonly string[];
   readonly scalarDefaultValue: typeof scalarDefaultValue;
   readonly reifyWkt: typeof reifyWkt;
+  readonly safeIdentifier: typeof safeIdentifier;
+  readonly safeObjectProperty: typeof safeObjectProperty;
 }
 
 type RuntimeSymbolName =
@@ -61,6 +67,8 @@ export const codegenInfo: CodegenInfo = {
   reifyWkt,
   getUnwrappedFieldType,
   scalarDefaultValue,
+  safeIdentifier,
+  safeObjectProperty,
   // prettier-ignore
   symbols: {
     proto2:               {typeOnly: false, privateImportPath: "./proto2.js",        publicImportPath: packageName},

packages/protobuf/src/private/names.ts

@@ -50,32 +50,28 @@ export function localName(
       const pkg = desc.file.proto.package;
       const offset = pkg === undefined ? 0 : pkg.length + 1;
       const name = desc.typeName.substring(offset).replace(/\./g, "_");
-      if (reservedIdent[name]) {
-        return name + "$";
-      }
-      return name;
+      // For services, we only care about safe identifiers, not safe object properties,
+      // but we have shipped v1 with a bug that respected object properties, and we
+      // do not want to introduce a breaking change, so we continue to escape for
+      // safe object properties.
+      // See https://github.com/bufbuild/protobuf-es/pull/391
+      return safeObjectProperty(safeIdentifier(name));
     }
     case "enum_value": {
       const sharedPrefix = desc.parent.sharedPrefix;
       if (sharedPrefix === undefined) {
         return desc.name;
       }
       const name = desc.name.substring(sharedPrefix.length);
-      if (reservedObjectProperties[name]) {
-        return name + "$";
-      }
-      return name;
+      return safeObjectProperty(name);
     }
     case "rpc": {
       let name = desc.name;
       if (name.length == 0) {
         return name;
       }
       name = name[0].toLowerCase() + name.substring(1);
-      if (reservedObjectProperties[name]) {
-        return name + "$";
-      }
-      return name;
+      return safeObjectProperty(name);
     }
   }
 }
@@ -84,15 +80,12 @@ export function localName(
  * Returns the name of a field in generated code.
  */
 export function localFieldName(protoName: string, inOneof: boolean) {
-  let name = protoCamelCase(protoName);
+  const name = protoCamelCase(protoName);
   if (inOneof) {
     // oneof member names are not properties, but values of the `case` property.
     return name;
   }
-  if (reservedObjectProperties[name] || reservedMessageProperties[name]) {
-    name = name + "$";
-  }
-  return name;
+  return safeObjectProperty(safeMessageProperty(name));
 }
 
 /**
@@ -180,98 +173,138 @@ function protoCamelCase(snakeCase: string): string {
   return b.join("");
 }
 
-// Names that cannot be used for identifiers, such as class names,
-// but _can_ be used for object properties.
-const reservedIdent: { [k: string]: boolean } = {
+/**
+ * Names that cannot be used for identifiers, such as class names,
+ * but _can_ be used for object properties.
+ */
+const reservedIdentifiers = new Set([
   // ECMAScript 2015 keywords
-  break: true,
-  case: true,
-  catch: true,
-  class: true,
-  const: true,
-  continue: true,
-  debugger: true,
-  default: true,
-  delete: true,
-  do: true,
-  else: true,
-  export: true,
-  extends: true,
-  false: true,
-  finally: true,
-  for: true,
-  function: true,
-  if: true,
-  import: true,
-  in: true,
-  instanceof: true,
-  new: true,
-  null: true,
-  return: true,
-  super: true,
-  switch: true,
-  this: true,
-  throw: true,
-  true: true,
-  try: true,
-  typeof: true,
-  var: true,
-  void: true,
-  while: true,
-  with: true,
-  yield: true,
+  "break",
+  "case",
+  "catch",
+  "class",
+  "const",
+  "continue",
+  "debugger",
+  "default",
+  "delete",
+  "do",
+  "else",
+  "export",
+  "extends",
+  "false",
+  "finally",
+  "for",
+  "function",
+  "if",
+  "import",
+  "in",
+  "instanceof",
+  "new",
+  "null",
+  "return",
+  "super",
+  "switch",
+  "this",
+  "throw",
+  "true",
+  "try",
+  "typeof",
+  "var",
+  "void",
+  "while",
+  "with",
+  "yield",
 
   // ECMAScript 2015 future reserved keywords
-  enum: true,
-  implements: true,
-  interface: true,
-  let: true,
-  package: true,
-  private: true,
-  protected: true,
-  public: true,
-  static: true,
+  "enum",
+  "implements",
+  "interface",
+  "let",
+  "package",
+  "private",
+  "protected",
+  "public",
+  "static",
 
   // Class name cannot be 'Object' when targeting ES5 with module CommonJS
-  Object: true,
+  "Object",
 
   // TypeScript keywords that cannot be used for types (as opposed to variables)
-  bigint: true,
-  number: true,
-  boolean: true,
-  string: true,
-  object: true,
+  "bigint",
+  "number",
+  "boolean",
+  "string",
+  "object",
 
   // Identifiers reserved for the runtime, so we can generate legible code
-  globalThis: true,
-  Uint8Array: true,
-  Partial: true,
-};
+  "globalThis",
+  "Uint8Array",
+  "Partial",
+]);
 
-// Names that cannot be used for object properties because they are reserved
-// by built-in JavaScript properties.
-const reservedObjectProperties: { [k: string]: boolean } = {
+/**
+ * Names that cannot be used for object properties because they are reserved
+ * by built-in JavaScript properties.
+ */
+const reservedObjectProperties = new Set([
   // names reserved by JavaScript
-  constructor: true,
-  toString: true,
-  toJSON: true,
-  valueOf: true,
-};
+  "constructor",
+  "toString",
+  "toJSON",
+  "valueOf",
+]);
 
-// Names that cannot be used for object properties because they are reserved
-// by the runtime.
-const reservedMessageProperties: { [k: string]: boolean } = {
+/**
+ * Names that cannot be used for object properties because they are reserved
+ * by the runtime.
+ */
+const reservedMessageProperties = new Set([
   // names reserved by the runtime
-  getType: true,
-  clone: true,
-  equals: true,
-  fromBinary: true,
-  fromJson: true,
-  fromJsonString: true,
-  toBinary: true,
-  toJson: true,
-  toJsonString: true,
+  "getType",
+  "clone",
+  "equals",
+  "fromBinary",
+  "fromJson",
+  "fromJsonString",
+  "toBinary",
+  "toJson",
+  "toJsonString",
 
   // names reserved by the runtime for the future
-  toObject: true,
+  "toObject",
+]);
+
+const fallback = <T extends string>(name: T) => `${name}$` as const;
+
+/**
+ * Will wrap names that are Object prototype properties or names reserved
+ * for `Message`s.
+ */
+const safeMessageProperty = (name: string): string => {
+  if (reservedMessageProperties.has(name)) {
+    return fallback(name);
+  }
+  return name;
+};
+
+/**
+ * Names that cannot be used for object properties because they are reserved
+ * by built-in JavaScript properties.
+ */
+export const safeObjectProperty = (name: string): string => {
+  if (reservedObjectProperties.has(name)) {
+    return fallback(name);
+  }
+  return name;
+};
+
+/**
+ * Names that can be used for identifiers or class properties
+ */
+export const safeIdentifier = (name: string): string => {
+  if (reservedIdentifiers.has(name)) {
+    return fallback(name);
+  }
+  return name;
 };