Core: Fill in & warn against $.fn.{push,sort,splice}

Fixes gh-473
Closes gh-529

Author: mgol

src/jquery/core.js

@@ -8,6 +8,10 @@ import {
 import "../disablePatches.js";
 
 var findProp,
+	arr = [],
+	push = arr.push,
+	sort = arr.sort,
+	splice = arr.splice,
 	class2type = {},
 	oldInit = jQuery.fn.init,
 	oldFind = jQuery.find,
@@ -177,3 +181,15 @@ if ( jQueryVersionSince( "3.3.0" ) ) {
 		"jQuery.isWindow() is deprecated"
 	);
 }
+
+if ( jQueryVersionSince( "4.0.0" ) ) {
+
+	// `push`, `sort` & `splice` are used internally by jQuery <4, so we only
+	// warn in jQuery 4+.
+	migrateWarnProp( jQuery.fn, "push", push, "push",
+		"jQuery.fn.push() is deprecated and removed; use .add or convert to an array" );
+	migrateWarnProp( jQuery.fn, "sort", sort, "sort",
+		"jQuery.fn.sort() is deprecated and removed; convert to an array before sorting" );
+	migrateWarnProp( jQuery.fn, "splice", splice, "splice",
+		"jQuery.fn.splice() is deprecated and removed; use .slice or .not with .eq" );
+}

test/unit/jquery/core.js

@@ -1,6 +1,12 @@
 
 QUnit.module( "core" );
 
+function getTagNames( elem ) {
+	return elem.toArray().map( function( node ) {
+		return node.tagName.toLowerCase();
+	} );
+}
+
 QUnit.test( "jQuery(html, props)", function( assert ) {
 	assert.expect( 2 );
 
@@ -449,3 +455,53 @@ TestManager.runIframeTest( "old pre-3.0 jQuery", "core-jquery2.html",
 
 		assert.ok( /jQuery 3/.test( log ), "logged: " + log );
 } );
+
+QUnit[ jQueryVersionSince( "4.0.0" ) ? "test" : "skip" ]( "jQuery.fn.push", function( assert ) {
+	assert.expect( 2 );
+
+	expectWarning( assert, "jQuery.fn.push", 1, function() {
+		var node = jQuery( "<div></div>" )[ 0 ],
+			elem = jQuery( "<p></p><span></span>" );
+
+		elem.push( node );
+
+		assert.deepEqual( getTagNames( elem ), [ "p", "span", "div" ],
+			"div added in-place" );
+	} );
+} );
+
+QUnit[ jQueryVersionSince( "4.0.0" ) ? "test" : "skip" ]( "jQuery.fn.sort", function( assert ) {
+	assert.expect( 2 );
+
+	expectWarning( assert, "jQuery.fn.sort", 1, function() {
+		var elem = jQuery( "<span></span><div></div><p></p>" );
+
+		elem.sort( function( node1, node2 ) {
+			const tag1 = node1.tagName.toLowerCase();
+			const tag2 = node2.tagName.toLowerCase();
+			if ( tag1 < tag2 ) {
+				return -1;
+			}
+			if ( tag1 > tag2 ) {
+				return 1;
+			}
+			return 0;
+		} );
+
+		assert.deepEqual( getTagNames( elem ), [ "div", "p", "span" ],
+			"element sorted in-place" );
+	} );
+} );
+
+QUnit[ jQueryVersionSince( "4.0.0" ) ? "test" : "skip" ]( "jQuery.fn.splice", function( assert ) {
+	assert.expect( 2 );
+
+	expectWarning( assert, "jQuery.fn.splice", 1, function() {
+		var elem = jQuery( "<span></span><div></div><p></p>" );
+
+		elem.splice( 1, 1, jQuery( "<i></i>" )[ 0 ], jQuery( "<b></b>" )[ 0 ] );
+
+		assert.deepEqual( getTagNames( elem ), [ "span", "i", "b", "p" ],
+			"splice removed & added in-place" );
+	} );
+} );

warnings.md

@@ -178,6 +178,14 @@ See jQuery-ui [commit](https://github.com/jquery/jquery-ui/commit/c0093b599fcd58
 
 **Solution**: Review code that uses `jQuery.type()` and use a type check that is appropriate for the situation. For example. if the code expects a plain function, check for `typeof arg === "function"`.
 
+### \[push\] JQMIGRATE: jQuery.fn.push() is deprecated and removed; use .add or convert to an array
+### \[sort\] JQMIGRATE: jQuery.fn.sort() is deprecated and removed; convert to an array before sorting
+### \[splice\] JQMIGRATE: jQuery.fn.splice() is deprecated and removed; use .slice or .not with .eq
+
+**Cause**: jQuery used to add the Array `push`, `sort` & `splice` methods to the jQuery prototype. They behaved differently to other jQuery APIs - they modify the jQuery collections in place, they don't play nice with APIs like `.end()`, they were also never documented.
+
+**Solution**: Replace `.push( node )` with `.add( node )`, `.splice( index )` with `.not( elem.eq( index ) )`. In more complex cases, call `.toArray()` first, manipulate the resulting array and convert back to the jQuery object by passing the resulting array to `$()`.
+
 ### \[unique\] JQMIGRATE: jQuery.unique is deprecated; use jQuery.uniqueSort
 
 **Cause**: The fact that `jQuery.unique` sorted its results in DOM order was surprising to many who did not read the documentation carefully. As of jQuery 3.0 this function is being renamed to make it clear.