[fix] handle lone returns in migration tool, turn on strict mode

.changeset/pretty-teachers-smash.md

@@ -0,0 +1,5 @@
+---
+'svelte-migrate': patch
+---
+
+handle lone return statements

packages/migrate/migrations/routes/migrate_page_js/index.js

@@ -25,16 +25,16 @@ export function migrate_page(content) {
 	const file = parse(content);
 	if (!file) return give_up + content;
 
-	if (!file.exports.map.has('load') && !file.exports.complex) {
+	const name = file.exports.map.get('load');
+
+	if (!name && !file.exports.complex) {
 		// there's no load function here, so there's nothing to do
 		return content;
 	}
 
-	const name = file.exports.map.get('load');
-
 	for (const statement of file.ast.statements) {
-		const fn = get_function_node(statement, name);
-		if (fn) {
+		const fn = name ? get_function_node(statement, name) : undefined;
+		if (fn?.body) {
 			check_fn_param(fn, file.code);
 
 			/** @type {Set<string>} */

packages/migrate/migrations/routes/migrate_page_js/samples.md

@@ -280,3 +280,17 @@ export const load = (input) => {
 	return {};
 }
 ```
+
+## A load function that returns nothing
+
+```js before
+export function load() {
+	return;
+}
+```
+
+```js after
+export function load() {
+	return;
+}
+```

packages/migrate/migrations/routes/migrate_page_server/index.js

@@ -27,9 +27,10 @@ export function migrate_page_server(content) {
 	const unmigrated = new Set(methods);
 
 	for (const statement of file.ast.statements) {
-		if (file.exports.map.has('GET')) {
-			const GET = get_function_node(statement, file.exports.map.get('GET'));
-			if (GET) {
+		const GET_id = file.exports.map.get('GET');
+		if (GET_id) {
+			const GET = get_function_node(statement, GET_id);
+			if (GET?.body) {
 				// possible TODOs — handle errors and redirects
 				rewrite_returns(GET.body, (expr, node) => {
 					const nodes = ts.isObjectLiteralExpression(expr) && get_object_nodes(expr);
@@ -47,8 +48,8 @@ export function migrate_page_server(content) {
 		}
 
 		for (const method of non_get_methods) {
-			const fn = get_function_node(statement, file.exports.map.get(method));
-			if (fn) {
+			const fn = get_function_node(statement, /** @type{string} */ (file.exports.map.get(method)));
+			if (fn?.body) {
 				rewrite_returns(fn.body, (expr, node) => {
 					manual_return_migration(node || fn, file.code, TASKS.PAGE_ENDPOINT);
 				});

packages/migrate/migrations/routes/migrate_page_server/samples.md

@@ -68,4 +68,18 @@ export function POST() {
 	throw new Error("@migration task: Migrate this return statement (https://github.com/sveltejs/kit/discussions/5774#discussioncomment-3292699)");
 	return {};
 }
-```
+```
+
+## A function that returns nothing
+
+```js before
+export function GET() {
+	return;
+}
+```
+
+```js after
+export function GET() {
+	return;
+}
+```

packages/migrate/migrations/routes/migrate_scripts/index.js

@@ -46,7 +46,7 @@ export function migrate_scripts(content, is_error, moved) {
 				const delete_var = (/** @type {string } */ key) => {
 					const declaration = declared?.get(key);
 					if (declaration && !declaration.import) {
-						declared.delete(key);
+						declared?.delete(key);
 					}
 				};
 				delete_var('load');
@@ -61,7 +61,7 @@ export function migrate_scripts(content, is_error, moved) {
 						declaration.import.from === '@sveltejs/kit' &&
 						!new RegExp(`\\W${key}\\W`).test(except_str(content, match))
 					) {
-						declared.delete(key);
+						declared?.delete(key);
 					}
 				};
 				delete_kit_type('Load');
@@ -98,7 +98,7 @@ function find_declarations(content) {
 	const file = parse(content);
 	if (!file) return;
 
-	/** @type {Map<string, {name: string, import: {from: string, type_only: boolean}}>} */
+	/** @type {Map<string, {name: string, import?: {from: string, type_only: boolean}}>} */
 	const declared = new Map();
 	/**
 	 * @param {string} name
@@ -111,7 +111,9 @@ function find_declarations(content) {
 	for (const statement of file.ast.statements) {
 		if (ts.isImportDeclaration(statement) && statement.importClause) {
 			let type_only = statement.importClause.isTypeOnly;
-			const from = ts.isStringLiteral(statement.moduleSpecifier) && statement.moduleSpecifier.text;
+			const from = ts.isStringLiteral(statement.moduleSpecifier)
+				? statement.moduleSpecifier.text
+				: '';
 
 			if (statement.importClause.name) {
 				add(statement.importClause.name.text, { from, type_only });
@@ -137,7 +139,7 @@ function find_declarations(content) {
 				}
 			}
 		} else if (ts.isFunctionDeclaration(statement) || ts.isClassDeclaration(statement)) {
-			if (ts.isIdentifier(statement.name)) {
+			if (statement.name && ts.isIdentifier(statement.name)) {
 				add(statement.name.text);
 			}
 		} else if (ts.isExportDeclaration(statement) && !statement.exportClause) {

packages/migrate/migrations/routes/migrate_server/index.js

@@ -30,8 +30,8 @@ export function migrate_server(content) {
 
 	for (const statement of file.ast.statements) {
 		for (const method of methods) {
-			const fn = get_function_node(statement, file.exports.map.get(method));
-			if (fn) {
+			const fn = get_function_node(statement, /** @type{string} */ (file.exports.map.get(method)));
+			if (fn?.body) {
 				rewrite_returns(fn.body, (expr, node) => {
 					// leave `() => new Response(...)` alone
 					if (is_new(expr, 'Response')) return;

packages/migrate/migrations/routes/migrate_server/samples.md

@@ -112,4 +112,18 @@ export const GET = () => createResponse('text');
 ```js after
 throw new Error("@migration task: Migrate this return statement (https://github.com/sveltejs/kit/discussions/5774#discussioncomment-3292701)");
 export const GET = () => createResponse('text');
-```
+```
+
+## A function that returns nothing
+
+```js before
+export function GET() {
+	return;
+}
+```
+
+```js after
+export function GET() {
+	return;
+}
+```

packages/migrate/migrations/routes/utils.js

@@ -179,7 +179,7 @@ export function guess_indent(content) {
 
 	// Otherwise, we need to guess the multiple
 	const min = spaced.reduce((previous, current) => {
-		const count = /^ +/.exec(current)[0].length;
+		const count = /^ +/.exec(current)?.[0].length ?? 0;
 		return Math.min(count, previous);
 	}, Infinity);
 
@@ -192,7 +192,7 @@ export function guess_indent(content) {
  */
 export function indent_at_line(content, offset) {
 	const substr = content.substring(content.lastIndexOf('\n', offset) + 1, offset);
-	return /\s*/.exec(substr)[0];
+	return /\s*/.exec(substr)?.[0] ?? '';
 }
 
 /**
@@ -282,13 +282,18 @@ export function get_exports(node) {
 	let complex = false;
 
 	for (const statement of node.statements) {
-		if (ts.isExportDeclaration(statement) && ts.isNamedExports(statement.exportClause)) {
+		if (
+			ts.isExportDeclaration(statement) &&
+			statement.exportClause &&
+			ts.isNamedExports(statement.exportClause)
+		) {
 			// export { x }, export { x as y }
 			for (const specifier of statement.exportClause.elements) {
 				map.set(specifier.name.text, specifier.propertyName?.text || specifier.name.text);
 			}
 		} else if (
 			ts.isFunctionDeclaration(statement) &&
+			statement.name &&
 			statement.modifiers?.[0]?.kind === ts.SyntaxKind.ExportKeyword
 		) {
 			// export function x ...
@@ -316,10 +321,14 @@ export function get_exports(node) {
 /**
  * @param {ts.Node} statement
  * @param {string[]} names
- * @returns {ts.FunctionDeclaration | ts.FunctionExpression | ts.ArrowFunction | void}
+ * @returns {ts.FunctionDeclaration | ts.FunctionExpression | ts.ArrowFunction | undefined}
  */
 export function get_function_node(statement, ...names) {
-	if (ts.isFunctionDeclaration(statement) && names.includes(statement.name.text)) {
+	if (
+		ts.isFunctionDeclaration(statement) &&
+		statement.name &&
+		names.includes(statement.name.text)
+	) {
 		// export function x ...
 		return statement;
 	}
@@ -329,6 +338,7 @@ export function get_function_node(statement, ...names) {
 			if (
 				ts.isIdentifier(declaration.name) &&
 				names.includes(declaration.name.text) &&
+				declaration.initializer &&
 				(ts.isArrowFunction(declaration.initializer) ||
 					ts.isFunctionExpression(declaration.initializer))
 			) {
@@ -340,10 +350,11 @@ export function get_function_node(statement, ...names) {
 }
 
 /**
- * Utility for rewriting return statements. If `node` is `undefined`,
- * it means it's a concise arrow function body (`() => ({}))`
+ * Utility for rewriting return statements.
+ * If `node` is `undefined`, it means it's a concise arrow function body (`() => ({}))`.
+ * Lone `return;` statements are left untouched.
  * @param {ts.Block | ts.ConciseBody} block
- * @param {(expression: ts.Expression, node: ts.ReturnStatement | void) => void} callback
+ * @param {(expression: ts.Expression, node: ts.ReturnStatement | undefined) => void} callback
  */
 export function rewrite_returns(block, callback) {
 	if (ts.isBlock(block)) {
@@ -358,7 +369,7 @@ export function rewrite_returns(block, callback) {
 				return;
 			}
 
-			if (ts.isReturnStatement(node)) {
+			if (ts.isReturnStatement(node) && node.expression) {
 				callback(node.expression, node);
 				return;
 			}
@@ -372,7 +383,7 @@ export function rewrite_returns(block, callback) {
 			block = block.expression;
 		}
 
-		callback(block, null);
+		callback(block, undefined);
 	}
 }
 

packages/migrate/package.json

@@ -32,6 +32,7 @@
 		"uvu": "^0.5.6"
 	},
 	"scripts": {
-		"test": "uvu"
+		"test": "uvu",
+		"check": "tsc"
 	}
 }

packages/migrate/tsconfig.json

@@ -3,7 +3,7 @@
 		"allowJs": true,
 		"checkJs": true,
 		"noEmit": true,
-		"noImplicitAny": true,
+		"strict": true,
 		"target": "esnext",
 		"module": "esnext",
 		"moduleResolution": "node",