Skip to content

Commit

Permalink
Add makeCheckFormattedRule to @ninjutsu-build/biome
Browse files Browse the repository at this point in the history
With the new idiom of static analysis rules being able to return
`validations` to add into the dependent edges, we write
`makeCheckFormattedRule` to do a non-destructive check that the files
are correctly formatted.

We will conditionally use this in our `configure.mjs` for CI as right
now if we submit a file that is incorrectly formatted the CI will just
fix it (in the temporary workspace) and continue without reporting the
issue.
  • Loading branch information
elliotgoodrich committed Mar 22, 2024
1 parent d7865ec commit 6881184
Show file tree
Hide file tree
Showing 4 changed files with 188 additions and 24 deletions.
4 changes: 2 additions & 2 deletions packages/biome/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/biome/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@ninjutsu-build/biome",
"version": "0.8.0",
"version": "0.8.1",
"description": "A biome plugin for ninjutsu-build",
"author": "Elliot Goodrich",
"scripts": {
Expand Down
87 changes: 66 additions & 21 deletions packages/biome/src/biome.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import test from "node:test";
import { strict as assert } from "node:assert";
import { makeLintRule, makeFormatRule, makeFormatToRule } from "./biome.js";
import {
makeCheckFormattedRule,
makeLintRule,
makeFormatRule,
makeFormatToRule,
} from "./biome.js";
import {
NinjaBuilder,
implicitDeps,
Expand Down Expand Up @@ -74,30 +79,70 @@ test("makeFormatToRule", () => {
assert.equal(out, "nice.js");
});

test("format then lint", () => {
test("makeCheckFormattedRule", () => {
const ninja = new NinjaBuilder();
const format = makeFormatRule(ninja);
const lint = makeLintRule(ninja);
const formatted = format({
in: "src/bar.js",
const checkFormatted = makeCheckFormattedRule(ninja);
const out: {
file: "ugly.js";
[validations]: `$builddir/.ninjutsu-build/biome/checkFormatted/ugly.js`;
} = checkFormatted({
in: "ugly.js",
configPath: "biome.json",
});
assert.deepEqual(formatted, {
file: "src/bar.js",
[orderOnlyDeps]: "$builddir/.ninjutsu-build/biome/format/src/bar.js",
assert.deepEqual(out, {
file: "ugly.js",
[validations]: "$builddir/.ninjutsu-build/biome/checkFormatted/ugly.js",
});
});

const linted = lint({
in: formatted,
configPath: "biome.json",
});
test("format then lint", () => {
const ninja = new NinjaBuilder();
const format = makeFormatRule(ninja);
const lint = makeLintRule(ninja);
const checkFormatted = makeCheckFormattedRule(ninja);

// Check anything that would take `linted` as an input would wait for
// formatting to finish (orderOnlyDeps) and guarantee that linting would
// be done (validations).
assert.deepEqual(linted, {
file: "src/bar.js",
[validations]: "$builddir/.ninjutsu-build/biome/lint/src/bar.js",
[orderOnlyDeps]: "$builddir/.ninjutsu-build/biome/format/src/bar.js",
});
{
const formatted = format({
in: "src/bar.js",
configPath: "biome.json",
});
assert.deepEqual(formatted, {
file: "src/bar.js",
[orderOnlyDeps]: "$builddir/.ninjutsu-build/biome/format/src/bar.js",
});

const linted = lint({
in: formatted,
configPath: "biome.json",
});

// Check anything that would take `linted` as an input would wait for
// formatting to finish (orderOnlyDeps) and guarantee that linting would
// be done (validations).
assert.deepEqual(linted, {
file: "src/bar.js",
[validations]: "$builddir/.ninjutsu-build/biome/lint/src/bar.js",
[orderOnlyDeps]: "$builddir/.ninjutsu-build/biome/format/src/bar.js",
});
}

{
const linted = lint({
in: "src/in.js",
configPath: "biome.json",
});

const formatChecked = checkFormatted({
in: linted,
configPath: "biome.json",
});

// We don't need to pass through the validations from `linted` because
// whenever the formatting validation is triggered it will trigger
// the linting one.
assert.deepEqual(formatChecked, {
file: "src/in.js",
[validations]: "$builddir/.ninjutsu-build/biome/checkFormatted/src/in.js",
});
}
});
119 changes: 119 additions & 0 deletions packages/biome/src/biome.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,125 @@ export function makeFormatToRule(
};
}

/**
* Create a rule in the specified `ninja` builder with the specified `name` that will
* run `biome format` on the input file and write the results to a unspecified file, whose
* path will be returned by the function along with a validation step on the unspecified
* file containing the results. This causes all build edges that depend on this input to
* add a validation step on checking whether the input file is correctly formatted.
*
* This is useful when build a ninja file for CI as you may not want to fix formatting
* issues with {@link makeFormatRule} and only alert when a file is not formatted.
*
* The returned function takes a `configPath` property, which is the path to the
* [`biome.json` configuration file](https://biome.dev/reference/configuration/). An optional
* `args` property exists to pass in any additional options to the CLI.
*
* For example the following will either format or check test files are formatted
* and run the tests. The `--ci` flag passed in will cause the generated ninja file to
* check the formatting in parallel to running the tests, but will not overwrite the
* source files. Whereas if no flag is passed in will will reformat the source files
* in parallel and then run the corresponding test once that has finished.
*
* ```ts
* import { NinjaBuilder } from "@ninjutsu-build/core";
* import { makeFormatRule, makeCheckFormattedRule } from "@ninjutsu-build/biome";
* import { globSync } from "glob";
*
* const ninja = new NinjaBuilder();
*
* const nonDestructive = process.argv.includes("--ci");
* const format = nonDestructive
* ? makeCheckFormattedRule(ninja)
* : makeFormatRule(ninja);
*
* for (const js of globSync("src/*.test.js", { posix: true })) {
* const formatted = format({
* in: js,
* configPath: "biome.json",
* args: "--no-errors-on-unmatched",
* });
* test({
* in: formatted,
* out: getInput(formatted) + ".txt",
* })
* }
*
* writeFileSync("build.ninja", ninja.output);
* ```
*
* Linting and other static analysis rules are commonly done as a
* [ninja validation step](https://ninja-build.org/manual.html#validations) to improve
* parallelism and avoid increasing the critical path.
*/
export function makeCheckFormattedRule(
ninja: NinjaBuilder,
name = "checkFormatted",
): <I extends string>(args: {
in: Input<I>;
configPath: string;
args?: string;
[implicitDeps]?: string | readonly string[];
[orderOnlyDeps]?: string | readonly string[];
[implicitOut]?: string | readonly string[];
[validations]?: (out: string) => string | readonly string[];
}) => {
file: I;
[validations]: `$builddir/.ninjutsu-build/biome/checkFormatted/${I}`;
[orderOnlyDeps]?: string | readonly string[];
} {
const checkFormatted = ninja.rule(name, {
command:
prefix +
join("node_modules", biomeCommand) +
" format $args --config-path $configPath $in > $out",
description: "Checking format of $in",
in: needs<Input<string>>(),
out: needs<string>(),
configPath: needs<string>(),
args: "",
});
return <I extends string>(a: {
in: Input<I>;
configPath: string;
args?: string;
[implicitDeps]?: string | readonly string[];
[orderOnlyDeps]?: string | readonly string[];
[implicitOut]?: string | readonly string[];
[validations]?: (out: string) => string | readonly string[];
}): {
file: I;
[validations]: `$builddir/.ninjutsu-build/biome/checkFormatted/${I}`;
[orderOnlyDeps]?: string | readonly string[];
} => {
const { configPath, [implicitDeps]: _implicitDeps = [], ...rest } = a;

const file = getInput(a.in);
const validationFile =
`$builddir/.ninjutsu-build/biome/checkFormatted/${file}` as const;
checkFormatted({
out: validationFile,
configPath: dirname(configPath),
...rest,
[implicitDeps]: _implicitDeps.concat(a.configPath),
});

// If there is a build-order dependency then we must return this to
// anyone depending on our output since we are forwarding it from our
// input and just injecting a validation step
const forwardDeps =
typeof a.in === "object" && orderOnlyDeps in a.in
? { [orderOnlyDeps]: a.in[orderOnlyDeps] }
: {};

return {
file,
[validations]: validationFile,
...forwardDeps,
};
};
}

/**
* Create a rule in the specified `ninja` builder with the specified `name` that will
* run `biome lint` on the input file and write the results to a unspecified file, whose
Expand Down

0 comments on commit 6881184

Please sign in to comment.