rjsx-mode icon indicating copy to clipboard operation
rjsx-mode copied to clipboard

Reserved keywords as attributes produce error

Open Fuco1 opened this issue 7 years ago • 9 comments

I use babel to translate jsx syntax into my own custom components not React (you can do this by including a pragma such as /** @jsx generate */ at the top of the file, generate is then the method fed the translated data).

I have this code

<amount width="25" minimumFractionDigits="4" class="text-dark font-weight-normal">
   {elk.SupplyPeak.Price}
</amount>

I get a parse error on class because it is reserved js word. Quick peak into rjsx-parse-identifier and js2-must-match-name shows that it tests for keywords and produces an error. Maybe we should be more permissive here?

Fuco1 avatar Mar 02 '18 16:03 Fuco1

I just gave this a try, but the problem is most reserved keywords are not parsed as NAME tokens, but rather their own tokens (js2-CLASS, js2-EXPORT, etc.). If there were a way to handle these generically, I'd be open to it. All that would be required would be writing a rjsx-must-match-name while still using the js2 tokenizer without introducing a long, brittle list of name-like tokens.

felipeochoa avatar Mar 07 '18 02:03 felipeochoa

I've tried hacking on that and got it to parse class as attribute but then the quoted value afterwards did not. It's quite a mess :/

Fuco1 avatar Mar 07 '18 09:03 Fuco1

I was thinking maybe it would be easy just adding KEYWORD_IS_NAME

Something like this:

(defun rjsx-match-token (match &optional dont-unget)
  "Get next token and return t if it matches MATCH, a bytecode.
Returns nil and consumes nothing if MATCH is not the next token."
  (setf js2-ti-lookahead 0)
  (if (/= (js2-get-token 'KEYWORD_IS_NAME) match)
      (ignore (unless dont-unget (js2-unget-token)))
    t))

(defun rjsx-must-match-name (msg-id)
  (if (rjsx-match-token js2-NAME nil)
      t
    (if (eq (js2-current-token-type) js2-RESERVED)
        (js2-report-error "msg.reserved.id" (js2-current-token-string))
      (js2-report-error msg-id)
      (js2-unget-token))
    nil))

(cl-defun rjsx-parse-identifier (&optional face &key (allow-ns t))
  "Parse a possibly namespaced identifier and fontify with FACE if given.
Returns a `js2-error-node' if unable to parse.  If the &key
argument ALLOW-NS is nil, does not allow namespaced names."
  (if (rjsx-must-match-name "msg.bad.jsx.ident")
      (let ((pn (make-rjsx-identifier))

But there is too many things going on that I don't understand. And this did not help much.

knobo avatar Mar 09 '19 16:03 knobo

If there is anything I could try out, give me some hints.

knobo avatar Mar 11 '19 12:03 knobo

IIRC, the KEYWORD_IS_NAME helps in some cases, but there are a lot of keywords it still parses specially. If you're asking about how rjsx-parse-identifier works (which I agree is a mess), the relevant bits are:

(cl-defun rjsx-parse-identifier (&optional face &key (allow-ns t))
  ;; snip
  (if (rjsx-must-match-name "msg.bad.jsx.ident")
          (if (js2-match-token js2-NAME)
  ;; snip
  (if (js2-match-token js2-NAME)
              (if (eq prev-token-end (js2-current-token-beg))
                  (progn (push (js2-current-token-string) name-parts)
                         (setq prev-token-end (js2-current-token-end)
                               name-start (or name-start (js2-current-token-beg))))
                (js2-unget-token)
                (setq continue nil))
  ;; snip

The complexity elsewhere comes from checking for namespaces and allowing dashes in names. (So it's stitching one token out of several js2-NAME, js2-COLON, js2-SUB, and js2-ASSIGN_SUB)

Does that help?

felipeochoa avatar Mar 12 '19 01:03 felipeochoa

How about not using js2 to parse the attribute name, and force js2 somehow to skip forward as much as we parse. Just check for {...var} and make js2 parse that for us?

knobo avatar Mar 12 '19 06:03 knobo

I mean, since html/xml is not javascript maybe not use javascript parser to parse it. I don't want to try parse it without js2 if that would not be accepted back in to rjsx. That's why I ask.

knobo avatar Mar 13 '19 06:03 knobo

The issue is not the parser but the lexer. But either way, yes, that's probably right. We already manipulate the lexer internals to handle -=, so we might as well do it all ourselves. If you're willing to dig into js2 to make this happen, by all means!

felipeochoa avatar Mar 13 '19 13:03 felipeochoa

So it's js2 that has to be fixed or improved?

knobo avatar Mar 26 '19 15:03 knobo