fix: argument validation of parameters with defaults, Map and Set (#7435

)

packages/transformers/src/validateArgs/transformProgram.ts

@@ -154,6 +154,7 @@ export default function transformProgram(
 					return {
 						name: p.getName(),
 						isRest: p.isRestParameter(),
+						hasInitializer: p.hasInitializer(),
 						type: p.getType(),
 						typeName: p.getTypeNode()?.getText(),
 					};
@@ -205,7 +206,7 @@ export default function transformProgram(
 
 			newSourceText += `
 
-export function validateArgs_${className}_${methodName}(options: any = {}) {
+export function validateArgs_${className}_${methodName}() {
 	return <T extends Function>(__decoratedMethod: T, { kind }: ClassMethodDecoratorContext): T | void => {
 		if (kind === "method") {
 			return function ${methodName}(this: any, ${paramSpreadWithUnknown}) {
@@ -273,6 +274,7 @@ function getTypeName(t: Type<tsm.Type>): string {
 interface ParameterInfo {
 	name: string;
 	isRest?: boolean;
+	hasInitializer?: boolean;
 	type: Type<tsm.Type>;
 	typeName: string | undefined;
 }
@@ -310,6 +312,24 @@ Type ${param.typeName} recursively references itself`,
 	}
 
 	const ctx = `{ kind: "${kind}", name: "${param.name}" }`;
+
+	// Parameters with a default value, but no `| undefined` in the type need to be treated as optional
+	if (
+		param.hasInitializer
+		&& !(
+			param.type.isUnion()
+			&& param.type.getUnionTypes().some((t) => t.isUndefined())
+		)
+	) {
+		return `v.optional(${ctx}, ${
+			getValidationFunction(
+				context,
+				{ ...param, hasInitializer: false },
+				kind,
+			)
+		})`;
+	}
+
 	if (
 		param.type.isNumberLiteral()
 		|| param.type.isStringLiteral()
@@ -554,23 +574,58 @@ Unable to find import specifier for class ${param.typeName}.`,
 			return `v.class(${ctx}, "${declaredName}", ${param.typeName})`;
 		}
 
-		if (isInterface) {
-			const variableDeclaration = valueDeclaration?.asKind(
-				tsm.SyntaxKind.VariableDeclaration,
-			);
-			const isAmbient = !!variableDeclaration
-				&& !!(variableDeclaration?.getCombinedModifierFlags()
-					& tsm.ModifierFlags.Ambient);
+		const variableDeclaration = valueDeclaration?.asKind(
+			tsm.SyntaxKind.VariableDeclaration,
+		);
+		const isAmbient = !!variableDeclaration
+			&& !!(variableDeclaration?.getCombinedModifierFlags()
+				& tsm.ModifierFlags.Ambient);
+
+		if (isAmbient) {
+			if (isInterface) {
+				if (symbolName === "Date") {
+					return `v.date(${ctx})`;
+				}
+				if (symbolName === "Uint8Array") {
+					return `v.uint8array(${ctx})`;
+				}
+			}
+
+			const structure = variableDeclaration?.getStructure();
 
-			if (isAmbient && symbolName === "Date") {
-				return `v.date(${ctx})`;
+			if (
+				structure?.name === "Map"
+				&& structure.type === "MapConstructor"
+			) {
+				return `v.class(${ctx}, "Map", Map)`;
 			}
 
-			if (isAmbient && symbolName === "Uint8Array") {
-				return `v.uint8array(${ctx})`;
+			if (
+				structure?.name === "Set"
+				&& structure.type === "SetConstructor"
+			) {
+				return `v.class(${ctx}, "Set", Set)`;
 			}
 		}
 
+		// Those are not detected as ambient interfaces for some reason
+		if (
+			symbolName === "ReadonlyMap"
+			&& symbol.getDeclarations().every((d) =>
+				d.isKind(tsm.SyntaxKind.InterfaceDeclaration)
+			)
+		) {
+			return `v.class(${ctx}, "Map", Map)`;
+		}
+		if (
+			symbolName === "ReadonlySet"
+			&& symbol.getDeclarations().every((d) =>
+				d.isKind(tsm.SyntaxKind.InterfaceDeclaration)
+			)
+		) {
+			return `v.class(${ctx}, "Set", Set)`;
+		}
+
 		if (isInterface || isObject) {
 			const expectedKind = isInterface
 				? tsm.SyntaxKind.InterfaceDeclaration

packages/transformers/test/fixtures/testDefaultValue.mts

@@ -0,0 +1,41 @@
+/* eslint-disable @typescript-eslint/no-unused-expressions */
+import { validateArgs } from "@zwave-js/transformers";
+import assert from "node:assert";
+
+class Test {
+	@validateArgs()
+	foo(arg1: number = 5): void {
+		arg1;
+		return void 0;
+	}
+
+	@validateArgs()
+	bar(arg1: number | string = 5): void {
+		arg1;
+		return void 0;
+	}
+}
+
+const test = new Test();
+// These should not throw
+test.foo();
+test.foo(1);
+test.foo(undefined);
+
+test.bar();
+test.bar(1);
+test.bar("str");
+test.bar(undefined);
+
+// These should throw
+assert.throws(
+	// @ts-expect-error
+	() => test.foo(true),
+	/optional parameter arg1 to be a number, got true/,
+);
+
+assert.throws(
+	// @ts-expect-error
+	() => test.bar(true),
+	/optional parameter arg1 to be one of (number \| string|string \| number), got true/,
+);

packages/transformers/test/fixtures/testMapAndSet.mts

@@ -0,0 +1,79 @@
+/* eslint-disable @typescript-eslint/no-unused-expressions */
+import { validateArgs } from "@zwave-js/transformers";
+import assert from "node:assert";
+
+class Test {
+	@validateArgs()
+	map(arg1: Map<any, any>): void {
+		arg1;
+		return void 0;
+	}
+
+	@validateArgs()
+	readonlyMap(arg1: ReadonlyMap<any, any>): void {
+		arg1;
+		return void 0;
+	}
+
+	@validateArgs()
+	set(arg1: Set<any>): void {
+		arg1;
+		return void 0;
+	}
+
+	@validateArgs()
+	readonlySet(arg1: ReadonlySet<any>): void {
+		arg1;
+		return void 0;
+	}
+}
+
+const test = new Test();
+// These should not throw
+test.map(new Map());
+test.readonlyMap(new Map()); // readonly does not exist at runtime
+test.set(new Set());
+test.readonlySet(new Set()); // readonly does not exist at runtime
+
+// These should throw
+assert.throws(
+	// @ts-expect-error
+	() => test.map({}),
+	/arg1 to be an instance of class Map, got object/,
+);
+assert.throws(
+	// @ts-expect-error
+	() => test.readonlyMap({}),
+	/arg1 to be an instance of class Map, got object/,
+);
+assert.throws(
+	// @ts-expect-error
+	() => test.map("string"),
+	/arg1 to be an instance of class Map, got string "string"/,
+);
+assert.throws(
+	// @ts-expect-error
+	() => test.readonlyMap("string"),
+	/arg1 to be an instance of class Map, got string "string"/,
+);
+
+assert.throws(
+	// @ts-expect-error
+	() => test.set({}),
+	/arg1 to be an instance of class Set, got object/,
+);
+assert.throws(
+	// @ts-expect-error
+	() => test.readonlySet({}),
+	/arg1 to be an instance of class Set, got object/,
+);
+assert.throws(
+	// @ts-expect-error
+	() => test.set("string"),
+	/arg1 to be an instance of class Set, got string "string"/,
+);
+assert.throws(
+	// @ts-expect-error
+	() => test.readonlySet("string"),
+	/arg1 to be an instance of class Set, got string "string"/,
+);

packages/transformers/tsconfig.test.json

@@ -19,5 +19,5 @@
 		"customConditions": ["@@dev", "@@test_transformers"]
 	},
 	"include": ["test/fixtures/**/*.mts"]
-	// "include": ["test/fixtures/testCrossPackageImports.mts"]
+	// "include": ["test/fixtures/testMapAndSet.mts"]
 }