From 72d128763e4ee5e9b6092cf49c9bdd48fee0869e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20GS=20Pereira?= Date: Fri, 7 Aug 2026 17:24:49 -0300 Subject: [PATCH 1/3] feat(data-types): wire the .dt code view into the ST language server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Go-to-definition on a user type now lands in that type's own code view with the cursor on the declaration — a struct field target lands on its field line — instead of merely opening the form tab (DOPE-537). The buffer gets a real identity (`inmemory://dtview/.dt`) so the LSP can recognise it, and `resolveStLspContext` remaps it onto the aggregate datatypes document. Both frames open with a `TYPE` line, so a single shift derived from the type's span covers completion, hover, signature help, definition, references and formatting at once. The span map is computed from the serializer rather than tracked in the offset registry: there is no registration lifecycle to keep in sync and no window where a datatype edit and the stored offset disagree. Semantic tokens and diagnostics need more than a shift, because the model's text and the document the answers come from are two different strings that only agree while the buffer is committed: - The token window holds the entry's own lines and is rebased onto the view's frame via `outputStartLine`. Widening the window instead would drag in the previous entry's last line, whose columns overrun the 4-character `TYPE` line and make Monaco reject the whole batch. - While the buffer diverges from the store the window is empty. No colours beats colours describing the previous text. - A store change re-drives both, since the model's text is untouched by it and Monaco would otherwise never re-query. - The diagnostics mirror caches each publish together with the spans it was computed against, and replays it when a model mounts later. Replaying through freshly computed spans puts markers on the wrong line, or drops them, once the store has moved. The go-to-definition cursor is deliberately keyed on its own identity and not on the current display: including the display would re-fire the forced switch when the user toggles back to the table and pin the tab in code mode. Refs DOPE-537 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GvGrhB1LyJz9MBwYhjoBDY --- .../__tests__/code-view-toggle.test.tsx | 38 ++++++ .../_features/[workspace]/data-type/index.tsx | 28 ++++- .../variables-code-editor/index.tsx | 12 +- .../__tests__/semantic-tokens-shift.test.ts | 40 +++++++ .../internal/semantic-tokens-shift.ts | 10 +- .../services/lsp-shared/semantic-tokens.ts | 8 +- .../lsp-shared/start-language-service.ts | 13 ++ .../goto-definition-redirect.test.ts | 84 ++++++++++++- .../services/st-lsp/__tests__/types.test.ts | 28 ++++- .../st-lsp/goto-definition-redirect.ts | 56 ++++++--- src/frontend/services/st-lsp/index.ts | 113 ++++++++++++++++++ src/frontend/services/st-lsp/types.ts | 26 ++++ src/frontend/store/slices/editor/types.ts | 4 +- .../__tests__/data-type-serializer.test.ts | 54 ++++++++- .../utils/PLC/data-type-serializer.ts | 27 +++++ 15 files changed, 515 insertions(+), 26 deletions(-) create mode 100644 src/frontend/components/_features/[workspace]/data-type/__tests__/code-view-toggle.test.tsx create mode 100644 src/frontend/services/lsp-shared/__tests__/semantic-tokens-shift.test.ts diff --git a/src/frontend/components/_features/[workspace]/data-type/__tests__/code-view-toggle.test.tsx b/src/frontend/components/_features/[workspace]/data-type/__tests__/code-view-toggle.test.tsx new file mode 100644 index 000000000..f6851127c --- /dev/null +++ b/src/frontend/components/_features/[workspace]/data-type/__tests__/code-view-toggle.test.tsx @@ -0,0 +1,38 @@ +import { fireEvent, render, screen } from '@testing-library/react' + +// The code view pulls in Monaco, which cannot run in jsdom. +vi.mock('@root/frontend/components/_organisms/variables-code-editor', () => ({ + VariablesCodeEditor: () =>
, +})) + +vi.mock('@root/frontend/utils/feature-flags', () => ({ + isDataTypeFilesEnabled: () => true, +})) + +import { useOpenPLCStore } from '@root/frontend/store' + +import { DataTypeEditor } from '../index' + +/** Reproduce what a go-to-definition redirect leaves behind. */ +function arriveFromGotoDefinition(name: string) { + const { editorActions } = useOpenPLCStore.getState() + editorActions.updateModelStructureForName(name, { display: 'code' }) + editorActions.setEditorCursor(name, { lineNumber: 2, column: 3, offset: 0, target: 'data-type' }) +} + +describe('DataTypeEditor code view toggle', () => { + it('lets the user switch back to the table after a goto-definition jump', () => { + const created = useOpenPLCStore.getState().datatypeActions.create({ name: 'Motor', derivation: 'structure' }) + expect(created.ok).toBe(true) + + arriveFromGotoDefinition('Motor') + render() + expect(screen.getByTestId('variables-code-editor')).toBeTruthy() + + fireEvent.click(screen.getByLabelText('Data type table visualization')) + + // The jump's cursor is still on the model; it must not re-assert code + // mode and pin the tab there. + expect(screen.queryByTestId('variables-code-editor')).toBeNull() + }) +}) diff --git a/src/frontend/components/_features/[workspace]/data-type/index.tsx b/src/frontend/components/_features/[workspace]/data-type/index.tsx index ae5a87ab9..2c6eabadf 100644 --- a/src/frontend/components/_features/[workspace]/data-type/index.tsx +++ b/src/frontend/components/_features/[workspace]/data-type/index.tsx @@ -1,9 +1,10 @@ -import { ComponentPropsWithoutRef, useEffect, useRef, useState } from 'react' +import { ComponentPropsWithoutRef, useEffect, useMemo, useRef, useState } from 'react' import type { PLCDataType } from '../../../../../middleware/shared/ports/types' import { CodeIcon } from '../../../../assets/icons/interface/CodeIcon' import { TableIcon } from '../../../../assets/icons/interface/TableIcon' import { usePouSnapshot } from '../../../../hooks/use-pou-snapshot' +import { dtViewUri } from '../../../../services/st-lsp/types' import { useOpenPLCStore } from '../../../../store' import { extractSearchQuery } from '../../../../store/slices/search/utils' import { cn } from '../../../../utils/cn' @@ -152,6 +153,29 @@ const DataTypeEditor = ({ dataTypeName, ...rest }: DatatypeEditorProps) => { commitCodeRef.current = commitCode }) + // Stable reference, or the child's cursor-jump effect re-fires every + // keystroke and re-selects the navigated line. + const codeCursorPosition = useMemo( + () => + model?.cursorPosition?.target === 'data-type' + ? { + lineNumber: model.cursorPosition.lineNumber, + column: model.cursorPosition.column, + target: 'data-type' as const, + } + : undefined, + [model?.cursorPosition?.target, model?.cursorPosition?.lineNumber, model?.cursorPosition?.column], + ) + + // Goto-definition can land here while the tab is still in table mode. + // Keyed on the cursor alone: including `display` would re-fire on the + // user's own switch back to table and pin the tab in code mode. + useEffect(() => { + if (!codeCursorPosition || display === 'code') return + updateModelStructureForName(dataTypeName, { display: 'code', code: editorCode }) + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [codeCursorPosition]) + useEffect(() => { if (display !== 'code') return @@ -327,6 +351,8 @@ const DataTypeEditor = ({ dataTypeName, ...rest }: DatatypeEditorProps) => { code={editorCode} onCodeChange={setEditorCode} shouldUseDarkMode={shouldUseDarkMode} + modelUri={dtViewUri(dataTypeName)} + cursorPosition={codeCursorPosition} />
{parseError &&

Error: {parseError}

} diff --git a/src/frontend/components/_organisms/variables-code-editor/index.tsx b/src/frontend/components/_organisms/variables-code-editor/index.tsx index 944cea274..c2d1daed6 100644 --- a/src/frontend/components/_organisms/variables-code-editor/index.tsx +++ b/src/frontend/components/_organisms/variables-code-editor/index.tsx @@ -41,6 +41,12 @@ interface VariablesCodeEditorProps { * cross-routing. */ pouName?: string + /** + * Explicit Monaco model URI, for surfaces that route to the LSP but + * aren't POU variables (the data type `.dt` code view). Takes + * precedence over the URI derived from `pouName`. + */ + modelUri?: string /** * Programmatic cursor jump (e.g. compile-error click → vars-text * view, or Go to Definition redirect for a variable declaration). @@ -54,7 +60,7 @@ interface VariablesCodeEditorProps { * (it's meant for the body editor). Undefined or * `target === 'variables'` is honoured here. */ - cursorPosition?: { lineNumber: number; column: number; target?: 'body' | 'variables' } + cursorPosition?: { lineNumber: number; column: number; target?: 'body' | 'variables' | 'data-type' } } const VariablesCodeEditor = ({ @@ -63,9 +69,11 @@ const VariablesCodeEditor = ({ shouldUseDarkMode, cursorPosition, pouName, + modelUri, language = 'st', readOnly = false, }: VariablesCodeEditorProps) => { + const resolvedModelUri = modelUri ?? (pouName ? pouVarsUri(pouName) : undefined) const editorRef = useRef(null) const containerRef = useRef(null) const [editorMounted, setEditorMounted] = useState(false) @@ -146,7 +154,7 @@ const VariablesCodeEditor = ({ height='100%' width='100%' language={language} - {...(pouName ? { path: pouVarsUri(pouName) } : {})} + {...(resolvedModelUri ? { path: resolvedModelUri } : {})} defaultValue={''} value={code} onMount={handleEditorDidMount} diff --git a/src/frontend/services/lsp-shared/__tests__/semantic-tokens-shift.test.ts b/src/frontend/services/lsp-shared/__tests__/semantic-tokens-shift.test.ts new file mode 100644 index 000000000..447baa6bc --- /dev/null +++ b/src/frontend/services/lsp-shared/__tests__/semantic-tokens-shift.test.ts @@ -0,0 +1,40 @@ +/** + * @jest-environment jsdom + */ +import { shiftSemanticTokensToBody } from '../internal/semantic-tokens-shift' + +// One token per line of a synthesised datatypes document: +// 0 `TYPE` col 0 +// 1 ` Colors : (RED);` col 2 +// 2 ` Motor : STRUCT` col 2 +// 3 ` speed : INT;` col 4 +// 4 ` END_STRUCT;` col 2 +const AGGREGATE = [0, 0, 4, 0, 0, 1, 2, 6, 0, 0, 1, 2, 5, 0, 0, 1, 4, 5, 0, 0, 1, 2, 10, 0, 0] + +describe('shiftSemanticTokensToBody', () => { + it('drops tokens before the window and rebases the rest to line 0 by default', () => { + expect(Array.from(shiftSemanticTokensToBody(AGGREGATE, 1, 2))).toEqual([0, 2, 6, 0, 0]) + }) + + it('rebases onto outputStartLine so a view can render its own frame above the window', () => { + // `Motor` occupies lines 2..4; its `.dt` view renders them under a + // local `TYPE` line, so they land on local lines 1..3. + expect(Array.from(shiftSemanticTokensToBody(AGGREGATE, 2, 5, 1))).toEqual([ + 1, 2, 5, 0, 0, 1, 4, 5, 0, 0, 1, 2, 10, 0, 0, + ]) + }) + + it('never emits the line above the window, whose columns can overrun a short frame line', () => { + const out = Array.from(shiftSemanticTokensToBody(AGGREGATE, 2, 5, 1)) + // The `Colors` token (length 6) would not fit on the 4-character `TYPE` line. + expect(out).not.toContain(6) + }) + + it('keeps everything from startLine when no end is given', () => { + expect(Array.from(shiftSemanticTokensToBody(AGGREGATE, 3))).toEqual([0, 4, 5, 0, 0, 1, 2, 10, 0, 0]) + }) + + it('returns an empty stream when the window selects nothing', () => { + expect(Array.from(shiftSemanticTokensToBody(AGGREGATE, 9, 12))).toEqual([]) + }) +}) diff --git a/src/frontend/services/lsp-shared/internal/semantic-tokens-shift.ts b/src/frontend/services/lsp-shared/internal/semantic-tokens-shift.ts index da257c4ca..2b4e042e3 100644 --- a/src/frontend/services/lsp-shared/internal/semantic-tokens-shift.ts +++ b/src/frontend/services/lsp-shared/internal/semantic-tokens-shift.ts @@ -7,10 +7,15 @@ * 1. Decode deltas to absolute (line, col) positions. * 2. Keep tokens whose line is in `[startLine, endLineExclusive)`. * Both ends are LSP coordinates. - * 3. Subtract `startLine` from each surviving token's line so the + * 3. Rebase each surviving token onto `outputStartLine` so the * output is Monaco-relative. * 4. Re-encode as a delta stream Monaco can consume directly. * + * `outputStartLine` exists so a view that renders its own framing + * above the window doesn't have to widen the window to compensate: + * doing that drags in the preceding line's tokens, whose columns can + * overrun the shorter frame line ("end character > line length"). + * * Used in two modes by the ST LSP today: * - **Body view**: `startLine = bodyLineOffset` (preamble line count), * `endLineExclusive = ∞` — drop the preamble, keep everything @@ -29,6 +34,7 @@ export function shiftSemanticTokensToBody( data: number[], startLine: number, endLineExclusive: number = Number.POSITIVE_INFINITY, + outputStartLine: number = 0, ): Uint32Array { // Decode to absolute positions. const abs: Array<{ line: number; col: number; len: number; type: number; mods: number }> = [] @@ -51,7 +57,7 @@ export function shiftSemanticTokensToBody( for (const t of abs) { if (t.line < startLine) continue if (t.line >= endLineExclusive) continue - const shiftedLine = t.line - startLine + const shiftedLine = t.line - startLine + outputStartLine const dLine = shiftedLine - prevLine const dStart = dLine === 0 ? t.col - prevCol : t.col out.push(dLine, dStart, t.len, t.type, t.mods) diff --git a/src/frontend/services/lsp-shared/semantic-tokens.ts b/src/frontend/services/lsp-shared/semantic-tokens.ts index 987d13452..9690ff17a 100644 --- a/src/frontend/services/lsp-shared/semantic-tokens.ts +++ b/src/frontend/services/lsp-shared/semantic-tokens.ts @@ -39,12 +39,14 @@ export interface SemanticTokensRegistration extends monaco.IDisposable { * default keeps everything from `[lineOffset, +∞)` — drop the * preamble, keep the rest. ST's variables-text view overrides this * to clip the end at the body line so only VAR-block tokens render. + * `outputStartLine` rebases the kept window for views that render + * their own framing above it (the data type `.dt` code view). */ export type ResolveSemanticTokensViewport = ( lspUri: string, modelUri: string, lineOffset: number, -) => { startLine: number; endLineExclusive: number } +) => { startLine: number; endLineExclusive: number; outputStartLine?: number } const defaultViewport: ResolveSemanticTokensViewport = (_lspUri, _modelUri, lineOffset) => ({ startLine: lineOffset, @@ -87,10 +89,10 @@ export function registerLspSemanticTokens(opts: RegisterLspSemanticTokensOptions textDocument: { uri: lspUri }, }) if (!result) return null - const { startLine, endLineExclusive } = resolveViewport(lspUri, modelUri, lineOffset) + const { startLine, endLineExclusive, outputStartLine } = resolveViewport(lspUri, modelUri, lineOffset) return { ...(result.resultId ? { resultId: result.resultId } : {}), - data: shiftSemanticTokensToBody(result.data, startLine, endLineExclusive), + data: shiftSemanticTokensToBody(result.data, startLine, endLineExclusive, outputStartLine), } }, releaseDocumentSemanticTokens() { diff --git a/src/frontend/services/lsp-shared/start-language-service.ts b/src/frontend/services/lsp-shared/start-language-service.ts index 66af71bf7..5d7a5ecd7 100644 --- a/src/frontend/services/lsp-shared/start-language-service.ts +++ b/src/frontend/services/lsp-shared/start-language-service.ts @@ -84,6 +84,14 @@ export interface LanguageService { changeDocument(uri: string, content: string, version?: number): void /** Send `textDocument/didClose`. */ closeDocument(uri: string): void + /** + * Ask Monaco to re-query semantic tokens for every model in this + * language. Needed when a model's tokens derive from a *different* + * document (the datatype `.dt` view reads the aggregate doc), where + * a change to that document leaves the model's text untouched and + * therefore triggers no re-query of its own. + */ + refreshSemanticTokens(): void /** Tear down providers + transport. */ dispose(): void } @@ -232,6 +240,7 @@ export function startLanguageService(opts: StartLanguageServiceOptions): Languag openDocument: () => undefined, changeDocument: () => undefined, closeDocument: () => undefined, + refreshSemanticTokens: () => undefined, dispose: () => undefined, } } @@ -333,6 +342,10 @@ export function startLanguageService(opts: StartLanguageServiceOptions): Languag return { ready, + refreshSemanticTokens() { + semanticTokensRegistration?.refresh() + }, + openDocument(uri, content) { if (disposed) return const existing = documents.get(uri) diff --git a/src/frontend/services/st-lsp/__tests__/goto-definition-redirect.test.ts b/src/frontend/services/st-lsp/__tests__/goto-definition-redirect.test.ts index 4d06cbd12..0dd2b0aa5 100644 --- a/src/frontend/services/st-lsp/__tests__/goto-definition-redirect.test.ts +++ b/src/frontend/services/st-lsp/__tests__/goto-definition-redirect.test.ts @@ -1,8 +1,9 @@ /** * @jest-environment jsdom */ -import type { PLCPou } from '../../../../middleware/shared/ports/types' +import type { PLCDataType, PLCPou } from '../../../../middleware/shared/ports/types' import { openPLCStoreBase } from '../../../store' +import * as featureFlags from '../../../utils/feature-flags' import { setBodyLineOffset } from '../../lsp-shared/body-offsets' import { redirectDefinitionToStore } from '../goto-definition-redirect' @@ -62,6 +63,87 @@ describe('redirectDefinitionToStore', () => { ).toBe(false) }) + describe('datatypes URI with data types present', () => { + // Aggregate doc: line 0 `TYPE`, 1 `Colors : (...)`, then Motor's + // STRUCT spans 2..4 (declaration, one field, END_STRUCT). + const dataTypes: PLCDataType[] = [ + { name: 'Colors', derivation: 'enumerated', values: [{ description: 'RED' }], initialValue: '' }, + { + name: 'Motor', + derivation: 'structure', + variable: [{ name: 'speed', type: { definition: 'base-type', value: 'INT' } }], + }, + ] + + beforeEach(() => { + setProjectPous([]) + openPLCStoreBase.setState((s) => ({ + ...s, + project: { ...s.project, data: { ...s.project.data, dataTypes } }, + })) + jest.spyOn(featureFlags, 'isDataTypeFilesEnabled').mockReturnValue(true) + }) + + afterEach(() => { + jest.restoreAllMocks() + }) + + it('opens the owning type in code mode with the cursor on its declaration line', () => { + expect( + redirectDefinitionToStore({ + uri: 'inmemory://datatypes/__project__.st', + range: { start: { line: 1, character: 2 }, end: { line: 1, character: 8 } }, + }), + ).toBe(true) + + const state = openPLCStoreBase.getState() + expect(state.selectedTab).toBe('Colors') + // The active editor holds the fresh model — `updateModelStructureForName` + // writes there when the name matches, leaving `editors[]` behind. + const model = state.editor + expect(model.type === 'plc-datatype' && model.structure.display).toBe('code') + // Entry line 0 sits below the view's own `TYPE` frame → Monaco line 2. + expect(model.cursorPosition).toEqual({ lineNumber: 2, column: 3, offset: 0, target: 'data-type' }) + }) + + it('lands on a struct field line inside the owning type', () => { + // Aggregate line 3 = Motor's `speed` field (entry starts at 2). + expect( + redirectDefinitionToStore({ + uri: 'inmemory://datatypes/__project__.st', + range: { start: { line: 3, character: 4 }, end: { line: 3, character: 9 } }, + }), + ).toBe(true) + + const state = openPLCStoreBase.getState() + expect(state.selectedTab).toBe('Motor') + expect(state.editor.cursorPosition?.lineNumber).toBe(3) + }) + + it('returns false for the END_TYPE framing line past the last entry', () => { + expect( + redirectDefinitionToStore({ + uri: 'inmemory://datatypes/__project__.st', + range: { start: { line: 5, character: 0 }, end: { line: 5, character: 0 } }, + }), + ).toBe(false) + }) + + it('opens the form tab without a cursor when the code view is not built in', () => { + jest.spyOn(featureFlags, 'isDataTypeFilesEnabled').mockReturnValue(false) + expect( + redirectDefinitionToStore({ + uri: 'inmemory://datatypes/__project__.st', + range: { start: { line: 1, character: 0 }, end: { line: 1, character: 0 } }, + }), + ).toBe(true) + + const model = openPLCStoreBase.getState().editor + expect(model.type === 'plc-datatype' && model.structure.display).toBe('table') + expect(model.cursorPosition).toBeUndefined() + }) + }) + it('returns false when the target POU does not exist in the project', () => { setProjectPous([]) expect( diff --git a/src/frontend/services/st-lsp/__tests__/types.test.ts b/src/frontend/services/st-lsp/__tests__/types.test.ts index 63ad17e47..65df02511 100644 --- a/src/frontend/services/st-lsp/__tests__/types.test.ts +++ b/src/frontend/services/st-lsp/__tests__/types.test.ts @@ -1,4 +1,13 @@ -import { parsePouUri, parsePouVarsUri, POU_DECLARATION_LINE_COUNT, pouUri, pouVarsUri, stubUri } from '../types' +import { + dtViewUri, + parseDtViewUri, + parsePouUri, + parsePouVarsUri, + POU_DECLARATION_LINE_COUNT, + pouUri, + pouVarsUri, + stubUri, +} from '../types' describe('pouUri / stubUri', () => { it('produces well-formed in-memory URIs', () => { @@ -71,3 +80,20 @@ describe('POU_DECLARATION_LINE_COUNT', () => { expect(POU_DECLARATION_LINE_COUNT).toBe(1) }) }) + +describe('dtViewUri / parseDtViewUri', () => { + it('round-trips a data type name, encoding included', () => { + expect(dtViewUri('Motor')).toBe('inmemory://dtview/Motor.dt') + expect(parseDtViewUri(dtViewUri('My Type'))).toBe('My Type') + }) + + it('returns null for the other ST URI shapes', () => { + expect(parseDtViewUri(pouUri('Motor'))).toBeNull() + expect(parseDtViewUri(pouVarsUri('Motor'))).toBeNull() + expect(parseDtViewUri('inmemory://datatypes/__project__.st')).toBeNull() + }) + + it('does not collide with the pouvars parser', () => { + expect(parsePouVarsUri(dtViewUri('Motor'))).toBeNull() + }) +}) diff --git a/src/frontend/services/st-lsp/goto-definition-redirect.ts b/src/frontend/services/st-lsp/goto-definition-redirect.ts index 7697f2e9e..d778456df 100644 --- a/src/frontend/services/st-lsp/goto-definition-redirect.ts +++ b/src/frontend/services/st-lsp/goto-definition-redirect.ts @@ -39,10 +39,17 @@ import type { PLCDataType } from '../../../middleware/shared/ports/types' import { sanitizeAxisName, softMotionAxisNames } from '../../../middleware/shared/utils/ethercat' import { openPLCStoreBase } from '../../store' import { CreateEditorObjectFromTab } from '../../store/slices/tabs/utils' -import { serializeDataTypesToLines } from '../../utils/PLC/data-type-serializer' +import { isDataTypeFilesEnabled } from '../../utils/feature-flags' +import { dataTypeLineSpans } from '../../utils/PLC/data-type-serializer' import { getBodyLineOffset } from '../lsp-shared/body-offsets' import { normaliseLocation, routeToPou, routeToPouBody, routeToPouPreamble } from '../lsp-shared/definition-redirect' -import { DATA_TYPES_URI, parsePouUri, RESOURCE_GLOBALS_URI, SOFTMOTION_GLOBALS_URI } from './types' +import { + DATA_TYPES_URI, + DT_VIEW_FRAME_LINE_COUNT, + parsePouUri, + RESOURCE_GLOBALS_URI, + SOFTMOTION_GLOBALS_URI, +} from './types' /** * Map an LSP line in the synthesised datatypes document to the @@ -56,18 +63,18 @@ import { DATA_TYPES_URI, parsePouUri, RESOURCE_GLOBALS_URI, SOFTMOTION_GLOBALS_U * line counts that drift the moment a new field separator or * derivation lands on disk. */ -function findDataTypeAtLine(lspLine: number, dataTypes: PLCDataType[]): PLCDataType | null { +function findDataTypeAtLine( + lspLine: number, + dataTypes: PLCDataType[], +): { dataType: PLCDataType; lineInEntry: number } | null { // Synthesised doc: line 0 is `TYPE`, entries start at line 1. if (lspLine < 1) return null - const entries = serializeDataTypesToLines(dataTypes) const byName = new Map(dataTypes.map((dt) => [dt.name, dt])) - let cursor = 1 - for (const entry of entries) { - const span = entry.lines.length - if (lspLine >= cursor && lspLine < cursor + span) { - return byName.get(entry.name) ?? null + for (const [name, span] of dataTypeLineSpans(dataTypes)) { + if (lspLine >= span.start && lspLine < span.start + span.length) { + const dataType = byName.get(name) + return dataType ? { dataType, lineInEntry: lspLine - span.start } : null } - cursor += span } return null } @@ -103,6 +110,28 @@ function openDataTypeEditor(dataType: PLCDataType): boolean { return true } +/** + * Open the type's tab in code mode with the cursor on `lineInEntry` + * (0 = its declaration line). Falls back to the form tab when the + * `.dt` code view isn't built into this release. + */ +function routeToDataTypeCodeView(dataType: PLCDataType, lineInEntry: number, characterLsp: number): boolean { + if (!openDataTypeEditor(dataType)) return false + if (!isDataTypeFilesEnabled()) return true + const { + editorActions: { setEditorCursor, updateModelStructureForName }, + } = openPLCStoreBase.getState() + updateModelStructureForName(dataType.name, { display: 'code' }) + setEditorCursor(dataType.name, { + // The view renders its own `TYPE` frame line before the entry. + lineNumber: lineInEntry + DT_VIEW_FRAME_LINE_COUNT + 1, + column: Math.max(1, characterLsp + 1), + offset: 0, + target: 'data-type', + }) + return true +} + /** * Open the EtherCAT device (drive) editor for `deviceId` on `busName`, mirroring * the project-tree click path. Used to redirect go-to-definition on a SoftMotion @@ -206,10 +235,9 @@ export function redirectDefinitionToStore(loc: Location | LocationLink): boolean // branch the redirect would dead-end silently. if (target.uri === DATA_TYPES_URI) { const dataTypes = openPLCStoreBase.getState().project.data.dataTypes - const dt = findDataTypeAtLine(target.lineLsp, dataTypes) - if (!dt) return false - openDataTypeEditor(dt) - return true + const hit = findDataTypeAtLine(target.lineLsp, dataTypes) + if (!hit) return false + return routeToDataTypeCodeView(hit.dataType, hit.lineInEntry, target.characterLsp) } const parsed = parsePouUri(target.uri) diff --git a/src/frontend/services/st-lsp/index.ts b/src/frontend/services/st-lsp/index.ts index dd74eb89f..d8a4e2bb3 100644 --- a/src/frontend/services/st-lsp/index.ts +++ b/src/frontend/services/st-lsp/index.ts @@ -25,6 +25,7 @@ * disposed only at shutdown. */ +import type * as monaco from 'monaco-editor' import { type CompletionItem as LspCompletionItem, type CompletionList, @@ -35,6 +36,7 @@ import { } from 'vscode-languageserver-protocol' import { openPLCStoreBase } from '../../store' +import { dataTypeLineSpans, serializeDataTypeToText } from '../../utils/PLC/data-type-serializer' import { serializePouScopeForQuery } from '../../utils/PLC/pou-signature-serializer' import { getBodyLineOffset, @@ -49,6 +51,10 @@ import { redirectDefinitionToStore } from './goto-definition-redirect' import { redirectToGraphicalPou } from './graphical-redirect' import { registerScopedQueryApi, type ScopedCompletionItem } from './scoped-query' import { + DATA_TYPES_URI, + DT_VIEW_FRAME_LINE_COUNT, + dtViewUri, + parseDtViewUri, parsePouUri, parsePouVarsUri, POU_DECLARATION_LINE_COUNT, @@ -85,6 +91,9 @@ interface LoadStlibBufferParams { * that's `pou://`; for graphical/hybrid POUs it's `stub://`. * Either way the declaration is a single line at LSP index 0, * so the offset is a constant 1. + * - `dtview://.dt` (per-type code view): remap to the + * aggregate datatypes document. Both frames open with a `TYPE` + * line, so the shift is the type's span start minus that frame. * - Anything else: pass through unchanged. */ function resolveStLspContext(modelUri: string): LspContext { @@ -95,9 +104,64 @@ function resolveStLspContext(modelUri: string): LspContext { const lspUri = isStLanguage ? pouUri(varsPou) : stubUri(varsPou) return { lspUri, lineOffset: POU_DECLARATION_LINE_COUNT } } + const dtName = parseDtViewUri(modelUri) + if (dtName !== null) { + const span = dataTypeLineSpans(openPLCStoreBase.getState().project.data.dataTypes).get(dtName) + // An unknown name (type deleted while its tab is open) keeps the + // frame shift so requests stay inside the document. + return { lspUri: DATA_TYPES_URI, lineOffset: (span?.start ?? 1) - DT_VIEW_FRAME_LINE_COUNT } + } return { lspUri: modelUri, lineOffset: getBodyLineOffset(modelUri) } } +/** + * Aggregate-document line window backing a `.dt` code view, or null. + * Holds the entry's own lines only — widening it to cover the view's + * `TYPE` frame would pull in the previous entry's last line, whose + * columns overrun that 4-character frame line. + */ +function dtViewWindow(modelUri: string): { start: number; endExclusive: number } | null { + const dtName = parseDtViewUri(modelUri) + if (dtName === null) return null + const span = dataTypeLineSpans(openPLCStoreBase.getState().project.data.dataTypes).get(dtName) + if (!span) return null + return { start: span.start, endExclusive: span.start + span.length } +} + +/** + * True while a `.dt` model's text still matches what the store would + * serialise for that type. An uncommitted edit breaks the match, and + * tokens resolved against the aggregate document would then be painted + * onto text they don't describe — wrong colours, and columns past the + * end of shorter lines. + */ +function dtViewMatchesStore(dtName: string, monacoApi: typeof monaco): boolean { + const dataType = openPLCStoreBase.getState().project.data.dataTypes.find((d) => d.name === dtName) + if (!dataType) return false + const model = monacoApi.editor.getModels().find((m) => m.uri.toString() === dtViewUri(dtName)) + if (!model) return false + return model.getValue() === serializeDataTypeToText(dataType) +} + +let lastDataTypeDiagnostics: Diagnostic[] = [] + +/** Fan the aggregate doc's diagnostics out to every mounted `.dt` model. */ +function applyDataTypeDiagnostics(monacoApi: typeof monaco, markerOwner: string, defaultSource: string): void { + for (const [name, span] of dataTypeLineSpans(openPLCStoreBase.getState().project.data.dataTypes)) { + const model = monacoApi.editor.getModels().find((m) => m.uri.toString() === dtViewUri(name)) + if (!model) continue + const shift = span.start - DT_VIEW_FRAME_LINE_COUNT + const owned = lastDataTypeDiagnostics.filter( + (d) => d.range.start.line >= span.start && d.range.start.line < span.start + span.length, + ) + monacoApi.editor.setModelMarkers( + model, + markerOwner, + owned.map((d) => lspDiagnosticToMonaco(d, monacoApi, shift, defaultSource)), + ) + } +} + export function startStLsp(opts: StLspStartOptions): StLspService { const { stlibSource, monaco: monacoApi, workerUrlOverride, onCrash } = opts @@ -151,6 +215,20 @@ export function startStLsp(opts: StLspStartOptions): StLspService { // VAR-block region; body editors keep everything from the // body line onwards. resolveSemanticTokensViewport: (lspUri, modelUri, lineOffset) => { + const dtName = parseDtViewUri(modelUri) + if (dtName !== null) { + const dtWindow = dtViewWindow(modelUri) + // Empty window while the buffer is uncommitted: no colours beats + // colours describing the previous text. + if (!dtWindow || !monacoApi || !dtViewMatchesStore(dtName, monacoApi)) { + return { startLine: 0, endLineExclusive: 0 } + } + return { + startLine: dtWindow.start, + endLineExclusive: dtWindow.endExclusive, + outputStartLine: DT_VIEW_FRAME_LINE_COUNT, + } + } const isVarsView = parsePouVarsUri(modelUri) !== null return { startLine: lineOffset, @@ -162,6 +240,16 @@ export function startStLsp(opts: StLspStartOptions): StLspService { markerOwner: MARKER_OWNER, diagnosticSource: DIAGNOSTIC_SOURCE, diagnosticsMirror: (params, ctx) => { + // Same trick for the aggregate datatypes doc: strucpp publishes + // against one URI, but each type renders in its own `.dt` view. + if (params.uri === DATA_TYPES_URI) { + // Replayed whenever a `.dt` model mounts later — the mirror is + // event-driven, so a model created after the last publish would + // otherwise show no markers at all. + lastDataTypeDiagnostics = params.diagnostics + applyDataTypeDiagnostics(ctx.monacoApi, ctx.markerOwner, ctx.defaultSource) + return + } // Mirror VAR-block diagnostics onto the variables-text editor // for the same POU (if mounted). The variables editor uses a // separate Monaco model under `pouvars://.st`; strucpp @@ -194,6 +282,28 @@ export function startStLsp(opts: StLspStartOptions): StLspService { ...(onCrash ? { onCrash } : {}), }) + // A `.dt` view's colours and markers come from the aggregate document, + // so a change there leaves the model's own text untouched and Monaco + // never re-queries on its own. Re-drive both from the store instead. + const dtViewSyncDisposables: Array<() => void> = [] + if (monacoApi) { + const api = monacoApi + dtViewSyncDisposables.push( + openPLCStoreBase.subscribe( + (state) => state.project.data.dataTypes, + () => { + sharedService.refreshSemanticTokens() + applyDataTypeDiagnostics(api, MARKER_OWNER, DIAGNOSTIC_SOURCE) + }, + ), + ) + const onModelAdded = api.editor.onDidCreateModel((model) => { + if (parseDtViewUri(model.uri.toString()) === null) return + applyDataTypeDiagnostics(api, MARKER_OWNER, DIAGNOSTIC_SOURCE) + }) + dtViewSyncDisposables.push(() => onModelAdded.dispose()) + } + // --------------------------------------------------------------------------- // Scoped completion for the graphical (LD/FBD) editors. // @@ -435,6 +545,9 @@ export function startStLsp(opts: StLspStartOptions): StLspService { dispose() { registerScopedQueryApi(null) + for (const off of dtViewSyncDisposables) off() + dtViewSyncDisposables.length = 0 + lastDataTypeDiagnostics = [] sharedService.dispose() }, } diff --git a/src/frontend/services/st-lsp/types.ts b/src/frontend/services/st-lsp/types.ts index ee506b6d5..647804cf7 100644 --- a/src/frontend/services/st-lsp/types.ts +++ b/src/frontend/services/st-lsp/types.ts @@ -31,6 +31,13 @@ export const STUB_URI_AUTHORITY = 'stub' */ export const POUVARS_URI_AUTHORITY = 'pouvars' +/** + * URI scheme for the per-type `.dt` code view. Like `pouvars://`, the + * LSP never indexes it — requests remap onto `DATA_TYPES_URI` with the + * type's line span as the offset. + */ +export const DTVIEW_URI_AUTHORITY = 'dtview' + /** * URI for the synthesized `TYPE…END_TYPE` document carrying every * user-defined `PLCDataType` (structures, enumerations, arrays). @@ -142,6 +149,18 @@ export function parsePouVarsUri(uri: string): string | null { return decodeURIComponent(match[1]) } +/** Make a synthetic in-memory URI for a data type's `.dt` code view. */ +export function dtViewUri(name: string): string { + return `${POU_URI_SCHEME}://${DTVIEW_URI_AUTHORITY}/${encodeURIComponent(name)}.dt` +} + +/** If `uri` is a `dtview://` URI, return the data type name; otherwise null. */ +export function parseDtViewUri(uri: string): string | null { + const match = new RegExp(`^${POU_URI_SCHEME}://${DTVIEW_URI_AUTHORITY}/(.+)\\.dt$`).exec(uri) + if (!match) return null + return decodeURIComponent(match[1]) +} + /** * Number of lines the synthesized declaration occupies before the * VAR blocks. Currently always 1 ("PROGRAM main", "FUNCTION foo : @@ -151,6 +170,13 @@ export function parsePouVarsUri(uri: string): string | null { */ export const POU_DECLARATION_LINE_COUNT = 1 +/** + * Lines the `.dt` code view renders before the type's own declaration — + * its local `TYPE` frame line. The aggregate document has the same + * frame, so a type's shift between the two is `span.start - DT_VIEW_FRAME_LINE_COUNT`. + */ +export const DT_VIEW_FRAME_LINE_COUNT = 1 + /** * Returns the POU name encoded in a URI minted by `pouUri` or * `stubUri`, or `null` if the URI doesn't match one of those diff --git a/src/frontend/store/slices/editor/types.ts b/src/frontend/store/slices/editor/types.ts index 39786825c..7857366bd 100644 --- a/src/frontend/store/slices/editor/types.ts +++ b/src/frontend/store/slices/editor/types.ts @@ -76,13 +76,15 @@ export type CursorPosition = { * editor. Triggers a forced switch to text mode if the panel * is currently in table mode, and the body editor ignores * positions tagged this way. + * - `data-type` — targets a data type's `.dt` code view, with the + * same forced switch out of table mode. * * Used by Go to Definition redirects: when the LSP points at a * variable declaration (synthesized header line), we surface that * line inside the variables panel instead of clamping the cursor * to the body's line 1. */ - target?: 'body' | 'variables' + target?: 'body' | 'variables' | 'data-type' } // --------------------------------------------------------------------------- diff --git a/src/frontend/utils/PLC/__tests__/data-type-serializer.test.ts b/src/frontend/utils/PLC/__tests__/data-type-serializer.test.ts index 2010918ed..898e8f847 100644 --- a/src/frontend/utils/PLC/__tests__/data-type-serializer.test.ts +++ b/src/frontend/utils/PLC/__tests__/data-type-serializer.test.ts @@ -11,7 +11,12 @@ * one of these up. */ import type { PLCDataType } from '../../../../middleware/shared/ports/types' -import { serializeDataTypesToLines, serializeDataTypesToST, serializeDataTypeToText } from '../data-type-serializer' +import { + dataTypeLineSpans, + serializeDataTypesToLines, + serializeDataTypesToST, + serializeDataTypeToText, +} from '../data-type-serializer' const enumerated = (name: string, values: string[], initialValue?: string): PLCDataType => ({ name, @@ -255,3 +260,50 @@ describe('serializeDataTypesToLines', () => { expect(reconstructed).toBe(flat) }) }) + +describe('dataTypeLineSpans', () => { + it('places entries after the TYPE frame and accumulates multi-line spans', () => { + const spans = dataTypeLineSpans([ + { name: 'Colors', derivation: 'enumerated', values: [{ description: 'RED' }], initialValue: '' }, + { + name: 'Motor', + derivation: 'structure', + variable: [ + { name: 'speed', type: { definition: 'base-type', value: 'INT' } }, + { name: 'torque', type: { definition: 'base-type', value: 'INT' } }, + ], + }, + { + name: 'Buffer', + derivation: 'array', + dimensions: [{ dimension: '0..9' }], + baseType: { definition: 'base-type', value: 'INT' }, + initialValue: '', + }, + ]) + + // Line 0 is `TYPE`; the struct occupies declaration + 2 fields + END_STRUCT. + expect(spans.get('Colors')).toEqual({ start: 1, length: 1 }) + expect(spans.get('Motor')).toEqual({ start: 2, length: 4 }) + expect(spans.get('Buffer')).toEqual({ start: 6, length: 1 }) + }) + + it('agrees with the rendered aggregate document', () => { + const dataTypes: PLCDataType[] = [ + { name: 'Colors', derivation: 'enumerated', values: [{ description: 'RED' }], initialValue: '' }, + { + name: 'Motor', + derivation: 'structure', + variable: [{ name: 'speed', type: { definition: 'base-type', value: 'INT' } }], + }, + ] + const lines = serializeDataTypesToST(dataTypes).split('\n') + for (const [name, span] of dataTypeLineSpans(dataTypes)) { + expect(lines[span.start]).toContain(name) + } + }) + + it('is empty for no data types', () => { + expect(dataTypeLineSpans([]).size).toBe(0) + }) +}) diff --git a/src/frontend/utils/PLC/data-type-serializer.ts b/src/frontend/utils/PLC/data-type-serializer.ts index 26b7c1170..c9ece49c0 100644 --- a/src/frontend/utils/PLC/data-type-serializer.ts +++ b/src/frontend/utils/PLC/data-type-serializer.ts @@ -111,6 +111,33 @@ export function serializeDataTypesToLines(dataTypes: PLCDataType[]): SerializedD return out } +/** Where one data type's lines sit inside the aggregate `TYPE…END_TYPE` block. */ +export interface DataTypeLineSpan { + /** 0-indexed first line of the entry in the aggregate document. */ + start: number + /** Line count of the entry. */ + length: number +} + +/** + * Line spans of every entry in the aggregate document, keyed by name. + * Line 0 is the `TYPE` frame, so entries start at 1. + * + * The per-type `.dt` code view renders the same lines under its own + * `TYPE…END_TYPE` frame, so `start - 1` is the shift between the two + * frames — that is what the LSP layer needs to talk to the aggregate + * document on a per-type buffer's behalf. + */ +export function dataTypeLineSpans(dataTypes: PLCDataType[]): Map { + const spans = new Map() + let start = 1 + for (const entry of serializeDataTypesToLines(dataTypes)) { + spans.set(entry.name, { start, length: entry.lines.length }) + start += entry.lines.length + } + return spans +} + /** * Serialise every entry in `dataTypes` to a single ST `TYPE` block. * Returns `''` when there's nothing to emit — the LSP sync layer From 528c0468c92284c311e7ee8035a1f917bf4a4f7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20GS=20Pereira?= Date: Fri, 7 Aug 2026 17:39:20 -0300 Subject: [PATCH 2/3] fix(st-lsp): stop a malformed synthetic URI from throwing out of the parsers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `parsePouUri`, `parsePouVarsUri` and `parseDtViewUri` decoded their name segment with a bare `decodeURIComponent`, which raises `URIError` on input like `%ZZ`. All three run inside `resolveStLspContext`, on every model URI the providers see, so one malformed URI would take hover, completion and definition down for that model rather than simply not matching. Not reachable today — these URIs are only minted by the matching builders, which encode the name — but the guard belongs in all three rather than in whichever one was touched last. Refs DOPE-537 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GvGrhB1LyJz9MBwYhjoBDY --- .../services/st-lsp/__tests__/types.test.ts | 17 ++++++++++++ src/frontend/services/st-lsp/types.ts | 26 ++++++++++++++----- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/frontend/services/st-lsp/__tests__/types.test.ts b/src/frontend/services/st-lsp/__tests__/types.test.ts index 65df02511..597dd5fdd 100644 --- a/src/frontend/services/st-lsp/__tests__/types.test.ts +++ b/src/frontend/services/st-lsp/__tests__/types.test.ts @@ -97,3 +97,20 @@ describe('dtViewUri / parseDtViewUri', () => { expect(parsePouVarsUri(dtViewUri('Motor'))).toBeNull() }) }) + +describe('malformed percent encoding', () => { + // These parsers run on every model URI the providers see, so a throw + // here would take hover / completion down for that model. + it('returns null instead of throwing, for every synthetic URI shape', () => { + expect(parsePouUri('inmemory://pou/%ZZ.st')).toBeNull() + expect(parsePouUri('inmemory://stub/%ZZ.st')).toBeNull() + expect(parsePouVarsUri('inmemory://pouvars/%ZZ.st')).toBeNull() + expect(parseDtViewUri('inmemory://dtview/%ZZ.dt')).toBeNull() + }) + + it('still decodes well-formed encodings', () => { + expect(parsePouUri(pouUri('My POU'))?.name).toBe('My POU') + expect(parsePouVarsUri(pouVarsUri('My POU'))).toBe('My POU') + expect(parseDtViewUri(dtViewUri('My Type'))).toBe('My Type') + }) +}) diff --git a/src/frontend/services/st-lsp/types.ts b/src/frontend/services/st-lsp/types.ts index 647804cf7..d667f8372 100644 --- a/src/frontend/services/st-lsp/types.ts +++ b/src/frontend/services/st-lsp/types.ts @@ -146,7 +146,7 @@ export function pouVarsUri(name: string): string { export function parsePouVarsUri(uri: string): string | null { const match = new RegExp(`^${POU_URI_SCHEME}://${POUVARS_URI_AUTHORITY}/(.+)\\.st$`).exec(uri) if (!match) return null - return decodeURIComponent(match[1]) + return decodeUriSegment(match[1]) } /** Make a synthetic in-memory URI for a data type's `.dt` code view. */ @@ -158,7 +158,7 @@ export function dtViewUri(name: string): string { export function parseDtViewUri(uri: string): string | null { const match = new RegExp(`^${POU_URI_SCHEME}://${DTVIEW_URI_AUTHORITY}/(.+)\\.dt$`).exec(uri) if (!match) return null - return decodeURIComponent(match[1]) + return decodeUriSegment(match[1]) } /** @@ -177,6 +177,21 @@ export const POU_DECLARATION_LINE_COUNT = 1 */ export const DT_VIEW_FRAME_LINE_COUNT = 1 +/** + * Decode a name segment out of a synthetic URI, or `null` when the + * encoding is malformed. These parsers run on every model URI the LSP + * providers see, so a bare `decodeURIComponent` would turn a stray + * `%ZZ` into a thrown `URIError` and take hover / completion down with + * it for that model. + */ +function decodeUriSegment(segment: string): string | null { + try { + return decodeURIComponent(segment) + } catch { + return null + } +} + /** * Returns the POU name encoded in a URI minted by `pouUri` or * `stubUri`, or `null` if the URI doesn't match one of those @@ -186,8 +201,7 @@ export const DT_VIEW_FRAME_LINE_COUNT = 1 export function parsePouUri(uri: string): { kind: 'pou' | 'stub'; name: string } | null { const match = new RegExp(`^${POU_URI_SCHEME}://(${POU_URI_AUTHORITY}|${STUB_URI_AUTHORITY})/(.+)\\.st$`).exec(uri) if (!match) return null - return { - kind: match[1] === POU_URI_AUTHORITY ? 'pou' : 'stub', - name: decodeURIComponent(match[2]), - } + const name = decodeUriSegment(match[2]) + if (name === null) return null + return { kind: match[1] === POU_URI_AUTHORITY ? 'pou' : 'stub', name } } From 63ad4b2705a7f9747aa7ee76c08e89f9ebde3d9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20GS=20Pereira?= Date: Fri, 7 Aug 2026 18:50:29 -0300 Subject: [PATCH 3/3] fix(st-lsp): stop an unknown .dt name from resolving as the first type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `.dt` view whose name has no entry in the aggregate document fell back to `span?.start ?? 1`, which is the first entry's mapping — so hover, completion and go-to-definition answered for an unrelated type. An unparseable `.dt` file reaches this path with a live, visible buffer. Pass the view's own un-indexed URI through instead: strucpp guards every handler on the document being known, so each provider answers nothing. Move the frame arithmetic into `dtview-context.ts` so it can be tested without a worker, and drop the duplicated shift in the diagnostics fan-out. Gate the store subscription on `isDataTypeFilesEnabled()` and on a `.dt` model actually being mounted. `refreshSemanticTokens()` re-tokenises every model in the ST language, so with the flag off a datatype table edit was triggering a worker round trip per open ST editor. Convert the goto-definition cursor at the call site, next to the POU conversions, rather than half inside the routing helper. Reported by review on #657 / #998. The frame-line aliasing (DOPE-554) and the formatting end-clip (DOPE-555) are tracked separately. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GvGrhB1LyJz9MBwYhjoBDY --- .../st-lsp/__tests__/dtview-context.test.ts | 99 +++++++++++++++++++ .../services/st-lsp/dtview-context.ts | 52 ++++++++++ .../st-lsp/goto-definition-redirect.ts | 21 ++-- src/frontend/services/st-lsp/index.ts | 51 ++++------ 4 files changed, 184 insertions(+), 39 deletions(-) create mode 100644 src/frontend/services/st-lsp/__tests__/dtview-context.test.ts create mode 100644 src/frontend/services/st-lsp/dtview-context.ts diff --git a/src/frontend/services/st-lsp/__tests__/dtview-context.test.ts b/src/frontend/services/st-lsp/__tests__/dtview-context.test.ts new file mode 100644 index 000000000..735cd1bbc --- /dev/null +++ b/src/frontend/services/st-lsp/__tests__/dtview-context.test.ts @@ -0,0 +1,99 @@ +/** + * @jest-environment jsdom + */ +import type { Diagnostic } from 'vscode-languageserver-protocol' + +import type { PLCDataType } from '../../../../middleware/shared/ports/types' +import { diagnosticsInSpan, dtViewLineOffset, dtViewSpan, dtViewWindow } from '../dtview-context' + +const enumType = (name: string): PLCDataType => ({ + name, + derivation: 'enumerated', + values: [{ description: 'RED' }], + initialValue: 'RED', +}) + +const structType = (name: string, fields: string[]): PLCDataType => ({ + name, + derivation: 'structure', + variable: fields.map((field) => ({ + name: field, + type: { definition: 'base-type', value: 'INT' }, + })), +}) + +// Aggregate document, LSP (0-indexed) lines: +// 0 TYPE +// 1 Colors : (RED) := RED; +// 2 Motor : STRUCT +// 3 speed : INT; +// 4 END_STRUCT; +// 5 END_TYPE +const DATA_TYPES: PLCDataType[] = [enumType('Colors'), structType('Motor', ['speed'])] + +const diagnosticAt = (line: number): Diagnostic => ({ + range: { start: { line, character: 0 }, end: { line, character: 4 } }, + message: `line ${line}`, +}) + +describe('dtViewSpan', () => { + it('returns the entry span for a type in the document', () => { + expect(dtViewSpan(DATA_TYPES, 'Motor')).toEqual({ start: 2, length: 3 }) + }) + + it('returns null for a name the document has no entry for', () => { + expect(dtViewSpan(DATA_TYPES, 'Missing')).toBeNull() + }) + + it('returns null for every name when the document is empty', () => { + expect(dtViewSpan([], 'Colors')).toBeNull() + }) +}) + +describe('dtViewLineOffset', () => { + it('is zero for the first entry — both frames open with their own TYPE line', () => { + const span = dtViewSpan(DATA_TYPES, 'Colors') + expect(span && dtViewLineOffset(span)).toBe(0) + }) + + it('shifts a later entry by its distance down the document', () => { + const span = dtViewSpan(DATA_TYPES, 'Motor') + expect(span && dtViewLineOffset(span)).toBe(1) + }) + + it('shifts the last entry of a longer document', () => { + const dataTypes = [enumType('A'), enumType('B'), structType('C', ['x', 'y'])] + const span = dtViewSpan(dataTypes, 'C') + expect(span && dtViewLineOffset(span)).toBe(2) + }) +}) + +describe('dtViewWindow', () => { + it('covers the entry lines only, never the frame line above them', () => { + const span = dtViewSpan(DATA_TYPES, 'Motor') + expect(span && dtViewWindow(span)).toEqual({ startLine: 2, endLineExclusive: 5 }) + }) + + it('is a single line for a one-line entry', () => { + const span = dtViewSpan(DATA_TYPES, 'Colors') + expect(span && dtViewWindow(span)).toEqual({ startLine: 1, endLineExclusive: 2 }) + }) +}) + +describe('diagnosticsInSpan', () => { + it('keeps only what falls inside the entry', () => { + const span = dtViewSpan(DATA_TYPES, 'Motor') + const kept = span ? diagnosticsInSpan([diagnosticAt(1), diagnosticAt(3), diagnosticAt(5)], span) : [] + expect(kept.map((d) => d.message)).toEqual(['line 3']) + }) + + it('excludes the line directly above the entry — that belongs to the previous type', () => { + const span = dtViewSpan(DATA_TYPES, 'Motor') + expect(span && diagnosticsInSpan([diagnosticAt(1)], span)).toEqual([]) + }) + + it('returns nothing when the entry is clean', () => { + const span = dtViewSpan(DATA_TYPES, 'Colors') + expect(span && diagnosticsInSpan([diagnosticAt(3)], span)).toEqual([]) + }) +}) diff --git a/src/frontend/services/st-lsp/dtview-context.ts b/src/frontend/services/st-lsp/dtview-context.ts new file mode 100644 index 000000000..22ccfbd10 --- /dev/null +++ b/src/frontend/services/st-lsp/dtview-context.ts @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +// Copyright (C) 2025 Autonomy / OpenPLC Project +/** + * Coordinate translation between a `.dt` code view and the aggregate + * datatypes document. + * + * A `.dt` view renders one type under its own `TYPE` frame line, while + * strucpp only ever sees `DATA_TYPES_URI` — every type in one document, + * under one frame. So the two frames differ by the type's position in + * the aggregate, and every request crossing the seam has to be shifted + * by it. + * + * These helpers take `dataTypes` rather than reading the store, so the + * arithmetic is testable on its own — the seam is where the token and + * marker defects in DOPE-537 came from. + */ + +import type { Diagnostic } from 'vscode-languageserver-protocol' + +import type { PLCDataType } from '../../../middleware/shared/ports/types' +import { type DataTypeLineSpan, dataTypeLineSpans } from '../../utils/PLC/data-type-serializer' +import { DT_VIEW_FRAME_LINE_COUNT } from './types' + +/** The aggregate document's line span for `dtName`, or null if it has none. */ +export function dtViewSpan(dataTypes: PLCDataType[], dtName: string): DataTypeLineSpan | null { + return dataTypeLineSpans(dataTypes).get(dtName) ?? null +} + +/** + * Lines to add to a `.dt` view position to reach the aggregate document. + * Both frames open with a `TYPE` line, so the shift is the entry's start + * minus that frame — 0 for the first type. + */ +export function dtViewLineOffset(span: DataTypeLineSpan): number { + return span.start - DT_VIEW_FRAME_LINE_COUNT +} + +/** + * Aggregate-document line window backing the view. Holds the entry's own + * lines only — widening it to cover the view's `TYPE` frame would pull in + * the previous entry's last line, whose columns overrun that 4-character + * frame line. + */ +export function dtViewWindow(span: DataTypeLineSpan): { startLine: number; endLineExclusive: number } { + return { startLine: span.start, endLineExclusive: span.start + span.length } +} + +/** The published diagnostics that fall inside the entry's own lines. */ +export function diagnosticsInSpan(diagnostics: Diagnostic[], span: DataTypeLineSpan): Diagnostic[] { + const { startLine, endLineExclusive } = dtViewWindow(span) + return diagnostics.filter((d) => d.range.start.line >= startLine && d.range.start.line < endLineExclusive) +} diff --git a/src/frontend/services/st-lsp/goto-definition-redirect.ts b/src/frontend/services/st-lsp/goto-definition-redirect.ts index d778456df..675d253fc 100644 --- a/src/frontend/services/st-lsp/goto-definition-redirect.ts +++ b/src/frontend/services/st-lsp/goto-definition-redirect.ts @@ -111,11 +111,11 @@ function openDataTypeEditor(dataType: PLCDataType): boolean { } /** - * Open the type's tab in code mode with the cursor on `lineInEntry` - * (0 = its declaration line). Falls back to the form tab when the - * `.dt` code view isn't built into this release. + * Open the type's tab in code mode at a Monaco position in its `.dt` + * view. Falls back to the form tab when the code view isn't built into + * this release. */ -function routeToDataTypeCodeView(dataType: PLCDataType, lineInEntry: number, characterLsp: number): boolean { +function routeToDataTypeCodeView(dataType: PLCDataType, monacoLine: number, monacoColumn: number): boolean { if (!openDataTypeEditor(dataType)) return false if (!isDataTypeFilesEnabled()) return true const { @@ -123,9 +123,8 @@ function routeToDataTypeCodeView(dataType: PLCDataType, lineInEntry: number, cha } = openPLCStoreBase.getState() updateModelStructureForName(dataType.name, { display: 'code' }) setEditorCursor(dataType.name, { - // The view renders its own `TYPE` frame line before the entry. - lineNumber: lineInEntry + DT_VIEW_FRAME_LINE_COUNT + 1, - column: Math.max(1, characterLsp + 1), + lineNumber: monacoLine, + column: monacoColumn, offset: 0, target: 'data-type', }) @@ -237,7 +236,13 @@ export function redirectDefinitionToStore(loc: Location | LocationLink): boolean const dataTypes = openPLCStoreBase.getState().project.data.dataTypes const hit = findDataTypeAtLine(target.lineLsp, dataTypes) if (!hit) return false - return routeToDataTypeCodeView(hit.dataType, hit.lineInEntry, target.characterLsp) + // Entry-relative line → `.dt` view line (its own `TYPE` frame sits + // above the entry) → Monaco's 1-indexed frame. + return routeToDataTypeCodeView( + hit.dataType, + hit.lineInEntry + DT_VIEW_FRAME_LINE_COUNT + 1, + target.characterLsp + 1, + ) } const parsed = parsePouUri(target.uri) diff --git a/src/frontend/services/st-lsp/index.ts b/src/frontend/services/st-lsp/index.ts index d8a4e2bb3..0c3ba077d 100644 --- a/src/frontend/services/st-lsp/index.ts +++ b/src/frontend/services/st-lsp/index.ts @@ -36,6 +36,7 @@ import { } from 'vscode-languageserver-protocol' import { openPLCStoreBase } from '../../store' +import { isDataTypeFilesEnabled } from '../../utils/feature-flags' import { dataTypeLineSpans, serializeDataTypeToText } from '../../utils/PLC/data-type-serializer' import { serializePouScopeForQuery } from '../../utils/PLC/pou-signature-serializer' import { @@ -47,6 +48,7 @@ import { suppressNoDefinitionFound, } from '../lsp-shared' import { parseScopedCompletionType } from './completion-type' +import { diagnosticsInSpan, dtViewLineOffset, dtViewSpan, dtViewWindow } from './dtview-context' import { redirectDefinitionToStore } from './goto-definition-redirect' import { redirectToGraphicalPou } from './graphical-redirect' import { registerScopedQueryApi, type ScopedCompletionItem } from './scoped-query' @@ -106,28 +108,17 @@ function resolveStLspContext(modelUri: string): LspContext { } const dtName = parseDtViewUri(modelUri) if (dtName !== null) { - const span = dataTypeLineSpans(openPLCStoreBase.getState().project.data.dataTypes).get(dtName) - // An unknown name (type deleted while its tab is open) keeps the - // frame shift so requests stay inside the document. - return { lspUri: DATA_TYPES_URI, lineOffset: (span?.start ?? 1) - DT_VIEW_FRAME_LINE_COUNT } + const span = dtViewSpan(openPLCStoreBase.getState().project.data.dataTypes, dtName) + // A name absent from the document (unparseable `.dt` file) has no + // span to shift by. Pass the view's own URI through: the worker + // never indexed it, so every provider answers nothing rather than + // answering for whichever type happens to be first. + if (!span) return { lspUri: modelUri, lineOffset: 0 } + return { lspUri: DATA_TYPES_URI, lineOffset: dtViewLineOffset(span) } } return { lspUri: modelUri, lineOffset: getBodyLineOffset(modelUri) } } -/** - * Aggregate-document line window backing a `.dt` code view, or null. - * Holds the entry's own lines only — widening it to cover the view's - * `TYPE` frame would pull in the previous entry's last line, whose - * columns overrun that 4-character frame line. - */ -function dtViewWindow(modelUri: string): { start: number; endExclusive: number } | null { - const dtName = parseDtViewUri(modelUri) - if (dtName === null) return null - const span = dataTypeLineSpans(openPLCStoreBase.getState().project.data.dataTypes).get(dtName) - if (!span) return null - return { start: span.start, endExclusive: span.start + span.length } -} - /** * True while a `.dt` model's text still matches what the store would * serialise for that type. An uncommitted edit breaks the match, and @@ -150,14 +141,12 @@ function applyDataTypeDiagnostics(monacoApi: typeof monaco, markerOwner: string, for (const [name, span] of dataTypeLineSpans(openPLCStoreBase.getState().project.data.dataTypes)) { const model = monacoApi.editor.getModels().find((m) => m.uri.toString() === dtViewUri(name)) if (!model) continue - const shift = span.start - DT_VIEW_FRAME_LINE_COUNT - const owned = lastDataTypeDiagnostics.filter( - (d) => d.range.start.line >= span.start && d.range.start.line < span.start + span.length, - ) monacoApi.editor.setModelMarkers( model, markerOwner, - owned.map((d) => lspDiagnosticToMonaco(d, monacoApi, shift, defaultSource)), + diagnosticsInSpan(lastDataTypeDiagnostics, span).map((d) => + lspDiagnosticToMonaco(d, monacoApi, dtViewLineOffset(span), defaultSource), + ), ) } } @@ -217,17 +206,13 @@ export function startStLsp(opts: StLspStartOptions): StLspService { resolveSemanticTokensViewport: (lspUri, modelUri, lineOffset) => { const dtName = parseDtViewUri(modelUri) if (dtName !== null) { - const dtWindow = dtViewWindow(modelUri) + const span = dtViewSpan(openPLCStoreBase.getState().project.data.dataTypes, dtName) // Empty window while the buffer is uncommitted: no colours beats // colours describing the previous text. - if (!dtWindow || !monacoApi || !dtViewMatchesStore(dtName, monacoApi)) { + if (!span || !monacoApi || !dtViewMatchesStore(dtName, monacoApi)) { return { startLine: 0, endLineExclusive: 0 } } - return { - startLine: dtWindow.start, - endLineExclusive: dtWindow.endExclusive, - outputStartLine: DT_VIEW_FRAME_LINE_COUNT, - } + return { ...dtViewWindow(span), outputStartLine: DT_VIEW_FRAME_LINE_COUNT } } const isVarsView = parsePouVarsUri(modelUri) !== null return { @@ -286,12 +271,16 @@ export function startStLsp(opts: StLspStartOptions): StLspService { // so a change there leaves the model's own text untouched and Monaco // never re-queries on its own. Re-drive both from the store instead. const dtViewSyncDisposables: Array<() => void> = [] - if (monacoApi) { + if (monacoApi && isDataTypeFilesEnabled()) { const api = monacoApi + const hasDtViewModel = () => api.editor.getModels().some((m) => parseDtViewUri(m.uri.toString()) !== null) dtViewSyncDisposables.push( openPLCStoreBase.subscribe( (state) => state.project.data.dataTypes, () => { + // `refresh()` re-tokenises every ST model in the language, so it + // must not fire for a datatype edit made with no `.dt` view open. + if (!hasDtViewModel()) return sharedService.refreshSemanticTokens() applyDataTypeDiagnostics(api, MARKER_OWNER, DIAGNOSTIC_SOURCE) },