1711715 - Improve Ergonomic Brand Check error messages

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Matthew Gaudet (he/him) [:mgaudet]]

Looking at this error message, and I realize it's not great.

> class A { #x; h(o) { return !#x in o; }}}
SyntaxError: private names aren't valid in this context

We can probably do better.

Comment 1

[Alexandra Silva (:alexandra_)]

hi @(In reply to Matthew Gaudet (he/him) [:mgaudet] from comment #0)

Looking at this error message, and I realize it's not great.

> class A { #x; h(o) { return !#x in o; }}}
SyntaxError: private names aren't valid in this context

We can probably do better.

hi o/

I would like to work on this bug :)
Could you explain better why this message is not great? And..what is expected to be great? I mean, it could help me to decide what I can do.

Comment 2

[Matthew Gaudet (he/him) [:mgaudet]]

Hi Alexandra,

You can totally take on this bug. Let’s start by talking a bit about why this is a confusing error message to me, and what I think would be better.

This example was written by me while I was testing something else out. Let’s be clear: This code is actually buggy, based on the language precedence rules: Think about if I had written:

!’x’ in o

This silently is a bug; because ! has higher precedence than in, what this is really is asking is (!’x’) in o, not what you might have hoped for !(‘x’ in o)

Now, private fields kindly emit an error, which is good,: but the message “private names aren’t valid in this context” isn’t super helpful to me; the question is “What context?”

So, we can use SearchFox to find where this message is emitted, which finds three places. My suspicion here is that this is actually being emitted in here, where we parse a unary expression.

It would be good to check that my suspicion is right; a good way to do that is to build the spider monkey shell, then write a simple test case, then edit that call site to have a printf or MOZ_CRASH to verify that this is in fact the code that issues that message.

How to change the message is hard. Maybe something along the lines of “Cannot apply unary operator to private name”? Which is more clear.

Before getting started, you'll want to

• Have a checkout of the Firefox source code
• Make sure you can build SpiderMonkey
• Understand the basics of using Mercurial
• Understand how to submit a patch
• Read this walkthrough about how development works in Firefox.

Comment 3

[Bryan Thrall [:bthrall]]

Attachment: Bug 1711715 - Remove check for enabling private fields r?yulia

@mgaudet says we've shipped private fields for long enough that it's
never coming out.

Comment 4

[Bryan Thrall [:bthrall]]

Attachment: Bug 1711715 - Clarify error message for invalid private name in unary expression r?yulia

Depends on D139117

Comment 5

[Bryan Thrall [:bthrall]]

Attachment: Bug 1711715 - Clarify error message for invalid private name due to operator precedence r?yulia

The test could be as simple as 1 + #x in y, but we want an expression that
fails to parse because of only this one error.

Depends on D139118

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1d6aaab87a63
Remove check for enabling private fields r=yulia,mgaudet
https://hg.mozilla.org/integration/autoland/rev/7573e83b4966
Clarify error message for invalid private name in unary expression r=yulia
https://hg.mozilla.org/integration/autoland/rev/fc7c9b4732f6
Clarify error message for invalid private name due to operator precedence r=yulia

Comment 7

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/1d6aaab87a63
https://hg.mozilla.org/mozilla-central/rev/7573e83b4966
https://hg.mozilla.org/mozilla-central/rev/fc7c9b4732f6