Merge pull request #130 from HeMet/master

Fix NSArray.indexOfObject(inSortedRange:options:usingComparator) for fringe cases

Author: phausler

Foundation/NSArray.swift

@@ -412,22 +412,44 @@ public class NSArray : NSObject, NSCopying, NSMutableCopying, NSSecureCoding, NS
     }
 
     public func indexOfObject(obj: AnyObject, inSortedRange r: NSRange, options opts: NSBinarySearchingOptions, usingComparator cmp: NSComparator) -> Int {
-        guard (r.location + r.length) <= count else {
-            NSInvalidArgument("range \(r) extends beyond bounds [0 .. \(count - 1)]")
+        let lastIndex = r.location + r.length - 1
+        
+        // argument validation
+        guard lastIndex < count else {
+            let bounds = count == 0 ? "for empty array" : "[0 .. \(count - 1)]"
+            NSInvalidArgument("range \(r) extends beyond bounds \(bounds)")
         }
 
         if opts.contains(.FirstEqual) && opts.contains(.LastEqual) {
             NSInvalidArgument("both NSBinarySearching.FirstEqual and NSBinarySearching.LastEqual options cannot be specified")
         }
 
+        let searchForInsertionIndex = opts.contains(.InsertionIndex)
+        
+        // fringe cases
+        if r.length == 0 {
+            return  searchForInsertionIndex ? r.location : NSNotFound
+        }
+        
+        let leastObj = objectAtIndex(r.location)
+        if cmp(obj, leastObj) == .OrderedAscending {
+            return searchForInsertionIndex ? r.location : NSNotFound
+        }
+        
+        let greatestObj = objectAtIndex(lastIndex)
+        if cmp(obj, greatestObj) == .OrderedDescending {
+            return searchForInsertionIndex ? lastIndex + 1 : NSNotFound
+        }
+        
+        // common processing
         let firstEqual = opts.contains(.FirstEqual)
         let lastEqual = opts.contains(.LastEqual)
         let anyEqual = !(firstEqual || lastEqual)
 
         var result = NSNotFound
         var indexOfLeastGreaterThanObj = NSNotFound
         var start = r.location
-        var end = r.location + r.length - 1
+        var end = lastIndex
 
         loop: while start <= end {
             let middle = start + (end - start) / 2
@@ -459,18 +481,14 @@ public class NSArray : NSObject, NSCopying, NSMutableCopying, NSSecureCoding, NS
             }
         }
 
-        guard opts.contains(.InsertionIndex) && lastEqual else {
+        guard searchForInsertionIndex && lastEqual else {
             return result
         }
 
         guard result == NSNotFound else {
             return result + 1
         }
 
-        guard indexOfLeastGreaterThanObj != NSNotFound else {
-            return count
-        }
-        
         return indexOfLeastGreaterThanObj
     }
 

Foundation/NSObjCRuntime.swift

@@ -80,7 +80,7 @@ public let NSNotFound: Int = Int.max
     fatalError("\(fn) is not yet implemented")
 }
 
-@noreturn func NSInvalidArgument(message: String, method: String = __FUNCTION__) {
+@noreturn internal func NSInvalidArgument(message: String, method: String = __FUNCTION__) {
     fatalError("\(method): \(message)")
 }
 

TestFoundation/TestNSArray.swift

@@ -29,6 +29,7 @@ class TestNSArray : XCTestCase {
             ("test_getObjects", test_getObjects),
             ("test_objectAtIndex", test_objectAtIndex),
             ("test_binarySearch", test_binarySearch),
+            ("test_binarySearchFringeCases", test_binarySearchFringeCases),
             ("test_replaceObjectsInRange_withObjectsFromArray", test_replaceObjectsInRange_withObjectsFromArray),
             ("test_replaceObjectsInRange_withObjectsFromArray_range", test_replaceObjectsInRange_withObjectsFromArray_range),
         ]
@@ -140,8 +141,42 @@ class TestNSArray : XCTestCase {
         let indexOfLeastGreaterObjectThanFive = objectIndexInArray(array, value: 5, startingFrom: 0, length: 10, options: [.InsertionIndex, .LastEqual])
         XCTAssertTrue(indexOfLeastGreaterObjectThanFive == 7, "If both .InsertionIndex and .LastEqual are specified NSArray returns the index of the least greater object...")
 
-        let endOfArray = objectIndexInArray(array, value: 10, startingFrom: 0, length: 13, options: [.InsertionIndex, .LastEqual])
-        XCTAssertTrue(endOfArray == array.count, "...or the index at the end of the array if the object is larger than all other elements.")
+        let rangeStart = 0
+        let rangeLength = 13
+        let endOfArray = objectIndexInArray(array, value: 10, startingFrom: rangeStart, length: rangeLength, options: [.InsertionIndex, .LastEqual])
+        XCTAssertTrue(endOfArray == (rangeStart + rangeLength), "...or the index at the end of the array if the object is larger than all other elements.")
+    }
+    
+    func test_binarySearchFringeCases() {
+        let array = NSArray(array: [
+            NSNumber(int: 0), NSNumber(int: 1), NSNumber(int: 2), NSNumber(int: 2), NSNumber(int: 3),
+            NSNumber(int: 4), NSNumber(int: 4), NSNumber(int: 6), NSNumber(int: 7), NSNumber(int: 7),
+            NSNumber(int: 7), NSNumber(int: 8), NSNumber(int: 9), NSNumber(int: 9)])
+        
+        let emptyArray = NSArray()
+//        Same as for non empty NSArray but error message ends with 'bounds for empty array'.
+//        let _ = objectIndexInArray(emptyArray, value: 0, startingFrom: 0, length: 1)
+        
+        let notFoundInEmptyArray = objectIndexInArray(emptyArray, value: 9, startingFrom: 0, length: 0)
+        XCTAssertEqual(notFoundInEmptyArray, NSNotFound, "Empty NSArray return NSNotFound for any valid arguments.")
+        
+        let startIndex = objectIndexInArray(emptyArray, value: 7, startingFrom: 0, length: 0, options: [.InsertionIndex])
+        XCTAssertTrue(startIndex == 0, "For Empty NSArray any objects should be inserted at start.")
+        
+        let rangeStart = 0
+        let rangeLength = 13
+        
+        let leastSearch = objectIndexInArray(array, value: -1, startingFrom: rangeStart, length: rangeLength)
+        XCTAssertTrue(leastSearch == NSNotFound, "If object is less than least object in the range then there is no change it could be found.")
+        
+        let greatestSearch = objectIndexInArray(array, value: 15, startingFrom: rangeStart, length: rangeLength)
+        XCTAssertTrue(greatestSearch == NSNotFound, "If object is greater than greatest object in the range then there is no change it could be found.")
+        
+        let leastInsert = objectIndexInArray(array, value: -1, startingFrom: rangeStart, length: rangeLength, options: .InsertionIndex)
+        XCTAssertTrue(leastInsert == rangeStart, "If object is less than least object in the range it should be inserted at range' location.")
+        
+        let greatestInsert = objectIndexInArray(array, value: 15, startingFrom: rangeStart, length: rangeLength, options: .InsertionIndex)
+        XCTAssertTrue(greatestInsert == (rangeStart + rangeLength), "If object is greater than greatest object in the range it should be inserted at range' end.")
     }
 
     func objectIndexInArray(array: NSArray, value: Int, startingFrom: Int, length: Int, options: NSBinarySearchingOptions = []) -> Int {