bug#61053: 29.0.60; typescript-ts-mode does not correctly highlight function-valued variables. [PATCH]

bug#61053: 29.0.60; typescript-ts-mode does not correctly highlight function-valued variables. [PATCH]

[Jostein Kjønigsen]

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

When working with codebases where people define functions by assigned 
arrow-expressions to local variables, typescript-ts-mode (and 
tsx-ts-mode) currently does not highlight them as function declarations.

// this works
function demoFunction() {
}

// this doesnt
const demoFunction = () => {
};

We actually have a selector for this, but it is not getting triggered, 
because of what looks like ordering issues.

We also have override :t for almost every single feature in this mode, 
making it hard to know how selectors gets applied.

Attached is a patch whic:

1. reorders selectors to correctly highlight function-declaration 
(required change, but not sufficient)
2. disables override everywhere, except for declaration, in order to 
keep fontification correct.

So far I haven't been able to observe any ill side-effects from this 
change, but it might be worth double-checking.

--
Jostein

In GNU Emacs 29.0.60 (build 1, x86_64-pc-linux-gnu, GTK+ Version
  3.24.36, cairo version 1.17.6) of 2023-01-15 built on thinkpad-t14s
Repository revision: 59c3c53efa43e82f0f2e48a4c27d5bd623201d4a
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: TypeScript[TSX]

Minor modes in effect:
   electric-pair-mode: t
   global-git-commit-mode: t
   magit-auto-revert-mode: t
   highlight-symbol-mode: t
   flycheck-mode: t
   editorconfig-mode: t
   company-mode: t
   eglot--managed-mode: t
   flymake-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
   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
   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 emacsbug sort find-dired dired-aux pulse tabify cus-start
helm-command helm-elisp helm-eval edebug helm-info markdown-mode color
mail-extr json-ts-mode elec-pair typescript-ts-mode js c-ts-mode cc-mode
cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars
cc-defs treesit winner ffap tramp-archive tramp-gvfs tramp-cache
time-stamp zeroconf dbus vc-hg vc-bzr vc-src vc-sccs vc-cvs vc-rcs
log-view vc bug-reference flyspell ispell magit-extras magit-bookmark
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 transient magit-git
magit-base magit-section crm misearch multi-isearch face-remap vc-git
diff-mode vc-dispatcher conf-mode executable display-line-numbers
disp-table vc-svn bookmark 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:
nil

-- 
Vennlig hilsen
*Jostein Kjønigsen*

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

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

[-- Attachment #2: 0001-typescript-ts-mode-fix-fontification-of-function-val.patch --]
[-- Type: text/x-patch, Size: 4003 bytes --]

From e5de720c5635eeae4b1a3728d94eb4bdb60174d0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jostein=20Kj=C3=B8nigsen?= <jostein@kjonigsen.net>
Date: Wed, 25 Jan 2023 10:38:09 +0100
Subject: [PATCH] typescript-ts-mode: fix fontification of function-valued
 variables

- remove overrides all over the place... since that means we cant
  override the one place we actually need to!
---
 lisp/progmodes/typescript-ts-mode.el | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/lisp/progmodes/typescript-ts-mode.el b/lisp/progmodes/typescript-ts-mode.el
index 34030968806..25cc327d05f 100644
--- a/lisp/progmodes/typescript-ts-mode.el
+++ b/lisp/progmodes/typescript-ts-mode.el
@@ -132,26 +132,21 @@ typescript-ts-mode--font-lock-settings
 Argument LANGUAGE is either `typescript' or `tsx'."
   (treesit-font-lock-rules
    :language language
-   :override t
    :feature 'comment
    `((comment) @font-lock-comment-face)
 
    :language language
-   :override t
    :feature 'constant
    `(((identifier) @font-lock-constant-face
       (:match "^[A-Z_][A-Z_\\d]*$" @font-lock-constant-face))
-
      [(true) (false) (null)] @font-lock-constant-face)
 
    :language language
-   :override t
    :feature 'keyword
    `([,@typescript-ts-mode--keywords] @font-lock-keyword-face
      [(this) (super)] @font-lock-keyword-face)
 
    :language language
-   :override t
    :feature 'string
    `((regex pattern: (regex_pattern)) @font-lock-regexp-face
      (string) @font-lock-string-face
@@ -159,7 +154,7 @@ typescript-ts-mode--font-lock-settings
      (template_substitution ["${" "}"] @font-lock-misc-punctuation-face))
 
    :language language
-   :override t
+   :override t ;; for functions assigned to variables
    :feature 'declaration
    `((function
       name: (identifier) @font-lock-function-name-face)
@@ -174,6 +169,10 @@ typescript-ts-mode--font-lock-settings
      (required_parameter (identifier) @font-lock-variable-name-face)
      (optional_parameter (identifier) @font-lock-variable-name-face)
 
+     (variable_declarator
+      name: (identifier) @font-lock-function-name-face
+      value: [(function) (arrow_function)])
+
      (variable_declarator
       name: (identifier) @font-lock-variable-name-face)
 
@@ -188,10 +187,6 @@ typescript-ts-mode--font-lock-settings
      (arrow_function
       parameter: (identifier) @font-lock-variable-name-face)
 
-     (variable_declarator
-      name: (identifier) @font-lock-function-name-face
-      value: [(function) (arrow_function)])
-
      (variable_declarator
       name: (array_pattern
              (identifier)
@@ -205,7 +200,6 @@ typescript-ts-mode--font-lock-settings
      (import_clause (named_imports (import_specifier (identifier)) @font-lock-variable-name-face)))
 
    :language language
-   :override t
    :feature 'identifier
    `((nested_type_identifier
       module: (identifier) @font-lock-type-face)
@@ -234,7 +228,6 @@ typescript-ts-mode--font-lock-settings
        (_ (_ (_ (identifier) @font-lock-variable-name-face)))]))
 
    :language language
-   :override t
    :feature 'property
    `((property_signature
       name: (property_identifier) @font-lock-property-face)
@@ -249,7 +242,6 @@ typescript-ts-mode--font-lock-settings
       @font-lock-property-face))
 
    :language language
-   :override t
    :feature 'expression
    '((assignment_expression
       left: [(identifier) @font-lock-function-name-face
@@ -266,7 +258,6 @@ typescript-ts-mode--font-lock-settings
         property: (property_identifier) @font-lock-function-name-face)]))
 
    :language language
-   :override t
    :feature 'pattern
    `((pair_pattern
       key: (property_identifier) @font-lock-property-face)
@@ -274,7 +265,6 @@ typescript-ts-mode--font-lock-settings
      (array_pattern (identifier) @font-lock-variable-name-face))
 
    :language language
-   :override t
    :feature 'jsx
    `((jsx_opening_element
       [(nested_identifier (identifier)) (identifier)]
-- 
2.39.1

bug#61053: 29.0.60; typescript-ts-mode does not correctly highlight function-valued variables. [PATCH]

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

Jostein Kjønigsen <jostein@secure.kjonigsen.net> writes:

> When working with codebases where people define functions by assigned arrow-expressions to local variables,
> typescript-ts-mode (and tsx-ts-mode) currently does not highlight them as function declarations.
>
> // this works
> function demoFunction() {
> }
>
> // this doesnt
> const demoFunction = () => {
> };
>
> We actually have a selector for this, but it is not getting triggered, because of what looks like ordering issues.
>
> We also have override :t for almost every single feature in this mode, making it hard to know how selectors gets
> applied.
>
> Attached is a patch whic:
>
> 1. reorders selectors to correctly highlight function-declaration (required change, but not sufficient)
> 2. disables override everywhere, except for declaration, in order to keep fontification correct.
>
> So far I haven't been able to observe any ill side-effects from this change, but it might be worth double-checking.
>


Thanks, Jostein!  Looks like it works on my end aswell.

Applied and pushed, so closing this.  Keep them coming :)

Theo

bug#61053: 29.0.60; typescript-ts-mode does not correctly highlight function-valued variables. [PATCH]

[Eli Zaretskii]

> Cc: Theodor Thornhill <theo@thornhill.no>
> Date: Wed, 25 Jan 2023 10:41:23 +0100
> From: Jostein Kjønigsen <jostein@secure.kjonigsen.net>
> 
> When working with codebases where people define functions by assigned arrow-expressions to local
> variables, typescript-ts-mode (and tsx-ts-mode) currently does not highlight them as function declarations.
> 
> // this works
> function demoFunction() {
> }
> 
> // this doesnt
> const demoFunction = () => {
> };
> 
> We actually have a selector for this, but it is not getting triggered, because of what looks like ordering
> issues.
> 
> We also have override :t for almost every single feature in this mode, making it hard to know how selectors
> gets applied.
> 
> Attached is a patch whic:
> 
> 1. reorders selectors to correctly highlight function-declaration (required change, but not sufficient)
> 2. disables override everywhere, except for declaration, in order to keep fontification correct.

Thanks.

When fixing such "tricky" issues, please always include in the code
comments which explain the tricky stuff, in this case why some code
must be before the other.  Otherwise we run the risk that someone,
some day, will reorder the code, and we get the bug back.

Adding tests for this is even better (but doesn't make the comments
less important).