Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python Highlighting - type is matched too aggressively as a keyword #3902

Closed
justin-snyder-slgg opened this issue Jan 10, 2024 · 3 comments · Fixed by #3917
Closed

Python Highlighting - type is matched too aggressively as a keyword #3902

justin-snyder-slgg opened this issue Jan 10, 2024 · 3 comments · Fixed by #3917
Labels
C: Syntax T: bug A bug in an existing language feature

Comments

@justin-snyder-slgg
Copy link

justin-snyder-slgg commented Jan 10, 2024

Description of the bug

I noticed that recently, Sublime started highlighting type as a keyword, but this causes some issues because it is officially only a soft keyword that also happens to share a name with a builtin function. Depending on context, type could be a keyword or an identifier, and this has some minor rippling effects on later highlights in the line (and sometimes spilling onto later lines)

Steps to reproduce

  1. Start in safe mode
  2. Set language to python
  3. Type the following:
class Foo:
	type: list[str] = ['a']

Observe the weird highlighting that acts as if type is a keyword here even though it isn't

Expected behavior

The official grammar for type statements is as follows:

type_stmt ::= 'type' identifier [type_params] "=" expression

Therefore I propose that type should only be highlighted as a keyword in the following situations:

  • when it is the only token on the line (the assumption being that you are about to type out a type statement)
  • when it is the first token on the line and is followed immediately by another identifier

As soon as type is followed by anything other than an identifier, it should revert to being highlighted as a builtin or identifier. (Ideally it should be highlighted as a regular identifier when it is on the left side of an assignment or type annotation, but being highlighted as a builtin (as it was before) didn't cause any cascading issues with syntax highlighting and is tolerable)

Actual behavior

Currently, it appears that type is treated as a keyword when it is the first token on a line, regardless of what tokens follow it on that line.

Screenshot 2024-01-10 at 3 59 43 PM

Sublime Text build number

4169

Operating system & version

MacOS Sonoma 14.1.1

(Linux) Desktop environment and/or window manager

No response

Additional information

No response

OpenGL context information

No response

@BenjaminSchaaf BenjaminSchaaf transferred this issue from sublimehq/sublime_text Jan 11, 2024
@deathaxe
Copy link
Collaborator

Despite type possibly being a normal variable, depending on context, it has been scoped support.function before even if not looking like a function call. So are all tokens, which denote python 2 or 3 builtins.

As a result it used to be highlighted comparable prominent various color schemes, before.

With a restriction applied to type only being scoped keyword.declaration if followed by an identifier as proposed in OP, situation wouldn't change significantly for such color schemes.

Mariana:

grafik

Monokai

grafik

Brackets

grafik

@FichteFoll
Copy link
Collaborator

FichteFoll commented Jan 17, 2024

Scoping it as support.function is definitely better than as keyword.declaration (because that would just be wrong) so I don't think we need to argue about that.

As for whether or not we should scope the target of assigmnets as support.function when it's the same as a built-in, I strongly think yes, because in most cases you're shadowing the built-in type in that case and it only kinda works out in a class definition, because outside of the class body itself (including within functions defined inside of it) you can only access that type attribute via attribute access. However, within the class definition, it still makes a difference if you do:

class A:
  type = 'a'
  other = type
# vs
class B:
  other = type

Also, because Python is a language with significant whitespace (indentation) that is infeasible to determine and track with syntax definitions, there is no point in trying to make a special exception for when type is defined inside a class body.

@FichteFoll FichteFoll added the T: bug A bug in an existing language feature label Jan 17, 2024
@deathaxe
Copy link
Collaborator

Both scopes are sub-optimal and I rather tend to also dislike scoping builtin-function-identifiers special in non-function-call expressions. Shadowing existing globals is not a syntax error or disallowed in any way and fully within responsibility of the author.

deathaxe added a commit to deathaxe/sublime-packages that referenced this issue Feb 3, 2024
Fixes sublimehq#3902

This commit ensures to highlight `type` more context sensitively.

It may have the following meanings:

1. built-in `type()` function
2. type alias declaration keyword: `type Alias`
3. ordinary variable used in many data classes

The built-in function (1.) most likely appears in r-value expressions, such as

   `my_type = type(var)`

At beginning of statements this commit prefers scoping `type` as keyword (2.)
or variable (3.) as this is the most likely use case.

The implementation ensures to start `meta` scope of alias declarations at bol
as class or function declarations do.
deathaxe added a commit to deathaxe/sublime-packages that referenced this issue Feb 3, 2024
Fixes sublimehq#3902

This commit ensures to highlight `type` more context sensitively.

It may have the following meanings:

1. built-in `type()` function
2. type alias declaration keyword: `type Alias`
3. ordinary variable used in many data classes

The built-in function (1.) most likely appears in r-value expressions, such as

   `my_type = type(var)`

At beginning of statements this commit prefers scoping `type` as keyword (2.)
or variable (3.) as this is the most likely use case.

The implementation ensures to start `meta` scope of alias declarations at bol
as class or function declarations do.
deathaxe added a commit to deathaxe/sublime-packages that referenced this issue Feb 3, 2024
Fixes sublimehq#3902

This commit ensures to highlight `type` more context sensitively.

It may have the following meanings:

1. built-in `type()` function
2. type alias declaration keyword: `type Alias`
3. ordinary variable used in many data classes

The built-in function (1.) most likely appears in r-value expressions, such as

   `my_type = type(var)`

At beginning of statements this commit prefers scoping `type` as keyword (2.)
or variable (3.) as this is the most likely use case.

The implementation ensures to start `meta` scope of alias declarations at bol
as class or function declarations do.
deathaxe added a commit that referenced this issue Feb 3, 2024
Fixes #3902

This commit ensures to highlight `type` more context sensitively.

It may have the following meanings:

1. built-in `type()` function
2. type alias declaration keyword: `type Alias`
3. ordinary variable used in many data classes

The built-in function (1.) most likely appears in r-value expressions, such as

   `my_type = type(var)`

At beginning of statements this commit prefers scoping `type` as keyword (2.)
or variable (3.) as this is the most likely use case.

The implementation ensures to start `meta` scope of alias declarations at bol
as class or function declarations do.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: Syntax T: bug A bug in an existing language feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants