bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Denis Zubarev]

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

Tags: patch


Improve syntax highlighting for python-ts-mode.

- Fontify compound keywords "not in" and "is not" (fixes bug#67015)
- Fix fontification of strings inside of f-strings interpolation,
   e.g. for f"beg {'nested'}" - 'nested' was not fontified as string.
- Do not override the face of builtin functions (all, bytes etc.) with
  the function call face
- Add missing assignment expressions
- Fontify built-ins (dict,list,etc.) as types when they are used in type hints

I have added tests to treesit-tests.el, but I'm not sure that it is the
right place for them. I tried to create new file
python-ts-mode-tests.el, but when I run tests using make, I got
an error:
*** No rule to make target '../lisp/progmodes/python-ts-mode.el', needed by 'lisp/progmodes/python-ts-mode-tests.log'.  Stop.

It seems that python-ts-mode is required to be in its own file, but it
is defined in python.el.
Adding tests to python-tests.el seems like not good idea too, because
some CI code is searching for *-ts-mode-tests.el or treesit-tests.el files.


In GNU Emacs 30.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version
 3.24.33, cairo version 1.16.0) of 2023-11-11 built on NUC-here
Repository revision: 400a71b8f2c5a49dce4f542adfd2fdb59eb34243
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: Ubuntu 22.04.3 LTS

Configured using:
 'configure --with-modules --with-native-compilation=aot
 --with-imagemagick --with-json --with-tree-sitter --with-xft'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Improve-syntax-highlighting-for-python-ts-mode.patch --]
[-- Type: text/patch, Size: 11271 bytes --]

From 0b3165eb90bdc1b85688d3c9a1902c1f293972d1 Mon Sep 17 00:00:00 2001
From: Denis Zubarev <dvzubarev@yandex.ru>
Date: Sat, 11 Nov 2023 04:55:44 +0300
Subject: [PATCH] Improve syntax highlighting for python-ts-mode

* lisp/progmodes/python.el (python--treesit-keywords): Fontify compound
keywords "not in" and "is not" (bug#67015).
(python--treesit-fontify-string): Fix fontification of strings inside of
f-strings interpolation, e.g. for f"beg {'nested'}" - 'nested' was not
fontified as string.
(python--treesit-fontify-string-interpolation): Remove function.
(python--treesit-settings): Do not override the face of builtin
functions (all, bytes etc.) with the function call face.  Add missing
assignment expressions.  Fontify built-ins (dict,list,etc.) as types
when they are used in type hints.
* test/src/treesit-tests.el (python-ts-tests-with-temp-buffer):
(python-ts-mode-compound-keywords-face):
(python-ts-mode-named-assignement-face):
(python-ts-mode-nested-types-face):
(python-ts-mode-builtin-call-face):
(python-ts-mode-interpolation-nested-string):
(python-ts-mode-interpolation-doc-string): Add tests.
---
 lisp/progmodes/python.el  | 81 +++++++++++++++++-----------------
 test/src/treesit-tests.el | 93 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 134 insertions(+), 40 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index d3cb5a77e2..e3a4e0d2f9 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -979,7 +979,7 @@ python--treesit-keywords
     "raise" "return" "try" "while" "with" "yield"
     ;; These are technically operators, but we fontify them as
     ;; keywords.
-    "and" "in" "is" "not" "or"))
+    "and" "in" "is" "not" "or" "is not" "not in"))
 
 (defvar python--treesit-builtins
   '("abs" "all" "any" "ascii" "bin" "bool" "breakpoint" "bytearray"
@@ -1042,9 +1042,7 @@ python--treesit-fontify-string
 f-strings.  OVERRIDE is the override flag described in
 `treesit-font-lock-rules'.  START and END mark the region to be
 fontified."
-  (let* ((string-beg (treesit-node-start node))
-         (string-end (treesit-node-end node))
-         (maybe-expression (treesit-node-parent node))
+  (let* ((maybe-expression (treesit-node-parent node))
          (grandparent (treesit-node-parent
                        (treesit-node-parent
                         maybe-expression)))
@@ -1072,28 +1070,29 @@ python--treesit-fontify-string
                         (equal (treesit-node-type maybe-expression)
                                "expression_statement"))
                    'font-lock-doc-face
-                 'font-lock-string-face)))
-    ;; Don't highlight string prefixes like f/r/b.
-    (save-excursion
-      (goto-char string-beg)
-      (when (re-search-forward "[\"']" string-end t)
-        (setq string-beg (match-beginning 0))))
-    (treesit-fontify-with-override
-     string-beg string-end face override start end)))
+                 'font-lock-string-face))
+         ;; We will highlight only string_start/string_content/string_end children
+         ;; Do not touch interpolation node that can occur inside of the string
+         (string-nodes (treesit-filter-child
+                        node
+                        (lambda (ch) (member (treesit-node-type ch)
+                                             '("string_start"
+                                               "string_content"
+                                               "string_end")))
+                        t)))
+
+    (dolist (string-node string-nodes)
+      (let ((string-beg (treesit-node-start string-node))
+            (string-end (treesit-node-end string-node)))
+        (when (equal (treesit-node-type string-node) "string_start")
+          ;; Don't highlight string prefixes like f/r/b.
+          (save-excursion
+            (goto-char string-beg)
+            (when (re-search-forward "[\"']" string-end t)
+              (setq string-beg (match-beginning 0)))))
 
-(defun python--treesit-fontify-string-interpolation
-    (node _ start end &rest _)
-  "Fontify string interpolation.
-NODE is the string node.  Do not fontify the initial f for
-f-strings.  START and END mark the region to be
-fontified."
-  ;; This is kind of a hack, it basically removes the face applied by
-  ;; the string feature, so that following features can apply their
-  ;; face.
-  (let ((n-start (treesit-node-start node))
-        (n-end (treesit-node-end node)))
-    (remove-text-properties
-     (max start n-start) (min end n-end) '(face))))
+        (treesit-fontify-with-override
+         string-beg string-end face override start end)))))
 
 (defvar python--treesit-settings
   (treesit-font-lock-rules
@@ -1103,14 +1102,9 @@ python--treesit-settings
 
    :feature 'string
    :language 'python
-   '((string) @python--treesit-fontify-string)
+   '((string) @python--treesit-fontify-string
+     (interpolation ["{" "}"] @font-lock-misc-punctuation-face))
 
-   ;; HACK: This feature must come after the string feature and before
-   ;; other features.  Maybe we should make string-interpolation an
-   ;; option rather than a feature.
-   :feature 'string-interpolation
-   :language 'python
-   '((interpolation) @python--treesit-fontify-string-interpolation)
 
    :feature 'keyword
    :language 'python
@@ -1126,12 +1120,6 @@ python--treesit-settings
       name: (identifier) @font-lock-type-face)
      (parameters (identifier) @font-lock-variable-name-face))
 
-   :feature 'function
-   :language 'python
-   '((call function: (identifier) @font-lock-function-call-face)
-     (call function: (attribute
-                      attribute: (identifier) @font-lock-function-call-face)))
-
    :feature 'builtin
    :language 'python
    `(((identifier) @font-lock-builtin-face
@@ -1142,6 +1130,12 @@ python--treesit-settings
                       eol))
               @font-lock-builtin-face)))
 
+   :feature 'function
+   :language 'python
+   '((call function: (identifier) @font-lock-function-call-face)
+     (call function: (attribute
+                      attribute: (identifier) @font-lock-function-call-face)))
+
    :feature 'constant
    :language 'python
    '([(true) (false) (none)] @font-lock-constant-face)
@@ -1154,6 +1148,10 @@ python--treesit-settings
      (assignment left: (attribute
                         attribute: (identifier)
                         @font-lock-property-use-face))
+     (augmented_assignment left: (identifier)
+                           @font-lock-variable-name-face)
+     (named_expression name: (identifier)
+                       @font-lock-variable-name-face)
      (pattern_list (identifier)
                    @font-lock-variable-name-face)
      (tuple_pattern (identifier)
@@ -1168,15 +1166,18 @@ python--treesit-settings
    '((decorator "@" @font-lock-type-face)
      (decorator (call function: (identifier) @font-lock-type-face))
      (decorator (identifier) @font-lock-type-face))
-
    :feature 'type
    :language 'python
+   ;; override built-in faces when dict/list are used for type hints
+   :override t
    `(((identifier) @font-lock-type-face
       (:match ,(rx-to-string
                 `(seq bol (or ,@python--treesit-exceptions)
                       eol))
               @font-lock-type-face))
-     (type (identifier) @font-lock-type-face))
+     (type [(identifier) (none)] @font-lock-type-face)
+     (type
+      (_ [(identifier) (none)] @font-lock-type-face)))
 
    :feature 'escape-sequence
    :language 'python
diff --git a/test/src/treesit-tests.el b/test/src/treesit-tests.el
index 791e902bd0..71c7b92957 100644
--- a/test/src/treesit-tests.el
+++ b/test/src/treesit-tests.el
@@ -1167,6 +1167,99 @@ treesit-defun-navigation-top-level
    treesit--ert-defun-navigation-top-level-master
    'top-level))
 
+;;; python-ts-mode font-lock tests
+
+(defmacro python-ts-tests-with-temp-buffer (contents &rest body)
+  "Create a `python-ts-mode' enabled temp buffer with CONTENTS.
+BODY is code to be executed within the temp buffer.  Point is
+always located at the beginning of buffer."
+  (declare (indent 1) (debug t))
+  `(with-temp-buffer
+     (skip-unless (treesit-language-available-p 'python))
+     (require 'python)
+     (let ((python-indent-guess-indent-offset nil))
+       (python-ts-mode)
+       (insert ,contents)
+       (goto-char (point-min))
+       ,@body)))
+
+(ert-deftest python-ts-mode-compound-keywords-face ()
+  (dolist (test '("is not" "not in"))
+    (python-ts-tests-with-temp-buffer (concat "t " test " t")
+      (font-lock-ensure)
+      (forward-to-word)
+      (should (eq (face-at-point) font-lock-keyword-face))
+      (forward-to-word)
+      (should (eq (face-at-point) font-lock-keyword-face)))))
+
+(ert-deftest python-ts-mode-named-assignement-face ()
+  (python-ts-tests-with-temp-buffer "var := 3"
+    (font-lock-ensure)
+    (should (eq (face-at-point) font-lock-variable-name-face))))
+
+(ert-deftest python-ts-mode-nested-types-face ()
+  (python-ts-tests-with-temp-buffer "def func(v:dict[ list[ tuple[str] ], int | None] | None):"
+    (font-lock-ensure)
+    (dolist (test '("dict" "list" "tuple" "str" "int" "None" "None"))
+      (search-forward test)
+      (goto-char (match-beginning 0))
+      (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-builtin-call-face ()
+  (python-ts-tests-with-temp-buffer "all()"
+    ;; enable 'function' feature from 4th level
+    (setopt treesit-font-lock-level 4)
+    (font-lock-ensure)
+    (should (eq (face-at-point) font-lock-builtin-face))))
+
+(ert-deftest python-ts-mode-interpolation-nested-string ()
+  (python-ts-tests-with-temp-buffer "t = f\"beg {var + 'string'}\""
+    (font-lock-ensure)
+
+    (search-forward "var")
+    (goto-char (match-beginning 0))
+    (should (eq (face-at-point) font-lock-variable-name-face))
+
+    (goto-char (point-min))
+    (dolist (test '("f" "{" "+" "}"))
+      (search-forward test)
+      (goto-char (match-beginning 0))
+      (should (not (eq (face-at-point) font-lock-string-face))))
+
+
+    (goto-char (point-min))
+    (dolist (test '("beg" "'string'" "\""))
+      (search-forward test)
+      (goto-char (match-beginning 0))
+      (should (eq (face-at-point) font-lock-string-face)))))
+
+(ert-deftest python-ts-mode-interpolation-doc-string ()
+  (python-ts-tests-with-temp-buffer "f\"\"\"beg {'s1' + var + 's2'} end\"\"\""
+    (font-lock-ensure)
+
+    (search-forward "var")
+    (goto-char (match-beginning 0))
+    (should (eq (face-at-point) font-lock-variable-name-face))
+
+    (goto-char (point-min))
+    (dolist (test '("f" "{" "+" "}"))
+      (search-forward test)
+      (goto-char (match-beginning 0))
+      (should (not (eq (face-at-point) font-lock-string-face))))
+
+    (goto-char (point-min))
+    (dolist (test '("\"\"\"" "beg" "end" "\"\"\""))
+      (search-forward test)
+      (goto-char (match-beginning 0))
+      (should (eq (face-at-point) font-lock-doc-face)))
+
+    (goto-char (point-min))
+    (dolist (test '("'s1'" "'s2'"))
+      (search-forward test)
+      (goto-char (match-beginning 0))
+      (should (eq (face-at-point) font-lock-string-face)))))
+
+
 ;; TODO
 ;; - Functions in treesit.el
 ;; - treesit-load-name-override-list
-- 
2.34.1

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Eli Zaretskii]

> From: Denis Zubarev <dvzubarev@yandex.ru>
> Date: Sat, 11 Nov 2023 05:21:25 +0300
> 
> Improve syntax highlighting for python-ts-mode.
> 
> - Fontify compound keywords "not in" and "is not" (fixes bug#67015)
> - Fix fontification of strings inside of f-strings interpolation,
>    e.g. for f"beg {'nested'}" - 'nested' was not fontified as string.
> - Do not override the face of builtin functions (all, bytes etc.) with
>   the function call face
> - Add missing assignment expressions
> - Fontify built-ins (dict,list,etc.) as types when they are used in type hints

Yuan, any comments?

My only comment is that we should decide in which fontification
level(s) should the additional fontifications be.

> I have added tests to treesit-tests.el, but I'm not sure that it is the
> right place for them. I tried to create new file
> python-ts-mode-tests.el, but when I run tests using make, I got
> an error:
> *** No rule to make target '../lisp/progmodes/python-ts-mode.el', needed by 'lisp/progmodes/python-ts-mode-tests.log'.  Stop.
> 
> It seems that python-ts-mode is required to be in its own file, but it
> is defined in python.el.
> Adding tests to python-tests.el seems like not good idea too, because
> some CI code is searching for *-ts-mode-tests.el or treesit-tests.el files.

The tests should be in python-tests.el, and should be skipped if
(treesit-ready-p 'python t) returns nil.

What problems with CI did you see?  I don't think I understand your
last sentence above.

> >From 0b3165eb90bdc1b85688d3c9a1902c1f293972d1 Mon Sep 17 00:00:00 2001
> From: Denis Zubarev <dvzubarev@yandex.ru>
> Date: Sat, 11 Nov 2023 04:55:44 +0300
> Subject: [PATCH] Improve syntax highlighting for python-ts-mode

This submission is too large for us to accept without a copyright
assignment.  Would you like to start your assignment copyright
paperwork rolling at this time?  If yes, I will send you the form to
fill and the instructions to email it.

Thanks.

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Denis Zubarev]

[-- Attachment #1: Type: text/html, Size: 3583 bytes --]

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Eli Zaretskii]

> From: Denis Zubarev <dvzubarev@yandex.ru>
> Cc: "67061@debbugs.gnu.org" <67061@debbugs.gnu.org>
> Date: Sat, 11 Nov 2023 13:52:45 +0300
> 
>  My only comment is that we should decide in which fontification
>  level(s) should the additional fontifications be.
> 
> I didn't add any new features, just tweak the old ones to be more correct.

I thought at least some of these are new:

> > - Fontify compound keywords "not in" and "is not" (fixes bug#67015)
> > - Fix fontification of strings inside of f-strings interpolation,
> >    e.g. for f"beg {'nested'}" - 'nested' was not fontified as string.
> > - Do not override the face of builtin functions (all, bytes etc.) with
> >   the function call face
> > - Add missing assignment expressions
> > - Fontify built-ins (dict,list,etc.) as types when they are used in type hints

For example, isn't the first item an addition?  And what about
"missing assignment expressions"?

>  What problems with CI did you see? I don't think I understand your
>  last sentence above.
> 
> I've grepped the codebase and found those lines:
>  
> TREE-SITTER-FILES ?= $(shell cd .. ; \
>   find lisp src \( -name "*-ts-mode-tests.el" -o -name "treesit-tests.el" \) | \
>   sort | sed s/\\.el/.log/)
>  
> in test/infra/Makefile.in
> I thought that tree-sitter tests  should go to either *-ts-mode-tests.el or treesit-tests.el files.

No, I don't think so.  If having them in python-tests.el triggers some
problems, please show them, and let's take it from there.

>  Would you like to start your assignment copyright
>  paperwork rolling at this time? If yes, I will send you the form to
>  fill and the instructions to email it.
> 
> Yes, please.

Thanks, form sent off-list.

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Denis Zubarev]

[-- Attachment #1: Type: text/html, Size: 2946 bytes --]

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Dmitry Gutov]

On 11/11/2023 13:00, Eli Zaretskii wrote:
>>   What problems with CI did you see? I don't think I understand your
>>   last sentence above.
>>
>> I've grepped the codebase and found those lines:
>>   
>> TREE-SITTER-FILES ?= $(shell cd .. ; \
>>    find lisp src \( -name "*-ts-mode-tests.el" -o -name "treesit-tests.el" \) | \
>>    sort | sed s/\\.el/.log/)
>>   
>> in test/infra/Makefile.in
>> I thought that tree-sitter tests  should go to either *-ts-mode-tests.el or treesit-tests.el files.
> No, I don't think so.  If having them in python-tests.el triggers some
> problems, please show them, and let's take it from there.

I'm guessing the main problem with that is that the test-tree-sitter job 
on EMBA won't be picking those tests up.

Which is unfortunate, but not at all a deal-breaker (but if we could 
make the build of python-ts-mode-tests.log load python.el, that would be 
even better).

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Eli Zaretskii]

Ping!

> Cc: 67061@debbugs.gnu.org
> Date: Sat, 11 Nov 2023 09:32:48 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > From: Denis Zubarev <dvzubarev@yandex.ru>
> > Date: Sat, 11 Nov 2023 05:21:25 +0300
> > 
> > Improve syntax highlighting for python-ts-mode.
> > 
> > - Fontify compound keywords "not in" and "is not" (fixes bug#67015)
> > - Fix fontification of strings inside of f-strings interpolation,
> >    e.g. for f"beg {'nested'}" - 'nested' was not fontified as string.
> > - Do not override the face of builtin functions (all, bytes etc.) with
> >   the function call face
> > - Add missing assignment expressions
> > - Fontify built-ins (dict,list,etc.) as types when they are used in type hints
> 
> Yuan, any comments?

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Eli Zaretskii]

Ping! Ping!  Yuan, could you please chime in?  We are waiting for
Denis's legal paperwork to come through, but meanwhile I'd like your
opinions on the changes he proposed, and also on whether this should
be installed on the emacs-29 branch or on master.

> Cc: dvzubarev@yandex.ru, 67061@debbugs.gnu.org
> Date: Wed, 15 Nov 2023 15:28:33 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> Ping!
> 
> > Cc: 67061@debbugs.gnu.org
> > Date: Sat, 11 Nov 2023 09:32:48 +0200
> > From: Eli Zaretskii <eliz@gnu.org>
> > 
> > > From: Denis Zubarev <dvzubarev@yandex.ru>
> > > Date: Sat, 11 Nov 2023 05:21:25 +0300
> > > 
> > > Improve syntax highlighting for python-ts-mode.
> > > 
> > > - Fontify compound keywords "not in" and "is not" (fixes bug#67015)
> > > - Fix fontification of strings inside of f-strings interpolation,
> > >    e.g. for f"beg {'nested'}" - 'nested' was not fontified as string.
> > > - Do not override the face of builtin functions (all, bytes etc.) with
> > >   the function call face
> > > - Add missing assignment expressions
> > > - Fontify built-ins (dict,list,etc.) as types when they are used in type hints
> > 
> > Yuan, any comments?
> 
> 
> 
> 

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Dmitry Gutov]

On 25/11/2023 11:35, Eli Zaretskii wrote:
> Ping! Ping!  Yuan, could you please chime in?  We are waiting for
> Denis's legal paperwork to come through, but meanwhile I'd like your
> opinions on the changes he proposed, and also on whether this should
> be installed on the emacs-29 branch or on master.

FWIW, the changes and the explanations look reasonable to me. The 
presence of tests is a good sign too (though they'll probably need to 
move to another file).

The fixes are relevant to Emacs 29 for sure.

I don't know at what stage we're going to start worrying when adding new 
elements to the queries, though, in fear of breaking compatibility with 
some potential older version of the grammar.

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Eli Zaretskii]

> Date: Sun, 26 Nov 2023 04:17:59 +0200
> Cc: dvzubarev@yandex.ru, 67061@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 25/11/2023 11:35, Eli Zaretskii wrote:
> > Ping! Ping!  Yuan, could you please chime in?  We are waiting for
> > Denis's legal paperwork to come through, but meanwhile I'd like your
> > opinions on the changes he proposed, and also on whether this should
> > be installed on the emacs-29 branch or on master.
> 
> FWIW, the changes and the explanations look reasonable to me. The 
> presence of tests is a good sign too (though they'll probably need to 
> move to another file).
> 
> The fixes are relevant to Emacs 29 for sure.
> 
> I don't know at what stage we're going to start worrying when adding new 
> elements to the queries, though, in fear of breaking compatibility with 
> some potential older version of the grammar.

Thanks.

Yuan, please chime in, I'm waiting for your response.

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Denis Zubarev]

[-- Attachment #1: Type: text/html, Size: 3004 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-Improve-syntax-highlighting-for-python-ts-mode.patch --]
[-- Type: text/x-diff; name="0002-Improve-syntax-highlighting-for-python-ts-mode.patch", Size: 13514 bytes --]

From 43422e59d83e1ae4c842d51ebd065e1315c53cfc Mon Sep 17 00:00:00 2001
From: Denis Zubarev <dvzubarev@yandex.ru>
Date: Sat, 11 Nov 2023 04:55:44 +0300
Subject: [PATCH] Improve syntax highlighting for python-ts-mode

* lisp/progmodes/python.el (python--treesit-keywords): Fontify compound
keyword  "is not".
(python--treesit-fontify-string): Fix fontification of strings inside of
f-strings interpolation, e.g. for f"beg {'nested'}" - 'nested' was not
fontified as string.
(python--treesit-fontify-string-interpolation): Remove function.
(python--treesit-settings): Do not override the face of builtin
functions (all, bytes etc.) with the function call face.  Add missing
assignment expressions.  Fontify built-ins (dict,list,etc.) as types
when they are used in type hints.
* test/lisp/progmodes/python-tests.el (python-ts-tests-with-temp-buffer):
(python-ts-mode-compound-keywords-face):
(python-ts-mode-named-assignement-face-1):
(python-ts-mode-named-assignement-face-2):
(python-ts-mode-nested-types-face-1):
(python-ts-mode-nested-types-face-2):
(python-ts-mode-builtin-call-face):
(python-ts-mode-interpolation-nested-string):
(python-ts-mode-interpolation-doc-string): Add tests.
---
 lisp/progmodes/python.el            |  94 +++++++++++++----------
 test/lisp/progmodes/python-tests.el | 114 ++++++++++++++++++++++++++++
 2 files changed, 166 insertions(+), 42 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index ab3bf1b4ec..8a178d9c7e 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -979,7 +979,7 @@ python--treesit-keywords
     "raise" "return" "try" "while" "with" "yield"
     ;; These are technically operators, but we fontify them as
     ;; keywords.
-    "and" "in" "is" "not" "or" "not in"))
+    "and" "in" "is" "not" "or" "not in" "is not"))
 
 (defvar python--treesit-builtins
   '("abs" "all" "any" "ascii" "bin" "bool" "breakpoint" "bytearray"
@@ -1042,9 +1042,7 @@ python--treesit-fontify-string
 f-strings.  OVERRIDE is the override flag described in
 `treesit-font-lock-rules'.  START and END mark the region to be
 fontified."
-  (let* ((string-beg (treesit-node-start node))
-         (string-end (treesit-node-end node))
-         (maybe-expression (treesit-node-parent node))
+  (let* ((maybe-expression (treesit-node-parent node))
          (grandparent (treesit-node-parent
                        (treesit-node-parent
                         maybe-expression)))
@@ -1072,28 +1070,29 @@ python--treesit-fontify-string
                         (equal (treesit-node-type maybe-expression)
                                "expression_statement"))
                    'font-lock-doc-face
-                 'font-lock-string-face)))
-    ;; Don't highlight string prefixes like f/r/b.
-    (save-excursion
-      (goto-char string-beg)
-      (when (re-search-forward "[\"']" string-end t)
-        (setq string-beg (match-beginning 0))))
-    (treesit-fontify-with-override
-     string-beg string-end face override start end)))
+                 'font-lock-string-face))
+         ;; We will highlight only string_start/string_content/string_end children
+         ;; Do not touch interpolation node that can occur inside of the string
+         (string-nodes (treesit-filter-child
+                        node
+                        (lambda (ch) (member (treesit-node-type ch)
+                                             '("string_start"
+                                               "string_content"
+                                               "string_end")))
+                        t)))
+
+    (dolist (string-node string-nodes)
+      (let ((string-beg (treesit-node-start string-node))
+            (string-end (treesit-node-end string-node)))
+        (when (equal (treesit-node-type string-node) "string_start")
+          ;; Don't highlight string prefixes like f/r/b.
+          (save-excursion
+            (goto-char string-beg)
+            (when (re-search-forward "[\"']" string-end t)
+              (setq string-beg (match-beginning 0)))))
 
-(defun python--treesit-fontify-string-interpolation
-    (node _ start end &rest _)
-  "Fontify string interpolation.
-NODE is the string node.  Do not fontify the initial f for
-f-strings.  START and END mark the region to be
-fontified."
-  ;; This is kind of a hack, it basically removes the face applied by
-  ;; the string feature, so that following features can apply their
-  ;; face.
-  (let ((n-start (treesit-node-start node))
-        (n-end (treesit-node-end node)))
-    (remove-text-properties
-     (max start n-start) (min end n-end) '(face))))
+        (treesit-fontify-with-override
+         string-beg string-end face override start end)))))
 
 (defvar python--treesit-settings
   (treesit-font-lock-rules
@@ -1103,14 +1102,9 @@ python--treesit-settings
 
    :feature 'string
    :language 'python
-   '((string) @python--treesit-fontify-string)
+   '((string) @python--treesit-fontify-string
+     (interpolation ["{" "}"] @font-lock-misc-punctuation-face))
 
-   ;; HACK: This feature must come after the string feature and before
-   ;; other features.  Maybe we should make string-interpolation an
-   ;; option rather than a feature.
-   :feature 'string-interpolation
-   :language 'python
-   '((interpolation) @python--treesit-fontify-string-interpolation)
 
    :feature 'keyword
    :language 'python
@@ -1126,12 +1120,6 @@ python--treesit-settings
       name: (identifier) @font-lock-type-face)
      (parameters (identifier) @font-lock-variable-name-face))
 
-   :feature 'function
-   :language 'python
-   '((call function: (identifier) @font-lock-function-call-face)
-     (call function: (attribute
-                      attribute: (identifier) @font-lock-function-call-face)))
-
    :feature 'builtin
    :language 'python
    `(((identifier) @font-lock-builtin-face
@@ -1142,6 +1130,12 @@ python--treesit-settings
                       eol))
               @font-lock-builtin-face)))
 
+   :feature 'function
+   :language 'python
+   '((call function: (identifier) @font-lock-function-call-face)
+     (call function: (attribute
+                      attribute: (identifier) @font-lock-function-call-face)))
+
    :feature 'constant
    :language 'python
    '([(true) (false) (none)] @font-lock-constant-face)
@@ -1151,9 +1145,20 @@ python--treesit-settings
    `(;; Variable names and LHS.
      (assignment left: (identifier)
                  @font-lock-variable-name-face)
+     (assignment left: (subscript value: (identifier)
+                                  @font-lock-variable-name-face))
      (assignment left: (attribute
                         attribute: (identifier)
-                        @font-lock-property-use-face))
+                        @font-lock-variable-name-face))
+     (assignment left: (subscript (attribute
+                        attribute: (identifier)
+                        @font-lock-variable-name-face)))
+     (augmented_assignment left: (identifier)
+                           @font-lock-variable-name-face)
+     (named_expression name: (identifier)
+                       @font-lock-variable-name-face)
+     (for_statement left: (identifier)
+                    @font-lock-variable-name-face)
      (pattern_list (identifier)
                    @font-lock-variable-name-face)
      (tuple_pattern (identifier)
@@ -1168,15 +1173,20 @@ python--treesit-settings
    '((decorator "@" @font-lock-type-face)
      (decorator (call function: (identifier) @font-lock-type-face))
      (decorator (identifier) @font-lock-type-face))
-
    :feature 'type
    :language 'python
+   ;; override built-in faces when dict/list are used for type hints
+   :override t
    `(((identifier) @font-lock-type-face
       (:match ,(rx-to-string
                 `(seq bol (or ,@python--treesit-exceptions)
                       eol))
               @font-lock-type-face))
-     (type (identifier) @font-lock-type-face))
+     (type [(identifier) (none)] @font-lock-type-face)
+     (type
+      (_ [(identifier) (none)] @font-lock-type-face
+         (_ [(identifier) (none)] @font-lock-type-face
+            (_ [(identifier) (none)] @font-lock-type-face) :?) :?)))
 
    :feature 'escape-sequence
    :language 'python
@@ -6842,7 +6852,7 @@ python-ts-mode
                 '(( comment definition)
                   ( keyword string type)
                   ( assignment builtin constant decorator
-                    escape-sequence number string-interpolation )
+                    escape-sequence number )
                   ( bracket delimiter function operator variable property)))
     (setq-local treesit-font-lock-settings python--treesit-settings)
     (setq-local imenu-create-index-function
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index a44a11896f..f8e1d8d07a 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -7299,6 +7299,120 @@ python-tests--flymake-command-output-pattern
                          "Unused import a.b.c (unused-import)"
                        "W0611: Unused import a.b.c (unused-import)"))))))
 
+;;; python-ts-mode font-lock tests
+
+(defmacro python-ts-tests-with-temp-buffer (contents &rest body)
+  "Create a `python-ts-mode' enabled temp buffer with CONTENTS.
+BODY is code to be executed within the temp buffer.  Point is
+always located at the beginning of buffer."
+  (declare (indent 1) (debug t))
+  `(with-temp-buffer
+     (skip-unless (treesit-ready-p 'python))
+     (require 'python)
+     (let ((python-indent-guess-indent-offset nil))
+       (python-ts-mode)
+       (insert ,contents)
+       (goto-char (point-min))
+       ,@body)))
+
+(ert-deftest python-ts-mode-compound-keywords-face ()
+  (dolist (test '("is not" "not in"))
+    (python-ts-tests-with-temp-buffer (concat "t " test " t")
+      (font-lock-ensure)
+      (forward-to-word)
+      (should (eq (face-at-point) font-lock-keyword-face))
+      (forward-to-word)
+      (should (eq (face-at-point) font-lock-keyword-face)))))
+
+(ert-deftest python-ts-mode-named-assignement-face-1 ()
+  (python-ts-tests-with-temp-buffer "var := 3"
+    (font-lock-ensure)
+    (should (eq (face-at-point) font-lock-variable-name-face))))
+
+(ert-deftest python-ts-mode-named-assignement-face-2 ()
+  (python-ts-tests-with-temp-buffer "var1[ii] = 1; t.var2[jj] = 2"
+    (setopt treesit-font-lock-level 3)
+    (font-lock-ensure)
+    (dolist (test '("var1"  "var2"))
+      (search-forward test)
+      (goto-char (match-beginning 0))
+      (should (eq (face-at-point) font-lock-variable-name-face)))
+    (goto-char (point-min))
+    (dolist (test '("ii" "jj"))
+      (search-forward test)
+      (goto-char (match-beginning 0))
+      (should (eq (face-at-point) nil)))))
+
+(ert-deftest python-ts-mode-nested-types-face-1 ()
+  (python-ts-tests-with-temp-buffer "def func(v:dict[ list[ tuple[str] ], int | None] | None):"
+    (font-lock-ensure)
+    (dolist (test '("dict" "list" "tuple" "str" "int" "None" "None"))
+      (search-forward test)
+      (goto-char (match-beginning 0))
+      (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-nested-types-face-2 ()
+  (python-ts-tests-with-temp-buffer "def f(val: tuple[tuple, list[Lvl1 | Lvl2[Lvl3[Lvl3], Lvl2]]]):"
+    (font-lock-ensure)
+    (dolist (test '("tuple" "tuple" "list" "Lvl1" "Lvl2" "Lvl3" "Lvl3" "Lvl2"))
+      (search-forward test)
+      (goto-char (match-beginning 0))
+      (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-builtin-call-face ()
+  (python-ts-tests-with-temp-buffer "all()"
+    ;; enable 'function' feature from 4th level
+    (setopt treesit-font-lock-level 4)
+    (font-lock-ensure)
+    (should (eq (face-at-point) font-lock-builtin-face))))
+
+(ert-deftest python-ts-mode-interpolation-nested-string ()
+  (python-ts-tests-with-temp-buffer "t = f\"beg {True + 'string'}\""
+    (font-lock-ensure)
+
+    (search-forward "True")
+    (goto-char (match-beginning 0))
+    (should (eq (face-at-point) font-lock-constant-face))
+
+    (goto-char (point-min))
+    (dolist (test '("f" "{" "+" "}"))
+      (search-forward test)
+      (goto-char (match-beginning 0))
+      (should (not (eq (face-at-point) font-lock-string-face))))
+
+
+    (goto-char (point-min))
+    (dolist (test '("beg" "'string'" "\""))
+      (search-forward test)
+      (goto-char (match-beginning 0))
+      (should (eq (face-at-point) font-lock-string-face)))))
+
+(ert-deftest python-ts-mode-interpolation-doc-string ()
+  (python-ts-tests-with-temp-buffer "f\"\"\"beg {'s1' + True + 's2'} end\"\"\""
+    (font-lock-ensure)
+
+    (search-forward "True")
+    (goto-char (match-beginning 0))
+    (should (eq (face-at-point) font-lock-constant-face))
+
+    (goto-char (point-min))
+    (dolist (test '("f" "{" "+" "}"))
+      (search-forward test)
+      (goto-char (match-beginning 0))
+      (should (not (eq (face-at-point) font-lock-string-face))))
+
+    (goto-char (point-min))
+    (dolist (test '("\"\"\"" "beg" "end" "\"\"\""))
+      (search-forward test)
+      (goto-char (match-beginning 0))
+      (should (eq (face-at-point) font-lock-doc-face)))
+
+    (goto-char (point-min))
+    (dolist (test '("'s1'" "'s2'"))
+      (search-forward test)
+      (goto-char (match-beginning 0))
+      (should (eq (face-at-point) font-lock-string-face)))))
+
 (provide 'python-tests)
 
 ;;; python-tests.el ends here
-- 
2.34.1

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Eli Zaretskii]

> From: Denis Zubarev <dvzubarev@yandex.ru>
> Cc: "casouri@gmail.com" <casouri@gmail.com>,
> 	"67061@debbugs.gnu.org" <67061@debbugs.gnu.org>
> Date: Sat, 09 Dec 2023 03:39:39 +0300
> 
> I've moved tests to python-tests.el and added another fixes:
>  
> assignment feature:
> `for var in range(3)` highlight var as font-lock-variable-name-face
> `var1[ii] = 1; t.var2[jj] = 2` highlight var1, var2 as font-lock-variable-name-face
>  
> type feature:
> support nested optional types up to 3 level deep, for example `tuple[tuple, list[Lvl1 | Lvl2[Lvl3[Lvl3],
> Lvl2]]]`
>  
>  
> Summary of all changes in the patch:
>  
> keyword feature:
> Add "is not"  to the `python--treesit-keywords` list.
>  
> assignment feature:
> For all examples,
> `for var in range(3)`
> `var1[ii] = 1; t.var2[jj] = 2`
> `var := 3`
> `var *= 3`
> highlight var as font-lock-variable-name-face
>  
> string feature:
> Fix fontification of strings inside of f-strings interpolation,
> e.g. for f"beg {'nested'}" - 'nested' was not fontified as string.
>  
> function feature:
> Do not override the face of builtin functions (all, bytes etc.) with
> the function call face
>  
> type feature:
> Fontify built-ins (dict,list,etc.) as types when they are used in type hints.
> E.g. def func(v:dict[ list[ tuple[str] ], int | None] | None):

Thanks.

Yuan, would you please chime in and provide your comments, if any?

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Yuan Fu]

On 12/8/23 11:32 PM, Eli Zaretskii wrote:
>> From: Denis Zubarev <dvzubarev@yandex.ru>
>> Cc: "casouri@gmail.com" <casouri@gmail.com>,
>> 	"67061@debbugs.gnu.org" <67061@debbugs.gnu.org>
>> Date: Sat, 09 Dec 2023 03:39:39 +0300
>>
>> I've moved tests to python-tests.el and added another fixes:
>>   
>> assignment feature:
>> `for var in range(3)` highlight var as font-lock-variable-name-face
>> `var1[ii] = 1; t.var2[jj] = 2` highlight var1, var2 as font-lock-variable-name-face
>>   
>> type feature:
>> support nested optional types up to 3 level deep, for example `tuple[tuple, list[Lvl1 | Lvl2[Lvl3[Lvl3],
>> Lvl2]]]`
>>   
>>   
>> Summary of all changes in the patch:
>>   
>> keyword feature:
>> Add "is not"  to the `python--treesit-keywords` list.
>>   
>> assignment feature:
>> For all examples,
>> `for var in range(3)`
>> `var1[ii] = 1; t.var2[jj] = 2`
>> `var := 3`
>> `var *= 3`
>> highlight var as font-lock-variable-name-face
>>   
>> string feature:
>> Fix fontification of strings inside of f-strings interpolation,
>> e.g. for f"beg {'nested'}" - 'nested' was not fontified as string.
>>   
>> function feature:
>> Do not override the face of builtin functions (all, bytes etc.) with
>> the function call face
>>   
>> type feature:
>> Fontify built-ins (dict,list,etc.) as types when they are used in type hints.
>> E.g. def func(v:dict[ list[ tuple[str] ], int | None] | None):
> Thanks.
>
> Yuan, would you please chime in and provide your comments, if any?

This should be the last bug report that I missed. I'll look at this 
tomorrow, promise promise.

Yuan

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Dmitry Gutov]

On 09/12/2023 02:39, Denis Zubarev wrote:
> assignment feature:
> For all examples,
> `for var in range(3)`
> `var1[ii] = 1; t.var2[jj] = 2`
> `var := 3`
> `var *= 3`
> highlight var as font-lock-variable-name-face

Arguably, the last 2 lines are "variable references" rather than 
definitions (so font-lock-variable-use-face might make more sense), 
since such operators imply that a variable has already been defined 
previously.

And python--treesit-fontify-variable (in the 'variable' feature) already 
applies that face.

The first example, however, should indeed use font-lock-variable-name-face.

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Denis Zubarev]

[-- Attachment #1: Type: text/html, Size: 2051 bytes --]

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Dmitry Gutov]

On 10/12/2023 14:04, Denis Zubarev wrote:
>  > Arguably, the last 2 lines are "variable references" rather than 
> definitions
> `var := 3`  is assignment expressions. It allows variable assignments to 
> occur inside of larger expressions. For example `if (match := 
> pattern.search(data)) is not None:`.
> It mostly used to define new variables and act on them if some condition 
> is met.

Ah, thanks! This feature is newer than my working experience with Python.

> My rationale for `var *= 3` was that it is shorthand for `var = var * 3` 
> and currently the `var` on the left hand side is fontified with 
> `font-lock-variable-name-face`.

I think ideally, in cases when "var =" is not the first occurrence for 
the same var in a given scope, we wouldn't highlight it as "definition" 
either.

Speaking of font-lock-variable-use-face, I think it would be most useful 
if it helped with noticing typos (meaning, it would only be used for 
references to variables that have been defined previously, according to 
the rules of the language). But for that we still need to implement the 
scope tracking. And before that, well, my personal preference is not to 
highlight the references at all, but opinions differ.

> I wanted shorthand form to be consistent with the full form.
> Your point makes sense too, I don't have strong opinion about this.
> Also I'm not sure now about `var[ii] = 1`, since it is actually 
> accessing the list or dictionary element and 
> `font-lock-variable-use-face` may suit better here.

Yep. To sum up, I would add highlighting to your examples `for var in 
range(3)` and `var := 3` but not others.

> Question about new changes.
> Should I push them to this patch and provide description of new changes,
> or it would be better to wait for review and send them as new patch?

I suggest sending an updated patch for review in this case, but you can 
also wait for Yuan's comments first.

Also, please double-check that the tests pass. It might be something in 
the way I applied your latest patch, but in the example that you gave 
regarding the "type" feature,

   def func(v:dict[ list[ tuple[str] ], int | None] | None):

, only the last "None" gets highlighted for me with font-lock-type-face. 
There are corresponding test failures too. Again, could be something on 
my side, but it would help to verify.

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Yuan Fu]

On 12/10/23 4:00 PM, Dmitry Gutov wrote:
> On 10/12/2023 14:04, Denis Zubarev wrote:
>>  > Arguably, the last 2 lines are "variable references" rather than 
>> definitions
>> `var := 3`  is assignment expressions. It allows variable assignments 
>> to occur inside of larger expressions. For example `if (match := 
>> pattern.search(data)) is not None:`.
>> It mostly used to define new variables and act on them if some 
>> condition is met.
>
> Ah, thanks! This feature is newer than my working experience with Python.
>
>> My rationale for `var *= 3` was that it is shorthand for `var = var * 
>> 3` and currently the `var` on the left hand side is fontified with 
>> `font-lock-variable-name-face`.
>
> I think ideally, in cases when "var =" is not the first occurrence for 
> the same var in a given scope, we wouldn't highlight it as 
> "definition" either.
>
> Speaking of font-lock-variable-use-face, I think it would be most 
> useful if it helped with noticing typos (meaning, it would only be 
> used for references to variables that have been defined previously, 
> according to the rules of the language). But for that we still need to 
> implement the scope tracking. And before that, well, my personal 
> preference is not to highlight the references at all, but opinions 
> differ.
Personally I regard the "assignment" feature to mean "any assignment", 
rather than definition. But that's just me. For my personal taste, I 
would make *= always highlight its LHS.
>> I wanted shorthand form to be consistent with the full form.
>> Your point makes sense too, I don't have strong opinion about this.
>> Also I'm not sure now about `var[ii] = 1`, since it is actually 
>> accessing the list or dictionary element and 
>> `font-lock-variable-use-face` may suit better here.
>
> Yep. To sum up, I would add highlighting to your examples `for var in 
> range(3)` and `var := 3` but not others.
IMHO, for the assignment feature, we should stick to the narrow 
definition of assignments, ie, anything that looks like "a = b". Things 
like "for var in range(3)" could be highlighted by variable feature, I 
think.

And for var[i] = 1, I don't know either. I think I decided to not 
fontify it back then, but it wasn't based on any strong reasoning.
>> Question about new changes.
>> Should I push them to this patch and provide description of new changes,
>> or it would be better to wait for review and send them as new patch?
>
> I suggest sending an updated patch for review in this case, but you 
> can also wait for Yuan's comments first.
>
If you can consolidate everything into a single patch, and pair it with 
a summary, that'll be a great aid to me. Also, in case you don't know 
yet, we follow certain format for commit messages, you can check it out 
in the CONTRIBUTE file, "Commit messages" section.

Yuan

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Dmitry Gutov]

On 11/12/2023 09:10, Yuan Fu wrote:
>>> I wanted shorthand form to be consistent with the full form.
>>> Your point makes sense too, I don't have strong opinion about this.
>>> Also I'm not sure now about `var[ii] = 1`, since it is actually 
>>> accessing the list or dictionary element and 
>>> `font-lock-variable-use-face` may suit better here.
>>
>> Yep. To sum up, I would add highlighting to your examples `for var in 
>> range(3)` and `var := 3` but not others.
> IMHO, for the assignment feature, we should stick to the narrow 
> definition of assignments, ie, anything that looks like "a = b". Things 
> like "for var in range(3)" could be highlighted by variable feature, I 
> think.

I think "for var in range(3)" should be part of the "definition" feature 
because a variable is defined there. Alongside parameters.

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Denis Zubarev]

[-- Attachment #1: Type: text/html, Size: 7614 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0003-Improve-syntax-highlighting-for-python-ts-mode.patch --]
[-- Type: text/x-diff; name="0003-Improve-syntax-highlighting-for-python-ts-mode.patch", Size: 24425 bytes --]

From e5ec8f30c496c7f280e320cab9b013117c8b4135 Mon Sep 17 00:00:00 2001
From: Denis Zubarev <dvzubarev@yandex.ru>
Date: Sat, 11 Nov 2023 04:55:44 +0300
Subject: [PATCH] Improve syntax highlighting for python-ts-mode

Fix fontification of strings inside of f-strings interpolation, e.g. for
f"beg {'nested'}" - 'nested' was not fontified as string.  Do not
override the face of builtin functions (all, bytes etc.) with the
function call face.  Add missing assignment expressions (:= *=).
Highlight variables defined in for loop (for var1, var2 in ).  Fontify
built-ins (dict,list,etc.) as types when they are used in type hints.
Highlight union types (type1|type2).  Highlight base class names in the
class definition.  Fontify class patterns in case statement. Highlight
the second argument as a type in isinstance/issubclass call. Highlight
dotted decorator names.

* lisp/progmodes/python.el (python--treesit-keywords): Add compound
keyword "is not".
(python--treesit-fontify-string): fix f-string interpolation. Enable
interpolation highlighting only if string-interpolation is presented on
the third level of treesit-font-lock-feature-list.
(python--treesit-fontify-string-interpolation): Remove function.
(python--treesit-fontify-union-types): Fontify nested union types.
(python--treesit-fontify-dotted-decorator): Fontify all parts of
dotted decorator name.
(python--treesit-settings): Change/add rules.
* test/lisp/progmodes/python-tests.el (python-ts-tests-with-temp-buffer):
function for setting up test buffer.
(python-ts-mode-compound-keywords-face)
(python-ts-mode-var-for-loop-face)
(python-ts-mode-named-assignement-face-1)
(python-ts-mode-assignement-face-2)
(python-ts-mode-nested-types-face-1)
(python-ts-mode-union-types-face-1)
(python-ts-mode-union-types-face-2)
(python-ts-mode-types-face-1)
(python-ts-mode-types-face-2)
(python-ts-mode-types-face-3)
(python-ts-mode-isinstance-type-face-1)
(python-ts-mode-isinstance-type-face-2)
(python-ts-mode-superclass-type-face)
(python-ts-mode-class-patterns-face)
(python-ts-mode-dotted-decorator-face-1)
(python-ts-mode-dotted-decorator-face-2)
(python-ts-mode-builtin-call-face)
(python-ts-mode-interpolation-nested-string)
(python-ts-mode-disabled-string-interpolation)
(python-ts-mode-interpolation-doc-string): Add tests.
---
 lisp/progmodes/python.el            | 182 +++++++++++++-----
 test/lisp/progmodes/python-tests.el | 279 ++++++++++++++++++++++++++++
 2 files changed, 413 insertions(+), 48 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index ab3bf1b4ec..bacb0776f0 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -979,7 +979,7 @@ python--treesit-keywords
     "raise" "return" "try" "while" "with" "yield"
     ;; These are technically operators, but we fontify them as
     ;; keywords.
-    "and" "in" "is" "not" "or" "not in"))
+    "and" "in" "is" "not" "or" "not in" "is not"))
 
 (defvar python--treesit-builtins
   '("abs" "all" "any" "ascii" "bin" "bool" "breakpoint" "bytearray"
@@ -1042,9 +1042,7 @@ python--treesit-fontify-string
 f-strings.  OVERRIDE is the override flag described in
 `treesit-font-lock-rules'.  START and END mark the region to be
 fontified."
-  (let* ((string-beg (treesit-node-start node))
-         (string-end (treesit-node-end node))
-         (maybe-expression (treesit-node-parent node))
+  (let* ((maybe-expression (treesit-node-parent node))
          (grandparent (treesit-node-parent
                        (treesit-node-parent
                         maybe-expression)))
@@ -1072,28 +1070,76 @@ python--treesit-fontify-string
                         (equal (treesit-node-type maybe-expression)
                                "expression_statement"))
                    'font-lock-doc-face
-                 'font-lock-string-face)))
-    ;; Don't highlight string prefixes like f/r/b.
-    (save-excursion
-      (goto-char string-beg)
-      (when (re-search-forward "[\"']" string-end t)
-        (setq string-beg (match-beginning 0))))
-    (treesit-fontify-with-override
-     string-beg string-end face override start end)))
-
-(defun python--treesit-fontify-string-interpolation
-    (node _ start end &rest _)
-  "Fontify string interpolation.
-NODE is the string node.  Do not fontify the initial f for
-f-strings.  START and END mark the region to be
+                 'font-lock-string-face))
+
+         (ignore-interpolation (not (memq 'string-interpolation
+                                          (nth 2 treesit-font-lock-feature-list))))
+         ;; If interpolation is enabled, highlight only
+         ;; string_start/string_content/string_end children.  Do not
+         ;; touch interpolation node that can occur inside of the
+         ;; string.
+         (string-nodes (if ignore-interpolation
+                           (list node)
+                         (treesit-filter-child
+                          node
+                          (lambda (ch) (member (treesit-node-type ch)
+                                               '("string_start"
+                                                 "string_content"
+                                                 "string_end")))
+                          t))))
+
+    (dolist (string-node string-nodes)
+      (let ((string-beg (treesit-node-start string-node))
+            (string-end (treesit-node-end string-node)))
+        (when (or ignore-interpolation
+                  (equal (treesit-node-type string-node) "string_start"))
+          ;; Don't highlight string prefixes like f/r/b.
+          (save-excursion
+            (goto-char string-beg)
+            (when (re-search-forward "[\"']" string-end t)
+              (setq string-beg (match-beginning 0)))))
+
+        (treesit-fontify-with-override
+         string-beg string-end face override start end)))))
+
+(defun python--treesit-fontify-union-types (node override start end &rest _)
+  "Fontify nested union types in the type hints.
+For examlpe, Lvl1 | Lvl2[Lvl3[Lvl4[Lvl5 | None]], Lvl2].  This
+structure is represented via nesting binary_operator and
+subscript nodes.  This function iterates over all levels and
+highlight identifier nodes.  NODE is the binary_operator
+node.  OVERRIDE is the override flag described in
+`treesit-font-lock-rules'.  START and END mark the region to be
+fontified."
+  (dolist (child (treesit-node-children node t))
+    (pcase (treesit-node-type child)
+      ((or "identifier" "none")
+       (treesit-fontify-with-override
+        (treesit-node-start child) (treesit-node-end child)
+        'font-lock-type-face override start end))
+      ("attribute"
+       (when-let ((type-node (treesit-node-child-by-field-name child "attribute")))
+         (treesit-fontify-with-override
+          (treesit-node-start type-node) (treesit-node-end type-node)
+          'font-lock-type-face override start end)))
+      ((or "binary_operator" "subscript")
+       (python--treesit-fontify-union-types child override start end)))))
+
+(defun python--treesit-fontify-dotted-decorator (node override start end &rest _)
+  "Fontify dotted decorators.
+For example @pytes.mark.skip.  Iterate over all nested attribute
+nodes and highlight identifier nodes.  NODE is the first attribute
+node.  OVERRIDE is the override flag described in
+`treesit-font-lock-rules'.  START and END mark the region to be
 fontified."
-  ;; This is kind of a hack, it basically removes the face applied by
-  ;; the string feature, so that following features can apply their
-  ;; face.
-  (let ((n-start (treesit-node-start node))
-        (n-end (treesit-node-end node)))
-    (remove-text-properties
-     (max start n-start) (min end n-end) '(face))))
+  (dolist (child (treesit-node-children node t))
+    (pcase (treesit-node-type child)
+      ("identifier"
+       (treesit-fontify-with-override
+        (treesit-node-start child) (treesit-node-end child)
+        'font-lock-type-face override start end))
+      ("attribute"
+       (python--treesit-fontify-dotted-decorator child override start end)))))
 
 (defvar python--treesit-settings
   (treesit-font-lock-rules
@@ -1103,14 +1149,9 @@ python--treesit-settings
 
    :feature 'string
    :language 'python
-   '((string) @python--treesit-fontify-string)
+   '((string) @python--treesit-fontify-string
+     (interpolation ["{" "}"] @font-lock-misc-punctuation-face))
 
-   ;; HACK: This feature must come after the string feature and before
-   ;; other features.  Maybe we should make string-interpolation an
-   ;; option rather than a feature.
-   :feature 'string-interpolation
-   :language 'python
-   '((interpolation) @python--treesit-fontify-string-interpolation)
 
    :feature 'keyword
    :language 'python
@@ -1124,14 +1165,10 @@ python--treesit-settings
       name: (identifier) @font-lock-function-name-face)
      (class_definition
       name: (identifier) @font-lock-type-face)
+     (for_statement left: (identifier)
+                    @font-lock-variable-name-face)
      (parameters (identifier) @font-lock-variable-name-face))
 
-   :feature 'function
-   :language 'python
-   '((call function: (identifier) @font-lock-function-call-face)
-     (call function: (attribute
-                      attribute: (identifier) @font-lock-function-call-face)))
-
    :feature 'builtin
    :language 'python
    `(((identifier) @font-lock-builtin-face
@@ -1142,6 +1179,12 @@ python--treesit-settings
                       eol))
               @font-lock-builtin-face)))
 
+   :feature 'function
+   :language 'python
+   '((call function: (identifier) @font-lock-function-call-face)
+     (call function: (attribute
+                      attribute: (identifier) @font-lock-function-call-face)))
+
    :feature 'constant
    :language 'python
    '([(true) (false) (none)] @font-lock-constant-face)
@@ -1153,30 +1196,73 @@ python--treesit-settings
                  @font-lock-variable-name-face)
      (assignment left: (attribute
                         attribute: (identifier)
-                        @font-lock-property-use-face))
-     (pattern_list (identifier)
+                        @font-lock-variable-name-face))
+     (augmented_assignment left: (identifier)
+                           @font-lock-variable-name-face)
+     (named_expression name: (identifier)
+                       @font-lock-variable-name-face)
+     (pattern_list [(identifier)
+                    (list_splat_pattern (identifier))]
                    @font-lock-variable-name-face)
-     (tuple_pattern (identifier)
+     (tuple_pattern [(identifier)
+                     (list_splat_pattern (identifier))]
                     @font-lock-variable-name-face)
-     (list_pattern (identifier)
-                   @font-lock-variable-name-face)
-     (list_splat_pattern (identifier)
-                         @font-lock-variable-name-face))
+     (list_pattern [(identifier)
+                    (list_splat_pattern (identifier))]
+                   @font-lock-variable-name-face))
 
    :feature 'decorator
    :language 'python
+   ;; Override function call face.
+   :override t
    '((decorator "@" @font-lock-type-face)
      (decorator (call function: (identifier) @font-lock-type-face))
-     (decorator (identifier) @font-lock-type-face))
+     (decorator (identifier) @font-lock-type-face)
+     (decorator [(attribute) (call (attribute))] @python--treesit-fontify-dotted-decorator))
 
    :feature 'type
    :language 'python
+   ;; Override built-in faces when dict/list are used for type hints.
+   :override t
    `(((identifier) @font-lock-type-face
       (:match ,(rx-to-string
                 `(seq bol (or ,@python--treesit-exceptions)
-                      eol))
+                  eol))
               @font-lock-type-face))
-     (type (identifier) @font-lock-type-face))
+     (type [(identifier) (none)] @font-lock-type-face)
+     (type (attribute attribute: (identifier) @font-lock-type-face))
+     ;; We don't want to highlight a package of the type
+     ;; (e.g. pack.ClassName).  So explicitly exclude patterns with
+     ;; attribute, since we handle dotted type name in the previous
+     ;; rule.  The following rule handle
+     ;; generic_type/list/tuple/splat_type nodes.
+     (type (_ !attribute [(identifier) (none)] @font-lock-type-face))
+     ;; collections.abc.Iterator[T] case.
+     (type (subscript (attribute attribute: (identifier) @font-lock-type-face)))
+     ;; Nested optional type hints, e.g. val: Lvl1 | Lvl2[Lvl3[Lvl4]].
+     (type (binary_operator) @python--treesit-fontify-union-types)
+     ;;class Type(Base1, Sequence[T]).
+     (class_definition
+      superclasses:
+      (argument_list [(identifier) @font-lock-type-face
+                      (attribute attribute: (identifier) @font-lock-type-face)
+                      (subscript (identifier) @font-lock-type-face)
+                      (subscript (attribute attribute: (identifier) @font-lock-type-face))]))
+
+     ;; Patern matching: case [str(), pack0.Type0()].  Take only the
+     ;; last identifier.
+     (class_pattern (dotted_name (identifier) @font-lock-type-face :anchor))
+
+
+     ;; Highlight the second argument as a type in isinstance/issubclass.
+     ((call function: (identifier) @func-name
+            (argument_list :anchor (_)
+                           [(identifier) @font-lock-type-face
+                            (attribute attribute: (identifier) @font-lock-type-face)
+                            (tuple (identifier) @font-lock-type-face)
+                            (tuple (attribute attribute: (identifier) @font-lock-type-face))
+                            (binary_operator) @python--treesit-fontify-union-types]))
+      (:match "^is\\(?:instance\\|subclass\\)$" @func-name)))
 
    :feature 'escape-sequence
    :language 'python
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index a44a11896f..8613ed8702 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -7299,6 +7299,285 @@ python-tests--flymake-command-output-pattern
                          "Unused import a.b.c (unused-import)"
                        "W0611: Unused import a.b.c (unused-import)"))))))
 
+;;; python-ts-mode font-lock tests
+
+(defmacro python-ts-tests-with-temp-buffer (contents &rest body)
+  "Create a `python-ts-mode' enabled temp buffer with CONTENTS.
+BODY is code to be executed within the temp buffer.  Point is
+always located at the beginning of buffer."
+  (declare (indent 1) (debug t))
+  `(with-temp-buffer
+     (skip-unless (treesit-ready-p 'python))
+     (require 'python)
+     (let ((python-indent-guess-indent-offset nil))
+       (python-ts-mode)
+       (setopt treesit-font-lock-level 3)
+       (insert ,contents)
+       (font-lock-ensure)
+       (goto-char (point-min))
+       ,@body)))
+
+(ert-deftest python-ts-mode-compound-keywords-face ()
+  (dolist (test '("is not" "not in"))
+    (python-ts-tests-with-temp-buffer
+     (concat "t " test " t")
+     (forward-to-word)
+     (should (eq (face-at-point) font-lock-keyword-face))
+     (forward-to-word)
+     (should (eq (face-at-point) font-lock-keyword-face)))))
+
+(ert-deftest python-ts-mode-var-for-loop-face ()
+  (python-ts-tests-with-temp-buffer
+   "for var in range(3):"
+   (dolist (test '("var"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-variable-name-face)))))
+
+(ert-deftest python-ts-mode-named-assignement-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "var := 3"
+   (should (eq (face-at-point) font-lock-variable-name-face))))
+
+(ert-deftest python-ts-mode-assignement-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "var, *rest = call()"
+   (dolist (test '("var" "rest"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-variable-name-face))))
+
+  (python-ts-tests-with-temp-buffer
+   "def func(*args):"
+   (dolist (test '("args"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-variable-name-face))))))
+
+(ert-deftest python-ts-mode-nested-types-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "def func(v:dict[ list[ tuple[str] ], int | None] | None):"
+   (dolist (test '("dict" "list" "tuple" "str" "int" "None" "None"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-union-types-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "def f(val: tuple[tuple, list[Lvl1 | Lvl2[Lvl3[Lvl4[Lvl5 | None]], Lvl2]]]):"
+   (dolist (test '("tuple" "tuple" "list" "Lvl1" "Lvl2" "Lvl3" "Lvl4" "Lvl5" "None" "Lvl2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-union-types-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "def f(val: Type0 | Type1[Type2, pack0.Type3] | pack1.pack2.Type4 | None):"
+   (dolist (test '("Type0" "Type1" "Type2" "Type3" "Type4" "None"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+
+   (goto-char (point-min))
+   (dolist (test '("pack0" "pack1" "pack2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-types-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "def f(val: Callable[[Type0], (Type1, Type2)]):"
+   (dolist (test '("Callable" "Type0" "Type1" "Type2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-types-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "def annot3(val:pack0.Type0)->pack1.pack2.pack3.Type1:"
+   (dolist (test '("Type0" "Type1"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+   (goto-char (point-min))
+   (dolist (test '("pack0" "pack1" "pack2" "pack3"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-types-face-3 ()
+  (python-ts-tests-with-temp-buffer
+   "def annot3(val:collections.abc.Iterator[Type0]):"
+   (dolist (test '("Iterator" "Type0"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+   (goto-char (point-min))
+   (dolist (test '("collections" "abc"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-isinstance-type-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "isinstance(var1, pkg.Type0)
+    isinstance(var2, (str, dict, Type1, type(None)))
+    isinstance(var3, my_type())"
+
+   (dolist (test '("var1" "pkg" "var2" "type" "None" "var3" "my_type"))
+     (let ((case-fold-search nil))
+       (search-forward test))
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))
+
+   (goto-char (point-min))
+   (dolist (test '("Type0" "str" "dict" "Type1"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-isinstance-type-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "issubclass(mytype, int|list|collections.abc.Iterable)"
+   (dolist (test '("int" "list" "Iterable"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-superclass-type-face ()
+  (python-ts-tests-with-temp-buffer
+   "class Temp(Base1, pack0.Base2,  Sequence[T1, T2]):"
+
+   (dolist (test '("Base1" "Base2" "Sequence" "T1" "T2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+
+   (goto-char (point-min))
+   (dolist (test '("pack0"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-class-patterns-face ()
+  (python-ts-tests-with-temp-buffer
+   "match tt:
+        case str():
+            pass
+        case [Type0() | bytes(b) | pack0.pack1.Type1()]:
+            pass
+        case {'i': int(i), 'f': float() as f}:
+            pass"
+
+   (dolist (test '("str" "Type0" "bytes" "Type1" "int" "float"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+
+   (goto-char (point-min))
+   (dolist (test '("pack0" "pack1"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-dotted-decorator-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "@pytest.mark.skip
+    @pytest.mark.skip(reason='msg')
+    def test():"
+
+   (dolist (test '("pytest" "mark" "skip" "pytest" "mark" "skip"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-dotted-decorator-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "@pytest.mark.skip(reason='msg')
+    def test():"
+
+   (setopt treesit-font-lock-level 4)
+   (dolist (test '("pytest" "mark" "skip"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-builtin-call-face ()
+  (python-ts-tests-with-temp-buffer
+   "all()"
+   ;; enable 'function' feature from 4th level
+   (setopt treesit-font-lock-level 4)
+   (should (eq (face-at-point) font-lock-builtin-face))))
+
+(ert-deftest python-ts-mode-interpolation-nested-string ()
+  (python-ts-tests-with-temp-buffer
+   "t = f\"beg {True + 'string'}\""
+
+   (search-forward "True")
+   (goto-char (match-beginning 0))
+   (should (eq (face-at-point) font-lock-constant-face))
+
+   (goto-char (point-min))
+   (dolist (test '("f" "{" "+" "}"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-string-face))))
+
+
+   (goto-char (point-min))
+   (dolist (test '("beg" "'string'" "\""))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-string-face)))))
+
+(ert-deftest python-ts-mode-disabled-string-interpolation ()
+  (python-ts-tests-with-temp-buffer
+   "t = f\"beg {True + var}\""
+
+   (unwind-protect
+       (progn
+         (setf (nth 2 treesit-font-lock-feature-list)
+               (remq 'string-interpolation (nth 2 treesit-font-lock-feature-list)))
+         (setopt treesit-font-lock-level 3)
+
+         (search-forward "f")
+         (goto-char (match-beginning 0))
+         (should (not (eq (face-at-point) font-lock-string-face)))
+
+         (dolist (test '("\"" "beg" "{" "True" "var" "}" "\""))
+           (search-forward test)
+           (goto-char (match-beginning 0))
+           (should (eq (face-at-point) font-lock-string-face))))
+
+    (setf (nth 2 treesit-font-lock-feature-list)
+          (append (nth 2 treesit-font-lock-feature-list) '(string-interpolation))))))
+
+(ert-deftest python-ts-mode-interpolation-doc-string ()
+  (python-ts-tests-with-temp-buffer
+   "f\"\"\"beg {'s1' + True + 's2'} end\"\"\""
+
+   (search-forward "True")
+   (goto-char (match-beginning 0))
+   (should (eq (face-at-point) font-lock-constant-face))
+
+   (goto-char (point-min))
+   (dolist (test '("f" "{" "+" "}"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-string-face))))
+
+   (goto-char (point-min))
+   (dolist (test '("\"\"\"" "beg" "end" "\"\"\""))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-doc-face)))
+
+   (goto-char (point-min))
+   (dolist (test '("'s1'" "'s2'"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-string-face)))))
+
 (provide 'python-tests)
 
 ;;; python-tests.el ends here
-- 
2.34.1

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Yuan Fu]

On 12/11/23 5:18 PM, Denis Zubarev wrote:
> Yuan and Dmitry, thank you for review and suggestions.
> > Can we do this instead: in python--treesit-fontify-string, we check if
>   string-interpolation feature is enabled, if it is, fontify string_start,
>   string_content and string_end only; if not, fontify the whole string.
> Done.
> Enable interpolation highlighting only if 'string-interpolation is 
> presented on the third level of treesit-font-lock-feature-list.
> Personally, If I saw a f-string with an interpolation fontified as 
> string, I would assume that it is bug.
> Clearly it is not a string, so it should be highlighted distinctly.
> But if it is a convention across all languages, we should follow it.

I  encourage everyone to also think in terms of fontification levels, in 
addition to features.

For many Emacs users, they want a quieter or even minimal fontification. 
Some people only want comment and function names highlighted, and they 
can get it by setting the fontification level to 1, because 
python-ts-mode only activates the comment and definition feature at 
level 1. The string feature is at level 2, this level is still 
relatively simplistic. And full string interpolation probably don't 
belong at that level. That's why I separated it out into another 
feature, and placed string-interpolation at level 3.

> > I think "for var in range(3)" should be part of the "definition" feature
>   because a variable is defined there. Alongside parameters.
> I added it to definitions.

Again, if we think of fontification levels, the definition feature is 
about fontifying the function names of definitions, and it's at a low 
level (level 1). Non-essential fontification like "var" shouldn't be 
activated at that level. So I suggest to put it in the variable feature, 
along with many other non-essential fontifications. (Variable feature is 
placed at level 4.)

> My thoughts about parameters. I started to extend rules for them since 
> they are very limited now.
> But I'm not sure what face to use for them.
> I would like to not use the same face as for assignments, because I'd 
> want to highlight them differently.
> It seems that there is no appropriate face in font-lock.el, so I ended 
> up creating my own face in my config.
> Does it make sense to add new face for parameters in font-lock.el?
> Or it is too small feature for its own face?
> I also apply this face for keyword argument in function calls.
To be honest, I don't have any good ideas. Perhaps we can add a 
parameter face that inherits from variable name face by default, Dmitry, 
WDYT?
> Summary for all changes in the patch.
> definition feature:
> `for var in range(3)` highlight var as font-lock-variable-name-face
> assignment feature:
> var := 3 (named_expression)
> var *= 3 (augmented_assignment)
> Highlight var as font-lock-variable-name-face.
> Make list_splat_pattern query more precise.
> list_splat_pattern may appear not only in assignments: var, *rest = 
> call(),
> but in the parameter list too: def f(*args).
> Highlight args only for the first case in assignment feature.
> type feature:
> Fontify built-ins (dict,list,etc.) as types when they are used in type 
> hints.
> support nested union types, for example `Lvl1 | Lvl2[Lvl3[Lvl3], Lvl2]`.
> This structure is represented via nesting binary_operator and 
> subscript nodes in the grammar.
> Function python--treesit-fontify-union-types iterates over all 
> children and highlight identifier nodes.
> Fontify base class names in the class definition: class Temp(Base1, 
> pack0.Base2):
> Fontify class patterns in case statement: case [TempC() | bytes(b)]:
> Highlight the second argument as a type in isinstance/issubclass call:
> isinstance(var2, (str, dict, Type1)); issubclass(var1, int|str)
> For all dotted names of a type highlight only the last part of the name,
> e.g. collections.abc.Iterator.
> decorator feature:
> Highlight dotted names: @pytest.mark.skip
> Function python--treesit-fontify-dotted-decorator iterates over all 
> nested attribute nodes and highlight identifier nodes.
> When font-lock-level is set 4, `skip` had function-call face in: 
> @pytest.mark.skip(reason='t')
> Add `:override t` to decorator feature to override function-call face.
> string feature:
> Enable interpolation highlighting only if string-interpolation is 
> presented on the third level of treesit-font-lock-feature-list.
> Fix fontification of strings inside of f-strings interpolation,
> e.g. for f"beg {'nested'}" - 'nested' was not fontified as string.
Instead of the third level, the check should use the value 
treesit-font-lock-level. And it should check for each level smaller than 
or equal to treesit-font-lock-level.
> function feature:
> Do not override the face of builtin functions (all, bytes etc.) with
> the function call face
> keyword feature:
> Add "is not"  to the `python--treesit-keywords` list.
>
Thanks, they look good. The patch is getting rather large, let's focus 
on getting the existing changes merged rather than adding new stuff to 
it. Though I think your copyright assignment hasn't completed, right?

Yuan

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Dmitry Gutov]

On 12/12/2023 10:24, Yuan Fu wrote:
>> > I think "for var in range(3)" should be part of the "definition" 
>> feature
>>   because a variable is defined there. Alongside parameters.
>> I added it to definitions.
> 
> Again, if we think of fontification levels, the definition feature is 
> about fontifying the function names of definitions, and it's at a low 
> level (level 1). Non-essential fontification like "var" shouldn't be 
> activated at that level. So I suggest to put it in the variable feature, 
> along with many other non-essential fontifications. (Variable feature is 
> placed at level 4.)

I disagree: 'var' in this example is not much different from a function 
parameter. It's a definite place where a variable's name introduced in 
the current scope.

Python doesn't have special keywords for variable declarations (unlike 
'let' in JavaScript or typed declaration in C), so the first time a 
variable is introduced serves as its declaration. For assignments, we 
can't easily determine which is the first time for a given scope, but 
examples like 'for var in ...' or 'except ZeroDivisionError as e:' or 
'[... for var in ...]' are all unambiguously variable definitions.

So I think that:

a) All variable definitions (functions parameters or not) should use 
font-lock-variable-name-face -- to make it easier to find where a given 
symbol is introduced.
b) No font-lock-variable-name-face highlights should be put into the 
'variable' feature, which is disabled by default. All of the examples 
above should either go into 'definition', or if somebody does like that 
approach, into some new 'variable-declaration' feature (enabled by 
default). And maybe some into 'assignment', which is on feature level 3.
c) The 'variable' feature should, at this point, only contain the 
relatively useless highlights, since we don't track variable lifetimes 
yet. That's why it's disabled by default.

The current situation across ts modes is that js-ts-mode has variable 
declarations in the 'definition' feature (and not by my hand, FWIW); 
ruby-ts-mode has a separate 'parameter-definition' feature that 
encompasses both parameters and other variables; in c-ts-mode 
highlighting for 'int i = 4' is split between 'definition' and 
'assignment' (the latter seemingly redundant); typescript-ts-mode and 
rust-ts-mode also follow the principle, more or less.

>> My thoughts about parameters. I started to extend rules for them since 
>> they are very limited now.
>> But I'm not sure what face to use for them.
>> I would like to not use the same face as for assignments, because I'd 
>> want to highlight them differently.
>> It seems that there is no appropriate face in font-lock.el, so I ended 
>> up creating my own face in my config.
>> Does it make sense to add new face for parameters in font-lock.el?
>> Or it is too small feature for its own face?
>> I also apply this face for keyword argument in function calls.
> To be honest, I don't have any good ideas. Perhaps we can add a 
> parameter face that inherits from variable name face by default, Dmitry, 
> WDYT?

As per above, parameters don't seem too different from any other 
variable declarations from my POV. They are similarly useful, so I'd 
highlight them the same way.

Do we want to have a common face which would inherit from 
font-lock-variable-name-face and would be used solely for 
function/methods parameters and nothing else? I don't object, but I 
don't quite see the point either.

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Yuan Fu]

On 12/12/23 4:44 PM, Dmitry Gutov wrote:
> On 12/12/2023 10:24, Yuan Fu wrote:
>>> > I think "for var in range(3)" should be part of the "definition" 
>>> feature
>>>   because a variable is defined there. Alongside parameters.
>>> I added it to definitions.
>>
>> Again, if we think of fontification levels, the definition feature is 
>> about fontifying the function names of definitions, and it's at a low 
>> level (level 1). Non-essential fontification like "var" shouldn't be 
>> activated at that level. So I suggest to put it in the variable 
>> feature, along with many other non-essential fontifications. 
>> (Variable feature is placed at level 4.)
>
> I disagree: 'var' in this example is not much different from a 
> function parameter. It's a definite place where a variable's name 
> introduced in the current scope.
>
> Python doesn't have special keywords for variable declarations (unlike 
> 'let' in JavaScript or typed declaration in C), so the first time a 
> variable is introduced serves as its declaration. For assignments, we 
> can't easily determine which is the first time for a given scope, but 
> examples like 'for var in ...' or 'except ZeroDivisionError as e:' or 
> '[... for var in ...]' are all unambiguously variable definitions.

Sure, I don't really care too much about which feature should a rule be 
in; what I do care about is to keep first and second fontification level 
relatively quite and minimal, and keep level 3 reasonably conservative. 
And people that want a lot of highlight can turn on level 4.

>
> So I think that:
>
> a) All variable definitions (functions parameters or not) should use 
> font-lock-variable-name-face -- to make it easier to find where a 
> given symbol is introduced.
> b) No font-lock-variable-name-face highlights should be put into the 
> 'variable' feature, which is disabled by default. All of the examples 
> above should either go into 'definition', or if somebody does like 
> that approach, into some new 'variable-declaration' feature (enabled 
> by default). And maybe some into 'assignment', which is on feature 
> level 3.
> c) The 'variable' feature should, at this point, only contain the 
> relatively useless highlights, since we don't track variable lifetimes 
> yet. That's why it's disabled by default.
>
> The current situation across ts modes is that js-ts-mode has variable 
> declarations in the 'definition' feature (and not by my hand, FWIW); 

Gah!

> ruby-ts-mode has a separate 'parameter-definition' feature that 
> encompasses both parameters and other variables; 

> in c-ts-mode highlighting for 'int i = 4' is split between 
> 'definition' and 'assignment' (the latter seemingly redundant); 

Should've been in assignment IMO. I probably overlooked it.

> typescript-ts-mode and rust-ts-mode also follow the principle, more or 
> less.

Well, the only ts-mode that I actually wrote is python-ts-mode. For 
other major modes, I can only suggest. Even for python-ts-mode, I don't 
want to exert my personal opinion onto it too much, except for keeping 
font-lock level 1 and 2 quiet.

>>> My thoughts about parameters. I started to extend rules for them 
>>> since they are very limited now.
>>> But I'm not sure what face to use for them.
>>> I would like to not use the same face as for assignments, because 
>>> I'd want to highlight them differently.
>>> It seems that there is no appropriate face in font-lock.el, so I 
>>> ended up creating my own face in my config.
>>> Does it make sense to add new face for parameters in font-lock.el?
>>> Or it is too small feature for its own face?
>>> I also apply this face for keyword argument in function calls.
>> To be honest, I don't have any good ideas. Perhaps we can add a 
>> parameter face that inherits from variable name face by default, 
>> Dmitry, WDYT?
>
> As per above, parameters don't seem too different from any other 
> variable declarations from my POV. They are similarly useful, so I'd 
> highlight them the same way.
>
> Do we want to have a common face which would inherit from 
> font-lock-variable-name-face and would be used solely for 
> function/methods parameters and nothing else? I don't object, but I 
> don't quite see the point either.

I agree.

Yuan

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Dmitry Gutov]

On 13/12/2023 05:49, Yuan Fu wrote:

>> Python doesn't have special keywords for variable declarations (unlike 
>> 'let' in JavaScript or typed declaration in C), so the first time a 
>> variable is introduced serves as its declaration. For assignments, we 
>> can't easily determine which is the first time for a given scope, but 
>> examples like 'for var in ...' or 'except ZeroDivisionError as e:' or 
>> '[... for var in ...]' are all unambiguously variable definitions.
> 
> Sure, I don't really care too much about which feature should a rule be 
> in; what I do care about is to keep first and second fontification level 
> relatively quite and minimal, and keep level 3 reasonably conservative. 
> And people that want a lot of highlight can turn on level 4.

I don't mind if assignments in python-ts-mode go to level 3, that's what 
ruby-ts-mode does anyway. But '[... for var in ...]' really should use 
variable-name-face and it should be in the default config (level 3 at 
most). I think the 'definition' feature is good for it (going by the 
name, since it's an implicit variable declaration), but it could be 
split off into a separate feature too.

>> in c-ts-mode highlighting for 'int i = 4' is split between 
>> 'definition' and 'assignment' (the latter seemingly redundant); 
> 
> Should've been in assignment IMO. I probably overlooked it.

The current state is that the query in 'definition' can highlight both 
'int i;' and 'int i = 4;'. The query in 'assignment' in c-ts-mode only 
highlights 'int i = 4;'.

If you just keep the latter query, 'int i;' would stay unfontified. If 
you move the corresponding query from 'definition' to 'assignment', it 
would start matching non-assignment declarations too. Might seem odd.

>> typescript-ts-mode and rust-ts-mode also follow the principle, more or 
>> less.
> 
> Well, the only ts-mode that I actually wrote is python-ts-mode. For 
> other major modes, I can only suggest. Even for python-ts-mode, I don't 
> want to exert my personal opinion onto it too much, except for keeping 
> font-lock level 1 and 2 quiet.

For my part, I mostly care about keeping the level 3 feature-rich 
enough, but precise at the same time. And without frivolous highlights 
(only a little more fruit-salady than the pre-treesit modes).

>>>> My thoughts about parameters. I started to extend rules for them 
>>>> since they are very limited now.
>>>> But I'm not sure what face to use for them.
>>>> I would like to not use the same face as for assignments, because 
>>>> I'd want to highlight them differently.
>>>> It seems that there is no appropriate face in font-lock.el, so I 
>>>> ended up creating my own face in my config.
>>>> Does it make sense to add new face for parameters in font-lock.el?
>>>> Or it is too small feature for its own face?
>>>> I also apply this face for keyword argument in function calls.
>>> To be honest, I don't have any good ideas. Perhaps we can add a 
>>> parameter face that inherits from variable name face by default, 
>>> Dmitry, WDYT?
>>
>> As per above, parameters don't seem too different from any other 
>> variable declarations from my POV. They are similarly useful, so I'd 
>> highlight them the same way.
>>
>> Do we want to have a common face which would inherit from 
>> font-lock-variable-name-face and would be used solely for 
>> function/methods parameters and nothing else? I don't object, but I 
>> don't quite see the point either.
> 
> I agree.

Then I suppose we should clarify whether Denis wants a face that only 
matches function parameters, or implicit variable declarations as well. 
Or maybe instead a face that is only used for assignments (only first 
assignments?) -- which would separate them from the two semantic units 
above.

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Yuan Fu]

> On Dec 13, 2023, at 10:28 AM, Dmitry Gutov <dmitry@gutov.dev> wrote:
> 
> On 13/12/2023 05:49, Yuan Fu wrote:
> 
>>> Python doesn't have special keywords for variable declarations (unlike 'let' in JavaScript or typed declaration in C), so the first time a variable is introduced serves as its declaration. For assignments, we can't easily determine which is the first time for a given scope, but examples like 'for var in ...' or 'except ZeroDivisionError as e:' or '[... for var in ...]' are all unambiguously variable definitions.
>> Sure, I don't really care too much about which feature should a rule be in; what I do care about is to keep first and second fontification level relatively quite and minimal, and keep level 3 reasonably conservative. And people that want a lot of highlight can turn on level 4.
> 
> I don't mind if assignments in python-ts-mode go to level 3, that's what ruby-ts-mode does anyway.

Assignment is in level 3 for python-ts-mode.

> But '[... for var in ...]' really should use variable-name-face and it should be in the default config (level 3 at most).

I’m fine with that.

> I think the 'definition' feature is good for it (going by the name, since it's an implicit variable declaration), but it could be split off into a separate feature too.

As long as it’s not added to the definition feature, because, again, definition is at level 1 and I don’t want to keep level 1 minimal.

Maybe we can use local-definition, or something similar, to signify that this feature highlights scoped definitions.

> 
>>> in c-ts-mode highlighting for 'int i = 4' is split between 'definition' and 'assignment' (the latter seemingly redundant); 
>> Should've been in assignment IMO. I probably overlooked it.
> 
> The current state is that the query in 'definition' can highlight both 'int i;' and 'int i = 4;'. The query in 'assignment' in c-ts-mode only highlights 'int i = 4;'.
> 
> If you just keep the latter query, 'int i;' would stay unfontified. If you move the corresponding query from 'definition' to 'assignment', it would start matching non-assignment declarations too. Might seem odd.

Right… hmm… This one is hard to decide...

> 
>>> typescript-ts-mode and rust-ts-mode also follow the principle, more or less.
>> Well, the only ts-mode that I actually wrote is python-ts-mode. For other major modes, I can only suggest. Even for python-ts-mode, I don't want to exert my personal opinion onto it too much, except for keeping font-lock level 1 and 2 quiet.
> 
> For my part, I mostly care about keeping the level 3 feature-rich enough, but precise at the same time. And without frivolous highlights (only a little more fruit-salady than the pre-treesit modes).

Sounds good to me :-)

>>>>> My thoughts about parameters. I started to extend rules for them since they are very limited now.
>>>>> But I'm not sure what face to use for them.
>>>>> I would like to not use the same face as for assignments, because I'd want to highlight them differently.
>>>>> It seems that there is no appropriate face in font-lock.el, so I ended up creating my own face in my config.
>>>>> Does it make sense to add new face for parameters in font-lock.el?
>>>>> Or it is too small feature for its own face?
>>>>> I also apply this face for keyword argument in function calls.
>>>> To be honest, I don't have any good ideas. Perhaps we can add a parameter face that inherits from variable name face by default, Dmitry, WDYT?
>>> 
>>> As per above, parameters don't seem too different from any other variable declarations from my POV. They are similarly useful, so I'd highlight them the same way.
>>> 
>>> Do we want to have a common face which would inherit from font-lock-variable-name-face and would be used solely for function/methods parameters and nothing else? I don't object, but I don't quite see the point either.
>> I agree.
> 
> Then I suppose we should clarify whether Denis wants a face that only matches function parameters, or implicit variable declarations as well. Or maybe instead a face that is only used for assignments (only first assignments?) -- which would separate them from the two semantic units above.

I’m ok with either. And I’ll leave it to you guys to decide, like I did other faces we added in Emacs 29 ;-)

Yuan

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Dmitry Gutov]

On 14/12/2023 07:54, Yuan Fu wrote:
>> I think the 'definition' feature is good for it (going by the name, since it's an implicit variable declaration), but it could be split off into a separate feature too.
> As long as it’s not added to the definition feature, because, again, definition is at level 1 and I don’t want to keep level 1 minimal.
> 
> Maybe we can use local-definition, or something similar, to signify that this feature highlights scoped definitions.

But you think function parameters should be in 'definition'? They are 
also "scoped", I would say.

If not, we could have a separate feature 'variable-definition' which 
would include parameters as well.

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Yuan Fu]

> On Dec 14, 2023, at 3:51 AM, Dmitry Gutov <dmitry@gutov.dev> wrote:
> 
> On 14/12/2023 07:54, Yuan Fu wrote:
>>> I think the 'definition' feature is good for it (going by the name, since it's an implicit variable declaration), but it could be split off into a separate feature too.
>> As long as it’s not added to the definition feature, because, again, definition is at level 1 and I don’t want to keep level 1 minimal.
>> Maybe we can use local-definition, or something similar, to signify that this feature highlights scoped definitions.
> 
> But you think function parameters should be in 'definition'? They are also "scoped", I would say.

I don’t think function parameters should be in ‘definition’. In fact, in my head, only the variable/function/class name of top-level constructs should be in ‘definition’. Now I can see that the name ‘definition’ is too vague for my original intent, and most ts modes probably don’t share the same interpretation as I do...

Maybe we can allow definition to include more things and move it to level 3, and add a more restricted ’top-level-definition’ to level 1 to take it’s current role. WDYT?

Yuan

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Dmitry Gutov]

On 17/12/2023 03:07, Yuan Fu wrote:
> 
>> On Dec 14, 2023, at 3:51 AM, Dmitry Gutov<dmitry@gutov.dev>  wrote:
>>
>> On 14/12/2023 07:54, Yuan Fu wrote:
>>>> I think the 'definition' feature is good for it (going by the name, since it's an implicit variable declaration), but it could be split off into a separate feature too.
>>> As long as it’s not added to the definition feature, because, again, definition is at level 1 and I don’t want to keep level 1 minimal.
>>> Maybe we can use local-definition, or something similar, to signify that this feature highlights scoped definitions.
>> But you think function parameters should be in 'definition'? They are also "scoped", I would say.
> I don’t think function parameters should be in ‘definition’. In fact, in my head, only the variable/function/class name of top-level constructs should be in ‘definition’. Now I can see that the name ‘definition’ is too vague for my original intent, and most ts modes probably don’t share the same interpretation as I do...
> 
> Maybe we can allow definition to include more things and move it to level 3, and add a more restricted ’top-level-definition’ to level 1 to take it’s current role. WDYT?

What about a split like function-definition/variable-definition? Not all 
functions are top-level. But there are also classes...

Or the features could just be called 'definition' and 
'variable-definition', and that the former only contains functions and 
classes might simply be implied by the existence of the latter.

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Denis Zubarev]

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

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Wed, 13 Dec 2023 21:54:16 -0800
> Cc: Denis Zubarev <dvzubarev@yandex.ru>,
>  Eli Zaretskii <eliz@gnu.org>,
>  "67061@debbugs.gnu.org" <67061@debbugs.gnu.org>
> 
> > Then I suppose we should clarify whether Denis wants a face that only matches function parameters, or implicit variable declarations as well. Or maybe instead a face that is only used for assignments (only first assignments?) -- which would separate them from the two semantic units above.
> 
> I’m ok with either. And I’ll leave it to you guys to decide, like I did other faces we added in Emacs 29 ;-)

I'm not sure how to move forward here.  The copyright assignment is on
file now, and Denis posted a rebased patch, but I'm not sure we all
agree that it should be installed.  is there anything else that needs
to be done before Denis's patch can be installed on the emacs-29
branch?

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Denis Zubarev]

[-- Attachment #1: Type: text/html, Size: 6660 bytes --]

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Dmitry Gutov]

On 17/12/2023 03:56, Denis Zubarev wrote:
>  > Do we want to have a common face which would inherit from
>  > font-lock-variable-name-face and would be used solely for
>  > function/methods parameters and nothing else? I don't object, but I
>  > don't quite see the point either.
> The point is to make it easy for users to customize faces of features
> independently from each other.
> It is not only about variables/parameters.

Granularity of faces can be increased, but one should also consider 
which nodes go together better with which others.

E.g. even if variable-assignment is a separate face, we would need to 
make it inherit from one of the more basic faces.

> For example, if I want to change a face for decorators, I have to change
> font-lock-type-face, which will change also all type faces.
> I like approach from the helix editor. They introduce many captures with
> different levels of specificity, for example @variable for (identifier),
> @variable.parameter for function parameters, @variable.builtin for
> self|cls etc. I guess by default the default face defined for a @variable
> is used. But one can customize variable.parameter to their liking 
> without touching any
> other variables.
>  > Then I suppose we should clarify whether Denis wants a face that only
>  > matches function parameters, or implicit variable declarations as well.
>  > Or maybe instead a face that is only used for assignments (only first
>  > assignments?) -- which would separate them from the two semantic units
>  > above.
> I think ideally, there should be a face for a feature (or even multiple
> faces).
> For example, faces for variables in helix notation:
> - @variable
> - @variable.definition
> - @variable.definition.parameter
> - @variable.assignment
> - @variable.use

I think this is fairly similar to our faces hierarchy, where children 
inherit attributes from the parent. Just using a shorter notation.

Going back to what is a good thing for highlighting assignments, I would 
separate "first assignments" from the rest, and either inherit their 
face from "variable definition", or simply used the same face. Only in 
languages like Python or Ruby, or course, where any first assignment is 
an implicit variable declaration.

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Eli Zaretskii]

> Date: Tue, 12 Dec 2023 00:24:41 -0800
> Cc: "67061@debbugs.gnu.org" <67061@debbugs.gnu.org>
> From: Yuan Fu <casouri@gmail.com>
> 
> Thanks, they look good. The patch is getting rather large, let's focus 
> on getting the existing changes merged rather than adding new stuff to 
> it. Though I think your copyright assignment hasn't completed, right?

Denis's assignment is on file, so we are good to go with his
contributions.

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Denis Zubarev]

[-- Attachment #1: Type: text/html, Size: 10642 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0004-Improve-syntax-highlighting-for-python-ts-mode.patch --]
[-- Type: text/x-diff; name="0004-Improve-syntax-highlighting-for-python-ts-mode.patch", Size: 27957 bytes --]

From 89fa2cad793e00062b823e855a141bd940c4c175 Mon Sep 17 00:00:00 2001
From: Denis Zubarev <dvzubarev@yandex.ru>
Date: Sat, 11 Nov 2023 04:55:44 +0300
Subject: [PATCH] Improve syntax highlighting for python-ts-mode

Fix fontification of strings inside of f-strings interpolation, e.g. for
f"beg {'nested'}" - 'nested' was not fontified as string.  Do not
override the face of builtin functions (all, bytes etc.) with the
function call face.  Add missing assignment expressions (:= *=).
Highlight variables defined in for loop (for var1, var2 in ).  Fontify
built-ins (dict,list,etc.) as types when they are used in type hints.
Highlight union types (type1|type2).  Highlight base class names in the
class definition.  Fontify class patterns in case statement. Highlight
the second argument as a type in isinstance/issubclass call. Highlight
dotted decorator names.

Add new feature variable-definition for variables defined for local
scopes (for var in [], with T as var, etc.).

* lisp/progmodes/python.el (python--treesit-keywords): Add compound
keyword "is not".
(python--treesit-fontify-string): fix f-string interpolation. Enable
interpolation highlighting only if string-interpolation is presented on
the enabled levels of treesit-font-lock-feature-list.
(python--treesit-fontify-string-interpolation): Remove function.
(python--treesit-fontify-union-types): Fontify nested union types.
(python--treesit-fontify-dotted-decorator): Fontify all parts of
dotted decorator name.
(python--treesit-settings): Change/add rules.
* test/lisp/progmodes/python-tests.el (python-ts-tests-with-temp-buffer):
function for setting up test buffer.
(python-ts-mode-compound-keywords-face)
(python-ts-mode-var-for-loop-face)
(python-ts-mode-named-assignement-face-1)
(python-ts-mode-assignement-face-2)
(python-ts-mode-nested-types-face-1)
(python-ts-mode-union-types-face-1)
(python-ts-mode-union-types-face-2)
(python-ts-mode-types-face-1)
(python-ts-mode-types-face-2)
(python-ts-mode-types-face-3)
(python-ts-mode-isinstance-type-face-1)
(python-ts-mode-isinstance-type-face-2)
(python-ts-mode-superclass-type-face)
(python-ts-mode-class-patterns-face)
(python-ts-mode-dotted-decorator-face-1)
(python-ts-mode-dotted-decorator-face-2)
(python-ts-mode-builtin-call-face)
(python-ts-mode-interpolation-nested-string)
(python-ts-mode-disabled-string-interpolation)
(python-ts-mode-interpolation-doc-string): Add tests.
---
 lisp/progmodes/python.el            | 196 ++++++++++++----
 test/lisp/progmodes/python-tests.el | 350 ++++++++++++++++++++++++++++
 2 files changed, 497 insertions(+), 49 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index ab3bf1b4ec..e7dcd8f3fb 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -979,7 +979,7 @@ python--treesit-keywords
     "raise" "return" "try" "while" "with" "yield"
     ;; These are technically operators, but we fontify them as
     ;; keywords.
-    "and" "in" "is" "not" "or" "not in"))
+    "and" "in" "is" "not" "or" "not in" "is not"))
 
 (defvar python--treesit-builtins
   '("abs" "all" "any" "ascii" "bin" "bool" "breakpoint" "bytearray"
@@ -1042,9 +1042,7 @@ python--treesit-fontify-string
 f-strings.  OVERRIDE is the override flag described in
 `treesit-font-lock-rules'.  START and END mark the region to be
 fontified."
-  (let* ((string-beg (treesit-node-start node))
-         (string-end (treesit-node-end node))
-         (maybe-expression (treesit-node-parent node))
+  (let* ((maybe-expression (treesit-node-parent node))
          (grandparent (treesit-node-parent
                        (treesit-node-parent
                         maybe-expression)))
@@ -1072,28 +1070,78 @@ python--treesit-fontify-string
                         (equal (treesit-node-type maybe-expression)
                                "expression_statement"))
                    'font-lock-doc-face
-                 'font-lock-string-face)))
-    ;; Don't highlight string prefixes like f/r/b.
-    (save-excursion
-      (goto-char string-beg)
-      (when (re-search-forward "[\"']" string-end t)
-        (setq string-beg (match-beginning 0))))
-    (treesit-fontify-with-override
-     string-beg string-end face override start end)))
-
-(defun python--treesit-fontify-string-interpolation
-    (node _ start end &rest _)
-  "Fontify string interpolation.
-NODE is the string node.  Do not fontify the initial f for
-f-strings.  START and END mark the region to be
+                 'font-lock-string-face))
+
+         (ignore-interpolation (not
+                                (seq-some
+                                 (lambda (feats) (memq 'string-interpolation feats))
+                                 (seq-take treesit-font-lock-feature-list treesit-font-lock-level))))
+         ;; If interpolation is enabled, highlight only
+         ;; string_start/string_content/string_end children.  Do not
+         ;; touch interpolation node that can occur inside of the
+         ;; string.
+         (string-nodes (if ignore-interpolation
+                           (list node)
+                         (treesit-filter-child
+                          node
+                          (lambda (ch) (member (treesit-node-type ch)
+                                               '("string_start"
+                                                 "string_content"
+                                                 "string_end")))
+                          t))))
+
+    (dolist (string-node string-nodes)
+      (let ((string-beg (treesit-node-start string-node))
+            (string-end (treesit-node-end string-node)))
+        (when (or ignore-interpolation
+                  (equal (treesit-node-type string-node) "string_start"))
+          ;; Don't highlight string prefixes like f/r/b.
+          (save-excursion
+            (goto-char string-beg)
+            (when (re-search-forward "[\"']" string-end t)
+              (setq string-beg (match-beginning 0)))))
+
+        (treesit-fontify-with-override
+         string-beg string-end face override start end)))))
+
+(defun python--treesit-fontify-union-types (node override start end &rest _)
+  "Fontify nested union types in the type hints.
+For examlpe, Lvl1 | Lvl2[Lvl3[Lvl4[Lvl5 | None]], Lvl2].  This
+structure is represented via nesting binary_operator and
+subscript nodes.  This function iterates over all levels and
+highlight identifier nodes.  NODE is the binary_operator
+node.  OVERRIDE is the override flag described in
+`treesit-font-lock-rules'.  START and END mark the region to be
+fontified."
+  (dolist (child (treesit-node-children node t))
+    (pcase (treesit-node-type child)
+      ((or "identifier" "none")
+       (treesit-fontify-with-override
+        (treesit-node-start child) (treesit-node-end child)
+        'font-lock-type-face override start end))
+      ("attribute"
+       (when-let ((type-node (treesit-node-child-by-field-name child "attribute")))
+         (treesit-fontify-with-override
+          (treesit-node-start type-node) (treesit-node-end type-node)
+          'font-lock-type-face override start end)))
+      ((or "binary_operator" "subscript")
+       (python--treesit-fontify-union-types child override start end)))))
+
+(defun python--treesit-fontify-dotted-decorator (node override start end &rest _)
+  "Fontify dotted decorators.
+For example @pytes.mark.skip.  Iterate over all nested attribute
+nodes and highlight identifier nodes.  NODE is the first attribute
+node.  OVERRIDE is the override flag described in
+`treesit-font-lock-rules'.  START and END mark the region to be
 fontified."
-  ;; This is kind of a hack, it basically removes the face applied by
-  ;; the string feature, so that following features can apply their
-  ;; face.
-  (let ((n-start (treesit-node-start node))
-        (n-end (treesit-node-end node)))
-    (remove-text-properties
-     (max start n-start) (min end n-end) '(face))))
+  (dolist (child (treesit-node-children node t))
+    (pcase (treesit-node-type child)
+      ("identifier"
+       (treesit-fontify-with-override
+        (treesit-node-start child) (treesit-node-end child)
+        'font-lock-type-face override start end))
+      ("attribute"
+       (python--treesit-fontify-dotted-decorator child override start end)))))
 
 (defvar python--treesit-settings
   (treesit-font-lock-rules
@@ -1103,14 +1151,9 @@ python--treesit-settings
 
    :feature 'string
    :language 'python
-   '((string) @python--treesit-fontify-string)
+   '((string) @python--treesit-fontify-string
+     (interpolation ["{" "}"] @font-lock-misc-punctuation-face))
 
-   ;; HACK: This feature must come after the string feature and before
-   ;; other features.  Maybe we should make string-interpolation an
-   ;; option rather than a feature.
-   :feature 'string-interpolation
-   :language 'python
-   '((interpolation) @python--treesit-fontify-string-interpolation)
 
    :feature 'keyword
    :language 'python
@@ -1126,12 +1169,6 @@ python--treesit-settings
       name: (identifier) @font-lock-type-face)
      (parameters (identifier) @font-lock-variable-name-face))
 
-   :feature 'function
-   :language 'python
-   '((call function: (identifier) @font-lock-function-call-face)
-     (call function: (attribute
-                      attribute: (identifier) @font-lock-function-call-face)))
-
    :feature 'builtin
    :language 'python
    `(((identifier) @font-lock-builtin-face
@@ -1142,6 +1179,12 @@ python--treesit-settings
                       eol))
               @font-lock-builtin-face)))
 
+   :feature 'function
+   :language 'python
+   '((call function: (identifier) @font-lock-function-call-face)
+     (call function: (attribute
+                      attribute: (identifier) @font-lock-function-call-face)))
+
    :feature 'constant
    :language 'python
    '([(true) (false) (none)] @font-lock-constant-face)
@@ -1153,30 +1196,85 @@ python--treesit-settings
                  @font-lock-variable-name-face)
      (assignment left: (attribute
                         attribute: (identifier)
-                        @font-lock-property-use-face))
-     (pattern_list (identifier)
+                        @font-lock-variable-name-face))
+     (augmented_assignment left: (identifier)
+                           @font-lock-variable-name-face)
+     (named_expression name: (identifier)
+                       @font-lock-variable-name-face)
+     (pattern_list [(identifier)
+                    (list_splat_pattern (identifier))]
                    @font-lock-variable-name-face)
-     (tuple_pattern (identifier)
+     (tuple_pattern [(identifier)
+                     (list_splat_pattern (identifier))]
                     @font-lock-variable-name-face)
-     (list_pattern (identifier)
-                   @font-lock-variable-name-face)
-     (list_splat_pattern (identifier)
-                         @font-lock-variable-name-face))
+     (list_pattern [(identifier)
+                    (list_splat_pattern (identifier))]
+                   @font-lock-variable-name-face))
+
+   :feature 'variable-definition
+   :language 'python
+   `((for_statement left: (identifier)
+      @font-lock-variable-name-face)
+     (for_in_clause left: (identifier)
+                    @font-lock-variable-name-face)
+     (as_pattern_target (identifier)
+                        @font-lock-variable-name-face)
+     (case_pattern (as_pattern "as" :anchor (identifier)
+                               @font-lock-variable-name-face)))
 
    :feature 'decorator
    :language 'python
+   ;; Override function call face.
+   :override t
    '((decorator "@" @font-lock-type-face)
      (decorator (call function: (identifier) @font-lock-type-face))
-     (decorator (identifier) @font-lock-type-face))
+     (decorator (identifier) @font-lock-type-face)
+     (decorator [(attribute) (call (attribute))] @python--treesit-fontify-dotted-decorator))
 
    :feature 'type
    :language 'python
+   ;; Override built-in faces when dict/list are used for type hints.
+   :override t
    `(((identifier) @font-lock-type-face
       (:match ,(rx-to-string
                 `(seq bol (or ,@python--treesit-exceptions)
-                      eol))
+                  eol))
               @font-lock-type-face))
-     (type (identifier) @font-lock-type-face))
+     (type [(identifier) (none)] @font-lock-type-face)
+     (type (attribute attribute: (identifier) @font-lock-type-face))
+     ;; We don't want to highlight a package of the type
+     ;; (e.g. pack.ClassName).  So explicitly exclude patterns with
+     ;; attribute, since we handle dotted type name in the previous
+     ;; rule.  The following rule handle
+     ;; generic_type/list/tuple/splat_type nodes.
+     (type (_ !attribute [[(identifier) (none)] @font-lock-type-face
+                          (attribute attribute: (identifier) @font-lock-type-face) ]))
+     ;; collections.abc.Iterator[T] case.
+     (type (subscript (attribute attribute: (identifier) @font-lock-type-face)))
+     ;; Nested optional type hints, e.g. val: Lvl1 | Lvl2[Lvl3[Lvl4]].
+     (type (binary_operator) @python--treesit-fontify-union-types)
+     ;;class Type(Base1, Sequence[T]).
+     (class_definition
+      superclasses:
+      (argument_list [(identifier) @font-lock-type-face
+                      (attribute attribute: (identifier) @font-lock-type-face)
+                      (subscript (identifier) @font-lock-type-face)
+                      (subscript (attribute attribute: (identifier) @font-lock-type-face))]))
+
+     ;; Patern matching: case [str(), pack0.Type0()].  Take only the
+     ;; last identifier.
+     (class_pattern (dotted_name (identifier) @font-lock-type-face :anchor))
+
+
+     ;; Highlight the second argument as a type in isinstance/issubclass.
+     ((call function: (identifier) @func-name
+            (argument_list :anchor (_)
+                           [(identifier) @font-lock-type-face
+                            (attribute attribute: (identifier) @font-lock-type-face)
+                            (tuple (identifier) @font-lock-type-face)
+                            (tuple (attribute attribute: (identifier) @font-lock-type-face))
+                            (binary_operator) @python--treesit-fontify-union-types]))
+      (:match "^is\\(?:instance\\|subclass\\)$" @func-name)))
 
    :feature 'escape-sequence
    :language 'python
@@ -6841,7 +6939,7 @@ python-ts-mode
     (setq-local treesit-font-lock-feature-list
                 '(( comment definition)
                   ( keyword string type)
-                  ( assignment builtin constant decorator
+                  ( assignment variable-definition builtin constant decorator
                     escape-sequence number string-interpolation )
                   ( bracket delimiter function operator variable property)))
     (setq-local treesit-font-lock-settings python--treesit-settings)
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index a44a11896f..251b24eba2 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -7299,6 +7299,356 @@ python-tests--flymake-command-output-pattern
                          "Unused import a.b.c (unused-import)"
                        "W0611: Unused import a.b.c (unused-import)"))))))
 
+;;; python-ts-mode font-lock tests
+
+(defmacro python-ts-tests-with-temp-buffer (contents &rest body)
+  "Create a `python-ts-mode' enabled temp buffer with CONTENTS.
+BODY is code to be executed within the temp buffer.  Point is
+always located at the beginning of buffer."
+  (declare (indent 1) (debug t))
+  `(with-temp-buffer
+     (skip-unless (treesit-ready-p 'python))
+     (require 'python)
+     (let ((python-indent-guess-indent-offset nil))
+       (python-ts-mode)
+       (setopt treesit-font-lock-level 3)
+       (insert ,contents)
+       (font-lock-ensure)
+       (goto-char (point-min))
+       ,@body)))
+
+(ert-deftest python-ts-mode-compound-keywords-face ()
+  (dolist (test '("is not" "not in"))
+    (python-ts-tests-with-temp-buffer
+     (concat "t " test " t")
+     (forward-to-word)
+     (should (eq (face-at-point) font-lock-keyword-face))
+     (forward-to-word)
+     (should (eq (face-at-point) font-lock-keyword-face)))))
+
+(ert-deftest python-ts-mode-var-for-loop-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "for var in range(3):"
+   (dolist (test '("var"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-variable-name-face)))))
+
+(ert-deftest python-ts-mode-var-for-loop-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "for var1, (var2, var3) in []:"
+   (dolist (test '("var1" "var2" "var3"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-variable-name-face)))))
+
+(ert-deftest python-ts-mode-var-for-loop-face-3 ()
+  (python-ts-tests-with-temp-buffer
+   "[var for var in [] if var ]"
+   (search-forward "var")
+   (goto-char (match-beginning 0))
+   (should (not (eq (face-at-point) font-lock-variable-name-face)))
+
+   (search-forward "var" nil nil 2)
+   (goto-char (match-beginning 0))
+   (should (eq (face-at-point) font-lock-variable-name-face))
+
+   (search-forward "var" nil nil 2)
+   (goto-char (match-beginning 0))
+   (should (not (eq (face-at-point) font-lock-variable-name-face)))))
+
+(ert-deftest python-ts-mode-as-pattern-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "try:
+    pass
+except Exception as excp:
+    pass"
+
+   (search-forward "excp")
+   (goto-char (match-beginning 0))
+   (should (eq (face-at-point) font-lock-variable-name-face))))
+
+(ert-deftest python-ts-mode-as-pattern-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "with ctx() as var:"
+   (search-forward "var")
+   (goto-char (match-beginning 0))
+   (should (eq (face-at-point) font-lock-variable-name-face))))
+
+(ert-deftest python-ts-mode-as-pattern-face-3 ()
+  (python-ts-tests-with-temp-buffer
+   "with ctx() as var:"
+   (search-forward "var")
+   (goto-char (match-beginning 0))
+   (should (eq (face-at-point) font-lock-variable-name-face))))
+
+(ert-deftest python-ts-mode-as-pattern-face-4 ()
+  (python-ts-tests-with-temp-buffer
+   "match v:
+    case (list() as lvar, Inst() as ivar):"
+   (dolist (test '("lvar" "ivar"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-variable-name-face)))))
+
+(ert-deftest python-ts-mode-named-assignement-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "var := 3"
+   (should (eq (face-at-point) font-lock-variable-name-face))))
+
+(ert-deftest python-ts-mode-assignement-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "var, *rest = call()"
+   (dolist (test '("var" "rest"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-variable-name-face))))
+
+  (python-ts-tests-with-temp-buffer
+   "def func(*args):"
+   (dolist (test '("args"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-variable-name-face))))))
+
+(ert-deftest python-ts-mode-nested-types-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "def func(v:dict[ list[ tuple[str] ], int | None] | None):"
+   (dolist (test '("dict" "list" "tuple" "str" "int" "None" "None"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-union-types-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "def f(val: tuple[tuple, list[Lvl1 | Lvl2[Lvl3[Lvl4[Lvl5 | None]], Lvl2]]]):"
+   (dolist (test '("tuple" "tuple" "list" "Lvl1" "Lvl2" "Lvl3" "Lvl4" "Lvl5" "None" "Lvl2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-union-types-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "def f(val: Type0 | Type1[Type2, pack0.Type3] | pack1.pack2.Type4 | None):"
+   (dolist (test '("Type0" "Type1" "Type2" "Type3" "Type4" "None"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+
+   (goto-char (point-min))
+   (dolist (test '("pack0" "pack1" "pack2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-types-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "def f(val: Callable[[Type0], (Type1, Type2)]):"
+   (dolist (test '("Callable" "Type0" "Type1" "Type2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-types-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "def annot3(val:pack0.Type0)->pack1.pack2.pack3.Type1:"
+   (dolist (test '("Type0" "Type1"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+   (goto-char (point-min))
+   (dolist (test '("pack0" "pack1" "pack2" "pack3"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-types-face-3 ()
+  (python-ts-tests-with-temp-buffer
+   "def annot3(val:collections.abc.Iterator[Type0]):"
+   (dolist (test '("Iterator" "Type0"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+   (goto-char (point-min))
+   (dolist (test '("collections" "abc"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-isinstance-type-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "isinstance(var1, pkg.Type0)
+    isinstance(var2, (str, dict, Type1, type(None)))
+    isinstance(var3, my_type())"
+
+   (dolist (test '("var1" "pkg" "var2" "type" "None" "var3" "my_type"))
+     (let ((case-fold-search nil))
+       (search-forward test))
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))
+
+   (goto-char (point-min))
+   (dolist (test '("Type0" "str" "dict" "Type1"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-isinstance-type-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "issubclass(mytype, int|list|collections.abc.Iterable)"
+   (dolist (test '("int" "list" "Iterable"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-superclass-type-face ()
+  (python-ts-tests-with-temp-buffer
+   "class Temp(Base1, pack0.Base2,  Sequence[T1, T2]):"
+
+   (dolist (test '("Base1" "Base2" "Sequence" "T1" "T2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+
+   (goto-char (point-min))
+   (dolist (test '("pack0"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-class-patterns-face ()
+  (python-ts-tests-with-temp-buffer
+   "match tt:
+        case str():
+            pass
+        case [Type0() | bytes(b) | pack0.pack1.Type1()]:
+            pass
+        case {'i': int(i), 'f': float() as f}:
+            pass"
+
+   (dolist (test '("str" "Type0" "bytes" "Type1" "int" "float"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+
+   (goto-char (point-min))
+   (dolist (test '("pack0" "pack1"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-dotted-decorator-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "@pytest.mark.skip
+    @pytest.mark.skip(reason='msg')
+    def test():"
+
+   (dolist (test '("pytest" "mark" "skip" "pytest" "mark" "skip"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-dotted-decorator-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "@pytest.mark.skip(reason='msg')
+    def test():"
+
+   (setopt treesit-font-lock-level 4)
+   (dolist (test '("pytest" "mark" "skip"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-builtin-call-face ()
+  (python-ts-tests-with-temp-buffer
+   "all()"
+   ;; enable 'function' feature from 4th level
+   (setopt treesit-font-lock-level 4)
+   (should (eq (face-at-point) font-lock-builtin-face))))
+
+(ert-deftest python-ts-mode-interpolation-nested-string ()
+  (python-ts-tests-with-temp-buffer
+   "t = f\"beg {True + 'string'}\""
+
+   (search-forward "True")
+   (goto-char (match-beginning 0))
+   (should (eq (face-at-point) font-lock-constant-face))
+
+   (goto-char (point-min))
+   (dolist (test '("f" "{" "+" "}"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-string-face))))
+
+
+   (goto-char (point-min))
+   (dolist (test '("beg" "'string'" "\""))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-string-face)))))
+
+(ert-deftest python-ts-mode-level-fontification-wo-interpolation ()
+  (python-ts-tests-with-temp-buffer
+   "t = f\"beg {True + var}\""
+
+   (setopt treesit-font-lock-level 2)
+   (search-forward "f")
+   (goto-char (match-beginning 0))
+   (should (not (eq (face-at-point) font-lock-string-face)))
+
+   (dolist (test '("\"" "beg" "{" "True" "var" "}" "\""))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-string-face)))))
+
+(ert-deftest python-ts-mode-disabled-string-interpolation ()
+  (python-ts-tests-with-temp-buffer
+   "t = f\"beg {True + var}\""
+
+   (unwind-protect
+       (progn
+         (setf (nth 2 treesit-font-lock-feature-list)
+               (remq 'string-interpolation (nth 2 treesit-font-lock-feature-list)))
+         (setopt treesit-font-lock-level 3)
+
+         (search-forward "f")
+         (goto-char (match-beginning 0))
+         (should (not (eq (face-at-point) font-lock-string-face)))
+
+         (dolist (test '("\"" "beg" "{" "True" "var" "}" "\""))
+           (search-forward test)
+           (goto-char (match-beginning 0))
+           (should (eq (face-at-point) font-lock-string-face))))
+
+    (setf (nth 2 treesit-font-lock-feature-list)
+          (append (nth 2 treesit-font-lock-feature-list) '(string-interpolation))))))
+
+(ert-deftest python-ts-mode-interpolation-doc-string ()
+  (python-ts-tests-with-temp-buffer
+   "f\"\"\"beg {'s1' + True + 's2'} end\"\"\""
+
+   (search-forward "True")
+   (goto-char (match-beginning 0))
+   (should (eq (face-at-point) font-lock-constant-face))
+
+   (goto-char (point-min))
+   (dolist (test '("f" "{" "+" "}"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-string-face))))
+
+   (goto-char (point-min))
+   (dolist (test '("\"\"\"" "beg" "end" "\"\"\""))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-doc-face)))
+
+   (goto-char (point-min))
+   (dolist (test '("'s1'" "'s2'"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-string-face)))))
+
 (provide 'python-tests)
 
 ;;; python-tests.el ends here
-- 
2.34.1

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Yuan Fu]

> On Dec 16, 2023, at 4:26 PM, Denis Zubarev <dvzubarev@yandex.ru> wrote:
> 
> Sorry for the delayed response.
>  > For many Emacs users, they want a quieter or even minimal fontification.
>  I'm not against it. I just think that highlighting of an interpolation
> as a string is wrong. Is it possible to set quiet fontification in
> emacs-lisp mode, in such a way that `keywords' in doc-strings were
> fontified as a doc-string itself? I think it is similar to
> interpolation, it serves the purpose of separating different semantic
> elements from each other. IMHO, users who like quiet levels will benefit
> from interpolation highlighted differently.

I definitely can see your point, and it makes a lot of sense. I don’t really know people who want quieter fontification wants (probably both approach has their supporters) so can’t speak for them. But in general, I think it wouldn’t hurt to have the option.

>  > Instead of the third level, the check should use the value
> > treesit-font-lock-level. And it should check for each level smaller than
> > or equal to treesit-font-lock-level.
>  Done.

Thank you for your hard work! I’m just here talking and you went ahead and did all the work :-)

>  > Non-essential fontification like "var" shouldn't be
> > activated at that level. So I suggest to put it in the variable feature,
> > along with many other non-essential fontifications. (Variable feature is
> > placed at level 4.)
>  I added a new feature variable-definition for variables defined for local scopes and put it on the 3rd level.
> I also added rules to variable-definition feature for variables in list
> comprehension ( [var+1 for var in []] ) and as_pattern (with T as var:,
> except E as var:, case str() as var:).
>  I've noticed that vars in `for var1, (var2, var3) in []:` are highlighted by the rule from the assignment feature (specifically `pattern_list`, `tuple_pattern`).
> It seems easy to fix `pattern_list`, but not so easy for
> `tuple_pattern`, since this node may occur recursively.
> I didn't touch these rules for now.

Ok, makes sense.

>    Summary for all changes in the patch.
>  New feature variable-definition:
> `for var in range(3)`
> `[var+1 for var in []]`
> `with T as var:`
> `except E as var:`
> `case str() as var:`
> highlight var as font-lock-variable-name-face
>  assignment feature:
> var := 3 (named_expression)
> var *= 3 (augmented_assignment)
> Highlight var as font-lock-variable-name-face.
>  Make list_splat_pattern query more precise.
> list_splat_pattern may appear not only in assignments: var, *rest = call(),
> but in the parameter list too: def f(*args).
> Highlight args only for the first case in assignment feature.
>   type feature:
> Fontify built-ins (dict,list,etc.) as types when they are used in type hints.
> support nested union types, for example `Lvl1 | Lvl2[Lvl3[Lvl3], Lvl2]`.
> This structure is represented via nesting binary_operator and subscript nodes in the grammar.
> Function python--treesit-fontify-union-types iterates over all children and highlight identifier nodes.
>  Fontify base class names in the class definition: class Temp(Base1, pack0.Base2):
> Fontify class patterns in case statement: case [TempC() | bytes(b)]:
> Highlight the second argument as a type in isinstance/issubclass call:
> isinstance(var2, (str, dict, Type1)); issubclass(var1, int|str)
>  For all dotted names of a type highlight only the last part of the name,
> e.g. collections.abc.Iterator.
>   decorator feature:
> Highlight dotted names: @pytest.mark.skip
> Function python--treesit-fontify-dotted-decorator iterates over all nested attribute nodes and highlight identifier nodes.
>  When font-lock-level is set 4, `skip` had function-call face in: @pytest.mark.skip(reason='t')
> Add `:override t` to decorator feature to override function-call face.
>   string feature:
> Enable interpolation highlighting only if string-interpolation is
> presented on the enabled levels of treesit-font-lock-feature-list.
> Fix fontification of strings inside of f-strings interpolation,
> e.g. for f"beg {'nested'}" - 'nested' was not fontified as string.
>   function feature:
> Do not override the face of builtin functions (all, bytes etc.) with
> the function call face
>   keyword feature:
> Add "is not"  to the `python--treesit-keywords` list.

Thanks. I think the only thing that’s still up to discussion is the variable-definition rules. Others can be merged to emacs-29.

Yuan

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Denis Zubarev]

[-- Attachment #1: Type: text/html, Size: 5574 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0005-Improve-syntax-highlighting-for-python-ts-mode.patch --]
[-- Type: text/x-diff; name="0005-Improve-syntax-highlighting-for-python-ts-mode.patch", Size: 24674 bytes --]

From dd4863ad55310b084839df8f508bf57364d489b7 Mon Sep 17 00:00:00 2001
From: Denis Zubarev <dvzubarev@yandex.ru>
Date: Sat, 11 Nov 2023 04:55:44 +0300
Subject: [PATCH] Improve syntax highlighting for python-ts-mode

Fix fontification of strings inside of f-strings interpolation, e.g. for
f"beg {'nested'}" - 'nested' was not fontified as string.  Do not
override the face of builtin functions (all, bytes etc.) with the
function call face.  Add missing assignment expressions (:= *=).
Highlight variables defined in for loop (for var1, var2 in ).  Fontify
built-ins (dict,list,etc.) as types when they are used in type hints.
Highlight union types (type1|type2).  Highlight base class names in the
class definition.  Fontify class patterns in case statement. Highlight
the second argument as a type in isinstance/issubclass call. Highlight
dotted decorator names.

* lisp/progmodes/python.el (python--treesit-keywords): Add compound
keyword "is not".
(python--treesit-fontify-string): fix f-string interpolation. Enable
interpolation highlighting only if string-interpolation is presented on
the enabled levels of treesit-font-lock-feature-list.
(python--treesit-fontify-string-interpolation): Remove function.
(python--treesit-fontify-union-types): Fontify nested union types.
(python--treesit-fontify-dotted-decorator): Fontify all parts of
dotted decorator name.
(python--treesit-settings): Change/add rules.
* test/lisp/progmodes/python-tests.el (python-ts-tests-with-temp-buffer):
function for setting up test buffer.
(python-ts-mode-compound-keywords-face)
(python-ts-mode-var-for-loop-face)
(python-ts-mode-named-assignement-face-1)
(python-ts-mode-assignement-face-2)
(python-ts-mode-nested-types-face-1)
(python-ts-mode-union-types-face-1)
(python-ts-mode-union-types-face-2)
(python-ts-mode-types-face-1)
(python-ts-mode-types-face-2)
(python-ts-mode-types-face-3)
(python-ts-mode-isinstance-type-face-1)
(python-ts-mode-isinstance-type-face-2)
(python-ts-mode-superclass-type-face)
(python-ts-mode-class-patterns-face)
(python-ts-mode-dotted-decorator-face-1)
(python-ts-mode-dotted-decorator-face-2)
(python-ts-mode-builtin-call-face)
(python-ts-mode-interpolation-nested-string)
(python-ts-mode-disabled-string-interpolation)
(python-ts-mode-interpolation-doc-string): Add tests.
---
 lisp/progmodes/python.el            | 183 +++++++++++++-----
 test/lisp/progmodes/python-tests.el | 285 ++++++++++++++++++++++++++++
 2 files changed, 420 insertions(+), 48 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index ab3bf1b4ec..1b33a45965 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -979,7 +979,7 @@ python--treesit-keywords
     "raise" "return" "try" "while" "with" "yield"
     ;; These are technically operators, but we fontify them as
     ;; keywords.
-    "and" "in" "is" "not" "or" "not in"))
+    "and" "in" "is" "not" "or" "not in" "is not"))
 
 (defvar python--treesit-builtins
   '("abs" "all" "any" "ascii" "bin" "bool" "breakpoint" "bytearray"
@@ -1042,9 +1042,7 @@ python--treesit-fontify-string
 f-strings.  OVERRIDE is the override flag described in
 `treesit-font-lock-rules'.  START and END mark the region to be
 fontified."
-  (let* ((string-beg (treesit-node-start node))
-         (string-end (treesit-node-end node))
-         (maybe-expression (treesit-node-parent node))
+  (let* ((maybe-expression (treesit-node-parent node))
          (grandparent (treesit-node-parent
                        (treesit-node-parent
                         maybe-expression)))
@@ -1072,28 +1070,78 @@ python--treesit-fontify-string
                         (equal (treesit-node-type maybe-expression)
                                "expression_statement"))
                    'font-lock-doc-face
-                 'font-lock-string-face)))
-    ;; Don't highlight string prefixes like f/r/b.
-    (save-excursion
-      (goto-char string-beg)
-      (when (re-search-forward "[\"']" string-end t)
-        (setq string-beg (match-beginning 0))))
-    (treesit-fontify-with-override
-     string-beg string-end face override start end)))
-
-(defun python--treesit-fontify-string-interpolation
-    (node _ start end &rest _)
-  "Fontify string interpolation.
-NODE is the string node.  Do not fontify the initial f for
-f-strings.  START and END mark the region to be
+                 'font-lock-string-face))
+
+         (ignore-interpolation (not
+                                (seq-some
+                                 (lambda (feats) (memq 'string-interpolation feats))
+                                 (seq-take treesit-font-lock-feature-list treesit-font-lock-level))))
+         ;; If interpolation is enabled, highlight only
+         ;; string_start/string_content/string_end children.  Do not
+         ;; touch interpolation node that can occur inside of the
+         ;; string.
+         (string-nodes (if ignore-interpolation
+                           (list node)
+                         (treesit-filter-child
+                          node
+                          (lambda (ch) (member (treesit-node-type ch)
+                                               '("string_start"
+                                                 "string_content"
+                                                 "string_end")))
+                          t))))
+
+    (dolist (string-node string-nodes)
+      (let ((string-beg (treesit-node-start string-node))
+            (string-end (treesit-node-end string-node)))
+        (when (or ignore-interpolation
+                  (equal (treesit-node-type string-node) "string_start"))
+          ;; Don't highlight string prefixes like f/r/b.
+          (save-excursion
+            (goto-char string-beg)
+            (when (re-search-forward "[\"']" string-end t)
+              (setq string-beg (match-beginning 0)))))
+
+        (treesit-fontify-with-override
+         string-beg string-end face override start end)))))
+
+(defun python--treesit-fontify-union-types (node override start end &rest _)
+  "Fontify nested union types in the type hints.
+For examlpe, Lvl1 | Lvl2[Lvl3[Lvl4[Lvl5 | None]], Lvl2].  This
+structure is represented via nesting binary_operator and
+subscript nodes.  This function iterates over all levels and
+highlight identifier nodes.  NODE is the binary_operator
+node.  OVERRIDE is the override flag described in
+`treesit-font-lock-rules'.  START and END mark the region to be
 fontified."
-  ;; This is kind of a hack, it basically removes the face applied by
-  ;; the string feature, so that following features can apply their
-  ;; face.
-  (let ((n-start (treesit-node-start node))
-        (n-end (treesit-node-end node)))
-    (remove-text-properties
-     (max start n-start) (min end n-end) '(face))))
+  (dolist (child (treesit-node-children node t))
+    (pcase (treesit-node-type child)
+      ((or "identifier" "none")
+       (treesit-fontify-with-override
+        (treesit-node-start child) (treesit-node-end child)
+        'font-lock-type-face override start end))
+      ("attribute"
+       (when-let ((type-node (treesit-node-child-by-field-name child "attribute")))
+         (treesit-fontify-with-override
+          (treesit-node-start type-node) (treesit-node-end type-node)
+          'font-lock-type-face override start end)))
+      ((or "binary_operator" "subscript")
+       (python--treesit-fontify-union-types child override start end)))))
+
+(defun python--treesit-fontify-dotted-decorator (node override start end &rest _)
+  "Fontify dotted decorators.
+For example @pytes.mark.skip.  Iterate over all nested attribute
+nodes and highlight identifier nodes.  NODE is the first attribute
+node.  OVERRIDE is the override flag described in
+`treesit-font-lock-rules'.  START and END mark the region to be
+fontified."
+  (dolist (child (treesit-node-children node t))
+    (pcase (treesit-node-type child)
+      ("identifier"
+       (treesit-fontify-with-override
+        (treesit-node-start child) (treesit-node-end child)
+        'font-lock-type-face override start end))
+      ("attribute"
+       (python--treesit-fontify-dotted-decorator child override start end)))))
 
 (defvar python--treesit-settings
   (treesit-font-lock-rules
@@ -1103,14 +1151,9 @@ python--treesit-settings
 
    :feature 'string
    :language 'python
-   '((string) @python--treesit-fontify-string)
+   '((string) @python--treesit-fontify-string
+     (interpolation ["{" "}"] @font-lock-misc-punctuation-face))
 
-   ;; HACK: This feature must come after the string feature and before
-   ;; other features.  Maybe we should make string-interpolation an
-   ;; option rather than a feature.
-   :feature 'string-interpolation
-   :language 'python
-   '((interpolation) @python--treesit-fontify-string-interpolation)
 
    :feature 'keyword
    :language 'python
@@ -1126,12 +1169,6 @@ python--treesit-settings
       name: (identifier) @font-lock-type-face)
      (parameters (identifier) @font-lock-variable-name-face))
 
-   :feature 'function
-   :language 'python
-   '((call function: (identifier) @font-lock-function-call-face)
-     (call function: (attribute
-                      attribute: (identifier) @font-lock-function-call-face)))
-
    :feature 'builtin
    :language 'python
    `(((identifier) @font-lock-builtin-face
@@ -1142,6 +1179,12 @@ python--treesit-settings
                       eol))
               @font-lock-builtin-face)))
 
+   :feature 'function
+   :language 'python
+   '((call function: (identifier) @font-lock-function-call-face)
+     (call function: (attribute
+                      attribute: (identifier) @font-lock-function-call-face)))
+
    :feature 'constant
    :language 'python
    '([(true) (false) (none)] @font-lock-constant-face)
@@ -1153,30 +1196,74 @@ python--treesit-settings
                  @font-lock-variable-name-face)
      (assignment left: (attribute
                         attribute: (identifier)
-                        @font-lock-property-use-face))
-     (pattern_list (identifier)
+                        @font-lock-variable-name-face))
+     (augmented_assignment left: (identifier)
+                           @font-lock-variable-name-face)
+     (named_expression name: (identifier)
+                       @font-lock-variable-name-face)
+     (pattern_list [(identifier)
+                    (list_splat_pattern (identifier))]
                    @font-lock-variable-name-face)
-     (tuple_pattern (identifier)
+     (tuple_pattern [(identifier)
+                     (list_splat_pattern (identifier))]
                     @font-lock-variable-name-face)
-     (list_pattern (identifier)
-                   @font-lock-variable-name-face)
-     (list_splat_pattern (identifier)
-                         @font-lock-variable-name-face))
+     (list_pattern [(identifier)
+                    (list_splat_pattern (identifier))]
+                   @font-lock-variable-name-face))
 
    :feature 'decorator
    :language 'python
+   ;; Override function call face.
+   :override t
    '((decorator "@" @font-lock-type-face)
      (decorator (call function: (identifier) @font-lock-type-face))
-     (decorator (identifier) @font-lock-type-face))
+     (decorator (identifier) @font-lock-type-face)
+     (decorator [(attribute) (call (attribute))] @python--treesit-fontify-dotted-decorator))
 
    :feature 'type
    :language 'python
+   ;; Override built-in faces when dict/list are used for type hints.
+   :override t
    `(((identifier) @font-lock-type-face
       (:match ,(rx-to-string
                 `(seq bol (or ,@python--treesit-exceptions)
-                      eol))
+                  eol))
               @font-lock-type-face))
-     (type (identifier) @font-lock-type-face))
+     (type [(identifier) (none)] @font-lock-type-face)
+     (type (attribute attribute: (identifier) @font-lock-type-face))
+     ;; We don't want to highlight a package of the type
+     ;; (e.g. pack.ClassName).  So explicitly exclude patterns with
+     ;; attribute, since we handle dotted type name in the previous
+     ;; rule.  The following rule handle
+     ;; generic_type/list/tuple/splat_type nodes.
+     (type (_ !attribute [[(identifier) (none)] @font-lock-type-face
+                          (attribute attribute: (identifier) @font-lock-type-face) ]))
+     ;; collections.abc.Iterator[T] case.
+     (type (subscript (attribute attribute: (identifier) @font-lock-type-face)))
+     ;; Nested optional type hints, e.g. val: Lvl1 | Lvl2[Lvl3[Lvl4]].
+     (type (binary_operator) @python--treesit-fontify-union-types)
+     ;;class Type(Base1, Sequence[T]).
+     (class_definition
+      superclasses:
+      (argument_list [(identifier) @font-lock-type-face
+                      (attribute attribute: (identifier) @font-lock-type-face)
+                      (subscript (identifier) @font-lock-type-face)
+                      (subscript (attribute attribute: (identifier) @font-lock-type-face))]))
+
+     ;; Patern matching: case [str(), pack0.Type0()].  Take only the
+     ;; last identifier.
+     (class_pattern (dotted_name (identifier) @font-lock-type-face :anchor))
+
+
+     ;; Highlight the second argument as a type in isinstance/issubclass.
+     ((call function: (identifier) @func-name
+            (argument_list :anchor (_)
+                           [(identifier) @font-lock-type-face
+                            (attribute attribute: (identifier) @font-lock-type-face)
+                            (tuple (identifier) @font-lock-type-face)
+                            (tuple (attribute attribute: (identifier) @font-lock-type-face))
+                            (binary_operator) @python--treesit-fontify-union-types]))
+      (:match "^is\\(?:instance\\|subclass\\)$" @func-name)))
 
    :feature 'escape-sequence
    :language 'python
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index a44a11896f..fd4d593613 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -7299,6 +7299,291 @@ python-tests--flymake-command-output-pattern
                          "Unused import a.b.c (unused-import)"
                        "W0611: Unused import a.b.c (unused-import)"))))))
 
+;;; python-ts-mode font-lock tests
+
+(defmacro python-ts-tests-with-temp-buffer (contents &rest body)
+  "Create a `python-ts-mode' enabled temp buffer with CONTENTS.
+BODY is code to be executed within the temp buffer.  Point is
+always located at the beginning of buffer."
+  (declare (indent 1) (debug t))
+  `(with-temp-buffer
+     (skip-unless (treesit-ready-p 'python))
+     (require 'python)
+     (let ((python-indent-guess-indent-offset nil))
+       (python-ts-mode)
+       (setopt treesit-font-lock-level 3)
+       (insert ,contents)
+       (font-lock-ensure)
+       (goto-char (point-min))
+       ,@body)))
+
+(ert-deftest python-ts-mode-compound-keywords-face ()
+  (dolist (test '("is not" "not in"))
+    (python-ts-tests-with-temp-buffer
+     (concat "t " test " t")
+     (forward-to-word)
+     (should (eq (face-at-point) font-lock-keyword-face))
+     (forward-to-word)
+     (should (eq (face-at-point) font-lock-keyword-face)))))
+
+(ert-deftest python-ts-mode-named-assignement-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "var := 3"
+   (should (eq (face-at-point) font-lock-variable-name-face))))
+
+(ert-deftest python-ts-mode-assignement-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "var, *rest = call()"
+   (dolist (test '("var" "rest"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-variable-name-face))))
+
+  (python-ts-tests-with-temp-buffer
+   "def func(*args):"
+   (dolist (test '("args"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-variable-name-face))))))
+
+(ert-deftest python-ts-mode-nested-types-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "def func(v:dict[ list[ tuple[str] ], int | None] | None):"
+   (dolist (test '("dict" "list" "tuple" "str" "int" "None" "None"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-union-types-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "def f(val: tuple[tuple, list[Lvl1 | Lvl2[Lvl3[Lvl4[Lvl5 | None]], Lvl2]]]):"
+   (dolist (test '("tuple" "tuple" "list" "Lvl1" "Lvl2" "Lvl3" "Lvl4" "Lvl5" "None" "Lvl2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-union-types-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "def f(val: Type0 | Type1[Type2, pack0.Type3] | pack1.pack2.Type4 | None):"
+   (dolist (test '("Type0" "Type1" "Type2" "Type3" "Type4" "None"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+
+   (goto-char (point-min))
+   (dolist (test '("pack0" "pack1" "pack2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-types-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "def f(val: Callable[[Type0], (Type1, Type2)]):"
+   (dolist (test '("Callable" "Type0" "Type1" "Type2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-types-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "def annot3(val:pack0.Type0)->pack1.pack2.pack3.Type1:"
+   (dolist (test '("Type0" "Type1"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+   (goto-char (point-min))
+   (dolist (test '("pack0" "pack1" "pack2" "pack3"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-types-face-3 ()
+  (python-ts-tests-with-temp-buffer
+   "def annot3(val:collections.abc.Iterator[Type0]):"
+   (dolist (test '("Iterator" "Type0"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+   (goto-char (point-min))
+   (dolist (test '("collections" "abc"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-isinstance-type-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "isinstance(var1, pkg.Type0)
+    isinstance(var2, (str, dict, Type1, type(None)))
+    isinstance(var3, my_type())"
+
+   (dolist (test '("var1" "pkg" "var2" "type" "None" "var3" "my_type"))
+     (let ((case-fold-search nil))
+       (search-forward test))
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))
+
+   (goto-char (point-min))
+   (dolist (test '("Type0" "str" "dict" "Type1"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-isinstance-type-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "issubclass(mytype, int|list|collections.abc.Iterable)"
+   (dolist (test '("int" "list" "Iterable"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-superclass-type-face ()
+  (python-ts-tests-with-temp-buffer
+   "class Temp(Base1, pack0.Base2,  Sequence[T1, T2]):"
+
+   (dolist (test '("Base1" "Base2" "Sequence" "T1" "T2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+
+   (goto-char (point-min))
+   (dolist (test '("pack0"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-class-patterns-face ()
+  (python-ts-tests-with-temp-buffer
+   "match tt:
+        case str():
+            pass
+        case [Type0() | bytes(b) | pack0.pack1.Type1()]:
+            pass
+        case {'i': int(i), 'f': float() as f}:
+            pass"
+
+   (dolist (test '("str" "Type0" "bytes" "Type1" "int" "float"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+
+   (goto-char (point-min))
+   (dolist (test '("pack0" "pack1"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-dotted-decorator-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "@pytest.mark.skip
+    @pytest.mark.skip(reason='msg')
+    def test():"
+
+   (dolist (test '("pytest" "mark" "skip" "pytest" "mark" "skip"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-dotted-decorator-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "@pytest.mark.skip(reason='msg')
+    def test():"
+
+   (setopt treesit-font-lock-level 4)
+   (dolist (test '("pytest" "mark" "skip"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-builtin-call-face ()
+  (python-ts-tests-with-temp-buffer
+   "all()"
+   ;; enable 'function' feature from 4th level
+   (setopt treesit-font-lock-level 4)
+   (should (eq (face-at-point) font-lock-builtin-face))))
+
+(ert-deftest python-ts-mode-interpolation-nested-string ()
+  (python-ts-tests-with-temp-buffer
+   "t = f\"beg {True + 'string'}\""
+
+   (search-forward "True")
+   (goto-char (match-beginning 0))
+   (should (eq (face-at-point) font-lock-constant-face))
+
+   (goto-char (point-min))
+   (dolist (test '("f" "{" "+" "}"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-string-face))))
+
+
+   (goto-char (point-min))
+   (dolist (test '("beg" "'string'" "\""))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-string-face)))))
+
+(ert-deftest python-ts-mode-level-fontification-wo-interpolation ()
+  (python-ts-tests-with-temp-buffer
+   "t = f\"beg {True + var}\""
+
+   (setopt treesit-font-lock-level 2)
+   (search-forward "f")
+   (goto-char (match-beginning 0))
+   (should (not (eq (face-at-point) font-lock-string-face)))
+
+   (dolist (test '("\"" "beg" "{" "True" "var" "}" "\""))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-string-face)))))
+
+(ert-deftest python-ts-mode-disabled-string-interpolation ()
+  (python-ts-tests-with-temp-buffer
+   "t = f\"beg {True + var}\""
+
+   (unwind-protect
+       (progn
+         (setf (nth 2 treesit-font-lock-feature-list)
+               (remq 'string-interpolation (nth 2 treesit-font-lock-feature-list)))
+         (setopt treesit-font-lock-level 3)
+
+         (search-forward "f")
+         (goto-char (match-beginning 0))
+         (should (not (eq (face-at-point) font-lock-string-face)))
+
+         (dolist (test '("\"" "beg" "{" "True" "var" "}" "\""))
+           (search-forward test)
+           (goto-char (match-beginning 0))
+           (should (eq (face-at-point) font-lock-string-face))))
+
+    (setf (nth 2 treesit-font-lock-feature-list)
+          (append (nth 2 treesit-font-lock-feature-list) '(string-interpolation))))))
+
+(ert-deftest python-ts-mode-interpolation-doc-string ()
+  (python-ts-tests-with-temp-buffer
+   "f\"\"\"beg {'s1' + True + 's2'} end\"\"\""
+
+   (search-forward "True")
+   (goto-char (match-beginning 0))
+   (should (eq (face-at-point) font-lock-constant-face))
+
+   (goto-char (point-min))
+   (dolist (test '("f" "{" "+" "}"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-string-face))))
+
+   (goto-char (point-min))
+   (dolist (test '("\"\"\"" "beg" "end" "\"\"\""))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-doc-face)))
+
+   (goto-char (point-min))
+   (dolist (test '("'s1'" "'s2'"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-string-face)))))
+
 (provide 'python-tests)
 
 ;;; python-tests.el ends here
-- 
2.34.1

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Eli Zaretskii]

> From: Denis Zubarev <dvzubarev@yandex.ru>
> Cc: Dmitry Gutov <dmitry@gutov.dev>,
> 	Eli Zaretskii <eliz@gnu.org>,
> 	"67061@debbugs.gnu.org" <67061@debbugs.gnu.org>
> Date: Sun, 17 Dec 2023 05:07:01 +0300
> 
> > Thanks. I think the only thing that’s still up to discussion is the variable-definition rules. Others can
> be merged to emacs-29.
>  
> I can extract part with variable-definition into the next patch.
> In case it is ok, I attached patch without new variable-definition feature.

Thanks, but it doesn't apply cleanly to the current emacs-29 branch.
Would you mind please rebasing the patch on the emacs-29 branch and
resubmitting it?

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Denis Zubarev]

[-- Attachment #1: Type: text/html, Size: 1579 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0008-Improve-syntax-highlighting-for-python-ts-mode.patch --]
[-- Type: text/x-diff; name="0008-Improve-syntax-highlighting-for-python-ts-mode.patch", Size: 28552 bytes --]

From b7a170673e26e9cf2d049107a72ce6df0a8a230e Mon Sep 17 00:00:00 2001
From: Denis Zubarev <dvzubarev@yandex.ru>
Date: Sat, 11 Nov 2023 04:55:44 +0300
Subject: [PATCH] Improve syntax highlighting for python-ts-mode

Fix fontification of strings inside of f-strings interpolation, e.g. for
f"beg {'nested'}" - 'nested' was not fontified as string.  Do not
override the face of builtin functions (all, bytes etc.) with the
function call face.  Add missing assignment expressions (:= *=).
Fontify built-ins (dict,list,etc.) as types when they are used in type
hints.  Highlight union types (type1|type2).  Highlight base class names
in the class definition.  Fontify class patterns in case statements.
Highlight the second argument as a type in isinstance/issubclass call.
Highlight dotted decorator names.

* lisp/progmodes/python.el (python--treesit-keywords): Add compound
keyword "is not".
(python--treesit-builtin-types): New variable that stores all python
built-in types.
(python--treesit-type-regex): New variable. Regex matches if text is either
built-in type or text starts with capital letter.
(python--treesit-builtins): Extract built-in types to other variable.
(python--treesit-fontify-string): fix f-string interpolation. Enable
interpolation highlighting only if string-interpolation is presented on
the enabled levels of treesit-font-lock-feature-list.
(python--treesit-fontify-string-interpolation): Remove function.
(python--treesit-fontify-union-types): Fontify nested union types.
(python--treesit-fontify-union-types-strict): Fontify nested union
types, only if type identifier matches against
python--treesit-type-regex.
(python--treesit-fontify-dotted-decorator): Fontify all parts of
dotted decorator name.
(python--treesit-settings): Change/add rules.
* test/lisp/progmodes/python-tests.el (python-ts-tests-with-temp-buffer):
function for setting up test buffer.
(python-ts-mode-compound-keywords-face)
(python-ts-mode-named-assignement-face-1)
(python-ts-mode-assignement-face-2)
(python-ts-mode-nested-types-face-1)
(python-ts-mode-union-types-face-1)
(python-ts-mode-union-types-face-2)
(python-ts-mode-types-face-1)
(python-ts-mode-types-face-2)
(python-ts-mode-types-face-3)
(python-ts-mode-isinstance-type-face-1)
(python-ts-mode-isinstance-type-face-2)
(python-ts-mode-isinstance-type-face-3)
(python-ts-mode-superclass-type-face)
(python-ts-mode-class-patterns-face)
(python-ts-mode-dotted-decorator-face-1)
(python-ts-mode-dotted-decorator-face-2)
(python-ts-mode-builtin-call-face)
(python-ts-mode-interpolation-nested-string)
(python-ts-mode-disabled-string-interpolation)
(python-ts-mode-interpolation-doc-string): Add tests.
---
 lisp/progmodes/python.el            | 240 ++++++++++++++++------
 test/lisp/progmodes/python-tests.el | 302 ++++++++++++++++++++++++++++
 2 files changed, 480 insertions(+), 62 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index d7250148fa..f5f89ad552 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -969,19 +969,30 @@ python--treesit-keywords
     "raise" "return" "try" "while" "with" "yield"
     ;; These are technically operators, but we fontify them as
     ;; keywords.
-    "and" "in" "is" "not" "or" "not in"))
+    "and" "in" "is" "not" "or" "not in" "is not"))
+
+(defvar python--treesit-builtin-types
+  '("int" "float" "complex" "bool" "list" "tuple" "range" "str"
+    "bytes" "bytearray" "memoryview" "set" "frozenset" "dict"))
+
+(defvar python--treesit-type-regex
+  (rx-to-string `(seq bol (or
+                           ,@python--treesit-builtin-types
+                           (seq (?  "_") (any "A-Z") (+ (any "a-zA-Z_0-9"))))
+                  eol)))
 
 (defvar python--treesit-builtins
-  '("abs" "all" "any" "ascii" "bin" "bool" "breakpoint" "bytearray"
-    "bytes" "callable" "chr" "classmethod" "compile" "complex"
-    "delattr" "dict" "dir" "divmod" "enumerate" "eval" "exec"
-    "filter" "float" "format" "frozenset" "getattr" "globals"
-    "hasattr" "hash" "help" "hex" "id" "input" "int" "isinstance"
-    "issubclass" "iter" "len" "list" "locals" "map" "max"
-    "memoryview" "min" "next" "object" "oct" "open" "ord" "pow"
-    "print" "property" "range" "repr" "reversed" "round" "set"
-    "setattr" "slice" "sorted" "staticmethod" "str" "sum" "super"
-    "tuple" "type" "vars" "zip" "__import__"))
+  (append python--treesit-builtin-types
+          '("abs" "all" "any" "ascii" "bin" "breakpoint"
+            "callable" "chr" "classmethod" "compile"
+            "delattr" "dir" "divmod" "enumerate" "eval" "exec"
+            "filter" "format" "getattr" "globals"
+            "hasattr" "hash" "help" "hex" "id" "input" "isinstance"
+            "issubclass" "iter" "len" "locals" "map" "max"
+            "min" "next" "object" "oct" "open" "ord" "pow"
+            "print" "property" "repr" "reversed" "round"
+            "setattr" "slice" "sorted" "staticmethod" "sum" "super"
+            "type" "vars" "zip" "__import__")))
 
 (defvar python--treesit-constants
   '("Ellipsis" "False" "None" "NotImplemented" "True" "__debug__"
@@ -1032,9 +1043,7 @@ python--treesit-fontify-string
 f-strings.  OVERRIDE is the override flag described in
 `treesit-font-lock-rules'.  START and END mark the region to be
 fontified."
-  (let* ((string-beg (treesit-node-start node))
-         (string-end (treesit-node-end node))
-         (maybe-expression (treesit-node-parent node))
+  (let* ((maybe-expression (treesit-node-parent node))
          (grandparent (treesit-node-parent
                        (treesit-node-parent
                         maybe-expression)))
@@ -1062,28 +1071,92 @@ python--treesit-fontify-string
                         (equal (treesit-node-type maybe-expression)
                                "expression_statement"))
                    'font-lock-doc-face
-                 'font-lock-string-face)))
-    ;; Don't highlight string prefixes like f/r/b.
-    (save-excursion
-      (goto-char string-beg)
-      (when (re-search-forward "[\"']" string-end t)
-        (setq string-beg (match-beginning 0))))
-    (treesit-fontify-with-override
-     string-beg string-end face override start end)))
-
-(defun python--treesit-fontify-string-interpolation
-    (node _ start end &rest _)
-  "Fontify string interpolation.
-NODE is the string node.  Do not fontify the initial f for
-f-strings.  START and END mark the region to be
+                 'font-lock-string-face))
+
+         (ignore-interpolation (not
+                                (seq-some
+                                 (lambda (feats) (memq 'string-interpolation feats))
+                                 (seq-take treesit-font-lock-feature-list treesit-font-lock-level))))
+         ;; If interpolation is enabled, highlight only
+         ;; string_start/string_content/string_end children.  Do not
+         ;; touch interpolation node that can occur inside of the
+         ;; string.
+         (string-nodes (if ignore-interpolation
+                           (list node)
+                         (treesit-filter-child
+                          node
+                          (lambda (ch) (member (treesit-node-type ch)
+                                               '("string_start"
+                                                 "string_content"
+                                                 "string_end")))
+                          t))))
+
+    (dolist (string-node string-nodes)
+      (let ((string-beg (treesit-node-start string-node))
+            (string-end (treesit-node-end string-node)))
+        (when (or ignore-interpolation
+                  (equal (treesit-node-type string-node) "string_start"))
+          ;; Don't highlight string prefixes like f/r/b.
+          (save-excursion
+            (goto-char string-beg)
+            (when (re-search-forward "[\"']" string-end t)
+              (setq string-beg (match-beginning 0)))))
+
+        (treesit-fontify-with-override
+         string-beg string-end face override start end)))))
+
+(defun python--treesit-fontify-union-types (node override start end &optional type-regex &rest _)
+  "Fontify nested union types in the type hints.
+For examlpe, Lvl1 | Lvl2[Lvl3[Lvl4[Lvl5 | None]], Lvl2].  This
+structure is represented via nesting binary_operator and
+subscript nodes.  This function iterates over all levels and
+highlight identifier nodes. If TYPE-REGEX is not nil fontify type
+identifier only if it matches against TYPE-REGEX. NODE is the
+binary_operator node.  OVERRIDE is the override flag described in
+`treesit-font-lock-rules'.  START and END mark the region to be
+fontified."
+  (dolist (child (treesit-node-children node t))
+    (let (font-node)
+      (pcase (treesit-node-type child)
+        ((or "identifier" "none")
+         (setq font-node child))
+        ("attribute"
+         (when-let ((type-node (treesit-node-child-by-field-name child "attribute")))
+           (setq font-node type-node)))
+        ((or "binary_operator" "subscript")
+         (python--treesit-fontify-union-types child override start end type-regex)))
+
+      (when (and font-node
+                 (or (null type-regex)
+                     (let ((case-fold-search nil))
+                       (string-match-p type-regex (treesit-node-text font-node)))))
+        (treesit-fontify-with-override
+         (treesit-node-start font-node) (treesit-node-end font-node)
+         'font-lock-type-face override start end)))))
+
+(defun python--treesit-fontify-union-types-strict (node override start end &rest _)
+  "Fontify nested union types.
+Same as `python--treesit-fontify-union-types' but type identifier
+should match against `python--treesit-type-regex'.  For NODE,
+OVERRIDE, START and END description see
+`python--treesit-fontify-union-types'."
+  (python--treesit-fontify-union-types node override start end python--treesit-type-regex))
+
+(defun python--treesit-fontify-dotted-decorator (node override start end &rest _)
+  "Fontify dotted decorators.
+For example @pytes.mark.skip.  Iterate over all nested attribute
+nodes and highlight identifier nodes.  NODE is the first attribute
+node.  OVERRIDE is the override flag described in
+`treesit-font-lock-rules'.  START and END mark the region to be
 fontified."
-  ;; This is kind of a hack, it basically removes the face applied by
-  ;; the string feature, so that following features can apply their
-  ;; face.
-  (let ((n-start (treesit-node-start node))
-        (n-end (treesit-node-end node)))
-    (remove-text-properties
-     (max start n-start) (min end n-end) '(face))))
+  (dolist (child (treesit-node-children node t))
+    (pcase (treesit-node-type child)
+      ("identifier"
+       (treesit-fontify-with-override
+        (treesit-node-start child) (treesit-node-end child)
+        'font-lock-type-face override start end))
+      ("attribute"
+       (python--treesit-fontify-dotted-decorator child override start end)))))
 
 (defvar python--treesit-settings
   (treesit-font-lock-rules
@@ -1093,14 +1166,9 @@ python--treesit-settings
 
    :feature 'string
    :language 'python
-   '((string) @python--treesit-fontify-string)
+   '((string) @python--treesit-fontify-string
+     (interpolation ["{" "}"] @font-lock-misc-punctuation-face))
 
-   ;; HACK: This feature must come after the string feature and before
-   ;; other features.  Maybe we should make string-interpolation an
-   ;; option rather than a feature.
-   :feature 'string-interpolation
-   :language 'python
-   '((interpolation) @python--treesit-fontify-string-interpolation)
 
    :feature 'keyword
    :language 'python
@@ -1117,12 +1185,6 @@ python--treesit-settings
      (parameters (identifier) @font-lock-variable-name-face)
      (parameters (default_parameter name: (identifier) @font-lock-variable-name-face)))
 
-   :feature 'function
-   :language 'python
-   '((call function: (identifier) @font-lock-function-call-face)
-     (call function: (attribute
-                      attribute: (identifier) @font-lock-function-call-face)))
-
    :feature 'builtin
    :language 'python
    `(((identifier) @font-lock-builtin-face
@@ -1133,6 +1195,19 @@ python--treesit-settings
                       eol))
               @font-lock-builtin-face)))
 
+   :feature 'decorator
+   :language 'python
+   '((decorator "@" @font-lock-type-face)
+     (decorator (call function: (identifier) @font-lock-type-face))
+     (decorator (identifier) @font-lock-type-face)
+     (decorator [(attribute) (call (attribute))] @python--treesit-fontify-dotted-decorator))
+
+   :feature 'function
+   :language 'python
+   '((call function: (identifier) @font-lock-function-call-face)
+     (call function: (attribute
+                      attribute: (identifier) @font-lock-function-call-face)))
+
    :feature 'constant
    :language 'python
    '([(true) (false) (none)] @font-lock-constant-face)
@@ -1144,30 +1219,71 @@ python--treesit-settings
                  @font-lock-variable-name-face)
      (assignment left: (attribute
                         attribute: (identifier)
-                        @font-lock-property-use-face))
-     (pattern_list (identifier)
+                        @font-lock-variable-name-face))
+     (augmented_assignment left: (identifier)
+                           @font-lock-variable-name-face)
+     (named_expression name: (identifier)
+                       @font-lock-variable-name-face)
+     (pattern_list [(identifier)
+                    (list_splat_pattern (identifier))]
                    @font-lock-variable-name-face)
-     (tuple_pattern (identifier)
+     (tuple_pattern [(identifier)
+                     (list_splat_pattern (identifier))]
                     @font-lock-variable-name-face)
-     (list_pattern (identifier)
-                   @font-lock-variable-name-face)
-     (list_splat_pattern (identifier)
-                         @font-lock-variable-name-face))
+     (list_pattern [(identifier)
+                    (list_splat_pattern (identifier))]
+                   @font-lock-variable-name-face))
 
-   :feature 'decorator
-   :language 'python
-   '((decorator "@" @font-lock-type-face)
-     (decorator (call function: (identifier) @font-lock-type-face))
-     (decorator (identifier) @font-lock-type-face))
 
    :feature 'type
    :language 'python
+   ;; Override built-in faces when dict/list are used for type hints.
+   :override t
    `(((identifier) @font-lock-type-face
       (:match ,(rx-to-string
                 `(seq bol (or ,@python--treesit-exceptions)
-                      eol))
+                  eol))
               @font-lock-type-face))
-     (type (identifier) @font-lock-type-face))
+     (type [(identifier) (none)] @font-lock-type-face)
+     (type (attribute attribute: (identifier) @font-lock-type-face))
+     ;; We don't want to highlight a package of the type
+     ;; (e.g. pack.ClassName).  So explicitly exclude patterns with
+     ;; attribute, since we handle dotted type name in the previous
+     ;; rule.  The following rule handle
+     ;; generic_type/list/tuple/splat_type nodes.
+     (type (_ !attribute [[(identifier) (none)] @font-lock-type-face
+                          (attribute attribute: (identifier) @font-lock-type-face) ]))
+     ;; collections.abc.Iterator[T] case.
+     (type (subscript (attribute attribute: (identifier) @font-lock-type-face)))
+     ;; Nested optional type hints, e.g. val: Lvl1 | Lvl2[Lvl3[Lvl4]].
+     (type (binary_operator) @python--treesit-fontify-union-types)
+     ;;class Type(Base1, Sequence[T]).
+     (class_definition
+      superclasses:
+      (argument_list [(identifier) @font-lock-type-face
+                      (attribute attribute: (identifier) @font-lock-type-face)
+                      (subscript (identifier) @font-lock-type-face)
+                      (subscript (attribute attribute: (identifier) @font-lock-type-face))]))
+
+     ;; Patern matching: case [str(), pack0.Type0()].  Take only the
+     ;; last identifier.
+     (class_pattern (dotted_name (identifier) @font-lock-type-face :anchor))
+
+     ;; Highlight the second argument as a type in isinstance/issubclass.
+     ((call function: (identifier) @func-name
+            (argument_list :anchor (_)
+                           [(identifier) @font-lock-type-face
+                            (attribute attribute: (identifier) @font-lock-type-face)
+                            (tuple (identifier) @font-lock-type-face)
+                            (tuple (attribute attribute: (identifier) @font-lock-type-face))]
+                           (:match ,python--treesit-type-regex @font-lock-type-face)))
+      (:match "^is\\(?:instance\\|subclass\\)$" @func-name))
+
+     ;; isinstance(t, int|float).
+     ((call function: (identifier) @func-name
+            (argument_list :anchor (_)
+                           (binary_operator) @python--treesit-fontify-union-types-strict))
+      (:match "^is\\(?:instance\\|subclass\\)$" @func-name)))
 
    :feature 'escape-sequence
    :language 'python
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index e1b4c0a74c..59287970ca 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -7122,6 +7122,308 @@ python-tests--flymake-command-output-pattern
                          "Unused import a.b.c (unused-import)"
                        "W0611: Unused import a.b.c (unused-import)"))))))
 
+;;; python-ts-mode font-lock tests
+
+(defmacro python-ts-tests-with-temp-buffer (contents &rest body)
+  "Create a `python-ts-mode' enabled temp buffer with CONTENTS.
+BODY is code to be executed within the temp buffer.  Point is
+always located at the beginning of buffer."
+  (declare (indent 1) (debug t))
+  `(with-temp-buffer
+     (skip-unless (treesit-ready-p 'python))
+     (require 'python)
+     (let ((python-indent-guess-indent-offset nil))
+       (python-ts-mode)
+       (setopt treesit-font-lock-level 3)
+       (insert ,contents)
+       (font-lock-ensure)
+       (goto-char (point-min))
+       ,@body)))
+
+(ert-deftest python-ts-mode-compound-keywords-face ()
+  (dolist (test '("is not" "not in"))
+    (python-ts-tests-with-temp-buffer
+     (concat "t " test " t")
+     (forward-to-word 1)
+     (should (eq (face-at-point) font-lock-keyword-face))
+     (forward-to-word 1)
+     (should (eq (face-at-point) font-lock-keyword-face)))))
+
+(ert-deftest python-ts-mode-named-assignement-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "var := 3"
+   (should (eq (face-at-point) font-lock-variable-name-face))))
+
+(ert-deftest python-ts-mode-assignement-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "var, *rest = call()"
+   (dolist (test '("var" "rest"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-variable-name-face))))
+
+  (python-ts-tests-with-temp-buffer
+   "def func(*args):"
+   (dolist (test '("args"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-variable-name-face))))))
+
+(ert-deftest python-ts-mode-nested-types-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "def func(v:dict[ list[ tuple[str] ], int | None] | None):"
+   (dolist (test '("dict" "list" "tuple" "str" "int" "None" "None"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-union-types-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "def f(val: tuple[tuple, list[Lvl1 | Lvl2[Lvl3[Lvl4[Lvl5 | None]], Lvl2]]]):"
+   (dolist (test '("tuple" "tuple" "list" "Lvl1" "Lvl2" "Lvl3" "Lvl4" "Lvl5" "None" "Lvl2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-union-types-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "def f(val: Type0 | Type1[Type2, pack0.Type3] | pack1.pack2.Type4 | None):"
+   (dolist (test '("Type0" "Type1" "Type2" "Type3" "Type4" "None"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+
+   (goto-char (point-min))
+   (dolist (test '("pack0" "pack1" "pack2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-types-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "def f(val: Callable[[Type0], (Type1, Type2)]):"
+   (dolist (test '("Callable" "Type0" "Type1" "Type2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-types-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "def annot3(val:pack0.Type0)->pack1.pack2.pack3.Type1:"
+   (dolist (test '("Type0" "Type1"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+   (goto-char (point-min))
+   (dolist (test '("pack0" "pack1" "pack2" "pack3"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-types-face-3 ()
+  (python-ts-tests-with-temp-buffer
+   "def annot3(val:collections.abc.Iterator[Type0]):"
+   (dolist (test '("Iterator" "Type0"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+   (goto-char (point-min))
+   (dolist (test '("collections" "abc"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-isinstance-type-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "isinstance(var1, pkg.Type0)
+    isinstance(var2, (str, dict, Type1, type(None)))
+    isinstance(var3, my_type())"
+
+   (dolist (test '("var1" "pkg" "var2" "type" "None" "var3" "my_type"))
+     (let ((case-fold-search nil))
+       (search-forward test))
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))
+
+   (goto-char (point-min))
+   (dolist (test '("Type0" "str" "dict" "Type1"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-isinstance-type-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "issubclass(mytype, int|list|collections.abc.Iterable)"
+   (dolist (test '("int" "list" "Iterable"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-isinstance-type-face-3 ()
+  (python-ts-tests-with-temp-buffer
+   "issubclass(mytype, typevar1)
+    isinstance(mytype, (Type1, typevar2, tuple, abc.Coll))
+    isinstance(mytype, pkg0.Type2|self.typevar3|typevar4)"
+
+   (dolist (test '("typevar1" "typevar2" "pkg0" "self" "typevar3" "typevar4"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))
+
+   (goto-char (point-min))
+   (dolist (test '("Type1" "tuple" "Coll" "Type2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-superclass-type-face ()
+  (python-ts-tests-with-temp-buffer
+   "class Temp(Base1, pack0.Base2,  Sequence[T1, T2]):"
+
+   (dolist (test '("Base1" "Base2" "Sequence" "T1" "T2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+
+   (goto-char (point-min))
+   (dolist (test '("pack0"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-class-patterns-face ()
+  (python-ts-tests-with-temp-buffer
+   "match tt:
+        case str():
+            pass
+        case [Type0() | bytes(b) | pack0.pack1.Type1()]:
+            pass
+        case {'i': int(i), 'f': float() as f}:
+            pass"
+
+   (dolist (test '("str" "Type0" "bytes" "Type1" "int" "float"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+
+   (goto-char (point-min))
+   (dolist (test '("pack0" "pack1"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-dotted-decorator-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "@pytest.mark.skip
+    @pytest.mark.skip(reason='msg')
+    def test():"
+
+   (dolist (test '("pytest" "mark" "skip" "pytest" "mark" "skip"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-dotted-decorator-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "@pytest.mark.skip(reason='msg')
+    def test():"
+
+   (setopt treesit-font-lock-level 4)
+   (dolist (test '("pytest" "mark" "skip"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-builtin-call-face ()
+  (python-ts-tests-with-temp-buffer
+   "all()"
+   ;; enable 'function' feature from 4th level
+   (setopt treesit-font-lock-level 4)
+   (should (eq (face-at-point) font-lock-builtin-face))))
+
+(ert-deftest python-ts-mode-interpolation-nested-string ()
+  (python-ts-tests-with-temp-buffer
+   "t = f\"beg {True + 'string'}\""
+
+   (search-forward "True")
+   (goto-char (match-beginning 0))
+   (should (eq (face-at-point) font-lock-constant-face))
+
+   (goto-char (point-min))
+   (dolist (test '("f" "{" "+" "}"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-string-face))))
+
+
+   (goto-char (point-min))
+   (dolist (test '("beg" "'string'" "\""))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-string-face)))))
+
+(ert-deftest python-ts-mode-level-fontification-wo-interpolation ()
+  (python-ts-tests-with-temp-buffer
+   "t = f\"beg {True + var}\""
+
+   (setopt treesit-font-lock-level 2)
+   (search-forward "f")
+   (goto-char (match-beginning 0))
+   (should (not (eq (face-at-point) font-lock-string-face)))
+
+   (dolist (test '("\"" "beg" "{" "True" "var" "}" "\""))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-string-face)))))
+
+(ert-deftest python-ts-mode-disabled-string-interpolation ()
+  (python-ts-tests-with-temp-buffer
+   "t = f\"beg {True + var}\""
+
+   (unwind-protect
+       (progn
+         (setf (nth 2 treesit-font-lock-feature-list)
+               (remq 'string-interpolation (nth 2 treesit-font-lock-feature-list)))
+         (setopt treesit-font-lock-level 3)
+
+         (search-forward "f")
+         (goto-char (match-beginning 0))
+         (should (not (eq (face-at-point) font-lock-string-face)))
+
+         (dolist (test '("\"" "beg" "{" "True" "var" "}" "\""))
+           (search-forward test)
+           (goto-char (match-beginning 0))
+           (should (eq (face-at-point) font-lock-string-face))))
+
+    (setf (nth 2 treesit-font-lock-feature-list)
+          (append (nth 2 treesit-font-lock-feature-list) '(string-interpolation))))))
+
+(ert-deftest python-ts-mode-interpolation-doc-string ()
+  (python-ts-tests-with-temp-buffer
+   "f\"\"\"beg {'s1' + True + 's2'} end\"\"\""
+
+   (search-forward "True")
+   (goto-char (match-beginning 0))
+   (should (eq (face-at-point) font-lock-constant-face))
+
+   (goto-char (point-min))
+   (dolist (test '("f" "{" "+" "}"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-string-face))))
+
+   (goto-char (point-min))
+   (dolist (test '("\"\"\"" "beg" "end" "\"\"\""))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-doc-face)))
+
+   (goto-char (point-min))
+   (dolist (test '("'s1'" "'s2'"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-string-face)))))
+
 (provide 'python-tests)
 
 ;;; python-tests.el ends here
-- 
2.34.1

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Eli Zaretskii]

> From: Denis Zubarev <dvzubarev@yandex.ru>
> Cc: "casouri@gmail.com" <casouri@gmail.com>,
> 	"dmitry@gutov.dev" <dmitry@gutov.dev>,
> 	"67061@debbugs.gnu.org" <67061@debbugs.gnu.org>
> Date: Sat, 30 Dec 2023 13:53:38 +0300
> 
> It seems, there is no consensus about new variable-definition feature. So I removed it from this
> patch.
> Later I will send a new patch for discussing it.
>  
> I rebased on the latest emacs-29 and also fixed a test that failed on this branch.
> Please find the patch in the attachment.

Thanks, now installed on the emacs-29 branch.

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Dmitry Gutov]

On 17/12/2023 02:26, Denis Zubarev wrote:

> Summary for all changes in the patch.
> New feature variable-definition:
> `for var in range(3)`
> `[var+1 for var in []]`
> `with T as var:`
> `except E as var:`
> `case str() as var:`
> highlight var as font-lock-variable-name-face
> assignment feature:
> var := 3 (named_expression)
> var *= 3 (augmented_assignment)
> Highlight var as font-lock-variable-name-face.

I still think variable-name-face is not the best fit for 
augmented_assignment, but admittedly it's a minor thing.

> type feature:
> Fontify built-ins (dict,list,etc.) as types when they are used in type 
> hints.
> support nested union types, for example `Lvl1 | Lvl2[Lvl3[Lvl3], Lvl2]`.
> This structure is represented via nesting binary_operator and subscript 
> nodes in the grammar.
> Function python--treesit-fontify-union-types iterates over all children 
> and highlight identifier nodes.

If you recall my earlier complaint that these highlightings didn't work 
(and the tests didn't pass), this happened due to an older Python grammar.

More specifically, these highlights, and the type-related face tests, 
don't work with the Python ts grammar I had from March 7th 2023. The 
queries didn't lead to errors either (that's a good thing), but maybe 
we'll want to revisit these highlights later to add support for the 
older grammar as well.

> Fontify base class names in the class definition: class Temp(Base1, 
> pack0.Base2):
> Fontify class patterns in case statement: case [TempC() | bytes(b)]:
> Highlight the second argument as a type in isinstance/issubclass call:
> isinstance(var2, (str, dict, Type1)); issubclass(var1, int|str)

I'm not sure highlighting types based on the caller method and position 
is a good idea. I think that's backward, logically. If one puts a 
non-type value in such argument, and we would highlight it as a type -- 
that seems like the wrong message.

OTOH, see this reddit thread and this screenshot:

https://www.reddit.com/r/emacs/comments/18kr1gl/how_can_i_configure_pythontsmode_to_fontify_more/

https://preview.redd.it/y8l3k8tt4x6c1.png?width=3840&format=png&auto=webp&s=0a6882e66d4b334c07e856934ce847e63aa2db2c

One of the complaints is that "User" is not highlighted as a type when 
used in other, non-built-in methods, which like a reasonable question to 
me. Yes, Python is dynamic, but using CamelCase for types is a fairly 
regular convention, so highlighting such identifiers as types can work. 
You can see rust-ts-mode for an example of this approach.

> decorator feature:
> Highlight dotted names: @pytest.mark.skip
> Function python--treesit-fontify-dotted-decorator iterates over all 
> nested attribute nodes and highlight identifier nodes.
> When font-lock-level is set 4, `skip` had function-call face in: 
> @pytest.mark.skip(reason='t')
> Add `:override t` to decorator feature to override function-call face.
> string feature:

Could we just move it above the 'function' feature, so that the override 
is not needed?

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Denis Zubarev]

[-- Attachment #1: Type: text/html, Size: 6706 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0006-Improve-syntax-highlighting-for-python-ts-mode.patch --]
[-- Type: text/x-diff; name="0006-Improve-syntax-highlighting-for-python-ts-mode.patch", Size: 31804 bytes --]

From a8ad11285fbb938137bcf8f3114819c1e5ba7c9d Mon Sep 17 00:00:00 2001
From: Denis Zubarev <dvzubarev@yandex.ru>
Date: Sat, 11 Nov 2023 04:55:44 +0300
Subject: [PATCH] Improve syntax highlighting for python-ts-mode

Fix fontification of strings inside of f-strings interpolation, e.g. for
f"beg {'nested'}" - 'nested' was not fontified as string.  Do not
override the face of builtin functions (all, bytes etc.) with the
function call face.  Add missing assignment expressions (:= *=).
Fontify built-ins (dict,list,etc.) as types when they are used in type
hints.  Highlight union types (type1|type2).  Highlight base class names
in the class definition.  Fontify class patterns in case statements.
Highlight the second argument as a type in isinstance/issubclass call.
Highlight dotted decorator names.

Add new feature variable-definition for variables defined in local
scopes (for var in [], with T as var, etc.).

* lisp/progmodes/python.el (python--treesit-keywords): Add compound
keyword "is not".
(python--treesit-builtin-types): New variable that stores all python
built-in types.
(python--treesit-type-regex): New variable. Regex matches if text is either
built-in type or text starts with capital letter.
(python--treesit-builtins): Extract built-in types to other variable.
(python--treesit-fontify-string): fix f-string interpolation. Enable
interpolation highlighting only if string-interpolation is presented on
the enabled levels of treesit-font-lock-feature-list.
(python--treesit-fontify-string-interpolation): Remove function.
(python--treesit-fontify-union-types): Fontify nested union types.
(python--treesit-fontify-union-types-strict): Fontify nested union
types, only if type identifier matches against
python--treesit-type-regex.
(python--treesit-fontify-dotted-decorator): Fontify all parts of
dotted decorator name.
(python--treesit-settings): Change/add rules.
* test/lisp/progmodes/python-tests.el (python-ts-tests-with-temp-buffer):
function for setting up test buffer.
(python-ts-mode-compound-keywords-face)
(python-ts-mode-var-for-loop-face)
(python-ts-mode-named-assignement-face-1)
(python-ts-mode-assignement-face-2)
(python-ts-mode-nested-types-face-1)
(python-ts-mode-union-types-face-1)
(python-ts-mode-union-types-face-2)
(python-ts-mode-types-face-1)
(python-ts-mode-types-face-2)
(python-ts-mode-types-face-3)
(python-ts-mode-isinstance-type-face-1)
(python-ts-mode-isinstance-type-face-2)
(python-ts-mode-isinstance-type-face-3)
(python-ts-mode-superclass-type-face)
(python-ts-mode-class-patterns-face)
(python-ts-mode-dotted-decorator-face-1)
(python-ts-mode-dotted-decorator-face-2)
(python-ts-mode-builtin-call-face)
(python-ts-mode-interpolation-nested-string)
(python-ts-mode-disabled-string-interpolation)
(python-ts-mode-interpolation-doc-string): Add tests.
---
 lisp/progmodes/python.el            | 252 ++++++++++++++-----
 test/lisp/progmodes/python-tests.el | 367 ++++++++++++++++++++++++++++
 2 files changed, 557 insertions(+), 62 deletions(-)

diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index ab3bf1b4ec..e9df64fc1b 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -979,19 +979,30 @@ python--treesit-keywords
     "raise" "return" "try" "while" "with" "yield"
     ;; These are technically operators, but we fontify them as
     ;; keywords.
-    "and" "in" "is" "not" "or" "not in"))
+    "and" "in" "is" "not" "or" "not in" "is not"))
+
+(defvar python--treesit-builtin-types
+  '("int" "float" "complex" "bool" "list" "tuple" "range" "str"
+    "bytes" "bytearray" "memoryview" "set" "frozenset" "dict"))
+
+(defvar python--treesit-type-regex
+  (rx-to-string `(seq bol (or
+                           ,@python--treesit-builtin-types
+                           (seq (?  "_") (any "A-Z") (+ (any "a-zA-Z_0-9"))))
+                  eol)))
 
 (defvar python--treesit-builtins
-  '("abs" "all" "any" "ascii" "bin" "bool" "breakpoint" "bytearray"
-    "bytes" "callable" "chr" "classmethod" "compile" "complex"
-    "delattr" "dict" "dir" "divmod" "enumerate" "eval" "exec"
-    "filter" "float" "format" "frozenset" "getattr" "globals"
-    "hasattr" "hash" "help" "hex" "id" "input" "int" "isinstance"
-    "issubclass" "iter" "len" "list" "locals" "map" "max"
-    "memoryview" "min" "next" "object" "oct" "open" "ord" "pow"
-    "print" "property" "range" "repr" "reversed" "round" "set"
-    "setattr" "slice" "sorted" "staticmethod" "str" "sum" "super"
-    "tuple" "type" "vars" "zip" "__import__"))
+  (append python--treesit-builtin-types
+          '("abs" "all" "any" "ascii" "bin" "breakpoint"
+            "callable" "chr" "classmethod" "compile"
+            "delattr" "dir" "divmod" "enumerate" "eval" "exec"
+            "filter" "format" "getattr" "globals"
+            "hasattr" "hash" "help" "hex" "id" "input" "isinstance"
+            "issubclass" "iter" "len" "locals" "map" "max"
+            "min" "next" "object" "oct" "open" "ord" "pow"
+            "print" "property" "repr" "reversed" "round"
+            "setattr" "slice" "sorted" "staticmethod" "sum" "super"
+            "type" "vars" "zip" "__import__")))
 
 (defvar python--treesit-constants
   '("Ellipsis" "False" "None" "NotImplemented" "True" "__debug__"
@@ -1042,9 +1053,7 @@ python--treesit-fontify-string
 f-strings.  OVERRIDE is the override flag described in
 `treesit-font-lock-rules'.  START and END mark the region to be
 fontified."
-  (let* ((string-beg (treesit-node-start node))
-         (string-end (treesit-node-end node))
-         (maybe-expression (treesit-node-parent node))
+  (let* ((maybe-expression (treesit-node-parent node))
          (grandparent (treesit-node-parent
                        (treesit-node-parent
                         maybe-expression)))
@@ -1072,28 +1081,92 @@ python--treesit-fontify-string
                         (equal (treesit-node-type maybe-expression)
                                "expression_statement"))
                    'font-lock-doc-face
-                 'font-lock-string-face)))
-    ;; Don't highlight string prefixes like f/r/b.
-    (save-excursion
-      (goto-char string-beg)
-      (when (re-search-forward "[\"']" string-end t)
-        (setq string-beg (match-beginning 0))))
-    (treesit-fontify-with-override
-     string-beg string-end face override start end)))
-
-(defun python--treesit-fontify-string-interpolation
-    (node _ start end &rest _)
-  "Fontify string interpolation.
-NODE is the string node.  Do not fontify the initial f for
-f-strings.  START and END mark the region to be
+                 'font-lock-string-face))
+
+         (ignore-interpolation (not
+                                (seq-some
+                                 (lambda (feats) (memq 'string-interpolation feats))
+                                 (seq-take treesit-font-lock-feature-list treesit-font-lock-level))))
+         ;; If interpolation is enabled, highlight only
+         ;; string_start/string_content/string_end children.  Do not
+         ;; touch interpolation node that can occur inside of the
+         ;; string.
+         (string-nodes (if ignore-interpolation
+                           (list node)
+                         (treesit-filter-child
+                          node
+                          (lambda (ch) (member (treesit-node-type ch)
+                                               '("string_start"
+                                                 "string_content"
+                                                 "string_end")))
+                          t))))
+
+    (dolist (string-node string-nodes)
+      (let ((string-beg (treesit-node-start string-node))
+            (string-end (treesit-node-end string-node)))
+        (when (or ignore-interpolation
+                  (equal (treesit-node-type string-node) "string_start"))
+          ;; Don't highlight string prefixes like f/r/b.
+          (save-excursion
+            (goto-char string-beg)
+            (when (re-search-forward "[\"']" string-end t)
+              (setq string-beg (match-beginning 0)))))
+
+        (treesit-fontify-with-override
+         string-beg string-end face override start end)))))
+
+(defun python--treesit-fontify-union-types (node override start end &optional type-regex &rest _)
+  "Fontify nested union types in the type hints.
+For examlpe, Lvl1 | Lvl2[Lvl3[Lvl4[Lvl5 | None]], Lvl2].  This
+structure is represented via nesting binary_operator and
+subscript nodes.  This function iterates over all levels and
+highlight identifier nodes. If TYPE-REGEX is not nil fontify type
+identifier only if it matches against TYPE-REGEX. NODE is the
+binary_operator node.  OVERRIDE is the override flag described in
+`treesit-font-lock-rules'.  START and END mark the region to be
+fontified."
+  (dolist (child (treesit-node-children node t))
+    (let (font-node)
+      (pcase (treesit-node-type child)
+        ((or "identifier" "none")
+         (setq font-node child))
+        ("attribute"
+         (when-let ((type-node (treesit-node-child-by-field-name child "attribute")))
+           (setq font-node type-node)))
+        ((or "binary_operator" "subscript")
+         (python--treesit-fontify-union-types child override start end type-regex)))
+
+      (when (and font-node
+                 (or (null type-regex)
+                     (let ((case-fold-search nil))
+                       (string-match-p type-regex (treesit-node-text font-node)))))
+        (treesit-fontify-with-override
+         (treesit-node-start font-node) (treesit-node-end font-node)
+         'font-lock-type-face override start end)))))
+
+(defun python--treesit-fontify-union-types-strict (node override start end &rest _)
+  "Fontify nested union types.
+Same as `python--treesit-fontify-union-types' but type identifier
+should match against `python--treesit-type-regex'.  For NODE,
+OVERRIDE, START and END description see
+`python--treesit-fontify-union-types'."
+  (python--treesit-fontify-union-types node override start end python--treesit-type-regex))
+
+(defun python--treesit-fontify-dotted-decorator (node override start end &rest _)
+  "Fontify dotted decorators.
+For example @pytes.mark.skip.  Iterate over all nested attribute
+nodes and highlight identifier nodes.  NODE is the first attribute
+node.  OVERRIDE is the override flag described in
+`treesit-font-lock-rules'.  START and END mark the region to be
 fontified."
-  ;; This is kind of a hack, it basically removes the face applied by
-  ;; the string feature, so that following features can apply their
-  ;; face.
-  (let ((n-start (treesit-node-start node))
-        (n-end (treesit-node-end node)))
-    (remove-text-properties
-     (max start n-start) (min end n-end) '(face))))
+  (dolist (child (treesit-node-children node t))
+    (pcase (treesit-node-type child)
+      ("identifier"
+       (treesit-fontify-with-override
+        (treesit-node-start child) (treesit-node-end child)
+        'font-lock-type-face override start end))
+      ("attribute"
+       (python--treesit-fontify-dotted-decorator child override start end)))))
 
 (defvar python--treesit-settings
   (treesit-font-lock-rules
@@ -1103,14 +1176,9 @@ python--treesit-settings
 
    :feature 'string
    :language 'python
-   '((string) @python--treesit-fontify-string)
+   '((string) @python--treesit-fontify-string
+     (interpolation ["{" "}"] @font-lock-misc-punctuation-face))
 
-   ;; HACK: This feature must come after the string feature and before
-   ;; other features.  Maybe we should make string-interpolation an
-   ;; option rather than a feature.
-   :feature 'string-interpolation
-   :language 'python
-   '((interpolation) @python--treesit-fontify-string-interpolation)
 
    :feature 'keyword
    :language 'python
@@ -1126,12 +1194,6 @@ python--treesit-settings
       name: (identifier) @font-lock-type-face)
      (parameters (identifier) @font-lock-variable-name-face))
 
-   :feature 'function
-   :language 'python
-   '((call function: (identifier) @font-lock-function-call-face)
-     (call function: (attribute
-                      attribute: (identifier) @font-lock-function-call-face)))
-
    :feature 'builtin
    :language 'python
    `(((identifier) @font-lock-builtin-face
@@ -1142,6 +1204,19 @@ python--treesit-settings
                       eol))
               @font-lock-builtin-face)))
 
+   :feature 'decorator
+   :language 'python
+   '((decorator "@" @font-lock-type-face)
+     (decorator (call function: (identifier) @font-lock-type-face))
+     (decorator (identifier) @font-lock-type-face)
+     (decorator [(attribute) (call (attribute))] @python--treesit-fontify-dotted-decorator))
+
+   :feature 'function
+   :language 'python
+   '((call function: (identifier) @font-lock-function-call-face)
+     (call function: (attribute
+                      attribute: (identifier) @font-lock-function-call-face)))
+
    :feature 'constant
    :language 'python
    '([(true) (false) (none)] @font-lock-constant-face)
@@ -1153,30 +1228,83 @@ python--treesit-settings
                  @font-lock-variable-name-face)
      (assignment left: (attribute
                         attribute: (identifier)
-                        @font-lock-property-use-face))
-     (pattern_list (identifier)
+                        @font-lock-variable-name-face))
+     (augmented_assignment left: (identifier)
+                           @font-lock-variable-name-face)
+     (named_expression name: (identifier)
+                       @font-lock-variable-name-face)
+     (pattern_list [(identifier)
+                    (list_splat_pattern (identifier))]
                    @font-lock-variable-name-face)
-     (tuple_pattern (identifier)
+     (tuple_pattern [(identifier)
+                     (list_splat_pattern (identifier))]
                     @font-lock-variable-name-face)
-     (list_pattern (identifier)
-                   @font-lock-variable-name-face)
-     (list_splat_pattern (identifier)
-                         @font-lock-variable-name-face))
+     (list_pattern [(identifier)
+                    (list_splat_pattern (identifier))]
+                   @font-lock-variable-name-face))
 
-   :feature 'decorator
+   :feature 'variable-definition
    :language 'python
-   '((decorator "@" @font-lock-type-face)
-     (decorator (call function: (identifier) @font-lock-type-face))
-     (decorator (identifier) @font-lock-type-face))
+   `((for_statement left: (identifier)
+      @font-lock-variable-name-face)
+     (for_in_clause left: (identifier)
+                    @font-lock-variable-name-face)
+     (as_pattern_target (identifier)
+                        @font-lock-variable-name-face)
+     (case_pattern (as_pattern "as" :anchor (identifier)
+                               @font-lock-variable-name-face)))
+
 
    :feature 'type
    :language 'python
+   ;; Override built-in faces when dict/list are used for type hints.
+   :override t
    `(((identifier) @font-lock-type-face
       (:match ,(rx-to-string
                 `(seq bol (or ,@python--treesit-exceptions)
-                      eol))
+                  eol))
               @font-lock-type-face))
-     (type (identifier) @font-lock-type-face))
+     (type [(identifier) (none)] @font-lock-type-face)
+     (type (attribute attribute: (identifier) @font-lock-type-face))
+     ;; We don't want to highlight a package of the type
+     ;; (e.g. pack.ClassName).  So explicitly exclude patterns with
+     ;; attribute, since we handle dotted type name in the previous
+     ;; rule.  The following rule handle
+     ;; generic_type/list/tuple/splat_type nodes.
+     (type (_ !attribute [[(identifier) (none)] @font-lock-type-face
+                          (attribute attribute: (identifier) @font-lock-type-face) ]))
+     ;; collections.abc.Iterator[T] case.
+     (type (subscript (attribute attribute: (identifier) @font-lock-type-face)))
+     ;; Nested optional type hints, e.g. val: Lvl1 | Lvl2[Lvl3[Lvl4]].
+     (type (binary_operator) @python--treesit-fontify-union-types)
+     ;;class Type(Base1, Sequence[T]).
+     (class_definition
+      superclasses:
+      (argument_list [(identifier) @font-lock-type-face
+                      (attribute attribute: (identifier) @font-lock-type-face)
+                      (subscript (identifier) @font-lock-type-face)
+                      (subscript (attribute attribute: (identifier) @font-lock-type-face))]))
+
+     ;; Patern matching: case [str(), pack0.Type0()].  Take only the
+     ;; last identifier.
+     (class_pattern (dotted_name (identifier) @font-lock-type-face :anchor))
+
+
+     ;; Highlight the second argument as a type in isinstance/issubclass.
+     ((call function: (identifier) @func-name
+            (argument_list :anchor (_)
+                           [(identifier) @font-lock-type-face
+                            (attribute attribute: (identifier) @font-lock-type-face)
+                            (tuple (identifier) @font-lock-type-face)
+                            (tuple (attribute attribute: (identifier) @font-lock-type-face))]
+                           (:match ,python--treesit-type-regex @font-lock-type-face)))
+      (:match "^is\\(?:instance\\|subclass\\)$" @func-name))
+
+     ;; isinstance(t, int|float).
+     ((call function: (identifier) @func-name
+            (argument_list :anchor (_)
+                           (binary_operator) @python--treesit-fontify-union-types-strict))
+      (:match "^is\\(?:instance\\|subclass\\)$" @func-name)))
 
    :feature 'escape-sequence
    :language 'python
@@ -6841,7 +6969,7 @@ python-ts-mode
     (setq-local treesit-font-lock-feature-list
                 '(( comment definition)
                   ( keyword string type)
-                  ( assignment builtin constant decorator
+                  ( assignment variable-definition builtin constant decorator
                     escape-sequence number string-interpolation )
                   ( bracket delimiter function operator variable property)))
     (setq-local treesit-font-lock-settings python--treesit-settings)
diff --git a/test/lisp/progmodes/python-tests.el b/test/lisp/progmodes/python-tests.el
index a44a11896f..fc667ba73d 100644
--- a/test/lisp/progmodes/python-tests.el
+++ b/test/lisp/progmodes/python-tests.el
@@ -7299,6 +7299,373 @@ python-tests--flymake-command-output-pattern
                          "Unused import a.b.c (unused-import)"
                        "W0611: Unused import a.b.c (unused-import)"))))))
 
+;;; python-ts-mode font-lock tests
+
+(defmacro python-ts-tests-with-temp-buffer (contents &rest body)
+  "Create a `python-ts-mode' enabled temp buffer with CONTENTS.
+BODY is code to be executed within the temp buffer.  Point is
+always located at the beginning of buffer."
+  (declare (indent 1) (debug t))
+  `(with-temp-buffer
+     (skip-unless (treesit-ready-p 'python))
+     (require 'python)
+     (let ((python-indent-guess-indent-offset nil))
+       (python-ts-mode)
+       (setopt treesit-font-lock-level 3)
+       (insert ,contents)
+       (font-lock-ensure)
+       (goto-char (point-min))
+       ,@body)))
+
+(ert-deftest python-ts-mode-compound-keywords-face ()
+  (dolist (test '("is not" "not in"))
+    (python-ts-tests-with-temp-buffer
+     (concat "t " test " t")
+     (forward-to-word)
+     (should (eq (face-at-point) font-lock-keyword-face))
+     (forward-to-word)
+     (should (eq (face-at-point) font-lock-keyword-face)))))
+
+(ert-deftest python-ts-mode-var-for-loop-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "for var in range(3):"
+   (dolist (test '("var"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-variable-name-face)))))
+
+(ert-deftest python-ts-mode-var-for-loop-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "for var1, (var2, var3) in []:"
+   (dolist (test '("var1" "var2" "var3"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-variable-name-face)))))
+
+(ert-deftest python-ts-mode-var-for-loop-face-3 ()
+  (python-ts-tests-with-temp-buffer
+   "[var for var in [] if var ]"
+   (search-forward "var")
+   (goto-char (match-beginning 0))
+   (should (not (eq (face-at-point) font-lock-variable-name-face)))
+
+   (search-forward "var" nil nil 2)
+   (goto-char (match-beginning 0))
+   (should (eq (face-at-point) font-lock-variable-name-face))
+
+   (search-forward "var" nil nil 2)
+   (goto-char (match-beginning 0))
+   (should (not (eq (face-at-point) font-lock-variable-name-face)))))
+
+(ert-deftest python-ts-mode-as-pattern-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "try:
+    pass
+except Exception as excp:
+    pass"
+
+   (search-forward "excp")
+   (goto-char (match-beginning 0))
+   (should (eq (face-at-point) font-lock-variable-name-face))))
+
+(ert-deftest python-ts-mode-as-pattern-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "with ctx() as var:"
+   (search-forward "var")
+   (goto-char (match-beginning 0))
+   (should (eq (face-at-point) font-lock-variable-name-face))))
+
+(ert-deftest python-ts-mode-as-pattern-face-3 ()
+  (python-ts-tests-with-temp-buffer
+   "with ctx() as var:"
+   (search-forward "var")
+   (goto-char (match-beginning 0))
+   (should (eq (face-at-point) font-lock-variable-name-face))))
+
+(ert-deftest python-ts-mode-as-pattern-face-4 ()
+  (python-ts-tests-with-temp-buffer
+   "match v:
+    case (list() as lvar, Inst() as ivar):"
+   (dolist (test '("lvar" "ivar"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-variable-name-face)))))
+
+(ert-deftest python-ts-mode-named-assignement-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "var := 3"
+   (should (eq (face-at-point) font-lock-variable-name-face))))
+
+(ert-deftest python-ts-mode-assignement-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "var, *rest = call()"
+   (dolist (test '("var" "rest"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-variable-name-face))))
+
+  (python-ts-tests-with-temp-buffer
+   "def func(*args):"
+   (dolist (test '("args"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-variable-name-face))))))
+
+(ert-deftest python-ts-mode-nested-types-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "def func(v:dict[ list[ tuple[str] ], int | None] | None):"
+   (dolist (test '("dict" "list" "tuple" "str" "int" "None" "None"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-union-types-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "def f(val: tuple[tuple, list[Lvl1 | Lvl2[Lvl3[Lvl4[Lvl5 | None]], Lvl2]]]):"
+   (dolist (test '("tuple" "tuple" "list" "Lvl1" "Lvl2" "Lvl3" "Lvl4" "Lvl5" "None" "Lvl2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-union-types-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "def f(val: Type0 | Type1[Type2, pack0.Type3] | pack1.pack2.Type4 | None):"
+   (dolist (test '("Type0" "Type1" "Type2" "Type3" "Type4" "None"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+
+   (goto-char (point-min))
+   (dolist (test '("pack0" "pack1" "pack2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-types-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "def f(val: Callable[[Type0], (Type1, Type2)]):"
+   (dolist (test '("Callable" "Type0" "Type1" "Type2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-types-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "def annot3(val:pack0.Type0)->pack1.pack2.pack3.Type1:"
+   (dolist (test '("Type0" "Type1"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+   (goto-char (point-min))
+   (dolist (test '("pack0" "pack1" "pack2" "pack3"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-types-face-3 ()
+  (python-ts-tests-with-temp-buffer
+   "def annot3(val:collections.abc.Iterator[Type0]):"
+   (dolist (test '("Iterator" "Type0"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+   (goto-char (point-min))
+   (dolist (test '("collections" "abc"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-isinstance-type-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "isinstance(var1, pkg.Type0)
+    isinstance(var2, (str, dict, Type1, type(None)))
+    isinstance(var3, my_type())"
+
+   (dolist (test '("var1" "pkg" "var2" "type" "None" "var3" "my_type"))
+     (let ((case-fold-search nil))
+       (search-forward test))
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))
+
+   (goto-char (point-min))
+   (dolist (test '("Type0" "str" "dict" "Type1"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-isinstance-type-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "issubclass(mytype, int|list|collections.abc.Iterable)"
+   (dolist (test '("int" "list" "Iterable"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-isinstance-type-face-3 ()
+  (python-ts-tests-with-temp-buffer
+   "issubclass(mytype, typevar1)
+    isinstance(mytype, (Type1, typevar2, tuple, abc.Coll))
+    isinstance(mytype, pkg0.Type2|self.typevar3|typevar4)"
+
+   (dolist (test '("typevar1" "typevar2" "pkg0" "self" "typevar3" "typevar4"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))
+
+   (goto-char (point-min))
+   (dolist (test '("Type1" "tuple" "Coll" "Type2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-superclass-type-face ()
+  (python-ts-tests-with-temp-buffer
+   "class Temp(Base1, pack0.Base2,  Sequence[T1, T2]):"
+
+   (dolist (test '("Base1" "Base2" "Sequence" "T1" "T2"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+
+   (goto-char (point-min))
+   (dolist (test '("pack0"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-class-patterns-face ()
+  (python-ts-tests-with-temp-buffer
+   "match tt:
+        case str():
+            pass
+        case [Type0() | bytes(b) | pack0.pack1.Type1()]:
+            pass
+        case {'i': int(i), 'f': float() as f}:
+            pass"
+
+   (dolist (test '("str" "Type0" "bytes" "Type1" "int" "float"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))
+
+   (goto-char (point-min))
+   (dolist (test '("pack0" "pack1"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-type-face))))))
+
+(ert-deftest python-ts-mode-dotted-decorator-face-1 ()
+  (python-ts-tests-with-temp-buffer
+   "@pytest.mark.skip
+    @pytest.mark.skip(reason='msg')
+    def test():"
+
+   (dolist (test '("pytest" "mark" "skip" "pytest" "mark" "skip"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-dotted-decorator-face-2 ()
+  (python-ts-tests-with-temp-buffer
+   "@pytest.mark.skip(reason='msg')
+    def test():"
+
+   (setopt treesit-font-lock-level 4)
+   (dolist (test '("pytest" "mark" "skip"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-type-face)))))
+
+(ert-deftest python-ts-mode-builtin-call-face ()
+  (python-ts-tests-with-temp-buffer
+   "all()"
+   ;; enable 'function' feature from 4th level
+   (setopt treesit-font-lock-level 4)
+   (should (eq (face-at-point) font-lock-builtin-face))))
+
+(ert-deftest python-ts-mode-interpolation-nested-string ()
+  (python-ts-tests-with-temp-buffer
+   "t = f\"beg {True + 'string'}\""
+
+   (search-forward "True")
+   (goto-char (match-beginning 0))
+   (should (eq (face-at-point) font-lock-constant-face))
+
+   (goto-char (point-min))
+   (dolist (test '("f" "{" "+" "}"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-string-face))))
+
+
+   (goto-char (point-min))
+   (dolist (test '("beg" "'string'" "\""))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-string-face)))))
+
+(ert-deftest python-ts-mode-level-fontification-wo-interpolation ()
+  (python-ts-tests-with-temp-buffer
+   "t = f\"beg {True + var}\""
+
+   (setopt treesit-font-lock-level 2)
+   (search-forward "f")
+   (goto-char (match-beginning 0))
+   (should (not (eq (face-at-point) font-lock-string-face)))
+
+   (dolist (test '("\"" "beg" "{" "True" "var" "}" "\""))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-string-face)))))
+
+(ert-deftest python-ts-mode-disabled-string-interpolation ()
+  (python-ts-tests-with-temp-buffer
+   "t = f\"beg {True + var}\""
+
+   (unwind-protect
+       (progn
+         (setf (nth 2 treesit-font-lock-feature-list)
+               (remq 'string-interpolation (nth 2 treesit-font-lock-feature-list)))
+         (setopt treesit-font-lock-level 3)
+
+         (search-forward "f")
+         (goto-char (match-beginning 0))
+         (should (not (eq (face-at-point) font-lock-string-face)))
+
+         (dolist (test '("\"" "beg" "{" "True" "var" "}" "\""))
+           (search-forward test)
+           (goto-char (match-beginning 0))
+           (should (eq (face-at-point) font-lock-string-face))))
+
+    (setf (nth 2 treesit-font-lock-feature-list)
+          (append (nth 2 treesit-font-lock-feature-list) '(string-interpolation))))))
+
+(ert-deftest python-ts-mode-interpolation-doc-string ()
+  (python-ts-tests-with-temp-buffer
+   "f\"\"\"beg {'s1' + True + 's2'} end\"\"\""
+
+   (search-forward "True")
+   (goto-char (match-beginning 0))
+   (should (eq (face-at-point) font-lock-constant-face))
+
+   (goto-char (point-min))
+   (dolist (test '("f" "{" "+" "}"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (not (eq (face-at-point) font-lock-string-face))))
+
+   (goto-char (point-min))
+   (dolist (test '("\"\"\"" "beg" "end" "\"\"\""))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-doc-face)))
+
+   (goto-char (point-min))
+   (dolist (test '("'s1'" "'s2'"))
+     (search-forward test)
+     (goto-char (match-beginning 0))
+     (should (eq (face-at-point) font-lock-string-face)))))
+
 (provide 'python-tests)
 
 ;;; python-tests.el ends here
-- 
2.34.1

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Dmitry Gutov]

On 19/12/2023 02:14, Denis Zubarev wrote:
>  > If you recall my earlier complaint that these highlightings didn't work
>  > (and the tests didn't pass), this happened due to an older Python 
> grammar.
> Thank you for investigating this. It seems this commit introduced
> changes to type nodes hierarchy 
> (https://github.com/tree-sitter/tree-sitter-python/commit/bcbf41589f4dc38a98bda4ca4c924eb5cae26f7b).

Could be this one, yes.

>  > The queries didn't lead to errors either (that's a good thing), but maybe
>  > we'll want to revisit these highlights later to add support for the
>  > older grammar as well.
> It may lead to unnecessarily complex rules. I don't
> know is it worth it, since users can easily update grammars.

No problem.

>  > I'm not sure highlighting types based on the caller method and position
>  > is a good idea. I think that's backward, logically. If one puts a
>  > non-type value in such argument, and we would highlight it as a type --
>  > that seems like the wrong message.
> These two functions expect a type (or tuple of types) as the second
> argument. To address your concerns about highlighting as a type a
> non-type variable, I added regexp python--treesit-type-regex. This regex
> matches if text is either built-in type or text starts with capital
> letter. I extracted built-in types from the python--treesit-builtins
> into its own variable python--treesit-builtin-types.
> python--treesit-builtins is now constructing by appending
> python--treesit-builtin-types and other built-ins. I hope it is ok.

Thank you. I'm actually not sure if we _have to_ check the identifier 
names in this context (any chance to have a false negative, miss some 
valid types?), but it probably doesn't hurt either.

>  > One of the complaints is that "User" is not highlighted as a type when
>  > used in other, non-built-in methods, which like a reasonable question to
>  > me. Yes, Python is dynamic, but using CamelCase for types is a fairly
>  > regular convention, so highlighting such identifiers as types can work.
> It is good idea, to highlight some variables as types. But I think it
> should be done on the 4th level. One could split the variable feature
> into multiple features: variable-type, variable-argument, variable-use,
> etc. So for variable-type feature we can use python--treesit-type-regex
> and highlight matched identifiers with type face. For now I wanted to
> properly highlight types in places where they expected to be.

I wouldn't mind the level 4 (after all, python-mode is also conservative 
here and doesn't add such highlighting), but I'd rather not add the 
special handling for isinstance/issubclass thing for the reasons 
previously outlined.

Perhaps Yuan will disagree. I'm just here to say that the rest of the 
patch LGTM.

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Yuan Fu]

> On Dec 20, 2023, at 3:34 PM, Dmitry Gutov <dmitry@gutov.dev> wrote:
> 
> On 19/12/2023 02:14, Denis Zubarev wrote:
>> > If you recall my earlier complaint that these highlightings didn't work
>> > (and the tests didn't pass), this happened due to an older Python grammar.
>> Thank you for investigating this. It seems this commit introduced
>> changes to type nodes hierarchy (https://github.com/tree-sitter/tree-sitter-python/commit/bcbf41589f4dc38a98bda4ca4c924eb5cae26f7b).
> 
> Could be this one, yes.
> 
>> > The queries didn't lead to errors either (that's a good thing), but maybe
>> > we'll want to revisit these highlights later to add support for the
>> > older grammar as well.
>> It may lead to unnecessarily complex rules. I don't
>> know is it worth it, since users can easily update grammars.
> 
> No problem.
> 
>> > I'm not sure highlighting types based on the caller method and position
>> > is a good idea. I think that's backward, logically. If one puts a
>> > non-type value in such argument, and we would highlight it as a type --
>> > that seems like the wrong message.
>> These two functions expect a type (or tuple of types) as the second
>> argument. To address your concerns about highlighting as a type a
>> non-type variable, I added regexp python--treesit-type-regex. This regex
>> matches if text is either built-in type or text starts with capital
>> letter. I extracted built-in types from the python--treesit-builtins
>> into its own variable python--treesit-builtin-types.
>> python--treesit-builtins is now constructing by appending
>> python--treesit-builtin-types and other built-ins. I hope it is ok.
> 
> Thank you. I'm actually not sure if we _have to_ check the identifier names in this context (any chance to have a false negative, miss some valid types?), but it probably doesn't hurt either.
> 
>> > One of the complaints is that "User" is not highlighted as a type when
>> > used in other, non-built-in methods, which like a reasonable question to
>> > me. Yes, Python is dynamic, but using CamelCase for types is a fairly
>> > regular convention, so highlighting such identifiers as types can work.
>> It is good idea, to highlight some variables as types. But I think it
>> should be done on the 4th level. One could split the variable feature
>> into multiple features: variable-type, variable-argument, variable-use,
>> etc. So for variable-type feature we can use python--treesit-type-regex
>> and highlight matched identifiers with type face. For now I wanted to
>> properly highlight types in places where they expected to be.
> 
> I wouldn't mind the level 4 (after all, python-mode is also conservative here and doesn't add such highlighting), but I'd rather not add the special handling for isinstance/issubclass thing for the reasons previously outlined.
> 
> Perhaps Yuan will disagree. I'm just here to say that the rest of the patch LGTM.

I wouldn’t mind either, go crazy with level 4 :-) I wouldn’t even mind it in level 3, since they are indeed types. Using a separate feature is a good idea, so people who doesn’t want it can turn it off.

Yuan

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Denis Zubarev]

[-- Attachment #1: Type: text/html, Size: 5389 bytes --]

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Dmitry Gutov]

On 23/12/2023 23:45, Denis Zubarev wrote:
> Just adding a rule for highlighting CamelCase identifiers as types would
> lead to many false positives. For example, global variables or an object
> instantiation.

It seems like the convention is to use ALL_CAPITAL for constants and 
CamelCase for classes/constructors. Those can be distinguished with a 
regexp, with single-char names being sorted into constants, which they 
usually are.

I suppose some code could be violating that, but perhaps we should 
remind such authors about that with highlighting as well.

Regarding object instantiation, I'd be happy to see the class name in 
Class(...) instantiation calls highlighted with font-lock-type-face. 
that's more useful that telling the user that they are seeing a function 
call by the means of font-lock-function-call-face.

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Eli Zaretskii]

> Date: Mon, 1 Jan 2024 19:42:16 +0200
> Cc: "67061@debbugs.gnu.org" <67061@debbugs.gnu.org>
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 23/12/2023 23:45, Denis Zubarev wrote:
> > Just adding a rule for highlighting CamelCase identifiers as types would
> > lead to many false positives. For example, global variables or an object
> > instantiation.
> 
> It seems like the convention is to use ALL_CAPITAL for constants and 
> CamelCase for classes/constructors. Those can be distinguished with a 
> regexp, with single-char names being sorted into constants, which they 
> usually are.
> 
> I suppose some code could be violating that, but perhaps we should 
> remind such authors about that with highlighting as well.
> 
> Regarding object instantiation, I'd be happy to see the class name in 
> Class(...) instantiation calls highlighted with font-lock-type-face. 
> that's more useful that telling the user that they are seeing a function 
> call by the means of font-lock-function-call-face.

Is there anything else left to do here, or should I close this bug?

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Eli Zaretskii]

Ping! Should this bug be closed now?

> Cc: casouri@gmail.com, dvzubarev@yandex.ru, 67061@debbugs.gnu.org
> Date: Tue, 09 Jan 2024 22:03:33 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > Date: Mon, 1 Jan 2024 19:42:16 +0200
> > Cc: "67061@debbugs.gnu.org" <67061@debbugs.gnu.org>
> > From: Dmitry Gutov <dmitry@gutov.dev>
> > 
> > On 23/12/2023 23:45, Denis Zubarev wrote:
> > > Just adding a rule for highlighting CamelCase identifiers as types would
> > > lead to many false positives. For example, global variables or an object
> > > instantiation.
> > 
> > It seems like the convention is to use ALL_CAPITAL for constants and 
> > CamelCase for classes/constructors. Those can be distinguished with a 
> > regexp, with single-char names being sorted into constants, which they 
> > usually are.
> > 
> > I suppose some code could be violating that, but perhaps we should 
> > remind such authors about that with highlighting as well.
> > 
> > Regarding object instantiation, I'd be happy to see the class name in 
> > Class(...) instantiation calls highlighted with font-lock-type-face. 
> > that's more useful that telling the user that they are seeing a function 
> > call by the means of font-lock-function-call-face.
> 
> Is there anything else left to do here, or should I close this bug?
> 
> 
> 
> 

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Eli Zaretskii]

Ping! Ping! Can this bug be closed now, or do we have anything left to
do?

> Cc: dvzubarev@yandex.ru, 67061@debbugs.gnu.org
> Date: Sat, 20 Jan 2024 11:08:18 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> Ping! Should this bug be closed now?
> 
> > Cc: casouri@gmail.com, dvzubarev@yandex.ru, 67061@debbugs.gnu.org
> > Date: Tue, 09 Jan 2024 22:03:33 +0200
> > From: Eli Zaretskii <eliz@gnu.org>
> > 
> > > Date: Mon, 1 Jan 2024 19:42:16 +0200
> > > Cc: "67061@debbugs.gnu.org" <67061@debbugs.gnu.org>
> > > From: Dmitry Gutov <dmitry@gutov.dev>
> > > 
> > > On 23/12/2023 23:45, Denis Zubarev wrote:
> > > > Just adding a rule for highlighting CamelCase identifiers as types would
> > > > lead to many false positives. For example, global variables or an object
> > > > instantiation.
> > > 
> > > It seems like the convention is to use ALL_CAPITAL for constants and 
> > > CamelCase for classes/constructors. Those can be distinguished with a 
> > > regexp, with single-char names being sorted into constants, which they 
> > > usually are.
> > > 
> > > I suppose some code could be violating that, but perhaps we should 
> > > remind such authors about that with highlighting as well.
> > > 
> > > Regarding object instantiation, I'd be happy to see the class name in 
> > > Class(...) instantiation calls highlighted with font-lock-type-face. 
> > > that's more useful that telling the user that they are seeing a function 
> > > call by the means of font-lock-function-call-face.
> > 
> > Is there anything else left to do here, or should I close this bug?
> > 
> > 
> > 
> > 
> 
> 
> 
> 

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Denis Zubarev]

[-- Attachment #1: Type: text/html, Size: 2889 bytes --]

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Eli Zaretskii]

> From: Denis Zubarev <dvzubarev@yandex.ru>
> Cc: "67061@debbugs.gnu.org" <67061@debbugs.gnu.org>
> Date: Sat, 27 Jan 2024 13:47:08 +0300
> 
> I think, It can be closed since the patch was merged. I may send a new patch if I continue work on
> this.

Thanks, closing.

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Stefan Kangas]

Dmitry Gutov <dmitry@gutov.dev> writes:

> I don't know at what stage we're going to start worrying when adding new
> elements to the queries, though, in fear of breaking compatibility with
> some potential older version of the grammar.

Right.  But can we really stop installing support for new language
features that users expect?

I'm not super close to the tree-sitter stuff to be honest, so apologies
if I misunderstood something.  But IIUC, the current situation means
that we can't depend on concrete versions of grammars, which means we
can't depend on that to make adaptions.  But that situation is not
really caused by us, right?

So maybe at the point when we find problems in practice, we should just
throw up our hands and urge users to upgrade.  Perhaps it'll encourage
more work on improving the situation with grammar versioning.

The biggest problems will be with faster moving languages, of course.
And who knows how common they will be even then - only experience will
tell.

Just my two cents.

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Dmitry Gutov]

On 13/12/2023 23:16, Stefan Kangas wrote:
> Dmitry Gutov <dmitry@gutov.dev> writes:
> 
>> I don't know at what stage we're going to start worrying when adding new
>> elements to the queries, though, in fear of breaking compatibility with
>> some potential older version of the grammar.
> 
> Right.  But can we really stop installing support for new language
> features that users expect?

Some less important ones -- maybe. E.g. syntax highlighting is less 
essential than indentation. Though it's usually easier to implement 
(with tree-sitter, at least).

> I'm not super close to the tree-sitter stuff to be honest, so apologies
> if I misunderstood something.  But IIUC, the current situation means
> that we can't depend on concrete versions of grammars, which means we
> can't depend on that to make adaptions.  But that situation is not
> really caused by us, right?

We made our choices here too:

- Trying to support different versions of grammars, not just the latest 
ones. Or a "pinned" revision.
- Adding treesit modes to the core, rather than publishing them to ELPA.

> So maybe at the point when we find problems in practice, we should just
> throw up our hands and urge users to upgrade.  Perhaps it'll encourage
> more work on improving the situation with grammar versioning.

Simply asking our users to upgrade to the latest grammar won't work if 
the major mode they are using is only compatible with some older grammar 
version. And it's not upgradable because the major mode is not in ELPA.

We might solve this in the future with a two-step: "throwing up our 
hands" and publishing major modes to "ELPA core". treesit.el will 
probably need to stabilize a bit more before that, though.

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Stefan Kangas]

Dmitry Gutov <dmitry@gutov.dev> writes:

> We might solve this in the future with a two-step: "throwing up our
> hands" and publishing major modes to "ELPA core". treesit.el will
> probably need to stabilize a bit more before that, though.

Yeah, that's what I had in mind.  It's not hard to make packages into
:core packages, we just have to be careful not to use
backwards-incompatible stuff in them.

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Yuan Fu]

> On Dec 14, 2023, at 2:49 PM, Stefan Kangas <stefankangas@gmail.com> wrote:
> 
> Dmitry Gutov <dmitry@gutov.dev> writes:
> 
>> We might solve this in the future with a two-step: "throwing up our
>> hands" and publishing major modes to "ELPA core". treesit.el will
>> probably need to stabilize a bit more before that, though.
> 
> Yeah, that's what I had in mind.  It's not hard to make packages into
> :core packages, we just have to be careful not to use
> backwards-incompatible stuff in them.

Maybe we can think of that before Emacs 30 releases? By then treesit.el should be more or less stable and feature-complete.

Yuan

bug#67061: [PATCH] Improve syntax highlighting for python-ts-mode

[Yuan Fu]

Here's my long due review:

> Improve syntax highlighting for python-ts-mode.
>
> - Fontify compound keywords "not in" and "is not" (fixes bug#67015)
This is great, though you'll need to rebase your changes on top of 
latest emacs-29 now, because I added one of the keywords when fixing 
bug#67015. Sorry for the trouble.
> - Fix fontification of strings inside of f-strings interpolation,
>     e.g. for f"beg {'nested'}" - 'nested' was not fontified as string.
I see that you removed string-interpolation feature, I don't know how I 
feel about it. For one, if this fix is to be applied to emacs-29, we 
cannot remove that feature—that's a breaking change IMO.

Can we do this instead: in python--treesit-fontify-string, we check if 
string-interpolation feature is enabled, if it is, fontify string_start, 
string_content and string_end only; if not, fontify the whole string.
> - Do not override the face of builtin functions (all, bytes etc.) with
>    the function call face
> - Fontify built-ins (dict,list,etc.) as types when they are used in type hints
LGTM

> - Add missing assignment expressions

Do you have examples on the type of assignments you are adding 
fontification for?

Some general tips: for comments, we always write complete sentences, 
capitalize the first character and ends with a period. Also I see double 
empty lines and missing empty lines at places.

Yuan