From f0d0565c100fcebd76ccbf894c49b44d5e002398 Mon Sep 17 00:00:00 2001 From: Brian Terlson Date: Wed, 19 Apr 2023 17:41:05 -0700 Subject: [PATCH 1/3] Fix a few compiler bugs --- packages/compiler/core/path-utils.ts | 167 ++++++++++++++++++ .../builders/object-builder.ts | 15 +- .../emitter-framework/type-emitter.ts | 1 - .../test/emitter-framework/emitter.test.ts | 19 ++ .../emitter-framework/typescript-emitter.ts | 1 + 5 files changed, 199 insertions(+), 4 deletions(-) diff --git a/packages/compiler/core/path-utils.ts b/packages/compiler/core/path-utils.ts index fd0d6db7b0e..d03b127571a 100644 --- a/packages/compiler/core/path-utils.ts +++ b/packages/compiler/core/path-utils.ts @@ -470,7 +470,174 @@ export function normalizeSlashes(path: string): string { } //#endregion +// #region relative paths +type GetCanonicalFileName = (fileName: string) => string; +/** @internal */ +function equateValues(a: T, b: T) { + return a === b; +} + +/** + * Compare the equality of two strings using a case-sensitive ordinal comparison. + * + * Case-sensitive comparisons compare both strings one code-point at a time using the integer + * value of each code-point after applying `toUpperCase` to each string. We always map both + * strings to their upper-case form as some unicode characters do not properly round-trip to + * lowercase (such as `ẞ` (German sharp capital s)). + * + * @internal + */ +function equateStringsCaseInsensitive(a: string, b: string) { + return a === b || (a !== undefined && b !== undefined && a.toUpperCase() === b.toUpperCase()); +} + +/** + * Compare the equality of two strings using a case-sensitive ordinal comparison. + * + * Case-sensitive comparisons compare both strings one code-point at a time using the + * integer value of each code-point. + * + * @internal + */ +function equateStringsCaseSensitive(a: string, b: string) { + return equateValues(a, b); +} + +/** + * Returns its argument. + * + * @internal + */ +function identity(x: T) { + return x; +} + +/** + * Determines whether a path is an absolute disk path (e.g. starts with `/`, or a dos path + * like `c:`, `c:\` or `c:/`). + * + * @internal + */ +function isRootedDiskPath(path: string) { + return getEncodedRootLength(path) > 0; +} + +/** + * Determines whether a path starts with an absolute path component (i.e. `/`, `c:/`, `file://`, etc.). + * + * ```ts + * // POSIX + * pathIsAbsolute("/path/to/file.ext") === true + * // DOS + * pathIsAbsolute("c:/path/to/file.ext") === true + * // URL + * pathIsAbsolute("file:///path/to/file.ext") === true + * // Non-absolute + * pathIsAbsolute("path/to/file.ext") === false + * pathIsAbsolute("./path/to/file.ext") === false + * ``` + * + * @internal + */ +function pathIsAbsolute(path: string): boolean { + return getEncodedRootLength(path) !== 0; +} + +/** + * Determines whether a path starts with a relative path component (i.e. `.` or `..`). + * + * @internal + */ +function pathIsRelative(path: string): boolean { + return /^\.\.?($|[\\/])/.test(path); +} + +/** + * Ensures a path is either absolute (prefixed with `/` or `c:`) or dot-relative (prefixed + * with `./` or `../`) so as not to be confused with an unprefixed module name. + * + * ```ts + * ensurePathIsNonModuleName("/path/to/file.ext") === "/path/to/file.ext" + * ensurePathIsNonModuleName("./path/to/file.ext") === "./path/to/file.ext" + * ensurePathIsNonModuleName("../path/to/file.ext") === "../path/to/file.ext" + * ensurePathIsNonModuleName("path/to/file.ext") === "./path/to/file.ext" + * ``` + * + * @internal + */ +function ensurePathIsNonModuleName(path: string): string { + return !pathIsAbsolute(path) && !pathIsRelative(path) ? "./" + path : path; +} + +/** @internal */ +function getPathComponentsRelativeTo( + from: string, + to: string, + stringEqualityComparer: (a: string, b: string) => boolean, + getCanonicalFileName: GetCanonicalFileName +) { + const fromComponents = reducePathComponents(getPathComponents(from)); + const toComponents = reducePathComponents(getPathComponents(to)); + + let start: number; + for (start = 0; start < fromComponents.length && start < toComponents.length; start++) { + const fromComponent = getCanonicalFileName(fromComponents[start]); + const toComponent = getCanonicalFileName(toComponents[start]); + const comparer = start === 0 ? equateStringsCaseInsensitive : stringEqualityComparer; + if (!comparer(fromComponent, toComponent)) break; + } + + if (start === 0) { + return toComponents; + } + + const components = toComponents.slice(start); + const relative: string[] = []; + for (; start < fromComponents.length; start++) { + relative.push(".."); + } + return ["", ...relative, ...components]; +} + +/** + * Gets a relative path that can be used to traverse between `from` and `to`. + */ +export function getRelativePathFromDirectory(from: string, to: string, ignoreCase: boolean): string; +/** + * Gets a relative path that can be used to traverse between `from` and `to`. + */ +export function getRelativePathFromDirectory( + fromDirectory: string, + to: string, + getCanonicalFileName: GetCanonicalFileName +): string; // eslint-disable-line @typescript-eslint/unified-signatures +export function getRelativePathFromDirectory( + fromDirectory: string, + to: string, + getCanonicalFileNameOrIgnoreCase: GetCanonicalFileName | boolean +) { + if (getRootLength(fromDirectory) > 0 !== getRootLength(to) > 0) { + throw new Error("Paths must either both be absolute or both be relative"); + } + const getCanonicalFileName = + typeof getCanonicalFileNameOrIgnoreCase === "function" + ? getCanonicalFileNameOrIgnoreCase + : identity; + const ignoreCase = + typeof getCanonicalFileNameOrIgnoreCase === "boolean" + ? getCanonicalFileNameOrIgnoreCase + : false; + const pathComponents = getPathComponentsRelativeTo( + fromDirectory, + to, + ignoreCase ? equateStringsCaseInsensitive : equateStringsCaseSensitive, + getCanonicalFileName + ); + return getPathFromPathComponents(pathComponents); +} + +// #endregion const enum CharacterCodes { nullCharacter = 0, maxAsciiCharacter = 0x7f, diff --git a/packages/compiler/emitter-framework/builders/object-builder.ts b/packages/compiler/emitter-framework/builders/object-builder.ts index bc3ce70bec2..e83f4e00d5f 100644 --- a/packages/compiler/emitter-framework/builders/object-builder.ts +++ b/packages/compiler/emitter-framework/builders/object-builder.ts @@ -6,11 +6,20 @@ import { EmitEntity, EmitterResult } from "../types.js"; // eslint-disable-next-line @typescript-eslint/no-unused-vars export interface ObjectBuilder extends Record {} export class ObjectBuilder { - constructor(initializer: Record = {}) { - for (const [key, value] of Object.entries(initializer)) { - this.set(key, value as any); + constructor(initializer: Record | Placeholder> = {}) { + if (initializer instanceof Placeholder) { + initializer.onValue((v) => { + for (const [key, value] of Object.entries(v)) { + this.set(key, value as any); + } + }); + } else { + for (const [key, value] of Object.entries(initializer)) { + this.set(key, value as any); + } } } + set(key: string, v: EmitEntity | Placeholder | T) { let value = v; if (v instanceof EmitterResult) { diff --git a/packages/compiler/emitter-framework/type-emitter.ts b/packages/compiler/emitter-framework/type-emitter.ts index 0d39fd8d781..a6978231542 100644 --- a/packages/compiler/emitter-framework/type-emitter.ts +++ b/packages/compiler/emitter-framework/type-emitter.ts @@ -387,7 +387,6 @@ export class TypeEmitter> { } arrayDeclarationReferenceContext(array: Model): Context { - this.emitter.emitType(array.indexer!.value); return {}; } diff --git a/packages/compiler/test/emitter-framework/emitter.test.ts b/packages/compiler/test/emitter-framework/emitter.test.ts index 48d3d507344..7a7d2cf4e38 100644 --- a/packages/compiler/test/emitter-framework/emitter.test.ts +++ b/packages/compiler/test/emitter-framework/emitter.test.ts @@ -139,6 +139,16 @@ describe("emitter-framework: typescript emitter", () => { assert.match(contents, /z: 12/); }); + it("emits unknown", async () => { + const contents = await emitTypeSpecToTs(` + model A { + x: unknown + } + `); + + assert.match(contents, /x: unknown/); + }); + it("emits array literals", async () => { const contents = await emitTypeSpecToTs(` model MyArray2 is Array; @@ -155,6 +165,15 @@ describe("emitter-framework: typescript emitter", () => { assert.match(contents, /y: string\[\]/); assert.match(contents, /z: \(string \| number\)\[\]/); }); + + it("emits arrays of unknown", async () => { + const contents = await emitTypeSpecToTs(` + model MyArray2 is Array; + `); + + assert.match(contents, /MyArray2 extends Array/); + }); + // todo: what to do with optionals not at the end?? it("emits operations", async () => { const contents = await emitTypeSpecToTs(` diff --git a/packages/compiler/test/emitter-framework/typescript-emitter.ts b/packages/compiler/test/emitter-framework/typescript-emitter.ts index 5c274c0a365..f8cf826b704 100644 --- a/packages/compiler/test/emitter-framework/typescript-emitter.ts +++ b/packages/compiler/test/emitter-framework/typescript-emitter.ts @@ -35,6 +35,7 @@ export function isArrayType(m: Model) { } export const intrinsicNameToTSType = new Map([ + ["unknown", "unknown"], ["string", "string"], ["int32", "number"], ["int16", "number"], From ce5097fb28abe274ab7730194fea4828e9f1a5f7 Mon Sep 17 00:00:00 2001 From: Brian Terlson Date: Fri, 21 Apr 2023 10:55:50 -0700 Subject: [PATCH 2/3] changes --- .../@typespec/compiler/few-fixes_2023-04-21-17-55.json | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 common/changes/@typespec/compiler/few-fixes_2023-04-21-17-55.json diff --git a/common/changes/@typespec/compiler/few-fixes_2023-04-21-17-55.json b/common/changes/@typespec/compiler/few-fixes_2023-04-21-17-55.json new file mode 100644 index 00000000000..9c5917b3181 --- /dev/null +++ b/common/changes/@typespec/compiler/few-fixes_2023-04-21-17-55.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@typespec/compiler", + "comment": "Add relative file path utils, and allow emitter framework's ObjectBuilder to be initialized with a placeholder object.", + "type": "none" + } + ], + "packageName": "@typespec/compiler" +} \ No newline at end of file From 6c205c88e5f6000cfba4d3c2dadbc44996b47f3f Mon Sep 17 00:00:00 2001 From: Brian Terlson Date: Fri, 21 Apr 2023 14:12:29 -0700 Subject: [PATCH 3/3] spelling, export a useful thing, delete a non-useful thing --- cspell.yaml | 1 + packages/compiler/core/path-utils.ts | 12 +----------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/cspell.yaml b/cspell.yaml index 92fce3962e8..53f4c6b32b4 100644 --- a/cspell.yaml +++ b/cspell.yaml @@ -66,6 +66,7 @@ words: - xplat - keyer - interner + - unprefixed ignorePaths: - "**/node_modules/**" - "**/dist/**" diff --git a/packages/compiler/core/path-utils.ts b/packages/compiler/core/path-utils.ts index d03b127571a..b042ba5d7b2 100644 --- a/packages/compiler/core/path-utils.ts +++ b/packages/compiler/core/path-utils.ts @@ -513,16 +513,6 @@ function identity(x: T) { return x; } -/** - * Determines whether a path is an absolute disk path (e.g. starts with `/`, or a dos path - * like `c:`, `c:\` or `c:/`). - * - * @internal - */ -function isRootedDiskPath(path: string) { - return getEncodedRootLength(path) > 0; -} - /** * Determines whether a path starts with an absolute path component (i.e. `/`, `c:/`, `file://`, etc.). * @@ -566,7 +556,7 @@ function pathIsRelative(path: string): boolean { * * @internal */ -function ensurePathIsNonModuleName(path: string): string { +export function ensurePathIsNonModuleName(path: string): string { return !pathIsAbsolute(path) && !pathIsRelative(path) ? "./" + path : path; }