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

Semantic Highlighting for Constants #5611

Open
maflAT opened this issue Mar 9, 2024 · 1 comment
Open

Semantic Highlighting for Constants #5611

maflAT opened this issue Mar 9, 2024 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@maflAT
Copy link

maflAT commented Mar 9, 2024

This issue is kinda related to #5315, #3754, and #3100
but concerned with the handling of constants instead of type aliases.

Code Snippet

from typing import ClassVar, Final, final
#                                                     # semantic token type (modifiers) | Tooltips

a_variable = None                                     # variable (declaration)
A_CONSTANT = None                                     # variable (declaration, readonly)
another_constant: Final = None                        # variable (declaration)

def a_function(): ...                                 # function (declaration)
def another_function(): ...                           # function (declaration)

another_function = a_function                         # function ()
a_function_alias = a_function                         # variable (declaration)
A_CONSTANT_FUNCTION_ALIAS = a_function                # variable (declaration, readonly)
another_constant_function_alias: Final = a_function   # variable (declaration)

class AClass:
    a_class_variable: ClassVar = None                 # property (declaration)
    A_CLASS_CONSTANT: ClassVar = None                 # property (declaration)
    another_class_constant: Final = None              # property (declaration)

    def __init__(self) -> None:
        self.an_instance_variable = None              # property (declaration)
        self.AN_INSTANCE_CONSTANT = None              # property (declaration)
        self.another_instance_constant: Final = None  # property (declaration)

    def a_method(self): ...                           # method (declaration)
    def another_method(self): ...                     # method (declaration)
    @final
    def a_final_method(self): ...                     # method (declaration)

    another_method = a_method                         # method ()
    a_method_alias = a_method                         # property (declaration)
    A_CONSTANT_METHOD_ALIAS = a_method                # property (declaration)
    another_constant_method_alias: Final = a_method   # property (declaration)

    @property
    def a_property(self): ...                         # property (declaration)
    @property
    def A_CONSTANT_PROPERTY(self): ...                # property (declaration)
    @final
    @property
    def another_constant_property(self): ...          # property (declaration)

Expected behavior

Semantic highlighting displays constants in a way that is consistent with other systems of the Python extension, especially the type checker.

Actual behavior

There are significant differences between the rendered code (and underlying semantic tokens) and other systems like tooltips, outline view and auto-completion list. See the screenshot below for comparison.

Code Rendering, Outline and Tooltips (Dark Modern)

Code

From what I have observed, there are only 2 situations where constants are recognized by the semantic highlighting system.

  • uppercased symbols in the global scope
  • Enum members

There are many more situations in which other systems correctly infer if a symbol is constant, and treat it accordingly. However, there is no semantic highlighting for these situations.

  1. Final type is not recognized
    grafik
    This is probably the most obvious and easiest to solve.
    another_constant should arguably be treated identical to A_CONSTANT.

  2. Constants are not recognized at the class scope
    grafik
    All class- and instance attributes are treated as 'property'. There is no distinction between constant and variable. Constants at the class level should arguably be treated equivalently to those at module level. The distinction might be a bit trickier to implement because those class attributes might be descriptors. However, I have found that the type checker does a very good job of finding out if this is the case or not.

  3. There are 3 different cases when assigning a function to a name
    grafik
    This is more of an observation and a discussion point than a bug report.
    3.1. another_constant_function_alias should again be treated identical to A_CONSTANT_FUNCTION_ALIAS. I think this is obvious.
    3.2. a_function_alias is treated as a variable. I think this is fitting in this situation.
    3.3. Assigning a_function to another_function is a special case. It seems, if the first appearance of a name is in a function definition, it is given the 'function' token and keeps it even if it is reassigned later. This is inconsistent with a_function_alias but I think it makes sense and helps readability in this case.
    3.4 A_CONSTANT_FUNCTION_ALIAS is correctly inferred as constant. However, I would argue that it should be rendered identically to another_function. The reason is as follows:
    There are many cases in the standard library and third party packages that alias their internal functions, methods and classes to their external API under different names. When using this API it's impossible to see at a glance what's what. What you see is an arbitrary distinction between originally defined names and later reassigned names. But what you're actually interested in is a distinction between classes, functions/methods and non-callable constants and variables.
    That's the same reason that's behind the type alias issues mentioned at the top.
    This is also how the tooltips are handled. (All 3 cases are displayed in yellow and constant/variable distinction is separate.)

  4. There are 4 different cases for assigning methods
    grafik
    This is basically the same as 3. with the additional case of a 'final' decorated method. Right now this distinction is only relevant for the type checker.

  5. There is no distinction between properties and other (variable) class attributes
    grafik
    This is arguably correct, but it would be nice to have a distinction between properties (or descriptors in general) and other variable class attributes. This is currently the case for tooltips and the auto-complete list. The outline view does not distinguish between properties and methods.

Environment data

  • Language Server version: 2024.2.106
  • OS and version: win32 x64
  • Python version (and distribution if applicable, e.g. Anaconda): 3.12
  • python.analysis.indexing: true
  • python.analysis.typeCheckingMode: standard
@github-actions github-actions bot added the needs repro Issue has not been reproduced yet label Mar 9, 2024
@StellaHuang95 StellaHuang95 added enhancement New feature or request and removed needs repro Issue has not been reproduced yet labels Mar 11, 2024
@DetachHead
Copy link

basedpyright's semantic highlighting supports this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants