Repository navigation
Fix variable member autocomplete - #4
Merged
Merged
Conversation
Reviewer's GuideAutocomplete now discovers GLSL variables by file and enclosing-function scope, so member resolution honors local shadowing and word completion suggests only visible user variables ahead of built-ins. Unit and end-to-end tests cover the new scope behavior, with development-only editor hooks supporting reliable cursor placement in live documents; generated docs assets are updated accordingly. Sequence diagram for scope-aware GLSL variable autocompletesequenceDiagram
participant Editor as CodeMirror Editor
participant Completion as glslCompletionSource
participant Scope as enclosingFunction
participant Variables as variablesInScope
participant Resolver as resolveVariable
Editor->>Completion: request completion
Completion->>Scope: enclosingFunction(doc, ctx.pos)
Scope-->>Completion: current function scope
Completion->>Variables: variablesInScope(doc, scope)
Variables-->>Completion: visible variables and types
Completion-->>Editor: variable options before built-ins
Editor->>Completion: request member completion
Completion->>Scope: enclosingFunction(doc, ctx.pos)
Scope-->>Completion: current function scope
Completion->>Resolver: resolveVariable(doc, scope, name)
Resolver-->>Completion: effective variable type
Completion-->>Editor: membersForType(type)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/glsl-vars.ts" line_range="61" />
<code_context>
+
+// Optional qualifier span and precision, then <type> <name> [array] [= init];
+const DECL = /^(?:(?:const|uniform|in|out|varying|attribute)\s+)?(?:(?:highp|mediump|lowp)\s+)?(\w+)\s+(\w+)(?:\s*\[[^\]]*\])?(?:\s*=\s*[^;]*?)?\s*;?$/;
+// A function definition — return type, name, params, opening brace on one line.
+const FUNC_HEADER = /^([\w.]+)\s+(\w+)\s*\([^)]*\)\s*\{\s*$/;
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Function definitions whose opening brace is on the following line do not match `FUNC_HEADER`, so their declarations are recorded at file scope. Same-name locals in such functions then leak across function boundaries and member autocomplete resolves the wrong variable type.
**Triggers:** When GLSL is formatted as `void main()` followed by a separate `{` line.
**Suggested fix:** Track a function header across lines or recognize the opening brace independently of the header line.
</issue_to_address>
### Comment 2
<location path="src/glsl-vars.ts" line_range="96-101" />
<code_context>
+ if (fm) {
+ // A function definition opens a new scope for the rest of its body.
+ depth += braces(line);
+ stack.push({ name: fm[2], baseDepth: depth });
+ scope = fm[2];
+ } else {
+ depth += braces(line);
+ while (stack.length && stack[stack.length - 1].baseDepth > depth) stack.pop();
+ scope = stack.length ? stack[stack.length - 1].name : "";
+ }
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** An empty one-line function such as `void main() {}` has a brace delta of zero, so its stack entry gets `baseDepth: 0` and is never removed by the `baseDepth > depth` check. The parser therefore treats every subsequent declaration as belonging to that function instead of file scope.
**Triggers:** When the source contains an empty one-line function before later declarations or functions.
**Suggested fix:** Pop function scopes when the function's closing brace is already on its header line, or track whether the function is still open separately from brace depth.
</issue_to_address>
### Comment 3
<location path="src/glsl-vars.ts" line_range="60-132" />
<code_context>
+const DECL = /^(?:(?:const|uniform|in|out|varying|attribute)\s+)?(?:(?:highp|mediump|lowp)\s+)?(\w+)\s+(\w+)(?:\s*\[[^\]]*\])?(?:\s*=\s*[^;]*?)?\s*;?$/;
</code_context>
<issue_to_address>
**issue (bug_risk):** Function parameters are not matched by `DECL` and are never added to `scanVariables`, so a parameter such as `vec3 normal` cannot be resolved and `normal.` produces no member completions. Parameters are also absent from word completion even though they are variables in the enclosing function scope.
**Triggers:** When the identifier before `.` or `[` is a function parameter.
**Suggested fix:** Parse function parameters and add each parameter to the corresponding function scope before resolving or listing variables.
</issue_to_address>
### Comment 4
<location path="src/glsl-vars.ts" line_range="145-150" />
<code_context>
+ * in the same function shadows it. Out-of-scope declarations (other functions)
+ * are never consulted. Among the visible candidates the later one wins.
+ */
+export function resolveVariable(src: string, scope: string, name: string): string | null {
+ const decls = scanVariables(src).get(name);
+ if (!decls) return null;
+ let type: string | null = null;
+ for (const d of decls) {
+ if (d.scope === scope || d.scope === "") type = d.type;
+ }
+ return type;
</code_context>
<issue_to_address>
**issue (bug_risk):** `resolveVariable` and `variablesInScope` consider every declaration in the matching function scope, regardless of its position relative to the cursor, and the last matching declaration wins. Before a local declaration, autocomplete therefore resolves or offers a variable that is not yet visible in GLSL source order.
**Triggers:** When completion is requested before a same-function declaration with the referenced name.
**Suggested fix:** Pass the cursor position into resolution and filter declarations to those occurring before that position before applying shadowing.
</issue_to_address>Sourcery assessment
Approval pending. 4 findings to address first.
Blocking findings: src/glsl-vars.ts:61, src/glsl-vars.ts:101, src/glsl-vars.ts:132, src/glsl-vars.ts:150
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+145
to
+150
| export function resolveVariable(src: string, scope: string, name: string): string | null { | ||
| const decls = scanVariables(src).get(name); | ||
| if (!decls) return null; | ||
| let type: string | null = null; | ||
| for (const d of decls) { | ||
| if (d.scope === scope || d.scope === "") type = d.type; |
There was a problem hiding this comment.
issue (bug_risk): resolveVariable and variablesInScope consider every declaration in the matching function scope, regardless of its position relative to the cursor, and the last matching declaration wins. Before a local declaration, autocomplete therefore resolves or offers a variable that is not yet visible in GLSL source order.
Triggers: When completion is requested before a same-function declaration with the referenced name.
Suggested fix: Pass the cursor position into resolution and filter declarations to those occurring before that position before applying shadowing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Make GLSL variable autocomplete scope-aware so identifiers resolve to the correct visible declaration and provide accurate completion options.
New Features:
Bug Fixes:
Enhancements:
Tests: