Skip to content

chore: add exported-function-args-maximum Deno Style Guide linter plugin #6561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
66 changes: 66 additions & 0 deletions _tools/lint_plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
*/

import { toCamelCase, toPascalCase } from "@std/text";
import { toFileUrl } from "@std/path/to-file-url";
import { resolve } from "@std/path/resolve";

const PASCAL_CASE_REGEXP = /^_?(?:[A-Z][a-z0-9]*)*_?$/;
const UPPER_CASE_ONLY = /^_?[A-Z]{2,}$/;
Expand Down Expand Up @@ -310,5 +312,69 @@
};
},
},
// https://docs.deno.com/runtime/contributing/style_guide/#exported-functions%3A-max-2-args%2C-put-the-rest-into-an-options-object
"exported-function-args-maximum": {
create(context) {
const url = toFileUrl(resolve(context.filename));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might import.meta.resolve() work here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

context.filename is main.ts in test cases, and that throws with import.meta.resolve (main.ts is not a valid specifier, but a valid file path)


// assertions and testing utils generally don't follow this rule

Check warning on line 320 in _tools/lint_plugin.ts

View check run for this annotation

Codecov / codecov/patch

_tools/lint_plugin.ts#L320

Added line #L320 was not covered by tests
if (url.href.includes("/assert/")) return {};
if (url.href.includes("/testing/mock.ts")) return {};
if (url.href.includes("/testing/unstable_stub.ts")) return {};
if (url.href.includes("/testing/snapshot.ts")) return {};

// exports from private utils don't need to follow this rule

Check warning on line 326 in _tools/lint_plugin.ts

View check run for this annotation

Codecov / codecov/patch

_tools/lint_plugin.ts#L326

Added line #L326 was not covered by tests
if (url.href.includes("/_")) return {};
// internal exports don't need to follow this rule

Check warning on line 328 in _tools/lint_plugin.ts

View check run for this annotation

Codecov / codecov/patch

_tools/lint_plugin.ts#L328

Added line #L328 was not covered by tests
if (url.href.includes("/internal/")) return {};
// bytes API generally don't follow this rule

Check warning on line 330 in _tools/lint_plugin.ts

View check run for this annotation

Codecov / codecov/patch

_tools/lint_plugin.ts#L330

Added line #L330 was not covered by tests
if (url.href.includes("/bytes/")) return {};

return {
ExportNamedDeclaration(node) {
if (node.declaration?.type !== "FunctionDeclaration") return;
const { params, id } = node.declaration;
if (params.length < 3) return;
if (params.length === 3) {
const param = params.at(-1)!;

switch (param.type) {
case "Identifier":
if (param.name === "options") return;
// Function as 3rd argument is valid (e.g. pooledMap)

Check warning on line 344 in _tools/lint_plugin.ts

View check run for this annotation

Codecov / codecov/patch

_tools/lint_plugin.ts#L344

Added line #L344 was not covered by tests
if (
param.typeAnnotation?.typeAnnotation?.type ===
"TSFunctionType"
) return;
// attributes: Pick<T, "foo" | "bar"> as 3rd argument is valid

Check warning on line 349 in _tools/lint_plugin.ts

View check run for this annotation

Codecov / codecov/patch

_tools/lint_plugin.ts#L349

Added line #L349 was not covered by tests
if (
param.typeAnnotation?.typeAnnotation?.typeName?.name ===
"Pick"
) return;
break;
case "AssignmentPattern": {
if (param.right.type === "ObjectExpression") return;
break;
}

Check warning on line 358 in _tools/lint_plugin.ts

View check run for this annotation

Codecov / codecov/patch

_tools/lint_plugin.ts#L356-L358

Added lines #L356 - L358 were not covered by tests
}

return context.report({
node: id ?? declaration,
message:
"Third argument of export function is not an options object or function.",
hint:
"Export functions can have 0-2 required arguments, plus (if necessary) an options object (so max 3 total).",
});
}
context.report({
node: id ?? declaration,
message: "Exported function has more than three arguments.",
hint:
"Export functions can have 0-2 required arguments, plus (if necessary) an options object (so max 3 total).",
});
},
};
},
},
},
} satisfies Deno.lint.Plugin;
64 changes: 64 additions & 0 deletions _tools/lint_plugin_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,3 +310,67 @@ new CustomError("Can't parse input");
],
);
});

Deno.test("deno-style-guide/exported-function-args-maximum", {
ignore: !Deno.version.deno.startsWith("2"),
}, () => {
// Good
assertLintPluginDiagnostics(
`
function foo() {
}
function foo(bar: unknown) {
}
function foo(bar: unknown, baz: unknown) {
}
function foo(bar: unknown, baz: unknown, options: Record<string, unknown>) {
}
function foo(bar: unknown, baz: unknown, bat: unknown, bay: unknown) {
}
export function foo() {
}
export function foo(bar: unknown) {
}
export function foo(bar: unknown, baz: unknown) {
}
export function foo(bar: unknown, baz: unknown, options: Record<string, unknown>) {
}
// function as last argument is allowed
export function foo(bar: unknown, baz: unknown, bat: () => unknown) {
}
// Pick<T> is usually an object
export function foo(bar: unknown, baz: unknown, bat: Pick<SomeType, "foo">) {
}
`,
[],
);

// Bad
assertLintPluginDiagnostics(
`
export function foo(bar: unknown, baz: unknown, bat: unknown) {
}
export function foo(bar: unknown, baz: unknown, bat: unknown, options: Record<string, unknown>) {
}
`,
[
{
fix: [],
hint:
"Export functions can have 0-2 required arguments, plus (if necessary) an options object (so max 3 total).",
id: "deno-style-guide/exported-function-args-maximum",
message:
"Third argument of export function is not an options object or function.",
range: [17, 20],
},
{
fix: [],
hint:
"Export functions can have 0-2 required arguments, plus (if necessary) an options object (so max 3 total).",
id: "deno-style-guide/exported-function-args-maximum",
message: "Exported function has more than three arguments.",
range: [83, 86],
},
],
);
});
1 change: 1 addition & 0 deletions collections/reduce_groups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import { mapValues } from "./map_values.ts";
* });
* ```
*/
// deno-lint-ignore deno-style-guide/exported-function-args-maximum
export function reduceGroups<T, A>(
record: Readonly<Record<string, ReadonlyArray<T>>>,
reducer: (accumulator: A, current: T) => A,
Expand Down
1 change: 1 addition & 0 deletions collections/running_reduce.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
* assertEquals(sumSteps, [1, 3, 6, 10, 15]);
* ```
*/
// deno-lint-ignore deno-style-guide/exported-function-args-maximum
export function runningReduce<T, O>(
array: readonly T[],
reducer: (accumulator: O, current: T, currentIndex: number) => O,
Expand Down
1 change: 1 addition & 0 deletions encoding/varint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ export function decodeVarint32(buf: Uint8Array, offset = 0): [number, number] {
* assertEquals(encodeVarint(42n, buf), [new Uint8Array([42]), 1]);
* ```
*/
// deno-lint-ignore deno-style-guide/exported-function-args-maximum
export function encodeVarint(
num: bigint | number,
buf: Uint8Array = new Uint8Array(MaxVarintLen64),
Expand Down
2 changes: 2 additions & 0 deletions fs/unstable_chown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { mapError } from "./_map_error.ts";
* @param uid The user id (UID) of the new owner, or `null` for no change.
* @param gid The group id (GID) of the new owner, or `null` for no change.
*/
// deno-lint-ignore deno-style-guide/exported-function-args-maximum
export async function chown(
path: string | URL,
uid: number | null,
Expand Down Expand Up @@ -61,6 +62,7 @@ export async function chown(
* @param uid The user id (UID) of the new owner, or `null` for no change.
* @param gid The group id (GID) of the new owner, or `null` for no change.
*/
// deno-lint-ignore deno-style-guide/exported-function-args-maximum
export function chownSync(
path: string | URL,
uid: number | null,
Expand Down
2 changes: 2 additions & 0 deletions fs/unstable_utime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { mapError } from "./_map_error.ts";
* @param atime The new access time
* @param mtime The new modification time
*/
// deno-lint-ignore deno-style-guide/exported-function-args-maximum
export async function utime(
path: string | URL,
atime: number | Date,
Expand Down Expand Up @@ -78,6 +79,7 @@ export async function utime(
* @param atime The new access time
* @param mtime The new modification time
*/
// deno-lint-ignore deno-style-guide/exported-function-args-maximum
export function utimeSync(
path: string | URL,
atime: number | Date,
Expand Down
1 change: 1 addition & 0 deletions streams/to_transform_stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
* );
* ```
*/
// deno-lint-ignore deno-style-guide/exported-function-args-maximum
export function toTransformStream<I, O>(
transformer: (src: ReadableStream<I>) => Iterable<O> | AsyncIterable<O>,
writableStrategy?: QueuingStrategy<I>,
Expand Down
1 change: 1 addition & 0 deletions webgpu/create_capture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export interface CreateCapture {
* @param height The height of the capture texture.
* @returns The texture to render to and buffer to read from.
*/
// deno-lint-ignore deno-style-guide/exported-function-args-maximum
export function createCapture(
device: GPUDevice,
width: number,
Expand Down
1 change: 1 addition & 0 deletions webgpu/row_padding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export function getRowPadding(width: number): Padding {
* @param height The height of the output buffer.
* @returns The resliced buffer.
*/
// deno-lint-ignore deno-style-guide/exported-function-args-maximum
export function resliceBufferWithPadding(
buffer: Uint8Array,
width: number,
Expand Down
1 change: 1 addition & 0 deletions webgpu/texture_with_data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ function textureMipLevelSize(
* @param data The data to write to the texture.
* @returns The newly created texture.
*/
// deno-lint-ignore deno-style-guide/exported-function-args-maximum
export function createTextureWithData(
device: GPUDevice,
descriptor: GPUTextureDescriptor,
Expand Down
Loading