ronantakizawa/codereview-bench huggingface dataset:
Repository: jquery/jquery
File: src/ajax/xhr.js (JavaScript)

Code before:
import jQuery from "../core.js";

import "../ajax.js";

jQuery.ajaxSettings.xhr = function() {
	return new window.XMLHttpRequest();
};

var xhrSuccessStatus = {

	// File protocol always yields status code 0, assume 200
	0: 200
};

jQuery.ajaxTransport( function( options ) {
	var callback;

	// Cross domain only allowed if supported through XMLHttpRequest
	return {
		send: function( headers, complete ) {
			var i,
				xhr = options.xhr();

			xhr.open(
				options.type,
				options.url,
				options.async,
				options.username,
				options.password
			);

			// Apply custom fields if provided
			if ( options.xhrFields ) {
				for ( i in options.xhrFields ) {
					xhr[ i ] = options.xhrFields[ i ];
				}
			}

			// Override mime type if needed
			if ( options.mimeType && xhr.overrideMimeType ) {
				xhr.overrideMimeType( options.mimeType );
			}


Review comment (suggestion):
This is unrelated but this comment has long been invalid… We only have support checks on `3.x-stable` here.

Code after:
import jQuery from "../core.js";

import "../ajax.js";

jQuery.ajaxSettings.xhr = function() {
	return new window.XMLHttpRequest();
};

var xhrSuccessStatus = {

	// File protocol always yields status code 0, assume 200
	0: 200
};

jQuery.ajaxTransport( function( options ) {
	var callback;

	return {
		send: function( headers, complete ) {
			var i,
				xhr = options.xhr();

			xhr.open(
				options.type,
				options.url,
				options.async,
				options.username,
				options.password
			);

			// Apply custom fields if provided
			if ( options.xhrFields ) {
				for ( i in options.xhrFields ) {
					xhr[ i ] = options.xhrFields[ i ];
				}
			}

			// Override mime type if needed
			if ( options.mimeType && xhr.overrideMimeType ) {
				xhr.overrideMimeType( options.mimeType );
			}

			// X-Requested-With header
Repository: jquery/jquery
File: src/selector/rbuggyQSA.js (JavaScript)

Code before:
if ( isIE ) {
	rbuggyQSA.push(

		// Support: IE 9 - 11+
		// IE's :disabled selector does not pick up the children of disabled fieldsets
		":enabled",
		":disabled",

		// Support: IE 11+
		// IE 11 doesn't find elements on a `[name='']` query in some cases.
		// Adding a temporary attribute to the document before the selection works
		// around the issue.
		"\\[" + whitespace + "*name" + whitespace + "*=" +
			whitespace + "*(?:''|\"\")"
	);
}

if ( !support.cssSupportsSelector ) {

	// Support: Chrome 105+, Safari 15.4+
	// `:has()` uses a forgiving selector list as an argument so our regular
	// `try-catch` mechanism fails to catch `:has()` with arguments not supported
	// natively like `:has(:contains("Foo"))`. Where supported & spec-compliant,
	// we now use `CSS.supports("selector(:is(SELECTOR_TO_BE_TESTED))")` but,
	// outside that, let's mark `:has` as buggy to always use jQuery traversal
	// for `:has()`.
	rbuggyQSA.push( ":has" );
}

rbuggyQSA = rbuggyQSA.length && new RegExp( rbuggyQSA.join( "|" ) );

export default rbuggyQSA;


Review comment (bug):
```suggestion
		// we now use `CSS.supports("selector(:is(SELECTOR_TO_BE_TESTED))")`, but
		// outside that we mark `:has` as buggy.
```

Code after:
if ( isIE ) {
	rbuggyQSA.push(

		// Support: IE 9 - 11+
		// IE's :disabled selector does not pick up the children of disabled fieldsets
		":enabled",
		":disabled",

		// Support: IE 11+
		// IE 11 doesn't find elements on a `[name='']` query in some cases.
		// Adding a temporary attribute to the document before the selection works
		// around the issue.
		"\\[" + whitespace + "*name" + whitespace + "*=" +
			whitespace + "*(?:''|\"\")"
	);
}

if ( !support.cssSupportsSelector ) {

	// Support: Chrome 105+, Safari 15.4+
	// `:has()` uses a forgiving selector list as an argument so our regular
	// `try-catch` mechanism fails to catch `:has()` with arguments not supported
	// natively like `:has(:contains("Foo"))`. Where supported & spec-compliant,
	// we now use `CSS.supports("selector(:is(SELECTOR_TO_BE_TESTED))")`, but
	// outside that we mark `:has` as buggy.
	rbuggyQSA.push( ":has" );
}

rbuggyQSA = rbuggyQSA.length && new RegExp( rbuggyQSA.join( "|" ) );

export default rbuggyQSA;
Repository: jquery/jquery
File: src/selector/rbuggyQSA.js (JavaScript)

Code before:

if ( isIE ) {
	rbuggyQSA.push(

		// Support: IE 9 - 11+
		// IE's :disabled selector does not pick up the children of disabled fieldsets
		":enabled",
		":disabled",

		// Support: IE 11+
		// IE 11 doesn't find elements on a `[name='']` query in some cases.
		// Adding a temporary attribute to the document before the selection works
		// around the issue.
		"\\[" + whitespace + "*name" + whitespace + "*=" +
			whitespace + "*(?:''|\"\")"
	);
}

if ( !support.cssSupportsSelector ) {

	// Support: Chrome 105+, Safari 15.4+
	// `:has()` uses a forgiving selector list as an argument so our regular
	// `try-catch` mechanism fails to catch `:has()` with arguments not supported
	// natively like `:has(:contains("Foo"))`. Where supported & spec-compliant,
		// we now use `CSS.supports("selector(:is(SELECTOR_TO_BE_TESTED))")`, but
		// outside that we mark `:has` as buggy.
	rbuggyQSA.push( ":has" );
}

rbuggyQSA = rbuggyQSA.length && new RegExp( rbuggyQSA.join( "|" ) );

export default rbuggyQSA;


Review comment (bug):
```suggestion
	// we now use `CSS.supports("selector(:is(SELECTOR_TO_BE_TESTED))")`, but
	// outside that we mark `:has` as buggy.
```

Code after:

if ( isIE ) {
	rbuggyQSA.push(

		// Support: IE 9 - 11+
		// IE's :disabled selector does not pick up the children of disabled fieldsets
		":enabled",
		":disabled",

		// Support: IE 11+
		// IE 11 doesn't find elements on a `[name='']` query in some cases.
		// Adding a temporary attribute to the document before the selection works
		// around the issue.
		"\\[" + whitespace + "*name" + whitespace + "*=" +
			whitespace + "*(?:''|\"\")"
	);
}

if ( !support.cssSupportsSelector ) {

	// Support: Chrome 105+, Safari 15.4+
	// `:has()` uses a forgiving selector list as an argument so our regular
	// `try-catch` mechanism fails to catch `:has()` with arguments not supported
	// natively like `:has(:contains("Foo"))`. Where supported & spec-compliant,
	// we now use `CSS.supports("selector(:is(SELECTOR_TO_BE_TESTED))")`, but
	// outside that we mark `:has` as buggy.
	rbuggyQSA.push( ":has" );
}

rbuggyQSA = rbuggyQSA.length && new RegExp( rbuggyQSA.join( "|" ) );

export default rbuggyQSA;
Repository: jquery/jquery
File: src/selector/rbuggyQSA.js (JavaScript)

Code before:
var rbuggyQSA = [];

if ( isIE ) {
	rbuggyQSA.push(

		// Support: IE 9 - 11+
		// IE's :disabled selector does not pick up the children of disabled fieldsets
		":enabled",
		":disabled",

		// Support: IE 11+
		// IE 11 doesn't find elements on a `[name='']` query in some cases.
		// Adding a temporary attribute to the document before the selection works
		// around the issue.
		"\\[" + whitespace + "*name" + whitespace + "*=" +
			whitespace + "*(?:''|\"\")"
	);
}

if ( !support.cssHas ) {

	// Support: Chrome 105 - 110+, Safari 15.4 - 16.3+
	// In some browsers, `:has()` uses a forgiving selector list as an argument,
	// so our regular `try-catch` mechanism fails to catch `:has()` with arguments
	// not supported natively, like `:has(:contains("Foo"))`. The spec now requires
	// `:has()` parsing to be non-forgiving but browsers have not adjusted yet.
	rbuggyQSA.push( ":has" );
}

rbuggyQSA = rbuggyQSA.length && new RegExp( rbuggyQSA.join( "|" ) );

export default rbuggyQSA;


Review comment (bug):
```suggestion
	// Our regular `try-catch` mechanism fails to detect natively-unsupported
	// pseudo-classes inside `:has()` (such as `:has(:contains("Foo"))`)
	// in browsers that parse the `:has()` argument as a forgiving selector list.
	// https://drafts.csswg.org/selectors/#relational now requires the argument
	// to be parsed unforgivingly, but browsers have not yet fully adjusted.
```

Code after:
var rbuggyQSA = [];

if ( isIE ) {
	rbuggyQSA.push(

		// Support: IE 9 - 11+
		// IE's :disabled selector does not pick up the children of disabled fieldsets
		":enabled",
		":disabled",

		// Support: IE 11+
		// IE 11 doesn't find elements on a `[name='']` query in some cases.
		// Adding a temporary attribute to the document before the selection works
		// around the issue.
		"\\[" + whitespace + "*name" + whitespace + "*=" +
			whitespace + "*(?:''|\"\")"
	);
}

if ( !support.cssHas ) {

	// Support: Chrome 105 - 110+, Safari 15.4 - 16.3+
	// Our regular `try-catch` mechanism fails to detect natively-unsupported
	// pseudo-classes inside `:has()` (such as `:has(:contains("Foo"))`)
	// in browsers that parse the `:has()` argument as a forgiving selector list.
	// https://drafts.csswg.org/selectors/#relational now requires the argument
	// to be parsed unforgivingly, but browsers have not yet fully adjusted.
	rbuggyQSA.push( ":has" );
}

rbuggyQSA = rbuggyQSA.length && new RegExp( rbuggyQSA.join( "|" ) );

export default rbuggyQSA;
Repository: jquery/jquery
File: src/selector/support.js (JavaScript)

Code before:
import document from "../var/document.js";
import support from "../var/support.js";

// Support: Chrome 105 - 110+, Safari 15.4 - 16.3+
// Make sure forgiving mode is not used in `:has()`.
// `*` is needed as Safari & newer Chrome implemented something in between
// for `:has()` - it throws in `qSA` if it only contains an unsupported
// argument but multiple ones, one of which is supported, are fine.
// We want to play safe in case `:is()` gets the same treatment.
//
// Note that we don't need to detect the complete lack of support for `:has()`
// as then the `qSA` path will throw and fall back to jQuery traversal anyway.
try {
	document.querySelector( ":has(*,:jqfake)" );
	support.cssHas = false;
} catch ( e ) {
	support.cssHas = true;
}

export default support;


Review comment (bug):
```suggestion
// Make sure the the `:has()` argument is parsed unforgivingly.
// We include `*` in the test to detect buggy implementations that are
// _selectively_ forgiving (specifically when the list includes at least
// one valid selector).
// Note that we treat complete lack of support for `:has()` as if it were
// spec-compliant support, which is fine because use of `:has()` in such
// environments will fail in the qSA path and fall back to jQuery traversal
// anyway.
```

Code after:
import document from "../var/document.js";
import support from "../var/support.js";

// Support: Chrome 105 - 110+, Safari 15.4 - 16.3+
// Make sure the the `:has()` argument is parsed unforgivingly.
// We include `*` in the test to detect buggy implementations that are
// _selectively_ forgiving (specifically when the list includes at least
// one valid selector).
// Note that we treat complete lack of support for `:has()` as if it were
// spec-compliant support, which is fine because use of `:has()` in such
// environments will fail in the qSA path and fall back to jQuery traversal
// anyway.
try {
	document.querySelector( ":has(*,:jqfake)" );
	support.cssHas = false;
} catch ( e ) {
	support.cssHas = true;
}

export default support;
Repository: jquery/jquery
File: build/release/cdn.js (JavaScript)

Code before:
					"\"file\":\"" + unpathedFile.replace( /\.min\.map/, ".min.js\"" ) )
				.replace( /"sources":\["([^"]+)"\]/,
					"\"sources\":[\"" + unpathedFile.replace( /\.min\.map/, ".js" ) + "\"]" );
			fs.writeFileSync( releaseFile, text );
		} else if ( builtFile !== releaseFile ) {
			shell.cp( "-f", builtFile, releaseFile );
		}
	} );
}

function makeArchives( Release, callback ) {

	Release.chdir( Release.dir.repo );

	function makeArchive( cdn, files, callback ) {
		if ( Release.preRelease ) {
			console.log( "Skipping archive creation for " + cdn + "; this is a beta release." );
			callback();
			return;
		}

		console.log( "Creating production archive for " + cdn );

		var i,
			sum,
			result,
			archiver = require( "archiver" )( "zip" ),
			md5file = cdnFolder + "/" + cdn + "-md5.txt",
			output = fs.createWriteStream(
				cdnFolder + "/" + cdn + "-jquery-" + Release.newVersion + ".zip"
			),
			rmd5 = /[a-f0-9]{32}/,
			rver = /VER/;

		output.on( "close", callback );

		output.on( "error", function( err ) {
			throw err;
		} );

		archiver.pipe( output );

		files = files.map( function( item ) {
			return "dist" + ( rver.test( item ) ? "/cdn" : "" ) + "/" +
				item.replace( rver, Release.newVersion );
		} );

		if ( os.platform() === "win32" ) {
			sum = [];
			for ( i = 0; i < files.length; i++ ) {
				result = Release.exec(

Review comment (suggestion):
The style usually used in other places is to put all uninitialized variable declarations in one line:
```suggestion
		var i, sum, result,
```

Code after:
					"\"file\":\"" + unpathedFile.replace( /\.min\.map/, ".min.js\"" ) )
				.replace( /"sources":\["([^"]+)"\]/,
					"\"sources\":[\"" + unpathedFile.replace( /\.min\.map/, ".js" ) + "\"]" );
			fs.writeFileSync( releaseFile, text );
		} else if ( builtFile !== releaseFile ) {
			shell.cp( "-f", builtFile, releaseFile );
		}
	} );
}

function makeArchives( Release, callback ) {

	Release.chdir( Release.dir.repo );

	function makeArchive( cdn, files, callback ) {
		if ( Release.preRelease ) {
			console.log( "Skipping archive creation for " + cdn + "; this is a beta release." );
			callback();
			return;
		}

		console.log( "Creating production archive for " + cdn );

		var i, sum, result,
			archiver = require( "archiver" )( "zip" ),
			md5file = cdnFolder + "/" + cdn + "-md5.txt",
			output = fs.createWriteStream(
				cdnFolder + "/" + cdn + "-jquery-" + Release.newVersion + ".zip"
			),
			rmd5 = /[a-f0-9]{32}/,
			rver = /VER/;

		output.on( "close", callback );

		output.on( "error", function( err ) {
			throw err;
		} );

		archiver.pipe( output );

		files = files.map( function( item ) {
			return "dist" + ( rver.test( item ) ? "/cdn" : "" ) + "/" +
				item.replace( rver, Release.newVersion );
		} );

		if ( os.platform() === "win32" ) {
			sum = [];
			for ( i = 0; i < files.length; i++ ) {
				result = Release.exec(
					"certutil -hashfile " + files[ i ] + " MD5", "Error retrieving md5sum"
				);
Repository: jquery/jquery
File: build/tasks/compare_size.mjs (JavaScript)

Code before:
	return files;
}

function saveCache( loc, cache ) {
	return fs.promises.writeFile( loc, JSON.stringify( cache ) );
}

function compareSizes( existing, current, padLength ) {
	if ( typeof current !== "number" ) {
		return chalk.grey( `${existing}`.padStart( padLength ) );
	}
	const delta = current - existing;
	if ( delta > 0 ) {
		return chalk.red( `+${delta}`.padStart( padLength ) );
	}
	return chalk.green( `${delta}`.padStart( padLength ) );
}

function sortBranches( a, b ) {
	if ( a === lastRunBranch ) {
		return 1;
	}
	if ( b === lastRunBranch ) {
		return -1;
	}
	return a.localeCompare( b );
}

export async function compareSize( { cache = ".sizecache.json", files } = {} ) {
	if ( !files || !files.length ) {
		throw new Error( "No files specified" );
	}

	const branch = await getBranchName();
	const commit = await getCommitHash();
	const sizeCache = await getCache( cache );

	let rawPadLength = 0;
	let gzPadLength = 0;
	const results = await Promise.all(
		files.map( async function( filename ) {

			let contents = await fs.promises.readFile( filename, "utf8" );

			// Remove the short SHA and .dirty from comparisons.
			// The short SHA so commits can be compared against each other
			// and .dirty to compare with the existing branch during development.
			const sha = /jQuery v\d+.\d+.\d+(?:-\w+)?\+(?:slim.)?([^ \.]+(?:\.dirty)?)/.exec( contents )[ 1 ];
			contents = contents.replace( new RegExp( sha, "g" ), "" );

			const size = Buffer.byteLength( contents, "utf8" );

Review comment (suggestion):
`localeCompare` may not be stable across environments. Adding two more checks for `>` & `<` would be a bit safer.

Code after:
	return files;
}

function saveCache( loc, cache ) {
	return fs.promises.writeFile( loc, JSON.stringify( cache ) );
}

function compareSizes( existing, current, padLength ) {
	if ( typeof current !== "number" ) {
		return chalk.grey( `${existing}`.padStart( padLength ) );
	}
	const delta = current - existing;
	if ( delta > 0 ) {
		return chalk.red( `+${delta}`.padStart( padLength ) );
	}
	return chalk.green( `${delta}`.padStart( padLength ) );
}

function sortBranches( a, b ) {
	if ( a === lastRunBranch ) {
		return 1;
	}
	if ( b === lastRunBranch ) {
		return -1;
	}
	if ( a < b ) {
		return -1;
	}
	if ( a > b ) {
		return 1;
	}
	return 0;
}

export async function compareSize( { cache = ".sizecache.json", files } = {} ) {
	if ( !files || !files.length ) {
		throw new Error( "No files specified" );
	}

	const branch = await getBranchName();
	const commit = await getCommitHash();
	const sizeCache = await getCache( cache );

	let rawPadLength = 0;
	let gzPadLength = 0;
	const results = await Promise.all(
		files.map( async function( filename ) {

			let contents = await fs.promises.readFile( filename, "utf8" );

			// Remove the short SHA and .dirty from comparisons.
Repository: jquery/jquery
File: build/release/authors.js (JavaScript)

Code before:

function uniq( arr ) {
	const unique = [];
	for ( const item of arr ) {
		if ( ignore.some( re => re.test( item ) ) ) {
			continue;
		}
		if ( item && !unique.find( ( e ) => compareAuthors( e, item ) ) ) {
			unique.push( item );
		}
	}
	return unique;
}

function cleanupSizzle() {
	console.log( "Cleaning up..." );
	return exec( "npx rimraf .sizzle" );
}

function cloneSizzle() {
	console.log( "Cloning Sizzle..." );
	return exec( "git clone https://github.com/jquery/sizzle .sizzle" );
}

async function getLastAuthor() {
	const authorsTxt = await fs.promises.readFile( "AUTHORS.txt", "utf8" );
	return authorsTxt.trim().split( rnewline ).pop();
}

async function logAuthors( preCommand ) {
	let command = "git log --pretty=format:\"[%at] %aN <%aE>\"";
	if ( preCommand ) {
		command = preCommand + " && " + command;
	}
	const { stdout } = await exec( command );
	return uniq( stdout.trim().split( rnewline ).reverse() );
}

async function getSizzleAuthors() {
	await cloneSizzle();
	const authors = await logAuthors( "cd .sizzle" );
	await cleanupSizzle();
	return authors;
}

function sortAuthors( a, b ) {
	const [ , aDate ] = rdate.exec( a );
	const [ , bDate ] = rdate.exec( b );
	return parseInt( aDate ) - parseInt( bDate );
}


Review comment (suggestion):
`fs.promises` is deprecated, the new way to import it is via `node:fs/promises`. I guess the problem is the latter doesn't work on Node 10 but we are not going to run this task on our rusty old Jenkins, are we? Let's use the official API then.

Code after:

function uniq( arr ) {
	const unique = [];
	for ( const item of arr ) {
		if ( ignore.some( re => re.test( item ) ) ) {
			continue;
		}
		if ( item && !unique.find( ( e ) => compareAuthors( e, item ) ) ) {
			unique.push( item );
		}
	}
	return unique;
}

function cleanupSizzle() {
	console.log( "Cleaning up..." );
	return exec( "npx rimraf .sizzle" );
}

function cloneSizzle() {
	console.log( "Cloning Sizzle..." );
	return exec( "git clone https://github.com/jquery/sizzle .sizzle" );
}

async function getLastAuthor() {
	const authorsTxt = await fs.promises.readFile( "AUTHORS.txt", "utf8" );
	return authorsTxt.trim().split( rnewline ).pop();
}

async function logAuthors( preCommand ) {
	let command = "git log --pretty=format:\"[%at] %aN <%aE>\"";
	if ( preCommand ) {
		command = `${ preCommand } && ${ command }`;
	}
	const { stdout } = await exec( command );
	return uniq( stdout.trim().split( rnewline ).reverse() );
}

async function getSizzleAuthors() {
	await cloneSizzle();
	const authors = await logAuthors( "cd .sizzle" );
	await cleanupSizzle();
	return authors;
}

function sortAuthors( a, b ) {
	const [ , aDate ] = rdate.exec( a );
	const [ , bDate ] = rdate.exec( b );
	return Number( aDate ) - Number( bDate );
}
Repository: jquery/jquery
File: build/release/authors.js (JavaScript)

Code before:
		if ( item && !unique.find( ( e ) => compareAuthors( e, item ) ) ) {
			unique.push( item );
		}
	}
	return unique;
}

function cleanupSizzle() {
	console.log( "Cleaning up..." );
	return exec( "npx rimraf .sizzle" );
}

function cloneSizzle() {
	console.log( "Cloning Sizzle..." );
	return exec( "git clone https://github.com/jquery/sizzle .sizzle" );
}

async function getLastAuthor() {
	const authorsTxt = await fs.promises.readFile( "AUTHORS.txt", "utf8" );
	return authorsTxt.trim().split( rnewline ).pop();
}

async function logAuthors( preCommand ) {
	let command = "git log --pretty=format:\"[%at] %aN <%aE>\"";
	if ( preCommand ) {
		command = preCommand + " && " + command;
	}
	const { stdout } = await exec( command );
	return uniq( stdout.trim().split( rnewline ).reverse() );
}

async function getSizzleAuthors() {
	await cloneSizzle();
	const authors = await logAuthors( "cd .sizzle" );
	await cleanupSizzle();
	return authors;
}

function sortAuthors( a, b ) {
	const [ , aDate ] = rdate.exec( a );
	const [ , bDate ] = rdate.exec( b );
	return parseInt( aDate ) - parseInt( bDate );
}

function formatAuthor( author ) {
	return author.replace( rdate, "" );
}

async function getAuthors() {
	console.log( "Getting authors..." );
	const authors = await logAuthors();

Review comment (suggestion):
[Suggestion] I prefer template literals:
```suggestion
		command = `${ preCommand } && ${ command }`;
```

Code after:
		if ( item && !unique.find( ( e ) => compareAuthors( e, item ) ) ) {
			unique.push( item );
		}
	}
	return unique;
}

function cleanupSizzle() {
	console.log( "Cleaning up..." );
	return exec( "npx rimraf .sizzle" );
}

function cloneSizzle() {
	console.log( "Cloning Sizzle..." );
	return exec( "git clone https://github.com/jquery/sizzle .sizzle" );
}

async function getLastAuthor() {
	const authorsTxt = await fs.promises.readFile( "AUTHORS.txt", "utf8" );
	return authorsTxt.trim().split( rnewline ).pop();
}

async function logAuthors( preCommand ) {
	let command = "git log --pretty=format:\"[%at] %aN <%aE>\"";
	if ( preCommand ) {
		command = `${ preCommand } && ${ command }`;
	}
	const { stdout } = await exec( command );
	return uniq( stdout.trim().split( rnewline ).reverse() );
}

async function getSizzleAuthors() {
	await cloneSizzle();
	const authors = await logAuthors( "cd .sizzle" );
	await cleanupSizzle();
	return authors;
}

function sortAuthors( a, b ) {
	const [ , aDate ] = rdate.exec( a );
	const [ , bDate ] = rdate.exec( b );
	return Number( aDate ) - Number( bDate );
}

function formatAuthor( author ) {
	return author.replace( rdate, "" );
}

async function getAuthors() {
	console.log( "Getting authors..." );
	const authors = await logAuthors();