ronantakizawa/codereview-bench huggingface dataset:
Repository: tauri-apps/tauri
File: crates/tauri-runtime-wry/src/lib.rs (Rust)

Code before:
              active_tracing_spans: active_tracing_spans.clone(),
            },
            web_context,
          );
          if prevent_default {
            return;
          }
        }

        handle_event_loop(
          event,
          event_loop,
          control_flow,
          EventLoopIterationContext {
            callback: &mut callback,
            windows: windows.clone(),
            window_id_map: window_id_map.clone(),
            #[cfg(feature = "tracing")]
            active_tracing_spans: active_tracing_spans.clone(),
          },
        );
      });
  }

  fn run<F: FnMut(RunEvent<T>) + 'static>(self, callback: F) {
    self.event_loop.run(make_event_handler(&self, callback))
  }

  #[cfg(desktop)]
  fn run_return<F: FnMut(RunEvent<T>) + 'static>(mut self, callback: F) -> i32 {
    use tao::platform::run_return::EventLoopExtRunReturn;

    self
      .event_loop
      .run_return(make_event_handler(&self, callback))
  }
}

fn make_event_handler<T, F>(
  runtime: Wry<T>,
  mut callback: F,
) -> impl FnMut(Event<'_, T>, &EventLoopWindowTarget<T>, &mut ControlFlow)
where
  T: UserEvent,
  F: FnMut(RunEvent<T>) + 'static,
{
  let windows = self.context.main_thread.windows.clone();
  let window_id_map = self.context.window_id_map.clone();
  let web_context = self.context.main_thread.web_context.clone();
  let plugins = self.context.plugins.clone();


Review comment (suggestion):
`event_loop.run` take the partial ownship, so you can't borrow `&self`.

*I don't know why Tao took ownership; maybe it was for the sake of cross-platform API consistency?*

Code after:
              active_tracing_spans: active_tracing_spans.clone(),
            },
            web_context,
          );
          if prevent_default {
            return;
          }
        }

        handle_event_loop(
          event,
          event_loop,
          control_flow,
          EventLoopIterationContext {
            callback: &mut callback,
            windows: windows.clone(),
            window_id_map: window_id_map.clone(),
            #[cfg(feature = "tracing")]
            active_tracing_spans: active_tracing_spans.clone(),
          },
        );
      });
  }

  fn run<F: FnMut(RunEvent<T>) + 'static>(self, callback: F) {
    let event_handler = make_event_handler(&self, callback);

    self.event_loop.run(event_handler)
  }

  #[cfg(not(target_os = "ios"))]
  fn run_return<F: FnMut(RunEvent<T>) + 'static>(mut self, callback: F) -> i32 {
    use tao::platform::run_return::EventLoopExtRunReturn;

    let event_handler = make_event_handler(&self, callback);

    self.event_loop.run_return(event_handler)
  }

  #[cfg(target_os = "ios")]
  fn run_return<F: FnMut(RunEvent<T>) + 'static>(mut self, callback: F) -> i32 {
    self.run(callback);
    0
  }
}

fn make_event_handler<T, F>(
  runtime: &Wry<T>,
  mut callback: F,
) -> impl FnMut(Event<'_, Message<T>>, &EventLoopWindowTarget<Message<T>>, &mut ControlFlow)
where
Repository: tauri-apps/tauri
File: crates/tauri-bundler/src/bundle/macos/ios.rs (Rust)

Code before:
  )?;

  writeln!(
    file,
    "  <key>CFBundleIdentifier</key>\n  <string>{}</string>",
    settings.bundle_identifier()
  )?;
  writeln!(
    file,
    "  <key>CFBundleDisplayName</key>\n  <string>{}</string>",
    settings.product_name()
  )?;
  writeln!(
    file,
    "  <key>CFBundleName</key>\n  <string>{}</string>",
    settings.product_name()
  )?;
  writeln!(
    file,
    "  <key>CFBundleExecutable</key>\n  <string>{}</string>",
    settings.main_binary_name()?
  )?;
  writeln!(
    file,
    "  <key>CFBundleVersion</key>\n  <string>{}</string>",
    settings.bundle_version()
  )?;
  writeln!(
    file,
    "  <key>CFBundleShortVersionString</key>\n  <string>{}</string>",
    settings.version_string()
  )?;
  writeln!(
    file,
    "  <key>CFBundleDevelopmentRegion</key>\n  <string>en_US</string>"
  )?;

  if !icon_filenames.is_empty() {
    writeln!(file, "  <key>CFBundleIconFiles</key>\n  <array>")?;
    for filename in icon_filenames {
      writeln!(file, "    <string>{}</string>", filename)?;
    }
    writeln!(file, "  </array>")?;
  }
  writeln!(file, "  <key>LSRequiresIPhoneOS</key>\n  <true/>")?;
  writeln!(file, "</dict>\n</plist>")?;
  file.flush()?;
  Ok(())
}


Review comment (suggestion):
this iOS module here isn't used at all - we'll need a different logic to inject the bundle version for iOS

Code after:
  )?;

  writeln!(
    file,
    "  <key>CFBundleIdentifier</key>\n  <string>{}</string>",
    settings.bundle_identifier()
  )?;
  writeln!(
    file,
    "  <key>CFBundleDisplayName</key>\n  <string>{}</string>",
    settings.product_name()
  )?;
  writeln!(
    file,
    "  <key>CFBundleName</key>\n  <string>{}</string>",
    settings.product_name()
  )?;
  writeln!(
    file,
    "  <key>CFBundleExecutable</key>\n  <string>{}</string>",
    settings.main_binary_name()?
  )?;
  writeln!(
    file,
    "  <key>CFBundleVersion</key>\n  <string>{}</string>",
    settings
      .ios()
      .bundle_version
      .as_deref()
      .unwrap_or_else(|| settings.version_string())
  )?;
  writeln!(
    file,
    "  <key>CFBundleShortVersionString</key>\n  <string>{}</string>",
    settings.version_string()
  )?;
  writeln!(
    file,
    "  <key>CFBundleDevelopmentRegion</key>\n  <string>en_US</string>"
  )?;

  if !icon_filenames.is_empty() {
    writeln!(file, "  <key>CFBundleIconFiles</key>\n  <array>")?;
    for filename in icon_filenames {
      writeln!(file, "    <string>{}</string>", filename)?;
    }
    writeln!(file, "  </array>")?;
  }
  writeln!(file, "  <key>LSRequiresIPhoneOS</key>\n  <true/>")?;
  writeln!(file, "</dict>\n</plist>")?;
  file.flush()?;
Repository: tauri-apps/tauri
File: crates/tauri-utils/src/acl/schema.rs (Rust)

Code before:
    }

    // schema for each permission set
    for set in self.permission_sets() {
      let schema = Self::perm_id_schema(
        name,
        &set.identifier,
        Some(&add_permissions_to_description(
          &set.description,
          &set.permissions,
        )),
      );
      permission_schemas.push(schema);
    }

    // schema for each permission
    for perm in self.permissions() {
      let schema = Self::perm_id_schema(name, &perm.identifier, perm.description.as_deref());
      permission_schemas.push(schema);
    }

    permission_schemas
  }
}

fn add_permissions_to_description(description: &str, permissions: &[String]) -> String {
  if permissions.is_empty() {
    return description.to_string();
  }
  let permissions_list = permissions
    .iter()
    .map(|permission| format!("- `{permission}`"))
    .collect::<Vec<_>>()
    .join("\n");
  format!("{description}\n\n{permissions_list}")
}

impl<'a>
  PermissionSchemaGenerator<
    'a,
    Values<'a, std::string::String, PermissionSet>,
    Values<'a, std::string::String, Permission>,
  > for Manifest
{
  fn has_default_permission_set(&self) -> bool {
    self.default_permission.is_some()
  }

  fn default_set_description(&self) -> Option<&str> {
    self
      .default_permission

Review comment (security):
ideally this could be a default method on the trait `PermissionSchemaGenerator`

```rs
trait PermissionSchemaGenerator {
    fn default_set_description_with_permissions(&self) -> String;
        // then here you can use `self.default_set_description()`
        // and `self.default_set_permissions()`
    }
}
```

Code after:
      }
    }

    // schema for each permission set
    for set in self.permission_sets() {
      let description = add_permissions_to_description(&set.description, &set.permissions, false);
      let schema = Self::perm_id_schema(name, &set.identifier, Some(&description));
      permission_schemas.push(schema);
    }

    // schema for each permission
    for perm in self.permissions() {
      let schema = Self::perm_id_schema(name, &perm.identifier, perm.description.as_deref());
      permission_schemas.push(schema);
    }

    permission_schemas
  }
}

fn add_permissions_to_description(
  description: &str,
  permissions: &[String],
  is_default: bool,
) -> String {
  if permissions.is_empty() {
    return description.to_string();
  }
  let permissions_list = permissions
    .iter()
    .map(|permission| format!("- `{permission}`"))
    .collect::<Vec<_>>()
    .join("\n");
  let default_permission_set = if is_default {
    "default permission set"
  } else {
    "permission set"
  };
  format!("{description}\n#### This {default_permission_set} includes:\n\n{permissions_list}")
}

impl<'a>
  PermissionSchemaGenerator<
    'a,
    Values<'a, std::string::String, PermissionSet>,
    Values<'a, std::string::String, Permission>,
  > for Manifest
{
  fn has_default_permission_set(&self) -> bool {
    self.default_permission.is_some()
  }
Repository: tauri-apps/tauri
File: crates/tauri-bundler/src/bundle.rs (Rust)

Code before:
// Copyright 2016-2019 Cargo-Bundle developers <https://github.com/burtonageo/cargo-bundle>
// Copyright 2019-2024 Tauri Programme within The Commons Conservancy
// SPDX-License-Identifier: Apache-2.0
// SPDX-License-Identifier: MIT

mod category;
#[cfg(target_os = "linux")]
mod linux;
#[cfg(target_os = "macos")]
mod macos;
mod platform;
mod settings;
mod updater_bundle;
mod windows;

impl From<goblin::error::Error> for crate::error::Error {
  fn from(value: goblin::error::Error) -> Self {
    crate::Error::BinaryParseError(value.to_string())
  }
}

/// Patch a binary with bundle type information
fn patch_binary(binary: &PathBuf, package_type: &PackageType) -> crate::Result<()> {
  log::info!(
    "Patching binary {:?} for type {}",
    binary,
    package_type.short_name()
  );
  #[cfg(target_os = "linux")]
  linux::patch_binary(binary, package_type)?;

  #[cfg(target_os = "windows")]
  windows::patch_binary(binary, package_type)?;

  Ok(())
}

use tauri_utils::display_path;

pub use self::{
  category::AppCategory,

Review comment (suggestion):
this can be done inside crate::error::Error::BinaryParseError by leverating thiserror `#[from]` attribute

Code after:
// Copyright 2016-2019 Cargo-Bundle developers <https://github.com/burtonageo/cargo-bundle>
// Copyright 2019-2024 Tauri Programme within The Commons Conservancy
// SPDX-License-Identifier: Apache-2.0
// SPDX-License-Identifier: MIT

mod category;
#[cfg(target_os = "linux")]
mod linux;
#[cfg(target_os = "macos")]
mod macos;
mod platform;
mod settings;
mod updater_bundle;
mod windows;

use tauri_utils::{display_path, platform::Target as TargetPlatform};

/// Patch a binary with bundle type information
fn patch_binary(binary: &PathBuf, package_type: &PackageType) -> crate::Result<()> {
  match package_type {
    #[cfg(target_os = "linux")]
    PackageType::AppImage | PackageType::Deb | PackageType::Rpm => {
      log::info!(
        "Patching binary {:?} for type {}",
        binary,
        package_type.short_name()
      );
      linux::patch_binary(binary, package_type)?;
    }
    PackageType::Nsis | PackageType::WindowsMsi => {
      log::info!(
        "Patching binary {:?} for type {}",
        binary,
        package_type.short_name()
      );
      windows::patch_binary(binary, package_type)?;
    }
    _ => (),
  }

  Ok(())
Repository: tauri-apps/tauri
File: crates/tauri-runtime-wry/src/dialog/windows.rs (Rust)

Code before:
// Copyright 2019-2024 Tauri Programme within The Commons Conservancy
// SPDX-License-Identifier: Apache-2.0
// SPDX-License-Identifier: MIT

use windows::core::{w, HSTRING, PCWSTR};

enum Level {
  Error,
  #[allow(unused)]
  Warning,
  #[allow(unused)]
  Info,
}

pub fn error<S: AsRef<str>>(err: S) {
  dialog_inner(err, Level::Error);
}

fn dialog_inner<S: AsRef<str>>(err: S, level: Level) {
  let err = err.as_ref();

  let title = match level {
    Level::Warning => w!("Warning"),
    Level::Error => w!("Error"),
    Level::Info => w!("Info"),
  };

  #[cfg(not(feature = "common-controls-v6"))]
  {
    use windows::Win32::UI::WindowsAndMessaging::*;

    let err = remove_hyperlink(err);
    let err = HSTRING::from(err);
    let err = PCWSTR(err.as_ptr());

    unsafe {
      MessageBoxW(
        None,
        err,
        title,
        match level {
          Level::Warning => MB_ICONWARNING,
          Level::Error => MB_ICONERROR,
          Level::Info => MB_ICONINFORMATION,
        },
      )
    };

Review comment (suggestion):
```suggestion
pub fn error<S: AsRef<str>>(err: S) {
  dialog_inner(err.as_ref(), Level::Error);
}

fn dialog_inner(err: &str, level: Level) {
  let title = match level {
```

Code after:
// Copyright 2019-2024 Tauri Programme within The Commons Conservancy
// SPDX-License-Identifier: Apache-2.0
// SPDX-License-Identifier: MIT

use windows::core::{w, HSTRING, PCWSTR};

enum Level {
  Error,
  #[allow(unused)]
  Warning,
  #[allow(unused)]
  Info,
}

pub fn error<S: AsRef<str>>(err: S) {
  dialog_inner(err.as_ref(), Level::Error);
}

fn dialog_inner(err: &str, level: Level) {
  let title = match level {
    Level::Warning => w!("Warning"),
    Level::Error => w!("Error"),
    Level::Info => w!("Info"),
  };

  #[cfg(not(feature = "common-controls-v6"))]
  {
    use windows::Win32::UI::WindowsAndMessaging::*;

    let err = remove_hyperlink(err);
    let err = HSTRING::from(err);

    unsafe {
      MessageBoxW(
        None,
        err,
        title,
        match level {
          Level::Warning => MB_ICONWARNING,
          Level::Error => MB_ICONERROR,
          Level::Info => MB_ICONINFORMATION,
        },
      )
    };
  }

  #[cfg(feature = "common-controls-v6")]
Repository: tauri-apps/tauri
File: crates/tauri/src/window/mod.rs (Rust)

Code before:
        #[cfg(desktop)]
        window_menu,
      );

      if let Some(webview) = detached_window.webview {
        app_manager.webview.attach_webview(
          window.clone(),
          webview.webview,
          webview.use_https_scheme,
        );
      }

      window
    })?;

    #[cfg(desktop)]
    if let Some(handler) = self.on_menu_event {
      window.on_menu_event(handler);
    }

    let app_manager = self.manager.manager_owned();
    let window_label = window.label().to_string();
    let window_ = window.clone();
    // run on the main thread to fix a deadlock on webview.eval if the tracing feature is enabled
    let _ = window.run_on_main_thread(move || {
      let _ = crate::vibrancy::set_window_effects(&window_, self.window_effects);
      let event = crate::EventName::from_str("tauri://window-created");
      let payload = Some(crate::webview::CreatedEvent {
        label: window_label,
      });
      let _ = app_manager.emit(event, EmitPayload::Serialize(&payload));
    });

    Ok(window)
  }
}

/// Desktop APIs.
#[cfg(desktop)]
#[cfg_attr(not(feature = "unstable"), allow(dead_code))]
impl<'a, R: Runtime, M: Manager<R>> WindowBuilder<'a, R, M> {
  /// Sets the menu for the window.
  #[must_use]
  pub fn menu(mut self, menu: Menu<R>) -> Self {
    self.menu.replace(menu);
    self
  }

  /// Show window in the center of the screen.
  #[must_use]
  pub fn center(mut self) -> Self {

Review comment (suggestion):
I think we should still check if `self.window_effects` is some, or we'll be calling `clear_effects` unnecessarily

Code after:
        #[cfg(desktop)]
        window_menu,
      );

      if let Some(webview) = detached_window.webview {
        app_manager.webview.attach_webview(
          window.clone(),
          webview.webview,
          webview.use_https_scheme,
        );
      }

      window
    })?;

    #[cfg(desktop)]
    if let Some(handler) = self.on_menu_event {
      window.on_menu_event(handler);
    }

    let app_manager = self.manager.manager_owned();
    let window_label = window.label().to_string();
    let window_ = window.clone();
    // run on the main thread to fix a deadlock on webview.eval if the tracing feature is enabled
    let _ = window.run_on_main_thread(move || {
      if let Some(effects) = self.window_effects {
        _ = crate::vibrancy::set_window_effects(&window_, Some(effects));
      }
      let event = crate::EventName::from_str("tauri://window-created");
      let payload = Some(crate::webview::CreatedEvent {
        label: window_label,
      });
      let _ = app_manager.emit(event, EmitPayload::Serialize(&payload));
    });

    Ok(window)
  }
}

/// Desktop APIs.
#[cfg(desktop)]
#[cfg_attr(not(feature = "unstable"), allow(dead_code))]
impl<'a, R: Runtime, M: Manager<R>> WindowBuilder<'a, R, M> {
  /// Sets the menu for the window.
  #[must_use]
  pub fn menu(mut self, menu: Menu<R>) -> Self {
    self.menu.replace(menu);
    self
  }

  /// Show window in the center of the screen.
Repository: tauri-apps/tauri
File: crates/tauri-runtime-wry/src/lib.rs (Rust)

Code before:
    webview_getter!(self, WebviewMessage::Cookies)?
  }

  fn set_cookie(&self, cookie: &Cookie<'_>) -> Result<()> {
    send_user_message(
      &self.context,
      Message::Webview(
        *self.window_id.lock().unwrap(),
        self.webview_id,
        WebviewMessage::SetCookie(cookie.clone().into_owned()),
      ),
    )?;
    Ok(())
  }

  fn delete_cookie(&self, cookie: &Cookie<'_>) -> Result<()> {
    send_user_message(
      &self.context,
      Message::Webview(
        *self.window_id.lock().unwrap(),
        self.webview_id,
        WebviewMessage::DeleteCookie(cookie.clone().into_owned()),
      ),
    )?;
    Ok(())
  }

  fn set_auto_resize(&self, auto_resize: bool) -> Result<()> {
    send_user_message(
      &self.context,
      Message::Webview(
        *self.window_id.lock().unwrap(),
        self.webview_id,
        WebviewMessage::SetAutoResize(auto_resize),
      ),
    )
  }

  #[cfg(all(feature = "tracing", not(target_os = "android")))]
  fn eval_script<S: Into<String>>(&self, script: S) -> Result<()> {
    // use a channel so the EvaluateScript task uses the current span as parent
    let (tx, rx) = channel();
    getter!(
      self,
      rx,
      Message::Webview(
        *self.window_id.lock().unwrap(),
        self.webview_id,
        WebviewMessage::EvaluateScript(script.into(), tx, tracing::Span::current()),
      )
    )

Review comment (performance):
I've changed my mind. I think the signature here should use `cookie: Cookie<'static>` instead of implicitly cloning it.
If we change our minds in the future, we can still optimize it using `AsRef`.

But I'm going to sleep now.

Code after:
    webview_getter!(self, WebviewMessage::Cookies)?
  }

  fn set_cookie(&self, cookie: Cookie<'_>) -> Result<()> {
    send_user_message(
      &self.context,
      Message::Webview(
        *self.window_id.lock().unwrap(),
        self.webview_id,
        WebviewMessage::SetCookie(cookie.into_owned()),
      ),
    )?;
    Ok(())
  }

  fn delete_cookie(&self, cookie: Cookie<'_>) -> Result<()> {
    send_user_message(
      &self.context,
      Message::Webview(
        *self.window_id.lock().unwrap(),
        self.webview_id,
        WebviewMessage::DeleteCookie(cookie.clone().into_owned()),
      ),
    )?;
    Ok(())
  }

  fn set_auto_resize(&self, auto_resize: bool) -> Result<()> {
    send_user_message(
      &self.context,
      Message::Webview(
        *self.window_id.lock().unwrap(),
        self.webview_id,
        WebviewMessage::SetAutoResize(auto_resize),
      ),
    )
  }

  #[cfg(all(feature = "tracing", not(target_os = "android")))]
  fn eval_script<S: Into<String>>(&self, script: S) -> Result<()> {
    // use a channel so the EvaluateScript task uses the current span as parent
    let (tx, rx) = channel();
    getter!(
      self,
      rx,
      Message::Webview(
        *self.window_id.lock().unwrap(),
        self.webview_id,
        WebviewMessage::EvaluateScript(script.into(), tx, tracing::Span::current()),
      )
    )
Repository: tauri-apps/tauri
File: crates/tauri-runtime-wry/src/lib.rs (Rust)

Code before:
      }
    }
  }
}

pub struct WindowWrapper {
  label: String,
  inner: Option<Arc<Window>>,
  // whether this window has child webviews
  // or it's just a container for a single webview
  has_children: AtomicBool,
  webviews: Vec<WebviewWrapper>,
  window_event_listeners: WindowEventListeners,
  #[cfg(windows)]
  background_color: Option<tao::window::RGBA>,
  #[cfg(windows)]
  is_window_transparent: bool,
  #[cfg(windows)]
  surface: Option<softbuffer::Surface<Arc<Window>, Arc<Window>>>,
  focused_webview: Arc<Mutex<Option<String>>>,
}

impl WindowWrapper {
  pub fn label(&self) -> String {
    self.label.clone()
  }
}

impl fmt::Debug for WindowWrapper {
  fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
    f.debug_struct("WindowWrapper")
      .field("label", &self.label)
      .field("inner", &self.inner)
      .finish()
  }
}

#[derive(Debug, Clone)]
pub struct EventProxy<T: UserEvent>(TaoEventLoopProxy<Message<T>>);

#[cfg(target_os = "ios")]
#[allow(clippy::non_send_fields_in_send_ty)]
unsafe impl<T: UserEvent> Sync for EventProxy<T> {}

impl<T: UserEvent> EventLoopProxy<T> for EventProxy<T> {
  fn send_event(&self, event: T) -> Result<()> {
    self
      .0
      .send_event(Message::UserEvent(event))
      .map_err(|_| Error::EventLoopClosed)
  }

Review comment (suggestion):
```suggestion
  pub fn label(&self) -> &str {
    &self.label
  }
```

Don't really care much, but I think it should probably be something like this

Code after:
      }
    }
  }
}

pub struct WindowWrapper {
  label: String,
  inner: Option<Arc<Window>>,
  // whether this window has child webviews
  // or it's just a container for a single webview
  has_children: AtomicBool,
  webviews: Vec<WebviewWrapper>,
  window_event_listeners: WindowEventListeners,
  #[cfg(windows)]
  background_color: Option<tao::window::RGBA>,
  #[cfg(windows)]
  is_window_transparent: bool,
  #[cfg(windows)]
  surface: Option<softbuffer::Surface<Arc<Window>, Arc<Window>>>,
  focused_webview: Arc<Mutex<Option<String>>>,
}

impl WindowWrapper {
  pub fn label(&self) -> &str {
    &self.label
  }
}

impl fmt::Debug for WindowWrapper {
  fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
    f.debug_struct("WindowWrapper")
      .field("label", &self.label)
      .field("inner", &self.inner)
      .finish()
  }
}

#[derive(Debug, Clone)]
pub struct EventProxy<T: UserEvent>(TaoEventLoopProxy<Message<T>>);

#[cfg(target_os = "ios")]
#[allow(clippy::non_send_fields_in_send_ty)]
unsafe impl<T: UserEvent> Sync for EventProxy<T> {}

impl<T: UserEvent> EventLoopProxy<T> for EventProxy<T> {
  fn send_event(&self, event: T) -> Result<()> {
    self
      .0
      .send_event(Message::UserEvent(event))
      .map_err(|_| Error::EventLoopClosed)
  }