[Python] Add syntax highlighting for type comments #1925

TeamSpen210 · 2019-03-24T22:33:38Z

This makes #type: comments highlighted as normal code (since type checkers expect these to be valid syntax), and treats #type: ignore as a keyword. I'm not sure though if it should additionally have the comment.* scope for the line - any schemes would probably ignore the additional highlighting and make it appear as a comment.

Thom1729 · 2019-03-24T22:49:10Z

Can there be a space in between type and :?

TeamSpen210 · 2019-03-24T22:53:16Z

It's not explicit in the PEP, but both MyPy and PyCharm treat # type : as a normal comment.

keith-hall · 2019-03-24T23:17:59Z

I'm not sure though if it should additionally have the comment.* scope for the line - any schemes would probably ignore the additional highlighting and make it appear as a comment.

seems unlikely, comments could have a background color for example, plus the scope specificity means the keyword etc will take precedence. I vote for giving it a distinct comment.line.type-hint scope or something so color scheme authors can choose what to do.

TeamSpen210 · 2019-03-24T23:29:37Z

In that case, # type: SomeClass looks a bit odd, since non-builtin names don't get a specific scope, other than meta.generic-name.python. So there's no specific rule to override the comment one.

keith-hall · 2019-03-25T03:06:46Z

ah good point. But in this context, do we not know for sure that it is a class, such that it can be assigned support.class.python instead of a generic meta scope?

FichteFoll · 2019-03-25T15:37:30Z

I've had this on my todo, but ideally I'd like to see #1842 (comment) resolved because it will hugely affect the type of scopes to be used.

Python currently doesn't have highlighting of user types, i.e. class names, but I would like to change that for both type comments and variable/function annotations. However, I'm undecided on the particular scope to use. Do they get variable.type or variable.class? Can I use support.type for classes provided by the typing module? How do people feel about built-in types getting a different scope than user types? This is traditionally easy to answer for languages that have primitive and complex types like Java, but not for Python.

wbond

Apparently I had let a bunch of comments, but forgot to "complete" the review - sorry!

Python/Python.sublime-syntax

- Add Python to type hint scopes - Highlight Mypy's ignore codes - handle type: ignore_something_else as a non-special class name

Python/syntax_test_python.py

wbond

I noted a number of things that should be tweaked, some stylistic stuff and a couple of questions.

At the high level I think one of my concerns with the current state is how the comment scopes are stacked multiple times in places. This generally it not what we want to do, but rather specialize. For example, perhaps the first part of the comment is comment.line.type-comment, but as you hit the second # it would switch to comment.line.number-sign.

Python/Python.sublime-syntax

Python/syntax_test_python.py

- Removes some out-of-order stacked scopes - Removes some duplicated scopes - Removes some improper suffixes - Changes context names to fix existing style - Simplifies scope assertions in test file

wbond · 2020-07-21T16:24:25Z

I think I've resolved the issues I found during review. If anyone is up for a final review, I'll plan on merging this soon.

kaste

I can't or hardly can comment on the actual syntax definition but added some comments about the python examples in the tests.

Python/syntax_test_python.py

kaste · 2020-07-21T20:37:50Z

Python/syntax_test_python.py

@@ -1490,70 +1490,80 @@ class Starship:
 # Type comments - type: ignore must be by itself.

 primes = 5  # type: ignore # type: not-a-type-comment


Just for clarification

primes = 5 # type: ignore # type: str // technically ok but weird primes = 5 # type: str # type: ignore // not tested here but typical primes = 5 # type: str # type: str // not tested here and invalid

(I don't know how fine-grained these tests are here.)

The impression I had gotten from the previous implementation was that line comments added into a type comment effectively ended the processing of mypy. Is that true?

From your comment above, it almost sounds like mypy will process nested comments that start with type: and the only logical combinations are type: ignore and a normal type: comment, right?

Maybe mypy stops after the first type: ignore comment. 🤷 After that everything is "egal". The only thing you see in practice is # type: str # type: ignore to brutally force a type. And typical # type: ignore[foo-rule] # mypy bug https:\\issue-nr because you really get hit by a mypy bug every other day.

Python/syntax_test_python.py

rwols · 2020-07-21T20:57:08Z

I think this needs more work. (this is build 4079, perhaps it includes some tweaks to the syntax engine that causes this)

wbond · 2020-07-22T11:50:39Z

@rwols Can you paste text of a couple of snippets where you see failures?

@kaste I'm tempted to get rid of trying to mark things as invalid, and instead just highlight the things known to be valid, thoughts? You may also have some thoughts @FichteFoll as you've worked on marking various things as invalid in this syntax before.

wbond · 2020-07-22T14:43:40Z

@rwols I've added fixes for the screenshots you uploaded, if you wouldn't mind checking again?

@kaste I think I've taken out the invalid syntax you've pointed out.

rwols · 2020-07-22T16:53:42Z

Sorry for not pasting the text, it was late :) The last two commits e3b57a9 and 97ae638 fixed the problem.

My only remark now is that the syntax contexts from the return type and the type: ... annotation are different. In the screenshots above you can see that -> List[str] is highlighted differently than # type: List[str].

Minimal reproducible case:

def f() -> List[str]:
    #      ^^^^ support.class
    x = []  # type: List[str]
    #               ^^^^ support.class

Perhaps it can be argued that they shouldn't share the same context. But I don't know what else you'd use the -> ...: syntax for.

This is still broken:

class SessionBufferProtocol(Protocol):

    session = None  # type: Session
    view = None  # type: sublime.View
    #                           ^ - invalid.illegal
    session_views = None  # type: WeakSet[SessionViewProtocol]
    file_name = None  # type: str

    def on_diagnostics_async(self, diagnostics: List[Dict[str, Any]], version: Optional[int]) -> None:
        ...

It's perfectly legal to type-hint something from another module.

rwols · 2020-07-22T16:55:46Z

Minimal reproduction case:

x = []  # type: List[sublime.Region]

rwols · 2020-07-22T17:01:03Z

There's also this issue:

x = []  # type: 'List[ParameterInformation]'
#               ^ - invalid.illegal
#                                          ^ - invalid.illegal

While it looks a bit weird, it's allowed to wrap the type in a string (single-quotes or double-quotes). Used for forward declarations.

wbond · 2020-07-22T17:28:02Z

I figured I'd take a quick pass at some fixes, but it seems this requires a fair bit more work. I'm probably gonna hit pause for now and move onto other things.

kaste · 2020-07-22T20:58:03Z

FWIW: # type: (int, str, List[str], bool) -> Dict[str, str] # type: comment for the function type is illegal, no second # type: is allowed here.

There is also no "tuple" thing # type: (str, int). No parens allowed. Generally only [] as a subscript, for example Tuple[int, str].

The only fancy thing left is str | int for the Union[str, int], but not in top position. T.i. a = 5 # type: str | int is not allowed; a = {} # type: Dict[str, str | int] is. Hilarious.

kaste · 2020-07-22T20:59:22Z

Python/syntax_test_python.py

@@ -1519,51 +1519,46 @@ class Starship:
 #                                                      ^^^^^ comment.line.number-sign - comment.line.type-hint

 # With/for allow commas
-for a, b in lst: # type: str, int
-#                           ^ comment.line.type-hint punctuation.separator.sequence
+for a, b in lst: # type: (str, int)


The parens in (str, int)are not allowed. It's actually Tuple[str, int]. They take explicit > implicit seriously.

I am uncertain here. I've created a little test file with

from typing import Tuple def testing(results): for a, b in results: # type: Tuple[int, str] print(a, b)

Hovering a or b with LSP and LSP-pylsp installed, doesn't show anything, while an example using current syntax with or without parentheses works fine.

def testing(results): for a, b in results: # type: (int, str) print(a, b)

It also makes sense as I found some comments about such type hints to date back to python 2.7 which doesn't know about typing module.

The related spec can be found at https://mypy.readthedocs.io/en/stable/type_inference_and_annotations.html#declaring-multiple-variable-types-at-a-time

kaste · 2020-07-22T21:04:56Z

Generally, I don't expect Sublime marks my syntax errors as "invalid". The invalid scope is hard to edit and read, and I sure don't expect it at all. I always thought it is an escape hatch just before the syntax engine becomes incapable of "fixing" it, i.e. keeping the colors reasonable. (T.i. I don't expect it to read my code actually.)

I just found it strange to have so many invalid examples in the few python test lines.

FichteFoll · 2020-07-23T16:58:32Z

You may also have some thoughts @FichteFoll as you've worked on marking various things as invalid in this syntax before.

I generally prefer to use invalid scopes only when the syntax parsing reaches a state it cannot recover due to invalid syntax, such as keywords being used when they are not allowed, or when the user is likely to have made an error such as for stray braces. It seems debatable, whether a type comment is a strongly parsed region, considering that on one hand comments are free-form, but on the other hand type comments are expected to be parsed by a type checker.

I have never used type comments in Python myself, so I can only comment in a general sense, but my biggest issue is matching any identifier as support.type.python, which strongly depends on #1842, as mentioned earlier. Traditionally, support types were used for names provided by the language itself, including constants such as Python's True or None. While the ST scope naming docs say that support.{type,class} may also be used for user-defined names, I personally haven't used a syntax definition doing that.

In fact, running a grep on the entire Packages repos show support.type to be used for:

Rust: selected names
CSS: all properties (arguably a special case)
JavaScript: provided objects from multiple environments, e.g. node modules or browser objects
TypeScript: selected names and type variables
Go: selected names (stacked with storage.type)
Scala: any type
C#: any non-primitive type
ASP: selected names? (unsure)
C++: selected names
Java: namespaces in paths ???
Python: selected names
YAML: tag definitions
Erlang: unsure

support.class:

ActionScript: selected names
AppleScript: selected names (including common packages like itunes)
ASP: selected names
Haskell: selected names (only when deriving)
Java: all classes, also in imports
JavaScript: for dollar identifiers, selected classes, and method definitions
TypeScript: any type in type annotations
LaTeX: package references and \documentclass
Objective-C(++): selected names
Php: all classes, imports, special scope for selected names
Ruby: all classes
Scala: all classes

wbond · 2020-07-23T17:52:27Z

Just as a procedural note, I probably am not going to have the time to hash out RFC scope names before we get to an ST4 public release. I do want to address them at some point in the future, but they don't make the priority cut at this point.

FichteFoll · 2020-07-27T15:52:06Z

Added results for support.class to my comment above.

jrappen · 2021-10-08T12:27:52Z

@TeamSpen210 Maybe you could update this PR, merge the latest changes from master and address the requested changes.

Tests are run against the current dev build from https://www.sublimetext.com/dev.

deathaxe · 2021-10-08T14:07:29Z

I guess he would already have done it, if he was interested in.

deathaxe · 2023-05-10T14:06:22Z

Closing in favor of #3736, which covers all kinds of type annotation supported by current versions of python.

This commit adds dedicated contexts for type hints. It provides highlighting for type comments including feedback from #1925. Existing type annotations are refactored to follow a common scope naming scheme for type hints. It is inspired by PHP, TS and TSX, which use `meta.type` to scope type hint expressions. The content however remains being handled by normal expressions, even though the primary use case these days are type-hints. That's required as python accepts all kinds of expressions within annotations. Known type classes from typing module are scoped as `support.class.typing`.

TeamSpen210 force-pushed the type-comment branch from 5361354 to 5850d11 Compare March 24, 2019 23:11

[Python] Add syntax highlighting for type comments

137308c

TeamSpen210 force-pushed the type-comment branch from 5850d11 to 137308c Compare March 25, 2019 04:54

[Python] Add function type hint comment syntax

ed61bb4

Thom1729 mentioned this pull request Mar 28, 2019

[RFC] Scopes for module/namespace access #1842

Open

wbond requested changes Oct 16, 2019

View reviewed changes

[Python] Small fixes/improvements to type comments

56ff18e

- Add Python to type hint scopes - Highlight Mypy's ignore codes - handle type: ignore_something_else as a non-special class name

TeamSpen210 requested a review from wbond October 17, 2019 08:29

kaste reviewed Oct 17, 2019

View reviewed changes

Python/syntax_test_python.py Outdated Show resolved Hide resolved

Python/syntax_test_python.py Outdated Show resolved Hide resolved

wbond requested changes Oct 18, 2019

View reviewed changes

wbond added the changes requested label Oct 25, 2019

wbond added 2 commits July 21, 2020 08:29

Merge branch 'master' into type-comment

482f0ec

[Python] Clean up type comment scopes

14bf602

- Removes some out-of-order stacked scopes - Removes some duplicated scopes - Removes some improper suffixes - Changes context names to fix existing style - Simplifies scope assertions in test file

kaste reviewed Jul 21, 2020

View reviewed changes

Python/syntax_test_python.py Outdated Show resolved Hide resolved

wbond added 3 commits July 22, 2020 10:17

Merge branch 'master' into type-comment

2a57368

[Python] Fixed various type comment tests to use valid code

e3b57a9

[Python] Ensure type comments are exited at newline

97ae638

wbond added help wanted and removed changes requested labels Jul 22, 2020

kaste reviewed Jul 22, 2020

View reviewed changes

deathaxe mentioned this pull request May 10, 2023

[Python] Add support for type comments and annotations #3736

Merged

deathaxe closed this May 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Python] Add syntax highlighting for type comments #1925

[Python] Add syntax highlighting for type comments #1925

TeamSpen210 commented Mar 24, 2019

Thom1729 commented Mar 24, 2019

TeamSpen210 commented Mar 24, 2019

keith-hall commented Mar 24, 2019

TeamSpen210 commented Mar 24, 2019

keith-hall commented Mar 25, 2019

FichteFoll commented Mar 25, 2019 •

edited

Loading

wbond left a comment

wbond left a comment

wbond commented Jul 21, 2020 •

edited

Loading

kaste left a comment

kaste Jul 21, 2020

wbond Jul 22, 2020

kaste Jul 22, 2020

rwols commented Jul 21, 2020 •

edited

Loading

wbond commented Jul 22, 2020

wbond commented Jul 22, 2020

rwols commented Jul 22, 2020

rwols commented Jul 22, 2020

rwols commented Jul 22, 2020

wbond commented Jul 22, 2020

kaste commented Jul 22, 2020

kaste Jul 22, 2020

deathaxe Sep 5, 2021

deathaxe Sep 6, 2021

kaste commented Jul 22, 2020

FichteFoll commented Jul 23, 2020 •

edited

Loading

wbond commented Jul 23, 2020

FichteFoll commented Jul 27, 2020

jrappen commented Oct 8, 2021

deathaxe commented Oct 8, 2021

deathaxe commented May 10, 2023

		@@ -1490,70 +1490,80 @@ class Starship:
		# Type comments - type: ignore must be by itself.

		primes = 5 # type: ignore # type: not-a-type-comment

[Python] Add syntax highlighting for type comments #1925

[Python] Add syntax highlighting for type comments #1925

Conversation

TeamSpen210 commented Mar 24, 2019

Thom1729 commented Mar 24, 2019

TeamSpen210 commented Mar 24, 2019

keith-hall commented Mar 24, 2019

TeamSpen210 commented Mar 24, 2019

keith-hall commented Mar 25, 2019

FichteFoll commented Mar 25, 2019 • edited Loading

wbond left a comment

Choose a reason for hiding this comment

wbond left a comment

Choose a reason for hiding this comment

wbond commented Jul 21, 2020 • edited Loading

kaste left a comment

Choose a reason for hiding this comment

kaste Jul 21, 2020

Choose a reason for hiding this comment

wbond Jul 22, 2020

Choose a reason for hiding this comment

kaste Jul 22, 2020

Choose a reason for hiding this comment

rwols commented Jul 21, 2020 • edited Loading

wbond commented Jul 22, 2020

wbond commented Jul 22, 2020

rwols commented Jul 22, 2020

rwols commented Jul 22, 2020

rwols commented Jul 22, 2020

wbond commented Jul 22, 2020

kaste commented Jul 22, 2020

kaste Jul 22, 2020

Choose a reason for hiding this comment

deathaxe Sep 5, 2021

Choose a reason for hiding this comment

deathaxe Sep 6, 2021

Choose a reason for hiding this comment

kaste commented Jul 22, 2020

FichteFoll commented Jul 23, 2020 • edited Loading

wbond commented Jul 23, 2020

FichteFoll commented Jul 27, 2020

jrappen commented Oct 8, 2021

deathaxe commented Oct 8, 2021

deathaxe commented May 10, 2023

FichteFoll commented Mar 25, 2019 •

edited

Loading

wbond commented Jul 21, 2020 •

edited

Loading

rwols commented Jul 21, 2020 •

edited

Loading

FichteFoll commented Jul 23, 2020 •

edited

Loading