search

microsoft / pylance-release

Documentation and issues for Pylance
Creative Commons Attribution 4.0 International
1.72k stars 766 forks source link

Semantic Highlighting for Constants #5611

Open maflAT opened 8 months ago

maflAT commented 8 months ago

This issue is kinda related to https://github.com/microsoft/pylance-release/issues/5315, https://github.com/microsoft/pylance-release/issues/3754, and https://github.com/microsoft/pylance-release/issues/3100 but concerned with the handling of constants instead of type aliases.

Code Snippet

 ```python 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](https://github.com/microsoft/pylance-release/assets/13979550/0cc76452-888a-4288-bd18-56b935b87399)

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

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

DetachHead commented 8 months ago

basedpyright's semantic highlighting supports this