Skip to content

Commit

Permalink
Feature: Add format: absolute-path to options validator (#2708)
Browse files Browse the repository at this point in the history
fix #2493 

Add a new known format for the json schema validation for emitter
options.

---------

Co-authored-by: Mark Cowlishaw <[email protected]>
  • Loading branch information
timotheeguerin and markcowl authored Dec 5, 2023
1 parent f646e19 commit 0b01d95
Show file tree
Hide file tree
Showing 11 changed files with 261 additions and 29 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@typespec/compiler",
"comment": "Emitter API: Added `absolute-path` as a known format for emitter options which will validate the value passed by the user resolve to an absolute path.",
"type": "none"
}
],
"packageName": "@typespec/compiler"
}
18 changes: 18 additions & 0 deletions docs/extending-typespec/emitters-basics.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,24 @@ export async function $onEmit(context: EmitContext<EmitterOptions>) {
}
```

### Emitter options known format:

### `absolute-path`

Specify that the value for this option should resolve to an absolute path. e.g. `"{project-root}/dir"`.

:::important
It is recommended that all options that involve path use this. Using relative path can be confusing for users on as it is not clear what the relative path is relative to. And more importantly relative path if not careful are resolved relative to the `cwd` in node file system which result in spec only compiling from the the project root.
:::

Example:

```js
{
"asset-dir": { type: "string", format: "absolute-path", nullable: true },
}
```

### Configuration options convention

- Name options `kebab-case`. So it can be inline with the rest of the cli
Expand Down
1 change: 1 addition & 0 deletions packages/compiler/src/config/config-to-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export function resolveOptionsFromConfig(config: TypeSpecConfig, options: Config
const resolvedOptions: CompilerOptions = omitUndefined({
outputDir: expandedConfig.outputDir,
config: config.filename,
configFile: config,
additionalImports: expandedConfig["imports"],
warningAsError: expandedConfig.warnAsError,
trace: expandedConfig.trace,
Expand Down
7 changes: 5 additions & 2 deletions packages/compiler/src/core/options.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { EmitterOptions } from "../config/types.js";
import { EmitterOptions, TypeSpecConfig } from "../config/types.js";
import { LinterRuleSet, ParseOptions } from "./types.js";

export interface CompilerOptions {
Expand All @@ -11,7 +11,7 @@ export interface CompilerOptions {
outputDir?: string;

/**
* Path to config YAML file or folder in which to search for default tspconfig.yaml file.
* Path to config YAML file used, this is also where the project root should be.
*/
config?: string;

Expand Down Expand Up @@ -63,4 +63,7 @@ export interface CompilerOptions {

/** Ruleset to enable for linting. */
linterRuleSet?: LinterRuleSet;

/** @internal */
readonly configFile?: TypeSpecConfig;
}
8 changes: 7 additions & 1 deletion packages/compiler/src/core/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,13 @@ export async function compile(
if (libDefinition?.emitter?.options) {
const diagnostics = libDefinition?.emitterOptionValidator?.validate(
emitterOptions,
NoTarget
options.configFile?.file
? {
kind: "path-target",
path: ["options", emitterNameOrPath],
script: options.configFile.file,
}
: NoTarget
);
if (diagnostics && diagnostics.length > 0) {
program.reportDiagnostics(diagnostics);
Expand Down
83 changes: 68 additions & 15 deletions packages/compiler/src/core/schema-validator.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
import Ajv, { DefinedError, Options } from "ajv";
import Ajv, { ErrorObject, Options } from "ajv";
import { getLocationInYamlScript } from "../yaml/diagnostics.js";
import { YamlScript } from "../yaml/types.js";
import { YamlPathTarget, YamlScript } from "../yaml/types.js";
import { compilerAssert } from "./diagnostics.js";
import { Diagnostic, JSONSchemaType, JSONSchemaValidator, NoTarget, SourceFile } from "./types.js";
import { createDiagnostic } from "./messages.js";
import { isPathAbsolute } from "./path-utils.js";
import {
Diagnostic,
DiagnosticTarget,
JSONSchemaType,
JSONSchemaValidator,
NoTarget,
SourceFile,
} from "./types.js";

export interface JSONSchemaValidatorOptions {
coerceTypes?: boolean;
Expand All @@ -13,17 +22,23 @@ export function createJSONSchemaValidator<T>(
schema: JSONSchemaType<T>,
options: JSONSchemaValidatorOptions = { strict: true }
): JSONSchemaValidator {
const ajv = new (Ajv as any)({
const ajv: import("ajv").default = new (Ajv as any)({
strict: options.strict,
coerceTypes: options.coerceTypes,
allErrors: true,
} satisfies Options);

ajv.addFormat("absolute-path", {
type: "string",
validate: (path) => {
return !path.startsWith(".") && isPathAbsolute(path);
},
});
return { validate };

function validate(
config: unknown,
target: YamlScript | SourceFile | typeof NoTarget
target: YamlScript | YamlPathTarget | SourceFile | typeof NoTarget
): Diagnostic[] {
const validate = ajv.compile(schema);
const valid = validate(config);
Expand All @@ -34,7 +49,7 @@ export function createJSONSchemaValidator<T>(

const diagnostics = [];
for (const error of validate.errors ?? []) {
const diagnostic = ajvErrorToDiagnostic(error, target);
const diagnostic = ajvErrorToDiagnostic(config, error, target);
diagnostics.push(diagnostic);
}

Expand All @@ -45,9 +60,19 @@ export function createJSONSchemaValidator<T>(
const IGNORED_AJV_PARAMS = new Set(["type", "errors"]);

function ajvErrorToDiagnostic(
error: DefinedError,
target: YamlScript | SourceFile | typeof NoTarget
obj: unknown,
error: ErrorObject<string, Record<string, any>, unknown>,
target: YamlScript | YamlPathTarget | SourceFile | typeof NoTarget
): Diagnostic {
const tspTarget = resolveTarget(error, target);
if (error.params.format === "absolute-path") {
return createDiagnostic({
code: "config-path-absolute",
format: { path: getErrorValue(obj, error) as any },
target: tspTarget,
});
}

const messageLines = [`Schema violation: ${error.message} (${error.instancePath || "/"})`];
for (const [name, value] of Object.entries(error.params).filter(
([name]) => !IGNORED_AJV_PARAMS.has(name)
Expand All @@ -61,16 +86,33 @@ function ajvErrorToDiagnostic(
code: "invalid-schema",
message,
severity: "error",
target:
target === NoTarget
? target
: "kind" in target
? getLocationInYamlScript(target, getErrorPath(error), "key")
: { file: target, pos: 0, end: 0 },
target: tspTarget,
};
}

function getErrorPath(error: DefinedError): string[] {
function resolveTarget(
error: ErrorObject<string, Record<string, any>, unknown>,
target: YamlScript | YamlPathTarget | SourceFile | typeof NoTarget
): DiagnosticTarget | typeof NoTarget {
if (target === NoTarget) {
return NoTarget;
}
if (!("kind" in target)) {
return { file: target, pos: 0, end: 0 };
}
switch (target.kind) {
case "yaml-script":
return getLocationInYamlScript(target, getErrorPath(error), "key");
case "path-target":
return getLocationInYamlScript(
target.script,
[...target.path, ...getErrorPath(error)],
"key"
);
}
}

function getErrorPath(error: ErrorObject<string, Record<string, any>, unknown>): string[] {
const instancePath = parseJsonPointer(error.instancePath);
switch (error.keyword) {
case "additionalProperties":
Expand All @@ -79,6 +121,17 @@ function getErrorPath(error: DefinedError): string[] {
return instancePath;
}
}
function getErrorValue(
obj: any,
error: ErrorObject<string, Record<string, any>, unknown>
): unknown {
const path = getErrorPath(error);
let current = obj;
for (const segment of path) {
current = current[segment];
}
return current;
}

/**
* Converts a json pointer into a array of reference tokens
Expand Down
11 changes: 9 additions & 2 deletions packages/compiler/src/core/types.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { JSONSchemaType as AjvJSONSchemaType } from "ajv";
import { TypeEmitter } from "../emitter-framework/type-emitter.js";
import { AssetEmitter } from "../emitter-framework/types.js";
import { YamlScript } from "../yaml/types.js";
import { YamlPathTarget, YamlScript } from "../yaml/types.js";
import { ModuleResolutionResult } from "./module-resolver.js";
import { Program } from "./program.js";
import type { TokenFlags } from "./scanner.js";
Expand Down Expand Up @@ -2037,6 +2037,9 @@ export type TypeOfDiagnostics<T extends DiagnosticMap<any>> = T extends Diagnost

export type JSONSchemaType<T> = AjvJSONSchemaType<T>;

/**
* @internal
*/
export interface JSONSchemaValidator {
/**
* Validate the configuration against its JSON Schema.
Expand All @@ -2045,7 +2048,10 @@ export interface JSONSchemaValidator {
* @param target Source file target to use for diagnostics.
* @returns Diagnostics produced by schema validation of the configuration.
*/
validate(config: unknown, target: YamlScript | SourceFile | typeof NoTarget): Diagnostic[];
validate(
config: unknown,
target: YamlScript | YamlPathTarget | SourceFile | typeof NoTarget
): Diagnostic[];
}

/** @deprecated Use TypeSpecLibraryDef */
Expand Down Expand Up @@ -2166,6 +2172,7 @@ export interface TypeSpecLibrary<
> extends TypeSpecLibraryDef<T, E> {
/**
* JSON Schema validator for emitter options
* @internal
*/
readonly emitterOptionValidator?: JSONSchemaValidator;

Expand Down
8 changes: 8 additions & 0 deletions packages/compiler/src/yaml/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,12 @@ export interface YamlScript {
readonly doc: Document.Parsed;
}

/**
* Represent the location of a value in a yaml script.
*/
export interface YamlPathTarget {
kind: "path-target";
script: YamlScript;
path: string[];
}
export type YamlDiagnosticTargetType = "value" | "key";
10 changes: 6 additions & 4 deletions packages/compiler/test/cli.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { deepStrictEqual, strictEqual } from "assert";
import { deepStrictEqual, ok, strictEqual } from "assert";
import { stringify } from "yaml";
import { TypeSpecRawConfig } from "../src/config/types.js";
import { CompileCliArgs, getCompilerOptions } from "../src/core/cli/actions/compile/args.js";
Expand Down Expand Up @@ -31,7 +31,9 @@ describe("compiler: cli", () => {
env
);
expectDiagnosticEmpty(diagnostics);
return options;
ok(options, "Options should have been set.");
const { configFile: config, ...rest } = options;
return rest;
}

it("no args and config: return empty options with output-dir at {cwd}/tsp-output", async () => {
Expand Down Expand Up @@ -188,15 +190,15 @@ describe("compiler: cli", () => {

interface TestUnifiedOptions<
K extends keyof CompileCliArgs & keyof TypeSpecRawConfig,
T extends keyof CompilerOptions,
T extends Exclude<keyof CompilerOptions, "configFile">,
> {
default: CompileCliArgs[K];
set: { in: CompileCliArgs[K]; alt: CompileCliArgs[K]; expected: CompilerOptions[T] }[];
}

function testUnifiedOptions<
K extends keyof CompileCliArgs & keyof TypeSpecRawConfig,
T extends keyof CompilerOptions,
T extends Exclude<keyof CompilerOptions, "configFile">,
>(name: K, resolvedName: T, data: TestUnifiedOptions<K, T>) {
describe(name, () => {
it("default", async () => {
Expand Down
14 changes: 9 additions & 5 deletions packages/compiler/test/config/resolve-compiler-option.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,15 @@ describe("compiler: resolve compiler options", () => {
describe("specifying explicit config file", () => {
const resolveOptions = async (path: string) => {
const fullPath = resolvePath(scenarioRoot, path);
return await resolveCompilerOptions(NodeHost, {
cwd: normalizePath(process.cwd()),
entrypoint: fullPath, // not really used here
configPath: fullPath,
});
const [{ configFile: config, ...options }, diagnostics] = await resolveCompilerOptions(
NodeHost,
{
cwd: normalizePath(process.cwd()),
entrypoint: fullPath, // not really used here
configPath: fullPath,
}
);
return [options, diagnostics] as const;
};

it("loads config at the given path", async () => {
Expand Down
Loading

0 comments on commit 0b01d95

Please sign in to comment.