Skip to content

🧹 Remove unwrap() on required arguments in calendar helper #548

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jpoehnelt wants to merge 1 commit into from
Closed

🧹 Remove unwrap() on required arguments in calendar helper #548

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/remove-calendar-unwrap.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@googleworkspace/cli": patch
---

Remove unwrap() on required arguments in the calendar helper to improve error handling and prevent potential panics.
16 changes: 12 additions & 4 deletions src/helpers/calendar.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,10 +424,18 @@ fn build_insert_request(
matches: &ArgMatches,
doc: &crate::discovery::RestDescription,
) -> Result<(String, String, Vec<String>), GwsError> {
let calendar_id = matches.get_one::<String>("calendar").unwrap();
let summary = matches.get_one::<String>("summary").unwrap();
let start = matches.get_one::<String>("start").unwrap();
let end = matches.get_one::<String>("end").unwrap();
let calendar_id = matches
.get_one::<String>("calendar")
.ok_or_else(|| GwsError::Validation("Missing required argument: calendar".to_string()))?;
let summary = matches
.get_one::<String>("summary")
.ok_or_else(|| GwsError::Validation("Missing required argument: summary".to_string()))?;
let start = matches
.get_one::<String>("start")
.ok_or_else(|| GwsError::Validation("Missing required argument: start".to_string()))?;
let end = matches
.get_one::<String>("end")
.ok_or_else(|| GwsError::Validation("Missing required argument: end".to_string()))?;
Comment on lines +427 to +438
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 18, 2026

Choose a reason for hiding this comment

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

high

While replacing .unwrap() is generally good practice, in this specific case, the unwrap() calls were safe and would not panic. The clap argument parser is configured to ensure these values are always present:

  • summary, start, and end are marked as required(true).
  • calendar has a default_value("primary").

Because of this, matches.get_one() will always return Some(...) for these arguments, and clap would have already exited with an error if a required argument was missing. The added .ok_or_else(...) introduces error handling logic that is unreachable, which can be misleading for future readers. It's better to rely on the unwrap() here, which acts as a correct assertion that the value is present.

Suggested change
let calendar_id = matches
.get_one::<String>("calendar")
.ok_or_else(|| GwsError::Validation("Missing required argument: calendar".to_string()))?;
let summary = matches
.get_one::<String>("summary")
.ok_or_else(|| GwsError::Validation("Missing required argument: summary".to_string()))?;
let start = matches
.get_one::<String>("start")
.ok_or_else(|| GwsError::Validation("Missing required argument: start".to_string()))?;
let end = matches
.get_one::<String>("end")
.ok_or_else(|| GwsError::Validation("Missing required argument: end".to_string()))?;
let calendar_id = matches.get_one::<String>("calendar").unwrap();
let summary = matches.get_one::<String>("summary").unwrap();
let start = matches.get_one::<String>("start").unwrap();
let end = matches.get_one::<String>("end").unwrap();
References
  1. Avoid introducing changes that are outside the primary goal of a pull request to prevent scope creep. In this case, replacing safe unwrap() calls with unreachable error handling adds complexity without addressing a real issue, thus deviating from a truly beneficial improvement.

let location = matches.get_one::<String>("location");
let description = matches.get_one::<String>("description");
let attendees_vals = matches.get_many::<String>("attendee");
Expand Down
Loading