grafana / grafana/jsonnet-language-server
Variable shadowing not handled correctly
- Dominant language
- Go
- Stars
- 223
- Forks
- 33
- PR merge metrics
- No merged PRs in 30d
Description
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
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.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with pkg/server/definition_test.go and the two fixtures in pkg/server/testdata: shadow-function-parameter.jsonnet and shadow-list-comprehension.jsonnet. Run TestDefinition to reproduce the failures, then trace the definition lookup used by these cases. Done means both shadowing tests pass and x resolves to the parameter or comprehension variable rather than the outer local.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- devtools
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100