RegExp pattern against a non-string matchable: stringify or fail? #267

Activity

1. ljharb

   added

   question

   champion group discussion

   on Jul 8, 2022
2. theScottyJam commented on Jul 8, 2022

   

   theScottyJam

   on Jul 8, 2022

   Contributor

   More actions

   To be clear, we're talking about something like this?

   match (23) {
     when (/\d+/): ... // matches?
   }
   

   For the rest of the language, we can generally get by with the permissive nature of how it auto-courses values - we make sure we're aware of the type of each value we're dealing with, and we do what we can to make sure we never hand a value of the wrong type to the language's built-in function, and it won't coerce for us. We can layer on tools like TypeScript, which as a side-effect makes it so we're almost never relying on JavaScript's coercive nature (except for in places like the plus operator).

   Considering this is pattern matching, I think it's more important that we be restrictive on what matches and what doesn't, more so than what we do in the rest of the language. A major purpose of pattern-matching is to determine the types of values we're working with, so if some of the patterns are extra permissive, it makes pattern matching that much harder to utilize.
3. ljharb commented on Jul 8, 2022

   

   ljharb

   on Jul 8, 2022

   Last edited by ljharb

   MemberAuthor

   More actions

   In this case, though, if you want to filter out non-strings when we stringify, it's pretty straightforward:

   match (23) {
     when (${String} && /\d+/): ...
   }

   whereas if we do not stringify, and you want to stringify for a single case, this is how you have to do it:

   match (23) {
     if (${/\d+/}): ...
     // matches, but now you have to manually `.match` or `.exec` or `.matchAll` to get at the match results, and `with` binding can't work
   }
4. theScottyJam commented on Jul 8, 2022

   

   theScottyJam

   on Jul 8, 2022

   Last edited by ljharb

   Contributor

   More actions

   Hmm, good point.

   I guess the other option would be for either userland or the language to provide an asString custom matcher that you can use like this:

   const asString = {
     [Symbol.matcher] = val => ({
       matched: true,
       value: String(val),
     }
   };
   
   match (23) {
     when (asString with /\d+/)
   };
   

   I do feel like the act of properly transforming a value to match what's expected by a particular pattern is better suited for "value transforming matcher", rather than extra-permissive matchers, as I feel like you're less likely to run into bugs if you have to be intentional about the coercion vs having to explicitly opt out of it (with String &).

   Here's an example where a permisive regular expression matcher can lead to accidental, subtle bugs:

   const containsLegalCharacters = value => match (value) {
     when (/^[a-zA-Z\[\]\-\{\}]*$/): true
     when (${String}): false
     default: do { throw new Error('Invalid parameter'); }
   };

   If I were to pass in an object literal, it would stringify as [Object object], match the first pattern, and return true.
5. ljharb commented on Jul 8, 2022

   

   ljharb

   on Jul 8, 2022

   MemberAuthor

   More actions

   Indeed - but how often is someone going to have a regex that matches Object toString’s output, and not already be aware of the foot gun from “everything else that touches regex in the language”?

   👍React with 👍1Reacted by Scotty Jamison
6. Jack-Works commented on Jul 8, 2022

   

   Jack-Works

   on Jul 8, 2022 · Hidden as resolved

   show commentMore actions
7. ljharb commented on Jul 8, 2022

   

   ljharb

   on Jul 8, 2022 · Hidden as resolved

   Author

   show commentMore actions
8. tabatkins commented on Jul 11, 2022

   

   tabatkins

   on Jul 11, 2022

   Collaborator

   More actions

   My intention was definitely that we stringify the matchable in order to test against regex literals, exactly as if you passed the matchable to .test(). If the spec currently says something different we should fix it.
9. ljharb commented on Jul 11, 2022

   

   ljharb

   on Jul 11, 2022

   MemberAuthor

   More actions

   @tabatkins sounds good, i'll fix that shortly.
10. ljharb

    closed this as completedin 67160b3on Jul 11, 2022