bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Jostein Kjønigsen]

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

Steps to reproduce:

- set font-lock level 4 to enable highlighting of function-invocations
- create a new buffer and activate rust-ts-mode

Try writing a function which invokes functions indirectly trough 
field-properties. Example below:

pub fn should_handle(url: String) -> bool {
     if url.ends_with(".css")
        || url.ends_with(".js")
        || url.ends_with(".png")
        || url.ends_with(".jpg")
     {
         false
     } else {
         true
     }
}

Observe that:

- ends_with() is fontified as a property-access, not as a 
function-invocation.
- (plain function invocation is highlighted as expected though)

 From my preliminary inspection of the rust-ts-mode source-code, this 
seems to be because of a very general override later in the file:

rust-ts-mode.el, line 248 or so:

    :language 'rust
    :feature 'property
    :override t
    '((field_identifier) @font-lock-property-face
      (shorthand_field_initializer (identifier) @font-lock-property-face))

Dissabling "override" for this feature fixes the fontification of method 
invocations, but I haven't done enough testing to see if there are other 
unexpected side-effects.

--
Jostein


In GNU Emacs 29.0.60 (build 5, x86_64-pc-linux-gnu, GTK+ Version
  3.24.36, cairo version 1.17.6) of 2023-01-28 built on thinkpad-t14s
Repository revision: 362678d90e10d0e60642cb42594f9e15e39a3a0b
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12201007
System Description: Arch Linux

Configured using:
  'configure --with-json --with-tree-sitter'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY INOTIFY
PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS
TREE_SITTER WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
   value of $LANG: en_US.UTF-8
   locale-coding-system: utf-8-unix

Major mode: Rust

Minor modes in effect:
   global-git-commit-mode: t
   magit-auto-revert-mode: t
   electric-pair-mode: t
   highlight-symbol-mode: t
   flycheck-mode: t
   editorconfig-mode: t
   company-mode: t
   which-function-mode: t
   helm-mode: t
   helm-minibuffer-history-mode: t
   shell-dirtrack-mode: t
   helm--remap-mouse-mode: t
   async-bytecomp-package-mode: t
   delete-selection-mode: t
   global-auto-revert-mode: t
   yas-global-mode: t
   yas-minor-mode: t
   global-nlinum-mode: t
   nlinum-mode: t
   ido-yes-or-no-mode: t
   override-global-mode: t
   server-mode: t
   global-hl-line-mode: t
   pixel-scroll-precision-mode: t
   doom-modeline-mode: t
   tooltip-mode: t
   global-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
   column-number-mode: t
   line-number-mode: t
   auto-fill-function: yas--auto-fill
   transient-mark-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t
   hs-minor-mode: t

Load-path shadows:
/home/jostein/.emacs.d/elpa/transient-20230107.1528/transient hides 
/home/jostein/build/emacs/lisp/transient

Features:
(shadow sort emacsbug rust-ts-mode cargo cargo-process rust-utils
rust-mode rust-rustfmt rust-playpen rust-compile rust-cargo tramp-cmds
tramp-sh cus-start view org-element org-persist org-id org-refile
avl-tree oc-basic ol-eww ol-rmail ol-mhe ol-irc ol-info ol-gnus nnselect
gnus-art mm-uu mml2015 mm-view mml-smime smime gnutls dig gnus-sum
gnus-group gnus-undo gnus-start gnus-dbus gnus-cloud nnimap nnmail
mail-source utf7 nnoo gnus-spec gnus-int gnus-range gnus-win ol-docview
doc-view jka-compr ol-bibtex bibtex ol-bbdb ol-w3m ol-doi org-link-doi
help-fns radix-tree cl-print apropos mail-extr csharp-mode cc-langs
vc-hg vc-bzr vc-src vc-sccs vc-cvs vc-rcs log-view vc bug-reference
helm-bookmark helm-adaptive magit-bookmark bookmark executable flyspell
ispell sgml-mode facemenu helm-command helm-elisp helm-eval edebug
helm-info pulse magit-extras face-remap magit-submodule magit-obsolete
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 magit-diff smerge-mode diff git-commit
log-edit message sendmail yank-media rfc822 mml mml-sec epa derived epg
rfc6068 epg-config mm-decode mm-bodies mm-encode mail-parse rfc2231
rfc2047 rfc2045 ietf-drums mailabbrev gmm-utils mailheader pcvs-util
magit-core magit-autorevert magit-margin magit-transient magit-process
with-editor magit-mode magit-git magit-base magit-section crm
markdown-mode color combobulate combobulate-yaml combobulate-css
combobulate-js-ts combobulate-python combobulate-html combobulate-ui
transient combobulate-display let-alist combobulate-contrib
multiple-cursors mc-separate-operations rectangular-region-mode
mc-mark-pop mc-edit-lines mc-hide-unmatched-lines-mode mc-mark-more
mc-cycle-cursors multiple-cursors-core rect combobulate-manipulation
python combobulate-navigation combobulate-misc combobulate-interface
combobulate-rules combobulate-settings tempo elec-pair
typescript-ts-mode js c-ts-common cc-mode cc-fonts cc-guess cc-menus
cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs treesit disp-table
vc-git diff-mode vc-dispatcher misearch multi-isearch dired-aux
image-file image-converter helm-external helm-net find-dired grep vc-svn
winner ffap tramp-archive tramp-gvfs tramp-cache time-stamp zeroconf
dbus add-log ido-completing-read+ memoize minibuf-eldef elisp-slime-nav
paredit highlight-symbol flycheck editorconfig editorconfig-core
editorconfig-core-handle editorconfig-fnmatch company-oddmuse
company-keywords company-etags etags fileloop generator company-gtags
company-dabbrev-code company-dabbrev company-files company-clang
company-capf company-cmake company-semantic company-template
company-bbdb company eglot external-completion array jsonrpc ert ewoc
debug backtrace flymake-proc flymake warnings which-func hideshow eww
url-queue thingatpt shr pixel-fill kinsoku url-file svg xml dom puny
mm-url gnus nnheader gnus-util mail-utils range mm-util mail-prsvr
helm-imenu helm-mode helm-misc helm-files image-dired image-dired-tags
image-dired-external image-dired-util xdg image-mode dired
dired-loaddefs exif tramp tramp-loaddefs trampver tramp-integration
cus-edit pp cus-load wid-edit files-x tramp-compat shell parse-time
iso8601 ls-lisp helm-buffers helm-occur helm-tags helm-locate helm-grep
helm-regexp helm-utils helm-help helm-types helm helm-global-bindings
helm-easymenu helm-core async-bytecomp helm-source helm-multi-match
helm-lib async pcase imenu ob-plantuml org ob ob-tangle ob-ref ob-lob
ob-table ob-exp org-macro org-src ob-comint org-pcomplete pcomplete
org-list org-footnote org-faces org-entities time-date noutline outline
icons ob-emacs-lisp ob-core ob-eval org-cycle org-table ol org-fold
org-fold-core org-keys oc org-loaddefs find-func cal-menu calendar
cal-loaddefs org-version org-compat org-macs format-spec delsel
autorevert filenotify yasnippet nlinum linum ido-yes-or-no advice ido
edmacro kmacro use-package-bind-key bind-key easy-mmode xref project
server hl-line pixel-scroll cua-base compile-eslint compile
text-property-search comint ansi-osc ansi-color ring doom-modeline
doom-modeline-segments doom-modeline-env doom-modeline-core
all-the-icons all-the-icons-faces data-material data-weathericons
data-octicons data-fileicons data-faicons data-alltheicons shrink-path
rx f f-shortdoc s dash compat dracula-theme cl-extra help-mode
use-package-ensure use-package-core finder-inf yasnippet-autoloads
ido-yes-or-no-autoloads elisp-slime-nav-autoloads cmake-mode-autoloads
flycheck-autoloads pkg-info-autoloads magit-autoloads
all-the-icons-autoloads crontab-mode-autoloads powershell-autoloads
doom-modeline-autoloads undo-tree-autoloads rust-mode-autoloads
magit-section-autoloads paredit-autoloads dracula-theme-autoloads
cargo-autoloads yaml-mode-autoloads helm-autoloads popup-autoloads
queue-autoloads nlinum-autoloads bmx-mode-autoloads company-autoloads
git-commit-autoloads multiple-cursors-autoloads dap-mode-autoloads
lsp-treemacs-autoloads treemacs-autoloads cfrs-autoloads
posframe-autoloads hydra-autoloads pfuture-autoloads
ace-window-autoloads avy-autoloads bui-autoloads transient-autoloads
ido-completing-read+-autoloads memoize-autoloads with-editor-autoloads
compat-autoloads epl-autoloads lsp-docker-autoloads yaml-autoloads
highlight-symbol-autoloads expand-region-autoloads lsp-mode-autoloads
lv-autoloads markdown-mode-autoloads spinner-autoloads ht-autoloads
shrink-path-autoloads f-autoloads dash-autoloads s-autoloads info
editorconfig-autoloads helm-core-autoloads async-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 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 move-toolbar gtk x-toolkit xinput2 x multi-tty
make-network-process emacs)

Memory information:
((conses 16 923748 105144)
  (symbols 48 55100 49)
  (strings 32 257968 16692)
  (string-bytes 1 7664141)
  (vectors 16 115330)
  (vector-slots 8 2152154 163085)
  (floats 8 1189 588)
  (intervals 56 28029 2201)
  (buffers 984 86))

-- 
Vennlig hilsen
*Jostein Kjønigsen*

jostein@kjonigsen.net 🍵 jostein@gmail.com
https://jostein.kjønigsen.no <https://jostein.kjønigsen.no>

[-- Attachment #2: Type: text/html, Size: 15461 bytes --]

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Randy Taylor]

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

On Sunday, February 5th, 2023 at 15:15, Jostein Kjønigsen <jostein@secure.kjonigsen.net> wrote:

> Steps to reproduce:
>
> - set font-lock level 4 to enable highlighting of function-invocations
> - create a new buffer and activate rust-ts-mode
>
> Try writing a function which invokes functions indirectly trough field-properties. Example below:
>
> pub fn should_handle(url: String) -> bool {
> if url.ends_with(".css")
> || url.ends_with(".js")
> || url.ends_with(".png")
> || url.ends_with(".jpg")
> {
> false
> } else {
> true
> }
> }
> Observe that:
>
> - ends_with() is fontified as a property-access, not as a function-invocation.
> - (plain function invocation is highlighted as expected though)
>
> From my preliminary inspection of the rust-ts-mode source-code, this seems to be because of a very general override later in the file:
>
> rust-ts-mode.el, line 248 or so:
>
> :language 'rust
> :feature 'property
> :override t
> '((field_identifier) @font-lock-property-face
> (shorthand_field_initializer (identifier) @font-lock-property-face))
>
> Dissabling "override" for this feature fixes the fontification of method invocations, but I haven't done enough testing to see if there are other unexpected side-effects.

That's expected (at least to me) because it's a property. The same applies to c-ts-mode and go-ts-mode as well.

[-- Attachment #2: Type: text/html, Size: 2634 bytes --]

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Jostein Kjønigsen]

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


On Sun, Feb 5, 2023, at 22:30, Randy Taylor wrote:
> 
> That's expected (at least to me) because it's a property. The same applies to c-ts-mode and go-ts-mode as well.
> 
I mean… yea it’s a property, but it’s clearly a function-valued property, and you’re calling it. 

Even simpler: If you store a function reference in a plain variable and call that variable, is that a variable or a function you’re calling? ;)

As for consistency, csharp-ts-mode, js-ts-mode, typescript-ts-mode (and tsx-ts-mode) all does the  exact opposite: functions always takes precedence over properties.

In fact, they mostly highlight declarations of properties only, not regular access, which kind avoids the duality-problem we’re seeing in rust-ts-mode in this particular case. 

Would it be an option/goal to try to align these other modes with a such a model which is somewhat more clearly defined to make it simpler  to resolve cases like this?

—
Jostein

[-- Attachment #2: Type: text/html, Size: 1568 bytes --]

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Jostein Kjønigsen]

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



--
Vennlig hilsen
Jostein Kjønigsen

jostein@kjonigsen.net 🍵 jostein@gmail.com
https://jostein.kjønigsen.no <https://jostein.xn--kjnigsen-64a.no/>

On Sun, Feb 5, 2023, at 22:52, Jostein Kjønigsen wrote:
> 
> On Sun, Feb 5, 2023, at 22:30, Randy Taylor wrote:
>> 
>> That's expected (at least to me) because it's a property. The same applies to c-ts-mode and go-ts-mode as well.
>> 
> I mean… yea it’s a property, but it’s clearly a function-valued property, and you’re calling it. 
> 
> Even simpler: If you store a function reference in a plain variable and call that variable, is that a variable or a function you’re calling? ;)
> 
> As for consistency, csharp-ts-mode, js-ts-mode, typescript-ts-mode (and tsx-ts-mode) all does the  exact opposite: functions always takes precedence over properties.
> 
> In fact, they mostly highlight declarations of properties only, not regular access, which kind avoids the duality-problem we’re seeing in rust-ts-mode in this particular case. 
> 
> Would it be an option/goal to try to align these other modes with a such a model which is somewhat more clearly defined to make it simpler  to resolve cases like this?
> 
> —
> Jostein

Also worth noting, there are clearly highly tailored treesitter-queries to fontify these exact property-access based function-invocations. They are not getting fontified by luck or wide/general matchers elsewhere. 

Someone somewhere wanted these to be fontified, and had patches accepted for that exact purpose. 

To say that it is expected for them not to be used would then imply the queries should not be there in the first place. 

I’m not saying which side is right, but if we are to chose one, we should strive to do so consistently :) 

—
Jostein

[-- Attachment #2: Type: text/html, Size: 2962 bytes --]

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Randy Taylor]

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

On Sun, Feb 5, 2023, at 22:52, Jostein Kjønigsen wrote:
>>
>> On Sun, Feb 5, 2023, at 22:30, Randy Taylor wrote:
>>>
>>> That's expected (at least to me) because it's a property. The same applies to c-ts-mode and go-ts-mode as well.
>> I mean… yea it’s a property, but it’s clearly a function-valued property, and you’re calling it.
>>
>> Even simpler: If you store a function reference in a plain variable and call that variable, is that a variable or a function you’re calling? ;)
>>
>> As for consistency, csharp-ts-mode, js-ts-mode, typescript-ts-mode (and tsx-ts-mode) all does the exact opposite: functions always takes precedence over properties.
>>
>> In fact, they mostly highlight declarations of properties only, not regular access, which kind avoids the duality-problem we’re seeing in rust-ts-mode in this particular case.
>>
>> Would it be an option/goal to try to align these other modes with a such a model which is somewhat more clearly defined to make it simpler to resolve cases like this?

When I introduced font-lock-property-face, I intended for it to be used to highlight all properties.

Clearly that's not what most people had in mind, and it seems that many would prefer property functions (for lack of better term) be highlighted as functions.
I can live with that, and yes, all the modes should be consistent so Rust, Go, C/C++ and any anything else should be fixed up to abide.
I'll fix Go up sometime next week unless someone beats me to it.

Perhaps in the future we can introduce more fine-grained options for properties if it's desired.

>
>Also worth noting, there are clearly highly tailored treesitter-queries to fontify these exact property-access based function-invocations. They are not getting fontified by luck or wide/general matchers elsewhere.
>
>Someone somewhere wanted these to be fontified, and had patches accepted for that exact purpose.
>
>To say that it is expected for them not to be used would then imply the queries should not be there in the first place.

I don't know what this has to do with what I said.
My intention was to highlight all properties, and I picked the easiest query to do that.

>
>I’m not saying which side is right, but if we are to chose one, we should strive to do so consistently :)
>
>—>Jostein

[-- Attachment #2: Type: text/html, Size: 3227 bytes --]

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Dmitry Gutov]

On 06/02/2023 03:50, Randy Taylor wrote:
> On Sun, Feb 5, 2023, at 22:52, Jostein Kjønigsen wrote:
>>>
>>> On Sun, Feb 5, 2023, at 22:30, Randy Taylor wrote:
>>>>
>>>> That's expected (at least to me) because it's a property. The same applies to c-ts-mode and go-ts-mode as well.
>>> I mean… yea it’s a property, but it’s clearly a function-valued property, and you’re calling it.
>>>
>>> Even simpler: If you store a function reference in a plain variable and call that variable, is that a variable or a function you’re calling? ;)
>>>
>>> As for consistency, csharp-ts-mode, js-ts-mode, typescript-ts-mode (and tsx-ts-mode) all does the  exact opposite: functions always takes precedence over properties.
>>>
>>> In fact, they mostly highlight declarations of properties only, not regular access, which kind avoids the duality-problem we’re seeing in rust-ts-mode in this particular case. 
>>>
>>> Would it be an option/goal to try to align these other modes with a such a model which is somewhat more clearly defined to make it simpler  to resolve cases like this?
> 
> When I introduced font-lock-property-face, I intended for it to be used 
> to highlight all properties.
> 
> Clearly that's not what most people had in mind, and it seems that many 
> would prefer property functions (for lack of better term) be highlighted 
> as functions.
> I can live with that, and yes, all the modes should be consistent so 
> Rust, Go, C/C++ and any anything else should be fixed up to abide.
> I'll fix Go up sometime next week unless someone beats me to it.

We're getting very close to when the pretest should be cut, I'm not sure 
if we have an extra week. We're probably pushing the limits of the 
feature freeze here already.

I've pushed a fix for Go as well, it looked very similar to Rust's one.

Curiously, in both cases the 'function' feature already contained the 
necessary rule (which matched only "property functions"), which then was 
later overridden by a rule in 'property' feature.

> Perhaps in the future we can introduce more fine-grained options for 
> properties if it's desired.

Sure, in case you or others really feel that need. There are several 
ways we could go about adding that capability.

In the meantime, I've also added highlighting of variable declarations 
to go-ts-mode (to 'definition', so it's on feature level 3). This should 
finish bringing rust and go ts modes to parity with the rest.

Please take it for a spin when you have the time. I'm not too familiar 
with Go, so I could have missed some cases. But hopefully not.

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Randy Taylor]

On Sunday, February 5th, 2023 at 21:45, Dmitry Gutov <dgutov@yandex.ru> wrote:
> On 06/02/2023 03:50, Randy Taylor wrote:
> 
> > On Sun, Feb 5, 2023, at 22:52, Jostein Kjønigsen wrote:
> > 
> > > > On Sun, Feb 5, 2023, at 22:30, Randy Taylor wrote:
> > > > 
> > > > > That's expected (at least to me) because it's a property. The same applies to c-ts-mode and go-ts-mode as well.
> > > > > I mean… yea it’s a property, but it’s clearly a function-valued property, and you’re calling it.
> > > > 
> > > > Even simpler: If you store a function reference in a plain variable and call that variable, is that a variable or a function you’re calling? ;)
> > > > 
> > > > As for consistency, csharp-ts-mode, js-ts-mode, typescript-ts-mode (and tsx-ts-mode) all does the exact opposite: functions always takes precedence over properties.
> > > > 
> > > > In fact, they mostly highlight declarations of properties only, not regular access, which kind avoids the duality-problem we’re seeing in rust-ts-mode in this particular case.
> > > > 
> > > > Would it be an option/goal to try to align these other modes with a such a model which is somewhat more clearly defined to make it simpler to resolve cases like this?
> > 
> > When I introduced font-lock-property-face, I intended for it to be used
> > to highlight all properties.
> > 
> > Clearly that's not what most people had in mind, and it seems that many
> > would prefer property functions (for lack of better term) be highlighted
> > as functions.
> > I can live with that, and yes, all the modes should be consistent so
> > Rust, Go, C/C++ and any anything else should be fixed up to abide.
> > I'll fix Go up sometime next week unless someone beats me to it.
> 
> 
> We're getting very close to when the pretest should be cut, I'm not sure
> if we have an extra week. We're probably pushing the limits of the
> feature freeze here already.
> 
> I've pushed a fix for Go as well, it looked very similar to Rust's one.

Thanks! It's still Sunday over here, so I meant within the next couple of days or so.
My fault for speaking so self-centredly and lazily :).

> 
> Curiously, in both cases the 'function' feature already contained the
> necessary rule (which matched only "property functions"), which then was
> later overridden by a rule in 'property' feature.
> 
> > Perhaps in the future we can introduce more fine-grained options for
> > properties if it's desired.
> 
> 
> Sure, in case you or others really feel that need. There are several
> ways we could go about adding that capability.
> 
> In the meantime, I've also added highlighting of variable declarations
> to go-ts-mode (to 'definition', so it's on feature level 3). This should
> finish bringing rust and go ts modes to parity with the rest.
> 
> Please take it for a spin when you have the time. I'm not too familiar
> with Go, so I could have missed some cases. But hopefully not.

Will do, thanks again.

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Randy Taylor]

On Sunday, February 5th, 2023 at 21:45, Dmitry Gutov <dgutov@yandex.ru> wrote:
> 
> In the meantime, I've also added highlighting of variable declarations
> to go-ts-mode (to 'definition', so it's on feature level 3). This should
> finish bringing rust and go ts modes to parity with the rest.
> 
> Please take it for a spin when you have the time. I'm not too familiar
> with Go, so I could have missed some cases. But hopefully not.

Hi Dmitry,

I've given them a test spin.

c/c++-ts-modes look good to me.

go-ts-mode looks good to me.

rust-ts-mode looks good to me as well except the imports. Stuff like:
use std::fmt::{Display, Formatter};

is highlighted incorrectly. In the above example, std and fmt are highlighted as variables.

We should give them font-lock-constant-face.

I will try to propose a patch later today unless someone beats me to it.

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Dmitry Gutov]

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

Hi Randy.

On 07/02/2023 16:26, Randy Taylor wrote:

> I've given them a test spin.
> 
> c/c++-ts-modes look good to me.
> 
> go-ts-mode looks good to me.

Very good.

> rust-ts-mode looks good to me as well except the imports. Stuff like:
> use std::fmt::{Display, Formatter};
> 
> is highlighted incorrectly. In the above example, std and fmt are highlighted as variables.

This is a result of 'variable' being implemented as it is now -- 
highlighting all (identifier) nodes that no previous rule has matched.

That makes things complicated when we try to support customizable 
highlighting level where the user can mix and match the enabled features.

With imports, there was also another problem which I mentioned here: 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=61205#68

If we highlight the imports as constants on level 3, when the 'function' 
feature is disabled, the function names will get highlighted with 
font-lock-constant-face as well. That seems undesirable.

But -- and this just occurred to me today -- if we create a separate 
feature to add to level 4, with rules defined below 'function', that 
should satisfy all the constraints.

> We should give them font-lock-constant-face.
> 
> I will try to propose a patch later today unless someone beats me to it.

Try the attached patch, please.

On a distantly related note, we have terms like 'usize' which is 
normally a type (and highlighted as such), but can also feature in 
expressions like

   let row = usize::from_str_radix(row, 10).map_err(|_| error())?;

where it is now highlighted with font-lock-constant-face. Should we try 
to do anything about that? If there is a limited number of built-in 
types in that situation (e.g. all of them primitives), we could handle 
that with a regexp.

[-- Attachment #2: rust-ts-mode-highlight-module.diff --]
[-- Type: text/x-patch, Size: 1419 bytes --]

diff --git a/lisp/progmodes/rust-ts-mode.el b/lisp/progmodes/rust-ts-mode.el
index 5c71a8ad461..e2f198704f6 100644
--- a/lisp/progmodes/rust-ts-mode.el
+++ b/lisp/progmodes/rust-ts-mode.el
@@ -239,6 +239,19 @@ rust-ts-mode--font-lock-settings
    '((field_identifier) @font-lock-property-face
      (shorthand_field_initializer (identifier) @font-lock-property-face))
 
+   :language 'rust
+   :feature 'module
+   '((scoped_identifier name: (identifier) @font-lock-constant-face)
+     (scoped_identifier path: (identifier) @font-lock-constant-face)
+     (scoped_identifier
+      (scoped_identifier
+       path: (identifier) @font-lock-constant-face))
+     (scoped_identifier
+      path: [(identifier) @font-lock-constant-face
+             (scoped_identifier
+              name: (identifier) @font-lock-constant-face)])
+     (scoped_use_list path: (identifier) @font-lock-constant-face))
+
    :language 'rust
    :feature 'variable
    '((identifier) @font-lock-variable-name-face
@@ -344,7 +357,8 @@ rust-ts-mode
                   ( keyword string)
                   ( attribute builtin constant escape-sequence
                     number type)
-                  ( bracket delimiter error function operator property variable)))
+                  ( bracket delimiter error function operator property variable
+                    module)))
 
     ;; Imenu.
     (setq-local treesit-simple-imenu-settings

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Dmitry Gutov]

On 07/02/2023 20:16, Dmitry Gutov wrote:
> On a distantly related note, we have terms like 'usize' which is 
> normally a type (and highlighted as such), but can also feature in 
> expressions like
> 
>    let row = usize::from_str_radix(row, 10).map_err(|_| error())?;
> 
> where it is now highlighted with font-lock-constant-face. Should we try 
> to do anything about that? If there is a limited number of built-in 
> types in that situation (e.g. all of them primitives), we could handle 
> that with a regexp.

Or vice versa, in

   use std::{fmt, fs, usize};

should 'fmt', 'fs' and 'usize' be highlighted with 
font-lock-constant-face rather than font-lock-type-face?

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Randy Taylor]

On Tuesday, February 7th, 2023 at 13:25, Dmitry Gutov <dgutov@yandex.ru> wrote:
>> rust-ts-mode looks good to me as well except the imports. Stuff like:
>> use std::fmt::{Display, Formatter};
>>
>> is highlighted incorrectly. In the above example, std and fmt are highlighted as variables.
>
>This is a result of 'variable' being implemented as it is now --
>highlighting all (identifier) nodes that no previous rule has matched.
>
>That makes things complicated when we try to support customizable
>highlighting level where the user can mix and match the enabled features.
>
>With imports, there was also another problem which I mentioned here:
>https://debbugs.gnu.org/cgi/bugreport.cgi?bug=61205#68
>
>If we highlight the imports as constants on level 3, when the 'function'
>feature is disabled, the function names will get highlighted with
>font-lock-constant-face as well. That seems undesirable.
>
>But -- and this just occurred to me today -- if we create a separate
>feature to add to level 4, with rules defined below 'function', that
>should satisfy all the constraints.

I think this is a good idea.

>
>> We should give them font-lock-constant-face.
>>
>> I will try to propose a patch later today unless someone beats me to it.
>
>Try the attached patch, please.

Thanks, it looks good to me.

I think the following rule from the type feature:
(scoped_type_identifier path: (identifier) @font-lock-type-face)

Should be changed to font-lock-constant-face and moved to the module feature.

That way, things like the following will be highlighted correctly:
let date = DateTime::<chrono::Utc>::from_utc(date, chrono::Utc);
                      ^^^^^^ this guy

Unless I'm missing something.

>
>On a distantly related note, we have terms like 'usize' which is
>normally a type (and highlighted as such), but can also feature in
>expressions like
>
>   let row = usize::from_str_radix(row, 10).map_err(|_| error())?;
>
>where it is now highlighted with font-lock-constant-face. Should we try
>to do anything about that? If there is a limited number of built-in
>types in that situation (e.g. all of them primitives), we could handle
>that with a regexp.

Right. I think it makes sense to handle the primitives with a regex.
I'm not sure if there's anything else beyond those.
There's a list of them here: https://doc.rust-lang.org/reference/types.html
I think it would only apply to the numerical and textual types.

>
>Or vice versa, in
>
>   use std::{fmt, fs, usize};
>
>should 'fmt', 'fs' and 'usize' be highlighted with
>font-lock-constant-face rather than font-lock-type-face?

They should indeed be highlighted with font-lock-constant-face because they are modules.
We assume the types will be capitalized since that's all we can really do (and it's the convention anyway).

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Dmitry Gutov]

On 08/02/2023 05:38, Randy Taylor wrote:
> On Tuesday, February 7th, 2023 at 13:25, Dmitry Gutov <dgutov@yandex.ru> wrote:
>>> rust-ts-mode looks good to me as well except the imports. Stuff like:
>>> use std::fmt::{Display, Formatter};
>>>
>>> is highlighted incorrectly. In the above example, std and fmt are highlighted as variables.
>>
>> This is a result of 'variable' being implemented as it is now --
>> highlighting all (identifier) nodes that no previous rule has matched.
>>
>> That makes things complicated when we try to support customizable
>> highlighting level where the user can mix and match the enabled features.
>>
>> With imports, there was also another problem which I mentioned here:
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=61205#68
>>
>> If we highlight the imports as constants on level 3, when the 'function'
>> feature is disabled, the function names will get highlighted with
>> font-lock-constant-face as well. That seems undesirable.
>>
>> But -- and this just occurred to me today -- if we create a separate
>> feature to add to level 4, with rules defined below 'function', that
>> should satisfy all the constraints.
> 
> I think this is a good idea.
> 
>>
>>> We should give them font-lock-constant-face.
>>>
>>> I will try to propose a patch later today unless someone beats me to it.
>>
>> Try the attached patch, please.
> 
> Thanks, it looks good to me.
> 
> I think the following rule from the type feature:
> (scoped_type_identifier path: (identifier) @font-lock-type-face)
> 
> Should be changed to font-lock-constant-face and moved to the module feature.
> 
> That way, things like the following will be highlighted correctly:
> let date = DateTime::<chrono::Utc>::from_utc(date, chrono::Utc);
>                        ^^^^^^ this guy
> 
> Unless I'm missing something.

Should Utc in the above example be highlighted with 
font-lock-constant-face too?

What if it looked like this:

   let date = DateTime::<chrono::utc>::from_utc(date, chrono::utc);

If we decide purely based on capitalization, then I guess the rule 
should be present in both lists (with capitalized? regexp in one, and 
!capitalized? regexp in another), and a few more rules should be 
duplicated as well.

This becomes a little more painful semantically, given that the first 
'utc' in the example above is parsed into a (type_identifier) node, not 
just (identifier).

>> On a distantly related note, we have terms like 'usize' which is
>> normally a type (and highlighted as such), but can also feature in
>> expressions like
>>
>>    let row = usize::from_str_radix(row, 10).map_err(|_| error())?;
>>
>> where it is now highlighted with font-lock-constant-face. Should we try
>> to do anything about that? If there is a limited number of built-in
>> types in that situation (e.g. all of them primitives), we could handle
>> that with a regexp.
> 
> Right. I think it makes sense to handle the primitives with a regex.
> I'm not sure if there's anything else beyond those.
> There's a list of them here: https://doc.rust-lang.org/reference/types.html
> I think it would only apply to the numerical and textual types.

So 'usize' in the above is definitely a "type", not a "module"?

>> Or vice versa, in
>>
>>    use std::{fmt, fs, usize};
>>
>> should 'fmt', 'fs' and 'usize' be highlighted with
>> font-lock-constant-face rather than font-lock-type-face?
> 
> They should indeed be highlighted with font-lock-constant-face because they are modules.
> We assume the types will be capitalized since that's all we can really do (and it's the convention anyway).

If they're modules here, I suppose they should be highlighted the same in

   let row = usize::from_str_radix(...)

as well. The bright side is that will make a more complex regexp 
(enumerating the lowercase named types) unnecessary.

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Randy Taylor]

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

On Wednesday, February 8th, 2023 at 10:44, Dmitry Gutov <dgutov@yandex.ru> wrote:
>On 08/02/2023 05:38, Randy Taylor wrote:
>> I think the following rule from the type feature:
>> (scoped_type_identifier path: (identifier) @font-lock-type-face)
>>
>> Should be changed to font-lock-constant-face and moved to the module feature.
>>
>> That way, things like the following will be highlighted correctly:
>> let date = DateTime::<chrono::Utc>::from_utc(date, chrono::Utc);
>>                        ^^^^^^ this guy
>>
>> Unless I'm missing something.
>
>Should Utc in the above example be highlighted with
>font-lock-constant-face too?

No. It's a type.

>
>What if it looked like this:
>
>   let date = DateTime::<chrono::utc>::from_utc(date, chrono::utc);
>
>If we decide purely based on capitalization, then I guess the rule
>should be present in both lists (with capitalized? regexp in one, and
>!capitalized? regexp in another), and a few more rules should be
>duplicated as well.

In both cases, utc is still a type even if it's not capitalized.
My patch addresses this.

>
>This becomes a little more painful semantically, given that the first
>'utc' in the example above is parsed into a (type_identifier) node, not
>just (identifier).
>
>>> On a distantly related note, we have terms like 'usize' which is
>>> normally a type (and highlighted as such), but can also feature in
>>> expressions like
>>>
>>>    let row = usize::from_str_radix(row, 10).map_err(|_| error())?;
>>>
>>> where it is now highlighted with font-lock-constant-face. Should we try
>>> to do anything about that? If there is a limited number of built-in
>>> types in that situation (e.g. all of them primitives), we could handle
>>> that with a regexp.
>>
>> Right. I think it makes sense to handle the primitives with a regex.
>> I'm not sure if there's anything else beyond those.
>> There's a list of them here: https://doc.rust-lang.org/reference/types.html
>> I think it would only apply to the numerical and textual types.
>
>So 'usize' in the above is definitely a "type", not a "module"?

I think so. You can see on usize's documentation page (https://doc.rust-lang.org/std/primitive.usize.html)
that it provides that function, amongst many others.

>
>>> Or vice versa, in
>>>
>>>    use std::{fmt, fs, usize};
>>>
>>> should 'fmt', 'fs' and 'usize' be highlighted with
>>> font-lock-constant-face rather than font-lock-type-face?
>>
>> They should indeed be highlighted with font-lock-constant-face because they are modules.
>> We assume the types will be capitalized since that's all we can really do (and it's the convention anyway).
>
>If they're modules here, I suppose they should be highlighted the same in
>
>   let row = usize::from_str_radix(...)
>
>as well. The bright side is that will make a more complex regexp
>(enumerating the lowercase named types) unnecessary.

Yes, except for the primitives.

I have attached a patch which I think addresses most of the concerns (although I've been at it for a few hours and my brain is mush now).

The patch does the following:
- Separates import-related stuff and module use by leveraging the use_declaration query (simplifying things greatly IMO).
- Highlights primitive types used in scoped_identifiers.
- Properly highlights types belonging to a module no matter how deep it is (or your money back guaranteed!).
- Maybe some other stuff I forgot. I'm too tried now :).

A few questions:
- Should module be moved to level 3 to be with type?
- Do we still want the module feature, or should this stuff be put into type?

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-rust-ts-mode-type-and-module-highlighting.patch --]
[-- Type: text/x-patch; name=0001-Fix-rust-ts-mode-type-and-module-highlighting.patch, Size: 4522 bytes --]

From 62ca2499a8d645c78e9ae8b4e233577b7ec5b0ae Mon Sep 17 00:00:00 2001
From: Randy Taylor <dev@rjt.dev>
Date: Wed, 8 Feb 2023 21:43:04 -0500
Subject: [PATCH] Fix rust-ts-mode type and module highlighting

* lisp/progmodes/rust-ts-mode.el (rust-ts-mode--font-lock-settings):
Add new 'module' feature and simplify 'type' feature.
(rust-ts-mode): Add 'module' feature.
---
 lisp/progmodes/rust-ts-mode.el | 66 +++++++++++++++++++++++++---------
 1 file changed, 50 insertions(+), 16 deletions(-)

diff --git a/lisp/progmodes/rust-ts-mode.el b/lisp/progmodes/rust-ts-mode.el
index 5c71a8ad461..6ab786c6c80 100644
--- a/lisp/progmodes/rust-ts-mode.el
+++ b/lisp/progmodes/rust-ts-mode.el
@@ -204,6 +204,41 @@ rust-ts-mode--font-lock-settings
       (raw_string_literal)
       (string_literal)] @font-lock-string-face)
 
+   :language 'rust
+   :feature 'module
+   `((scoped_identifier path: (crate)
+                        name: (identifier) @font-lock-constant-face)
+
+     (scoped_use_list path: (identifier) @font-lock-constant-face)
+     (scoped_use_list path: (scoped_identifier
+                             name: (identifier) @font-lock-constant-face))
+
+     ((use_declaration
+       argument: (scoped_identifier
+                  path: (_) @font-lock-constant-face
+                  name: (identifier) @font-lock-type-face))
+      (:match "^[A-Z]" @font-lock-type-face))
+     (use_declaration
+      argument: (scoped_identifier
+                 name: (identifier) @font-lock-constant-face))
+
+     (use_declaration
+      argument: (scoped_identifier
+                 path: (scoped_identifier
+                        path: (_) @font-lock-constant-face
+                        name: (identifier) @font-lock-constant-face)
+                 name: (identifier) @font-lock-constant-face))
+
+     (use_declaration
+      argument: (scoped_use_list
+                 path: (scoped_identifier
+                        path: (_) @font-lock-constant-face
+                        name: (identifier) @font-lock-constant-face)))
+
+     ((use_list (identifier) @font-lock-type-face)
+      (:match "^[A-Z]" @font-lock-type-face))
+     (use_list (identifier) @font-lock-constant-face))
+
    :language 'rust
    :feature 'type
    `((enum_variant name: (identifier) @font-lock-type-face)
@@ -216,23 +251,21 @@ rust-ts-mode--font-lock-settings
      (mod_item name: (identifier) @font-lock-constant-face)
      (primitive_type) @font-lock-type-face
      (type_identifier) @font-lock-type-face
-     ((scoped_identifier name: (identifier) @font-lock-type-face)
-      (:match "^[A-Z]" @font-lock-type-face))
      ((scoped_identifier path: (identifier) @font-lock-type-face)
-      (:match "^[A-Z]" @font-lock-type-face))
-     ((scoped_identifier
-        (scoped_identifier
-         path: (identifier) @font-lock-type-face))
-      (:match "^[A-Z]" @font-lock-type-face))
-     ((scoped_identifier
-       path: [(identifier) @font-lock-type-face
-              (scoped_identifier
-               name: (identifier) @font-lock-type-face)])
-      (:match "^[A-Z]" @font-lock-type-face))
-     (scoped_type_identifier path: (identifier) @font-lock-type-face)
+      (:match
+       "^\\(u8\\|u16\\|u32\\|u64\\|u128\\|usize\\|i8\\|i16\\|i32\\|i64\\|i128\\|isize\\|char\\|str\\)$"
+       @font-lock-type-face))
+     (scoped_identifier path: (_) @font-lock-constant-face
+                        name: (identifier) @font-lock-type-face)
+     (scoped_identifier path: (scoped_identifier
+                               name: (identifier) @font-lock-constant-face))
+     (scoped_type_identifier path: (_) @font-lock-constant-face)
+     (scoped_type_identifier
+      path: (scoped_identifier
+             path: (_) @font-lock-constant-face
+             name: (identifier) @font-lock-constant-face))
      (type_identifier) @font-lock-type-face
-     (use_as_clause alias: (identifier) @font-lock-type-face)
-     (use_list (identifier) @font-lock-type-face))
+     (use_as_clause alias: (identifier) @font-lock-type-face))
 
    :language 'rust
    :feature 'property
@@ -344,7 +377,8 @@ rust-ts-mode
                   ( keyword string)
                   ( attribute builtin constant escape-sequence
                     number type)
-                  ( bracket delimiter error function operator property variable)))
+                  ( bracket delimiter error function module
+                    operator property variable)))
 
     ;; Imenu.
     (setq-local treesit-simple-imenu-settings
-- 
2.39.1

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Dmitry Gutov]

On 09/02/2023 05:38, Randy Taylor wrote:

>> What if it looked like this:
>>
>>    let date = DateTime::<chrono::utc>::from_utc(date, chrono::utc);
>>
>> If we decide purely based on capitalization, then I guess the rule
>> should be present in both lists (with capitalized? regexp in one, and
>> !capitalized? regexp in another), and a few more rules should be
>> duplicated as well.
> 
> In both cases, utc is still a type even if it's not capitalized.
> My patch addresses this.

So the end of a scoping chain must also be either a type or a method 
call? We might be able to use that, somehow.

Though the 'use' declarations might be exceptions, e.g.

   use crate::foo::baz::foobaz;crate

or

   use std::{fmt, fs, usize};

(fmt and fs are modules, not types).

>> This becomes a little more painful semantically, given that the first
>> 'utc' in the example above is parsed into a (type_identifier) node, not
>> just (identifier).
>>
>>>> On a distantly related note, we have terms like 'usize' which is
>>>> normally a type (and highlighted as such), but can also feature in
>>>> expressions like
>>>>
>>>>     let row = usize::from_str_radix(row, 10).map_err(|_| error())?;
>>>>
>>>> where it is now highlighted with font-lock-constant-face. Should we try
>>>> to do anything about that? If there is a limited number of built-in
>>>> types in that situation (e.g. all of them primitives), we could handle
>>>> that with a regexp.
>>>
>>> Right. I think it makes sense to handle the primitives with a regex.
>>> I'm not sure if there's anything else beyond those.
>>> There's a list of them here: https://doc.rust-lang.org/reference/types.html
>>> I think it would only apply to the numerical and textual types.
>>
>> So 'usize' in the above is definitely a "type", not a "module"?
> 
> I think so. You can see on usize's documentation page (https://doc.rust-lang.org/std/primitive.usize.html)
> that it provides that function, amongst many others.

I was thinking that it might also be referring to (apparently 
deprecated) https://doc.rust-lang.org/std/usize/index.html.

Sorry, I'm not really familiar with Rust. E.g. in Ruby every class can 
also serve as a "module" in the scoping sense. As a result we highlight 
both classes and modules with font-lock-type-face. This could also be an 
option here, if everything else fails.

But we could also highlight based on a "role" (constant if it's used as 
a scope, and type if it's used as a type).

Although one could say that 'Path' in

   Some(Path::new("./foo"))

is being used as a type as well, and 'Some' too. So it might not be the 
best fit.

Speaking of 'usize' again, what if some lib or the app defines an 
'usize' module for its custom functions acting on it? E.g. 
'my::app::usize'. A simple regexp matcher will probably highlight it as 
a type as well.

>>>> Or vice versa, in
>>>>
>>>>     use std::{fmt, fs, usize};
>>>>
>>>> should 'fmt', 'fs' and 'usize' be highlighted with
>>>> font-lock-constant-face rather than font-lock-type-face?
>>>
>>> They should indeed be highlighted with font-lock-constant-face because they are modules.
>>> We assume the types will be capitalized since that's all we can really do (and it's the convention anyway).
>>
>> If they're modules here, I suppose they should be highlighted the same in
>>
>>    let row = usize::from_str_radix(...)
>>
>> as well. The bright side is that will make a more complex regexp
>> (enumerating the lowercase named types) unnecessary.
> 
> Yes, except for the primitives.
> 
> I have attached a patch which I think addresses most of the concerns (although I've been at it for a few hours and my brain is mush now).
> 
> The patch does the following:
> - Separates import-related stuff and module use by leveraging the use_declaration query (simplifying things greatly IMO).
> - Highlights primitive types used in scoped_identifiers.
> - Properly highlights types belonging to a module no matter how deep it is (or your money back guaranteed!).
> - Maybe some other stuff I forgot. I'm too tried now :).

Thank you, I can sympathize -- this stuff gets complicated.

Some problems from my testing:

1. If I keep treesit-font-lock-level at its default value (3), some 
stuff gets misfontified:

   use std::collections::hash_map::{self, HashMap};

'hash_map' is highlighted as a type. 'HashMap' is not highlighted at all.

   use std::{fmt, fs, usize};

Only 'use' is highlighted here.

   test::test1();

'test1' is highlighted as a type (we discussed this problem with 
highlighting types by default -- it becomes necessary to filter out 
function calls, either with more complex queries, or with custom 
highlighter functions).

2. If I switch to treesit-font-lock-level 4:

   let boxed_i32 = Box::new(5_i32);

'Box' is highlighted with font-lock-constant-face. I think it's a type, 
though.

Also here's a pre-existing problem mentioned above:

   use std::{fmt, fs, usize};

'fmt' and 'fs' are not types. But they are highlighted with 
font-lock-type-face.

> A few questions:
> - Should module be moved to level 3 to be with type?
> - Do we still want the module feature, or should this stuff be put into type?

I suppose we should iron some kinds out first to get a better understanding.

But if it's all the same code complexity wise, it wouldn't be the worst 
idea to keep 'module' on level 4, so level 3's highlighting is still 
somewhat subdued.

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Randy Taylor]

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

On Thursday, February 9th, 2023 at 16:19, Dmitry Gutov <dgutov@yandex.ru> wrote:
>On 09/02/2023 05:38, Randy Taylor wrote:
>
>>> What if it looked like this:
>>>
>>>    let date = DateTime::<chrono::utc>::from_utc(date, chrono::utc);
>>>
>>> If we decide purely based on capitalization, then I guess the rule
>>> should be present in both lists (with capitalized? regexp in one, and
>>> !capitalized? regexp in another), and a few more rules should be
>>> duplicated as well.
>>
>> In both cases, utc is still a type even if it's not capitalized.
>> My patch addresses this.
>
>So the end of a scoping chain must also be either a type or a method
>call? We might be able to use that, somehow.

I believe so (with the exception of use declarations as you note).
Not familiar enough with Rust to say for sure :).

>
>Though the 'use' declarations might be exceptions, e.g.
>
>   use crate::foo::baz::foobaz;crate
>
>or
>
>   use std::{fmt, fs, usize};
>
>(fmt and fs are modules, not types).
>
>>> This becomes a little more painful semantically, given that the first
>>> 'utc' in the example above is parsed into a (type_identifier) node, not
>>> just (identifier).
>>>
>>>>> On a distantly related note, we have terms like 'usize' which is
>>>>> normally a type (and highlighted as such), but can also feature in
>>>>> expressions like
>>>>>
>>>>>     let row = usize::from_str_radix(row, 10).map_err(|_| error())?;
>>>>>
>>>>> where it is now highlighted with font-lock-constant-face. Should we try
>>>>> to do anything about that? If there is a limited number of built-in
>>>>> types in that situation (e.g. all of them primitives), we could handle
>>>>> that with a regexp.
>>>>
>>>> Right. I think it makes sense to handle the primitives with a regex.
>>>> I'm not sure if there's anything else beyond those.
>>>> There's a list of them here: https://doc.rust-lang.org/reference/types.html
>>>> I think it would only apply to the numerical and textual types.
>>>
>>> So 'usize' in the above is definitely a "type", not a "module"?
>>
>> I think so. You can see on usize's documentation page (https://doc.rust-lang.org/std/primitive.usize.html)
>> that it provides that function, amongst many others.
>
>I was thinking that it might also be referring to (apparently
>deprecated) https://doc.rust-lang.org/std/usize/index.html.

That module only provides the constants listed on that page.
The usize type itself provides a bunch of constants and functions (same for the rest of the primitives).

I'm curious how other editors/IDEs highlight this stuff, but I don't have any on hand ATM.

>
>Sorry, I'm not really familiar with Rust. E.g. in Ruby every class can
>also serve as a "module" in the scoping sense. As a result we highlight
>both classes and modules with font-lock-type-face. This could also be an
>option here, if everything else fails.
>
>But we could also highlight based on a "role" (constant if it's used as
>a scope, and type if it's used as a type).
>
>Although one could say that 'Path' in
>
>   Some(Path::new("./foo"))
>
>is being used as a type as well, and 'Some' too. So it might not be the
>best fit.
>
>Speaking of 'usize' again, what if some lib or the app defines an
>'usize' module for its custom functions acting on it? E.g.
>'my::app::usize'. A simple regexp matcher will probably highlight it as
>a type as well.

I don't think we should worry about those cases IMO.

>
>>>>> Or vice versa, in
>>>>>
>>>>>     use std::{fmt, fs, usize};
>>>>>
>>>>> should 'fmt', 'fs' and 'usize' be highlighted with
>>>>> font-lock-constant-face rather than font-lock-type-face?
>>>>
>>>> They should indeed be highlighted with font-lock-constant-face because they are modules.
>>>> We assume the types will be capitalized since that's all we can really do (and it's the convention anyway).
>>>
>>> If they're modules here, I suppose they should be highlighted the same in
>>>
>>>    let row = usize::from_str_radix(...)
>>>
>>> as well. The bright side is that will make a more complex regexp
>>> (enumerating the lowercase named types) unnecessary.
>>
>> Yes, except for the primitives.
>>
>> I have attached a patch which I think addresses most of the concerns (although I've been at it for a few hours and my brain is mush now).
>>
>> The patch does the following:
>> - Separates import-related stuff and module use by leveraging the use_declaration query (simplifying things greatly IMO).
>> - Highlights primitive types used in scoped_identifiers.
>> - Properly highlights types belonging to a module no matter how deep it is (or your money back guaranteed!).
>> - Maybe some other stuff I forgot. I'm too tried now :).
>
>Thank you, I can sympathize -- this stuff gets complicated.
>
>Some problems from my testing:
>
>1. If I keep treesit-font-lock-level at its default value (3), some
>stuff gets misfontified:

Sorry, I have only been testing with level 4.
This is also why I want type and module combined into one so we don't have to deal with this headache.

>
>   use std::collections::hash_map::{self, HashMap};
>
>'hash_map' is highlighted as a type. 'HashMap' is not highlighted at all.
>
>   use std::{fmt, fs, usize};
>
>Only 'use' is highlighted here.

This is because of how things are broken out into the module feature.
That some highlighting for those occurs is by overlap of queries in the type feature.
Which again is why I think module should be part of type.

>
>   test::test1();
>
>'test1' is highlighted as a type (we discussed this problem with
>highlighting types by default -- it becomes necessary to filter out
>function calls, either with more complex queries, or with custom
>highlighter functions).

Right, I added a query to filter that out now.

>
>2. If I switch to treesit-font-lock-level 4:
>
>   let boxed_i32 = Box::new(5_i32);
>
>'Box' is highlighted with font-lock-constant-face. I think it's a type,
>though.

Oops, I accidentally removed the rule for that. Added it back.

>
>Also here's a pre-existing problem mentioned above:
>
>   use std::{fmt, fs, usize};
>
>'fmt' and 'fs' are not types. But they are highlighted with
>font-lock-type-face.

This is really weird, I can reproduce it with emacs -Q but not with my normal config...
Also with emacs -Q this:
let date = DateTime::<chrono::hey::there::Utc>::from_utc(date, chrono::cool::this::Utc);

highlights incorrectly, where "there" is font-lock-variable-name-face. But with my normal config everything is fine.

I'll look into it tomorrow. Not really sure what in my config could cause this...

>
>> A few questions:
>> - Should module be moved to level 3 to be with type?
>> - Do we still want the module feature, or should this stuff be put into type?
>
>I suppose we should iron some kinds out first to get a better understanding.

Attached a new patch hopefully addressing most of the problems you ran into (minus the level 3 use declaration highlights).
Especially after the problems you ran into at level 3, I strongly think the module queries should get thrown into type (and they make sense there anyway IMO). Then all those issues go away.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-rust-ts-mode-type-and-module-highlighting.patch --]
[-- Type: text/x-patch; name=0001-Fix-rust-ts-mode-type-and-module-highlighting.patch, Size: 4451 bytes --]

From 2cb9b54cca94c8aeb201878f2e13d2e84bf70fcf Mon Sep 17 00:00:00 2001
From: Randy Taylor <dev@rjt.dev>
Date: Wed, 8 Feb 2023 21:43:04 -0500
Subject: [PATCH] Fix rust-ts-mode type and module highlighting

* lisp/progmodes/rust-ts-mode.el (rust-ts-mode--font-lock-settings):
Add new 'module' feature and simplify 'type' feature.
(rust-ts-mode): Add 'module' feature.
---
 lisp/progmodes/rust-ts-mode.el | 64 +++++++++++++++++++++++++++-------
 1 file changed, 52 insertions(+), 12 deletions(-)

diff --git a/lisp/progmodes/rust-ts-mode.el b/lisp/progmodes/rust-ts-mode.el
index 5c71a8ad461..9b51432ccb3 100644
--- a/lisp/progmodes/rust-ts-mode.el
+++ b/lisp/progmodes/rust-ts-mode.el
@@ -204,6 +204,41 @@ rust-ts-mode--font-lock-settings
       (raw_string_literal)
       (string_literal)] @font-lock-string-face)
 
+   :language 'rust
+   :feature 'module
+   `((scoped_identifier path: (crate)
+                        name: (identifier) @font-lock-constant-face)
+
+     (scoped_use_list path: (identifier) @font-lock-constant-face)
+     (scoped_use_list path: (scoped_identifier
+                             name: (identifier) @font-lock-constant-face))
+
+     ((use_declaration
+       argument: (scoped_identifier
+                  path: (_) @font-lock-constant-face
+                  name: (identifier) @font-lock-type-face))
+      (:match "^[A-Z]" @font-lock-type-face))
+     (use_declaration
+      argument: (scoped_identifier
+                 name: (identifier) @font-lock-constant-face))
+
+     (use_declaration
+      argument: (scoped_identifier
+                 path: (scoped_identifier
+                        path: (_) @font-lock-constant-face
+                        name: (identifier) @font-lock-constant-face)
+                 name: (identifier) @font-lock-constant-face))
+
+     (use_declaration
+      argument: (scoped_use_list
+                 path: (scoped_identifier
+                        path: (_) @font-lock-constant-face
+                        name: (identifier) @font-lock-constant-face)))
+
+     ((use_list (identifier) @font-lock-type-face)
+      (:match "^[A-Z]" @font-lock-type-face))
+     (use_list (identifier) @font-lock-constant-face))
+
    :language 'rust
    :feature 'type
    `((enum_variant name: (identifier) @font-lock-type-face)
@@ -220,19 +255,23 @@ rust-ts-mode--font-lock-settings
       (:match "^[A-Z]" @font-lock-type-face))
      ((scoped_identifier path: (identifier) @font-lock-type-face)
       (:match "^[A-Z]" @font-lock-type-face))
-     ((scoped_identifier
-        (scoped_identifier
-         path: (identifier) @font-lock-type-face))
-      (:match "^[A-Z]" @font-lock-type-face))
-     ((scoped_identifier
-       path: [(identifier) @font-lock-type-face
-              (scoped_identifier
-               name: (identifier) @font-lock-type-face)])
-      (:match "^[A-Z]" @font-lock-type-face))
-     (scoped_type_identifier path: (identifier) @font-lock-type-face)
+     ((scoped_identifier path: (identifier) @font-lock-type-face)
+      (:match
+       "^\\(u8\\|u16\\|u32\\|u64\\|u128\\|usize\\|i8\\|i16\\|i32\\|i64\\|i128\\|isize\\|char\\|str\\)$"
+       @font-lock-type-face))
+     (scoped_identifier path: (_) @font-lock-constant-face
+                        name: (identifier) @font-lock-type-face)
+     (scoped_identifier path: (scoped_identifier
+                               name: (identifier) @font-lock-constant-face))
+     (scoped_type_identifier path: (_) @font-lock-constant-face)
+     (scoped_type_identifier
+      path: (scoped_identifier
+             path: (_) @font-lock-constant-face
+             name: (identifier) @font-lock-constant-face))
      (type_identifier) @font-lock-type-face
      (use_as_clause alias: (identifier) @font-lock-type-face)
-     (use_list (identifier) @font-lock-type-face))
+     ;; Ensure function calls aren't highlighted as types.
+     (call_expression function: (scoped_identifier name: (identifier) @default)))
 
    :language 'rust
    :feature 'property
@@ -344,7 +383,8 @@ rust-ts-mode
                   ( keyword string)
                   ( attribute builtin constant escape-sequence
                     number type)
-                  ( bracket delimiter error function operator property variable)))
+                  ( bracket delimiter error function module
+                    operator property variable)))
 
     ;; Imenu.
     (setq-local treesit-simple-imenu-settings
-- 
2.39.1

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Randy Taylor]

On Friday, February 10th, 2023 at 17:50, Dmitry Gutov <dgutov@yandex.ru> wrote:
>On 10/02/2023 05:44, Randy Taylor wrote:
>> I believe so (with the exception of use declarations as you note).
>> Not familiar enough with Rust to say for sure :).
>
>So this is a discussion between two people who don't write Rust.
>
>Hmmm. :-)

You couldn't find anyone more qualified if you tried ;).

>
>>>>> So 'usize' in the above is definitely a "type", not a "module"?
>>>>
>>>> I think so. You can see on usize's documentation page (https://doc.rust-lang.org/std/primitive.usize.html)
>>>> that it provides that function, amongst many others.
>>>
>>> I was thinking that it might also be referring to (apparently
>>> deprecated) https://doc.rust-lang.org/std/usize/index.html.
>>
>> That module only provides the constants listed on that page.
>> The usize type itself provides a bunch of constants and functions (same for the rest of the primitives).
>>
>> I'm curious how other editors/IDEs highlight this stuff, but I don't have any on hand ATM.
>
>Here's some overview.

Thank you! It's quite comprehensive.

>
>I mentioned previously
>https://github.com/tree-sitter/tree-sitter-rust/blob/master/queries/highlights.scm,
>which apparently corresponds to how Github highlights Rust syntax. E.g.
>here:
>https://github.com/tree-sitter/tree-sitter-rust/blob/master/examples/ast.rs
>
>The capitalized identifiers are highlighted as "types", apparently, and
>the lowercase segments are not highlighted at all. For some reason the
>types are also not highlighted in variable and parameter declarations.
>That seems like a problem, FWIW.

Agreed. I guess they dumb it down for the web.

>
>If I press "edit in dev", navigating to
>https://github.dev/tree-sitter/tree-sitter-rust/blob/master/examples/ast.rs,
>that seems to open some webby version of VS Code, except with a
>different color theme.
>
>Some observations:
>
>- A lot more highlights.
>- The "modules" and the "Types" have the same color.

I like how neovim does it (which is basically how we're doing it, with separate highlights).

>- The identifiers at the end of a scope chain are not highlighted if
>they are a) lowercase and, b) not from a known set, apparently.

Right. We can do better here IMO, and highlight them regardless because we know what they should be (which is what my patch does).
The exception being modules which require a little more care.

>- So "use std::usize;" is highlighted and "use std::uuusizeee;" is not.
>- "use std::foo::usize;" is highlighted.
>
>This also matches with VS Code's behavior installed locally. Neither the
>"cloud" VS Code nor the local one use tree-sitter, IIUC.
>
>IntelliJ Rust doesn't highlight "modules" or types at all, AFAICT:
>https://intellij-rust.github.io/assets/intro_screen_editor.png
>And those guys usually write their own parsers, so the highlights are
>most likely parse tree based, just not with tree-sitter.
>
>>> Sorry, I'm not really familiar with Rust. E.g. in Ruby every class can
>>> also serve as a "module" in the scoping sense. As a result we highlight
>>> both classes and modules with font-lock-type-face. This could also be an
>>> option here, if everything else fails.
>>>
>>> But we could also highlight based on a "role" (constant if it's used as
>>> a scope, and type if it's used as a type).
>>>
>>> Although one could say that 'Path' in
>>>
>>>    Some(Path::new("./foo"))
>>>
>>> is being used as a type as well, and 'Some' too. So it might not be the
>>> best fit.
>>>
>>> Speaking of 'usize' again, what if some lib or the app defines an
>>> 'usize' module for its custom functions acting on it? E.g.
>>> 'my::app::usize'. A simple regexp matcher will probably highlight it as
>>> a type as well.
>>
>> I don't think we should worry about those cases IMO.
>
>Okay.
>
>>> Some problems from my testing:
>>>
>>> 1. If I keep treesit-font-lock-level at its default value (3), some
>>> stuff gets misfontified:
>>
>> Sorry, I have only been testing with level 4.
>> This is also why I want type and module combined into one so we don't have to deal with this headache.
>
>I'm not quite sure what's the best choice here (keeping in mind the
>problem with overreaching variable highlights on level 4), but
>logically, I think 'module' belongs with 'function' and 'property'
>because all three denote some basic syntactic elements which are easy to
>understand even without colors, and are harder to make a mistake in.

I am proposing to get rid of the module feature entirely and bring those queries into the type feature because:
- Of how much overlap there is between them
- It will make maintaining the queries much easier
  - It's already a headache dealing with them separately, and I'm not sure the best way to deal with the issues of them being separate (and the different levels of highlighting to worry about). It will probably be quite the hack to deal with it, and no matter what I tried stuff was always sneaking through.
- It also won't introduce that much more highlighting

>
>That is in contrast with highlighting of variable declarations, for
>example, which in Rust can use some non-trivial syntax, more prone to
>user error.
>
>>>    use std::collections::hash_map::{self, HashMap};
>>>
>>> 'hash_map' is highlighted as a type. 'HashMap' is not highlighted at all.
>>>
>>>    use std::{fmt, fs, usize};
>>>
>>> Only 'use' is highlighted here.
>>
>> This is because of how things are broken out into the module feature.
>> That some highlighting for those occurs is by overlap of queries in the type feature.
>> Which again is why I think module should be part of type.
>>
>>>
>>>    test::test1();
>>>
>>> 'test1' is highlighted as a type (we discussed this problem with
>>> highlighting types by default -- it becomes necessary to filter out
>>> function calls, either with more complex queries, or with custom
>>> highlighter functions).
>>
>> Right, I added a query to filter that out now.
>
>Thanks, that works now.
>
>>>
>>> 2. If I switch to treesit-font-lock-level 4:
>>>
>>>    let boxed_i32 = Box::new(5_i32);
>>>
>>> 'Box' is highlighted with font-lock-constant-face. I think it's a type,
>>> though.
>>
>> Oops, I accidentally removed the rule for that. Added it back.
>
>That too.
>
>>> Also here's a pre-existing problem mentioned above:
>>>
>>>    use std::{fmt, fs, usize};
>>>
>>> 'fmt' and 'fs' are not types. But they are highlighted with
>>> font-lock-type-face.
>>
>> This is really weird, I can reproduce it with emacs -Q but not with my normal config...
>> Also with emacs -Q this:
>> let date = DateTime::<chrono::hey::there::Utc>::from_utc(date, chrono::cool::this::Utc);
>>
>> highlights incorrectly, where "there" is font-lock-variable-name-face. But with my normal config everything is fine.
>>
>> I'll look into it tomorrow. Not really sure what in my config could cause this...
>
>Thank you.

I did a clean build and wasn't able to reproduce it anymore. Guess it was stale bytecode or something?
At level 4 everything highlights correctly I believe, and with level 3 the only issues are the module highlighting ones, and to deal with that I think the module feature should be merged into the type one as I mentioned above. Then we can call it a day :). I'll post a new patch with those changes if you agree.

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Dmitry Gutov]

On 12/02/2023 04:48, Randy Taylor wrote:

>> I'm not quite sure what's the best choice here (keeping in mind the
>> problem with overreaching variable highlights on level 4), but
>> logically, I think 'module' belongs with 'function' and 'property'
>> because all three denote some basic syntactic elements which are easy to
>> understand even without colors, and are harder to make a mistake in.
> 
> I am proposing to get rid of the module feature entirely and bring those queries into the type feature because:
> - Of how much overlap there is between them
> - It will make maintaining the queries much easier
>    - It's already a headache dealing with them separately, and I'm not sure the best way to deal with the issues of them being separate (and the different levels of highlighting to worry about). It will probably be quite the hack to deal with it, and no matter what I tried stuff was always sneaking through.
> - It also won't introduce that much more highlighting

Okay, let's try that.

>>>> Also here's a pre-existing problem mentioned above:
>>>>
>>>>     use std::{fmt, fs, usize};
>>>>
>>>> 'fmt' and 'fs' are not types. But they are highlighted with
>>>> font-lock-type-face.
>>>
>>> This is really weird, I can reproduce it with emacs -Q but not with my normal config...
>>> Also with emacs -Q this:
>>> let date = DateTime::<chrono::hey::there::Utc>::from_utc(date, chrono::cool::this::Utc);
>>>
>>> highlights incorrectly, where "there" is font-lock-variable-name-face. But with my normal config everything is fine.
>>>
>>> I'll look into it tomorrow. Not really sure what in my config could cause this...
>>
>> Thank you.
> 
> I did a clean build and wasn't able to reproduce it anymore. Guess it was stale bytecode or something?
> At level 4 everything highlights correctly I believe, and with level 3 the only issues are the module highlighting ones, and to deal with that I think the module feature should be merged into the type one as I mentioned above. Then we can call it a day :). I'll post a new patch with those changes if you agree.

Please go ahead, and thanks. :-)

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Randy Taylor]

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

On Sunday, February 12th, 2023 at 22:37, Dmitry Gutov <dgutov@yandex.ru> wrote:
> On 12/02/2023 04:48, Randy Taylor wrote:
> 
> > > I'm not quite sure what's the best choice here (keeping in mind the
> > > problem with overreaching variable highlights on level 4), but
> > > logically, I think 'module' belongs with 'function' and 'property'
> > > because all three denote some basic syntactic elements which are easy to
> > > understand even without colors, and are harder to make a mistake in.
> > 
> > I am proposing to get rid of the module feature entirely and bring those queries into the type feature because:
> > - Of how much overlap there is between them
> > - It will make maintaining the queries much easier
> > - It's already a headache dealing with them separately, and I'm not sure the best way to deal with the issues of them being separate (and the different levels of highlighting to worry about). It will probably be quite the hack to deal with it, and no matter what I tried stuff was always sneaking through.
> > - It also won't introduce that much more highlighting
> 
> 
> Okay, let's try that.
> 
> > > > > Also here's a pre-existing problem mentioned above:
> > > > > 
> > > > > use std::{fmt, fs, usize};
> > > > > 
> > > > > 'fmt' and 'fs' are not types. But they are highlighted with
> > > > > font-lock-type-face.
> > > > 
> > > > This is really weird, I can reproduce it with emacs -Q but not with my normal config...
> > > > Also with emacs -Q this:
> > > > let date = DateTime::chrono::hey::there::Utc::from_utc(date, chrono::cool::this::Utc);
> > > > 
> > > > highlights incorrectly, where "there" is font-lock-variable-name-face. But with my normal config everything is fine.
> > > > 
> > > > I'll look into it tomorrow. Not really sure what in my config could cause this...
> > > 
> > > Thank you.
> > 
> > I did a clean build and wasn't able to reproduce it anymore. Guess it was stale bytecode or something?
> > At level 4 everything highlights correctly I believe, and with level 3 the only issues are the module highlighting ones, and to deal with that I think the module feature should be merged into the type one as I mentioned above. Then we can call it a day :). I'll post a new patch with those changes if you agree.
> 
> 
> Please go ahead, and thanks. :-)

Hi Dmitry and Jostein,

Here is the promised patch in all its glory.
I have also attached a screenshot of how things look now.

Changes:
- Attributes are now highlighted with font-lock-preprocessor-face.
- Added import-specific queries to cover all (hopefully...) import highlights, and simplified the existing scoped identifier types.
  - Note: I decided that anything at the end of a scoped identifier for imports or within a use list that's lowercased should be the default face because we don't know if it's a module, function, type, or whatever else. rust-mode also does this, and now we match their highlighting almost one for one. neovim and the rest do it this way too, and it makes the most sense.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-rust-ts-mode-type-and-module-highlighting-Bug-61.patch --]
[-- Type: text/x-patch; name=0001-Fix-rust-ts-mode-type-and-module-highlighting-Bug-61.patch, Size: 6200 bytes --]

From 4a933e123daa6a05164c806493b10c3e7a8a0af8 Mon Sep 17 00:00:00 2001
From: Randy Taylor <dev@rjt.dev>
Date: Wed, 8 Feb 2023 21:43:04 -0500
Subject: [PATCH] Fix rust-ts-mode type and module highlighting (Bug#61302)

* lisp/progmodes/rust-ts-mode.el (rust-ts-mode--font-lock-settings):
Specify import queries to avoid clashing with scoped identifiers in
the code.  Highlight attributes with font-lock-preprocessor-face.
---
 lisp/progmodes/rust-ts-mode.el | 93 ++++++++++++++++++++++++++--------
 1 file changed, 72 insertions(+), 21 deletions(-)

diff --git a/lisp/progmodes/rust-ts-mode.el b/lisp/progmodes/rust-ts-mode.el
index 5c71a8ad461..d0ff49e2f26 100644
--- a/lisp/progmodes/rust-ts-mode.el
+++ b/lisp/progmodes/rust-ts-mode.el
@@ -124,8 +124,8 @@ rust-ts-mode--font-lock-settings
   (treesit-font-lock-rules
    :language 'rust
    :feature 'attribute
-   '((attribute_item) @font-lock-constant-face
-     (inner_attribute_item) @font-lock-constant-face)
+   '((attribute_item) @font-lock-preprocessor-face
+     (inner_attribute_item) @font-lock-preprocessor-face)
 
    :language 'rust
    :feature 'bracket
@@ -147,12 +147,6 @@ rust-ts-mode--font-lock-settings
    :feature 'comment
    '(([(block_comment) (line_comment)]) @font-lock-comment-face)
 
-   :language 'rust
-   :feature 'constant
-   `((boolean_literal) @font-lock-constant-face
-     ((identifier) @font-lock-constant-face
-      (:match "^[A-Z][A-Z\\d_]*$" @font-lock-constant-face)))
-
    :language 'rust
    :feature 'delimiter
    '((["," "." ";" ":" "::"]) @font-lock-delimiter-face)
@@ -206,7 +200,54 @@ rust-ts-mode--font-lock-settings
 
    :language 'rust
    :feature 'type
-   `((enum_variant name: (identifier) @font-lock-type-face)
+   `((scoped_use_list path: (identifier) @font-lock-constant-face)
+     (scoped_use_list path: (scoped_identifier
+                             name: (identifier) @font-lock-constant-face))
+
+     ((use_as_clause alias: (identifier) @font-lock-type-face)
+      (:match "^[A-Z]" @font-lock-type-face))
+     ((use_as_clause path: (identifier) @font-lock-type-face)
+      (:match "^[A-Z]" @font-lock-type-face))
+     ((use_as_clause path:
+                     (scoped_identifier path: (_)
+                                        name: (identifier) @font-lock-type-face))
+      (:match "^[A-Z]" @font-lock-type-face))
+     (use_as_clause path: (scoped_identifier name: (identifier) @default))
+
+     ((use_declaration
+       argument: (scoped_identifier
+                  path: (_) @font-lock-constant-face
+                  name: (identifier) @font-lock-type-face))
+      (:match "^[A-Z]" @font-lock-type-face))
+     (use_declaration
+      argument: (scoped_identifier
+                 name: (identifier) @default))
+
+     (use_declaration
+      argument: (scoped_identifier
+                 path: (scoped_identifier
+                        path: (_) @font-lock-constant-face
+                        name: (identifier) @font-lock-constant-face)
+                 name: (identifier) @default))
+
+     (use_declaration
+      argument: (scoped_use_list
+                 path: (scoped_identifier
+                        path: (_) @font-lock-constant-face
+                        name: (identifier) @font-lock-constant-face)))
+
+     ((use_list (identifier) @font-lock-type-face)
+      (:match "^[A-Z]" @font-lock-type-face))
+     (use_list (identifier) @default)
+     ((use_list (scoped_identifier path: (_)
+                                   name: (identifier) @font-lock-type-face))
+      (:match "^[A-Z]" @font-lock-type-face))
+     (use_list (scoped_identifier path: (_)
+                                  name: (identifier) @default))
+     (use_wildcard (scoped_identifier
+                    name: (identifier) @font-lock-constant-face))
+
+     (enum_variant name: (identifier) @font-lock-type-face)
      (match_arm
       pattern: (match_pattern (_ type: (identifier) @font-lock-type-face)))
      (match_arm
@@ -220,25 +261,35 @@ rust-ts-mode--font-lock-settings
       (:match "^[A-Z]" @font-lock-type-face))
      ((scoped_identifier path: (identifier) @font-lock-type-face)
       (:match "^[A-Z]" @font-lock-type-face))
-     ((scoped_identifier
-        (scoped_identifier
-         path: (identifier) @font-lock-type-face))
-      (:match "^[A-Z]" @font-lock-type-face))
-     ((scoped_identifier
-       path: [(identifier) @font-lock-type-face
-              (scoped_identifier
-               name: (identifier) @font-lock-type-face)])
-      (:match "^[A-Z]" @font-lock-type-face))
-     (scoped_type_identifier path: (identifier) @font-lock-type-face)
+     ((scoped_identifier path: (identifier) @font-lock-type-face)
+      (:match
+       "^\\(u8\\|u16\\|u32\\|u64\\|u128\\|usize\\|i8\\|i16\\|i32\\|i64\\|i128\\|isize\\|char\\|str\\)$"
+       @font-lock-type-face))
+     (scoped_identifier path: (_) @font-lock-constant-face
+                        name: (identifier) @font-lock-type-face)
+     (scoped_identifier path: (scoped_identifier
+                               name: (identifier) @font-lock-constant-face))
+     (scoped_type_identifier path: (_) @font-lock-constant-face)
+     (scoped_type_identifier
+      path: (scoped_identifier
+             path: (_) @font-lock-constant-face
+             name: (identifier) @font-lock-constant-face))
      (type_identifier) @font-lock-type-face
-     (use_as_clause alias: (identifier) @font-lock-type-face)
-     (use_list (identifier) @font-lock-type-face))
+     ;; Ensure function calls aren't highlighted as types.
+     (call_expression function: (scoped_identifier name: (identifier) @default)))
 
    :language 'rust
    :feature 'property
    '((field_identifier) @font-lock-property-face
      (shorthand_field_initializer (identifier) @font-lock-property-face))
 
+   ;; Must be under type, otherwise some imports can be highlighted as consants.
+   :language 'rust
+   :feature 'constant
+   `((boolean_literal) @font-lock-constant-face
+     ((identifier) @font-lock-constant-face
+      (:match "^[A-Z][A-Z\\d_]*$" @font-lock-constant-face)))
+
    :language 'rust
    :feature 'variable
    '((identifier) @font-lock-variable-name-face
-- 
2.39.1


[-- Attachment #3: 2023-02-13-222104.png --]
[-- Type: image/png, Size: 51274 bytes --]

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Jostein Kjønigsen]

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

On 2/14/23 04:25, Randy Taylor wrote:
>
> Hi Dmitry and Jostein,
>
> Here is the promised patch in all its glory.
> I have also attached a screenshot of how things look now.
>
> Changes:
> - Attributes are now highlighted with font-lock-preprocessor-face.
> - Added import-specific queries to cover all (hopefully...) import highlights, and simplified the existing scoped identifier types.
>    - Note: I decided that anything at the end of a scoped identifier for imports or within a use list that's lowercased should be the default face because we don't know if it's a module, function, type, or whatever else. rust-mode also does this, and now we match their highlighting almost one for one. neovim and the rest do it this way too, and it makes the most sense.

Haven given the latest patch a try on a less macro-ridden codebase I 
have to say it looks a lot better. In general I would say for this other 
code-base it looks favourable to rust-mode.

If there's one thing which still seems to be done better in rust-mode 
(not nitpicking the variable highlighting), it would be fully namespaces 
function calls within classes.

Consider the following code:

extern crate claxon;
let result = claxon::FlacReader::open(path);

While rust-ts-mode correctly identifies that open() is a function-call, 
rust-mode correctly identified FlacReader as a class.

Based on the tree-sitter output for this node, it seems plausible that 
one should be able to implement the same for rust-ts-mode as well.

(source_file (line_comment)
  (extern_crate_declaration extern (crate) name: (identifier) ;)
  (let_declaration let pattern: (identifier) =
   value:
    (call_expression
     function:
      (scoped_identifier
       path: (scoped_identifier path: (identifier) ::
                                name: 
(identifier))                                <-- this is the class
       :: name: (identifier))
     arguments: (arguments ( (identifier) )))
   ;))

That's nowhere near a "big" problem though. It's just icing on the cake ;)

--
Jostein

[-- Attachment #2.1: Type: text/html, Size: 2878 bytes --]

[-- Attachment #2.2: 5LcAjDfSb1da6nv4.png --]
[-- Type: image/png, Size: 16670 bytes --]

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Randy Taylor]

[-- Attachment #1.1: Type: text/plain, Size: 852 bytes --]

On Tuesday, February 14th, 2023 at 06:42, Jostein Kjønigsen <jostein@secure.kjonigsen.net> wrote:
>Haven given the latest patch a try on a less macro-ridden codebase I have to say it looks a lot better. In general I would say for this other code-base it looks favourable to rust-mode.
>
>If there's one thing which still seems to be done better in rust-mode (not nitpicking the variable highlighting), it would be fully namespaces function calls within classes.
>
>Consider the following code:
>
>extern crate claxon;
>let result = claxon::FlacReader::open(path);
>>While rust-ts-mode correctly identifies that open() is a function-call, rust-mode correctly identified FlacReader as a class.

Thanks for testing. I've attached a patch fixing this.

I had deleted a query I thought was now covered :). I look forward to the day we have highlight tests!

[-- Attachment #1.2: Type: text/html, Size: 1721 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-rust-ts-mode-type-and-module-highlighting-Bug-61.patch --]
[-- Type: text/x-patch; name=0001-Fix-rust-ts-mode-type-and-module-highlighting-Bug-61.patch, Size: 6200 bytes --]

From 5c0204dd3e358187b2da47ca732d6f131ce15cc5 Mon Sep 17 00:00:00 2001
From: Randy Taylor <dev@rjt.dev>
Date: Wed, 8 Feb 2023 21:43:04 -0500
Subject: [PATCH] Fix rust-ts-mode type and module highlighting (Bug#61302)

* lisp/progmodes/rust-ts-mode.el (rust-ts-mode--font-lock-settings):
Specify import queries to avoid clashing with scoped identifiers in
the code.  Highlight attributes with font-lock-preprocessor-face.
---
 lisp/progmodes/rust-ts-mode.el | 88 +++++++++++++++++++++++++++-------
 1 file changed, 72 insertions(+), 16 deletions(-)

diff --git a/lisp/progmodes/rust-ts-mode.el b/lisp/progmodes/rust-ts-mode.el
index 5c71a8ad461..e74e6f6d89e 100644
--- a/lisp/progmodes/rust-ts-mode.el
+++ b/lisp/progmodes/rust-ts-mode.el
@@ -124,8 +124,8 @@ rust-ts-mode--font-lock-settings
   (treesit-font-lock-rules
    :language 'rust
    :feature 'attribute
-   '((attribute_item) @font-lock-constant-face
-     (inner_attribute_item) @font-lock-constant-face)
+   '((attribute_item) @font-lock-preprocessor-face
+     (inner_attribute_item) @font-lock-preprocessor-face)
 
    :language 'rust
    :feature 'bracket
@@ -147,12 +147,6 @@ rust-ts-mode--font-lock-settings
    :feature 'comment
    '(([(block_comment) (line_comment)]) @font-lock-comment-face)
 
-   :language 'rust
-   :feature 'constant
-   `((boolean_literal) @font-lock-constant-face
-     ((identifier) @font-lock-constant-face
-      (:match "^[A-Z][A-Z\\d_]*$" @font-lock-constant-face)))
-
    :language 'rust
    :feature 'delimiter
    '((["," "." ";" ":" "::"]) @font-lock-delimiter-face)
@@ -206,7 +200,54 @@ rust-ts-mode--font-lock-settings
 
    :language 'rust
    :feature 'type
-   `((enum_variant name: (identifier) @font-lock-type-face)
+   `((scoped_use_list path: (identifier) @font-lock-constant-face)
+     (scoped_use_list path: (scoped_identifier
+                             name: (identifier) @font-lock-constant-face))
+
+     ((use_as_clause alias: (identifier) @font-lock-type-face)
+      (:match "^[A-Z]" @font-lock-type-face))
+     ((use_as_clause path: (identifier) @font-lock-type-face)
+      (:match "^[A-Z]" @font-lock-type-face))
+     ((use_as_clause path:
+                     (scoped_identifier path: (_)
+                                        name: (identifier) @font-lock-type-face))
+      (:match "^[A-Z]" @font-lock-type-face))
+     (use_as_clause path: (scoped_identifier name: (identifier) @default))
+
+     ((use_declaration
+       argument: (scoped_identifier
+                  path: (_) @font-lock-constant-face
+                  name: (identifier) @font-lock-type-face))
+      (:match "^[A-Z]" @font-lock-type-face))
+     (use_declaration
+      argument: (scoped_identifier
+                 name: (identifier) @default))
+
+     (use_declaration
+      argument: (scoped_identifier
+                 path: (scoped_identifier
+                        path: (_) @font-lock-constant-face
+                        name: (identifier) @font-lock-constant-face)
+                 name: (identifier) @default))
+
+     (use_declaration
+      argument: (scoped_use_list
+                 path: (scoped_identifier
+                        path: (_) @font-lock-constant-face
+                        name: (identifier) @font-lock-constant-face)))
+
+     ((use_list (identifier) @font-lock-type-face)
+      (:match "^[A-Z]" @font-lock-type-face))
+     (use_list (identifier) @default)
+     ((use_list (scoped_identifier path: (_)
+                                   name: (identifier) @font-lock-type-face))
+      (:match "^[A-Z]" @font-lock-type-face))
+     (use_list (scoped_identifier path: (_)
+                                  name: (identifier) @default))
+     (use_wildcard (scoped_identifier
+                    name: (identifier) @font-lock-constant-face))
+
+     (enum_variant name: (identifier) @font-lock-type-face)
      (match_arm
       pattern: (match_pattern (_ type: (identifier) @font-lock-type-face)))
      (match_arm
@@ -220,25 +261,40 @@ rust-ts-mode--font-lock-settings
       (:match "^[A-Z]" @font-lock-type-face))
      ((scoped_identifier path: (identifier) @font-lock-type-face)
       (:match "^[A-Z]" @font-lock-type-face))
-     ((scoped_identifier
-        (scoped_identifier
-         path: (identifier) @font-lock-type-face))
-      (:match "^[A-Z]" @font-lock-type-face))
      ((scoped_identifier
        path: [(identifier) @font-lock-type-face
               (scoped_identifier
                name: (identifier) @font-lock-type-face)])
       (:match "^[A-Z]" @font-lock-type-face))
-     (scoped_type_identifier path: (identifier) @font-lock-type-face)
+     ((scoped_identifier path: (identifier) @font-lock-type-face)
+      (:match
+       "^\\(u8\\|u16\\|u32\\|u64\\|u128\\|usize\\|i8\\|i16\\|i32\\|i64\\|i128\\|isize\\|char\\|str\\)$"
+       @font-lock-type-face))
+     (scoped_identifier path: (_) @font-lock-constant-face
+                        name: (identifier) @font-lock-type-face)
+     (scoped_identifier path: (scoped_identifier
+                               name: (identifier) @font-lock-constant-face))
+     (scoped_type_identifier path: (_) @font-lock-constant-face)
+     (scoped_type_identifier
+      path: (scoped_identifier
+             path: (_) @font-lock-constant-face
+             name: (identifier) @font-lock-constant-face))
      (type_identifier) @font-lock-type-face
-     (use_as_clause alias: (identifier) @font-lock-type-face)
-     (use_list (identifier) @font-lock-type-face))
+     ;; Ensure function calls aren't highlighted as types.
+     (call_expression function: (scoped_identifier name: (identifier) @default)))
 
    :language 'rust
    :feature 'property
    '((field_identifier) @font-lock-property-face
      (shorthand_field_initializer (identifier) @font-lock-property-face))
 
+   ;; Must be under type, otherwise some imports can be highlighted as consants.
+   :language 'rust
+   :feature 'constant
+   `((boolean_literal) @font-lock-constant-face
+     ((identifier) @font-lock-constant-face
+      (:match "^[A-Z][A-Z\\d_]*$" @font-lock-constant-face)))
+
    :language 'rust
    :feature 'variable
    '((identifier) @font-lock-variable-name-face
-- 
2.39.1

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Jostein Kjønigsen]

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

On 2/14/23 13:39, Randy Taylor wrote:
> Thanks for testing. I've attached a patch fixing this.
>
> I had deleted a query I thought was now covered :). I look forward to 
> the day we have highlight tests!

Tested. Can confirm this looks correct now!

--
Jostein

[-- Attachment #2: Type: text/html, Size: 1135 bytes --]

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Dmitry Gutov]

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

On 14/02/2023 14:39, Randy Taylor wrote:
> On Tuesday, February 14th, 2023 at 06:42, Jostein Kjønigsen 
> <jostein@secure.kjonigsen.net> wrote:
>>Haven  given the latest patch a try on a less macro-ridden codebase I have to 
> say it looks a lot better. In general I would say for this other 
> code-base it looks favourable to rust-mode.
>>
>>If  there's one thing which still seems to be done better in rust-mode 
> (not nitpicking the variable highlighting), it would be fully namespaces 
> function calls within classes.
>>
>>Consider the following code:
>>
>>extern crate claxon;
>>let result = claxon::FlacReader::open(path);
>>
>>While  rust-ts-mode correctly identifies that open() is a function-call, 
> rust-mode correctly identified FlacReader as a class.
> 
> Thanks for testing. I've attached a patch fixing this.
> 
> I had deleted a query I thought was now covered :). I look forward to 
> the day we have highlight tests!

Thank you, looks almost perfect (see below), I've pushed that to emacs-29.

Highlighting tests are a pain to write, but the initiative is always 
welcome. ;-) In the meantime, at least tree-sitter validates the 
queries, which lowers the odds of typos in font-lock rules.

Speaking of a problem, the solution with applying the explicit 'default' 
face doesn't seem ideal. Aside from the redundancy in the resulting 
buffer structure (with very little practical downside), it also makes 
the 'vc-diff' buffers look like this on the attached screenshots (the 
'default' face's background overrides the greens and reds).

I couldn't find a quick solution to this problem, so I pushed the 
existing code for now.

[-- Attachment #2: Screenshot from 2023-02-14 23-42-25.png --]
[-- Type: image/png, Size: 74181 bytes --]

[-- Attachment #3: Screenshot from 2023-02-14 23-42-40.png --]
[-- Type: image/png, Size: 76353 bytes --]

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Randy Taylor]

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

On Tuesday, February 14th, 2023 at 17:14, Dmitry Gutov <dgutov@yandex.ru> wrote:
> On 14/02/2023 14:39, Randy Taylor wrote:
> 
> > On Tuesday, February 14th, 2023 at 06:42, Jostein Kjønigsen
> > jostein@secure.kjonigsen.net wrote:
> > 
> > > Haven given the latest patch a try on a less macro-ridden codebase I have to
> > > say it looks a lot better. In general I would say for this other
> > > code-base it looks favourable to rust-mode.
> > > 
> > > If there's one thing which still seems to be done better in rust-mode
> > > (not nitpicking the variable highlighting), it would be fully namespaces
> > > function calls within classes.
> > > 
> > > Consider the following code:
> > > 
> > > extern crate claxon;
> > > let result = claxon::FlacReader::open(path);
> > > 
> > > While rust-ts-mode correctly identifies that open() is a function-call,
> > > rust-mode correctly identified FlacReader as a class.
> > 
> > Thanks for testing. I've attached a patch fixing this.
> > 
> > I had deleted a query I thought was now covered :). I look forward to
> > the day we have highlight tests!
> 
> 
> Thank you, looks almost perfect (see below), I've pushed that to emacs-29.

Thanks!

> 
> Highlighting tests are a pain to write, but the initiative is always
> welcome. ;-) In the meantime, at least tree-sitter validates the
> queries, which lowers the odds of typos in font-lock rules.
> 
> Speaking of a problem, the solution with applying the explicit 'default'
> face doesn't seem ideal. Aside from the redundancy in the resulting
> buffer structure (with very little practical downside), it also makes
> the 'vc-diff' buffers look like this on the attached screenshots (the
> 'default' face's background overrides the greens and reds).
> 
> I couldn't find a quick solution to this problem, so I pushed the
> existing code for now.

The following patch fixes it for me, but it's a hack since the face doesn't exist (and I don't think the override option matters for the current patch either, I only realized these things as I was about the send the email).

I tried using 'default with all the different override options and nothing worked. Yuan, any ideas?

Basically what we have here are general scoped_identifier queries that we want to apply only to the actual code (i.e. not imports). However, these scoped_identifiers also exist in the imports and the same highlighting semantics don't apply there, so we have specific queries for the different kinds of imports that specify how they want their scoped_identifier stuff highlighted. For some of those specific queries, we want to leave the face as it and make sure nothing else gets applied to it (i.e. default is what we want). Apparently that causes issues with vc-diff buffers, and maybe other things we have yet to discover.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-rust-ts-mode-highlights-overriding-vc-diff-buffe.patch --]
[-- Type: text/x-patch; name=0001-Fix-rust-ts-mode-highlights-overriding-vc-diff-buffe.patch, Size: 3871 bytes --]

From bd405ab9dc1b02d76f56a94b984b7bea9e597bc2 Mon Sep 17 00:00:00 2001
From: Randy Taylor <dev@rjt.dev>
Date: Tue, 14 Feb 2023 20:40:05 -0500
Subject: [PATCH] Fix rust-ts-mode highlights overriding vc-diff buffers
 (Bug#61302)

* lisp/progmodes/rust-ts-mode.el (rust-ts-mode--fontify-default): New
function.
(rust-ts-mode--font-lock-settings): Use it.
---
 lisp/progmodes/rust-ts-mode.el | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/lisp/progmodes/rust-ts-mode.el b/lisp/progmodes/rust-ts-mode.el
index f510b7bb6c9..0b2619c51ad 100644
--- a/lisp/progmodes/rust-ts-mode.el
+++ b/lisp/progmodes/rust-ts-mode.el
@@ -217,7 +217,8 @@ rust-ts-mode--font-lock-settings
                      (scoped_identifier path: (_)
                                         name: (identifier) @font-lock-type-face))
       (:match "^[A-Z]" @font-lock-type-face))
-     (use_as_clause path: (scoped_identifier name: (identifier) @default))
+     (use_as_clause
+      path: (scoped_identifier name: (identifier) @rust-ts-mode--fontify-default))
 
      ((use_declaration
        argument: (scoped_identifier
@@ -226,14 +227,14 @@ rust-ts-mode--font-lock-settings
       (:match "^[A-Z]" @font-lock-type-face))
      (use_declaration
       argument: (scoped_identifier
-                 name: (identifier) @default))
+                 name: (identifier) @rust-ts-mode--fontify-default))
 
      (use_declaration
       argument: (scoped_identifier
                  path: (scoped_identifier
                         path: (_) @font-lock-constant-face
                         name: (identifier) @font-lock-constant-face)
-                 name: (identifier) @default))
+                 name: (identifier) @rust-ts-mode--fontify-default))
 
      (use_declaration
       argument: (scoped_use_list
@@ -243,12 +244,12 @@ rust-ts-mode--font-lock-settings
 
      ((use_list (identifier) @font-lock-type-face)
       (:match "^[A-Z]" @font-lock-type-face))
-     (use_list (identifier) @default)
+     (use_list (identifier) @rust-ts-mode--fontify-default)
      ((use_list (scoped_identifier path: (_)
                                    name: (identifier) @font-lock-type-face))
       (:match "^[A-Z]" @font-lock-type-face))
      (use_list (scoped_identifier path: (_)
-                                  name: (identifier) @default))
+                                  name: (identifier) @rust-ts-mode--fontify-default))
      (use_wildcard (scoped_identifier
                     name: (identifier) @font-lock-constant-face))
 
@@ -286,7 +287,9 @@ rust-ts-mode--font-lock-settings
              name: (identifier) @font-lock-constant-face))
      (type_identifier) @font-lock-type-face
      ;; Ensure function calls aren't highlighted as types.
-     (call_expression function: (scoped_identifier name: (identifier) @default)))
+     (call_expression function:
+                      (scoped_identifier
+                       name: (identifier) @rust-ts-mode--fontify-default)))
 
    :language 'rust
    :feature 'property
@@ -304,7 +307,7 @@ rust-ts-mode--font-lock-settings
    :feature 'variable
    '((identifier) @font-lock-variable-name-face
      ;; Everything in a token_tree is an identifier.
-     (token_tree (identifier) @default))
+     (token_tree (identifier) @rust-ts-mode--fontify-default))
 
    :language 'rust
    :feature 'escape-sequence
@@ -317,6 +320,12 @@ rust-ts-mode--font-lock-settings
    '((ERROR) @font-lock-warning-face))
   "Tree-sitter font-lock settings for `rust-ts-mode'.")
 
+(defun rust-ts-mode--fontify-default (node _ start end &rest _)
+  "Fontify the node with the default face."
+  (treesit-fontify-with-override
+   (treesit-node-start node) (treesit-node-end node)
+   'default-face 'append start end))
+
 (defalias 'rust-ts-mode--fontify-pattern
   (and
    (treesit-available-p)
-- 
2.39.2

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Dmitry Gutov]

On 15/02/2023 04:07, Randy Taylor wrote:
> Basically what we have here are general scoped_identifier queries that we want to apply only to the actual code (i.e. not imports). However, these scoped_identifiers also exist in the imports and the same highlighting semantics don't apply there, so we have specific queries for the different kinds of imports that specify how they want their scoped_identifier stuff highlighted.

The only option I've been thinking of (and described in some related 
report), is to replace the face name with a highlighter in Lisp.

See rust-ts-mode--fontify-pattern as an example of such function.

But the new one will need to check that the parent is 
'scoped_identifier', and the grandparent is not a 'call_expression' 
node, or 'use_as_clause', or 'use_declaration', etc, and the name itself 
is lowercase -- when so, skip highlighting. And highlight with one of 
the two faces when otherwise.

Shouldn't be too hard to do, but I'm wary about the additional cost at 
runtime.

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Dmitry Gutov]

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

On 16/02/2023 03:53, Dmitry Gutov wrote:
> But the new one will need to check that the parent is 
> 'scoped_identifier', and the grandparent is not a 'call_expression' 
> node, or 'use_as_clause', or 'use_declaration', etc, and the name itself 
> is lowercase -- when so, skip highlighting. And highlight with one of 
> the two faces when otherwise.
> 
> Shouldn't be too hard to do, but I'm wary about the additional cost at 
> runtime.

So, this seems to work.

At the cost of some performance overhead due to :pred in the 'variable' 
query (the rest of the changes don't seem to affect the runtime -- guess 
the Lisp calls were balanced out by fewer queries).

[-- Attachment #2: rust-ts-fontify-scope.diff --]
[-- Type: text/x-patch, Size: 6377 bytes --]

diff --git a/lisp/progmodes/rust-ts-mode.el b/lisp/progmodes/rust-ts-mode.el
index ec823d09d8c..d6b28f43182 100644
--- a/lisp/progmodes/rust-ts-mode.el
+++ b/lisp/progmodes/rust-ts-mode.el
@@ -208,50 +208,11 @@ rust-ts-mode--font-lock-settings
    `((scoped_use_list path: (identifier) @font-lock-constant-face)
      (scoped_use_list path: (scoped_identifier
                              name: (identifier) @font-lock-constant-face))
-
-     ((use_as_clause alias: (identifier) @font-lock-type-face)
-      (:match "^[A-Z]" @font-lock-type-face))
-     ((use_as_clause path: (identifier) @font-lock-type-face)
-      (:match "^[A-Z]" @font-lock-type-face))
-     ((use_as_clause path:
-                     (scoped_identifier path: (_)
-                                        name: (identifier) @font-lock-type-face))
-      (:match "^[A-Z]" @font-lock-type-face))
-     (use_as_clause path: (scoped_identifier name: (identifier) @default))
-
-     ((use_declaration
-       argument: (scoped_identifier
-                  path: (_) @font-lock-constant-face
-                  name: (identifier) @font-lock-type-face))
-      (:match "^[A-Z]" @font-lock-type-face))
-     (use_declaration
-      argument: (scoped_identifier
-                 name: (identifier) @default))
-
-     (use_declaration
-      argument: (scoped_identifier
-                 path: (scoped_identifier
-                        path: (_) @font-lock-constant-face
-                        name: (identifier) @font-lock-constant-face)
-                 name: (identifier) @default))
-
-     (use_declaration
-      argument: (scoped_use_list
-                 path: (scoped_identifier
-                        path: (_) @font-lock-constant-face
-                        name: (identifier) @font-lock-constant-face)))
-
      ((use_list (identifier) @font-lock-type-face)
       (:match "^[A-Z]" @font-lock-type-face))
-     (use_list (identifier) @default)
-     ((use_list (scoped_identifier path: (_)
-                                   name: (identifier) @font-lock-type-face))
-      (:match "^[A-Z]" @font-lock-type-face))
-     (use_list (scoped_identifier path: (_)
-                                  name: (identifier) @default))
-     (use_wildcard (scoped_identifier
-                    name: (identifier) @font-lock-constant-face))
-
+     (use_wildcard [(identifier) @rust-ts-mode--fontify-scope
+                    (scoped_identifier
+                     name: (identifier) @rust-ts-mode--fontify-scope)])
      (enum_variant name: (identifier) @font-lock-type-face)
      (match_arm
       pattern: (match_pattern (_ type: (identifier) @font-lock-type-face)))
@@ -262,31 +223,13 @@ rust-ts-mode--font-lock-settings
      (mod_item name: (identifier) @font-lock-constant-face)
      (primitive_type) @font-lock-type-face
      (type_identifier) @font-lock-type-face
-     ((scoped_identifier name: (identifier) @font-lock-type-face)
-      (:match "^[A-Z]" @font-lock-type-face))
-     ((scoped_identifier path: (identifier) @font-lock-type-face)
-      (:match "^[A-Z]" @font-lock-type-face))
-     ((scoped_identifier
-       path: [(identifier) @font-lock-type-face
-              (scoped_identifier
-               name: (identifier) @font-lock-type-face)])
-      (:match "^[A-Z]" @font-lock-type-face))
+     ((scoped_identifier name: (identifier) @rust-ts-mode--fontify-tail))
      ((scoped_identifier path: (identifier) @font-lock-type-face)
       (:match
        "^\\(u8\\|u16\\|u32\\|u64\\|u128\\|usize\\|i8\\|i16\\|i32\\|i64\\|i128\\|isize\\|char\\|str\\)$"
        @font-lock-type-face))
-     (scoped_identifier path: (_) @font-lock-constant-face
-                        name: (identifier) @font-lock-type-face)
-     (scoped_identifier path: (scoped_identifier
-                               name: (identifier) @font-lock-constant-face))
-     (scoped_type_identifier path: (_) @font-lock-constant-face)
-     (scoped_type_identifier
-      path: (scoped_identifier
-             path: (_) @font-lock-constant-face
-             name: (identifier) @font-lock-constant-face))
-     (type_identifier) @font-lock-type-face
-     ;; Ensure function calls aren't highlighted as types.
-     (call_expression function: (scoped_identifier name: (identifier) @default)))
+     ((scoped_identifier path: (identifier) @rust-ts-mode--fontify-scope))
+     (type_identifier) @font-lock-type-face)
 
    :language 'rust
    :feature 'property
@@ -302,9 +245,8 @@ rust-ts-mode--font-lock-settings
 
    :language 'rust
    :feature 'variable
-   '((identifier) @font-lock-variable-name-face
-     ;; Everything in a token_tree is an identifier.
-     (token_tree (identifier) @default))
+   '(((identifier) @font-lock-variable-name-face
+      (:pred rust-ts-mode--variable-p @font-lock-variable-name-face)))
 
    :language 'rust
    :feature 'escape-sequence
@@ -317,6 +259,41 @@ rust-ts-mode--font-lock-settings
    '((ERROR) @font-lock-warning-face))
   "Tree-sitter font-lock settings for `rust-ts-mode'.")
 
+(defun rust-ts-mode--fontify-scope (node override start end &optional tail-p)
+  (let* ((case-fold-search nil)
+         (face
+          (cond
+           ((string-match-p "^[A-Z]" (treesit-node-text node))
+            'font-lock-type-face)
+           ((and tail-p
+                 (not (rust-ts-mode--tail-fontify-p (treesit-node-parent node))))
+            nil)
+           (t 'font-lock-constant-face))))
+    (and face
+         (treesit-fontify-with-override
+          (treesit-node-start node) (treesit-node-end node)
+          face
+          override start end))))
+
+(defun rust-ts-mode--fontify-tail (node override start end)
+  (rust-ts-mode--fontify-scope node override start end t))
+
+(defun rust-ts-mode--tail-fontify-p (node)
+  (not
+   (string-match-p "\\`\\(?:use_list\\|call_expression\\|use_as_clause\\|use_declaration\\)\\'"
+                   (treesit-node-type (treesit-node-parent node)))))
+
+(defun rust-ts-mode--variable-p (node)
+  (let* ((parent (treesit-node-parent node))
+         (parent-type (treesit-node-type parent)))
+    (cond
+     ;; Everything in a token_tree is an identifier.
+     ((equal "token_tree" parent-type)
+      nil)
+     ((equal "scoped_identifier" parent-type)
+      (rust-ts-mode--tail-fontify-p parent))
+     (t t))))
+
 (defalias 'rust-ts-mode--fontify-pattern
   (and
    (treesit-available-p)

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Randy Taylor]

On Friday, February 17th, 2023 at 22:27, Dmitry Gutov <dgutov@yandex.ru> wrote:
>On 16/02/2023 03:53, Dmitry Gutov wrote:
>> But the new one will need to check that the parent is
>> 'scoped_identifier', and the grandparent is not a 'call_expression'
>> node, or 'use_as_clause', or 'use_declaration', etc, and the name itself
>> is lowercase -- when so, skip highlighting. And highlight with one of
>> the two faces when otherwise.
>>
>> Shouldn't be too hard to do, but I'm wary about the additional cost at
>> runtime.
>
>So, this seems to work.
>
>At the cost of some performance overhead due to :pred in the 'variable'
>query (the rest of the changes don't seem to affect the runtime -- guess
>the Lisp calls were balanced out by fewer queries).

Thanks, I think this is actually a lot cleaner than the gazillion queries we had.
It looks good barring a few issues I've noticed.

use a::b::{self as ab, A as abc};

A should be highlighted as a type.
If abc is Abc, Abc should be highlighted as a type.

use std::Fs as Self_fs;

Self_fs should be highlighted as a type.

I only quickly tested, but re-adding these queries:
```
     ((use_as_clause alias: (identifier) @font-lock-type-face)
      (:match "^[A-Z]" @font-lock-type-face))
     ((use_as_clause path: (identifier) @font-lock-type-face)
      (:match "^[A-Z]" @font-lock-type-face))
```

fixes it. Otherwise, I haven't noticed anything else amiss for the types feature.

The variable feature is highlighting some things incorrectly (it was before too, but I think it's a little worse now).
Adding these to rust-ts-mode--variable-p takes care of the issues that I see.
```
     ((equal "extern_crate_declaration" parent-type)
      nil)
     ((equal "lifetime" parent-type)
      nil)
     ((equal "scoped_type_identifier" parent-type)
      nil)
     ((equal "use_as_clause" parent-type)
      nil)
     ((equal "use_list" parent-type)
      nil)
```

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Dmitry Gutov]

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

On 18/02/2023 22:42, Randy Taylor wrote:
> Thanks, I think this is actually a lot cleaner than the gazillion queries we had.
> It looks good barring a few issues I've noticed.
> 
> use a::b::{self as ab, A as abc};
> 
> A should be highlighted as a type.
> If abc is Abc, Abc should be highlighted as a type.
> 
> use std::Fs as Self_fs;
> 
> Self_fs should be highlighted as a type.
> 
> I only quickly tested, but re-adding these queries:
> ```
>       ((use_as_clause alias: (identifier) @font-lock-type-face)
>        (:match "^[A-Z]" @font-lock-type-face))
>       ((use_as_clause path: (identifier) @font-lock-type-face)
>        (:match "^[A-Z]" @font-lock-type-face))
> ```
> 
> fixes it. Otherwise, I haven't noticed anything else amiss for the types feature.

Thanks for testing. See the revised patch.

> The variable feature is highlighting some things incorrectly (it was before too, but I think it's a little worse now).
> Adding these to rust-ts-mode--variable-p takes care of the issues that I see.
> ```
>       ((equal "extern_crate_declaration" parent-type)
>        nil)
>       ((equal "lifetime" parent-type)
>        nil)
>       ((equal "scoped_type_identifier" parent-type)
>        nil)
>       ((equal "use_as_clause" parent-type)
>        nil)
>       ((equal "use_list" parent-type)
>        nil)
> ```

This should be taken care of by the new approach (enumeration of all 
allowed parent types).

[-- Attachment #2: rust-ts-fontify-scope-v3.diff --]
[-- Type: text/x-patch, Size: 7012 bytes --]

diff --git a/lisp/progmodes/rust-ts-mode.el b/lisp/progmodes/rust-ts-mode.el
index ec823d09d8c..928e3038c51 100644
--- a/lisp/progmodes/rust-ts-mode.el
+++ b/lisp/progmodes/rust-ts-mode.el
@@ -208,50 +208,15 @@ rust-ts-mode--font-lock-settings
    `((scoped_use_list path: (identifier) @font-lock-constant-face)
      (scoped_use_list path: (scoped_identifier
                              name: (identifier) @font-lock-constant-face))
-
      ((use_as_clause alias: (identifier) @font-lock-type-face)
       (:match "^[A-Z]" @font-lock-type-face))
      ((use_as_clause path: (identifier) @font-lock-type-face)
       (:match "^[A-Z]" @font-lock-type-face))
-     ((use_as_clause path:
-                     (scoped_identifier path: (_)
-                                        name: (identifier) @font-lock-type-face))
-      (:match "^[A-Z]" @font-lock-type-face))
-     (use_as_clause path: (scoped_identifier name: (identifier) @default))
-
-     ((use_declaration
-       argument: (scoped_identifier
-                  path: (_) @font-lock-constant-face
-                  name: (identifier) @font-lock-type-face))
-      (:match "^[A-Z]" @font-lock-type-face))
-     (use_declaration
-      argument: (scoped_identifier
-                 name: (identifier) @default))
-
-     (use_declaration
-      argument: (scoped_identifier
-                 path: (scoped_identifier
-                        path: (_) @font-lock-constant-face
-                        name: (identifier) @font-lock-constant-face)
-                 name: (identifier) @default))
-
-     (use_declaration
-      argument: (scoped_use_list
-                 path: (scoped_identifier
-                        path: (_) @font-lock-constant-face
-                        name: (identifier) @font-lock-constant-face)))
-
      ((use_list (identifier) @font-lock-type-face)
       (:match "^[A-Z]" @font-lock-type-face))
-     (use_list (identifier) @default)
-     ((use_list (scoped_identifier path: (_)
-                                   name: (identifier) @font-lock-type-face))
-      (:match "^[A-Z]" @font-lock-type-face))
-     (use_list (scoped_identifier path: (_)
-                                  name: (identifier) @default))
-     (use_wildcard (scoped_identifier
-                    name: (identifier) @font-lock-constant-face))
-
+     (use_wildcard [(identifier) @rust-ts-mode--fontify-scope
+                    (scoped_identifier
+                     name: (identifier) @rust-ts-mode--fontify-scope)])
      (enum_variant name: (identifier) @font-lock-type-face)
      (match_arm
       pattern: (match_pattern (_ type: (identifier) @font-lock-type-face)))
@@ -262,31 +227,13 @@ rust-ts-mode--font-lock-settings
      (mod_item name: (identifier) @font-lock-constant-face)
      (primitive_type) @font-lock-type-face
      (type_identifier) @font-lock-type-face
-     ((scoped_identifier name: (identifier) @font-lock-type-face)
-      (:match "^[A-Z]" @font-lock-type-face))
-     ((scoped_identifier path: (identifier) @font-lock-type-face)
-      (:match "^[A-Z]" @font-lock-type-face))
-     ((scoped_identifier
-       path: [(identifier) @font-lock-type-face
-              (scoped_identifier
-               name: (identifier) @font-lock-type-face)])
-      (:match "^[A-Z]" @font-lock-type-face))
+     ((scoped_identifier name: (identifier) @rust-ts-mode--fontify-tail))
      ((scoped_identifier path: (identifier) @font-lock-type-face)
       (:match
        "^\\(u8\\|u16\\|u32\\|u64\\|u128\\|usize\\|i8\\|i16\\|i32\\|i64\\|i128\\|isize\\|char\\|str\\)$"
        @font-lock-type-face))
-     (scoped_identifier path: (_) @font-lock-constant-face
-                        name: (identifier) @font-lock-type-face)
-     (scoped_identifier path: (scoped_identifier
-                               name: (identifier) @font-lock-constant-face))
-     (scoped_type_identifier path: (_) @font-lock-constant-face)
-     (scoped_type_identifier
-      path: (scoped_identifier
-             path: (_) @font-lock-constant-face
-             name: (identifier) @font-lock-constant-face))
-     (type_identifier) @font-lock-type-face
-     ;; Ensure function calls aren't highlighted as types.
-     (call_expression function: (scoped_identifier name: (identifier) @default)))
+     ((scoped_identifier path: (identifier) @rust-ts-mode--fontify-scope))
+     (type_identifier) @font-lock-type-face)
 
    :language 'rust
    :feature 'property
@@ -302,9 +249,23 @@ rust-ts-mode--font-lock-settings
 
    :language 'rust
    :feature 'variable
-   '((identifier) @font-lock-variable-name-face
-     ;; Everything in a token_tree is an identifier.
-     (token_tree (identifier) @default))
+   '((compound_assignment_expr right: (identifier) @font-lock-variable-name-face)
+     (assignment_expression right: (identifier) @font-lock-variable-name-face)
+     (let_declaration value: (identifier) @font-lock-variable-name-face)
+     (binary_expression left: (identifier) @font-lock-variable-name-face)
+     (binary_expression right: (identifier) @font-lock-variable-name-face)
+     (if_expression condition: (identifier) @font-lock-variable-name-face)
+     (let_condition value: (identifier) @font-lock-variable-name-face)
+     (while_expression condition: (identifier) @font-lock-variable-name-face)
+     (field_expression value: (identifier) @font-lock-variable-name-face)
+     (reference_expression value: (identifier) @font-lock-variable-name-face)
+     (match_expression value: (identifier) @font-lock-variable-name-face)
+     (match_arm value: (identifier) @font-lock-variable-name-face)
+     (arguments (identifier) @font-lock-variable-name-face)
+     (block (identifier) @font-lock-variable-name-face)
+     (array_expression (identifier) @font-lock-variable-name-face)
+     (tuple_expression (identifier) @font-lock-variable-name-face)
+     (return_expression (identifier) @font-lock-variable-name-face))
 
    :language 'rust
    :feature 'escape-sequence
@@ -317,6 +278,28 @@ rust-ts-mode--font-lock-settings
    '((ERROR) @font-lock-warning-face))
   "Tree-sitter font-lock settings for `rust-ts-mode'.")
 
+(defun rust-ts-mode--fontify-scope (node override start end &optional tail-p)
+  (let* ((case-fold-search nil)
+         (face
+          (cond
+           ((string-match-p "^[A-Z]" (treesit-node-text node))
+            'font-lock-type-face)
+           ((and
+             tail-p
+             (string-match-p
+              "\\`\\(?:use_list\\|call_expression\\|use_as_clause\\|use_declaration\\)\\'"
+              (treesit-node-type (treesit-node-parent node))))
+            nil)
+           (t 'font-lock-constant-face))))
+    (when face
+      (treesit-fontify-with-override
+       (treesit-node-start node) (treesit-node-end node)
+       face
+       override start end))))
+
+(defun rust-ts-mode--fontify-tail (node override start end)
+  (rust-ts-mode--fontify-scope node override start end t))
+
 (defalias 'rust-ts-mode--fontify-pattern
   (and
    (treesit-available-p)

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Randy Taylor]

On Saturday, February 18th, 2023 at 16:45, Dmitry Gutov <dgutov@yandex.ru> wrote:
> On 18/02/2023 22:42, Randy Taylor wrote:
> 
> > Thanks, I think this is actually a lot cleaner than the gazillion queries we had.
> > It looks good barring a few issues I've noticed.
> > 
> > use a::b::{self as ab, A as abc};
> > 
> > A should be highlighted as a type.
> > If abc is Abc, Abc should be highlighted as a type.
> > 
> > use std::Fs as Self_fs;
> > 
> > Self_fs should be highlighted as a type.
> > 
> > I only quickly tested, but re-adding these queries:
> > `((use_as_clause alias: (identifier) @font-lock-type-face) (:match "^[A-Z]" @font-lock-type-face)) ((use_as_clause path: (identifier) @font-lock-type-face) (:match "^[A-Z]" @font-lock-type-face))`
> > 
> > fixes it. Otherwise, I haven't noticed anything else amiss for the types feature.
> 
> 
> Thanks for testing. See the revised patch.

Fix one bug, introduce another ;).

Any use declaration tail (I guess that's the lingo we're using?) should not have a face applied to it if it's lowercase.

For example:
```
use deeply::nested::function as other_function;
```
function should not have any face applied to it.

```
use a::b::{C, d, e::F, g::h::I, g::h::i};
```
i should not have any face applied to it.

Only if they are capitalized should we give them a face: font-lock-type-face.
As it stands now, they are all font-lock-constant-face.

The previous patch was perfect except for the missing use_as_clause queries - if I add those back, I think everything is good unless I'm missing something.

> 
> > The variable feature is highlighting some things incorrectly (it was before too, but I think it's a little worse now).
> > Adding these to rust-ts-mode--variable-p takes care of the issues that I see.
> > `((equal "extern_crate_declaration" parent-type) nil) ((equal "lifetime" parent-type) nil) ((equal "scoped_type_identifier" parent-type) nil) ((equal "use_as_clause" parent-type) nil) ((equal "use_list" parent-type) nil)`
> 
> 
> This should be taken care of by the new approach (enumeration of all
> allowed parent types).

Missing:
     (closure_parameters (identifier) @font-lock-variable-name-face)
     (field_initializer value: (identifier) @font-lock-variable-name-face)

Would it be possible to alphabetize the queries in the variable feature BTW? It makes it easier to see what's there/missing.
And you know I like things alphabetized ;).

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Dmitry Gutov]

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

On 19/02/2023 01:31, Randy Taylor wrote:

> Fix one bug, introduce another 😉.
> 
> Any use declaration tail (I guess that's the lingo we're using?) should not have a face applied to it if it's lowercase.

I guess we do. Unless you have better naming suggestions ;-)

> For example:
> ```
> use deeply::nested::function as other_function;
> ```
> function should not have any face applied to it.
> 
> ```
> use a::b::{C, d, e::F, g::h::I, g::h::i};
> ```
> i should not have any face applied to it.
> 
> Only if they are capitalized should we give them a face: font-lock-type-face.
> As it stands now, they are all font-lock-constant-face.
> 
> The previous patch was perfect except for the missing use_as_clause queries - if I add those back, I think everything is good unless I'm missing something.

Thanks for catching that, I missed one treesit-node-parent call when 
inlining a function.

>>> The variable feature is highlighting some things incorrectly (it was before too, but I think it's a little worse now).
>>> Adding these to rust-ts-mode--variable-p takes care of the issues that I see.
>>> `((equal "extern_crate_declaration" parent-type) nil) ((equal "lifetime" parent-type) nil) ((equal "scoped_type_identifier" parent-type) nil) ((equal "use_as_clause" parent-type) nil) ((equal "use_list" parent-type) nil)`
>>
>> This should be taken care of by the new approach (enumeration of all
>> allowed parent types).
> Missing:
>       (closure_parameters (identifier) @font-lock-variable-name-face)

This one goes into the 'definition' feature. I just made that change 
today in emacs-29, check it out.

>       (field_initializer value: (identifier) @font-lock-variable-name-face)

Thanks, added. Also added unary_expression.

> Would it be possible to alphabetize the queries in the variable feature BTW? It makes it easier to see what's there/missing.
> And you know I like things alphabetized 😉.

No problem! I generally like to group by functionality, but alphabetic 
is fine, and this case seems particularly suited to it.

See the revised in attachment.

[-- Attachment #2: rust-ts-fontify-scope-v4.diff --]
[-- Type: text/x-patch, Size: 7178 bytes --]

diff --git a/lisp/progmodes/rust-ts-mode.el b/lisp/progmodes/rust-ts-mode.el
index e53cdb5ce7d..50df2543da1 100644
--- a/lisp/progmodes/rust-ts-mode.el
+++ b/lisp/progmodes/rust-ts-mode.el
@@ -209,50 +209,15 @@ rust-ts-mode--font-lock-settings
    `((scoped_use_list path: (identifier) @font-lock-constant-face)
      (scoped_use_list path: (scoped_identifier
                              name: (identifier) @font-lock-constant-face))
-
      ((use_as_clause alias: (identifier) @font-lock-type-face)
       (:match "^[A-Z]" @font-lock-type-face))
      ((use_as_clause path: (identifier) @font-lock-type-face)
       (:match "^[A-Z]" @font-lock-type-face))
-     ((use_as_clause path:
-                     (scoped_identifier path: (_)
-                                        name: (identifier) @font-lock-type-face))
-      (:match "^[A-Z]" @font-lock-type-face))
-     (use_as_clause path: (scoped_identifier name: (identifier) @default))
-
-     ((use_declaration
-       argument: (scoped_identifier
-                  path: (_) @font-lock-constant-face
-                  name: (identifier) @font-lock-type-face))
-      (:match "^[A-Z]" @font-lock-type-face))
-     (use_declaration
-      argument: (scoped_identifier
-                 name: (identifier) @default))
-
-     (use_declaration
-      argument: (scoped_identifier
-                 path: (scoped_identifier
-                        path: (_) @font-lock-constant-face
-                        name: (identifier) @font-lock-constant-face)
-                 name: (identifier) @default))
-
-     (use_declaration
-      argument: (scoped_use_list
-                 path: (scoped_identifier
-                        path: (_) @font-lock-constant-face
-                        name: (identifier) @font-lock-constant-face)))
-
      ((use_list (identifier) @font-lock-type-face)
       (:match "^[A-Z]" @font-lock-type-face))
-     (use_list (identifier) @default)
-     ((use_list (scoped_identifier path: (_)
-                                   name: (identifier) @font-lock-type-face))
-      (:match "^[A-Z]" @font-lock-type-face))
-     (use_list (scoped_identifier path: (_)
-                                  name: (identifier) @default))
-     (use_wildcard (scoped_identifier
-                    name: (identifier) @font-lock-constant-face))
-
+     (use_wildcard [(identifier) @rust-ts-mode--fontify-scope
+                    (scoped_identifier
+                     name: (identifier) @rust-ts-mode--fontify-scope)])
      (enum_variant name: (identifier) @font-lock-type-face)
      (match_arm
       pattern: (match_pattern (_ type: (identifier) @font-lock-type-face)))
@@ -263,31 +228,13 @@ rust-ts-mode--font-lock-settings
      (mod_item name: (identifier) @font-lock-constant-face)
      (primitive_type) @font-lock-type-face
      (type_identifier) @font-lock-type-face
-     ((scoped_identifier name: (identifier) @font-lock-type-face)
-      (:match "^[A-Z]" @font-lock-type-face))
-     ((scoped_identifier path: (identifier) @font-lock-type-face)
-      (:match "^[A-Z]" @font-lock-type-face))
-     ((scoped_identifier
-       path: [(identifier) @font-lock-type-face
-              (scoped_identifier
-               name: (identifier) @font-lock-type-face)])
-      (:match "^[A-Z]" @font-lock-type-face))
+     ((scoped_identifier name: (identifier) @rust-ts-mode--fontify-tail))
      ((scoped_identifier path: (identifier) @font-lock-type-face)
       (:match
        "^\\(u8\\|u16\\|u32\\|u64\\|u128\\|usize\\|i8\\|i16\\|i32\\|i64\\|i128\\|isize\\|char\\|str\\)$"
        @font-lock-type-face))
-     (scoped_identifier path: (_) @font-lock-constant-face
-                        name: (identifier) @font-lock-type-face)
-     (scoped_identifier path: (scoped_identifier
-                               name: (identifier) @font-lock-constant-face))
-     (scoped_type_identifier path: (_) @font-lock-constant-face)
-     (scoped_type_identifier
-      path: (scoped_identifier
-             path: (_) @font-lock-constant-face
-             name: (identifier) @font-lock-constant-face))
-     (type_identifier) @font-lock-type-face
-     ;; Ensure function calls aren't highlighted as types.
-     (call_expression function: (scoped_identifier name: (identifier) @default)))
+     ((scoped_identifier path: (identifier) @rust-ts-mode--fontify-scope))
+     (type_identifier) @font-lock-type-face)
 
    :language 'rust
    :feature 'property
@@ -303,9 +250,25 @@ rust-ts-mode--font-lock-settings
 
    :language 'rust
    :feature 'variable
-   '((identifier) @font-lock-variable-name-face
-     ;; Everything in a token_tree is an identifier.
-     (token_tree (identifier) @default))
+   '((assignment_expression right: (identifier) @font-lock-variable-name-face)
+     (arguments (identifier) @font-lock-variable-name-face)
+     (array_expression (identifier) @font-lock-variable-name-face)
+     (binary_expression left: (identifier) @font-lock-variable-name-face)
+     (binary_expression right: (identifier) @font-lock-variable-name-face)
+     (block (identifier) @font-lock-variable-name-face)
+     (compound_assignment_expr right: (identifier) @font-lock-variable-name-face)
+     (field_expression value: (identifier) @font-lock-variable-name-face)
+     (field_initializer value: (identifier) @font-lock-variable-name-face)
+     (if_expression condition: (identifier) @font-lock-variable-name-face)
+     (let_condition value: (identifier) @font-lock-variable-name-face)
+     (let_declaration value: (identifier) @font-lock-variable-name-face)
+     (match_arm value: (identifier) @font-lock-variable-name-face)
+     (match_expression value: (identifier) @font-lock-variable-name-face)
+     (reference_expression value: (identifier) @font-lock-variable-name-face)
+     (return_expression (identifier) @font-lock-variable-name-face)
+     (tuple_expression (identifier) @font-lock-variable-name-face)
+     (unary_expression (identifier) @font-lock-variable-name-face)
+     (while_expression condition: (identifier) @font-lock-variable-name-face))
 
    :language 'rust
    :feature 'escape-sequence
@@ -318,6 +281,28 @@ rust-ts-mode--font-lock-settings
    '((ERROR) @font-lock-warning-face))
   "Tree-sitter font-lock settings for `rust-ts-mode'.")
 
+(defun rust-ts-mode--fontify-scope (node override start end &optional tail-p)
+  (let* ((case-fold-search nil)
+         (face
+          (cond
+           ((string-match-p "^[A-Z]" (treesit-node-text node))
+            'font-lock-type-face)
+           ((and
+             tail-p
+             (string-match-p
+              "\\`\\(?:use_list\\|call_expression\\|use_as_clause\\|use_declaration\\)\\'"
+              (treesit-node-type (treesit-node-parent (treesit-node-parent node)))))
+            nil)
+           (t 'font-lock-constant-face))))
+    (when face
+      (treesit-fontify-with-override
+       (treesit-node-start node) (treesit-node-end node)
+       face
+       override start end))))
+
+(defun rust-ts-mode--fontify-tail (node override start end)
+  (rust-ts-mode--fontify-scope node override start end t))
+
 (defalias 'rust-ts-mode--fontify-pattern
   (and
    (treesit-available-p)

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Randy Taylor]

On Saturday, February 18th, 2023 at 19:13, Dmitry Gutov <dgutov@yandex.ru> wrote:
> On 19/02/2023 01:31, Randy Taylor wrote:
> 
> > Fix one bug, introduce another 😉.
> > 
> > Any use declaration tail (I guess that's the lingo we're using?) should not have a face applied to it if it's lowercase.
> 
> 
> I guess we do. Unless you have better naming suggestions ;-)

Nope, I like it :).

> 
> > For example:
> > `use deeply::nested::function as other_function;`
> > function should not have any face applied to it.
> > 
> > `use a::b::{C, d, e::F, g::h::I, g::h::i};`
> > i should not have any face applied to it.
> > 
> > Only if they are capitalized should we give them a face: font-lock-type-face.
> > As it stands now, they are all font-lock-constant-face.
> > 
> > The previous patch was perfect except for the missing use_as_clause queries - if I add those back, I think everything is good unless I'm missing something.
> 
> 
> Thanks for catching that, I missed one treesit-node-parent call when
> inlining a function.

Looks good now!

> 
> > > > The variable feature is highlighting some things incorrectly (it was before too, but I think it's a little worse now).
> > > > Adding these to rust-ts-mode--variable-p takes care of the issues that I see.
> > > > `((equal "extern_crate_declaration" parent-type) nil) ((equal "lifetime" parent-type) nil) ((equal "scoped_type_identifier" parent-type) nil) ((equal "use_as_clause" parent-type) nil) ((equal "use_list" parent-type) nil)`
> > > 
> > > This should be taken care of by the new approach (enumeration of all
> > > allowed parent types).
> > > Missing:
> > > (closure_parameters (identifier) @font-lock-variable-name-face)
> 
> 
> This one goes into the 'definition' feature. I just made that change
> today in emacs-29, check it out.
> 
> > (field_initializer value: (identifier) @font-lock-variable-name-face)
> 
> 
> Thanks, added. Also added unary_expression.
> 
> > Would it be possible to alphabetize the queries in the variable feature BTW? It makes it easier to see what's there/missing.
> > And you know I like things alphabetized 😉.
> 
> 
> No problem! I generally like to group by functionality, but alphabetic
> is fine, and this case seems particularly suited to it.
> 
> See the revised in attachment.

Ship it!

Thanks for working on this.

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Dmitry Gutov]

Version: 29.1

On 19/02/2023 02:50, Randy Taylor wrote:
>> See the revised in attachment.
> Ship it!
> 
> Thanks for working on this.

And done! Thank you as well, and closing.

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Dmitry Gutov]

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

On 18/02/2023 05:27, Dmitry Gutov wrote:
> On 16/02/2023 03:53, Dmitry Gutov wrote:
>> But the new one will need to check that the parent is 
>> 'scoped_identifier', and the grandparent is not a 'call_expression' 
>> node, or 'use_as_clause', or 'use_declaration', etc, and the name 
>> itself is lowercase -- when so, skip highlighting. And highlight with 
>> one of the two faces when otherwise.
>>
>> Shouldn't be too hard to do, but I'm wary about the additional cost at 
>> runtime.
> 
> So, this seems to work.
> 
> At the cost of some performance overhead due to :pred in the 'variable' 
> query (the rest of the changes don't seem to affect the runtime -- guess 
> the Lisp calls were balanced out by fewer queries).

It seems the check could be further simplified (no variable can be part 
of a scoping expression, I believe):

   (defun rust-ts-mode--variable-p (node)
     (let* ((parent (treesit-node-parent node))
            (parent-type (treesit-node-type parent)))
       ;; Everything in a token_tree is an identifier.
       (not (string-match-p "token_tree\\|scoped_identifier"
                           parent-type))))

But that does not improve performance.

I also tried to create a query with negation, but it seems you can't do 
that for parent type.

What works, and removes the performance drop, is enumerating all 
possible parent types which can contain an identifier to be highlighted 
as a variable. It's a moderately large list, see if I maybe missed some.

Randy, Jostein, feedback welcome.

[-- Attachment #2: rust-ts-fontify-scope-v2.diff --]
[-- Type: text/x-patch, Size: 7012 bytes --]

diff --git a/lisp/progmodes/rust-ts-mode.el b/lisp/progmodes/rust-ts-mode.el
index ec823d09d8c..064ba6b21e6 100644
--- a/lisp/progmodes/rust-ts-mode.el
+++ b/lisp/progmodes/rust-ts-mode.el
@@ -208,50 +208,11 @@ rust-ts-mode--font-lock-settings
    `((scoped_use_list path: (identifier) @font-lock-constant-face)
      (scoped_use_list path: (scoped_identifier
                              name: (identifier) @font-lock-constant-face))
-
-     ((use_as_clause alias: (identifier) @font-lock-type-face)
-      (:match "^[A-Z]" @font-lock-type-face))
-     ((use_as_clause path: (identifier) @font-lock-type-face)
-      (:match "^[A-Z]" @font-lock-type-face))
-     ((use_as_clause path:
-                     (scoped_identifier path: (_)
-                                        name: (identifier) @font-lock-type-face))
-      (:match "^[A-Z]" @font-lock-type-face))
-     (use_as_clause path: (scoped_identifier name: (identifier) @default))
-
-     ((use_declaration
-       argument: (scoped_identifier
-                  path: (_) @font-lock-constant-face
-                  name: (identifier) @font-lock-type-face))
-      (:match "^[A-Z]" @font-lock-type-face))
-     (use_declaration
-      argument: (scoped_identifier
-                 name: (identifier) @default))
-
-     (use_declaration
-      argument: (scoped_identifier
-                 path: (scoped_identifier
-                        path: (_) @font-lock-constant-face
-                        name: (identifier) @font-lock-constant-face)
-                 name: (identifier) @default))
-
-     (use_declaration
-      argument: (scoped_use_list
-                 path: (scoped_identifier
-                        path: (_) @font-lock-constant-face
-                        name: (identifier) @font-lock-constant-face)))
-
      ((use_list (identifier) @font-lock-type-face)
       (:match "^[A-Z]" @font-lock-type-face))
-     (use_list (identifier) @default)
-     ((use_list (scoped_identifier path: (_)
-                                   name: (identifier) @font-lock-type-face))
-      (:match "^[A-Z]" @font-lock-type-face))
-     (use_list (scoped_identifier path: (_)
-                                  name: (identifier) @default))
-     (use_wildcard (scoped_identifier
-                    name: (identifier) @font-lock-constant-face))
-
+     (use_wildcard [(identifier) @rust-ts-mode--fontify-scope
+                    (scoped_identifier
+                     name: (identifier) @rust-ts-mode--fontify-scope)])
      (enum_variant name: (identifier) @font-lock-type-face)
      (match_arm
       pattern: (match_pattern (_ type: (identifier) @font-lock-type-face)))
@@ -262,31 +223,13 @@ rust-ts-mode--font-lock-settings
      (mod_item name: (identifier) @font-lock-constant-face)
      (primitive_type) @font-lock-type-face
      (type_identifier) @font-lock-type-face
-     ((scoped_identifier name: (identifier) @font-lock-type-face)
-      (:match "^[A-Z]" @font-lock-type-face))
-     ((scoped_identifier path: (identifier) @font-lock-type-face)
-      (:match "^[A-Z]" @font-lock-type-face))
-     ((scoped_identifier
-       path: [(identifier) @font-lock-type-face
-              (scoped_identifier
-               name: (identifier) @font-lock-type-face)])
-      (:match "^[A-Z]" @font-lock-type-face))
+     ((scoped_identifier name: (identifier) @rust-ts-mode--fontify-tail))
      ((scoped_identifier path: (identifier) @font-lock-type-face)
       (:match
        "^\\(u8\\|u16\\|u32\\|u64\\|u128\\|usize\\|i8\\|i16\\|i32\\|i64\\|i128\\|isize\\|char\\|str\\)$"
        @font-lock-type-face))
-     (scoped_identifier path: (_) @font-lock-constant-face
-                        name: (identifier) @font-lock-type-face)
-     (scoped_identifier path: (scoped_identifier
-                               name: (identifier) @font-lock-constant-face))
-     (scoped_type_identifier path: (_) @font-lock-constant-face)
-     (scoped_type_identifier
-      path: (scoped_identifier
-             path: (_) @font-lock-constant-face
-             name: (identifier) @font-lock-constant-face))
-     (type_identifier) @font-lock-type-face
-     ;; Ensure function calls aren't highlighted as types.
-     (call_expression function: (scoped_identifier name: (identifier) @default)))
+     ((scoped_identifier path: (identifier) @rust-ts-mode--fontify-scope))
+     (type_identifier) @font-lock-type-face)
 
    :language 'rust
    :feature 'property
@@ -302,9 +245,23 @@ rust-ts-mode--font-lock-settings
 
    :language 'rust
    :feature 'variable
-   '((identifier) @font-lock-variable-name-face
-     ;; Everything in a token_tree is an identifier.
-     (token_tree (identifier) @default))
+   '((compound_assignment_expr right: (identifier) @font-lock-variable-name-face)
+     (assignment_expression right: (identifier) @font-lock-variable-name-face)
+     (let_declaration value: (identifier) @font-lock-variable-name-face)
+     (binary_expression left: (identifier) @font-lock-variable-name-face)
+     (binary_expression right: (identifier) @font-lock-variable-name-face)
+     (if_expression condition: (identifier) @font-lock-variable-name-face)
+     (let_condition value: (identifier) @font-lock-variable-name-face)
+     (while_expression condition: (identifier) @font-lock-variable-name-face)
+     (field_expression value: (identifier) @font-lock-variable-name-face)
+     (reference_expression value: (identifier) @font-lock-variable-name-face)
+     (match_expression value: (identifier) @font-lock-variable-name-face)
+     (match_arm value: (identifier) @font-lock-variable-name-face)
+     (arguments (identifier) @font-lock-variable-name-face)
+     (block (identifier) @font-lock-variable-name-face)
+     (array_expression (identifier) @font-lock-variable-name-face)
+     (tuple_expression (identifier) @font-lock-variable-name-face)
+     (return_expression (identifier) @font-lock-variable-name-face))
 
    :language 'rust
    :feature 'escape-sequence
@@ -317,6 +274,28 @@ rust-ts-mode--font-lock-settings
    '((ERROR) @font-lock-warning-face))
   "Tree-sitter font-lock settings for `rust-ts-mode'.")
 
+(defun rust-ts-mode--fontify-scope (node override start end &optional tail-p)
+  (let* ((case-fold-search nil)
+         (face
+          (cond
+           ((string-match-p "^[A-Z]" (treesit-node-text node))
+            'font-lock-type-face)
+           ((and
+             tail-p
+             (string-match-p
+              "\\`\\(?:use_list\\|call_expression\\|use_as_clause\\|use_declaration\\)\\'"
+              (treesit-node-type (treesit-node-parent node))))
+            nil)
+           (t 'font-lock-constant-face))))
+    (when face
+      (treesit-fontify-with-override
+       (treesit-node-start node) (treesit-node-end node)
+       face
+       override start end))))
+
+(defun rust-ts-mode--fontify-tail (node override start end)
+  (rust-ts-mode--fontify-scope node override start end t))
+
 (defalias 'rust-ts-mode--fontify-pattern
   (and
    (treesit-available-p)

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Jostein Kjønigsen]

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

Hey everyone.

When I was filing the initial bug-report, I sort of noticed how 
rust-ts-mode was ... lets say not as consistent or refined about things 
as the other -ts-modes have turned out to be, but for the time being I 
decided to focus on functions-calls only, because that seemed to be the 
biggest omission. That said, I did notice other parts had room for 
improvement too.

No need for me to file a bug-report for that now though, as I see you 
two have done some very thorough work in the meantime :D

If I were to complain about only -1- thing in that previous effort, it 
would be that you've compared rust-ts-mode to GitHub, VSCode, IntelliJ 
and what not ... but not rust-mode from MELPA! I think lots of the 
rust-ts-mode users will be coming from that major-mode, rather than 
other editors, so IMO it's quite a handy reference for how people might 
expect things to be.

Below is a screenshot which puts rust-ts-mode (with the latest patches 
from this thread) head to head against rust-mode from MELPA:

 From what I can tell, neither of them is perfect yet, but they both get 
some things right:

  * rust-ts-mode: function invocations :)
  * rust-ts-mode handles constants better (also escape-sequences, but
    not seen in this sample)
  * rust-mode: consistently fontify annotations (notice they are missing
    in rust-ts-mode, line 12 and 14). Also rust-mode use
    font-lock-preprocessor-face, which I think as a more appropriate
    face for this kind of syntax, than font-lock-constant-face (used in
    rust-ts-mode).
  * rust-mode: is able to handle nested macro-invocations. See line 42
    and 44 above. From what I can tell, this seems to be due to a
    short-coming in the tree-sitter grammar for rust, and we may be able
    to fix it upstream, instead of monkey-patch things based on regexp's
    in rust-ts-mode

As for things which are less great in rust-ts-mode:

  * some code does not seem to get fontified at all (types, keywords,
    etc). Line 14-17.
  * it seems to fontify all variables using font-lock-variable-name-face
    all over, regardless of it is a declaration or not. I realize this
    is not 100% consistent throughout the Emacs-verse, but I know other
    -ts-modes have aimed for declaration only, and so does rust-mode
    from MELPA too (although with some consistency-issues) which this
    would be replacing.
  * it does not seem to handle ::* imports properly? See line 9. The way
    I understand it, things preceeding the ::* should be considered a
    namespace too?
  * I know imports are difficult to be 100% accurate about, as seen in
    this thread. Are we importing a module or a class? Impossible to
    tell without looking at the referenced code! Aiming for visual
    consistency may be a better goal than 100% correctness, if the AST
    we're getting don't provide good enough information? (This has been
    done in other modes too)

With utmost respect for all the work put in so far (rust is a 
complicated language after all), and I realize this is subjective, to me 
rust-ts-mode does not yet seem /quite there/, in terms of correctness or 
consistency. And as I'm sure Eli will remind us, Emacs 29 release is 
getting very, very, very close.

Could it be an option to not have rust-ts-mode as part of Emacs 29, but 
leave it in git master for now? That would leave us time to sort out all 
these things properly, and also have good time to decide the things 
which actually needs to be agreed upon, before implementing the final fixes?

Just my 2 cents.

--
Jostein


On 2/10/23 04:44, Randy Taylor wrote:

> On Thursday, February 9th, 2023 at 16:19, Dmitry Gutov<dgutov@yandex.ru>  wrote:
>> On 09/02/2023 05:38, Randy Taylor wrote:
>>
>>>> What if it looked like this:
>>>>
>>>>     let date = DateTime::<chrono::utc>::from_utc(date, chrono::utc);
>>>>
>>>> If we decide purely based on capitalization, then I guess the rule
>>>> should be present in both lists (with capitalized? regexp in one, and
>>>> !capitalized? regexp in another), and a few more rules should be
>>>> duplicated as well.
>>> In both cases, utc is still a type even if it's not capitalized.
>>> My patch addresses this.
>> So the end of a scoping chain must also be either a type or a method
>> call? We might be able to use that, somehow.
> I believe so (with the exception of use declarations as you note).
> Not familiar enough with Rust to say for sure :).
>
>> Though the 'use' declarations might be exceptions, e.g.
>>
>>    use crate::foo::baz::foobaz;crate
>>
>> or
>>
>>    use std::{fmt, fs, usize};
>>
>> (fmt and fs are modules, not types).
>>
>>>> This becomes a little more painful semantically, given that the first
>>>> 'utc' in the example above is parsed into a (type_identifier) node, not
>>>> just (identifier).
>>>>
>>>>>> On a distantly related note, we have terms like 'usize' which is
>>>>>> normally a type (and highlighted as such), but can also feature in
>>>>>> expressions like
>>>>>>
>>>>>>      let row = usize::from_str_radix(row, 10).map_err(|_| error())?;
>>>>>>
>>>>>> where it is now highlighted with font-lock-constant-face. Should we try
>>>>>> to do anything about that? If there is a limited number of built-in
>>>>>> types in that situation (e.g. all of them primitives), we could handle
>>>>>> that with a regexp.
>>>>> Right. I think it makes sense to handle the primitives with a regex.
>>>>> I'm not sure if there's anything else beyond those.
>>>>> There's a list of them here:https://doc.rust-lang.org/reference/types.html
>>>>> I think it would only apply to the numerical and textual types.
>>>> So 'usize' in the above is definitely a "type", not a "module"?
>>> I think so. You can see on usize's documentation page (https://doc.rust-lang.org/std/primitive.usize.html)
>>> that it provides that function, amongst many others.
>> I was thinking that it might also be referring to (apparently
>> deprecated)https://doc.rust-lang.org/std/usize/index.html.
> That module only provides the constants listed on that page.
> The usize type itself provides a bunch of constants and functions (same for the rest of the primitives).
>
> I'm curious how other editors/IDEs highlight this stuff, but I don't have any on hand ATM.
>
>> Sorry, I'm not really familiar with Rust. E.g. in Ruby every class can
>> also serve as a "module" in the scoping sense. As a result we highlight
>> both classes and modules with font-lock-type-face. This could also be an
>> option here, if everything else fails.
>>
>> But we could also highlight based on a "role" (constant if it's used as
>> a scope, and type if it's used as a type).
>>
>> Although one could say that 'Path' in
>>
>>    Some(Path::new("./foo"))
>>
>> is being used as a type as well, and 'Some' too. So it might not be the
>> best fit.
>>
>> Speaking of 'usize' again, what if some lib or the app defines an
>> 'usize' module for its custom functions acting on it? E.g.
>> 'my::app::usize'. A simple regexp matcher will probably highlight it as
>> a type as well.
> I don't think we should worry about those cases IMO.
>
>>>>>> Or vice versa, in
>>>>>>
>>>>>>      use std::{fmt, fs, usize};
>>>>>>
>>>>>> should 'fmt', 'fs' and 'usize' be highlighted with
>>>>>> font-lock-constant-face rather than font-lock-type-face?
>>>>> They should indeed be highlighted with font-lock-constant-face because they are modules.
>>>>> We assume the types will be capitalized since that's all we can really do (and it's the convention anyway).
>>>> If they're modules here, I suppose they should be highlighted the same in
>>>>
>>>>     let row = usize::from_str_radix(...)
>>>>
>>>> as well. The bright side is that will make a more complex regexp
>>>> (enumerating the lowercase named types) unnecessary.
>>> Yes, except for the primitives.
>>>
>>> I have attached a patch which I think addresses most of the concerns (although I've been at it for a few hours and my brain is mush now).
>>>
>>> The patch does the following:
>>> - Separates import-related stuff and module use by leveraging the use_declaration query (simplifying things greatly IMO).
>>> - Highlights primitive types used in scoped_identifiers.
>>> - Properly highlights types belonging to a module no matter how deep it is (or your money back guaranteed!).
>>> - Maybe some other stuff I forgot. I'm too tried now :).
>> Thank you, I can sympathize -- this stuff gets complicated.
>>
>> Some problems from my testing:
>>
>> 1. If I keep treesit-font-lock-level at its default value (3), some
>> stuff gets misfontified:
> Sorry, I have only been testing with level 4.
> This is also why I want type and module combined into one so we don't have to deal with this headache.
>
>>    use std::collections::hash_map::{self, HashMap};
>>
>> 'hash_map' is highlighted as a type. 'HashMap' is not highlighted at all.
>>
>>    use std::{fmt, fs, usize};
>>
>> Only 'use' is highlighted here.
> This is because of how things are broken out into the module feature.
> That some highlighting for those occurs is by overlap of queries in the type feature.
> Which again is why I think module should be part of type.
>
>>    test::test1();
>>
>> 'test1' is highlighted as a type (we discussed this problem with
>> highlighting types by default -- it becomes necessary to filter out
>> function calls, either with more complex queries, or with custom
>> highlighter functions).
> Right, I added a query to filter that out now.
>
>> 2. If I switch to treesit-font-lock-level 4:
>>
>>    let boxed_i32 = Box::new(5_i32);
>>
>> 'Box' is highlighted with font-lock-constant-face. I think it's a type,
>> though.
> Oops, I accidentally removed the rule for that. Added it back.
>
>> Also here's a pre-existing problem mentioned above:
>>
>>    use std::{fmt, fs, usize};
>>
>> 'fmt' and 'fs' are not types. But they are highlighted with
>> font-lock-type-face.
> This is really weird, I can reproduce it with emacs -Q but not with my normal config...
> Also with emacs -Q this:
> let date = DateTime::<chrono::hey::there::Utc>::from_utc(date, chrono::cool::this::Utc);
>
> highlights incorrectly, where "there" is font-lock-variable-name-face. But with my normal config everything is fine.
>
> I'll look into it tomorrow. Not really sure what in my config could cause this...
>
>>> A few questions:
>>> - Should module be moved to level 3 to be with type?
>>> - Do we still want the module feature, or should this stuff be put into type?
>> I suppose we should iron some kinds out first to get a better understanding.
> Attached a new patch hopefully addressing most of the problems you ran into (minus the level 3 use declaration highlights).
> Especially after the problems you ran into at level 3, I strongly think the module queries should get thrown into type (and they make sense there anyway IMO). Then all those issues go away.

[-- Attachment #2.1: Type: text/html, Size: 14561 bytes --]

[-- Attachment #2.2: 7OuD4JJHi6YGp6lt.png --]
[-- Type: image/png, Size: 157052 bytes --]

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Randy Taylor]

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

On Monday, February 13th, 2023 at 05:17, Jostein Kjønigsen <jostein@secure.kjonigsen.net> wrote:
>Hey everyone.
>
>When I was filing the initial bug-report, I sort of noticed how rust-ts-mode was ... lets say not as consistent or refined about things as the other -ts-modes have turned out to be, but for the time being I decided to focus on functions-calls only, because that seemed to be the biggest omission. That said, I did notice other parts had room for improvement too.

In the future, it would've been nice to have bug reports filed for these regardless.
Especially since I had time last week to sink in to fixing these problems...

>
>No need for me to file a bug-report for that now though, as I see you two have done some very thorough work in the meantime :D
>
>If I were to complain about only -1- thing in that previous effort, it would be that you've compared rust-ts-mode to GitHub, VSCode, IntelliJ and what not ... but not rust-mode from MELPA! I think lots of the rust-ts-mode users will be coming from that major-mode, rather than other editors, so IMO it's quite a handy reference for how people might expect things to be.

I have been comparing to rust-mode, both now and when I was making rust-ts-mode.

>
>Below is a screenshot which puts rust-ts-mode (with the latest patches from this thread) head to head against rust-mode from MELPA:

Thanks. I wish you also would've included the code as text so I didn't have to type it all out :).

>
>From what I can tell, neither of them is perfect yet, but they both get some things right:
>
> rust-ts-mode: function invocations :)
> rust-ts-mode handles constants better (also escape-sequences, but not seen in this sample)
> rust-mode: consistently fontify annotations (notice they are missing in rust-ts-mode, line 12 and 14). Also rust-mode use font-lock-preprocessor-face, which I think as a more appropriate face for this kind of syntax, than font-lock-constant-face (used in rust-ts-mode).
> rust-mode: is able to handle nested macro-invocations. See line 42 and 44 above. From what I can tell, this seems to be due to a short-coming in the tree-sitter grammar for rust, and we may be able to fix it upstream, instead of monkey-patch things based on regexp's in rust-ts-mode
>
>As for things which are less great in rust-ts-mode:
>
> some code does not seem to get fontified at all (types, keywords, etc). Line 14-17.

Did you look at that with treesit-explore-mode?
It's inside a macro invocation which mostly consists of token_trees.
Not much helpful stuff for us to go on to highlight.

> it seems to fontify all variables using font-lock-variable-name-face all over, regardless of it is a declaration or not. I realize this is not 100% consistent throughout the Emacs-verse, but I know other -ts-modes have aimed for declaration only, and so does rust-mode from MELPA too (although with some consistency-issues) which this would be replacing.

Because that's what the variable feature is supposed to do, same as the function feature.
Perhaps rust-ts-mode's definition feature can be augmented to support that (and also note it's missing an assignment feature that some other modes have).

> it does not seem to handle ::* imports properly? See line 9. The way I understand it, things preceeding the ::* should be considered a namespace too?

Correct, I will fix that as part of my next patch I post.
Before, we weren't distinguishing imports and general scope identifiers which caused a lot of inconsistencies. Now we do, so it's just a matter of rounding up all the import-related queries.

> I know imports are difficult to be 100% accurate about, as seen in this thread. Are we importing a module or a class? Impossible to tell without looking at the referenced code! Aiming for visual consistency may be a better goal than 100% correctness, if the AST we're getting don't provide good enough information? (This has been done in other modes too)

That is what we're trying to do. I believe the patch I posted earlier in the thread covers most of these cases, minus the wildcard one you mentioned (which I will post a new patch addressing sometime later today). If you notice any others, please shout.

>
>With utmost respect for all the work put in so far (rust is a complicated language after all), and I realize this is subjective, to me rust-ts-mode does not yet seem quite there, in terms of correctness or consistency. And as I'm sure Eli will remind us, Emacs 29 release is getting very, very, very close.
>
>Could it be an option to not have rust-ts-mode as part of Emacs 29, but leave it in git master for now? That would leave us time to sort out all these things properly, and also have good time to decide the things which actually needs to be agreed upon, before implementing the final fixes?

I don't understand this. So there may be a few things missing or a few inconsistencies - so what? People can make bug reports for them and they can be fixed. rust-mode itself has inconsistencies and correctness issues as well, and other ts modes do too (like c/c++-ts-modes WRT macros).

>
>Just my 2 cents.
>
>-->Jostein

[-- Attachment #2: Type: text/html, Size: 6597 bytes --]

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Jostein Kjønigsen]

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

On 2/13/23 15:39, Randy Taylor wrote:
> In the future, it would've been nice to have bug reports filed for 
> these regardless.
> Especially since I had time last week to sink in to fixing these 
> problems...
I'll try to keep that in my for the future. But registering "high 
quality" bug-reports takes some effort too, and I feel like just rushing 
them because -I- lack time may come off as impolite.
>
> I have been comparing to rust-mode, both now and when I was making 
> rust-ts-mode.
Ok. Sorry for missing it. My bad :)
>
> Thanks. I wish you also would've included the code as text so I didn't 
> have to type it all out :).
Indeed. Where -are- my manners? Attached is the original source file!
>
> >    rust-mode: consistently fontify annotations (notice they are missing in rust-ts-mode, line 12 and 14). Also rust-mode use 
> font-lock-preprocessor-face, which I think as a more appropriate face 
> for this kind of syntax, than font-lock-constant-face (used in 
> rust-ts-mode).

Since this wasn't really mentioned in the reply... Any chance we can 
give font-lock-preprocessor-face some love too? Or should I make that a 
bug of its own?

> >    some code does not seem to get fontified at all (types, keywords, 
> etc). Line 14-17.
>
> Did you look at that with treesit-explore-mode?
> It's inside a macro invocation which mostly consists of token_trees.
> Not much helpful stuff for us to go on to highlight.

Like I said. Might need to be fixed upstream in the tree-sitter rust 
grammars.

I guess it seems like the rust-grammar in general could use some 
improvements...

>
> >    it seems to fontify all variables using font-lock-variable-name-face all over, regardless of it is a 
> declaration or not. I realize this is not 100% consistent throughout 
> the Emacs-verse, but I know other -ts-modes have aimed for declaration 
> only, and so does rust-mode from MELPA too (although with some 
> consistency-issues) which this would be replacing.
>
> Because that's what the variable feature is supposed to do, same as 
> the function feature.
> Perhaps rust-ts-mode's definition feature can be augmented to support 
> that (and also note it's missing an assignment feature that some other 
> modes have).

Right. Like I said, the Emacs-verse is not really 100% consistent about 
that, and I doubt it ever will be.

Personally I was asked to remove such fontification when submitting 
changes/improvements to typescrip-ts-mode and csharp-ts-mode... And in 
the end I think I like the results more that way, and just assumed this 
was supposed to be the standard.

Oh well.

>
> >    it does not seem to handle ::* imports properly? See line 9. The way I understand it, things preceeding the 
> ::* should be considered a namespace too?
>
> Correct, I will fix that as part of my next patch I post.
> Before, we weren't distinguishing imports and general scope 
> identifiers which caused a lot of inconsistencies. Now we do, so it's 
> just a matter of rounding up all the import-related queries.
Sounds great!
>
> >    I know imports are difficult to be 100% accurate about, as seen in this thread. Are we importing a module or 
> a class? Impossible to tell without looking at the referenced code! 
> Aiming for visual consistency may be a better goal than 100% 
> correctness, if the AST we're getting don't provide good enough 
> information? (This has been done in other modes too)
>
> That is what we're trying to do. I believe the patch I posted earlier 
> in the thread covers most of these cases, minus the wildcard one you 
> mentioned (which I will post a new patch addressing sometime later 
> today). If you notice any others, please shout.
Sure thing.
>
> I don't understand this. So there may be a few things missing or a few 
> inconsistencies - so what? People can make bug reports for them and 
> they can be fixed. rust-mode itself has inconsistencies and 
> correctness issues as well, and other ts modes do too (like 
> c/c++-ts-modes WRT macros).

I don't know. My WASM code may not have been the most typical rust code 
and as such may not serve as a great baseline for fontification?

I just assumed if it would be much work to fix up, it might be hard to 
make the required fixes in time.

If you (as one of the main implementers) disagree, who am I to argue? :)

--
Jostein

[-- Attachment #2: lib.rs --]
[-- Type: text/rust, Size: 1513 bytes --]

extern crate cfg_if;
extern crate wasm_bindgen;

mod utils;

use cfg_if::cfg_if;
use wasm_bindgen::prelude::*;

cfg_if! {
    // When the `wee_alloc` feature is enabled, use `wee_alloc` as the global
    // allocator.
    if #[cfg(feature = "wee_alloc")] {
        extern crate wee_alloc;
        #[global_allocator]
        static ALLOC: wee_alloc::WeeAlloc = wee_alloc::WeeAlloc::INIT;
    }
}

#[wasm_bindgen]
pub fn should_handle(url: String) -> bool {
    if url.ends_with(".css")
       || url.ends_with(".js")
       || url.ends_with(".png")
       || url.ends_with(".jpg")
    {
        false
    } else {
        true
    }
}

let result = format!("{}{}",
    "Well yes",
    format!("Or {} no?", "possibly"));

#[wasm_bindgen]
pub fn page(url: String) -> String {
    html(
        header(&format!("wasm-page: {}", &url)),
        body(format!(
            "{}{}",
            "<p>Hello from Rust &amp; wasm!</p>",
            format!(
                "<footer style=\"color: #cccccc\">Copyright Jostein Kjønigsen - {}</footer>",
                &url
            )
        ))
    )
}

fn header(title: &str) -> String {
    head(element("title", title))
}

fn html(head: String, body: String) -> String {
    element("html", &format!("{}\n{}", head, body))
}

fn head(content: String) -> String {
    element("head", &content)
}

fn body(content: String) -> String {
    element("body", &content)
}


fn element(name: &str, content: &str) -> String {
    format!("<{}>{}</{}>", name, content, name)
}

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Randy Taylor]

On Monday, February 13th, 2023 at 10:04, Jostein Kjønigsen <jostein@secure.kjonigsen.net> wrote:
>On 2/13/23 15:39, Randy Taylor wrote:
>> In the future, it would've been nice to have bug reports filed for
>> these regardless.
>> Especially since I had time last week to sink in to fixing these
>> problems...
>I'll try to keep that in my for the future. But registering "high
>quality" bug-reports takes some effort too, and I feel like just rushing
>them because -I- lack time may come off as impolite.

Fair enough. Personally, I wouldn't mind a simpler/less complete bug report if it's clearly stated that one didn't have enough time to fully detail it. That way I can at least try to fix it and if not, no harm, we just wait until the reporter has time to give more information.

>>
>> I have been comparing to rust-mode, both now and when I was making
>> rust-ts-mode.
>Ok. Sorry for missing it. My bad :)

No worries, you didn't miss anything. The exchange between Dmitry and I was simply that I was curious about how other editors and IDEs (especially ones with tree-sitter support like neovim) are highlighting these things, hence why rust-mode was not brought up.

>>
>> Thanks. I wish you also would've included the code as text so I didn't
>> have to type it all out :).
>Indeed. Where -are- my manners? Attached is the original source file!

Thanks :).

>>
>> >    rust-mode: consistently fontify annotations (notice they are missing in rust-ts-mode, line 12 and 14). Also rust-mode use
>> font-lock-preprocessor-face, which I think as a more appropriate face
>> for this kind of syntax, than font-lock-constant-face (used in
>> rust-ts-mode).
>
>Since this wasn't really mentioned in the reply... Any chance we can
>give font-lock-preprocessor-face some love too? Or should I make that a
>bug of its own?

Sorry about that, I forgot to reply to that. Hopefully I didn't miss anything else in this reply!
Yes, font-lock-preprocessor-face is more appropriate - I will include that change when I post my next patch.

>
>> >    some code does not seem to get fontified at all (types, keywords,
>> etc). Line 14-17.
>>
>> Did you look at that with treesit-explore-mode?
>> It's inside a macro invocation which mostly consists of token_trees.
>> Not much helpful stuff for us to go on to highlight.
>
>Like I said. Might need to be fixed upstream in the tree-sitter rust
>grammars.
>
>I guess it seems like the rust-grammar in general could use some
>improvements...

Yup, that seems to be the case. I did see your other report that was also macro-related, I just didn't have anything to add to it.
Although whether or not the grammar is capable of anything more here I have no idea.
Like C/C++, whenever macros are involved, trouble arrives it seems.

>
>>
>> >    it seems to fontify all variables using font-lock-variable-name-face all over, regardless of it is a
>> declaration or not. I realize this is not 100% consistent throughout
>> the Emacs-verse, but I know other -ts-modes have aimed for declaration
>> only, and so does rust-mode from MELPA too (although with some
>> consistency-issues) which this would be replacing.
>>
>> Because that's what the variable feature is supposed to do, same as
>> the function feature.
>> Perhaps rust-ts-mode's definition feature can be augmented to support
>> that (and also note it's missing an assignment feature that some other
>> modes have).
>
>Right. Like I said, the Emacs-verse is not really 100% consistent about
>that, and I doubt it ever will be.
>
>Personally I was asked to remove such fontification when submitting
>changes/improvements to typescrip-ts-mode and csharp-ts-mode... And in
>the end I think I like the results more that way, and just assumed this
>was supposed to be the standard.
>
>Oh well.

I don't see why we can't be consistent about it, but it's just that the variable and function features themselves are, to my understanding, meant to highlight all of that stuff fully. Then, the assignment, definition, and whatever smaller features can be used for folks who want more specific and "quieter" highlights. I never added those smaller ones into any of my modes because I want my code looking like a wild disco.

>
>>
>> >    it does not seem to handle ::* imports properly? See line 9. The way I understand it, things preceeding the
>> ::* should be considered a namespace too?
>>
>> Correct, I will fix that as part of my next patch I post.
>> Before, we weren't distinguishing imports and general scope
>> identifiers which caused a lot of inconsistencies. Now we do, so it's
>> just a matter of rounding up all the import-related queries.
>Sounds great!
>>
>> >    I know imports are difficult to be 100% accurate about, as seen in this thread. Are we importing a module or
>> a class? Impossible to tell without looking at the referenced code!
>> Aiming for visual consistency may be a better goal than 100%
>> correctness, if the AST we're getting don't provide good enough
>> information? (This has been done in other modes too)
>>
>> That is what we're trying to do. I believe the patch I posted earlier
>> in the thread covers most of these cases, minus the wildcard one you
>> mentioned (which I will post a new patch addressing sometime later
>> today). If you notice any others, please shout.
>Sure thing.
>>
>> I don't understand this. So there may be a few things missing or a few
>> inconsistencies - so what? People can make bug reports for them and
>> they can be fixed. rust-mode itself has inconsistencies and
>> correctness issues as well, and other ts modes do too (like
>> c/c++-ts-modes WRT macros).
>
>I don't know. My WASM code may not have been the most typical rust code
>and as such may not serve as a great baseline for fontification?

I did notice that macro-related stuff had issues, but that's because of parser limitations.
Aside from macros, I think everything else is fontified fine and much better than rust-mode (pending my patch fixing up imports).

>
>I just assumed if it would be much work to fix up, it might be hard to
>make the required fixes in time.
>
>If you (as one of the main implementers) disagree, who am I to argue? :)

I just don't see why any of those issues should stop it from being included in emacs-29 - none of them are showstoppers, and nothing is forcing people to use rust-ts-mode.

I expect lots of bug reports for all the ts modes once emacs-29 comes out :D.

>
>--
>Jostein

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Dmitry Gutov]

On 13/02/2023 16:39, Randy Taylor wrote:

>>
>>From what I can tell, neither of them is perfect yet, but they both get some things right:
>>
>>    rust-ts-mode: function invocations :)
>>    rust-ts-mode handles constants better (also escape-sequences, but not seen in this sample)
>>    rust-mode: consistently fontify annotations (notice they are missing in rust-ts-mode, line 12 and 14). Also rust-mode use font-lock-preprocessor-face, which I think as a more appropriate face for this kind of syntax, than font-lock-constant-face (used in rust-ts-mode).
>>    rust-mode: is able to handle nested macro-invocations. See line 42 and 44 above. From what I can tell, this seems to be due to a short-coming in the tree-sitter grammar for rust, and we may be able to fix it upstream, instead of monkey-patch things based on regexp's in rust-ts-mode
>>
>>As for things which are less great in rust-ts-mode:
>>
>>    some code does not seem to get fontified at all (types, keywords, etc). Line 14-17.
> 
> Did you look at that with treesit-explore-mode?
> It's inside a macro invocation which mostly consists of token_trees.
> Not much helpful stuff for us to go on to highlight.

Depending on the progress in improving the grammar, we could choose to 
add some ad-hoc Lisp based fontification to macro calls (using something 
similar to what rust-mode already does, I guess).

There's no hurry for that, though. Certainly not before Emacs 29 is out.

>>    it seems to fontify all variables using font-lock-variable-name-face all over, regardless of it is a declaration or not. I realize this is not 100% consistent throughout the Emacs-verse, but I know other -ts-modes have aimed for declaration only, and so does rust-mode from MELPA too (although with some consistency-issues) which this would be replacing.
> 
> Because that's what the variable feature is supposed to do, same as the 
> function feature.
> Perhaps rust-ts-mode's definition feature can be augmented to support 
> that

That should already work. Either try treesit-font-lock-level=3, or use 
level 4 but follow it with disabling the 'variable' feature, and you'll 
see variable bindings highlighted. In function parameters and 
let/for/match expressions.

> (and also note it's missing an assignment feature that some other 
> modes have).

I didn't bother with assignments for now (in the absence of feature 
requests), but they should be even easier to add.

Overall, I would recommend to drop the 'variable' feature as it is now 
(sorry for repeating myself), because if we reach the state where 
*everything but* variable references is already highlighted with some 
face, the variable references will stand out automatically (only they 
will be rendered with 'default' face). Adding 
font-lock-variable-name-face drops the distinction between a definition 
and a reference.

But I don't want to force this subject: if you like it enough, no 
problem. The users can disable it manually as well.

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Dmitry Gutov]

On 13/02/2023 21:57, Dmitry Gutov wrote:
> I didn't bother with assignments for now (in the absence of feature 
> requests), but they should be even easier to add.

E.g. like this:

diff --git a/lisp/progmodes/rust-ts-mode.el b/lisp/progmodes/rust-ts-mode.el
index 5c71a8ad461..f105c894b28 100644
--- a/lisp/progmodes/rust-ts-mode.el
+++ b/lisp/progmodes/rust-ts-mode.el
@@ -163,11 +163,15 @@ rust-ts-mode--font-lock-settings
       (macro_definition "macro_rules!" @font-lock-constant-face)
       (macro_definition (identifier) @font-lock-preprocessor-face)
       (field_declaration name: (field_identifier) @font-lock-property-face)
-     (parameter) @rust-ts-mode--fontify-pattern
-     (let_declaration) @rust-ts-mode--fontify-pattern
-     (for_expression) @rust-ts-mode--fontify-pattern
-     (let_condition) @rust-ts-mode--fontify-pattern
-     (match_arm) @rust-ts-mode--fontify-pattern)
+     (parameter pattern: (_) @rust-ts-mode--fontify-pattern)
+     (let_declaration pattern: (_) @rust-ts-mode--fontify-pattern)
+     (for_expression pattern: (_) @rust-ts-mode--fontify-pattern)
+     (let_condition pattern: (_) @rust-ts-mode--fontify-pattern)
+     (match_arm pattern: (_) @rust-ts-mode--fontify-pattern))
+
+   :language 'rust
+   :feature 'assignment
+   '(((assignment_expression left: (_) @rust-ts-mode--fontify-pattern)))

     :language 'rust
     :feature 'function
@@ -261,7 +265,7 @@ 'rust-ts-mode--fontify-pattern
     (treesit-available-p)
     `(lambda (node override start end &rest _)
        (let ((captures (treesit-query-capture
-                       (treesit-node-child-by-field-name node "pattern")
+                       node
                         ,(treesit-query-compile 'rust '((identifier) @id
 
(shorthand_field_identifier) @id)))))
          (pcase-dolist (`(_name . ,id) captures)
@@ -343,7 +347,7 @@ rust-ts-mode
                  '(( comment definition)
                    ( keyword string)
                    ( attribute builtin constant escape-sequence
-                    number type)
+                    number type assignment)
                    ( bracket delimiter error function operator property 
variable)))

      ;; Imenu.

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Dmitry Gutov]

On 13/02/2023 12:17, Jostein Kjønigsen wrote:
> it seems to fontify all variables using font-lock-variable-name-face all 
> over, regardless of it is a declaration or not. I realize this is not 
> 100% consistent throughout the Emacs-verse, but I know other -ts-modes 
> have aimed for declaration only, and so does rust-mode from MELPA too 
> (although with some consistency-issues) which this would be replacing.

To be fair, some ts modes add variable highlighting like that, and some 
don't. Those that do, keep it on treesit-font-lock-level=4, meaning it's 
disabled by default, together with the function call and property lookup 
highlighting.

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Dmitry Gutov]

Hi Randy,

Maybe I was too quick to commit the change. But let's discuss it.

On 05/02/2023 23:30, Randy Taylor wrote:
> That's expected (at least to me) because it's a property.

It's both a property and a function, isn't it?

> The same 
> applies to c-ts-mode and go-ts-mode as well.

Regarding c-ts-mode, it might be a simple oversight, given that 
constructs like

   p->handler (it)

do not come up very often. But if we take js-ts-mode, for example, where 
a property is often a function, the property name is highlighted as a 
function in a funcall.

And speaking of c-ts-mode, this can fix that omission:

diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
index 5093c3980b6..3740130be30 100644
--- a/lisp/progmodes/c-ts-mode.el
+++ b/lisp/progmodes/c-ts-mode.el
@@ -470,7 +470,9 @@ c-ts-mode--font-lock-settings
     :language mode
     :feature 'function
     '((call_expression
-      function: (identifier) @font-lock-function-name-face))
+      function:
+      [(identifier) @font-lock-function-name-face
+       (field_expression field: (field_identifier) 
@font-lock-function-name-face)]))

     :language mode
     :feature 'variable

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Randy Taylor]

On Sunday, February 5th, 2023 at 16:56, Dmitry Gutov <dgutov@yandex.ru> wrote:
> Hi Randy,
> 
> Maybe I was too quick to commit the change. But let's discuss it.

No, it's fine. You can also see my reply to Jostein.

BTW thanks for your work on the other modes and sorry for my lack of replies, I've been busy as of late.

> 
> On 05/02/2023 23:30, Randy Taylor wrote:
> 
> > That's expected (at least to me) because it's a property.
> 
> 
> It's both a property and a function, isn't it?

Sure. But it's still a property, and I wanted them all highlighted the same (or at least the ability to do so).

> 
> > The same
> > applies to c-ts-mode and go-ts-mode as well.
> 
> 
> Regarding c-ts-mode, it might be a simple oversight, given that
> constructs like
> 
> p->handler (it)
> 
> 
> do not come up very often. But if we take js-ts-mode, for example, where
> a property is often a function, the property name is highlighted as a
> function in a funcall.

Maybe in c-ts-mode it's not common, but in c++-ts-mode it is and it shares those rules.

> 
> And speaking of c-ts-mode, this can fix that omission:
> 
> diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
> index 5093c3980b6..3740130be30 100644
> --- a/lisp/progmodes/c-ts-mode.el
> +++ b/lisp/progmodes/c-ts-mode.el
> @@ -470,7 +470,9 @@ c-ts-mode--font-lock-settings
> :language mode
> :feature 'function
> '((call_expression
> - function: (identifier) @font-lock-function-name-face))
> + function:
> + [(identifier) @font-lock-function-name-face
> + (field_expression field: (field_identifier)
> @font-lock-function-name-face)]))
> 
> :language mode
> :feature 'variable

Haven't tested it, but the patch looks good to me, thanks.

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Dmitry Gutov]

On 06/02/2023 04:06, Randy Taylor wrote:
> On Sunday, February 5th, 2023 at 16:56, Dmitry Gutov <dgutov@yandex.ru> wrote:
>> Hi Randy,
>>
>> Maybe I was too quick to commit the change. But let's discuss it.
> 
> No, it's fine. You can also see my reply to Jostein.
> 
> BTW thanks for your work on the other modes and sorry for my lack of replies, I've been busy as of late.

It's all right. I've kind of gotten into the "flow" in the last few 
days, so it seems easy to sprinkle minor improvements like that here and 
there. 'M-x treesit-explorer-mode' helps a lot.

>>
>> On 05/02/2023 23:30, Randy Taylor wrote:
>>
>>> That's expected (at least to me) because it's a property.
>>
>>
>> It's both a property and a function, isn't it?
> 
> Sure. But it's still a property, and I wanted them all highlighted the same (or at least the ability to do so).
> 
>>
>>> The same
>>> applies to c-ts-mode and go-ts-mode as well.
>>
>>
>> Regarding c-ts-mode, it might be a simple oversight, given that
>> constructs like
>>
>> p->handler (it)
>>
>>
>> do not come up very often. But if we take js-ts-mode, for example, where
>> a property is often a function, the property name is highlighted as a
>> function in a funcall.
> 
> Maybe in c-ts-mode it's not common, but in c++-ts-mode it is and it shares those rules.

Sure.

>> And speaking of c-ts-mode, this can fix that omission:
>>
>> diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
>> index 5093c3980b6..3740130be30 100644
>> --- a/lisp/progmodes/c-ts-mode.el
>> +++ b/lisp/progmodes/c-ts-mode.el
>> @@ -470,7 +470,9 @@ c-ts-mode--font-lock-settings
>> :language mode
>> :feature 'function
>> '((call_expression
>> - function: (identifier) @font-lock-function-name-face))
>> + function:
>> + [(identifier) @font-lock-function-name-face
>> + (field_expression field: (field_identifier)
>> @font-lock-function-name-face)]))
>>
>> :language mode
>> :feature 'variable
> 
> Haven't tested it, but the patch looks good to me, thanks.

I've pushed it now, thanks for looking.

bug#61302: 29.0.60; rust-ts-mode does not show function-invocation on field-properties

[Dmitry Gutov]

Hi!

On 05/02/2023 22:15, Jostein Kjønigsen wrote:
> Steps to reproduce:
> 
> - set font-lock level 4 to enable highlighting of function-invocations
> - create a new buffer and activate rust-ts-mode
> 
> Try writing a function which invokes functions indirectly trough 
> field-properties. Example below:
> 
> pub fn should_handle(url: String) -> bool {
>      if url.ends_with(".css")
>         || url.ends_with(".js")
>         || url.ends_with(".png")
>         || url.ends_with(".jpg")
>      {
>          false
>      } else {
>          true
>      }
> }
> 
> Observe that:
> 
> - ends_with() is fontified as a property-access, not as a 
> function-invocation.
> - (plain function invocation is highlighted as expected though)
> 
>  From my preliminary inspection of the rust-ts-mode source-code, this 
> seems to be because of a very general override later in the file:
> 
> rust-ts-mode.el, line 248 or so:
> 
>     :language 'rust
>     :feature 'property
>     :override t
>     '((field_identifier) @font-lock-property-face
>       (shorthand_field_initializer (identifier) @font-lock-property-face))
> 
> Dissabling "override" for this feature fixes the fontification of method 
> invocations, but I haven't done enough testing to see if there are other 
> unexpected side-effects.

Thanks for the report. I've installed the patch below, commit a529b0d6463.

Note that I wouldn't recommend using the level 4 straight away, because 
of the 'variable' feature that's not very precise.

It's probably better to use level 3 and add the extra features you need 
using treesit-font-lock-recompute-features, or use level 4 and drop 
'variable'. But that's my opinion.

diff --git a/lisp/progmodes/rust-ts-mode.el b/lisp/progmodes/rust-ts-mode.el
index 18b42b9eced..5c71a8ad461 100644
--- a/lisp/progmodes/rust-ts-mode.el
+++ b/lisp/progmodes/rust-ts-mode.el
@@ -234,6 +234,11 @@ rust-ts-mode--font-lock-settings
       (use_as_clause alias: (identifier) @font-lock-type-face)
       (use_list (identifier) @font-lock-type-face))

+   :language 'rust
+   :feature 'property
+   '((field_identifier) @font-lock-property-face
+     (shorthand_field_initializer (identifier) @font-lock-property-face))
+
     :language 'rust
     :feature 'variable
     '((identifier) @font-lock-variable-name-face
@@ -245,12 +250,6 @@ rust-ts-mode--font-lock-settings
     :override t
     '((escape_sequence) @font-lock-escape-face)

-   :language 'rust
-   :feature 'property
-   :override t
-   '((field_identifier) @font-lock-property-face
-     (shorthand_field_initializer (identifier) @font-lock-property-face))
-
     :language 'rust
     :feature 'error
     :override t