Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add safeIdentifier and safeObjectProperty helpers #391

Merged
merged 10 commits into from
Mar 15, 2023
52 changes: 52 additions & 0 deletions packages/protobuf-test/src/name.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { codegenInfo } from "@bufbuild/protobuf";
import { describe, expect, test } from "@jest/globals";

describe("safeIdentifier", () => {
const { safeIdentifier } = codegenInfo;

// prettier-ignore
test("sanitized reserved identifiers", () => {
expect<"break$">(safeIdentifier("break")).toBe("break$");
expect<"case$">(safeIdentifier("case")).toBe("case$");
expect<"catch$">(safeIdentifier("catch")).toBe("catch$");
expect<"class$">(safeIdentifier("class")).toBe("class$");
expect<"const$">(safeIdentifier("const")).toBe("const$");
expect<"continue$">(safeIdentifier("continue")).toBe("continue$");
expect<"debugger$">(safeIdentifier("debugger")).toBe("debugger$");
expect<"default$">(safeIdentifier("default")).toBe("default$");
expect<"delete$">(safeIdentifier("delete")).toBe("delete$");
});

// prettier-ignore
test("does not modify other inputs which are not reserved identifiers", () => {
expect<"constructor">(safeIdentifier("constructor")).toBe("constructor");
expect<"toString">(safeIdentifier("toString")).toBe("toString");
expect<"toJSON">(safeIdentifier("toJSON")).toBe("toJSON");
expect<"valueOf">(safeIdentifier("valueOf")).toBe("valueOf");
});
});

describe("safeObjectProperty", () => {
const { safeObjectProperty } = codegenInfo;

// prettier-ignore
test("sanitizes reserved object property names", () => {
expect<"constructor$">(safeObjectProperty("constructor")).toBe("constructor$");
expect<"toString$">(safeObjectProperty("toString")).toBe("toString$");
expect<"toJSON$">(safeObjectProperty("toJSON")).toBe("toJSON$");
expect<"valueOf$">(safeObjectProperty("valueOf")).toBe("valueOf$");
});

// prettier-ignore
test("does not modify other inputs which are not reserved object properties", () => {
expect<"break">(safeObjectProperty("break")).toBe("break");
expect<"case">(safeObjectProperty("case")).toBe("case");
expect<"catch">(safeObjectProperty("catch")).toBe("catch");
expect<"class">(safeObjectProperty("class")).toBe("class");
expect<"const">(safeObjectProperty("const")).toBe("const");
expect<"continue">(safeObjectProperty("continue")).toBe("continue");
expect<"debugger">(safeObjectProperty("debugger")).toBe("debugger");
expect<"default">(safeObjectProperty("default")).toBe("default");
expect<"delete">(safeObjectProperty("delete")).toBe("delete");
});
});
6 changes: 5 additions & 1 deletion packages/protobuf/src/codegen-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// 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";
Expand All @@ -25,6 +25,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 =
Expand Down Expand Up @@ -61,6 +63,8 @@ export const codegenInfo: CodegenInfo = {
reifyWkt,
getUnwrappedFieldType,
scalarDefaultValue,
safeIdentifier,
safeObjectProperty,
// prettier-ignore
symbols: {
proto2: {typeOnly: false, privateImportPath: "./proto2.js", publicImportPath: packageName},
Expand Down
65 changes: 48 additions & 17 deletions packages/protobuf/src/private/names.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,32 +50,23 @@ 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;
return 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);
}
}
}
Expand All @@ -89,7 +80,7 @@ export function localFieldName(protoName: string, inOneof: boolean) {
// oneof member names are not properties, but values of the `case` property.
return name;
}
if (reservedObjectProperties[name] || reservedMessageProperties[name]) {
if (isReservedObjectProperty(name) || reservedMessageProperties[name]) {
name = name + "$";
dimitropoulos marked this conversation as resolved.
Show resolved Hide resolved
}
return name;
Expand Down Expand Up @@ -182,7 +173,7 @@ function protoCamelCase(snakeCase: string): string {

// Names that cannot be used for identifiers, such as class names,
// but _can_ be used for object properties.
const reservedIdent: { [k: string]: boolean } = {
const reservedIdent = {
// ECMAScript 2015 keywords
break: true,
case: true,
Expand Down Expand Up @@ -246,17 +237,17 @@ const reservedIdent: { [k: string]: boolean } = {
globalThis: true,
Uint8Array: true,
Partial: true,
};
} satisfies { [k: string]: boolean };
dimitropoulos marked this conversation as resolved.
Show resolved Hide resolved

// Names that cannot be used for object properties because they are reserved
// by built-in JavaScript properties.
const reservedObjectProperties: { [k: string]: boolean } = {
const reservedObjectProperties = {
// names reserved by JavaScript
constructor: true,
toString: true,
toJSON: true,
valueOf: true,
};
} satisfies { [k: string]: boolean };

// Names that cannot be used for object properties because they are reserved
// by the runtime.
Expand All @@ -275,3 +266,43 @@ const reservedMessageProperties: { [k: string]: boolean } = {
// names reserved by the runtime for the future
toObject: true,
};

type Fallback<T extends string> = `${T}$`;

const fallback = <T extends string>(word: T): Fallback<T> => `${word}$`;

type ReservedObjectProperty = keyof typeof reservedObjectProperties;

const isReservedObjectProperty = <T extends string>(
word: T | ReservedObjectProperty
): word is ReservedObjectProperty =>
reservedObjectProperties[word as ReservedObjectProperty];

export const safeObjectProperty = <T extends string>(
word: T
): T extends ReservedObjectProperty ? Fallback<T> : T => {
if (isReservedObjectProperty(word)) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-explicit-any -- required for narrowing
return fallback(word) as any;
dimitropoulos marked this conversation as resolved.
Show resolved Hide resolved
}
// eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-explicit-any -- required for narrowing
return word as any;
};

type ReservedIdentifier = keyof typeof reservedIdent;

const isReservedIdentifier = <T extends string>(
dimitropoulos marked this conversation as resolved.
Show resolved Hide resolved
word: T | ReservedIdentifier
): word is ReservedIdentifier =>
reservedIdent[word as ReservedIdentifier];

export const safeIdentifier = <T extends string>(
word: T
): T extends ReservedIdentifier ? Fallback<T> : T => {
if (isReservedIdentifier(word)) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-explicit-any -- required for narrowing
return fallback(word) as any;
}
// eslint-disable-next-line @typescript-eslint/no-unsafe-return, @typescript-eslint/no-explicit-any -- required for narrowing
return word as any;
};