Skip to content

Commit

Permalink
Warn when using windows style path separator (microsoft#4023)
Browse files Browse the repository at this point in the history
fix [microsoft#1261](microsoft#1261)

Using windows style separator can cause issue when trying to run a spec
on a non windows platform on top of being a pain to work with(escaping)

This PR adds validation to config element that are already validated as
`absolute-path` to also check they unix a unix style of path
  • Loading branch information
timotheeguerin authored Aug 5, 2024
1 parent 73120f3 commit d15af24
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 8 deletions.
7 changes: 7 additions & 0 deletions .chronus/changes/path-unix-style-2024-6-25-15-13-16.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: feature
packages:
- "@typespec/compiler"
---

Warn when using `\` in config file field that expect a path.
7 changes: 7 additions & 0 deletions packages/compiler/src/config/config-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,13 @@ function validatePathAbsolute(
target: target === NoTarget ? target : getLocationInYamlScript(target.file, target.path),
});
}
if (path.includes("\\")) {
return createDiagnostic({
code: "path-unix-style",
format: { path },
target: target === NoTarget ? target : getLocationInYamlScript(target.file, target.path),
});
}

return undefined;
}
6 changes: 6 additions & 0 deletions packages/compiler/src/core/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,12 @@ const diagnostics = {
default: paramMessage`Path "${"path"}" cannot be relative. Use {cwd} or {project-root} to specify what the path should be relative to.`,
},
},
"path-unix-style": {
severity: "warning",
messages: {
default: paramMessage`Path should use unix style separators. Use "/" instead of "\\".`,
},
},
"config-path-not-found": {
severity: "error",
messages: {
Expand Down
34 changes: 26 additions & 8 deletions packages/compiler/src/core/schema-validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,16 @@ export interface JSONSchemaValidatorOptions {
strict?: boolean;
}

function absolutePathStatus(path: string): "valid" | "not-absolute" | "windows-style" {
if (path.startsWith(".") || !isPathAbsolute(path)) {
return "not-absolute";
}
if (path.includes("\\")) {
return "windows-style";
}
return "valid";
}

export function createJSONSchemaValidator<T>(
schema: JSONSchemaType<T>,
options: JSONSchemaValidatorOptions = { strict: true }
Expand All @@ -30,9 +40,7 @@ export function createJSONSchemaValidator<T>(

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

Expand Down Expand Up @@ -66,11 +74,21 @@ function ajvErrorToDiagnostic(
): 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 value = getErrorValue(obj, error) as any;
const status = absolutePathStatus(value);
if (status === "windows-style") {
return createDiagnostic({
code: "path-unix-style",
format: { path: value },
target: tspTarget,
});
} else {
return createDiagnostic({
code: "config-path-absolute",
format: { path: value },
target: tspTarget,
});
}
}

const messageLines = [`Schema violation: ${error.message} (${error.instancePath || "/"})`];
Expand Down
10 changes: 10 additions & 0 deletions packages/compiler/test/core/emitter-options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,5 +117,15 @@ describe("compiler: emitter options", () => {
message: `Path "assets" cannot be relative. Use {cwd} or {project-root} to specify what the path should be relative to.`,
});
});

it("emit diagnostic if passing windows style path", async () => {
const diagnostics = await diagnoseEmitterOptions({
"asset-dir": "C:\\abc\\def",
});
expectDiagnostics(diagnostics, {
code: "path-unix-style",
message: `Path should use unix style separators. Use "/" instead of "\\".`,
});
});
});
});

0 comments on commit d15af24

Please sign in to comment.