Skip to content

feat(cli): UTExportedTypeDeclarations support for file associations - #12

Open
tomerqodo wants to merge 2 commits into
qodo_full_base_featcli_utexportedtypedeclarations_support_for_file_associations_pr12from
qodo_full_head_featcli_utexportedtypedeclarations_support_for_file_associations_pr12
Open

tomerqodo wants to merge 2 commits into
qodo_full_base_featcli_utexportedtypedeclarations_support_for_file_associations_pr12from
qodo_full_head_featcli_utexportedtypedeclarations_support_for_file_associations_pr12

Conversation

@tomerqodo

Copy link
Copy Markdown

Benchmark PR from agentic-review-benchmarks#12

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (3) 📎 Requirement gaps (0)

Grey Divider


Action required

1. FileAssociation.name panics via expect 📘 Rule violation ⛯ Reliability
Description
• The new macOS Info.plist generation code calls `association.name.as_ref().expect("File association
  must have a name"), which will panic at runtime when name` is omitted in config.
• This violates the requirement to avoid panics for fallible/config-driven operations and prevents
  graceful degradation with actionable error context.
Code

crates/tauri-bundler/src/bundle/macos/app.rs[R348-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(),
            );
Evidence
The checklist requires fallible operations to return Result (not panic) and to handle edge cases
gracefully. The added code uses expect(...) on a config-derived optional field (name), which
will crash instead of returning an error with context.

Rule 3: Generic: Robust Error Handling and Edge Case Management
AGENTS.md
crates/tauri-bundler/src/bundle/macos/app.rs[348-356]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`create_info_plist` panics when a `FileAssociation` has no `name` due to `expect("File association must have a name")`. This is config-driven and should not crash the bundling process.

## Issue Context
`create_info_plist` already returns `crate::Result<()>`, so it can gracefully return an error with context, or it can derive a default name (e.g., from the first extension) as documented.

## Fix Focus Areas
- crates/tauri-bundler/src/bundle/macos/app.rs[348-356]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. CFBundleTypeExtensions condition inverted 📘 Rule violation ✓ Correctness
Description
• The code inserts CFBundleTypeExtensions only when association.ext.is_empty(), which results in
  non-empty extension lists not being written to the Info.plist.
• This can silently break file associations (a critical edge case) because the configuration is
  present but not applied.
Code

crates/tauri-bundler/src/bundle/macos/app.rs[R328-339]

+            if association.ext.is_empty() {
+              dict.insert(
+                "CFBundleTypeExtensions".into(),
+                plist::Value::Array(
+                  association
+                    .ext
+                    .iter()
+                    .map(|ext| ext.to_string().into())
+                    .collect(),
+                ),
+              );
+            }
Evidence
The checklist requires explicit handling of empty/boundary cases. The new condition uses
is_empty() where the insertion logically belongs to the non-empty case, causing silent
misconfiguration rather than robust handling of the boundary condition.

Rule 3: Generic: Robust Error Handling and Edge Case Management
crates/tauri-bundler/src/bundle/macos/app.rs[328-339]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`CFBundleTypeExtensions` is only written when `association.ext.is_empty()`, which prevents normal (non-empty) extension lists from being included in `CFBundleDocumentTypes`.

## Issue Context
This likely breaks file association registration on macOS for the common case where `ext` is provided.

## Fix Focus Areas
- crates/tauri-bundler/src/bundle/macos/app.rs[328-339]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. UTTypeConformsTo uses wrong field 📘 Rule violation ✓ Correctness
Description
• The UTExportedTypeDeclarations generation populates UTTypeConformsTo from
  association.content_types, ignoring the dedicated exported_type.conforms_to field.
• This can silently produce incorrect exported type declarations on macOS, making the feature behave
  unexpectedly without any error surfaced to users.
Code

crates/tauri-bundler/src/bundle/macos/app.rs[R281-289]

+          if let Some(description) = &association.description {
+            dict.insert("UTTypeDescription".into(), description.clone().into());
+          }
+          if let Some(content_types) = &association.content_types {
+            dict.insert(
+              "UTTypeConformsTo".into(),
+              plist::Value::Array(content_types.iter().map(|s| s.clone().into()).collect()),
+            );
+          }
Evidence
The checklist calls out avoiding silent failures and properly handling inputs. The config model
explicitly defines ExportedFileAssociation::conforms_to, but the plist generation uses a different
field (FileAssociation::content_types) for UTTypeConformsTo, effectively dropping/ignoring the
exported-type-specific input.

Rule 3: Generic: Robust Error Handling and Edge Case Management
crates/tauri-bundler/src/bundle/macos/app.rs[281-289]
crates/tauri-utils/src/config.rs[1209-1216]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`UTTypeConformsTo` is being built from `FileAssociation::content_types` instead of `ExportedFileAssociation::conforms_to`, which likely drops the intended exported type metadata.

## Issue Context
`content_types` maps to `LSItemContentTypes` in `CFBundleDocumentTypes`, while exported type conformance should come from `exported_type.conforms_to`.

## Fix Focus Areas
- crates/tauri-bundler/src/bundle/macos/app.rs[271-289]
- crates/tauri-utils/src/config.rs[1209-1216]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



ⓘ The new review experience is currently in Beta. Learn more

Qodo Logo

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.

Action required

1. fileassociation.name panics via expect 📘 Rule violation ⛯ Reliability

• The new macOS Info.plist generation code calls `association.name.as_ref().expect("File association
  must have a name"), which will panic at runtime when name` is omitted in config.
• This violates the requirement to avoid panics for fallible/config-driven operations and prevents
  graceful degradation with actionable error context.
Agent prompt
## Issue description
`create_info_plist` panics when a `FileAssociation` has no `name` due to `expect("File association must have a name")`. This is config-driven and should not crash the bundling process.

## Issue Context
`create_info_plist` already returns `crate::Result<()>`, so it can gracefully return an error with context, or it can derive a default name (e.g., from the first extension) as documented.

## Fix Focus Areas
- crates/tauri-bundler/src/bundle/macos/app.rs[348-356]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +328 to +339
if association.ext.is_empty() {
dict.insert(
"CFBundleTypeExtensions".into(),
plist::Value::Array(
association
.ext
.iter()
.map(|ext| ext.to_string().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.

Action required

2. cfbundletypeextensions condition inverted 📘 Rule violation ✓ Correctness

• The code inserts CFBundleTypeExtensions only when association.ext.is_empty(), which results in
  non-empty extension lists not being written to the Info.plist.
• This can silently break file associations (a critical edge case) because the configuration is
  present but not applied.
Agent prompt
## Issue description
`CFBundleTypeExtensions` is only written when `association.ext.is_empty()`, which prevents normal (non-empty) extension lists from being included in `CFBundleDocumentTypes`.

## Issue Context
This likely breaks file association registration on macOS for the common case where `ext` is provided.

## Fix Focus Areas
- crates/tauri-bundler/src/bundle/macos/app.rs[328-339]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +281 to +289
if let Some(description) = &association.description {
dict.insert("UTTypeDescription".into(), description.clone().into());
}
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.

Action required

3. uttypeconformsto uses wrong field 📘 Rule violation ✓ Correctness

• The UTExportedTypeDeclarations generation populates UTTypeConformsTo from
  association.content_types, ignoring the dedicated exported_type.conforms_to field.
• This can silently produce incorrect exported type declarations on macOS, making the feature behave
  unexpectedly without any error surfaced to users.
Agent prompt
## Issue description
`UTTypeConformsTo` is being built from `FileAssociation::content_types` instead of `ExportedFileAssociation::conforms_to`, which likely drops the intended exported type metadata.

## Issue Context
`content_types` maps to `LSItemContentTypes` in `CFBundleDocumentTypes`, while exported type conformance should come from `exported_type.conforms_to`.

## Fix Focus Areas
- crates/tauri-bundler/src/bundle/macos/app.rs[271-289]
- crates/tauri-utils/src/config.rs[1209-1216]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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.

1 participant