ronantakizawa/codereview-bench huggingface dataset:
Repository: sveltejs/svelte
File: packages/svelte/src/compiler/phases/1-parse/state/tag.js (JavaScript)

Code before:

		return;
	}

	if (parser.eat('snippet')) {
		parser.require_whitespace();

		const name_start = parser.index;
		const name = parser.read_identifier();
		const name_end = parser.index;

		if (name === null) {
			e.expected_identifier(parser.index);
		}

		parser.eat('(', true);

		let parentheses = 1;
		let params = '';

		while (!parser.match(')') || parentheses !== 1) {
			if (parser.match('(')) parentheses++;
			if (parser.match(')')) parentheses--;
			params += parser.read(/^./);
		}

		const function_expression = parse_expression_at(`function ${name}(${params}){}`, parser.ts, 0);

		parser.index += 2;

		if (function_expression.type === 'FunctionExpression') {
			/** @type {ReturnType<typeof parser.append<import('#compiler').SnippetBlock>>} */
			const block = parser.append({
				type: 'SnippetBlock',
				start,
				end: -1,
				expression: {
					type: 'Identifier',
					start: name_start,
					end: name_end,
					name
				},
				parameters: function_expression.params.map((param) => {
					param.start += start + 1;
					param.end += start + 1;
					return param;
				}),
				body: create_fragment()
			});
			parser.stack.push(block);
			parser.fragments.push(block.body);

Review comment (bug):
[not a new bug](https://svelte-5-preview.vercel.app/#H4sIAAAAAAAAE32NMQ7CMAxFr2LM0IWIPQIkzkE6RK0rIlonalJEsXx3GiYmxvf0n77gEEbKaG-C7CdCi9eU8IBlTRXyk8ZCG-e4zF01p50xEB9gzMWx7Ml3d3iBzyCwwhmadwOqcqxeHTv-7jmWf438NNvXFPswBOrRlnkhbfUDiwRjwKYAAAA=), but this logic probably won't be sufficient because these characters could appear inside strings etc. we should probably fix it both here and inside `read_pattern`...

Code after:

		return;
	}

	if (parser.eat('snippet')) {
		parser.require_whitespace();

		const name_start = parser.index;
		const name = parser.read_identifier();
		const name_end = parser.index;

		if (name === null) {
			e.expected_identifier(parser.index);
		}

		let slice_end = parser.index;

		parser.eat('(', true);

		let parentheses = 1;
		let params = '';

		while (!parser.match(')') || parentheses !== 1) {
			if (parser.match('(')) parentheses++;
			if (parser.match(')')) parentheses--;
			params += parser.read(/^./);
		}

		let function_expression = /** @type {import('estree').ArrowFunctionExpression} */ (
			parse_expression_at(
				parser.template.slice(0, slice_end).replace(/\S/g, ' ') + `(${params}) => {}`,
				parser.ts,
				0
			)
		);

		parser.index += 2;

		/** @type {ReturnType<typeof parser.append<import('#compiler').SnippetBlock>>} */
		const block = parser.append({
			type: 'SnippetBlock',
			start,
			end: -1,
			expression: {
				type: 'Identifier',
				start: name_start,
				end: name_end,
				name
			},
			parameters: function_expression.params,
			body: create_fragment()
Repository: sveltejs/svelte
File: packages/svelte/src/internal/client/reactivity/props.js (JavaScript)

Code before:
			return child_value;
		}

		was_from_child = false;
		return (inner_current_value.v = parent_value);
	});

	if (!immutable) current_value.equals = safe_equals;

	return function (/** @type {any} */ reassigned_value, /** @type {boolean} */ mutation) {
		var current = get(current_value);

		// legacy nonsense — need to ensure the source is invalidated when necessary
		// also needed for when handling inspect logic so we can inspect the correct source signal
		if (is_signals_recorded) {
			// set this so that we don't reset to the parent value if `d`
			// is invalidated because of `invalidate_inner_signals` (rather
			// than because the parent or child value changed)
			from_child = was_from_child;
			// invoke getters so that signals are picked up by `invalidate_inner_signals`
			getter();
			get(inner_current_value);
		}

		if (arguments.length > 0) {
			const value = mutation ? get(current_value) : reassigned_value;
			if (!current_value.equals(value)) {
				from_child = true;
				set(inner_current_value, value);
				get(current_value); // force a synchronisation immediately
			}

			return reassigned_value;
		}

		return current;
	};
}


Review comment (suggestion):
```suggestion
			const value = mutation ? get(current_value) : new_value;
```

Code after:
			return child_value;
		}

		was_from_child = false;
		return (inner_current_value.v = parent_value);
	});

	if (!immutable) current_value.equals = safe_equals;

	return function (/** @type {any} */ value, /** @type {boolean} */ mutation) {
		var current = get(current_value);

		// legacy nonsense — need to ensure the source is invalidated when necessary
		// also needed for when handling inspect logic so we can inspect the correct source signal
		if (is_signals_recorded) {
			// set this so that we don't reset to the parent value if `d`
			// is invalidated because of `invalidate_inner_signals` (rather
			// than because the parent or child value changed)
			from_child = was_from_child;
			// invoke getters so that signals are picked up by `invalidate_inner_signals`
			getter();
			get(inner_current_value);
		}

		if (arguments.length > 0) {
			const new_value = mutation ? get(current_value) : value;

			if (!current_value.equals(new_value)) {
				from_child = true;
				set(inner_current_value, new_value);
				get(current_value); // force a synchronisation immediately
			}

			return value;
		}

		return current;
	};
}
Repository: sveltejs/svelte
File: packages/svelte/src/internal/client/dom/elements/bindings/media.js (JavaScript)

Code before:
		array.push({ start: ranges.start(i), end: ranges.end(i) });
	}

	return array;
}

/**
 * @param {HTMLVideoElement | HTMLAudioElement} media
 * @param {() => number | undefined} get_value
 * @param {(value: number) => void} update
 * @returns {void}
 */
export function bind_current_time(media, get_value, update) {
	/** @type {number} */
	var raf_id;
	var updating = false;

	// Ideally, listening to timeupdate would be enough, but it fires too infrequently for the currentTime
	// binding, which is why we use a raf loop, too. We additionally still listen to timeupdate because
	// the user could be scrubbing through the video using the native controls when the media is paused.
	var callback = () => {
		cancelAnimationFrame(raf_id);

		if (!media.paused) {
			raf_id = requestAnimationFrame(callback);
		}

		updating = true;
		update(media.currentTime);
	};

	raf_id = requestAnimationFrame(callback);
	media.addEventListener('timeupdate', callback);

	render_effect(() => {
		var value = get_value();

		// through isNaN we also allow number strings, which is more robust
		if (!updating && !isNaN(/** @type {any} */ (value))) {
			media.currentTime = /** @type {number} */ (value);
		}

		updating = false;
	});

	teardown(() => cancelAnimationFrame(raf_id));
}

/**
 * @param {HTMLVideoElement | HTMLAudioElement} media
 * @param {(array: Array<{ start: number; end: number }>) => void} update

Review comment (suggestion):
I wonder if a rAF loop is right here because media should play when off tab, but the rAF will not run at all.

Code after:
		array.push({ start: ranges.start(i), end: ranges.end(i) });
	}

	return array;
}

/**
 * @param {HTMLVideoElement | HTMLAudioElement} media
 * @param {() => number | undefined} get_value
 * @param {(value: number) => void} update
 * @returns {void}
 */
export function bind_current_time(media, get_value, update) {
	/** @type {number} */
	var raf_id;
	/** @type {number} */
	var value;

	// Ideally, listening to timeupdate would be enough, but it fires too infrequently for the currentTime
	// binding, which is why we use a raf loop, too. We additionally still listen to timeupdate because
	// the user could be scrubbing through the video using the native controls when the media is paused.
	var callback = () => {
		cancelAnimationFrame(raf_id);

		if (!media.paused) {
			raf_id = requestAnimationFrame(callback);
		}

		var next_value = media.currentTime;
		if (value !== next_value) {
			update((value = next_value));
		}
	};

	raf_id = requestAnimationFrame(callback);
	media.addEventListener('timeupdate', callback);

	render_effect(() => {
		var next_value = Number(get_value());

		if (value !== next_value && !isNaN(/** @type {any} */ (next_value))) {
			media.currentTime = value = next_value;
		}
	});

	teardown(() => cancelAnimationFrame(raf_id));
}

/**
 * @param {HTMLVideoElement | HTMLAudioElement} media
 * @param {(array: Array<{ start: number; end: number }>) => void} update
Repository: sveltejs/svelte
File: packages/svelte/src/reactivity/index-server.js (JavaScript)

Code before:
export const SvelteDate = globalThis.Date;
export const SvelteSet = globalThis.Set;
export const SvelteMap = globalThis.Map;
export const SvelteURL = globalThis.URL;
export const SvelteURLSearchParams = globalThis.URLSearchParams;

// Deprecated
export const Date = SvelteDate;
export const Set = SvelteSet;
export const Map = SvelteMap;
export const URL = SvelteURL;
export const URLSearchParams = SvelteURLSearchParams;


Review comment (suggestion):
We should be consistent on the server vs the client for whether the old exports still actually work or throw an error.

Code after:
export const SvelteDate = globalThis.Date;
export const SvelteSet = globalThis.Set;
export const SvelteMap = globalThis.Map;
export const SvelteURL = globalThis.URL;
export const SvelteURLSearchParams = globalThis.URLSearchParams;

/** @deprecated Use `SvelteDate` instead */
export function Date() {
	throw new Error('Date has been removed, use SvelteDate instead.');
}

/** @deprecated Use `SvelteSet` instead */
export function Set() {
	throw new Error('Set has been removed, use SvelteSet instead.');
}

/** @deprecated Use `SvelteMap` instead */
export function Map() {
	throw new Error('Map has been removed, use SvelteMap instead.');
}

/** @deprecated Use `SvelteURL` instead */
export function URL() {
	throw new Error('URL has been removed, use SvelteURL instead.');
}

/** @deprecated Use `SvelteURLSearchParams` instead */
export function URLSearchParams() {
	throw new Error('URLSearchParams has been removed, use SvelteURLSearchParams instead.');
}
Repository: sveltejs/svelte
File: packages/svelte/src/reactivity/index-client.js (JavaScript)

Code before:
export { SvelteDate } from './date.js';
export { SvelteSet } from './set.js';
export { SvelteMap } from './map.js';
export { SvelteURL, SvelteURLSearchParams } from './url.js';

/** @deprecated Use `SvelteDate` instead */
export function Date() {
	throw new Error('Date has been removed, use SvelteDate instead.');
}

/** @deprecated Use `SvelteSet` instead */
export function Set() {
	throw new Error('Set has been removed, use SvelteSet instead.');
}

/** @deprecated Use `SvelteMap` instead */
export function Map() {
	throw new Error('Map has been removed, use DeprecatedMap instead.');
}

/** @deprecated Use `SvelteURL` instead */
export function URL() {
	throw new Error('URL has been removed, use DeprecatedURL instead.');
}

/** @deprecated Use `SvelteURLSearchParams` instead */
export function URLSearchParams() {
	throw new Error('URLSearchParams has been removed, use SvelteURLSearchParams instead.');
}


Review comment (suggestion):
```suggestion
	throw new Error('Map has been removed, use SvelteMap instead.');
```

Code after:
export { SvelteDate } from './date.js';
export { SvelteSet } from './set.js';
export { SvelteMap } from './map.js';
export { SvelteURL, SvelteURLSearchParams } from './url.js';

/** @deprecated Use `SvelteDate` instead */
export function Date() {
	throw new Error('Date has been removed, use SvelteDate instead.');
}

/** @deprecated Use `SvelteSet` instead */
export function Set() {
	throw new Error('Set has been removed, use SvelteSet instead.');
}

/** @deprecated Use `SvelteMap` instead */
export function Map() {
	throw new Error('Map has been removed, use SvelteMap instead.');
}

/** @deprecated Use `SvelteURL` instead */
export function URL() {
	throw new Error('URL has been removed, use SvelteURL instead.');
}

/** @deprecated Use `SvelteURLSearchParams` instead */
export function URLSearchParams() {
	throw new Error('URLSearchParams has been removed, use SvelteURLSearchParams instead.');
}
Repository: sveltejs/svelte
File: packages/svelte/src/reactivity/index-client.js (JavaScript)

Code before:
export { SvelteDate } from './date.js';
export { SvelteSet } from './set.js';
export { SvelteMap } from './map.js';
export { SvelteURL, SvelteURLSearchParams } from './url.js';

/** @deprecated Use `SvelteDate` instead */
export function Date() {
	throw new Error('Date has been removed, use SvelteDate instead.');
}

/** @deprecated Use `SvelteSet` instead */
export function Set() {
	throw new Error('Set has been removed, use SvelteSet instead.');
}

/** @deprecated Use `SvelteMap` instead */
export function Map() {
	throw new Error('Map has been removed, use DeprecatedMap instead.');
}

/** @deprecated Use `SvelteURL` instead */
export function URL() {
	throw new Error('URL has been removed, use DeprecatedURL instead.');
}

/** @deprecated Use `SvelteURLSearchParams` instead */
export function URLSearchParams() {
	throw new Error('URLSearchParams has been removed, use SvelteURLSearchParams instead.');
}


Review comment (suggestion):
```suggestion
	throw new Error('URL has been removed, use SvelteURL instead.');
```

Code after:
export { SvelteDate } from './date.js';
export { SvelteSet } from './set.js';
export { SvelteMap } from './map.js';
export { SvelteURL, SvelteURLSearchParams } from './url.js';

/** @deprecated Use `SvelteDate` instead */
export function Date() {
	throw new Error('Date has been removed, use SvelteDate instead.');
}

/** @deprecated Use `SvelteSet` instead */
export function Set() {
	throw new Error('Set has been removed, use SvelteSet instead.');
}

/** @deprecated Use `SvelteMap` instead */
export function Map() {
	throw new Error('Map has been removed, use SvelteMap instead.');
}

/** @deprecated Use `SvelteURL` instead */
export function URL() {
	throw new Error('URL has been removed, use SvelteURL instead.');
}

/** @deprecated Use `SvelteURLSearchParams` instead */
export function URLSearchParams() {
	throw new Error('URLSearchParams has been removed, use SvelteURLSearchParams instead.');
}
Repository: sveltejs/svelte
File: packages/svelte/scripts/process-messages/templates/compile-warnings.js (JavaScript)

Code before:
	 * @param {[number, number] | undefined} position
	 */
	constructor(code, message, position) {
		super(code, message, position);
	}
}

/**
 * @param {null | NodeLike} node
 * @param {string} code
 * @param {string} message
 */
function w(node, code, message) {
	let stack = ignore_stack;
	if (node) {
		stack = ignore_map.get(node) ?? ignore_stack;
	}
	if (stack && stack.at(-1)?.has(code)) return;

	const warning = new InternalCompileWarning(
		code,
		message,
		node && node.start !== undefined ? [node.start, node.end ?? node.start] : undefined
	);

	if (filter_warning && !filter_warning(warning)) return;

	warnings.push(warning);
}

export const codes = CODES;

/**
 * MESSAGE
 * @param {null | NodeLike} node
 * @param {string} PARAMETER
 */
export function CODE(node, PARAMETER) {
	w(node, 'CODE', MESSAGE);
}


Review comment (suggestion):
If we make it required, we can get rid of `filter_warning &&`

```suggestion
	if (!filter_warning(warning)) return;
```

Code after:
	 * @param {[number, number] | undefined} position
	 */
	constructor(code, message, position) {
		super(code, message, position);
	}
}

/**
 * @param {null | NodeLike} node
 * @param {string} code
 * @param {string} message
 */
function w(node, code, message) {
	let stack = ignore_stack;
	if (node) {
		stack = ignore_map.get(node) ?? ignore_stack;
	}
	if (stack && stack.at(-1)?.has(code)) return;

	const warning = new InternalCompileWarning(
		code,
		message,
		node && node.start !== undefined ? [node.start, node.end ?? node.start] : undefined
	);

	if (!warning_filter(warning)) return;

	warnings.push(warning);
}

export const codes = CODES;

/**
 * MESSAGE
 * @param {null | NodeLike} node
 * @param {string} PARAMETER
 */
export function CODE(node, PARAMETER) {
	w(node, 'CODE', MESSAGE);
}
Repository: sveltejs/svelte
File: packages/svelte/src/compiler/warnings.js (JavaScript)

Code before:
	 * @param {string} code
	 * @param {string} message
	 * @param {[number, number] | undefined} position
	 */
	constructor(code, message, position) {
		super(code, message, position);
	}
}

/**
 * @param {null | NodeLike} node
 * @param {string} code
 * @param {string} message
 */
function w(node, code, message) {
	let stack = ignore_stack;

	if (node) {
		stack = ignore_map.get(node) ?? ignore_stack;
	}

	if (stack && stack.at(-1)?.has(code)) return;

	const warning = new InternalCompileWarning(code, message, node && node.start !== undefined ? [node.start, node.end ?? node.start] : undefined);

	if (filter_warning && !filter_warning(warning)) return;
	warnings.push(warning);
}

export const codes = [
	"a11y_accesskey",
	"a11y_aria_activedescendant_has_tabindex",
	"a11y_aria_attributes",
	"a11y_autocomplete_valid",
	"a11y_autofocus",
	"a11y_click_events_have_key_events",
	"a11y_distracting_elements",
	"a11y_figcaption_index",
	"a11y_figcaption_parent",
	"a11y_hidden",
	"a11y_img_redundant_alt",
	"a11y_incorrect_aria_attribute_type",
	"a11y_incorrect_aria_attribute_type_boolean",
	"a11y_incorrect_aria_attribute_type_id",
	"a11y_incorrect_aria_attribute_type_idlist",
	"a11y_incorrect_aria_attribute_type_integer",
	"a11y_incorrect_aria_attribute_type_token",
	"a11y_incorrect_aria_attribute_type_tokenlist",
	"a11y_incorrect_aria_attribute_type_tristate",
	"a11y_interactive_supports_focus",
	"a11y_invalid_attribute",

Review comment (suggestion):
```suggestion
	if (!filter_warning(warning)) return;
```

Code after:
	 * @param {string} code
	 * @param {string} message
	 * @param {[number, number] | undefined} position
	 */
	constructor(code, message, position) {
		super(code, message, position);
	}
}

/**
 * @param {null | NodeLike} node
 * @param {string} code
 * @param {string} message
 */
function w(node, code, message) {
	let stack = ignore_stack;

	if (node) {
		stack = ignore_map.get(node) ?? ignore_stack;
	}

	if (stack && stack.at(-1)?.has(code)) return;

	const warning = new InternalCompileWarning(code, message, node && node.start !== undefined ? [node.start, node.end ?? node.start] : undefined);

	if (!warning_filter(warning)) return;
	warnings.push(warning);
}

export const codes = [
	"a11y_accesskey",
	"a11y_aria_activedescendant_has_tabindex",
	"a11y_aria_attributes",
	"a11y_autocomplete_valid",
	"a11y_autofocus",
	"a11y_click_events_have_key_events",
	"a11y_distracting_elements",
	"a11y_figcaption_index",
	"a11y_figcaption_parent",
	"a11y_hidden",
	"a11y_img_redundant_alt",
	"a11y_incorrect_aria_attribute_type",
	"a11y_incorrect_aria_attribute_type_boolean",
	"a11y_incorrect_aria_attribute_type_id",
	"a11y_incorrect_aria_attribute_type_idlist",
	"a11y_incorrect_aria_attribute_type_integer",
	"a11y_incorrect_aria_attribute_type_token",
	"a11y_incorrect_aria_attribute_type_tokenlist",
	"a11y_incorrect_aria_attribute_type_tristate",
	"a11y_interactive_supports_focus",
	"a11y_invalid_attribute",
Repository: sveltejs/svelte
File: eslint.config.js (JavaScript)

Code before:
import svelte_config from '@sveltejs/eslint-config';
import lube from 'eslint-plugin-lube';

const no_compiler_imports = {
	meta: {
		type: /** @type {const} */ ('problem'),
		docs: {
			description:
				'Enforce that there are no imports to the compiler in runtime code. ' +
				'This prevent accidental inclusion of the compiler runtime and ' +
				"ensures that TypeScript does not pick up more ambient types (for example from Node) that shouldn't be available in the browser."
		}
	},
	create(context) {
		return {
			Program: () => {
				// Do a simple string search because ESLint doesn't provide a way to check JSDoc comments.
				// The string search could in theory yielf false positives, but in practice it's unlikely.
				const text = context.sourceCode.getText();
				const idx = Math.max(text.indexOf('../compiler/'), text.indexOf('#compiler'));
				if (idx !== -1) {
					context.report({
						loc: {
							start: context.sourceCode.getLocFromIndex(idx),
							end: context.sourceCode.getLocFromIndex(idx + 12)
						},
						message:
							'References to compiler code are forbidden in runtime code (both for type and value imports)'
					});
				}
			}
		};
	}
};

/** @type {import('eslint').Linter.FlatConfig[]} */
export default [
	...svelte_config,
	{
		languageOptions: {
			parserOptions: {
				project: true
			}

Review comment (suggestion):
```suggestion
				// The string search could in theory yield false positives, but in practice it's unlikely.
```

Code after:
import svelte_config from '@sveltejs/eslint-config';
import lube from 'eslint-plugin-lube';

const no_compiler_imports = {
	meta: {
		type: /** @type {const} */ ('problem'),
		docs: {
			description:
				'Enforce that there are no imports to the compiler in runtime code. ' +
				'This prevent accidental inclusion of the compiler runtime and ' +
				"ensures that TypeScript does not pick up more ambient types (for example from Node) that shouldn't be available in the browser."
		}
	},
	create(context) {
		return {
			Program: () => {
				// Do a simple string search because ESLint doesn't provide a way to check JSDoc comments.
				// The string search could in theory yield false positives, but in practice it's unlikely.
				const text = context.sourceCode.getText();
				const idx = Math.max(text.indexOf('../compiler/'), text.indexOf('#compiler'));
				if (idx !== -1) {
					context.report({
						loc: {
							start: context.sourceCode.getLocFromIndex(idx),
							end: context.sourceCode.getLocFromIndex(idx + 12)
						},
						message:
							'References to compiler code are forbidden in runtime code (both for type and value imports)'
					});
				}
			}
		};
	}
};

/** @type {import('eslint').Linter.FlatConfig[]} */
export default [
	...svelte_config,
	{
		languageOptions: {
			parserOptions: {
				project: true
			}