Skip to content

Commit

Permalink
tsp format should return non-zero exit code on failures (#2451)
Browse files Browse the repository at this point in the history
This PR fixes #2323 by returning a non-zero exit code upon any failure
when `tsp format` is executed at the command line. I'm also changing the
return type of `formatTypeSpecFiles` to return a list of file paths that
failed, not sure whether that explicitly qualifies as a breaking change.
If needed, I can create a new function to expose and wrap it with the
original function without changing its signature.
  • Loading branch information
daviwil authored Sep 25, 2023
1 parent 42aeb99 commit 0f2f13b
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@typespec/compiler",
"comment": "`tsp format` now returns a non-zero exit code when it fails to format a file",
"type": "none"
}
],
"packageName": "@typespec/compiler"
}
7 changes: 6 additions & 1 deletion packages/compiler/src/core/cli/actions/format.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { findUnformattedTypeSpecFiles, formatTypeSpecFiles } from "../../formatter-fs.js";
import { logDiagnostics } from "../../index.js";
import { CliCompilerHost } from "../types.js";

export interface FormatArgs {
Expand All @@ -23,9 +24,13 @@ export async function formatAction(host: CliCompilerHost, args: FormatArgs) {
process.exit(1);
}
} else {
await formatTypeSpecFiles(args["include"], {
const [_, diagnostics] = await formatTypeSpecFiles(args["include"], {
exclude: args["exclude"],
debug: args.debug,
});
if (diagnostics.length > 0) {
logDiagnostics(diagnostics, host.logSink);
process.exit(1);
}
}
}
26 changes: 23 additions & 3 deletions packages/compiler/src/core/formatter-fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,35 +3,55 @@ import { globby } from "globby";
import { resolveConfig } from "prettier";
import { PrettierParserError } from "../formatter/parser.js";
import { checkFormatTypeSpec, formatTypeSpec } from "./formatter.js";
import { Diagnostic, NoTarget } from "./index.js";
import { createDiagnostic } from "./messages.js";
import { normalizePath } from "./path-utils.js";

export interface TypeSpecFormatOptions {
exclude?: string[];
debug?: boolean;
}

export interface TypeSpecFormatResult {
/**
* The list of files which were formatted successfully, the paths of which are either relative or absolute based on the original file path patterns.
*/
formattedFiles: string[];
}

/**
* Format all the TypeSpec files.
* @param patterns List of wildcard pattern searching for TypeSpec files.
* @returns list of files which failed to format.
*/
export async function formatTypeSpecFiles(
patterns: string[],
{ exclude, debug }: TypeSpecFormatOptions
) {
): Promise<[TypeSpecFormatResult, readonly Diagnostic[]]> {
const files = await findFiles(patterns, exclude);
const diagnostics: Diagnostic[] = [];
const formattedFiles: string[] = [];
for (const file of files) {
try {
await formatTypeSpecFile(file);
formattedFiles.push(file);
} catch (e) {
if (e instanceof PrettierParserError) {
const details = debug ? e.message : "";
// eslint-disable-next-line no-console
console.error(`File '${file}' failed to format. ${details}`);
diagnostics.push(
createDiagnostic({
code: "format-failed",
format: { file, details },
target: NoTarget,
})
);
} else {
throw e;
}
}
}

return [{ formattedFiles }, diagnostics];
}

/**
Expand Down
10 changes: 10 additions & 0 deletions packages/compiler/src/core/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,16 @@ const diagnostics = {
},
},

/**
* Formatter
*/
"format-failed": {
severity: "error",
messages: {
default: paramMessage`File '${"file"}' failed to format. ${"details"}`,
},
},

/**
* Decorator
*/
Expand Down

0 comments on commit 0f2f13b

Please sign in to comment.