Skip to content

Variable shadowing not handled correctly #207

@liancheng

Description

@liancheng

When function parameters and comprehension variables have names colliding with variables defined in some outer scope, go to definitions and find references produce wrong results.

The following patch adds two test cases illustrating the issues:

diff --git a/pkg/server/definition_test.go b/pkg/server/definition_test.go
index 0a27308..00d740e 100644
--- a/pkg/server/definition_test.go
+++ b/pkg/server/definition_test.go
@@ -28,6 +28,28 @@ type definitionTestCase struct {
 }
 
 var definitionTestCases = []definitionTestCase{
+	{
+		name:     "test function parameter shadowing local variable",
+		filename: "testdata/shadow-function-parameter.jsonnet",
+		position: protocol.Position{Line: 2, Character: 2},
+		results: []definitionResult{{
+			targetRange: protocol.Range{
+				Start: protocol.Position{Line: 1, Character: 9},
+				End:   protocol.Position{Line: 1, Character: 10},
+			},
+		}},
+	},
+	{
+		name:     "test list comprehension variable shadowing local variable",
+		filename: "testdata/shadow-list-comprehension.jsonnet",
+		position: protocol.Position{Line: 2, Character: 2},
+		results: []definitionResult{{
+			targetRange: protocol.Range{
+				Start: protocol.Position{Line: 3, Character: 6},
+				End:   protocol.Position{Line: 3, Character: 7},
+			},
+		}},
+	},
 	{
 		name:     "test goto definition for var myvar",
 		filename: "./testdata/test_goto_definition.jsonnet",
diff --git a/pkg/server/testdata/shadow-function-parameter.jsonnet b/pkg/server/testdata/shadow-function-parameter.jsonnet
new file mode 100644
index 0000000..16cdc02
--- /dev/null
+++ b/pkg/server/testdata/shadow-function-parameter.jsonnet
@@ -0,0 +1,3 @@
+local x = 1;
+function(x)
+  x
diff --git a/pkg/server/testdata/shadow-list-comprehension.jsonnet b/pkg/server/testdata/shadow-list-comprehension.jsonnet
new file mode 100644
index 0000000..1131a42
--- /dev/null
+++ b/pkg/server/testdata/shadow-list-comprehension.jsonnet
@@ -0,0 +1,5 @@
+local x = 1;
+[
+  x
+  for x in std.range(1, 3)
+]

Test failure output:

=== FAIL: pkg/server TestDefinition/test_list_comprehension_variable_shadowing_local_variable (0.00s)
    definition_test.go:1066:
                Error Trace:    /Users/lian/local/src/jsonnet-language-server/main/pkg/server/definition_test.go:1066
                Error:          Not equal:
                                expected: []protocol.LocationLink{protocol.LocationLink{OriginSelectionRange:0:0-0:0, TargetURI:"file:///Users/lian/local/src/jsonnet-language-server/main/pkg/server/testdata/shadow-list-comprehension.jsonnet", TargetRange:3:6-3:7, TargetSelectionRange:3:6-3:7}}
                                actual  : []protocol.LocationLink{protocol.LocationLink{OriginSelectionRange:0:0-0:0, TargetURI:"file:///Users/lian/local/src/jsonnet-language-server/main/pkg/server/testdata/shadow-list-comprehension.jsonnet", TargetRange:0:6-0:11, TargetSelectionRange:0:6-0:7}}

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -15,3 +15,3 @@
                                    Start: (protocol.Position) {
                                -    Line: (uint32) 3,
                                +    Line: (uint32) 0,
                                     Character: (uint32) 6
                                @@ -19,4 +19,4 @@
                                    End: (protocol.Position) {
                                -    Line: (uint32) 3,
                                -    Character: (uint32) 7
                                +    Line: (uint32) 0,
                                +    Character: (uint32) 11
                                    }
                                @@ -25,3 +25,3 @@
                                    Start: (protocol.Position) {
                                -    Line: (uint32) 3,
                                +    Line: (uint32) 0,
                                     Character: (uint32) 6
                                @@ -29,3 +29,3 @@
                                    End: (protocol.Position) {
                                -    Line: (uint32) 3,
                                +    Line: (uint32) 0,
                                     Character: (uint32) 7
                Test:           TestDefinition/test_list_comprehension_variable_shadowing_local_variable

=== FAIL: pkg/server TestDefinition (0.03s)

In both test cases, the symbol x resolves to the top-level local variable instead of the function parameter and the list-comprehension variable.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions