Minimize semantic tokens returned for local variables (#2482)

* Minimize constant and method semantic tokens * Minimize semantic tokens returned for local variables * Ensure completeness in local var operator assignment capture
Shopify · Aug 23, 2024 · a101647 · a101647
1 parent e02ce6d
commit a101647
Show file tree

Hide file tree

Showing 16 changed files with 80 additions and 179 deletions.
diff --git a/lib/ruby_lsp/listeners/semantic_highlighting.rb b/lib/ruby_lsp/listeners/semantic_highlighting.rb
@@ -41,7 +41,6 @@ def initialize(dispatcher, response_builder)
           :on_block_node_leave,
           :on_self_node_enter,
           :on_module_node_enter,
-          :on_local_variable_write_node_enter,
           :on_local_variable_read_node_enter,
           :on_block_parameter_node_enter,
           :on_required_keyword_parameter_node_enter,
@@ -50,9 +49,6 @@ def initialize(dispatcher, response_builder)
           :on_optional_parameter_node_enter,
           :on_required_parameter_node_enter,
           :on_rest_parameter_node_enter,
-          :on_local_variable_and_write_node_enter,
-          :on_local_variable_operator_write_node_enter,
-          :on_local_variable_or_write_node_enter,
           :on_local_variable_target_node_enter,
           :on_block_local_variable_node_enter,
           :on_match_write_node_enter,
@@ -168,11 +164,6 @@ def on_self_node_enter(node)
         @response_builder.add_token(node.location, :variable, [:default_library])
       end
 
-      sig { params(node: Prism::LocalVariableWriteNode).void }
-      def on_local_variable_write_node_enter(node)
-        @response_builder.add_token(node.name_loc, @current_scope.type_for(node.name))
-      end
-
       sig { params(node: Prism::LocalVariableReadNode).void }
       def on_local_variable_read_node_enter(node)
         return if @inside_implicit_node
@@ -186,21 +177,6 @@ def on_local_variable_read_node_enter(node)
         @response_builder.add_token(node.location, @current_scope.type_for(node.name))
       end
 
-      sig { params(node: Prism::LocalVariableAndWriteNode).void }
-      def on_local_variable_and_write_node_enter(node)
-        @response_builder.add_token(node.name_loc, @current_scope.type_for(node.name))
-      end
-
-      sig { params(node: Prism::LocalVariableOperatorWriteNode).void }
-      def on_local_variable_operator_write_node_enter(node)
-        @response_builder.add_token(node.name_loc, @current_scope.type_for(node.name))
-      end
-
-      sig { params(node: Prism::LocalVariableOrWriteNode).void }
-      def on_local_variable_or_write_node_enter(node)
-        @response_builder.add_token(node.name_loc, @current_scope.type_for(node.name))
-      end
-
       sig { params(node: Prism::LocalVariableTargetNode).void }
       def on_local_variable_target_node_enter(node)
         # If we're inside a regex capture, Prism will add LocalVariableTarget nodes for each captured variable.

diff --git a/test/expectations/semantic_highlighting/aref_field.exp.json b/test/expectations/semantic_highlighting/aref_field.exp.json
@@ -1,14 +1,7 @@
 {
   "result": [
     {
-      "delta_line": 1,
-      "delta_start_char": 2,
-      "length": 1,
-      "token_type": 8,
-      "token_modifiers": 0
-    },
-    {
-      "delta_line": 1,
+      "delta_line": 2,
       "delta_start_char": 2,
       "length": 1,
       "token_type": 8,

diff --git a/test/expectations/semantic_highlighting/aref_variable.exp.json b/test/expectations/semantic_highlighting/aref_variable.exp.json
@@ -1,14 +1,7 @@
 {
   "result": [
     {
-      "delta_line": 1,
-      "delta_start_char": 2,
-      "length": 1,
-      "token_type": 8,
-      "token_modifiers": 0
-    },
-    {
-      "delta_line": 1,
+      "delta_line": 2,
       "delta_start_char": 2,
       "length": 1,
       "token_type": 8,

diff --git a/test/expectations/semantic_highlighting/call_invocation_with_variable_receiver.exp.json b/test/expectations/semantic_highlighting/call_invocation_with_variable_receiver.exp.json
@@ -1,14 +1,7 @@
 {
   "result": [
     {
-      "delta_line": 1,
-      "delta_start_char": 2,
-      "length": 3,
-      "token_type": 8,
-      "token_modifiers": 0
-    },
-    {
-      "delta_line": 1,
+      "delta_line": 2,
       "delta_start_char": 2,
       "length": 3,
       "token_type": 8,

diff --git a/test/expectations/semantic_highlighting/command_call.exp.json b/test/expectations/semantic_highlighting/command_call.exp.json
@@ -1,12 +1,5 @@
 {
   "result": [
-    {
-      "delta_line": 0,
-      "delta_start_char": 0,
-      "length": 3,
-      "token_type": 8,
-      "token_modifiers": 0
-    },
     {
       "delta_line": 1,
       "delta_start_char": 0,

diff --git a/test/expectations/semantic_highlighting/command_invocation_variable.exp.json b/test/expectations/semantic_highlighting/command_invocation_variable.exp.json
@@ -1,12 +1,5 @@
 {
   "result": [
-    {
-      "delta_line": 0,
-      "delta_start_char": 0,
-      "length": 3,
-      "token_type": 8,
-      "token_modifiers": 0
-    },
     {
       "delta_line": 1,
       "delta_start_char": 5,

diff --git a/test/expectations/semantic_highlighting/defs.exp.json b/test/expectations/semantic_highlighting/defs.exp.json
@@ -6,13 +6,6 @@
       "length": 4,
       "token_type": 8,
       "token_modifiers": 512
-    },
-    {
-      "delta_line": 1,
-      "delta_start_char": 2,
-      "length": 1,
-      "token_type": 8,
-      "token_modifiers": 0
     }
   ]
 }
diff --git a/test/expectations/semantic_highlighting/fcall_invocation_variable_arguments.exp.json b/test/expectations/semantic_highlighting/fcall_invocation_variable_arguments.exp.json
@@ -1,14 +1,7 @@
 {
   "result": [
     {
-      "delta_line": 1,
-      "delta_start_char": 2,
-      "length": 3,
-      "token_type": 8,
-      "token_modifiers": 0
-    },
-    {
-      "delta_line": 1,
+      "delta_line": 2,
       "delta_start_char": 13,
       "length": 3,
       "token_type": 8,

diff --git a/test/expectations/semantic_highlighting/local_variables.exp.json b/test/expectations/semantic_highlighting/local_variables.exp.json
@@ -1,14 +1,7 @@
 {
   "result": [
     {
-      "delta_line": 1,
-      "delta_start_char": 2,
-      "length": 3,
-      "token_type": 8,
-      "token_modifiers": 0
-    },
-    {
-      "delta_line": 1,
+      "delta_line": 2,
       "delta_start_char": 2,
       "length": 3,
       "token_type": 8,

diff --git a/test/expectations/semantic_highlighting/local_vars_and_params.exp.json b/test/expectations/semantic_highlighting/local_vars_and_params.exp.json
@@ -2,14 +2,7 @@
   "result": [
     {
       "delta_line": 1,
-      "delta_start_char": 2,
-      "length": 1,
-      "token_type": 8,
-      "token_modifiers": 0
-    },
-    {
-      "delta_line": 0,
-      "delta_start_char": 4,
+      "delta_start_char": 6,
       "length": 1,
       "token_type": 7,
       "token_modifiers": 0

diff --git a/test/expectations/semantic_highlighting/multibyte_characters.exp.json b/test/expectations/semantic_highlighting/multibyte_characters.exp.json
@@ -15,22 +15,8 @@
       "token_modifiers": 1
     },
     {
-      "delta_line": 2,
-      "delta_start_char": 6,
-      "length": 2,
-      "token_type": 8,
-      "token_modifiers": 0
-    },
-    {
-      "delta_line": 1,
-      "delta_start_char": 6,
-      "length": 2,
-      "token_type": 8,
-      "token_modifiers": 0
-    },
-    {
-      "delta_line": 0,
-      "delta_start_char": 5,
+      "delta_line": 3,
+      "delta_start_char": 11,
       "length": 2,
       "token_type": 8,
       "token_modifiers": 0

diff --git a/test/expectations/semantic_highlighting/pinned_variable.exp.json b/test/expectations/semantic_highlighting/pinned_variable.exp.json
@@ -1,14 +1,7 @@
 {
     "result": [
         {
-            "delta_line": 1,
-            "delta_start_char": 2,
-            "length": 1,
-            "token_type": 8,
-            "token_modifiers": 0
-        },
-        {
-            "delta_line": 2,
+            "delta_line": 3,
             "delta_start_char": 6,
             "length": 1,
             "token_type": 8,

diff --git a/test/expectations/semantic_highlighting/shadowed_block_arg.exp.json b/test/expectations/semantic_highlighting/shadowed_block_arg.exp.json
@@ -1,12 +1,5 @@
 {
   "result": [
-    {
-      "delta_line": 0,
-      "delta_start_char": 0,
-      "length": 3,
-      "token_type": 8,
-      "token_modifiers": 0
-    },
     {
       "delta_line": 3,
       "delta_start_char": 7,

diff --git a/test/expectations/semantic_highlighting/var_aref_variable.exp.json b/test/expectations/semantic_highlighting/var_aref_variable.exp.json
diff --git a/test/expectations/semantic_highlighting/var_field_variable.exp.json b/test/expectations/semantic_highlighting/var_field_variable.exp.json
@@ -1,50 +1,8 @@
 {
   "result": [
     {
-      "delta_line": 1,
-      "delta_start_char": 2,
-      "length": 1,
-      "token_type": 8,
-      "token_modifiers": 0
-    },
-    {
-      "delta_line": 1,
-      "delta_start_char": 2,
-      "length": 1,
-      "token_type": 8,
-      "token_modifiers": 0
-    },
-    {
-      "delta_line": 1,
-      "delta_start_char": 2,
-      "length": 1,
-      "token_type": 8,
-      "token_modifiers": 0
-    },
-    {
-      "delta_line": 1,
-      "delta_start_char": 2,
-      "length": 1,
-      "token_type": 8,
-      "token_modifiers": 0
-    },
-    {
-      "delta_line": 1,
-      "delta_start_char": 2,
-      "length": 1,
-      "token_type": 8,
-      "token_modifiers": 0
-    },
-    {
-      "delta_line": 1,
-      "delta_start_char": 2,
-      "length": 1,
-      "token_type": 8,
-      "token_modifiers": 0
-    },
-    {
-      "delta_line": 0,
-      "delta_start_char": 4,
+      "delta_line": 6,
+      "delta_start_char": 6,
       "length": 1,
       "token_type": 8,
       "token_modifiers": 0

diff --git a/vscode/grammars/ruby.cson.json b/vscode/grammars/ruby.cson.json
@@ -100,6 +100,51 @@
       "match": "(?<!\\.)\\belse(\\s)+if\\b",
       "name": "invalid.deprecated.ruby"
     },
+    {
+      "captures": {
+        "1": {
+          "name": "variable.ruby"
+        },
+        "3": {
+          "name": "keyword.operator.assignment.augmented.ruby"
+        }
+      },
+      "match": "^\\s*([a-z]([A-Za-z0-9_])*)\\s*(&&=)",
+      "comment": "A local variable and assignment"
+    },
+    {
+      "captures": {
+        "1": {
+          "name": "variable.ruby"
+        },
+        "3": {
+          "name": "keyword.operator.assignment.augmented.ruby"
+        }
+      },
+      "match": "^\\s*([a-z]([A-Za-z0-9_])*)\\s*(\\|\\|=)",
+      "comment": "A local variable or assignment"
+    },
+    {
+      "captures": {
+        "1": {
+          "name": "variable.ruby"
+        },
+        "3": {
+          "name": "keyword.operator.assignment.augmented.ruby"
+        }
+      },
+      "match": "^\\s*([a-z]([A-Za-z0-9_])*)\\s*((\\+|\\*|-|\\/|%|\\*\\*|&|\\||\\^|<<|>>)=)",
+      "comment": "A local variable operation assignment (+=, -=, *=, /=)"
+    },
+    {
+      "captures": {
+        "1": {
+          "name": "variable.ruby"
+        }
+      },
+      "match": "^\\s*([a-z]([A-Za-z0-9_])*)\\s*=[^=]",
+      "comment": "A local variable assignment"
+    },
     {
       "captures": {
         "1": {
@@ -238,6 +283,30 @@
       "match": "\\b[_A-Z]\\w*\\b",
       "name": "variable.other.constant.ruby"
     },
+    {
+      "begin": "(->)\\(",
+      "beginCaptures": {
+        "1": {
+          "name": "support.function.kernel.ruby"
+        }
+      },
+      "comment": "Lambda parameters.",
+      "end": "\\)",
+      "patterns": [
+        {
+          "begin": "(?=[&*_a-zA-Z])",
+          "end": "(?=[,)])",
+          "patterns": [
+            {
+              "include": "#method_parameters"
+            }
+          ]
+        },
+        {
+          "include": "#method_parameters"
+        }
+      ]
+    },
     {
       "begin": "(?x)\n(?=def\\b)                          # optimization to help Oniguruma fail fast\n(?<=^|\\s)(def)\\s+\n(\n  (?>[a-zA-Z_]\\w*(?>\\.|::))?      # method prefix\n  (?>                               # method name\n    [a-zA-Z_]\\w*(?>[?!]|=(?!>))?\n    |\n    ===?|!=|>[>=]?|<=>|<[<=]?|[%&`/\\|]|\\*\\*?|=?~|[-+]@?|\\[]=?\n  )\n)\n\\s*(\\()",
       "beginCaptures": {