bug#67684: 30.0.50; Font lock rule conflict in js-ts-mode at level 4

bug#67684: 30.0.50; Font lock rule conflict in js-ts-mode at level 4

[Loïc Lemaître]

Hi !

Hi !

I have noticed a font coloration issue in js-ts-mode when setting the 
treesit font lock level to 4: JSX attributes color is not the same than 
at level 3.
There should have a conflict between rules for "property_identifier" 
treesit type. The feature that triggers the conflict is "property" (of 
the level 4).

You can reproduce the issue by following these steps:
- M-: (setq treesit-font-lock-level 4)
- M-x js-ts-mode on a JSX buffer

Note that the bug already exists in Emacs 29.1.

Thanks !

Loïc

In GNU Emacs 30.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version
  3.24.33, cairo version 1.16.0) of 2023-12-07 built on
  loic-Latitude-E5470
Repository revision: e4e1e268c8e9f7de6fe5d4b05beb595a1781c02c
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12201001
System Description: Ubuntu 22.04.3 LTS

Configured using:
  'configure --with-native-compilation=aot --with-tree-sitter --with-json
  CC=gcc-12'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LCMS2 LIBSELINUX LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG
RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER X11 XDBE
XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
   value of $LANG: fr_FR.UTF-8
   value of $XMODIFIERS: @im=ibus
   locale-coding-system: utf-8-unix

Major mode: JavaScript

Minor modes in effect:
   eglot-inlay-hints-mode: t
   eglot--managed-mode: t
   whitespace-mode: t
   electric-pair-mode: t
   global-so-long-mode: t
   telephone-line-mode: t
   global-git-commit-mode: t
   magit-auto-revert-mode: t
   global-auto-revert-mode: t
   recentf-mode: t
   windmove-mode: t
   counsel-projectile-mode: t
   ivy-mode: t
   delete-selection-mode: t
   global-flycheck-mode: t
   flycheck-mode: t
   projectile-mode: t
   yas-global-mode: t
   yas-minor-mode: t
   global-company-mode: t
   company-mode: t
   TeX-PDF-mode: t
   TeX-source-correlate-mode: t
   which-key-mode: t
   highlight-indentation-current-column-mode: t
   ws-butler-global-mode: t
   ws-butler-mode: t
   override-global-mode: t
   tooltip-mode: t
   global-eldoc-mode: t
   eldoc-mode: t
   show-paren-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   blink-cursor-mode: t
   window-divider-mode: t
   minibuffer-regexp-mode: t
   line-number-mode: t
   transient-mark-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t
   hs-minor-mode: t

Load-path shadows:
/home/loic/.emacs.d/elpa/transient-20230830.1638/transient hides 
/home/loic/Travail/Temp/emacs/lisp/transient
/home/loic/.emacs.d/elpa/bind-key-20230203.2004/bind-key hides 
/home/loic/Travail/Temp/emacs/lisp/bind-key
/home/loic/.emacs.d/elpa/use-package-20230426.2324/use-package-ensure 
hides /home/loic/Travail/Temp/emacs/lisp/use-package/use-package-ensure
/home/loic/.emacs.d/elpa/use-package-20230426.2324/use-package hides 
/home/loic/Travail/Temp/emacs/lisp/use-package/use-package
/home/loic/.emacs.d/elpa/use-package-20230426.2324/use-package-bind-key 
hides /home/loic/Travail/Temp/emacs/lisp/use-package/use-package-bind-key
/home/loic/.emacs.d/elpa/use-package-20230426.2324/use-package-lint 
hides /home/loic/Travail/Temp/emacs/lisp/use-package/use-package-lint
/home/loic/.emacs.d/elpa/use-package-20230426.2324/use-package-diminish 
hides /home/loic/Travail/Temp/emacs/lisp/use-package/use-package-diminish
/home/loic/.emacs.d/elpa/use-package-20230426.2324/use-package-jump 
hides /home/loic/Travail/Temp/emacs/lisp/use-package/use-package-jump
/home/loic/.emacs.d/elpa/use-package-20230426.2324/use-package-core 
hides /home/loic/Travail/Temp/emacs/lisp/use-package/use-package-core
/home/loic/.emacs.d/elpa/use-package-20230426.2324/use-package-delight 
hides /home/loic/Travail/Temp/emacs/lisp/use-package/use-package-delight
/home/loic/.emacs.d/elpa/hierarchy-20190425.842/hierarchy hides 
/home/loic/Travail/Temp/emacs/lisp/emacs-lisp/hierarchy

Features:
(shadow sort mail-extr emacsbug mule-util vc-git flycheck-eglot eglot
external-completion jsonrpc ert ewoc debug backtrace jtsx
typescript-ts-mode whitespace hideshow company-oddmuse company-keywords
company-files company-dabbrev-code company-dabbrev company-yasnippet
company-capf time elec-pair so-long term disp-table ehelp telephone-line
telephone-line-segments telephone-line-separators telephone-line-utils
ox-gfm ox-md ox-odt rng-loc rng-uri rng-parse rng-match rng-dt rng-util
rng-pttrn nxml-parse nxml-ns nxml-enc xmltok nxml-util ox-latex
ox-icalendar org-agenda ox-html table ox-ascii ox-publish ox org-element
org-persist org-id org-refile org ob ob-tangle ob-ref ob-lob ob-table
org-macro org-src ob-comint org-pcomplete org-list org-footnote
org-faces org-entities noutline outline ob-emacs-lisp org-table org-keys
org-loaddefs cal-menu calendar cal-loaddefs avl-tree ol oc ob-exp
ob-core org-cycle org-fold org-fold-core org-compat ob-eval org-version
org-macs magit-submodule magit-blame magit-stash magit-reflog
magit-bisect magit-push magit-pull magit-fetch magit-clone magit-remote
magit-commit magit-sequence magit-notes magit-worktree magit-tag
magit-merge magit-branch magit-reset magit-files magit-refs magit-status
magit magit-repos magit-apply magit-wip magit-log which-func magit-diff
smerge-mode diff diff-mode git-commit log-edit message sendmail
yank-media puny rfc822 mml mml-sec epa epg rfc6068 epg-config gnus-util
time-date mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047
rfc2045 mm-util ietf-drums mail-prsvr mailabbrev mail-utils gmm-utils
mailheader pcvs-util add-log magit-core magit-autorevert autorevert
filenotify magit-margin magit-transient magit-process with-editor comp
comp-cstr shell pcomplete server magit-mode magit-git magit-base
magit-section cursor-sensor rg files-x vc vc-dispatcher rg-info-hack
rg-menu transient compat rg-ibuffer rg-result wgrep-rg wgrep rg-history
rg-header cus-edit pp cus-load dashboard dashboard-widgets recentf
tree-widget wid-edit ffap windmove framemove counsel-projectile counsel
xdg dired dired-loaddefs swiper ivy delsel ivy-faces ivy-overlay colir
color dockerfile-mode derived sh-script smie executable flycheck
find-func jest jest-traversal ibuffer-projectile projectile lisp-mnt
grep ibuf-ext ibuffer ibuffer-loaddefs magit-popup format-spec js2-mode
etags fileloop generator xref js cc-mode cc-fonts cc-guess cc-menus
cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs imenu pdf-loader
company-auctex yasnippet-snippets yasnippet company latex latex-flymake
flymake project compile text-property-search warnings icons thingatpt
tex-ispell tex-style tex dbus comp-run comp-common xml crm texmathp
c++-ts-mode c-ts-mode c-ts-common treesit advice virtualenvwrapper gud
comint ansi-osc ansi-color ring s dash py-autopep8 which-key
highlight-indentation ws-butler edmacro kmacro use-package-bind-key
bind-key easy-mmode snazzy-theme base16-theme exec-path-from-shell
cl-extra help-mode use-package-ensure use-package-core finder-inf
buttercup-autoloads auctex-autoloads tex-site company-web-autoloads
dashboard-autoloads dockerfile-mode-autoloads doom-themes-autoloads
evil-autoloads exec-path-from-shell-autoloads f-autoloads
flycheck-eglot-autoloads flycheck-autoloads goto-chg-autoloads
ibuffer-projectile-autoloads company-autoloads js2-mode-autoloads
json-mode-autoloads rx magit-autoloads pcase git-commit-autoloads
markdown-mode-autoloads mood-line-autoloads multiple-cursors-autoloads
nix-mode-autoloads magit-section-autoloads package-lint-autoloads
popper-autoloads popup-autoloads rainbow-mode-autoloads
realgud-autoloads realgud-recursive-autoloads loc-changes-autoloads
load-relative-autoloads rg-autoloads simple-modeline-autoloads
base16-theme-autoloads sqlformat-autoloads reformatter-autoloads
ivy-autoloads test-simple-autoloads transient-autoloads
treemacs-projectile-autoloads treemacs-autoloads posframe-autoloads
ht-autoloads projectile-autoloads dash-autoloads
web-completion-data-autoloads which-key-autoloads with-editor-autoloads
info compat-autoloads yaml-autoloads yaml-mode-autoloads
yasnippet-snippets-autoloads package browse-url url url-proxy
url-privacy url-expand url-methods url-history url-cookie
generate-lisp-file url-domsuf url-util mailcap url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs password-cache json subr-x
map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs cl-lib rmc
iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd touch-screen tool-bar dnd fontset image regexp-opt
fringe tabulated-list replace newcomment text-mode lisp-mode prog-mode
register page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting cairo gtk x-toolkit xinput2 x multi-tty move-toolbar
make-network-process native-compile emacs)

Memory information:
((conses 16 918736 127661) (symbols 48 52905 3) (strings 32 301817 
19250) (string-bytes 1 10190663)
  (vectors 16 97148) (vector-slots 8 1704214 34772) (floats 8 380 11535) 
(intervals 56 1687 287)
  (buffers 992 17))

bug#67684: 30.0.50; Font lock rule conflict in js-ts-mode at level 4

[Dmitry Gutov]

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

Hi!

On 07/12/2023 12:07, Loïc Lemaître wrote:
> I have noticed a font coloration issue in js-ts-mode when setting the 
> treesit font lock level to 4: JSX attributes color is not the same than 
> at level 3.
> There should have a conflict between rules for "property_identifier" 
> treesit type. The feature that triggers the conflict is "property" (of 
> the level 4).
> 
> You can reproduce the issue by following these steps:
> - M-: (setq treesit-font-lock-level 4)
> - M-x js-ts-mode on a JSX buffer
> 
> Note that the bug already exists in Emacs 29.1.

The attached patch seems to fix that.

But it removes a function added by Yuan in e78e69b33189, while 
reshuffling the highlights to reorder priorities.

Yuan, do you remember why you did it that way back then? Perhaps I'm 
missing some other conflict.

[-- Attachment #2: js-ts-font-lock-property.diff --]
[-- Type: text/x-patch, Size: 1917 bytes --]

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 5a669fdbd42..e2b748f6cb8 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -3570,16 +3570,6 @@ js--treesit-font-lock-settings
      ;; full namespace import (* as alias)
      (import_clause (namespace_import (identifier) @font-lock-variable-name-face)))
 
-   :language 'javascript
-   :feature 'property
-   '(((property_identifier) @font-lock-property-use-face
-      (:pred js--treesit-property-not-function-p
-             @font-lock-property-use-face))
-
-     (pair value: (identifier) @font-lock-variable-use-face)
-
-     ((shorthand_property_identifier) @font-lock-property-use-face))
-
    :language 'javascript
    :feature 'assignment
    '((assignment_expression
@@ -3606,6 +3596,12 @@ js--treesit-font-lock-settings
      (jsx_self_closing_element name: (_) @font-lock-function-call-face)
      (jsx_attribute (property_identifier) @font-lock-constant-face))
 
+   :language 'javascript
+   :feature 'property
+   '(((property_identifier) @font-lock-property-use-face)
+     (pair value: (identifier) @font-lock-variable-use-face)
+     ((shorthand_property_identifier) @font-lock-property-use-face))
+
    :language 'javascript
    :feature 'number
    '((number) @font-lock-number-face
@@ -3657,14 +3653,6 @@ js--fontify-template-string
       (setq font-beg (treesit-node-end child)
             child (treesit-node-next-sibling child)))))
 
-(defun js--treesit-property-not-function-p (node)
-  "Check that NODE, a property_identifier, is not used as a function."
-  (not (equal (treesit-node-type
-               (treesit-node-parent ; Maybe call_expression.
-                (treesit-node-parent ; Maybe member_expression.
-                 node)))
-              "call_expression")))
-
 (defvar js--treesit-lhs-identifier-query
   (when (treesit-available-p)
     (treesit-query-compile 'javascript '((identifier) @id

bug#67684: 30.0.50; Font lock rule conflict in js-ts-mode at level 4

[Loïc Lemaître]

Le 08/12/2023 à 02:33, Dmitry Gutov a écrit :
> Hi!
>
> On 07/12/2023 12:07, Loïc Lemaître wrote:
>> I have noticed a font coloration issue in js-ts-mode when setting the 
>> treesit font lock level to 4: JSX attributes color is not the same 
>> than at level 3.
>> There should have a conflict between rules for "property_identifier" 
>> treesit type. The feature that triggers the conflict is "property" 
>> (of the level 4).
>>
>> You can reproduce the issue by following these steps:
>> - M-: (setq treesit-font-lock-level 4)
>> - M-x js-ts-mode on a JSX buffer
>>
>> Note that the bug already exists in Emacs 29.1.
>
> The attached patch seems to fix that.
Yes I confirm, thanks !
>
> But it removes a function added by Yuan in e78e69b33189, while 
> reshuffling the highlights to reorder priorities.
>
> Yuan, do you remember why you did it that way back then? Perhaps I'm 
> missing some other conflict.

bug#67684: 30.0.50; Font lock rule conflict in js-ts-mode at level 4

[Yuan Fu]

On 12/7/23 5:33 PM, Dmitry Gutov wrote:
> Hi!
>
> On 07/12/2023 12:07, Loïc Lemaître wrote:
>> I have noticed a font coloration issue in js-ts-mode when setting the 
>> treesit font lock level to 4: JSX attributes color is not the same 
>> than at level 3.
>> There should have a conflict between rules for "property_identifier" 
>> treesit type. The feature that triggers the conflict is "property" 
>> (of the level 4).
>>
>> You can reproduce the issue by following these steps:
>> - M-: (setq treesit-font-lock-level 4)
>> - M-x js-ts-mode on a JSX buffer
>>
>> Note that the bug already exists in Emacs 29.1.
>
> The attached patch seems to fix that.
>
> But it removes a function added by Yuan in e78e69b33189, while 
> reshuffling the highlights to reorder priorities.
>
> Yuan, do you remember why you did it that way back then? Perhaps I'm 
> missing some other conflict.
For a method invocation like obj.fn(), the "fn" should be fontified in 
function-call-face. However, "fn" is a property_identifier in the parse 
tree, so it would be fontified in property-use-face by the property 
feature. The predicate in the property feature makes sure we skip 
properties that are function invocations. And down the line in function 
feature, you'll see

(call_expression
       function: [(identifier) @font-lock-function-call-face
                  (member_expression
                   property:
                   (property_identifier) @font-lock-function-call-face)])

Here, the property_identifier is fontified in function-call-face.

Yuan

bug#67684: 30.0.50; Font lock rule conflict in js-ts-mode at level 4

[Dmitry Gutov]

On 11/12/2023 03:37, Yuan Fu wrote:
> 
> 
> On 12/7/23 5:33 PM, Dmitry Gutov wrote:
>> Hi!
>>
>> On 07/12/2023 12:07, Loïc Lemaître wrote:
>>> I have noticed a font coloration issue in js-ts-mode when setting the 
>>> treesit font lock level to 4: JSX attributes color is not the same 
>>> than at level 3.
>>> There should have a conflict between rules for "property_identifier" 
>>> treesit type. The feature that triggers the conflict is "property" 
>>> (of the level 4).
>>>
>>> You can reproduce the issue by following these steps:
>>> - M-: (setq treesit-font-lock-level 4)
>>> - M-x js-ts-mode on a JSX buffer
>>>
>>> Note that the bug already exists in Emacs 29.1.
>>
>> The attached patch seems to fix that.
>>
>> But it removes a function added by Yuan in e78e69b33189, while 
>> reshuffling the highlights to reorder priorities.
>>
>> Yuan, do you remember why you did it that way back then? Perhaps I'm 
>> missing some other conflict.
> For a method invocation like obj.fn(), the "fn" should be fontified in 
> function-call-face. However, "fn" is a property_identifier in the parse 
> tree, so it would be fontified in property-use-face by the property 
> feature. The predicate in the property feature makes sure we skip 
> properties that are function invocations. And down the line in function 
> feature, you'll see
> 
> (call_expression
>        function: [(identifier) @font-lock-function-call-face
>                   (member_expression
>                    property:
>                    (property_identifier) @font-lock-function-call-face)])
> 
> Here, the property_identifier is fontified in function-call-face.

Thanks. So the patch I sent should also cover this, right?

It moves the 'property' feature to below 'function'. And below 'jsx', 
which fixes the problem reported in this bug.

bug#67684: 30.0.50; Font lock rule conflict in js-ts-mode at level 4

[Yuan Fu]

On 12/11/23 8:35 AM, Dmitry Gutov wrote:
> On 11/12/2023 03:37, Yuan Fu wrote:
>>
>>
>> On 12/7/23 5:33 PM, Dmitry Gutov wrote:
>>> Hi!
>>>
>>> On 07/12/2023 12:07, Loïc Lemaître wrote:
>>>> I have noticed a font coloration issue in js-ts-mode when setting 
>>>> the treesit font lock level to 4: JSX attributes color is not the 
>>>> same than at level 3.
>>>> There should have a conflict between rules for 
>>>> "property_identifier" treesit type. The feature that triggers the 
>>>> conflict is "property" (of the level 4).
>>>>
>>>> You can reproduce the issue by following these steps:
>>>> - M-: (setq treesit-font-lock-level 4)
>>>> - M-x js-ts-mode on a JSX buffer
>>>>
>>>> Note that the bug already exists in Emacs 29.1.
>>>
>>> The attached patch seems to fix that.
>>>
>>> But it removes a function added by Yuan in e78e69b33189, while 
>>> reshuffling the highlights to reorder priorities.
>>>
>>> Yuan, do you remember why you did it that way back then? Perhaps I'm 
>>> missing some other conflict.
>> For a method invocation like obj.fn(), the "fn" should be fontified 
>> in function-call-face. However, "fn" is a property_identifier in the 
>> parse tree, so it would be fontified in property-use-face by the 
>> property feature. The predicate in the property feature makes sure we 
>> skip properties that are function invocations. And down the line in 
>> function feature, you'll see
>>
>> (call_expression
>>        function: [(identifier) @font-lock-function-call-face
>>                   (member_expression
>>                    property:
>>                    (property_identifier) 
>> @font-lock-function-call-face)])
>>
>> Here, the property_identifier is fontified in function-call-face.
>
> Thanks. So the patch I sent should also cover this, right?
>
> It moves the 'property' feature to below 'function'. And below 'jsx', 
> which fixes the problem reported in this bug.

Ah, right. In that case, I don't have anything else to add.

Yuan

bug#67684: 30.0.50; Font lock rule conflict in js-ts-mode at level 4

[Dmitry Gutov]

Version: 29.2

On 12/12/2023 10:33, Yuan Fu wrote:
> 
> 
> On 12/11/23 8:35 AM, Dmitry Gutov wrote:
>> On 11/12/2023 03:37, Yuan Fu wrote:
>>>
>>>
>>> On 12/7/23 5:33 PM, Dmitry Gutov wrote:
>>>> Hi!
>>>>
>>>> On 07/12/2023 12:07, Loïc Lemaître wrote:
>>>>> I have noticed a font coloration issue in js-ts-mode when setting 
>>>>> the treesit font lock level to 4: JSX attributes color is not the 
>>>>> same than at level 3.
>>>>> There should have a conflict between rules for 
>>>>> "property_identifier" treesit type. The feature that triggers the 
>>>>> conflict is "property" (of the level 4).
>>>>>
>>>>> You can reproduce the issue by following these steps:
>>>>> - M-: (setq treesit-font-lock-level 4)
>>>>> - M-x js-ts-mode on a JSX buffer
>>>>>
>>>>> Note that the bug already exists in Emacs 29.1.
>>>>
>>>> The attached patch seems to fix that.
>>>>
>>>> But it removes a function added by Yuan in e78e69b33189, while 
>>>> reshuffling the highlights to reorder priorities.
>>>>
>>>> Yuan, do you remember why you did it that way back then? Perhaps I'm 
>>>> missing some other conflict.
>>> For a method invocation like obj.fn(), the "fn" should be fontified 
>>> in function-call-face. However, "fn" is a property_identifier in the 
>>> parse tree, so it would be fontified in property-use-face by the 
>>> property feature. The predicate in the property feature makes sure we 
>>> skip properties that are function invocations. And down the line in 
>>> function feature, you'll see
>>>
>>> (call_expression
>>>        function: [(identifier) @font-lock-function-call-face
>>>                   (member_expression
>>>                    property:
>>>                    (property_identifier) 
>>> @font-lock-function-call-face)])
>>>
>>> Here, the property_identifier is fontified in function-call-face.
>>
>> Thanks. So the patch I sent should also cover this, right?
>>
>> It moves the 'property' feature to below 'function'. And below 'jsx', 
>> which fixes the problem reported in this bug.
> 
> Ah, right. In that case, I don't have anything else to add.

Great! I've pushed the patch to emacs-29, and this bug is now closed.