diff --git a/src/semantic/type-utils.ts b/src/semantic/type-utils.ts index 058c4371..1d6a679f 100644 --- a/src/semantic/type-utils.ts +++ b/src/semantic/type-utils.ts @@ -537,14 +537,20 @@ export function resolveFieldType( return undefined; } +/** `__VLA_D_`, the AST builder's name for an `ARRAY [*]`. */ +const VLA_NAME = /^__VLA_\d+D_(.+)$/; + /** * Resolve the element type of an array type. - * Handles __INLINE_ARRAY_* internal types and user-defined array TYPE definitions. + * Handles __INLINE_ARRAY_* and __VLA_*D_* internal types, user-defined array + * TYPE definitions, and aliases that eventually name one. */ export function resolveArrayElementType( typeName: string, ast: CompilationUnit, + depth = 0, ): string | undefined { + if (depth >= MAX_TYPE_ALIAS_DEPTH) return undefined; const typeUpper = typeName.toUpperCase(); // Handle __INLINE_ARRAY_ internal types @@ -552,14 +558,20 @@ export function resolveArrayElementType( return typeUpper.substring("__INLINE_ARRAY_".length); } + const vla = VLA_NAME.exec(typeUpper); + if (vla) return vla[1]; + // Check user-defined array type definitions for (const td of ast.types) { - if ( - td.name.toUpperCase() === typeUpper && - td.definition.kind === "ArrayDefinition" - ) { + if (td.name.toUpperCase() !== typeUpper) continue; + if (td.definition.kind === "ArrayDefinition") { return td.definition.elementType.name.toUpperCase(); } + if (td.definition.kind === "TypeReference") { + // Alias — keep walking toward the underlying array, if any. + return resolveArrayElementType(td.definition.name, ast, depth + 1); + } + return undefined; } return undefined; diff --git a/vscode-extension/server/src/completion.ts b/vscode-extension/server/src/completion.ts index 20a4ac75..e3de0670 100644 --- a/vscode-extension/server/src/completion.ts +++ b/vscode-extension/server/src/completion.ts @@ -21,8 +21,9 @@ import type { FunctionBlockType, SymbolTables, Scope, + CompilationUnit, } from "strucpp"; -import { ELEMENTARY_TYPES, typeName } from "strucpp"; +import { ELEMENTARY_TYPES, resolveArrayElementType, typeName } from "strucpp"; import { getCursorContext } from "./cursor-context.js"; import { getScopeForContext } from "./resolve-symbol.js"; import { isTestFile, extractTestVarDeclarations } from "../../shared/test-utils.js"; @@ -217,30 +218,31 @@ function getDotAccessCompletions( const scope = getScopeForContext(symbolTables, pouScope); if (!scope) return []; - // Parse the chain: "a.b.c" → resolve segment by segment - const segments = prefixExpr.split("."); - const resolvedType = resolveChainType(segments, scope, symbolTables); + // Parse the chain: "a.b[0].c" → resolve segment by segment + const segments = parseChainSegments(prefixExpr); + const resolvedType = resolveChainType(segments, scope, symbolTables, analysis.ast); if (resolvedType) return getMembersForType(resolvedType, symbolTables); // For test files, try resolving via locally declared variable types if (testSource) { const testVars = extractTestVarDeclarations(stripCommentsAndStrings(testSource)); - const varType = testVars.get(segments[0].toUpperCase()); + const varType = testVars.get(segments[0].name.toUpperCase()); if (varType) { // Walk remaining segments through type chain - let currentTypeName = varType; - for (let i = 1; i < segments.length; i++) { - const nextType = resolveMemberType(currentTypeName, segments[i], symbolTables); - if (!nextType) return []; - currentTypeName = nextType; + let currentTypeName = unwrapSubscripts(varType, segments[0].subscripts, analysis.ast); + for (let i = 1; i < segments.length && currentTypeName !== undefined; i++) { + const nextType = resolveMemberType(currentTypeName, segments[i].name, symbolTables); + if (nextType === undefined) return []; + currentTypeName = unwrapSubscripts(nextType, segments[i].subscripts, analysis.ast); } + if (currentTypeName === undefined) return []; const typeInfo = resolveTypeName(currentTypeName, symbolTables); if (typeInfo) return getMembersForType(typeInfo, symbolTables); } } // Fallback: first segment may be a type name (e.g., EnumType.MEMBER) - const typeInfo = resolveTypeName(segments[0], symbolTables); + const typeInfo = resolveTypeName(segments[0].name, symbolTables); if (typeInfo) return getMembersForType(typeInfo, symbolTables); return []; @@ -251,30 +253,88 @@ interface ResolvedTypeInfo { name: string; } +/** One link of a dotted chain: an identifier plus however many subscripts follow it. */ +interface ChainSegment { + name: string; + subscripts: number; +} + +/** + * Split "a.b[0].c" into its links, counting the subscripts on each. Dots inside + * a subscript belong to the index expression (`arr[s.k]`), not to the chain, so + * the split only happens at bracket depth 0. + */ +function parseChainSegments(prefixExpr: string): ChainSegment[] { + const segments: ChainSegment[] = []; + let name = ""; + let subscripts = 0; + let depth = 0; + + for (const ch of prefixExpr) { + if (ch === "[") { + if (depth === 0) subscripts++; + depth++; + } else if (ch === "]") { + if (depth > 0) depth--; + } else if (depth === 0) { + if (ch === ".") { + segments.push({ name, subscripts }); + name = ""; + subscripts = 0; + } else { + name += ch; + } + } + } + segments.push({ name, subscripts }); + + return segments; +} + +/** Peel one array level per subscript. Undefined once a step is not an array. */ +function unwrapSubscripts( + typeName: string, + subscripts: number, + ast?: CompilationUnit, +): string | undefined { + if (subscripts === 0) return typeName; + if (!ast) return undefined; + let current: string | undefined = typeName; + for (let i = 0; i < subscripts && current !== undefined; i++) { + current = resolveArrayElementType(current, ast); + } + return current; +} + /** * Resolve a dotted identifier chain to its final type. * e.g., "player.position" → resolves player (Sprite FB) → position (Point struct) */ function resolveChainType( - segments: string[], + segments: ChainSegment[], scope: Scope, symbolTables: SymbolTables, + ast?: CompilationUnit, ): ResolvedTypeInfo | undefined { if (segments.length === 0) return undefined; // Resolve first segment via scope lookup - const firstSym = scope.lookup(segments[0]); + const firstSym = scope.lookup(segments[0].name); if (!firstSym || firstSym.kind !== "variable") return undefined; - let currentTypeName = getVariableTypeName(firstSym as VariableSymbol); - if (!currentTypeName) return undefined; + const declaredType = getVariableTypeName(firstSym as VariableSymbol); + if (declaredType === undefined) return undefined; + let currentTypeName = unwrapSubscripts(declaredType, segments[0].subscripts, ast); + if (currentTypeName === undefined) return undefined; // Walk remaining segments for (let i = 1; i < segments.length; i++) { - const memberName = segments[i]; + const memberName = segments[i].name; const nextType = resolveMemberType(currentTypeName, memberName, symbolTables); - if (!nextType) return undefined; - currentTypeName = nextType; + if (nextType === undefined) return undefined; + const unwrapped = unwrapSubscripts(nextType, segments[i].subscripts, ast); + if (unwrapped === undefined) return undefined; + currentTypeName = unwrapped; } // Determine what kind of type this is diff --git a/vscode-extension/server/src/cursor-context.ts b/vscode-extension/server/src/cursor-context.ts index 4ce8331e..a529e8d1 100644 --- a/vscode-extension/server/src/cursor-context.ts +++ b/vscode-extension/server/src/cursor-context.ts @@ -60,7 +60,13 @@ export function getCursorContext( const prefix = currentLine.substring(0, column - 1); // Check dot-access: prefix ends with identifier chain + "." - const dotMatch = prefix.match(/([\w]+(?:\.[\w]+)*)\.\s*$/); + // A link may carry subscripts (`pts[i].`, `grid[i, j].`, `mat[i][j].`), whose + // index may itself be subscripted (`arr[idx[i]].`). The chain resolver unwraps + // one array level per bracket group. + const subscript = /\[(?:[^[\]]|\[[^[\]]*\])*\]/.source; + const dotMatch = prefix.match( + new RegExp(`([\\w]+(?:${subscript})*(?:\\.[\\w]+(?:${subscript})*)*)\\.\\s*$`), + ); if (dotMatch) { return { kind: "dot-access", diff --git a/vscode-extension/server/src/resolve-symbol.ts b/vscode-extension/server/src/resolve-symbol.ts index 6c07fb4c..526b4c19 100644 --- a/vscode-extension/server/src/resolve-symbol.ts +++ b/vscode-extension/server/src/resolve-symbol.ts @@ -259,6 +259,9 @@ export function resolveSymbolAtPosition( case "VarDeclaration": { const vd = node as VarDeclaration; + // Field names are never defined as symbols; a lookup would answer + // with whatever global shares the name. + if (isStructField(ast, vd)) return { node, scope }; // VarDeclaration can declare multiple names; resolve the first one // (exact name matching would need column-level checking in the names list) if (vd.names.length > 0) { @@ -307,6 +310,16 @@ export function lookupSymbolByName( return null; } +/** True when the declaration is a STRUCT field rather than a POU local or a global. */ +function isStructField(ast: CompilationUnit, vd: VarDeclaration): boolean { + for (const type of ast.types) { + const definition = type.definition; + if (definition.kind !== "StructDefinition") continue; + if (definition.fields.includes(vd)) return true; + } + return false; +} + export function getScopeForContext( symbolTables: NonNullable, scope: EnclosingScope, diff --git a/vscode-extension/tests/unit/completion.test.ts b/vscode-extension/tests/unit/completion.test.ts index bac58254..e98a6f25 100644 --- a/vscode-extension/tests/unit/completion.test.ts +++ b/vscode-extension/tests/unit/completion.test.ts @@ -408,3 +408,136 @@ END_PROGRAM expect(labelled(items, "plain[")).toEqual([]); }); }); + +describe("dot-access past an array subscript", () => { + const SOURCE = `TYPE + Point : STRUCT + x : REAL; + y : REAL; + END_STRUCT; +END_TYPE + +TYPE + PointArray : ARRAY [0..9] OF Point; +END_TYPE + +TYPE + AliasArr : PointArray; +END_TYPE + +TYPE + Row : ARRAY [0..3] OF Point; +END_TYPE + +TYPE + Idx : STRUCT + k : INT; + END_STRUCT; +END_TYPE + +TYPE + Holder : STRUCT + pts : ARRAY [0..3] OF Point; + END_STRUCT; +END_TYPE + +PROGRAM Main +VAR + inlineArr : ARRAY [0..9] OF Point; + named : PointArray; + aliased : AliasArr; + mat : ARRAY [0..3] OF Row; + grid : ARRAY [0..3, 0..3] OF Point; + reals : ARRAY [0..9] OF REAL; + indices : ARRAY [0..3] OF INT; + holder : Holder; + plain : Point; + s : Idx; + i : INT; +END_VAR + PROBE +END_PROGRAM +`; + + /** Complete immediately after `expr`, which must end in a dot. */ + const completeAfter = (expr: string): string[] => { + const source = SOURCE.replace(" PROBE", ` ${expr}`); + const analysis = analyze(source, { fileName: "a.st" }); + const lines = source.split("\n"); + const line = lines.findIndex((l) => l.includes(expr)) + 1; + const col = lines[line - 1].indexOf(expr) + expr.length + 1; + return upperLabels(getCompletions(analysis, "a.st", line, col, source)); + }; + + it("offers the element type's fields for an inline array", () => { + expect(completeAfter("inlineArr[i].")).toEqual(["X", "Y"]); + }); + + it("offers the element type's fields for a named array type", () => { + expect(completeAfter("named[i].")).toEqual(["X", "Y"]); + }); + + it("offers the element type's fields for a multi-dimensional array", () => { + expect(completeAfter("grid[i, i].")).toEqual(["X", "Y"]); + }); + + it("offers the element type's fields for an array reached through a field", () => { + expect(completeAfter("holder.pts[i].")).toEqual(["X", "Y"]); + }); + + it("matches the non-array baseline", () => { + expect(completeAfter("inlineArr[i].")).toEqual(completeAfter("plain.")); + }); + + it("offers nothing for an array of an elementary type", () => { + // Not the keyword fallback: the context is dot-access, REAL has no members. + expect(completeAfter("reals[i].")).toEqual([]); + }); + + it("follows a TYPE alias to the array it names", () => { + expect(completeAfter("aliased[i].")).toEqual(["X", "Y"]); + }); + + it("peels one level per bracket group on an array of arrays", () => { + expect(completeAfter("mat[i][i].")).toEqual(["X", "Y"]); + }); + + it("keeps a dot inside the subscript out of the chain", () => { + expect(completeAfter("inlineArr[s.k].")).toEqual(["X", "Y"]); + }); + + it("accepts a subscripted index expression", () => { + expect(completeAfter("inlineArr[indices[i]].")).toEqual(["X", "Y"]); + }); +}); + +describe("dot-access past a variable-length array subscript", () => { + // `ARRAY [*]` reaches the AST as `__VLA_D_`, a different + // internal spelling from an inline array's `__INLINE_ARRAY_`. + const SOURCE = `TYPE + Point : STRUCT + x : REAL; + y : REAL; + END_STRUCT; +END_TYPE + +FUNCTION_BLOCK Sum +VAR_IN_OUT + vals : ARRAY [*] OF Point; +END_VAR +VAR + i : INT; +END_VAR + PROBE +END_FUNCTION_BLOCK +`; + + it("offers the element type's fields", () => { + const source = SOURCE.replace(" PROBE", " vals[i]."); + const analysis = analyze(source, { fileName: "v.st" }); + const lines = source.split("\n"); + const line = lines.findIndex((l) => l.includes("vals[i].")) + 1; + const col = lines[line - 1].indexOf("vals[i].") + "vals[i].".length + 1; + expect(upperLabels(getCompletions(analysis, "v.st", line, col, source))).toEqual(["X", "Y"]); + }); +}); diff --git a/vscode-extension/tests/unit/definition.test.ts b/vscode-extension/tests/unit/definition.test.ts index cf4b7958..2caa77f7 100644 --- a/vscode-extension/tests/unit/definition.test.ts +++ b/vscode-extension/tests/unit/definition.test.ts @@ -374,3 +374,37 @@ describe("getTypeDefinition", () => { expect(def!.range.start.line).toBe(declLine); }); }); + +describe("getDefinition on STRUCT field declarations", () => { + // The disruptive half of the same defect: a field resolving to a global + // symbol does not just show the wrong tooltip, it navigates there. + const SOURCE = `FUNCTION_BLOCK Motor +VAR_INPUT + Speed : INT; +END_VAR +END_FUNCTION_BLOCK + +TYPE + SensorData : STRUCT + Temperature : REAL; + Motor : REAL; + END_STRUCT; +END_TYPE +`; + + const definitionOn = (field: string) => { + const analysis = analyze(SOURCE, { fileName: "s.st" }); + const lines = SOURCE.split("\n"); + const line = lines.findIndex((l) => l.includes(` ${field} : REAL;`)); + const col = lines[line].indexOf(field) + 1; + return getDefinition(analysis, "s.st", line + 1, col, "file:///s.st"); + }; + + it("does not navigate from an ordinary field", () => { + expect(definitionOn("Temperature")).toBeNull(); + }); + + it("does not navigate from a field named after a function block", () => { + expect(definitionOn("Motor")).toBeNull(); + }); +}); diff --git a/vscode-extension/tests/unit/hover.test.ts b/vscode-extension/tests/unit/hover.test.ts index 19e600a2..762f071d 100644 --- a/vscode-extension/tests/unit/hover.test.ts +++ b/vscode-extension/tests/unit/hover.test.ts @@ -231,3 +231,87 @@ END_PROGRAM expect(text).not.toContain("__INLINE_ARRAY"); }); }); + +describe("getHover on STRUCT field declarations", () => { + // A field name is a declaration, never a reference. It is not defined as a + // symbol, so resolution must not fall through to the global scope and answer + // with whatever happens to share the name. + const SOURCE = `FUNCTION_BLOCK Motor +VAR_INPUT + Speed : INT; +END_VAR +END_FUNCTION_BLOCK + +FUNCTION Distance : REAL +VAR_INPUT + a : REAL; +END_VAR +Distance := a; +END_FUNCTION + +TYPE + SensorData : STRUCT + Temperature : REAL; + Motor : REAL; + Distance : REAL; + END_STRUCT; +END_TYPE +`; + + const hoverOn = (field: string) => { + const analysis = analyze(SOURCE, { fileName: "s.st" }); + const lines = SOURCE.split("\n"); + const line = lines.findIndex((l) => l.includes(` ${field} : REAL;`)); + const col = lines[line].indexOf(field) + 1; + return getHover(analysis, "s.st", line + 1, col); + }; + + it("answers nothing for an ordinary field", () => { + expect(hoverOn("Temperature")).toBeNull(); + }); + + it("answers nothing for a field named after a function block", () => { + expect(hoverOn("Motor")).toBeNull(); + }); + + it("answers nothing for a field named after a function", () => { + expect(hoverOn("Distance")).toBeNull(); + }); +}); + +describe("getHover on declarations outside a STRUCT", () => { + it("still resolves a POU local named after a function block", () => { + const source = `FUNCTION_BLOCK Motor +VAR_INPUT + Speed : INT; +END_VAR +END_FUNCTION_BLOCK + +PROGRAM Main +VAR + Motor : INT; +END_VAR +Motor := 1; +END_PROGRAM +`; + const analysis = analyze(source, { fileName: "p.st" }); + const lines = source.split("\n"); + const line = lines.findIndex((l) => l.includes(" Motor : INT;")); + const hover = getHover(analysis, "p.st", line + 1, lines[line].indexOf("Motor") + 1); + expect(hover).not.toBeNull(); + const value = (hover!.contents as { value: string }).value.toUpperCase(); + expect(value).toContain("INT"); + expect(value).not.toContain("FUNCTION_BLOCK"); + }); + + it("still resolves a global variable declaration", () => { + const source = `VAR_GLOBAL + Pressure : REAL; +END_VAR +`; + const analysis = analyze(source, { fileName: "g.st" }); + const hover = getHover(analysis, "g.st", 2, 3); + expect(hover).not.toBeNull(); + expect((hover!.contents as { value: string }).value.toUpperCase()).toContain("REAL"); + }); +}); diff --git a/vscode-extension/tests/unit/rename.test.ts b/vscode-extension/tests/unit/rename.test.ts index c667fe7b..1b148e5e 100644 --- a/vscode-extension/tests/unit/rename.test.ts +++ b/vscode-extension/tests/unit/rename.test.ts @@ -151,3 +151,47 @@ describe("getRenameEdits", () => { expect(edits).toBeNull(); }); }); + +describe("rename on a STRUCT field", () => { + // The most damaging shape of the same defect: before the field was excluded + // from global lookup, renaming it renamed the function block it collided + // with, along with every reference to that block. + const SOURCE = `FUNCTION_BLOCK Motor +VAR_INPUT + Speed : INT; +END_VAR +END_FUNCTION_BLOCK + +PROGRAM Main +VAR + drive : Motor; +END_VAR +drive(Speed := 1); +END_PROGRAM + +TYPE + SensorData : STRUCT + Motor : REAL; + END_STRUCT; +END_TYPE +`; + + const fieldPosition = () => { + const lines = SOURCE.split("\n"); + const line = lines.findIndex((l) => l.includes(" Motor : REAL;")); + return { line: line + 1, col: lines[line].indexOf("Motor") + 1 }; + }; + + it("offers no rename for a field named after a function block", () => { + const analysis = analyze(SOURCE, { fileName: "s.st" }); + const pos = fieldPosition(); + expect(prepareRename(analysis, "s.st", pos.line, pos.col)).toBeNull(); + }); + + it("produces no edits for a field named after a function block", () => { + const analysis = analyze(SOURCE, { fileName: "s.st" }); + const pos = fieldPosition(); + const edits = getRenameEdits(analysis, "s.st", pos.line, pos.col, "Speed2", "file:///s.st"); + expect(edits === null || Object.keys(edits).length === 0).toBe(true); + }); +}); diff --git a/vscode-extension/tests/unit/resolve-symbol.test.ts b/vscode-extension/tests/unit/resolve-symbol.test.ts index ad4f0df0..91e40fe1 100644 --- a/vscode-extension/tests/unit/resolve-symbol.test.ts +++ b/vscode-extension/tests/unit/resolve-symbol.test.ts @@ -100,3 +100,35 @@ describe("resolveSymbolAtPosition", () => { expect(resolved!.scope.parentName!.toUpperCase()).toBe("SPRITE"); }); }); + +describe("resolveSymbolAtPosition on a STRUCT field", () => { + const SOURCE = `FUNCTION_BLOCK Motor +VAR_INPUT + Speed : INT; +END_VAR +END_FUNCTION_BLOCK + +TYPE + SensorData : STRUCT + Motor : REAL; + END_STRUCT; +END_TYPE +`; + + it("resolves the declaration node without a symbol", () => { + const analysis = analyze(SOURCE, { fileName: "s.st" }); + const lines = SOURCE.split("\n"); + const line = lines.findIndex((l) => l.includes(" Motor : REAL;")); + const resolved = resolveSymbolAtPosition( + analysis, + "s.st", + line + 1, + lines[line].indexOf("Motor") + 1, + ); + expect(resolved).toBeDefined(); + expect(resolved!.node.kind).toBe("VarDeclaration"); + // The field shares its name with a global function block. A declaration + // must not borrow that symbol. + expect(resolved!.symbol).toBeUndefined(); + }); +});