From e967a4fc07ba5d37c278a3c799f689d8c75f1beb Mon Sep 17 00:00:00 2001 From: Thiago Alves Date: Mon, 10 Aug 2026 13:37:09 -0400 Subject: [PATCH] fix(variables): allow a comma in a declaration's type so inline multi-dimensional arrays parse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `m : ARRAY[0..1, 0..2] OF INT;` written in a POU's variables text failed the whole POU: the type capture group excluded the comma, so no line matched and `guessErrorReason` — whose allowed-character set also omitted the comma — blamed "invalid or unsupported characters". The only way to declare a 2D/3D array was to name an ARRAY data type first and reference that. Nothing downstream needed changing. `parseArrayType` has always split comma-separated bounds into multiple `dimensions`, and the reverse serializer emits `type.value`, which holds the full type string — so the round trip (text → variables → text) is unaffected. The data-type text parser (`PLC/data-type-text-parser.ts`) already allowed the comma; this brings the variable parser in line with it. Scope of the change is narrow by construction: the type group stays lazy and is still bounded by the following `AT` / `:=` / `;`, and a comma is never valid between a declaration's name and its type. In particular this does NOT start accepting multi-name declarations (`a, b : INT;`) — the name group is a single identifier that must be followed by the colon — and that stays covered by a test. Tests: inline 2D and 3D, with and without a space after the comma, of a base and of a user-defined type, with an initial value, in the alternate located (`name AT loc : type`) format, plus the two negative cases — multi-name still rejected, and an unrelated error no longer misreported as a bad character. Co-Authored-By: Claude Opus 5 (1M context) --- .../generate-iec-string-to-variables.test.ts | 76 +++++++++++++++++++ .../generate-iec-variables-to-string.test.ts | 23 ++++++ .../utils/generate-iec-string-to-variables.ts | 21 ++++- 3 files changed, 117 insertions(+), 3 deletions(-) diff --git a/src/frontend/utils/__tests__/generate-iec-string-to-variables.test.ts b/src/frontend/utils/__tests__/generate-iec-string-to-variables.test.ts index feda16195..9ee4360fe 100644 --- a/src/frontend/utils/__tests__/generate-iec-string-to-variables.test.ts +++ b/src/frontend/utils/__tests__/generate-iec-string-to-variables.test.ts @@ -160,6 +160,82 @@ describe('parseIecStringToVariables', () => { expect(result[0].documentation).toBe('buffer') }) + // ---- multi-dimensional arrays declared inline ---- + // + // The type group used to exclude the comma, so a 2D/3D array could only be + // declared by naming an ARRAY data type first; written inline it failed the + // whole POU with "invalid or unsupported characters". + + it('parses a 2D ARRAY declared inline', () => { + const input = 'VAR\n m : ARRAY[0..1, 0..2] OF INT;\nEND_VAR' + const result = parseIecStringToVariables(input) + + expect(result).toHaveLength(1) + expect(result[0].type.definition).toBe('array') + expect(result[0].type.value).toBe('ARRAY[0..1, 0..2] OF INT') + expect(result[0].type.data).toEqual({ + baseType: { definition: 'base-type', value: 'INT' }, + dimensions: [{ dimension: '0..1' }, { dimension: '0..2' }], + }) + }) + + it('parses a 3D ARRAY declared inline', () => { + const input = 'VAR\n c : ARRAY[0..1, 0..1, 0..1] OF INT;\nEND_VAR' + const result = parseIecStringToVariables(input) + + expect(result[0].type.data).toEqual({ + baseType: { definition: 'base-type', value: 'INT' }, + dimensions: [{ dimension: '0..1' }, { dimension: '0..1' }, { dimension: '0..1' }], + }) + }) + + it('parses a multi-dimensional ARRAY with no space after the comma', () => { + const input = 'VAR\n m : ARRAY[0..1,0..1] OF INT;\nEND_VAR' + const result = parseIecStringToVariables(input) + + expect(result[0].type.data?.dimensions).toEqual([{ dimension: '0..1' }, { dimension: '0..1' }]) + }) + + it('parses a multi-dimensional ARRAY of a user-defined type', () => { + const input = 'VAR\n grid : ARRAY[0..1, 0..1] OF Point;\nEND_VAR' + const result = parseIecStringToVariables(input) + + expect(result[0].type.data).toEqual({ + baseType: { definition: 'user-data-type', value: 'Point' }, + dimensions: [{ dimension: '0..1' }, { dimension: '0..1' }], + }) + }) + + it('keeps the initial value when a multi-dimensional ARRAY has one', () => { + const input = 'VAR\n m : ARRAY[0..1, 0..2] OF INT := [[1,2,3],[4,5,6]];\nEND_VAR' + const result = parseIecStringToVariables(input) + + expect(result[0].type.value).toBe('ARRAY[0..1, 0..2] OF INT') + expect(result[0].initialValue).toBe('[[1,2,3],[4,5,6]]') + }) + + it('parses a multi-dimensional ARRAY in the alternate located format', () => { + const input = 'VAR_GLOBAL\n m AT %MW0 : ARRAY[0..1, 0..1] OF INT;\nEND_VAR' + const result = parseIecStringToVariables(input) + + expect(result[0].location).toBe('%MW0') + expect(result[0].type.data?.dimensions).toHaveLength(2) + }) + + it('still rejects a multi-name declaration', () => { + // Allowing the comma in the type must not make `a, b : INT;` parse — the + // name group is a single identifier followed by the colon. + const input = 'VAR\n a, b : INT;\nEND_VAR' + expect(() => parseIecStringToVariables(input)).toThrow(/Syntax error on line 2/) + }) + + it('no longer blames a comma for an unrelated syntax error', () => { + // A comma is legal now, so the guessed reason must fall through instead of + // reporting "invalid or unsupported characters". + const input = 'VAR\n m ARRAY[0..1, 0..1] OF INT;\nEND_VAR' + expect(() => parseIecStringToVariables(input)).toThrow(/missing colon/) + }) + // ---- alternate format (line 115) ---- it('parses the alternate format: name AT location : type', () => { diff --git a/src/frontend/utils/__tests__/generate-iec-variables-to-string.test.ts b/src/frontend/utils/__tests__/generate-iec-variables-to-string.test.ts index 9c4ef4d29..f006b9a53 100644 --- a/src/frontend/utils/__tests__/generate-iec-variables-to-string.test.ts +++ b/src/frontend/utils/__tests__/generate-iec-variables-to-string.test.ts @@ -1,4 +1,5 @@ import type { PLCVariable } from '../../../middleware/shared/ports/types' +import { parseIecStringToVariables } from '../generate-iec-string-to-variables' import { generateIecVariablesToString, getIecVariableLineMap } from '../generate-iec-variables-to-string' const makeVariable = (overrides: Partial & Pick): PLCVariable => ({ @@ -212,4 +213,26 @@ describe('getIecVariableLineMap', () => { const map = getIecVariableLineMap([makeVariable({ name: 'OnlyOne', class: 'input' })]) expect(map.get('OnlyOne')?.column).toBe(5) }) + + // The variables table and the code view are two views of the same data, so + // text -> variables -> text has to be byte-stable. Multi-dimensional array + // types are the interesting case: the parser splits the bounds into + // `dimensions` but keeps the full type string in `type.value`, which is what + // this serializer emits. + it('round-trips inline multi-dimensional array declarations unchanged', () => { + const input = [ + ' VAR', + ' m : ARRAY[0..1, 0..2] OF INT := [[1,2,3],[4,5,6]];', + ' c : ARRAY[0..1, 0..1, 0..1] OF INT := [1,2,3,4,5,6,7,8];', + ' g : ARRAY[0..1, 0..1] OF Point;', + ' END_VAR', + ].join('\n') + + const vars = parseIecStringToVariables(input) + expect(vars).toHaveLength(3) + + const out = generateIecVariablesToString(vars) + expect(out).toBe(input) + expect(parseIecStringToVariables(out)).toEqual(vars) + }) }) diff --git a/src/frontend/utils/generate-iec-string-to-variables.ts b/src/frontend/utils/generate-iec-string-to-variables.ts index ac50b424d..4676f66a9 100644 --- a/src/frontend/utils/generate-iec-string-to-variables.ts +++ b/src/frontend/utils/generate-iec-string-to-variables.ts @@ -27,22 +27,37 @@ export const DISALLOWED_LOCATION_CLASSES: ReadonlyArray = 'temp', ] +// The type group accepts a comma so a multi-dimensional array can be declared +// inline: `m : ARRAY[0..1, 0..2] OF INT;`. `parseArrayType` below has always +// split multi-dimensional bounds, and the data-type text parser +// (`PLC/data-type-text-parser.ts`) already allows the comma — without it here, +// the only way to declare a 2D/3D array was to name an ARRAY data type first, +// and writing it inline failed the whole POU with "invalid or unsupported +// characters". +// +// The group stays lazy and is bounded by the following `AT` / `:=` / `;`, and a +// comma is never valid between a declaration's name and its type, so this can't +// swallow anything it didn't before. Note this does NOT enable multi-name +// declarations (`a, b : INT;`) — `name` is a single `\w+` followed by `:`. + // Primary format: name : type AT location := initialValue ; (* documentation *) const lineRegex = // eslint-disable-next-line no-useless-escape - /^\s*(?\w+)\s*:\s*(?[\w\s\[\]\.]+?)(?:\s+AT\s+(?[\w\d\._%]+))?\s*(?::=\s*(?[^;]+?))?\s*;\s*(?:\(\*\s*(?.*?)\s*\*\))?$/ + /^\s*(?\w+)\s*:\s*(?[\w\s\[\],\.]+?)(?:\s+AT\s+(?[\w\d\._%]+))?\s*(?::=\s*(?[^;]+?))?\s*;\s*(?:\(\*\s*(?.*?)\s*\*\))?$/ // Alternate format: name AT location : type := initialValue ; (* documentation *) // This format is used by some IEC 61131-3 tools and older versions of OpenPLC Editor const alternateLineRegex = // eslint-disable-next-line no-useless-escape - /^\s*(?\w+)\s+AT\s+(?[\w\d\._%]+)\s*:\s*(?[\w\s\[\]\.]+?)\s*(?::=\s*(?[^;]+?))?\s*;\s*(?:\(\*\s*(?.*?)\s*\*\))?$/ + /^\s*(?\w+)\s+AT\s+(?[\w\d\._%]+)\s*:\s*(?[\w\s\[\],\.]+?)\s*(?::=\s*(?[^;]+?))?\s*;\s*(?:\(\*\s*(?.*?)\s*\*\))?$/ const guessErrorReason = (line: string): string => { if (!line.includes(';')) return 'missing semicolon (;) at the end of the declaration' if (!line.includes(':')) return 'missing colon (:) between name and type' + // Comma is legal — multi-dimensional array bounds and comma-separated initial + // values both use it — so it must not be reported as an unsupported character. // eslint-disable-next-line no-useless-escape - if (/[^A-Za-z0-9_\s:;=%()/*\-.\[\]]/.test(line)) return 'invalid or unsupported characters' + if (/[^A-Za-z0-9_\s:;=%()/*\-.,\[\]]/.test(line)) return 'invalid or unsupported characters' return 'unrecognized declaration format' }