Skip to content
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.

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

Allow providing error message in Matcher protocol #292

Open
theScottyJam opened this issue Jul 12, 2023 · 4 comments
Open

Allow providing error message in Matcher protocol #292

theScottyJam opened this issue Jul 12, 2023 · 4 comments

Comments

@theScottyJam
Copy link
Contributor

theScottyJam commented Jul 12, 2023

It sounds like there's a fair amount of discussion around expanding pattern-matching to work in many contexts where variable declarations can happen, such as function parameters. If this happens, it would be very nice if objects conforming to the matchable protocol could provide a friendly error message explaining why the pattern-matching failed, so that engines don't have to show some generic "Failed to match a custom matcher protocol" error.

This is especially relevant if the custom matcher could be failing for a number of different reasons, and it would like to explain why exactly it failed.

As an example, say function (paramName matches <pattern>) { ... } was the syntax for applying patterns to function parameters, and say the matcher protocol could support error message strings. Then something like this would be possible:

// Creates an object following the matcher protocol,
// that ensures that the values being matched only contain
// the keys supplied, and no more.
function expectkeysFrom(...expectedKeys_) {
  const expectedKeys = new Set(expectedKeys_);
  return {
    [Symbol.matcher](obj) {
      if (value !== Object(obj)) {
        return { matched: false, error: 'Expected an object' };
      }

      const invalidKeys = Object.keys(obj)
        .filter(key => !expectedKeys.has(key));

      if (invalidKeys.length > 0) {
        return { matched: false, error: `Found unexpected extra keys: ${invalidKeys.join(', ')}` }
      }

      return { matched: true };
    },
  };
};

function magnitudeOf(
  point matches ${keysFrom('x', 'y')} & { x, y },
) {
  ...
}

// TypeError: Failed to validate the first argument for magnitudeOf(): Found unexpected extra keys: z
magnitudeOf({ x: 2, y: 3, z: 4 });

// TypeError: Failed to validate the first argument for magnitudeOf(): Expected an object
magnitudeOf(2);

This is a feature that could always be added after-the-fact, so I'm good if we just close this issue. I mostly wanted to bring it up as something good to keep in mind as this proposal evolves.

@ljharb
Copy link
Member

ljharb commented Jul 12, 2023

That seems very expensive to produce, and there could be infinite reasons why something failed to match. Additionally, there could be dozens of ways to match, and so each error message, to be maximally clear, would have to list all of the possibilities.

Separately, the spec doesn't contain any error messages; those all come from implementations, and I'm not sure it would be feasible to have the user provide the error message (as opposed to currently, where the user can only throw their own error)

@Jack-Works
Copy link
Member

It's possible to do this but I also doubt this will be very expensive. The solution might be like this (the syntax is arbitrary since we recently changing it):

function IsX(val) {
    return val === "x" ? { matches: true, value: val } : { matches: false, value: new MyError("val is not x") }
}

match (expr) {
    when expr is typeof "number": expr
    when expr is IsX(^1): expr
}
// Aggregated TypeError:
//     match failed because
//     [0] TypeError typeof expr is not "number"
//     [1] MyError val is not x
//     [2] TypeError There is no default case

@theScottyJam
Copy link
Contributor Author

theScottyJam commented Jul 12, 2023

Oh, yeah, I didn't think about how match can throw an error if there is no default case. Providing a useful aggregate error for that scenario could be nice as opposed to some generic bland error about how nothing matched.

Separately, the spec doesn't contain any error messages; those all come from implementations, and I'm not sure it would be feasible to have the user provide the error message (as opposed to currently, where the user can only throw their own error)

I don't think the spec has to describe exactly how the final error message should be put together. Instead, the user supplies a string saying what went wrong, and the engine can prefix it with whatever it wants as it puts together its own error. So, if I return the object { matched: false, message: 'Expected an object' }, one implementation might build an error saying "Failed to match because: Expected an object", while another might add some additional details from context if it wants and say "Failed to validate the first parameter of the function myFn because: Expected an object". Perhaps we could treat them completely as hints and allow implementations to ignore the string all-together, and give the error "Failed to match - it's up to you to figure out why, bwahaha", or whatever.

In the end, it's up to the engine to decide how much detail to add or omit, and they can choose depending on how expensive or readable it makes the error message.

@Jamesernator
Copy link

Jamesernator commented Sep 16, 2023

It's possible to do this but I also doubt this will be very expensive. The solution might be like this (the syntax is arbitrary since we recently changing it):

I do the same sort've thing for parsing, however exceptions are actually very expensive (at least in V8). Eliminating exceptions resulted in a substantial (19s to 300ms) speedup (when alternatives were common).

The simplest approach, which I've also used in parsing, is just to have failures be strings or objects which can be stringified (and thus can be part of the MatchError message).

For example for aggregate failure, I'd generally just do something like:

type Stringifiable = { toString(): string };

class AggregateFailure {
    failures: Array<Stringifiable>;

    constructor(failures: Array<Stringifiable>) {
        this.failures = failures;
    }
    
    toString() {
        return `Matching failed because:\n${ this.failures.map((failure, idx) => `[${ idx }] ${ failure.toString() }` ) }`;
    }
}

It also has the advantage that concatenated failure strings don't need to be generated if the top-level result wasn't actually a failure.

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

No branches or pull requests

4 participants