ronantakizawa/codereview-bench huggingface dataset:
Repository: expo/expo
File: packages/expo-image/ios/PSDCoder.swift (Swift)

Code before:
// Copyright 2024-present 650 Industries. All rights reserved.

import SDWebImage

internal final class PSDCoder: NSObject, SDImageCoder {
  nonisolated(unsafe) static let shared = PSDCoder()
  
  func canDecode(from data: Data?) -> Bool {
    guard let data = data, data.count >= 4 else { return false }
    
    // verify PSD magic bytes
    let signatureData = data[0..<4]
    let signature = String(data: signatureData, encoding: .ascii)
    return signature == "8BPS"
  }
  
  func decodedImage(with data: Data?, options: [SDImageCoderOption : Any]? = nil) -> UIImage? {
    guard let data = data else { return nil }
    
    if let scale = options?[SDImageCoderOption.decodeScaleFactor] as? CGFloat {
      return UIImage(data: data, scale: scale)
    }
    
    // UIImage is able to directly handle PSD data
    return UIImage(data: data)
  }
  
  func canEncode(to format: SDImageFormat) -> Bool {
    false
  }
  
  func encodedData(with image: UIImage?, format: SDImageFormat, options: [SDImageCoderOption : Any]? = nil) -> Data? {
    return nil
  }

Review comment (suggestion):
```suggestion
    guard let data, data.count >= 4 else {
      return false
    }
```

Code after:
// Copyright 2024-present 650 Industries. All rights reserved.

import SDWebImage

internal final class PSDCoder: NSObject, SDImageCoder {
  nonisolated(unsafe) static let shared = PSDCoder()
  
  func canDecode(from data: Data?) -> Bool {
    guard let data, data.count >= 4 else {
      return false
    }
    
    // verify PSD magic bytes
    let signatureData = data[0..<4]
    let signature = String(data: signatureData, encoding: .ascii)
    return signature == "8BPS"
  }
  
  func decodedImage(with data: Data?, options: [SDImageCoderOption : Any]? = nil) -> UIImage? {
    guard let data else {
      return nil
    }
    
    if let scale = options?[SDImageCoderOption.decodeScaleFactor] as? CGFloat {
      return UIImage(data: data, scale: scale)
    }
    
    // UIImage is able to directly handle PSD data
    return UIImage(data: data)
  }
  
  func canEncode(to format: SDImageFormat) -> Bool {
    return false
  }
Repository: expo/expo
File: packages/expo-image/ios/PSDCoder.swift (Swift)

Code before:
// Copyright 2024-present 650 Industries. All rights reserved.

import SDWebImage

internal final class PSDCoder: NSObject, SDImageCoder {
  nonisolated(unsafe) static let shared = PSDCoder()
  
  func canDecode(from data: Data?) -> Bool {
    guard let data = data, data.count >= 4 else { return false }
    
    // verify PSD magic bytes
    let signatureData = data[0..<4]
    let signature = String(data: signatureData, encoding: .ascii)
    return signature == "8BPS"
  }
  
  func decodedImage(with data: Data?, options: [SDImageCoderOption : Any]? = nil) -> UIImage? {
    guard let data = data else { return nil }
    
    if let scale = options?[SDImageCoderOption.decodeScaleFactor] as? CGFloat {
      return UIImage(data: data, scale: scale)
    }
    
    // UIImage is able to directly handle PSD data
    return UIImage(data: data)
  }
  
  func canEncode(to format: SDImageFormat) -> Bool {
    false
  }
  
  func encodedData(with image: UIImage?, format: SDImageFormat, options: [SDImageCoderOption : Any]? = nil) -> Data? {
    return nil
  }
}


Review comment (suggestion):
```suggestion
    guard let data else {
      return nil
    }
```

Code after:
// Copyright 2024-present 650 Industries. All rights reserved.

import SDWebImage

internal final class PSDCoder: NSObject, SDImageCoder {
  nonisolated(unsafe) static let shared = PSDCoder()
  
  func canDecode(from data: Data?) -> Bool {
    guard let data, data.count >= 4 else {
      return false
    }
    
    // verify PSD magic bytes
    let signatureData = data[0..<4]
    let signature = String(data: signatureData, encoding: .ascii)
    return signature == "8BPS"
  }
  
  func decodedImage(with data: Data?, options: [SDImageCoderOption : Any]? = nil) -> UIImage? {
    guard let data else {
      return nil
    }
    
    if let scale = options?[SDImageCoderOption.decodeScaleFactor] as? CGFloat {
      return UIImage(data: data, scale: scale)
    }
    
    // UIImage is able to directly handle PSD data
    return UIImage(data: data)
  }
  
  func canEncode(to format: SDImageFormat) -> Bool {
    return false
  }
  
  func encodedData(with image: UIImage?, format: SDImageFormat, options: [SDImageCoderOption : Any]? = nil) -> Data? {
    return nil
  }
}
Repository: expo/expo
File: packages/expo-image/ios/PSDCoder.swift (Swift)

Code before:

internal final class PSDCoder: NSObject, SDImageCoder {
  nonisolated(unsafe) static let shared = PSDCoder()
  
  func canDecode(from data: Data?) -> Bool {
    guard let data = data, data.count >= 4 else { return false }
    
    // verify PSD magic bytes
    let signatureData = data[0..<4]
    let signature = String(data: signatureData, encoding: .ascii)
    return signature == "8BPS"
  }
  
  func decodedImage(with data: Data?, options: [SDImageCoderOption : Any]? = nil) -> UIImage? {
    guard let data = data else { return nil }
    
    if let scale = options?[SDImageCoderOption.decodeScaleFactor] as? CGFloat {
      return UIImage(data: data, scale: scale)
    }
    
    // UIImage is able to directly handle PSD data
    return UIImage(data: data)
  }
  
  func canEncode(to format: SDImageFormat) -> Bool {
    false
  }
  
  func encodedData(with image: UIImage?, format: SDImageFormat, options: [SDImageCoderOption : Any]? = nil) -> Data? {
    return nil
  }
}


Review comment (suggestion):
Being explicit is a good thing 😅 
```suggestion
    return false
```

Code after:

internal final class PSDCoder: NSObject, SDImageCoder {
  nonisolated(unsafe) static let shared = PSDCoder()
  
  func canDecode(from data: Data?) -> Bool {
    guard let data, data.count >= 4 else {
      return false
    }
    
    // verify PSD magic bytes
    let signatureData = data[0..<4]
    let signature = String(data: signatureData, encoding: .ascii)
    return signature == "8BPS"
  }
  
  func decodedImage(with data: Data?, options: [SDImageCoderOption : Any]? = nil) -> UIImage? {
    guard let data else {
      return nil
    }
    
    if let scale = options?[SDImageCoderOption.decodeScaleFactor] as? CGFloat {
      return UIImage(data: data, scale: scale)
    }
    
    // UIImage is able to directly handle PSD data
    return UIImage(data: data)
  }
  
  func canEncode(to format: SDImageFormat) -> Bool {
    return false
  }
  
  func encodedData(with image: UIImage?, format: SDImageFormat, options: [SDImageCoderOption : Any]? = nil) -> Data? {
    return nil
  }
}
Repository: expo/expo
File: docs/ui/components/Tag/PlatformIcon.tsx (TypeScript)

Code before:
  platform?: PlatformName;
};

export const PlatformIcon = ({ platform }: PlatformIconProps) => {
  switch (platform) {
    case 'ios':
    case 'macos':
    case 'tvos':
      return (
        <AppleIcon
          className={mergeClasses(
            'icon-2xs mt-[-0.5px] shrink-0 text-palette-blue12 opacity-80',
            platform === 'macos' && 'text-palette-purple12',
            platform === 'tvos' && 'text-palette-pink12'
          )}
        />
      );
    case 'android':
      return (
        <AndroidIcon className="icon-2xs mt-[-0.5px] shrink-0 text-palette-green12 opacity-80" />
      );
    case 'web':
      return <AtSignIcon className="icon-2xs shrink-0 text-palette-orange12 opacity-80" />;
    case 'expo':
      return <ExpoGoLogo className="icon-2xs shrink-0 text-palette-purple12 opacity-80" />;
    case 'dev-builds':
      return <BuildIcon className="icon-2xs shrink-0 text-palette-gray12 opacity-80" />;
    default:
      return null;
  }
};


Review comment (suggestion):
I don't think we should show a tag at all for dev builds since _all_ libraries are supported in dev builds

Code after:
};

export const PlatformIcon = ({ platform }: PlatformIconProps) => {
  switch (platform) {
    case 'ios':
    case 'macos':
    case 'tvos':
      return (
        <AppleIcon
          className={mergeClasses(
            'icon-2xs mt-[-0.5px] shrink-0 text-palette-blue12 opacity-80',
            platform === 'macos' && 'text-palette-purple12',
            platform === 'tvos' && 'text-palette-pink12'
          )}
        />
      );
    case 'android':
      return (
        <AndroidIcon className="icon-2xs mt-[-0.5px] shrink-0 text-palette-green12 opacity-80" />
      );
    case 'web':
      return <AtSignIcon className="icon-2xs shrink-0 text-palette-orange12 opacity-80" />;
    case 'expo':
      return <ExpoGoLogo className="icon-2xs shrink-0 text-current opacity-80" />;
    default:
      return null;
  }
};
Repository: expo/expo
File: packages/expo/android/src/main/java/expo/modules/fetch/ResponseSink.kt (Kotlin)

Code before:

package expo.modules.fetch

import java.nio.ByteBuffer

internal class ResponseSink {
  private val bodyQueue: MutableList<ByteArray> = mutableListOf()
  private var isFinalized = false
  var bodyUsed = false
    private set

  internal fun appendBufferBody(data: ByteArray) {
    bodyUsed = true
    bodyQueue.add(data)
  }

  fun finalize(): ByteArray {
    val size = bodyQueue.sumOf { it.size }
    val byteBuffer = ByteBuffer.allocate(size)
    for (byteArray in bodyQueue) {
      byteBuffer.put(byteArray)
    }
    bodyQueue.clear()
    bodyUsed = true
    isFinalized = true
    return byteBuffer.array()
  }

  fun finalizeDirect(): ByteBuffer {
    val size = bodyQueue.sumOf { it.size }
    val byteBuffer = ByteBuffer.allocateDirect(size)
    for (byteArray in bodyQueue) {
      byteBuffer.put(byteArray)
    }
    bodyQueue.clear()
    bodyUsed = true
    isFinalized = true
    return byteBuffer
  }
}


Review comment (performance):
Theoretically, we could optimize it further by introducing a fast path and getting rid of `bodyQueue`:
- Condition: response `Transfer-Encoding != Chunked` and `Content-Length` is known, we can pre-allocate `ByteBuffer.`
  - Instead of appending to `bodyQueue`, we could append directly to the pre-allocated buffer.
- Otherwise, fall back to the current `bodyQueue` approach.

This adds some complexity and requires more testing, so I didn't want to introduce it here

Code after:

package expo.modules.fetch

import java.nio.ByteBuffer

internal class ResponseSink {
  private val bodyQueue: MutableList<ByteArray> = mutableListOf()
  private var isFinalized = false
  var bodyUsed = false
    private set

  internal fun appendBufferBody(data: ByteArray) {
    bodyUsed = true
    bodyQueue.add(data)
  }

  fun finalize(directBuffer: Boolean): ByteBuffer {
    val size = bodyQueue.sumOf { it.size }
    val byteBuffer = if (directBuffer) {
      ByteBuffer.allocateDirect(size)
    } else {
      ByteBuffer.allocate(size)
    }
    for (byteArray in bodyQueue) {
      byteBuffer.put(byteArray)
    }
    bodyQueue.clear()
    bodyUsed = true
    isFinalized = true
    return byteBuffer
  }
}
Repository: expo/expo
File: packages/expo/android/src/main/java/expo/modules/fetch/ResponseSink.kt (Kotlin)

Code before:
import java.nio.ByteBuffer

internal class ResponseSink {
  private val bodyQueue: MutableList<ByteArray> = mutableListOf()
  private var isFinalized = false
  var bodyUsed = false
    private set

  internal fun appendBufferBody(data: ByteArray) {
    bodyUsed = true
    bodyQueue.add(data)
  }

  fun finalize(): ByteArray {
    val size = bodyQueue.sumOf { it.size }
    val byteBuffer = ByteBuffer.allocate(size)
    for (byteArray in bodyQueue) {
      byteBuffer.put(byteArray)
    }
    bodyQueue.clear()
    bodyUsed = true
    isFinalized = true
    return byteBuffer.array()
  }

  fun finalizeDirect(): ByteBuffer {
    val size = bodyQueue.sumOf { it.size }
    val byteBuffer = ByteBuffer.allocateDirect(size)
    for (byteArray in bodyQueue) {
      byteBuffer.put(byteArray)
    }
    bodyQueue.clear()
    bodyUsed = true
    isFinalized = true
    return byteBuffer
  }
}


Review comment (suggestion):
the only difference between finalize and finalizeDirect is the ByteBuffer.allocateDirect. maybe to create a shared finalized with flag to use either ByteBuffer.allocateDirect or ByteBuffer.allocate.

that would prevent we miss some code parity in the future.

Code after:
import java.nio.ByteBuffer

internal class ResponseSink {
  private val bodyQueue: MutableList<ByteArray> = mutableListOf()
  private var isFinalized = false
  var bodyUsed = false
    private set

  internal fun appendBufferBody(data: ByteArray) {
    bodyUsed = true
    bodyQueue.add(data)
  }

  fun finalize(directBuffer: Boolean): ByteBuffer {
    val size = bodyQueue.sumOf { it.size }
    val byteBuffer = if (directBuffer) {
      ByteBuffer.allocateDirect(size)
    } else {
      ByteBuffer.allocate(size)
    }
    for (byteArray in bodyQueue) {
      byteBuffer.put(byteArray)
    }
    bodyQueue.clear()
    bodyUsed = true
    isFinalized = true
    return byteBuffer
  }
}
Repository: expo/expo
File: packages/expo-localization/src/ExpoLocalization.native.ts (TypeScript)

Code before:
import { type EventSubscription, requireNativeModule } from 'expo-modules-core';

const ExpoLocalizationModule = requireNativeModule('ExpoLocalization');

export function addLocaleListener(
  // NOTE(@kitten): We never use the event's data
  listener: (event?: unknown) => void
): EventSubscription {
  return ExpoLocalizationModule.addListener('onLocaleSettingsChanged', listener);
}

export function addCalendarListener(
  // NOTE(@kitten): We never use the event's data
  listener: (event?: unknown) => void
): EventSubscription {
  return ExpoLocalizationModule.addListener('onCalendarSettingsChanged', listener);
}

/**
 * @deprecated use subscription.remove() instead
 * */
export function removeSubscription(subscription: EventSubscription) {
  subscription.remove();
}

export default ExpoLocalizationModule;


Review comment (suggestion):
```suggestion
/**
 * @deprecated use subscription.remove() instead.
 */
```

Code after:
import { type EventSubscription, requireNativeModule } from 'expo-modules-core';

const ExpoLocalizationModule = requireNativeModule('ExpoLocalization');

export function addLocaleListener(
  // NOTE(@kitten): We never use the event's data
  listener: (event?: unknown) => void
): EventSubscription {
  return ExpoLocalizationModule.addListener('onLocaleSettingsChanged', listener);
}

export function addCalendarListener(
  // NOTE(@kitten): We never use the event's data
  listener: (event?: unknown) => void
): EventSubscription {
  return ExpoLocalizationModule.addListener('onCalendarSettingsChanged', listener);
}

/**
 * @deprecated use subscription.remove() instead.
 */
export function removeSubscription(subscription: EventSubscription) {
  subscription.remove();
}

export default ExpoLocalizationModule;
Repository: expo/expo
File: packages/expo-modules-core/ios/Worklets/WorkletRuntimeFactory.h (C/C++)

Code before:
// Copyright 2025-present 650 Industries. All rights reserved.

@class EXAppContext;
@class EXRuntime;
@class EXJavaScriptValue;
@class EXJavaScriptRuntime;

@interface WorkletRuntimeFactory : NSObject

+(nonnull EXRuntime *)createWorkletRuntime:(nonnull EXAppContext *)appContext fromPointer:(unsigned long)pointer;

+(unsigned long)extractRuntimePointer:(nonnull EXJavaScriptValue *)jsValue runtime:(nonnull EXJavaScriptRuntime *)runtime;

@end


Review comment (refactor):
`void *` is more common

```suggestion
+ (nonnull EXRuntime *)createWorkletRuntime:(nonnull EXAppContext *)appContext fromPointer:(nonnull void *)pointer;

+ (nullable void *)extractRuntimePointer:(nonnull EXJavaScriptValue *)jsValue runtime:(nonnull EXJavaScriptRuntime *)runtime;
```

Code after:
// Copyright 2025-present 650 Industries. All rights reserved.

#import <ExpoModulesCore/EXRuntime.h>
#import <ExpoModulesJSI/EXJavaScriptValue.h>

@class EXAppContext;

@interface WorkletRuntimeFactory : NSObject

+ (nonnull EXRuntime *)createWorkletRuntime:(nonnull EXAppContext *)appContext fromPointer:(nullable void *)pointer;

+ (nullable void *)extractRuntimePointer:(nonnull EXJavaScriptValue *)jsValue runtime:(nonnull EXJavaScriptRuntime *)runtime;

@end
Repository: expo/expo
File: packages/expo-modules-core/ios/Worklets/WorkletRuntimeFactory.mm (Objective-C++)

Code before:
// Copyright 2025-present 650 Industries. All rights reserved.

#import <ExpoModulesCore/WorkletRuntimeFactory.h>
#import <ExpoModulesCore/EXRuntime.h>
#import <ExpoModulesJSI/EXJavaScriptValue.h>

#if WORKLETS_ENABLED

#include <worklets/WorkletRuntime/WorkletRuntime.h>
#include <ExpoModulesCore/WorkletJSCallInvoker.h>

#endif

@implementation WorkletRuntimeFactory

+(nonnull EXRuntime *)createWorkletRuntime:(nonnull EXAppContext *)appContext fromPointer:(unsigned long)pointer
{
#if WORKLETS_ENABLED
  jsi::Runtime* jsRuntime = reinterpret_cast<jsi::Runtime *>(pointer);

  auto workletRuntime = worklets::WorkletRuntime::getWeakRuntimeFromJSIRuntime(*jsRuntime);

  return [[EXRuntime alloc] initWithRuntime:*jsRuntime
                                callInvoker:std::make_shared<expo::WorkletJSCallInvoker>(workletRuntime)];
#else
  @throw [NSException exceptionWithName:@"WorkletException"
                                 reason:@"Worklets integration is disabled"
                               userInfo:nil];
#endif
}

+(unsigned long)extractRuntimePointer:(nonnull EXJavaScriptValue *)jsValue runtime:(nonnull EXJavaScriptRuntime *)runtime
{
  jsi::Value rawValue = [jsValue get];
  jsi::Runtime *rawRuntime = [runtime get];

  jsi::Object workletRuntimeObject = rawValue.getObject(*rawRuntime);
  if (!workletRuntimeObject.isArrayBuffer(*rawRuntime)) {
    return 0;
  }


Review comment (suggestion):
```suggestion
+ (nonnull EXRuntime *)createWorkletRuntime:(nonnull EXAppContext *)appContext fromPointer:(nonnull void *)pointer
```

Code after:
// Copyright 2025-present 650 Industries. All rights reserved.

#import <ExpoModulesCore/WorkletRuntimeFactory.h>

#if WORKLETS_ENABLED

#include <worklets/WorkletRuntime/WorkletRuntime.h>
#include <ExpoModulesCore/WorkletJSCallInvoker.h>

#endif

@implementation WorkletRuntimeFactory

+ (nonnull EXRuntime *)createWorkletRuntime:(nonnull EXAppContext *)appContext fromPointer:(nullable void *)pointer
{
#if WORKLETS_ENABLED
  jsi::Runtime* jsRuntime = reinterpret_cast<jsi::Runtime *>(pointer);

  auto workletRuntime = worklets::WorkletRuntime::getWeakRuntimeFromJSIRuntime(*jsRuntime);

  return [[EXRuntime alloc] initWithRuntime:*jsRuntime
                                callInvoker:std::make_shared<expo::WorkletJSCallInvoker>(workletRuntime)];
#else
  @throw [NSException exceptionWithName:@"WorkletException"
                                 reason:@"Worklets integration is disabled"
                               userInfo:nil];
#endif
}

+ (nullable void *)extractRuntimePointer:(nonnull EXJavaScriptValue *)jsValue runtime:(nonnull EXJavaScriptRuntime *)runtime
{
  jsi::Value rawValue = [jsValue get];
  jsi::Runtime *rawRuntime = [runtime get];

  jsi::Object workletRuntimeObject = rawValue.getObject(*rawRuntime);
  if (!workletRuntimeObject.isArrayBuffer(*rawRuntime)) {
    return NULL;
  }

  size_t pointerSize = sizeof(uintptr_t *);
  jsi::ArrayBuffer workletRuntimeArrayBuffer = workletRuntimeObject.getArrayBuffer(*rawRuntime);
Repository: expo/expo
File: packages/expo-modules-core/ios/Core/Modules/CoreModule.swift (Swift)

Code before:
// Copyright 2015-present 650 Industries. All rights reserved.

import React
import Foundation

internal final class WorkletUIRuntimeException: Exception, @unchecked Sendable {
  override var reason: String {
    "Cannot find UI worklet runtime"
  }
}

internal final class WorkletUIRuntimePointerExtractionException: Exception, @unchecked Sendable {
  override var reason: String {
    "Cannot extract pointer to UI worklet runtime"
  }
}

private let WORKLET_RUNTIME_KEY = "_WORKLET_RUNTIME"

// The core module that describes the `global.expo` object.
internal final class CoreModule: Module {
  internal func definition() -> ModuleDefinition {
    Constant("expoModulesCoreVersion") {
      let version = CoreModuleHelper.getVersion()
      let components = version.split(separator: "-")[0].split(separator: ".").compactMap { Int($0) }

      return [
        "version": version,
        "major": components[0],
        "minor": components[1],
        "patch": components[2]
      ]
    }

    Constant("cacheDir") {
      FileManager.default.urls(for: .cachesDirectory, in: .userDomainMask).first?.path ?? ""
    }

    Constant("documentsDir") {
      FileManager.default.urls(for: .documentDirectory, in: .userDomainMask).first?.path ?? ""
    }

Review comment (suggestion):
Maybe better to put exceptions at the end of the file

Code after:
// Copyright 2015-present 650 Industries. All rights reserved.

import React
import Foundation

private let WORKLET_RUNTIME_KEY = "_WORKLET_RUNTIME"

// The core module that describes the `global.expo` object.
internal final class CoreModule: Module {
  internal func definition() -> ModuleDefinition {
    Constant("expoModulesCoreVersion") {
      let version = CoreModuleHelper.getVersion()
      let components = version.split(separator: "-")[0].split(separator: ".").compactMap { Int($0) }

      return [
        "version": version,
        "major": components[0],
        "minor": components[1],
        "patch": components[2]
      ]
    }

    Constant("cacheDir") {
      FileManager.default.urls(for: .cachesDirectory, in: .userDomainMask).first?.path ?? ""
    }

    Constant("documentsDir") {
      FileManager.default.urls(for: .documentDirectory, in: .userDomainMask).first?.path ?? ""
    }

    Function("installOnUIRuntime") {
      guard let appContext else {
        throw Exceptions.AppContextLost()
      }

      // Check if was already installed
      if appContext._uiRuntime != nil {
        return
      }

      let runtime = try appContext.runtime