ronantakizawa/codereview-bench huggingface dataset:
Repository: vapor/vapor
File: Sources/Vapor/Engine/Responder.swift (Swift)

Code before:
/// MARK: Route

/// A basic, closure-based responder.
public struct RouteResponder<T>: Responder
    where T: ResponseEncodable
{
    /// Responder closure
    public typealias Closure = (Request) throws -> T

    /// The stored responder closure.
    public let closure: Closure

    /// Create a new basic responder.
    public init(closure: @escaping Closure) {
        self.closure = closure
    }

    /// See: HTTP.Responder.respond
    public func respond(to req: Request) throws -> Future<Response> {
        let encodable = try closure(req)
        return try encodable.encode(for: req)
    }
}

/// A basic, closure-based responder.
public struct ContentRouteResponder<C, T>: Responder
    where C: RequestDecodable, T: ResponseEncodable
{
    /// Responder closure
    public typealias Closure = (Request, C) throws -> T
    
    /// The stored responder closure.
    public let closure: Closure
    
    /// Create a new basic responder.
    public init(closure: @escaping Closure) {
        self.closure = closure
    }
    
    /// See: HTTP.Responder.respond
    public func respond(to req: Request) throws -> Future<Response> {
        return try C.decode(from: req).flatMap(to: Response.self) { content in
            let encodable = try self.closure(req, content)
            return try encodable.encode(for: req)
        }
    }
}

/// MARK: Router

/// Converts a router into a responder.

Review comment (suggestion):
this should probably be `RequestDecodableResponder` as well

Code after:
/// MARK: Route

/// A basic, closure-based responder.
public struct RouteResponder<T>: Responder
    where T: ResponseEncodable
{
    /// Responder closure
    public typealias Closure = (Request) throws -> T

    /// The stored responder closure.
    public let closure: Closure

    /// Create a new basic responder.
    public init(closure: @escaping Closure) {
        self.closure = closure
    }

    /// See: HTTP.Responder.respond
    public func respond(to req: Request) throws -> Future<Response> {
        let encodable = try closure(req)
        return try encodable.encode(for: req)
    }
}

/// A basic, closure-based responder.
public struct RequestDecodableResponder<C, T>: Responder
    where C: RequestDecodable, T: ResponseEncodable
{
    /// Responder closure
    public typealias Closure = (Request, C) throws -> T
    
    /// The stored responder closure.
    public let closure: Closure
    
    /// Create a new basic responder.
    public init(closure: @escaping Closure) {
        self.closure = closure
    }
    
    /// See: HTTP.Responder.respond
    public func respond(to req: Request) throws -> Future<Response> {
        return try C.decode(from: req).flatMap(to: Response.self) { content in
            let encodable = try self.closure(req, content)
            return try encodable.encode(for: req)
        }
    }
}

/// MARK: Router

/// Converts a router into a responder.
Repository: vapor/vapor
File: Sources/Validation/Validatable.swift (Swift)

Code before:
            do {
                try validation.validate(data)
            } catch var error as ValidationError {
                error.codingPath += key.codingPath
                errors.append(error)
            }
        }

        if !errors.isEmpty {
            throw ValidatableError(errors)
        }
    }
}

/// a collection of errors thrown by a validatable
/// models validations
struct ValidatableError: ValidationError {
    /// the errors thrown
    var errors: [ValidationError]

    /// See ValidationError.keyPath
    var codingPath: [CodingKey]

    /// See ValidationError.reason
    var reason: String {
        return errors.map { err in
            var error = err
            error.codingPath = codingPath + err.codingPath
            return error.reason
        }.joined(separator: ", ")
    }

    /// creates a new validatable error
    public init(_ errors: [ValidationError]) {
        self.errors = errors
        self.codingPath = []
    }
}


Review comment (suggestion):
Maybe we can go a consistent way here with a prefix `mutable` and do it in all the places as well without the *bang*. Like for this:
```swift
var reason: String {
    return errors.map { error in
        var mutableError = error
        mutableError.codingPath = codingPath + error.codingPath
        return mutableError.reason
    }.joined(separator: ", ")
}
```

Code after:
            do {
                try validation.validate(data)
            } catch var error as ValidationError {
                error.codingPath += key.codingPath
                errors.append(error)
            }
        }

        if !errors.isEmpty {
            throw ValidatableError(errors)
        }
    }
}

/// a collection of errors thrown by a validatable
/// models validations
struct ValidatableError: ValidationError {
    /// the errors thrown
    var errors: [ValidationError]

    /// See ValidationError.keyPath
    var codingPath: [CodingKey]

    /// See ValidationError.reason
    var reason: String {
        return errors.map { error in
            var mutableError = error
            mutableError.codingPath = codingPath + error.codingPath
            return mutableError.reason
        }.joined(separator: ", ")
    }

    /// creates a new validatable error
    public init(_ errors: [ValidationError]) {
        self.errors = errors
        self.codingPath = []
    }
}
Repository: vapor/vapor
File: Sources/Vapor/Response/TypedDataResponse.swift (Swift)

Code before:
/// A container representing `Data` with a specific content type (used for returning as a `Response`).
public struct TypedDataResponse: ResponseEncodable {
    /// The data held by this container.
    let data: Data
    /// The type of the data held by this container.
    let contentType: MediaType
    
    /// See `ResponseEncodable`.
    public func encode(for req: Request) throws -> EventLoopFuture<Response> {
        let res = req.makeResponse()
        res.http.body = data.convertToHTTPBody()
        res.http.contentType = contentType
        return try res.encode(for: req)
    }
}

extension Data: ResponseEncodable {
    /// See `ResponseEncodable`.
    public func encode(for req: Request) throws -> EventLoopFuture<Response> {
        return try response(type: .any).encode(for: req)
    }
    
    /**
     Get a `ResponseEncodable` container holding this data with your provided contentType.
     - parameter contentType: The type of data to return the container with.
     */
    public func response(type: MediaType) -> TypedDataResponse {

Review comment (suggestion):
We should make this type private and just publicize `ResponseEncodable`. 

Code after:
/// A container representing `Data` with a specific content type (used for returning as a `Response`).
private struct TypedDataResponse: ResponseEncodable {
    /// The data held by this container.
    let data: Data

    /// The type of the data held by this container.
    let contentType: MediaType
    
    /// See `ResponseEncodable`.
    public func encode(for req: Request) throws -> EventLoopFuture<Response> {
        let res = req.makeResponse()
        res.http.body = data.convertToHTTPBody()
        res.http.contentType = contentType
        return try res.encode(for: req)
    }
}

extension Data: ResponseEncodable {
    /// See `ResponseEncodable`.
    public func encode(for req: Request) throws -> EventLoopFuture<Response> {
        return try response(type: .any).encode(for: req)
    }
    
    /// Get a `ResponseEncodable` container holding this data with your provided contentType.
    ///
    ///     data.response(type: .html)
    ///
Repository: vapor/vapor
File: Sources/Vapor/Response/TypedDataResponse.swift (Swift)

Code before:
public struct TypedDataResponse: ResponseEncodable {
    /// The data held by this container.
    let data: Data
    /// The type of the data held by this container.
    let contentType: MediaType
    
    /// See `ResponseEncodable`.
    public func encode(for req: Request) throws -> EventLoopFuture<Response> {
        let res = req.makeResponse()
        res.http.body = data.convertToHTTPBody()
        res.http.contentType = contentType
        return try res.encode(for: req)
    }
}

extension Data: ResponseEncodable {
    /// See `ResponseEncodable`.
    public func encode(for req: Request) throws -> EventLoopFuture<Response> {
        return try response(type: .any).encode(for: req)
    }
    
    /**
     Get a `ResponseEncodable` container holding this data with your provided contentType.
     - parameter contentType: The type of data to return the container with.
     */
    public func response(type: MediaType) -> TypedDataResponse {
        return TypedDataResponse(data: self, contentType: type)
    }
}


Review comment (suggestion):
Here we should be able to just return `ResponseEncodable`.

Code after:
private struct TypedDataResponse: ResponseEncodable {
    /// The data held by this container.
    let data: Data

    /// The type of the data held by this container.
    let contentType: MediaType
    
    /// See `ResponseEncodable`.
    public func encode(for req: Request) throws -> EventLoopFuture<Response> {
        let res = req.makeResponse()
        res.http.body = data.convertToHTTPBody()
        res.http.contentType = contentType
        return try res.encode(for: req)
    }
}

extension Data: ResponseEncodable {
    /// See `ResponseEncodable`.
    public func encode(for req: Request) throws -> EventLoopFuture<Response> {
        return try response(type: .any).encode(for: req)
    }
    
    /// Get a `ResponseEncodable` container holding this data with your provided contentType.
    ///
    ///     data.response(type: .html)
    ///
    /// - parameters:
    ///     - type: The type of data to return the container with.
    public func response(type: MediaType) -> ResponseEncodable {
        return TypedDataResponse(data: self, contentType: type)
    }
}
Repository: vapor/vapor
File: Sources/Vapor/Response/TypedDataResponse.swift (Swift)

Code before:
/// A container representing `Data` with a specific content type (used for returning as a `Response`).
public struct TypedDataResponse: ResponseEncodable {
    /// The data held by this container.
    let data: Data
    /// The type of the data held by this container.
    let contentType: MediaType
    
    /// See `ResponseEncodable`.
    public func encode(for req: Request) throws -> EventLoopFuture<Response> {
        let res = req.makeResponse()
        res.http.body = data.convertToHTTPBody()
        res.http.contentType = contentType
        return try res.encode(for: req)
    }
}

extension Data: ResponseEncodable {
    /// See `ResponseEncodable`.
    public func encode(for req: Request) throws -> EventLoopFuture<Response> {
        return try response(type: .any).encode(for: req)
    }
    
    /**
     Get a `ResponseEncodable` container holding this data with your provided contentType.
     - parameter contentType: The type of data to return the container with.
     */
    public func response(type: MediaType) -> TypedDataResponse {
        return TypedDataResponse(data: self, contentType: type)
    }
}


Review comment (suggestion):
couple gardening notes here:
- vapor uses `///` format comments
- `- parameter`s should be listed as a separate bulleted list, see any of the other doc blocks for an example
- methods expected to be used directly by the end user should have a small example code section if possible! that really helps people figure out how to use it. 

Code after:
/// A container representing `Data` with a specific content type (used for returning as a `Response`).
private struct TypedDataResponse: ResponseEncodable {
    /// The data held by this container.
    let data: Data

    /// The type of the data held by this container.
    let contentType: MediaType
    
    /// See `ResponseEncodable`.
    public func encode(for req: Request) throws -> EventLoopFuture<Response> {
        let res = req.makeResponse()
        res.http.body = data.convertToHTTPBody()
        res.http.contentType = contentType
        return try res.encode(for: req)
    }
}

extension Data: ResponseEncodable {
    /// See `ResponseEncodable`.
    public func encode(for req: Request) throws -> EventLoopFuture<Response> {
        return try response(type: .any).encode(for: req)
    }
    
    /// Get a `ResponseEncodable` container holding this data with your provided contentType.
    ///
    ///     data.response(type: .html)
    ///
    /// - parameters:
    ///     - type: The type of data to return the container with.
    public func response(type: MediaType) -> ResponseEncodable {
        return TypedDataResponse(data: self, contentType: type)
    }
}
Repository: vapor/vapor
File: Sources/Vapor/Error/AbortError.swift (Swift)

Code before:
            return "Value of type '\(type)' required for key '\(ctx.codingPath.dotPath)'."
        case .valueNotFound(let type, let ctx):
            return "Value of type '\(type)' required for key '\(ctx.codingPath.dotPath)'."
        }
    }
}

extension NotFound: AbortError {
    /// See `AbortError.status`
    public var status: HTTPResponseStatus {
        return .notFound
    }
    
    /// See `AbortError.reason`
    public var reason: String {
        switch self {
        case let rootCase as Debuggable:
            return rootCase.failureReason ?? rootCase.reason
        default:
            return rootCause?.localizedDescription ?? "Not found."
        }
    }
    
    /// See `AbortError.identifier`
    public var identifier: String {
        return "NotFound"
    }
    
}

extension Array where Element == CodingKey {
    fileprivate var dotPath: String {
        return map { $0.stringValue }.joined(separator: ".")
    }
}


Review comment (suggestion):
Nit-pick - this should probably be `notFound` (lower case `n`) to match `DecodingError`

Code after:
            return "Value of type '\(type)' required for key '\(ctx.codingPath.dotPath)'."
        case .valueNotFound(let type, let ctx):
            return "Value of type '\(type)' required for key '\(ctx.codingPath.dotPath)'."
        }
    }
}

extension NotFound: AbortError {
    /// See `AbortError.status`
    public var status: HTTPResponseStatus {
        return .notFound
    }
    
    /// See `AbortError.reason`
    public var reason: String {
        switch rootCause {
        case let rootCause as Debuggable:
            return rootCause.reason
        default:
            return rootCause?.localizedDescription ?? "Not found."
        }
    }
    
    /// See `AbortError.identifier`
    public var identifier: String {
        return "notFound"
    }
    
}

extension Array where Element == CodingKey {
    fileprivate var dotPath: String {
        return map { $0.stringValue }.joined(separator: ".")
    }
}
Repository: vapor/vapor
File: Sources/Vapor/Middleware/ErrorMiddleware.swift (Swift)

Code before:

    /// Create a new `ErrorMiddleware`.
    ///
    /// - parameters:
    ///     - closure: Error-handling closure. Converts `Error` to `Response`.
    public init(_ closure: @escaping (Request, Error) -> (Response)) {
        self.closure = closure
    }

    /// See `Middleware`.
    public func respond(to req: Request, chainingTo next: Responder) throws -> Future<Response> {
        let response: Future<Response>
        do {
            response = try next.respond(to: req)
        } catch {
            response = req.eventLoop.newFailedFuture(error: error)
        }

        return response.mapIfError { error in
            return self.closure(req, error)
        }
    }
}

/// Structure of `ErrorMiddleware` default response.
public struct ErrorResponse: Encodable, Decodable {
    /// Always `true` to indicate this is a non-typical JSON response.
    var error: Bool

    /// The reason for the error.
    var reason: String
}

Review comment (suggestion):
you just made this available throughout the whole Vapor, I think this should stay, although public, just within the `ErrorMiddleware` scope

Code after:
                res.http.body = HTTPBody(string: "Oops: \(error)")
                res.http.headers.replaceOrAdd(name: .contentType, value: "text/plain; charset=utf-8")
            }
            return res
        }
    }

    /// Error-handling closure.
    private let closure: (Request, Error) -> (Response)

    /// Create a new `ErrorMiddleware`.
    ///
    /// - parameters:
    ///     - closure: Error-handling closure. Converts `Error` to `Response`.
    public init(_ closure: @escaping (Request, Error) -> (Response)) {
        self.closure = closure
    }

    /// See `Middleware`.
    public func respond(to req: Request, chainingTo next: Responder) throws -> Future<Response> {
        let response: Future<Response>
        do {
            response = try next.respond(to: req)
        } catch {
            response = req.eventLoop.newFailedFuture(error: error)
        }

        return response.mapIfError { error in
            return self.closure(req, error)
        }
    }
}
Repository: vapor/vapor
File: Sources/Vapor/Middleware/ErrorMiddleware.swift (Swift)

Code before:
/// Captures all errors and transforms them into an internal server error HTTP response.
public final class ErrorMiddleware: Middleware, ServiceType {
    /// Structure of `ErrorMiddleware` default response.
    internal struct ErrorResponse: Encodable, Decodable {
        /// Always `true` to indicate this is a non-typical JSON response.
        var error: Bool

        /// The reason for the error.
        var reason: String
    }

    /// See `ServiceType`.
    public static func makeService(for worker: Container) throws -> ErrorMiddleware {
        return try .default(environment: worker.environment, log: worker.make())
    }

    /// Create a default `ErrorMiddleware`. Logs errors to a `Logger` based on `Environment`
    /// and converts `Error` to `Response` based on conformance to `AbortError` and `Debuggable`.
    ///
    /// - parameters:
    ///     - environment: The environment to respect when presenting errors.
    ///     - log: Log destination.
    public static func `default`(environment: Environment, log: Logger) -> ErrorMiddleware {
        return .init { req, error in
            // log the error
            log.report(error: error, verbose: !environment.isRelease)

            // variables to determine
            let status: HTTPResponseStatus

Review comment (suggestion):
I’d say make this inherit from `Codable` instead. 

Code after:
/// Captures all errors and transforms them into an internal server error HTTP response.
public final class ErrorMiddleware: Middleware, ServiceType {
    /// Structure of `ErrorMiddleware` default response.
    internal struct ErrorResponse: Codable {
        /// Always `true` to indicate this is a non-typical JSON response.
        var error: Bool

        /// The reason for the error.
        var reason: String
    }

    /// See `ServiceType`.
    public static func makeService(for worker: Container) throws -> ErrorMiddleware {
        return try .default(environment: worker.environment, log: worker.make())
    }

    /// Create a default `ErrorMiddleware`. Logs errors to a `Logger` based on `Environment`
    /// and converts `Error` to `Response` based on conformance to `AbortError` and `Debuggable`.
    ///
    /// - parameters:
    ///     - environment: The environment to respect when presenting errors.
    ///     - log: Log destination.
    public static func `default`(environment: Environment, log: Logger) -> ErrorMiddleware {
        return .init { req, error in
            // log the error
            log.report(error: error, verbose: !environment.isRelease)

            // variables to determine
            let status: HTTPResponseStatus
Repository: vapor/vapor
File: Sources/Vapor/Server/NIOServerConfig.swift (Swift)

Code before:
    /// Number of client accepting workers.
    /// Should be equal to the number of logical cores.
    public var workerCount: Int

    /// Requests containing bodies larger than this maximum will be rejected, closing the connection.
    public var maxBodySize: Int

    /// When `true`, can prevent errors re-binding to a socket after successive server restarts.
    public var reuseAddress: Bool

    /// When `true`, OS will attempt to minimize TCP packet delay.
    public var tcpNoDelay: Bool

    /// Number of webSocket maxFrameSize.
    public var webSocketMaxFrameSize: Int

    /// Creates a new `NIOServerConfig`.
    public init(
        hostname: String,
        port: Int,
        backlog: Int,
        workerCount: Int,
        maxBodySize: Int,
        reuseAddress: Bool,
        tcpNoDelay: Bool,
        webSocketMaxFrameSize: Int
    ) {
        self.hostname = hostname
        self.port = port
        self.backlog = backlog
        self.workerCount = workerCount
        self.maxBodySize = maxBodySize
        self.reuseAddress = reuseAddress
        self.tcpNoDelay = tcpNoDelay
        self.webSocketMaxFrameSize = webSocketMaxFrameSize
    }
}


Review comment (suggestion):
Please add a default value to this initializer to avoid the breaking change.

Case in point: the ApplicationTests test source does not compile because of not filling this new parameter (see CircleCI).

Code after:
    /// Number of client accepting workers.
    /// Should be equal to the number of logical cores.
    public var workerCount: Int

    /// Requests containing bodies larger than this maximum will be rejected, closing the connection.
    public var maxBodySize: Int

    /// When `true`, can prevent errors re-binding to a socket after successive server restarts.
    public var reuseAddress: Bool

    /// When `true`, OS will attempt to minimize TCP packet delay.
    public var tcpNoDelay: Bool

    /// Number of webSocket maxFrameSize.
    public var webSocketMaxFrameSize: Int

    /// Creates a new `NIOServerConfig`.
    public init(
        hostname: String,
        port: Int,
        backlog: Int,
        workerCount: Int,
        maxBodySize: Int,
        reuseAddress: Bool,
        tcpNoDelay: Bool,
        webSocketMaxFrameSize: Int = 1 << 14
    ) {
        self.hostname = hostname
        self.port = port
        self.backlog = backlog
        self.workerCount = workerCount
        self.maxBodySize = maxBodySize
        self.reuseAddress = reuseAddress
        self.tcpNoDelay = tcpNoDelay
        self.webSocketMaxFrameSize = webSocketMaxFrameSize
    }
}