ronantakizawa/codereview-bench huggingface dataset:
Repository: airbnb/lottie-ios
File: Sources/Public/ImageProvider/AnimationImageProvider.swift (Swift)

Code before:
/// prebuilt Image Providers that supply images from a Bundle, or from a FilePath.
///
/// Additionally custom Image Providers can be made to load images from a URL,
/// or to Cache images.
public protocol AnimationImageProvider {

  /// Whether or not the resulting image of this image provider can be cached by Lottie. Defaults to true.
  /// If true, Lottie may internally cache the result of `imageForAsset`
  var cacheEligible: Bool { get }

  /// The image to display for the given `ImageAsset` defined in the `LottieAnimation` JSON file.
  func imageForAsset(asset: ImageAsset) -> CGImage?

  /// Specifies how the layer's contents are positioned or scaled within its bounds for a given asset.
  func contentsGravity(for asset: ImageAsset) -> CALayerContentsGravity?
}

extension AnimationImageProvider {
  public var cacheEligible: Bool {
    true
  }

  /// The default value is `.resize`, similar to that of `CALayer`.
  public func contentsGravity(for asset: ImageAsset) -> CALayerContentsGravity? {
    nil
  }
}


Review comment (suggestion):
Maybe it would be better for this to be non-optional, with the default defined like this:

```suggestion
  public func contentsGravity(for asset: ImageAsset) -> CALayerContentsGravity {
    .resize
  }
```

Code after:
/// prebuilt Image Providers that supply images from a Bundle, or from a FilePath.
///
/// Additionally custom Image Providers can be made to load images from a URL,
/// or to Cache images.
public protocol AnimationImageProvider {

  /// Whether or not the resulting image of this image provider can be cached by Lottie. Defaults to true.
  /// If true, Lottie may internally cache the result of `imageForAsset`
  var cacheEligible: Bool { get }

  /// The image to display for the given `ImageAsset` defined in the `LottieAnimation` JSON file.
  func imageForAsset(asset: ImageAsset) -> CGImage?

  /// Specifies how the layer's contents are positioned or scaled within its bounds for a given asset.
  /// Defaults to `.resize`, which stretches the image to fill the layer.
  func contentsGravity(for asset: ImageAsset) -> CALayerContentsGravity
}

extension AnimationImageProvider {
  public var cacheEligible: Bool {
    true
  }

  /// The default value is `.resize`, similar to that of `CALayer`.
  public func contentsGravity(for _: ImageAsset) -> CALayerContentsGravity {
    .resize
  }
}
Repository: airbnb/lottie-ios
File: Sources/Public/ImageProvider/AnimationImageProvider.swift (Swift)

Code before:
//

import CoreGraphics
import Foundation
import QuartzCore

// MARK: - AnimationImageProvider

/// Image provider is a protocol that is used to supply images to `LottieAnimationView`.
///
/// Some animations require a reference to an image. The image provider loads and
/// provides those images to the `LottieAnimationView`.  Lottie includes a couple of
/// prebuilt Image Providers that supply images from a Bundle, or from a FilePath.
///
/// Additionally custom Image Providers can be made to load images from a URL,
/// or to Cache images.
public protocol AnimationImageProvider {

  /// Whether or not the resulting image of this image provider can be cached by Lottie. Defaults to true.
  /// If true, Lottie may internally cache the result of `imageForAsset`
  var cacheEligible: Bool { get }

  /// The image to display for the given `ImageAsset` defined in the `LottieAnimation` JSON file.
  func imageForAsset(asset: ImageAsset) -> CGImage?

  /// Specifies how the layer's contents are positioned or scaled within its bounds for a given asset.
  func contentsGravity(for asset: ImageAsset) -> CALayerContentsGravity?
}

extension AnimationImageProvider {
  public var cacheEligible: Bool {
    true
  }

  /// The default value is `.resize`, similar to that of `CALayer`.
  public func contentsGravity(for asset: ImageAsset) -> CALayerContentsGravity? {
    nil
  }
}


Review comment (suggestion):
```suggestion
  /// Specifies how the layer's contents are positioned or scaled within its bounds for a given asset.
  /// Defaults to `.resize`, which stretches the image to fill the layer.
```

Code after:
//

import CoreGraphics
import Foundation
import QuartzCore

// MARK: - AnimationImageProvider

/// Image provider is a protocol that is used to supply images to `LottieAnimationView`.
///
/// Some animations require a reference to an image. The image provider loads and
/// provides those images to the `LottieAnimationView`.  Lottie includes a couple of
/// prebuilt Image Providers that supply images from a Bundle, or from a FilePath.
///
/// Additionally custom Image Providers can be made to load images from a URL,
/// or to Cache images.
public protocol AnimationImageProvider {

  /// Whether or not the resulting image of this image provider can be cached by Lottie. Defaults to true.
  /// If true, Lottie may internally cache the result of `imageForAsset`
  var cacheEligible: Bool { get }

  /// The image to display for the given `ImageAsset` defined in the `LottieAnimation` JSON file.
  func imageForAsset(asset: ImageAsset) -> CGImage?

  /// Specifies how the layer's contents are positioned or scaled within its bounds for a given asset.
  /// Defaults to `.resize`, which stretches the image to fill the layer.
  func contentsGravity(for asset: ImageAsset) -> CALayerContentsGravity
}

extension AnimationImageProvider {
  public var cacheEligible: Bool {
    true
  }

  /// The default value is `.resize`, similar to that of `CALayer`.
  public func contentsGravity(for _: ImageAsset) -> CALayerContentsGravity {
    .resize
  }
}
Repository: airbnb/lottie-ios
File: Sources/Private/CoreAnimation/Layers/ImageLayer.swift (Swift)

Code before:
  /// Called by CoreAnimation to create a shadow copy of this layer
  /// More details: https://developer.apple.com/documentation/quartzcore/calayer/1410842-init
  override init(layer: Any) {
    guard let typedLayer = layer as? Self else {
      fatalError("\(Self.self).init(layer:) incorrectly called with \(type(of: layer))")
    }

    imageLayer = typedLayer.imageLayer
    super.init(layer: typedLayer)
  }

  // MARK: Internal

  func setupImage(context: LayerContext) {
    guard
      let imageAsset = context.animation.assetLibrary?.imageAssets[imageLayer.referenceID],
      let image = context.imageProvider.imageForAsset(asset: imageAsset)
    else {
      self.imageAsset = nil
      contentsLayer.contents = nil
      return
    }

    self.imageAsset = imageAsset
    contentsLayer.contents = image
    contentsLayer.contentsGravity = context.imageProvider.contentsGravity(for: imageAsset) ?? .resize
    setNeedsLayout()
  }

  // MARK: Private

  private let imageLayer: ImageLayerModel
  private var imageAsset: ImageAsset?

}

// MARK: CustomLayoutLayer

extension ImageLayer: CustomLayoutLayer {
  func layout(superlayerBounds: CGRect) {
    anchorPoint = .zero

    guard let imageAsset = imageAsset else {
      bounds = superlayerBounds
      return
    }

    // Image layers specifically need to use the size of the image itself
    bounds = CGRect(
      x: superlayerBounds.origin.x,
      y: superlayerBounds.origin.y,

Review comment (suggestion):
We should also make this change to the `ImageCompositionLayer` used by the Main Thread rendering engine

Code after:
  /// Called by CoreAnimation to create a shadow copy of this layer
  /// More details: https://developer.apple.com/documentation/quartzcore/calayer/1410842-init
  override init(layer: Any) {
    guard let typedLayer = layer as? Self else {
      fatalError("\(Self.self).init(layer:) incorrectly called with \(type(of: layer))")
    }

    imageLayer = typedLayer.imageLayer
    super.init(layer: typedLayer)
  }

  // MARK: Internal

  func setupImage(context: LayerContext) {
    guard
      let imageAsset = context.animation.assetLibrary?.imageAssets[imageLayer.referenceID],
      let image = context.imageProvider.imageForAsset(asset: imageAsset)
    else {
      self.imageAsset = nil
      contentsLayer.contents = nil
      return
    }

    self.imageAsset = imageAsset
    contentsLayer.contents = image
    contentsLayer.contentsGravity = context.imageProvider.contentsGravity(for: imageAsset)
    setNeedsLayout()
  }

  // MARK: Private

  private let imageLayer: ImageLayerModel
  private var imageAsset: ImageAsset?

}

// MARK: CustomLayoutLayer

extension ImageLayer: CustomLayoutLayer {
  func layout(superlayerBounds: CGRect) {
    anchorPoint = .zero

    guard let imageAsset = imageAsset else {
      bounds = superlayerBounds
      return
    }

    // Image layers specifically need to use the size of the image itself
    bounds = CGRect(
      x: superlayerBounds.origin.x,
      y: superlayerBounds.origin.y,
Repository: airbnb/lottie-ios
File: Sources/Public/Animation/LottiePlaybackMode.swift (Swift)

Code before:
// Created by Cal Stephens on 8/3/23.
// Copyright © 2023 Airbnb Inc. All rights reserved.

import Foundation

// MARK: - LottieMarkerPosition

/// The position within a marker.
public enum LottieMarkerPosition: Hashable {
  case start
  case end
}

// MARK: - LottiePlaybackMode

/// Configuration for how a Lottie animation should be played
public enum LottiePlaybackMode: Hashable {

  case paused(at: PausedState)

  case play(_ mode: PlaybackMode)

  @available(*, deprecated, renamed: "LottiePlaybackMode.paused(at:)", message: "Will be removed in a future major release.")
  case progress(_ progress: AnimationProgressTime)

  @available(*, deprecated, renamed: "LottiePlaybackMode.paused(at:)", message: "Will be removed in a future major release.")
  case frame(_ frame: AnimationFrameTime)

  @available(*, deprecated, renamed: "LottiePlaybackMode.paused(at:)", message: "Will be removed in a future major release.")
  case time(_ time: TimeInterval)

  @available(*, deprecated, renamed: "LottiePlaybackMode.paused(at:)", message: "Will be removed in a future major release.")
  case pause


Review comment (refactor):
Could you move this to the bottom of the file below `LottiePlaybackMode`? Ideally `LottiePlaybackMode` would be the first type in this file

Code after:
// Created by Cal Stephens on 8/3/23.
// Copyright © 2023 Airbnb Inc. All rights reserved.

import Foundation

// MARK: - LottiePlaybackMode

/// Configuration for how a Lottie animation should be played
public enum LottiePlaybackMode: Hashable {

  /// The animation is paused at the given state (e.g. paused at a specific frame)
  case paused(at: PausedState)

  /// The animation is playing using the given playback mode (e.g. looping from the start to the end)
  case playing(_ mode: PlaybackMode)

  @available(*, deprecated, renamed: "LottiePlaybackMode.paused(at:)", message: "Will be removed in a future major release.")
  case progress(_ progress: AnimationProgressTime)

  @available(*, deprecated, renamed: "LottiePlaybackMode.paused(at:)", message: "Will be removed in a future major release.")
  case frame(_ frame: AnimationFrameTime)

  @available(*, deprecated, renamed: "LottiePlaybackMode.paused(at:)", message: "Will be removed in a future major release.")
  case time(_ time: TimeInterval)

  @available(*, deprecated, renamed: "LottiePlaybackMode.paused(at:)", message: "Will be removed in a future major release.")
  case pause

  @available(*, deprecated, renamed: "LottiePlaybackMode.playing(_:)", message: "Will be removed in a future major release.")
  case fromProgress(_ fromProgress: AnimationProgressTime?, toProgress: AnimationProgressTime, loopMode: LottieLoopMode)

  @available(*, deprecated, renamed: "LottiePlaybackMode.playing(_:)", message: "Will be removed in a future major release.")
  case fromFrame(_ fromFrame: AnimationFrameTime?, toFrame: AnimationFrameTime, loopMode: LottieLoopMode)
Repository: airbnb/lottie-ios
File: Sources/Private/CoreAnimation/Layers/LayerModel+makeAnimationLayer.swift (Swift)

Code before:
      return TransformLayer(layerModel: self)
    }

    switch (type, self) {
    case (.precomp, let preCompLayerModel as PreCompLayerModel):
      let preCompLayer = PreCompLayer(preCompLayer: preCompLayerModel)
      try preCompLayer.setup(context: context)
      return preCompLayer

    case (.solid, let solidLayerModel as SolidLayerModel):
      return SolidLayer(solidLayerModel)

    case (.shape, let shapeLayerModel as ShapeLayerModel):
      return try ShapeLayer(shapeLayer: shapeLayerModel, context: context)

    case (.image, let imageLayerModel as ImageLayerModel):
      return ImageLayer(imageLayer: imageLayerModel, context: context)

    case (.text, let textLayerModel as TextLayerModel):
      return try TextLayer(textLayerModel: textLayerModel, context: context)

    case (.null, _):
      return TransformLayer(layerModel: self)

    case (.unknown, _):
      return nil

    default:
      try context.logCompatibilityIssue("""
        Unexpected layer type combination ("\(type)" and "\(Swift.type(of: self))")
        """)

      return nil
    }
  }

}


Review comment (suggestion):
This fixes an issue where a layer with an unknown type would log a compatibility error and fall back to the main thread rendering engine, even though the main thread engine wouldn't be able to handle it either

Code after:
      return TransformLayer(layerModel: self)
    }

    switch (type, self) {
    case (.precomp, let preCompLayerModel as PreCompLayerModel):
      let preCompLayer = PreCompLayer(preCompLayer: preCompLayerModel)
      try preCompLayer.setup(context: context)
      return preCompLayer

    case (.solid, let solidLayerModel as SolidLayerModel):
      return SolidLayer(solidLayerModel)

    case (.shape, let shapeLayerModel as ShapeLayerModel):
      return try ShapeLayer(shapeLayer: shapeLayerModel, context: context)

    case (.image, let imageLayerModel as ImageLayerModel):
      return ImageLayer(imageLayer: imageLayerModel, context: context)

    case (.text, let textLayerModel as TextLayerModel):
      return try TextLayer(textLayerModel: textLayerModel, context: context)

    case (.null, _):
      return TransformLayer(layerModel: self)

    case (.unknown, _), (.precomp, _), (.solid, _), (.image, _), (.shape, _), (.text, _):
      return nil
    }
  }

}
Repository: airbnb/lottie-ios
File: Example/Example/LottieViewLayoutDemoView.swift (Swift)

Code before:
// Created by Cal Stephens on 6/28/23.
// Copyright © 2023 Airbnb Inc. All rights reserved.

import Lottie
import SwiftUI

struct LottieViewLayoutDemoView: View {
  var body: some View {
    HStack {
      VStack {
        LottieView(animation: .named("Samples/LottieLogo1"))
          .configure(\.contentMode, value: .scaleAspectFit)
          .looping()
          .frame(maxWidth: 100)

        Text("maxWidth: 100, contentMode: .scaleAspectFit")
      }

      VStack {
        LottieView(animation: .named("Samples/LottieLogo1"))
          .looping()
          .frame(maxHeight: 100)

        Text("maxHeight: 100")
      }

      VStack {
        LottieView(animation: .named("Samples/LottieLogo1"))
          .resizable()
          .looping()

        Text("resizable")
      }

      VStack {
        LottieView(animation: .named("Samples/LottieLogo1"))
          .looping()

Review comment (suggestion):
I was also wondering if it would be nicer to omit the labels entirely:

```suggestion
          .configure(\.contentMode, .scaleAspectFit)
```

If you think that's nicer I'm happy to adjust.

Code after:
// Created by Cal Stephens on 6/28/23.
// Copyright © 2023 Airbnb Inc. All rights reserved.

import Lottie
import SwiftUI

struct LottieViewLayoutDemoView: View {
  var body: some View {
    HStack {
      VStack {
        LottieView(animation: .named("Samples/LottieLogo1"))
          .configure(\.contentMode, to: .scaleAspectFit)
          .looping()
          .frame(maxWidth: 100)

        Text("maxWidth: 100, contentMode: .scaleAspectFit")
      }

      VStack {
        LottieView(animation: .named("Samples/LottieLogo1"))
          .looping()
          .frame(maxHeight: 100)

        Text("maxHeight: 100")
      }

      VStack {
        LottieView(animation: .named("Samples/LottieLogo1"))
          .resizable()
          .looping()

        Text("resizable")
      }

      VStack {
        LottieView(animation: .named("Samples/LottieLogo1"))
          .looping()
Repository: airbnb/lottie-ios
File: Sources/Public/Configuration/LottieURLSession.swift (Swift)

Code before:
///     with url: URL,
///     completionHandler: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void)
///     -> LottieDataTask
///   {
///     // Return a no-op task that never makes a network request
///     completionHandler(nil, nil, URLError(.cancelled))
///     return DisabledDataTask()
///   }
/// }
///
/// class DisabledDataTask: LottieDataTask {
///   func resume() { }
///   func cancel() { }
/// }
///
/// // In your test setup:
/// LottieConfiguration.shared.defaultURLSession = DisabledURLSession()
/// ```
public protocol LottieURLSession: Sendable {
  /// Creates a data task that retrieves the contents of the specified URL.
  ///
  /// - Parameters:
  ///   - url: The URL to retrieve.
  ///   - completionHandler: The completion handler to call when the load request is complete.
  /// - Returns: A `LottieDataTask` that can be resumed or cancelled.
  func lottieDataTask(
    with url: URL,
    completionHandler: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void)
    -> LottieDataTask
}

// MARK: - LottieDataTask

/// A protocol that abstracts a URL session data task for Lottie animations.
///
/// This protocol provides the minimal interface needed to control a data task:
/// resuming it to start the request, or cancelling it to abort.
public protocol LottieDataTask: Sendable {
  /// Resumes the task, if it is suspended.
  func resume()

  /// Cancels the task.
  func cancel()
}

// MARK: - URLSession + LottieURLSession

extension URLSession: LottieURLSession {
  public func lottieDataTask(
    with url: URL,
    completionHandler: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void)

Review comment (suggestion):
Would rather we just call this:

```suggestion
  func dataTask(
```

Code after:
// Created by Andy Bartholomew on 12/2/24.
// Copyright © 2024 Airbnb Inc. All rights reserved.

import Foundation

// MARK: - LottieURLSession

/// A protocol that abstracts URL session functionality for loading Lottie animations.
public protocol LottieURLSession: Sendable {
  /// Creates a data task that retrieves the contents of the specified URL.
  ///
  /// - Parameters:
  ///   - url: The URL to retrieve.
  ///   - completionHandler: The completion handler to call when the load request is complete.
  /// - Returns: A `URLSessionDataTask` that can be resumed or cancelled, or `nil` for mock implementations.
  func lottieDataTask(
    with url: URL,
    completionHandler: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void)
    -> URLSessionDataTask?
}

// MARK: - URLSession + LottieURLSession

extension URLSession: LottieURLSession {
  public func lottieDataTask(
    with url: URL,
    completionHandler: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void)
    -> URLSessionDataTask?
  {
    dataTask(with: url, completionHandler: completionHandler)
  }
}
Repository: airbnb/lottie-ios
File: Sources/Public/Configuration/LottieURLSession.swift (Swift)

Code before:
// Created by Claude on behalf of Airbnb Inc.
// Copyright © 2024 Airbnb Inc. All rights reserved.

import Foundation

// MARK: - LottieURLSession

/// A protocol that abstracts URL session functionality for loading Lottie animations.
///
/// This protocol allows you to inject a custom URL session implementation,
/// which is particularly useful for:
/// - Disabling network requests during screenshot tests for more stable results
/// - Mocking network responses in unit tests
/// - Implementing custom caching or request modification logic
///
/// By default, Lottie uses `URLSession.shared` for network requests.
/// To disable network requests (e.g., for screenshot tests), you can create
/// a mock implementation that ignores requests:
///
/// ```swift
/// class DisabledURLSession: LottieURLSession {
///   func lottieDataTask(
///     with url: URL,
///     completionHandler: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void)
///     -> LottieDataTask
///   {
///     // Return a no-op task that never makes a network request

Review comment (suggestion):
😛 

```suggestion
// Created by Andy Bartholomew on 12/2/25
// Copyright © 2025 Airbnb Inc. All rights reserved.
```

Code after:
// Created by Andy Bartholomew on 12/2/24.
// Copyright © 2024 Airbnb Inc. All rights reserved.

import Foundation

// MARK: - LottieURLSession

/// A protocol that abstracts URL session functionality for loading Lottie animations.
public protocol LottieURLSession: Sendable {
  /// Creates a data task that retrieves the contents of the specified URL.
  ///
  /// - Parameters:
  ///   - url: The URL to retrieve.
  ///   - completionHandler: The completion handler to call when the load request is complete.
  /// - Returns: A `URLSessionDataTask` that can be resumed or cancelled, or `nil` for mock implementations.
  func lottieDataTask(
    with url: URL,
    completionHandler: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void)
    -> URLSessionDataTask?
}

// MARK: - URLSession + LottieURLSession

extension URLSession: LottieURLSession {
  public func lottieDataTask(
    with url: URL,
    completionHandler: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void)
Repository: airbnb/lottie-ios
File: Sources/Public/Configuration/LottieURLSession.swift (Swift)

Code before:

// MARK: - LottieURLSession

/// A protocol that abstracts URL session functionality for loading Lottie animations.
public protocol LottieURLSession: Sendable {
  /// Creates a data task that retrieves the contents of the specified URL.
  ///
  /// - Parameters:
  ///   - url: The URL to retrieve.
  ///   - completionHandler: The completion handler to call when the load request is complete.
  /// - Returns: A `URLSessionDataTask` that can be resumed or cancelled, or `nil` for mock implementations.
  func dataTask(
    with url: URL,
    completionHandler: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void)
    -> URLSessionDataTask?
}

// MARK: - URLSession + LottieURLSession

extension URLSession: LottieURLSession {
  public func dataTask(
    with url: URL,
    completionHandler: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void)
    -> URLSessionDataTask?
  {
    dataTask(with: url, completionHandler: completionHandler) as URLSessionDataTask
  }
}


Review comment (suggestion):
Ah, ok maybe this is a bad idea, I forgot this would make it more difficult to call the normal `dataTask` method if you import Lottie. Maybe we should call this `lottieDataTask` after all. We don't want an `import Lottie` line to accidentally change the meaning of an existing `session.dataTask` call.

Code after:

// MARK: - LottieURLSession

/// A protocol that abstracts URL session functionality for loading Lottie animations.
public protocol LottieURLSession: Sendable {
  /// Creates a data task that retrieves the contents of the specified URL.
  ///
  /// - Parameters:
  ///   - url: The URL to retrieve.
  ///   - completionHandler: The completion handler to call when the load request is complete.
  /// - Returns: A `URLSessionDataTask` that can be resumed or cancelled, or `nil` for mock implementations.
  func lottieDataTask(
    with url: URL,
    completionHandler: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void)
    -> URLSessionDataTask?
}

// MARK: - URLSession + LottieURLSession

extension URLSession: LottieURLSession {
  public func lottieDataTask(
    with url: URL,
    completionHandler: @escaping @Sendable (Data?, URLResponse?, (any Error)?) -> Void)
    -> URLSessionDataTask?
  {
    dataTask(with: url, completionHandler: completionHandler)
  }
}