Common JS bugs are eminently avoidable in Rust

I was working on a Node.js application the other day and came across some code that looked like this:

function formattedName(person) {
  if (person.firstName && person.lastName) {
    return `${person.firstName} ${person.lastName}`;
  } else if (person.firstName && !person.lastName) {
    return person.firstName;
  } else if (!person.firstName && person.lastName) {
    person.lastName;
  }

  return "";
}

Can you spot the problems with it? I found and fixed at least two (but perhaps there are more that I missed). This code didn’t have a unit test, though if it had I suspect both of these problems would have been caught right away!

The first issue (the one that forced me to look at this code in the first place) is with the person variable. The code checks to make sure that firstName and lastName exist before using them, but it never checks to make sure that person is actually defined. Funny enough, when it’s not defined, the function throws an exception. The first fix was to add a guard clause:

function formattedName(person) {
  if (!person) {
    return "";
  }

  if (person.firstName && person.lastName) {
    return `${person.firstName} ${person.lastName}`;
  } else if (person.firstName && !person.lastName) {
    return person.firstName;
  } else if (!person.firstName && person.lastName) {
    person.lastName;
  }

  return "";
}

The second issue is with the second else if: there’s no return statement in there. Even if the code goes into that branch, it doesn’t matter because we swiftly exit it and go to the return '' statement anyway! That whole branch is totally useless.

It was an easy fix though—I just had to add a return:

function formattedName(person) {
  if (!person) {
    return "";
  }

  if (person.firstName && person.lastName) {
    return `${person.firstName} ${person.lastName}`;
  } else if (person.firstName && !person.lastName) {
    return person.firstName;
  } else if (!person.firstName && person.lastName) {
    return person.lastName;
  }

  return "";
}

Personally, I still don’t love this code and I think it could be simplified further (for instance, I think it would be better to have one return statement outside of the guard clause rather than four), but it’s not bad. At least everything is checked before it’s dereferenced now, and there’s a sensible default value.

However, as I was fixing it I couldn’t help but think that neither defect would have happened if we were using Rust. In general, the language constructs and style make it much easier to do the right thing.

For one, there’s no null or undefined in Rust. That whole concept is expressed through the type system using the Option type. Rather than the syntax allowing you to presume that something’s there and needing to check that there’s not, the Option tells you to presume that something isn’t there and check that there is. So in JavaScript, you might forget to do the check (which is what happened in this code). But in Rust, there’s no forgetting. You have an Option and the compiler will force you to be explicit about how to work with the value it may or may not contain.

Second, while it’s still possible to write if statements where one branch doesn’t return early, the idiomatic approach would be to use a match statement as the last expression in the function. A match statement must be exhaustive, so we can roll our catch-all return '' line right into it. Then the function can just return whatever comes out of the match without the possibility of skipping one of the branches, like we did in our Node code.

I’ll show how this code would look in idiomatic Rust. But before I show the idiomatic version, let me start by showing how the original broken code would look in Rust if we wrote it like we write JavaScript:

struct Person {
    first_name: Option<String>,
    last_name: Option<String>,
}

fn formatted_name(maybe_person: Option<Person>) -> String {
    let person = maybe_person.unwrap();

    if person.first_name.is_some() && person.last_name.is_some() {
      return format!("{} {}", person.first_name.unwrap(), person.last_name.unwrap());
    } else if person.first_name.is_some() && person.last_name.is_none() {
      return person.first_name.unwrap();
    } else if person.first_name.is_none() && person.last_name.is_some() {
      person.last_name.unwrap();
    }

    String::new()
}

This code actually compiles! It has the same two issues that our Node code did, but it does give us two bits of help that our Node code didn’t: the Option and the .unwrap() call to get to our person. You can see that I wrapped the incoming person argument in an Option to signify that it’s a nullable value (along with its two fields, first_name and last_name). That’s a signpost to a developer that this value needs to be treated with care! If you unwrap it without checking whether a person is present, your code will panic and crash. Sometimes you want that to happen, for instance, if a config value isn’t present at startup. But during normal code execution, that’s not usually what you want. So including that line let person = maybe_person.unwrap() is a really bad idea, yet that’s exactly what we do any time we use a JavaScript variable without checking it first! The difference with Rust is it tells you a variable might be undefined through its type, whereas JavaScript forces the developer to juggle that in their head without saying it explicitly. (I think this is one reason TypeScript is growing in popularity as developers are realizing this is a big problem).

Now here’s how the broken function looks in (slightly more) idiomatic Rust:

fn formatted_name(maybe_person: Option<Person>) -> String {
    match maybe_person {
        Some(Person { first_name: Some(first_name), last_name: Some(last_name) }) => {
            return format!("{} {}", first_name, last_name);
        }
        Some(Person { first_name: Some(first_name), last_name: None }) => {
            return first_name;
        }
        Some(Person { first_name: None, last_name: Some(last_name) }) => last_name,
    };
    String::new()
}

You can see because of our Option type, and because we stopped using .unwrap(), there’s no place in the code where we’re trying to use a value that doesn’t exist. This is a great step forward! We’re only building strings out of values if they’re there. We also have our String::new() at the end of the function, as we did in our Node code with return '', which will return an empty string if the match doesn’t cause us to bail early.

This code, however, does not compile. It complains with error[E0004]: non-exhaustive patterns: None not covered. In short, that error means that our match only covered some of the possible states maybe_person could be in, but it didn’t cover all of them. Namely, we did not cover the situation where maybe_person is None!

This is one of the wonderful things about the match statement: it’s a compiler-enforced way of doing what we were already trying to do with our last line, String::new(). We want a catch-all scenario in case our person doesn’t have all the data we’d like, but rather than having that catch-all dangling on its own, match would have us bring it into the different situations it’s already accounting for. Imagine: if we hadn’t already been thinking ahead about a catch-all with String::new(), what would our code have done if maybe_person wasn’t there? Or if neither first_name nor last_name were filled out? Rather than get into ambiguous, undefined behavior, the Rust designers decided it would be better to have match be exhaustive. That means a developer must be explicit about how to handle each case their code can encounter. In my opinion, it makes error handling much better. The compiler tells you when there’s a case you might not have accounted for!

Addressing our error, here’s our new code:

fn formatted_name(maybe_person: Option<Person>) -> String {
    match maybe_person {
        Some(Person { first_name: Some(first_name), last_name: Some(last_name) }) => {
            return format!("{} {}", first_name, last_name);
        }
        Some(Person { first_name: Some(first_name), last_name: None }) => {
            return first_name;
        }
        Some(Person { first_name: None, last_name: Some(last_name) }) => last_name,
        None => String::new(),
    }
}

Note that since the final statement of our function, String::new(), was moved into the match statement, we had to remove the semi-colon at the end of the match. That allows the output from our match to serve as the output from our function since Rust automatically returns the last thing evaluated in a function.

However, this new function doesn’t compile either! Now it tells us error[E0004]: non-exhaustive patterns: Some(Person { first_name: None, last_name: None }) not covered. We missed the case where first_name and last_name might not exist! If we go ahead and add that branch, the code looks like this:

fn formatted_name(maybe_person: Option<Person>) -> String {
    match maybe_person {
        Some(Person { first_name: Some(first_name), last_name: Some(last_name) }) => {
            return format!("{} {}", first_name, last_name);
        }
        Some(Person { first_name: Some(first_name), last_name: None }) => {
            return first_name;
        }
        Some(Person { first_name: None, last_name: Some(last_name) }) => last_name,
        Some(Person { first_name: None, last_name: None }) => String::new(),
        None => String::new(),
    }
}

The code compiles finally! At this point, we’re sure that we’re returning a string in all possible cases. Also, since the match is the last statement and each branch is mutually exclusive, we’re still avoiding the error with the early returns that we had in our JavaScript code. We’re not simply stepping over the branch where first_name is missing but last_name is present.

We can make a few more small edits to clean up this code, and we finally get an idiomatic, error-free Rust version of this function:

fn formatted_name(maybe_person: Option<Person>) -> String {
    match maybe_person {
        Some(Person { first_name: Some(first_name), last_name: Some(last_name) }) => format!("{} {}", first_name, last_name),
        Some(Person { first_name: Some(first_name), last_name: None }) => first_name,
        Some(Person { first_name: None, last_name: Some(last_name) }) => last_name,
        _ => String::new(),
    }
}

Note that I combined the last two arms into one using the _ pattern, since they were returning the same value. That’s the fallback pattern which will match anything that wasn’t matched in a previous arm. I also removed the early returns, since the output of the match will be returned anyway.

Overall, thanks to the Option type and the compiler-enforced exhaustiveness of match, this Rust code avoids the two defects that our original Node code ran into! While it’s certainly possible to write Rust that’s similar to our JS, it’s a bad idea. And while we could also write a unit test for our JS and find those defects before they hit production, and never have to go down a rabbit hole thinking about Rust, that’s also a bad idea! Because then we wouldn’t get to think about using Rust instead, would we? 😉