From d5e804f9be0323a96188dfbdbaf8fcc94a249506 Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Tue, 21 Jul 2026 20:57:54 -0400 Subject: [PATCH 1/2] fix(graphical): bind dotted box refs as constants + resolve globals in LSP scope query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Byte-identical companion to the openplc-editor PR (shared frontend surface). 1. Dotted / non-identifier box entries (member access like some_global_complex.structureVar, typed literals like T#500ms, reserved words) are bound to the block verbatim as a constant/reference instead of being routed to createVariable, whose isLegalIdentifier check rejects them with an Illegal Variable Name toast and discards the entry. New variable NAMES still reject those characters. Applied to all four box-commit sites (fbd/ladder autocomplete + block). 2. serializePouScopeForQuery re-emits the POU's VAR_EXTERNAL variables as plain VAR in the throwaway scope-query doc, so strucpp resolves globals and their struct/array members without a matching VAR_GLOBAL — restoring LSP autocomplete + validation for global variables in LD and FBD. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../fbd/autocomplete/index.tsx | 12 +++++++- .../_atoms/graphical-editor/fbd/block.tsx | 8 ++++++ .../ladder/autocomplete/index.tsx | 17 ++++++++++- .../_atoms/graphical-editor/ladder/block.tsx | 8 ++++++ .../pou-signature-serializer.test.ts | 28 +++++++++++++++++++ .../utils/PLC/pou-signature-serializer.ts | 12 +++++++- 6 files changed, 82 insertions(+), 3 deletions(-) diff --git a/src/frontend/components/_atoms/graphical-editor/fbd/autocomplete/index.tsx b/src/frontend/components/_atoms/graphical-editor/fbd/autocomplete/index.tsx index ed5144210..92d4dc968 100644 --- a/src/frontend/components/_atoms/graphical-editor/fbd/autocomplete/index.tsx +++ b/src/frontend/components/_atoms/graphical-editor/fbd/autocomplete/index.tsx @@ -10,7 +10,7 @@ import { } from '../../../../../services/graphical-scope' import { useOpenPLCStore } from '../../../../../store' import { cn } from '../../../../../utils/cn' -import { getLiteralType } from '../../../../../utils/keywords' +import { getLiteralType, isLegalIdentifier } from '../../../../../utils/keywords' import { toast } from '../../../../_features/[app]/toast/use-toast' import { useBoundPou } from '../../../../_features/[workspace]/editor/graphical/active-context' import { buildGenericNode } from '../../../../_molecules/graphical-editor/fbd/fbd-utils/nodes' @@ -130,6 +130,16 @@ const FBDBlockAutoComplete = forwardRef(block: BlockProps) => { if (matchingVariable) { variableToLink = matchingVariable } else if (createIfNotFound) { + // An entry that can't be a new variable NAME — a member/array reference, + // a typed literal (`T#500ms`), a reserved word — is bound to the block + // verbatim as a constant/reference instead of erroring. + if (!isLegalIdentifier(variableNameToSubmit)[0]) { + updateNodeVariable({ name: variableNameToSubmit }) + return + } const pouData = pous.find((p) => p.name === pouName) pushToHistory(pouName, { variables: pouData?.interface?.variables ?? [], diff --git a/src/frontend/components/_atoms/graphical-editor/ladder/autocomplete/index.tsx b/src/frontend/components/_atoms/graphical-editor/ladder/autocomplete/index.tsx index 5134e741c..4a87655e0 100644 --- a/src/frontend/components/_atoms/graphical-editor/ladder/autocomplete/index.tsx +++ b/src/frontend/components/_atoms/graphical-editor/ladder/autocomplete/index.tsx @@ -11,7 +11,7 @@ import { } from '../../../../../services/graphical-scope' import { useOpenPLCStore } from '../../../../../store' import { cn } from '../../../../../utils/cn' -import { getLiteralType } from '../../../../../utils/keywords' +import { getLiteralType, isLegalIdentifier } from '../../../../../utils/keywords' import { toast } from '../../../../_features/[app]/toast/use-toast' import { useBoundPou } from '../../../../_features/[workspace]/editor/graphical/active-context' import { GraphicalEditorAutocomplete } from '../../autocomplete' @@ -187,6 +187,21 @@ const VariablesBlockAutoComplete = forwardRef(block: BlockProps) => { if (matchingVariable) { variableToLink = matchingVariable } else if (createIfNotFound) { + // An entry that can't be a new variable NAME — a member/array reference, + // a typed literal (`T#500ms`), a reserved word — is bound to the block + // verbatim as a constant/reference instead of erroring. + if (!isLegalIdentifier(variableNameToSubmit)[0]) { + updateNodeVariable({ name: variableNameToSubmit }) + return + } const project = useOpenPLCStore.getState().project const currentPou = project.data.pous.find((p) => p.name === pouName) pushToHistory(pouName, { diff --git a/src/frontend/utils/PLC/__tests__/pou-signature-serializer.test.ts b/src/frontend/utils/PLC/__tests__/pou-signature-serializer.test.ts index bf9d4728c..51870235a 100644 --- a/src/frontend/utils/PLC/__tests__/pou-signature-serializer.test.ts +++ b/src/frontend/utils/PLC/__tests__/pou-signature-serializer.test.ts @@ -185,6 +185,34 @@ describe('serializePouSignatureToST', () => { expect(text.split('\n')[position.line]).toBe('in1.') }) + it('re-emits external variables as plain VAR so globals resolve in the throwaway doc', () => { + const pou = makePou({ + name: 'Main', + pouType: 'program', + interface: { + variables: [ + { + id: 'g1', + name: 'some_global_complex', + class: 'external', + type: { definition: 'derived', value: 'testing_arr' }, + documentation: '', + debug: false, + location: '', + }, + ], + }, + body: { language: 'fbd', value: {} as never }, + }) + const { text } = serializePouScopeForQuery(pou, 'some_global_complex.') + // The external is rewritten to a self-contained local VAR (type inline) + // so strucpp resolves the global + its struct/array members without a + // matching VAR_GLOBAL in this throwaway document. + expect(text).toContain('some_global_complex : testing_arr;') + expect(text).not.toContain('VAR_EXTERNAL') + expect(text).toContain('some_global_complex.') + }) + it('keeps the function return type while swapping only the name', () => { const pou = makePou({ name: 'AbsInt', diff --git a/src/frontend/utils/PLC/pou-signature-serializer.ts b/src/frontend/utils/PLC/pou-signature-serializer.ts index aa1cef02b..f361ae99b 100644 --- a/src/frontend/utils/PLC/pou-signature-serializer.ts +++ b/src/frontend/utils/PLC/pou-signature-serializer.ts @@ -139,7 +139,17 @@ export function serializePouScopeForQuery( pou.pouType === 'function' && pou.interface?.returnType ? `${startKeyword} ${name} : ${pou.interface.returnType}` : `${startKeyword} ${name}` - const variables = generateIecVariablesToString(pou.interface?.variables ?? []) + // External variables (`VAR_EXTERNAL`) reference resource globals declared in a + // separate CONFIGURATION document. This throwaway query doc isn't part of that + // configuration, so strucpp can't resolve a bare `VAR_EXTERNAL` here — the + // global and its struct/array members would come back unknown (the box shows + // yellow, no autocomplete). Re-emit externals as plain `VAR`: they carry their + // real type inline, so the symbol and its members resolve self-containedly. + // Scope-query-only — the POU's real stub keeps `VAR_EXTERNAL`. + const scopeVariables = (pou.interface?.variables ?? []).map((variable) => + variable.class === 'external' ? { ...variable, class: 'local' as const } : variable, + ) + const variables = generateIecVariablesToString(scopeVariables) const prefix = `${declaration}\n${variables}\n` // `prefix` ends with '\n', so split length - 1 is the 0-indexed line // the body expression sits on. From 169e7c8f72d101de514db4d032cdd533cf18ec63 Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Wed, 22 Jul 2026 06:14:07 -0400 Subject: [PATCH 2/2] fix(graphical): accept LSP Field kind so struct members autocomplete/validate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Struct-typed variables (local or via a global VAR_EXTERNAL) never offered their members in the LD/FBD variable boxes, e.g. `test_complex.structureBool` for a BOOL box. Root cause: strucpp returns STRUCT members with CompletionItemKind.Field (5), while graphical-scope only accepted Variable (6) — the kind FUNCTION_BLOCK instance members come back as. So struct fields were dropped by the kind filter before the type filter ran, in both the completion and validation paths. Accept Field (5) alongside Variable (6) via isValueCompletionKind at all three call sites (completion, member-drill, type resolution). Verified live in the browser: `.` now lists all fields in an ANY box and narrows to the type-compatible field (structureBool) in a BOOL box. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/frontend/services/graphical-scope.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/frontend/services/graphical-scope.ts b/src/frontend/services/graphical-scope.ts index 17d634a6e..da7c79f68 100644 --- a/src/frontend/services/graphical-scope.ts +++ b/src/frontend/services/graphical-scope.ts @@ -23,8 +23,17 @@ import type { PLCVariable } from '../../middleware/shared/ports/types' import { getVariableRestrictionType, validateVariableType } from '../utils/PLC/validate-variable-type' import { getScopedQueryApi } from './st-lsp' -/** LSP `CompletionItemKind.Variable` — strucpp's kind for in-scope variables and instance/struct members. */ +/** + * LSP `CompletionItemKind`s that denote a value symbol bindable to a box. + * strucpp emits `Variable` (6) for in-scope variables and FUNCTION_BLOCK + * instance members (`TON0.Q`), but `Field` (5) for STRUCT members + * (`my_struct.field`). Both must be accepted, or struct-member access never + * autocompletes or validates. + */ const LSP_KIND_VARIABLE = 6 +const LSP_KIND_FIELD = 5 +const isValueCompletionKind = (kind: number | undefined): boolean => + kind === LSP_KIND_VARIABLE || kind === LSP_KIND_FIELD /** Max instance/struct variables to drill into when a type-filtered search has no direct hits. */ const SCOPE_EXPAND_LIMIT = 8 @@ -92,7 +101,7 @@ export async function getScopeCompletions( const { anchor, segment } = splitExpression(value) const items = await api.completeInScope(pouName, anchor) const needle = segment.toLowerCase() - const matching = items.filter((item) => item.kind === LSP_KIND_VARIABLE && item.label.toLowerCase().includes(needle)) + const matching = items.filter((item) => isValueCompletionKind(item.kind) && item.label.toLowerCase().includes(needle)) const direct = matching .filter((item) => { @@ -120,7 +129,7 @@ export async function getScopeCompletions( const memberAnchor = `${anchor}${instance.label}.` const members = await api.completeInScope(pouName, memberAnchor) return members - .filter((m) => m.kind === LSP_KIND_VARIABLE && m.type && validateVariableType(m.type, expectedType).isValid) + .filter((m) => isValueCompletionKind(m.kind) && m.type && validateVariableType(m.type, expectedType).isValid) .map((m) => ({ label: `${instance.label}.${m.label}`, insertText: memberAnchor + m.label, type: m.type })) }), ) @@ -146,7 +155,9 @@ export async function resolveScopeExpressionType(pouName: string, expression: st if (items.length === 0) return { status: 'unavailable' } const { name, indexed } = stripSubscript(segment) - const match = items.find((item) => item.kind === LSP_KIND_VARIABLE && item.label.toLowerCase() === name.toLowerCase()) + const match = items.find( + (item) => isValueCompletionKind(item.kind) && item.label.toLowerCase() === name.toLowerCase(), + ) if (!match || !match.type) return { status: 'unknown' } if (indexed) {