Merge pull request #290 from CodaFi/janitorial-work-ethic

[NFCI] Cosmetic Improvements

Author: CodaFi

Sources/SwiftCheck/Arbitrary.swift

@@ -64,13 +64,14 @@ extension Arbitrary {
 extension FixedWidthInteger {
 	/// Shrinks any `IntegerType`.
 	public var shrinkIntegral : [Self] {
-		return unfoldr({ i in
+		let start = self < 0 ? self.multipliedReportingOverflow(by: -1).partialValue : self
+		return [Self](sequence(state: start) { i in
 			if i <= 0 {
 				return .none
 			}
-			let n = i / 2
-			return .some((n, n))
-		}, initial: self < 0 ? self.multipliedReportingOverflow(by: -1).partialValue : self)
+			i = i / 2
+			return .some(i)
+		})
 	}
 }
 
@@ -92,13 +93,14 @@ extension Bool : Arbitrary {
 extension BinaryInteger {
 	/// Shrinks any `Numeric` type.
 	public var shrinkIntegral : [Self] {
-		return unfoldr({ i in
+		let start = self < 0 ? (self * -1) : self
+		return [Self](sequence(state: start) { i in
 			if i <= 0 {
 				return .none
 			}
-			let n = i / 2
-			return .some((n, n))
-		}, initial: self < 0 ? (self * -1) : self)
+			i = i / 2
+			return .some(i)
+		})
 	}
 }
 
@@ -301,7 +303,12 @@ extension UnicodeScalar : Arbitrary {
 	/// The default shrinking function for `UnicodeScalar` values.
 	public static func shrink(_ x : UnicodeScalar) -> [UnicodeScalar] {
 		let s : UnicodeScalar = UnicodeScalar(UInt32(tolower(Int32(x.value))))!
-		return [ "a", "b", "c", s, "A", "B", "C", "1", "2", "3", "\n", " " ].nub.filter { $0 < x }
+		let scalarSet = Set<UnicodeScalar>([ "a", "b", "c", "A", "B", "C", "1", "2", "3", "\n", " " ])
+		if scalarSet.contains(s) {
+			return [ "a", "b", "c", "A", "B", "C", "1", "2", "3", "\n", " " ].filter { $0 < x }
+		} else {
+			return [ "a", "b", "c", s, "A", "B", "C", "1", "2", "3", "\n", " " ].filter { $0 < x }
+		}
 	}
 }
 
@@ -363,25 +370,6 @@ extension Mirror : Arbitrary {
 	}
 }
 
-
-// MARK: - Implementation Details Follow
-
-extension Array where Element : Hashable {
-	fileprivate var nub : [Element] {
-		return [Element](Set(self))
-	}
-}
-
-private func unfoldr<A, B>(_ f : (B) -> Optional<(A, B)>, initial : B) -> [A] {
-	var acc = [A]()
-	var ini = initial
-	while let next = f(ini) {
-		acc.append(next.0)
-		ini = next.1
-	}
-	return acc.reversed()
-}
-
 #if os(Linux)
 	import Glibc
 #else

Sources/SwiftCheck/Lattice.swift

@@ -90,7 +90,7 @@ extension AnyIndex : LatticeType {
 	}
 }
 
-/// float.h does not export Float80's limits, nor does the Swift Standard Library.
+// float.h does not export Float80's limits, nor does the Swift Standard Library.
 // rdar://18404510
 //extension Swift.Float80 : LatticeType {
 //    public static var min : Swift.Float80 {

Sources/SwiftCheck/Modifiers.swift

@@ -359,11 +359,6 @@ extension NonNegative : CoArbitrary {
 
 // MARK: - Implementation Details Follow
 
-
-private func undefined<A>() -> A {
-	fatalError("")
-}
-
 fileprivate final class ArrowOfImpl<T : Hashable & CoArbitrary, U : Arbitrary> : Arbitrary, CustomStringConvertible {
 	fileprivate var table : Dictionary<T, U>
 	fileprivate var arr : (T) -> U
@@ -375,7 +370,9 @@ fileprivate final class ArrowOfImpl<T : Hashable & CoArbitrary, U : Arbitrary> :
 
 	convenience init(_ arr : @escaping (T) -> U) {
 		var table = [T:U]()
-		self.init(table, { (_ : T) -> U in return undefined() })
+		self.init(table, { (_ : T) -> U in
+			fatalError("Table forced before initialization completed; is something re-entrant?")
+		})
 
 		self.arr = { x in
 			if let v = table[x] {
@@ -425,7 +422,11 @@ fileprivate final class IsoOfImpl<T : Hashable & CoArbitrary & Arbitrary, U : Eq
 
 	convenience init(_ embed : @escaping (T) -> U, _ project : @escaping (U) -> T) {
 		var table = [T:U]()
-		self.init(table, { (_ : T) -> U in return undefined() }, { (_ : U) -> T in return undefined() })
+		self.init(table, { (_ : T) -> U in
+			fatalError("Table forced before initialization completed; is something re-entrant?")
+		}, { (_ : U) -> T in
+			fatalError("Table forced before initialization completed; is something re-entrant?")
+		})
 
 		self.embed = { t in
 			if let v = table[t] {

Sources/SwiftCheck/Property.swift

@@ -671,6 +671,18 @@ private func printLabels(_ st : TestResult) {
 	}
 }
 
+/// Takes a sequence of trees of test results from several properties into
+/// a single labelled rose tree for the conjunction.
+///
+/// This function is written in continuation passsing style to facilitate the
+/// construction of a lazy rose tree that preserves the labels and callbacks
+/// associated with the sub-properties.
+///
+/// The general idea is that the properties are tested from first to last.  If
+/// any of them fails, the conjunct need not evaluate further and returns that
+/// first failing result.  Else, each passing sub-property continues on to the
+/// next sub-property and decorates the overall result with its callbacks and
+/// labels.  Discards act to skip that sub-property, as expected.
 private func conj(_ k : @escaping (TestResult) -> TestResult, xs : [Rose<TestResult>]) -> Rose<TestResult> {
 	guard let p = xs.first else {
 		return Rose.mkRose({ k(TestResult.succeeded) }, { [] })
@@ -679,16 +691,20 @@ private func conj(_ k : @escaping (TestResult) -> TestResult, xs : [Rose<TestRes
 		let rose = p.reduce
 		switch rose {
 		case .mkRose(let result, _):
-			if !result().expect {
+			guard result().expect else {
 				return Rose.pure(TestResult.failed("expectFailure may not occur inside a conjunction"))
 			}
 
 			switch result().ok {
 			case .some(true):
+				// Passed - Adjoin the labels and test the next property.
 				return conj(comp(addLabels(result()), comp(addCallbacks(result()), k)), xs: [Rose<TestResult>](xs[1..<xs.endIndex]))
 			case .some(false):
+				// Failed - Return the first failure and don't continue
+				// evaluating the other conjuncts.
 				return rose
 			case .none:
+				// Discard - Try the next property.
 				let rose2 = conj(comp(addCallbacks(result()), k), xs: [Rose<TestResult>](xs[1..<xs.endIndex])).reduce
 				switch rose2 {
 				case .mkRose(let result2, _):
@@ -711,23 +727,28 @@ private func conj(_ k : @escaping (TestResult) -> TestResult, xs : [Rose<TestRes
 	}))
 }
 
+/// Computes the disjunction of two test results.
 private func disj(_ p : Rose<TestResult>, q : Rose<TestResult>) -> Rose<TestResult> {
 	return p.flatMap { result1 in
-		if !result1.expect {
+		guard result1.expect else {
 			return Rose<TestResult>.pure(TestResult.failed("expectFailure may not occur inside a disjunction"))
 		}
 		switch result1.ok {
 		case .some(true):
+			// Passed - Don't evaluate the other tree.
 			return Rose<TestResult>.pure(result1)
 		case .some(false):
-			return q.flatMap { (result2 : TestResult) in
-				if !result2.expect {
+			// Failed - Try the other tree.
+			return q.flatMap { result2 in
+				guard result2.expect else {
 					return Rose<TestResult>.pure(TestResult.failed("expectFailure may not occur inside a disjunction"))
 				}
 				switch result2.ok {
 				case .some(true):
 					return Rose<TestResult>.pure(result2)
 				case .some(false):
+					// Failed - Combine the exceptions and callbacks from each
+					// sub-property.
 					let callbacks : [Callback] = [.afterFinalFailure(kind: .counterexample, { (_, _) in return print("") })]
 					return Rose<TestResult>.pure(TestResult(
 						ok:            .some(false),
@@ -745,8 +766,9 @@ private func disj(_ p : Rose<TestResult>, q : Rose<TestResult>) -> Rose<TestResu
 				}
 			}
 		case .none:
-			return q.flatMap { (result2 : TestResult) in
-				if !result2.expect {
+			// Discarded - Try the other property.
+			return q.flatMap { result2 in
+				guard result2.expect else {
 					return Rose<TestResult>.pure(TestResult.failed("expectFailure may not occur inside a disjunction"))
 				}
 				switch result2.ok {

Sources/SwiftCheck/Random.swift

@@ -338,7 +338,8 @@ extension Double : RandomType {
 	}
 }
 
-/// Implementation Details Follow
+// MARK: - Implementation Details
+
 private enum ClockTimeResult {
 	case success
 	case failure(Int)

Sources/SwiftCheck/Test.swift

@@ -535,7 +535,7 @@ public func exists<A : Arbitrary>(_ gen : Gen<A>, pf : @escaping (A) throws -> T
 
 // MARK: - Implementation Details
 
-internal enum Result {
+internal enum QuickCheckResult {
 	case success(
 		numTests  : Int,
 		labels    : [(String, Int)],
@@ -585,7 +585,7 @@ private indirect enum Either<L, R> {
 	case right(R)
 }
 
-internal func quickCheckWithResult(_ args : CheckerArguments, _ p : Testable) -> Result {
+internal func quickCheckWithResult(_ args : CheckerArguments, _ p : Testable) -> QuickCheckResult {
 	let istate = CheckerState(
 		name:                         args.name,
 		maxAllowableSuccessfulTests:  args.maxAllowableSuccessfulTests,
@@ -621,7 +621,7 @@ internal func quickCheckWithResult(_ args : CheckerArguments, _ p : Testable) ->
 // - giveUp: When the number of discarded tests exceeds the number given in the
 //           arguments we just give up turning the run loop to prevent excessive
 //           generation.
-private func test(_ st : CheckerState, caseGen : (StdGen, Int) -> Prop) -> Result {
+private func test(_ st : CheckerState, caseGen : (StdGen, Int) -> Prop) -> QuickCheckResult {
 	var state = st
 	while true {
 		switch runATest(state, caseGen: caseGen) {
@@ -631,9 +631,9 @@ private func test(_ st : CheckerState, caseGen : (StdGen, Int) -> Prop) -> Resul
 				return fail.0
 			case let (_, .noExpectedFailure(numTests, seed, sz, labels, output)):
 				return .noExpectedFailure(numTests: numTests, usedSeed: seed, usedSize: sz, labels: labels, output: output)
-				// Existential Failures need explicit propagation.  Existentials increment the
-				// discard count so we check if it has been surpassed.  If it has with any kind
-				// of success we're done.  If no successes are found we've failed checking the
+			// Existential Failures need explicit propagation.  Existentials increment the
+			// discard count so we check if it has been surpassed.  If it has with any kind
+			// of success we're done.  If no successes are found we've failed checking the
 			// existential and report it as such.  Otherwise turn the testing loop.
 			case (.existentialFailure(_, _, _, _, _, _, _), _):
 				if fail.1.successfulTestCount == 0 || fail.1.discardedTestCount >= fail.1.maxAllowableDiscardedTests {
@@ -661,7 +661,7 @@ private func test(_ st : CheckerState, caseGen : (StdGen, Int) -> Prop) -> Resul
 //
 // On success the next state is returned.  On failure the final result and state
 // are returned.
-private func runATest(_ st : CheckerState, caseGen : (StdGen, Int) -> Prop) -> Either<(Result, CheckerState), CheckerState> {
+private func runATest(_ st : CheckerState, caseGen : (StdGen, Int) -> Prop) -> Either<(QuickCheckResult, CheckerState), CheckerState> {
 	let size = st.computeSize(st.successfulTestCount, st.discardedTestCount)
 	let (rnd1, rnd2) = st.randomSeedGenerator.split
 
@@ -751,7 +751,7 @@ private func runATest(_ st : CheckerState, caseGen : (StdGen, Int) -> Prop) -> E
 
 				/// However, some existentials outlive their usefulness
 				if nstate.discardedTestCount >= nstate.maxAllowableDiscardedTests {
-					let resul = Result.existentialFailure(
+					let resul = QuickCheckResult.existentialFailure(
 						numTests:    (st.successfulTestCount + 1),
 						usedSeed:    st.randomSeedGenerator,
 						usedSize:    st.computeSize(st.successfulTestCount, st.discardedTestCount),
@@ -768,12 +768,12 @@ private func runATest(_ st : CheckerState, caseGen : (StdGen, Int) -> Prop) -> E
 			// Attempt a shrink.
 			let (numShrinks, _, _) = findMinimalFailingTestCase(st, res: res, ts: ts())
 
-			if !expect {
-				let s = Result.success(numTests: (st.successfulTestCount + 1), labels: summary(st), output: "+++ OK, failed as expected. ")
+			guard expect else {
+				let s = QuickCheckResult.success(numTests: (st.successfulTestCount + 1), labels: summary(st), output: "+++ OK, failed as expected. ")
 				return .left((s, st))
 			}
 
-			let stat = Result.failure(
+			let stat = QuickCheckResult.failure(
 				numTests:    (st.successfulTestCount + 1),
 				numShrinks:  numShrinks,
 				usedSeed:    st.randomSeedGenerator,
@@ -809,7 +809,7 @@ private func runATest(_ st : CheckerState, caseGen : (StdGen, Int) -> Prop) -> E
 	}
 }
 
-private func doneTesting(_ st : CheckerState) -> Result {
+private func doneTesting(_ st : CheckerState) -> QuickCheckResult {
 	if !st.hasFulfilledExpectedFailure {
 		if insufficientCoverage(st) {
 			printCond(st.silence, "+++ OK, failed as expected. ")
@@ -843,7 +843,7 @@ private func doneTesting(_ st : CheckerState) -> Result {
 	}
 }
 
-private func giveUp(_ st : CheckerState) -> Result {
+private func giveUp(_ st : CheckerState) -> QuickCheckResult {
 	printDistributionGraph(st)
 	return .gaveUp(numTests: st.successfulTestCount, labels: summary(st), output: "")
 }
@@ -944,7 +944,7 @@ private func reportMinimumCaseFound(_ st : CheckerState, res : TestResult) -> (I
 	return (st.successfulShrinkCount, st.failedShrinkStepCount - st.failedShrinkStepDistance, st.failedShrinkStepDistance)
 }
 
-private func reportExistentialFailure(_ st : CheckerState, res : Result) -> Result {
+private func reportExistentialFailure(_ st : CheckerState, res : QuickCheckResult) -> QuickCheckResult {
 	switch res {
 	case let .existentialFailure(_, _, _, reason, _, _, lastTest):
 		let testMsg = " (after \(st.discardedTestCount) test"

Sources/SwiftCheck/WitnessedArbitrary.swift

@@ -100,7 +100,7 @@ extension Dictionary : Arbitrary where Key : Arbitrary, Value : Arbitrary {
 	public static var arbitrary : Gen<Dictionary<Key, Value>> {
 		return [Key].arbitrary.flatMap { (k : [Key]) in
 			return [Value].arbitrary.flatMap { (v : [Value]) in
-				return Gen.pure(Dictionary(zip(k, v)))
+				return Gen.pure(Dictionary(zip(k, v)) { $1 })
 			}
 		}
 	}
@@ -173,7 +173,7 @@ extension Set : Arbitrary where Element : Arbitrary {
 	}
 }
 
-// MARK: - Implementation Details Follow
+// MARK: - Implementation Details
 
 private func removes<A : Arbitrary>(_ k : Int, n : Int, xs : [A]) -> [[A]] {
 	let xs2 : [A] = Array(xs.suffix(max(0, xs.count - k)))
@@ -189,25 +189,13 @@ private func removes<A : Arbitrary>(_ k : Int, n : Int, xs : [A]) -> [[A]] {
 }
 
 private func shrinkOne<A : Arbitrary>(_ xs : [A]) -> [[A]] {
-	if xs.isEmpty {
+	guard let x : A = xs.first else {
 		return [[A]]()
-	} else if let x : A = xs.first {
-		let xss = [A](xs[1..<xs.endIndex])
-		let a : [[A]] = A.shrink(x).map({ [$0] + xss })
-		let b : [[A]] = shrinkOne(xss).map({ [x] + $0 })
-		return a + b
 	}
-	fatalError("Array could not produce a first element")
-}
 
-extension Dictionary {
-	fileprivate init<S : Sequence>(_ pairs : S)
-		where S.Iterator.Element == (Key, Value)
-	{
-		self.init()
-		var g = pairs.makeIterator()
-		while let (k, v): (Key, Value) = g.next() {
-			self[k] = v
-		}
-	}
+	let xss = [A](xs[1..<xs.endIndex])
+	let a : [[A]] = A.shrink(x).map({ [$0] + xss })
+	let b : [[A]] = shrinkOne(xss).map({ [x] + $0 })
+	return a + b
 }
+