Skip to content

Code Review Bench PR #14128 - feat(cli): UTExportedTypeDeclarations support for file associations#12

Closed
ketkarameya wants to merge 2 commits intobase_pr_14128_20260125_2192from
corrupted_pr_14128_20260125_2192
Closed

Code Review Bench PR #14128 - feat(cli): UTExportedTypeDeclarations support for file associations#12
ketkarameya wants to merge 2 commits intobase_pr_14128_20260125_2192from
corrupted_pr_14128_20260125_2192

Conversation

@ketkarameya
Copy link
Copy Markdown

Benchmark PR for Gitar evaluation

Original PR: agentic-review-benchmarks#12

feat(cli): UTExportedTypeDeclarations support for file associations

),
);

if association.ext.is_empty() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 Bug: Inverted condition: CFBundleTypeExtensions added only when empty

The condition on line 328 is if association.ext.is_empty() which inserts CFBundleTypeExtensions only when the extensions list is empty — yielding an empty array. When extensions are present (the common case), they are silently omitted from the plist.

This is the exact opposite of the intended behavior. The original code unconditionally inserted the extensions. The new code was meant to make it conditional (skip when empty), but the negation is missing.

Impact: All macOS file associations with extensions will be broken — CFBundleTypeExtensions will never appear in the generated Info.plist for associations that actually have extensions, meaning macOS won't associate those file types with the application.

Was this helpful? React with 👍 / 👎

Suggested change
if association.ext.is_empty() {
if !association.ext.is_empty() {
  • Apply suggested fix

Comment on lines +284 to +289
if let Some(content_types) = &association.content_types {
dict.insert(
"UTTypeConformsTo".into(),
plist::Value::Array(content_types.iter().map(|s| s.clone().into()).collect()),
);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 Bug: Wrong field used for UTTypeConformsTo in exported type

In the UTExportedTypeDeclarations generation (lines 284-289), the code uses association.content_types to populate UTTypeConformsTo. However, content_types maps to LSItemContentTypes and represents what content types the app handles. The correct field is exported_type.conforms_to, which is the dedicated field on ExportedFileAssociation for this purpose.

The ExportedFileAssociation struct has a conforms_to: Option<Vec<String>> field (line 1216 in config.rs) that is never read anywhere in the bundler code. Meanwhile, the example config in tauri.conf.json puts conformsTo inside exportedType, confirming it should be exported_type.conforms_to.

Impact: The UTTypeConformsTo plist entry will never be generated correctly. Even when conformsTo is configured in the exported type, it is completely ignored. If contentTypes happens to be set on the association, its values will be incorrectly used as conformance declarations.

Was this helpful? React with 👍 / 👎

Suggested change
if let Some(content_types) = &association.content_types {
dict.insert(
"UTTypeConformsTo".into(),
plist::Value::Array(content_types.iter().map(|s| s.clone().into()).collect()),
);
}
if let Some(conforms_to) = &exported_type.conforms_to {
dict.insert(
"UTTypeConformsTo".into(),
plist::Value::Array(conforms_to.iter().map(|s| s.clone().into()).collect()),
);
}
  • Apply suggested fix

Comment on lines 348 to 356
dict.insert(
"CFBundleTypeName".into(),
association
.name
.as_ref()
.unwrap_or(&association.ext[0].0)
.expect("File association must have a name")
.to_string()
.into(),
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 Bug: Panic when file association has no name (was a fallback)

Line 353 changed from .unwrap_or(&association.ext[0].0) (a graceful fallback to the first extension) to .expect("File association must have a name") (a panic). The name field is Option<String> and its doc comment says "Default to ext[0]", indicating it was designed to be optional.

The new example entries added in this very PR (taurijson and taurid in tauri.conf.json) don't specify a name field, so this code will panic at runtime when processing those associations.

Impact: Building any macOS bundle with a file association that omits the name field will crash. This includes the example added in this PR.

Was this helpful? React with 👍 / 👎

Suggested change
dict.insert(
"CFBundleTypeName".into(),
association
.name
.as_ref()
.unwrap_or(&association.ext[0].0)
.expect("File association must have a name")
.to_string()
.into(),
);
dict.insert(
"CFBundleTypeName".into(),
association
.name
.as_ref()
.or(association.ext.first().map(|e| &e.0))
.cloned()
.unwrap_or_default()
.into(),
);
  • Apply suggested fix

@gitar-bot-staging
Copy link
Copy Markdown

Code Review 🚫 Blocked 0 resolved / 3 findings

Three critical bugs in the macOS bundler: (1) inverted is_empty() condition means CFBundleTypeExtensions is only written when there are no extensions, (2) UTTypeConformsTo reads from the wrong field (content_types instead of exported_type.conforms_to), and (3) .expect() replaces a graceful fallback causing a panic when name is omitted — which the PR's own example triggers.

🚨 Bug: Inverted condition: CFBundleTypeExtensions added only when empty

📄 crates/tauri-bundler/src/bundle/macos/app.rs:328

The condition on line 328 is if association.ext.is_empty() which inserts CFBundleTypeExtensions only when the extensions list is empty — yielding an empty array. When extensions are present (the common case), they are silently omitted from the plist.

This is the exact opposite of the intended behavior. The original code unconditionally inserted the extensions. The new code was meant to make it conditional (skip when empty), but the negation is missing.

Impact: All macOS file associations with extensions will be broken — CFBundleTypeExtensions will never appear in the generated Info.plist for associations that actually have extensions, meaning macOS won't associate those file types with the application.

Suggested fix
            if !association.ext.is_empty() {
🚨 Bug: Wrong field used for UTTypeConformsTo in exported type

📄 crates/tauri-bundler/src/bundle/macos/app.rs:284-289 📄 crates/tauri-utils/src/config.rs:1215-1216

In the UTExportedTypeDeclarations generation (lines 284-289), the code uses association.content_types to populate UTTypeConformsTo. However, content_types maps to LSItemContentTypes and represents what content types the app handles. The correct field is exported_type.conforms_to, which is the dedicated field on ExportedFileAssociation for this purpose.

The ExportedFileAssociation struct has a conforms_to: Option<Vec<String>> field (line 1216 in config.rs) that is never read anywhere in the bundler code. Meanwhile, the example config in tauri.conf.json puts conformsTo inside exportedType, confirming it should be exported_type.conforms_to.

Impact: The UTTypeConformsTo plist entry will never be generated correctly. Even when conformsTo is configured in the exported type, it is completely ignored. If contentTypes happens to be set on the association, its values will be incorrectly used as conformance declarations.

Suggested fix
            if let Some(conforms_to) = &exported_type.conforms_to {
              dict.insert(
                "UTTypeConformsTo".into(),
                plist::Value::Array(conforms_to.iter().map(|s| s.clone().into()).collect()),
              );
            }
🚨 Bug: Panic when file association has no name (was a fallback)

📄 crates/tauri-bundler/src/bundle/macos/app.rs:348-356

Line 353 changed from .unwrap_or(&association.ext[0].0) (a graceful fallback to the first extension) to .expect("File association must have a name") (a panic). The name field is Option<String> and its doc comment says "Default to ext[0]", indicating it was designed to be optional.

The new example entries added in this very PR (taurijson and taurid in tauri.conf.json) don't specify a name field, so this code will panic at runtime when processing those associations.

Impact: Building any macOS bundle with a file association that omits the name field will crash. This includes the example added in this PR.

Suggested fix
              dict.insert(
                "CFBundleTypeName".into(),
                association
                  .name
                  .as_ref()
                  .or(association.ext.first().map(|e| &e.0))
                  .cloned()
                  .unwrap_or_default()
                  .into(),
              );
Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@kayagokalp kayagokalp closed this Feb 25, 2026
@kayagokalp kayagokalp deleted the corrupted_pr_14128_20260125_2192 branch February 25, 2026 02:09
@kayagokalp kayagokalp restored the corrupted_pr_14128_20260125_2192 branch February 25, 2026 02:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants