bug#65234: [PATCH] Fix jsx font-lock in older tree-sitter-js grammars

bug#65234: [PATCH] Fix jsx font-lock in older tree-sitter-js grammars

[Danny Freeman via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 2239 bytes --]


I have a patch to fix a bug I found in js-ts-mode.

Description of Problem:
A new function was wrtiten to account for a breaking change in the
tree-sitter-javascript grammar, see [0]

This function returns some tree-sitter queries used by the js-ts-mode
font lock settings.

We attempted to check for the breaking change by running a query
(treesit-query-capture 'javascript '((member_expression) @capture))
which would in theory fail when an old grammar is being used.

The problem is that this query does not fail on old grammars, so the
backwards incompatible font-lock queries are always used. When
font-locking a JSX file, the queries that look like
(jsx_opening_element [(member_expression) ...
throw errors and font locking fails (only on jsx elements).

Solution:
Instead of fixing the bb1f97b compatibility check with something like

(treesit-query-capture 'javascript
  '((jsx_opening_element (member_expression) @capture)))

I have opted re-write the font lock rules that capture the same
nodes by the "name:" field instead of by the specific node names (where
the backwards incompatible change took place).

The possible nodes that can be in the "name" field: 
After [1]: "identifier" "member_expression" "jsx_namespace_name"
Before [2]: "identifier" "nested_identifier" "jsx_namespace_name"

"jsx_namespace_name" is an additional node captured by these rules. As
an example, in the JSX expression

<Foo:bar/>

"Foo:bar" is a node of type "jsx_namespace_name" and would be captured
with @font-lock-function-call-face, where before this change it was not
captured or highlighted at all. If there is a reason this should not be
captured and highlighted, a new query can be added in to capture these
nodes first with @default so they are not highlighted like before.

Some thoughts:
These backwards incompatible grammar changes are rough to deal with, but
I'm glad they are taken into account. I use nixos, and the current
stable version 23.05 packages the older version of the JS grammar for
emacs which is how I noticed this. I wish the developers of the js
grammar were more careful about backwards compatible changes. The fact
that they were renaming a node without much reason didn't seem to bother
any of the maintainers.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-jsx-font-lock-in-older-tree-sitter-js-grammars.patch --]
[-- Type: text/patch, Size: 2585 bytes --]

From ab7e8c1e7120fa129840e4951803542509bee116 Mon Sep 17 00:00:00 2001
From: dannyfreeman <danny@dfreeman.email>
Date: Fri, 11 Aug 2023 16:43:58 -0400
Subject: [PATCH] Fix jsx font-lock in older tree-sitter-js grammars

* lisp/progmodes/js.el (js--treesit-font-lock-settings): Use queries
that are backwards compatible with tree-sitter-javascript bb1f97b
* list/progmodes/js.el (-jsx--treesit-font-lock-compatibility-bb1f97b):
deleted unused function
---
 lisp/progmodes/js.el | 35 ++++-------------------------------
 1 file changed, 4 insertions(+), 31 deletions(-)

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index c583b6f6191..9d2990e7bc9 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -3501,35 +3501,6 @@ js--treesit-operators
     "&&" "||" "!")
   "JavaScript operators for tree-sitter font-locking.")
 
-(defun js-jsx--treesit-font-lock-compatibility-bb1f97b ()
-  "Font lock rules helper, to handle different releases of tree-sitter-javascript.
-Check if a node type is available, then return the right font lock rules."
-  ;; handle commit bb1f97b
-  (condition-case nil
-      (progn (treesit-query-capture 'javascript '((member_expression) @capture))
-	     '((jsx_opening_element
-		[(member_expression (identifier)) (identifier)]
-		@font-lock-function-call-face)
-
-	       (jsx_closing_element
-		[(member_expression (identifier)) (identifier)]
-		@font-lock-function-call-face)
-
-	       (jsx_self_closing_element
-		[(member_expression (identifier)) (identifier)]
-		@font-lock-function-call-face)))
-    (error '((jsx_opening_element
-	      [(nested_identifier (identifier)) (identifier)]
-	      @font-lock-function-call-face)
-
-	     (jsx_closing_element
-	      [(nested_identifier (identifier)) (identifier)]
-	      @font-lock-function-call-face)
-
-	     (jsx_self_closing_element
-	      [(nested_identifier (identifier)) (identifier)]
-	      @font-lock-function-call-face)))))
-
 (defvar js--treesit-font-lock-settings
   (treesit-font-lock-rules
 
@@ -3639,8 +3610,10 @@ js--treesit-font-lock-settings
 
    :language 'javascript
    :feature 'jsx
-   (append (js-jsx--treesit-font-lock-compatibility-bb1f97b)
-	   '((jsx_attribute (property_identifier) @font-lock-constant-face)))
+   '((jsx_opening_element name: (_) @font-lock-function-call-face)
+     (jsx_closing_element name: (_) @font-lock-function-call-face)
+     (jsx_self_closing_element name: (_) @font-lock-function-call-face)
+     (jsx_attribute (property_identifier) @font-lock-constant-face))
 
    :language 'javascript
    :feature 'number
-- 
2.40.1


[-- Attachment #3: Type: text/plain, Size: 398 bytes --]


Sources
[0]:
https://github.com/tree-sitter/tree-sitter-javascript/commit/bb1f97b643b77fc1f082d621bf533b4b14cf0c30
[1]:
https://github.com/tree-sitter/tree-sitter-javascript/blob/bb1f97b643b77fc1f082d621bf533b4b14cf0c30/grammar.js#L637-L641
[2]:
https://github.com/tree-sitter/tree-sitter-javascript/blob/5720b249490b3c17245ba772f6be4a43edb4e3b7/grammar.js#L635-L639

Thank you,
-- 
Danny Freeman

bug#65234: [PATCH] Fix jsx font-lock in older tree-sitter-js grammars

[Eli Zaretskii]

> Cc: Vincenzo Pupillo <v.pupillo@gmail.com>,
>  Daniel Colascione <dancol@dancol.org>
> Date: Fri, 11 Aug 2023 17:18:32 -0400
> From:  Danny Freeman via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> I have a patch to fix a bug I found in js-ts-mode.
> 
> Description of Problem:
> A new function was wrtiten to account for a breaking change in the
> tree-sitter-javascript grammar, see [0]
> 
> This function returns some tree-sitter queries used by the js-ts-mode
> font lock settings.
> 
> We attempted to check for the breaking change by running a query
> (treesit-query-capture 'javascript '((member_expression) @capture))
> which would in theory fail when an old grammar is being used.
> 
> The problem is that this query does not fail on old grammars, so the
> backwards incompatible font-lock queries are always used. When
> font-locking a JSX file, the queries that look like
> (jsx_opening_element [(member_expression) ...
> throw errors and font locking fails (only on jsx elements).
> 
> Solution:
> Instead of fixing the bb1f97b compatibility check with something like
> 
> (treesit-query-capture 'javascript
>   '((jsx_opening_element (member_expression) @capture)))
> 
> I have opted re-write the font lock rules that capture the same
> nodes by the "name:" field instead of by the specific node names (where
> the backwards incompatible change took place).
> 
> The possible nodes that can be in the "name" field: 
> After [1]: "identifier" "member_expression" "jsx_namespace_name"
> Before [2]: "identifier" "nested_identifier" "jsx_namespace_name"
> 
> "jsx_namespace_name" is an additional node captured by these rules. As
> an example, in the JSX expression
> 
> <Foo:bar/>
> 
> "Foo:bar" is a node of type "jsx_namespace_name" and would be captured
> with @font-lock-function-call-face, where before this change it was not
> captured or highlighted at all. If there is a reason this should not be
> captured and highlighted, a new query can be added in to capture these
> nodes first with @default so they are not highlighted like before.
> 
> Some thoughts:
> These backwards incompatible grammar changes are rough to deal with, but
> I'm glad they are taken into account. I use nixos, and the current
> stable version 23.05 packages the older version of the JS grammar for
> emacs which is how I noticed this. I wish the developers of the js
> grammar were more careful about backwards compatible changes. The fact
> that they were renaming a node without much reason didn't seem to bother
> any of the maintainers.

Yuan, Theo: any comments on this?  Is this safe enough to go into the
emacs-29 branch?

Thanks.

bug#65234: [PATCH] Fix jsx font-lock in older tree-sitter-js grammars

[Dmitry Gutov]

On 12/08/2023 08:41, Eli Zaretskii wrote:
> Yuan, Theo: any comments on this?  Is this safe enough to go into the
> emacs-29 branch?

It looks good to me (and works okay too). I also saw the same error in 
my testing.

Especially since Emacs 29.2 is not quite imminent, I think it's a good 
idea to install this: queries are simpler and thus more reliable. And 
the fixed-fix, of course.

bug#65234: [PATCH] Fix jsx font-lock in older tree-sitter-js grammars

[Eli Zaretskii]

> Date: Wed, 16 Aug 2023 05:04:10 +0300
> Cc: 65234@debbugs.gnu.org, v.pupillo@gmail.com, dancol@dancol.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 12/08/2023 08:41, Eli Zaretskii wrote:
> > Yuan, Theo: any comments on this?  Is this safe enough to go into the
> > emacs-29 branch?
> 
> It looks good to me (and works okay too). I also saw the same error in 
> my testing.
> 
> Especially since Emacs 29.2 is not quite imminent, I think it's a good 
> idea to install this: queries are simpler and thus more reliable. And 
> the fixed-fix, of course.

Thanks, installed on the emacs-29 branch, and closing the bug.