feat(compile): embed JSON→ST transpiler, retire xml2st binary - #843
Merged
Conversation
Ports the in-process JSON-fed transpiler (`generate-st-from-json/`) from openplc-web into the editor's backend and wires it through the shared `CompilerPlatformPort`/`LibraryBuildPort` surface. The bundled `xml2st` subprocess hop and `XmlGenerator` pre-step are no longer invoked during compile or library-build — both run entirely in-process against the renderer's project IR. - Renames `transpileXmlToSt` → `transpileToSt` across both ports. - Editor port impls (`editor-compiler-platform-port`, `desktop-library-build-port`) project the editor's IPC schema-shape payload via the new `fromSchemaShape` adapter. - Pipeline + library-build orchestrator cast the schema-shape payload to `never` at the port call site (matches openplc-web's pattern; each port impl knows the actual shape it receives). - Adds `@xmldom/xmldom@^0.9.10` for the transitional DOM fallback the graphical-POU walker still uses. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Lost in the diff? Review this PR in Change Stack to follow the change map from intent to exact ranges. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughReplaces XML→ST subprocess with in-process JSON→ST transpilation. Adds IR types, emitters, LD/SFC walkers, PLCOpen DOM helpers, and updates compiler/library pipelines and shared ports to use transpileToSt with projectData. Adds ChangesJSON→ST transpiler integration and pipeline updates
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (16)
src/middleware/shared/ports/compiler-platform-port.ts (1)
83-101: ⚡ Quick winOrphaned documentation block describes removed functionality.
This docstring block still describes the removed
TranspileXmlToStArgs(XML input,xml2stArgsCLI tokens, etc.) but now sits above the newTranspileToStArgswhich takesprojectData. The old documentation should be removed since it no longer applies.🧹 Proposed fix to remove stale documentation
-/** `xml2st` input: a single XML string (the IEC 61131-3 PLC XML the - * shared `XmlGenerator` produces) plus an array of extra CLI tokens - * to append to the xml2st invocation. Defined here (not on either - * adapter) because xml2st flag drift was the root cause of the - * initial cross-platform STRUCT bug — editor's local xml2st passed - * `--keep-structs` but the web's compile-service `/generate-st` - * endpoint hardcoded an unflagged invocation, so structs declared - * in the project compiled fine on the desktop and blew up on the - * web with `Undefined type 'MY_STRUCT'` errors out of strucpp. - * Pushing the flag set into a single shared field means any future - * xml2st option is a one-line pipeline change with no per-platform - * drift possible. - * - * Editor's adapter passes `xml2stArgs` verbatim to the local - * binary (trusted). Web's adapter filters against its own - * known-args allowlist before forwarding to the compile-service - * `/generate-st` endpoint, logging a warning for anything it - * doesn't recognise (defence in depth — service has its own - * allowlist too). */ /** * Input to the in-process JSON → ST transpiler. Each adapter🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/middleware/shared/ports/compiler-platform-port.ts` around lines 83 - 101, Remove the stale xml2st-focused docblock that describes TranspileXmlToStArgs/xml2stArgs and update the surrounding comment area to reflect the new API: delete the orphaned documentation above TranspileToStArgs and, if needed, replace it with a short accurate comment describing TranspileToStArgs and its projectData parameter (or leave no docblock) so the comment matches the current symbols (TranspileToStArgs, projectData) in compiler-platform-port.ts.src/backend/editor/compiler/desktop-library-build-port.ts (1)
92-94: ⚡ Quick winWarnings logged as 'info' instead of 'warning' (same as editor port).
Same issue as in
editor-compiler-platform-port.ts- warnings should be logged at'warning'level for consistency.💡 Proposed fix
for (const warning of result.warnings) { - log(warning, 'info') + log(warning, 'warning') }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/editor/compiler/desktop-library-build-port.ts` around lines 92 - 94, The loop that logs compiler warnings currently calls log(warning, 'info'); change it to use the 'warning' level instead so warnings are emitted consistently (replace the 'info' level in the for (const warning of result.warnings) block to 'warning'); mirror the same fix applied in editor-compiler-platform-port.ts and ensure result.warnings uses log(warning, 'warning').src/backend/shared/library/build-pipeline.ts (2)
7-26: ⚡ Quick winModule docstring describes retired XML generation flow.
The documentation still describes the old pipeline:
- Step 1 mentions "runs the canonical XmlGenerator"
- Step 2 mentions "caller runs xml2st on the resulting plc.xml"
These descriptions are now stale since the pipeline produces
projectDatafor the JSON transpiler instead of XML.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/library/build-pipeline.ts` around lines 7 - 26, Update the module docstring to reflect the new JSON-transpiler-based flow: replace references to XmlGenerator and xml2st with the current behavior where prepareXmlForLibraryBuild (or its renamed equivalent) now synthesizes a transient PLCProject/projectData and emits projectData for the JSON transpiler, the caller invokes the JSON transpiler to produce program.st, and libraryBuildFromTranspiledSt consumes that program.st (splits per-POU, drops the synthetic main, runs strucpp.compileStlib with knownPous and manifest) to produce the .stlib bytes; ensure the docstring mentions projectData and the JSON transpiler explicitly and removes stale XML/xml2st wording.
242-257: ⚡ Quick winFunction name
prepareXmlForLibraryBuildis now misleading.This function no longer generates XML - it prepares the stubbed project data for the JSON transpiler. Consider renaming to
prepareProjectForLibraryBuildorprepareDataForLibraryBuildto accurately reflect its purpose.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/library/build-pipeline.ts` around lines 242 - 257, Rename the misleading function prepareXmlForLibraryBuild to a name that reflects it returns stubbed project data (e.g., prepareProjectForLibraryBuild or prepareDataForLibraryBuild); update the function declaration, its export, and all internal references/usages to the new identifier, and adjust any tests or imports that call prepareXmlForLibraryBuild so they import/use the new name (keep the implementation and returned PrepareXmlOutcome type intact, only change the symbol name).src/backend/shared/compile/pipeline.ts (1)
7-7: ⚡ Quick winModule docstring references retired
xml2st transport.Line 7 still describes "xml2st transport" as one of three platform-specific differences. Since transpilation is now in-process via the JSON transpiler, this documentation is stale and should be updated.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/compile/pipeline.ts` at line 7, Update the module docstring that currently mentions "xml2st transport" to reflect the new in-process JSON transpiler: locate the top-of-file module comment in pipeline.ts (the module docstring that contains the phrase "xml2st transport") and replace that reference with wording like "in-process JSON transpiler" or "JSON transpiler" so the docstring accurately describes the platform differences and no longer references the retired xml2st transport.src/backend/editor/compiler/editor-compiler-platform-port.ts (1)
183-185: ⚡ Quick winWarnings logged as 'info' instead of 'warning'.
Transpiler warnings are logged at
'info'level, but thePlatformLogtype supports'warning'. For consistency with the severity of the diagnostic, consider logging warnings at the'warning'level.💡 Proposed fix
for (const warning of result.warnings) { - log(warning, 'info') + log(warning, 'warning') }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/editor/compiler/editor-compiler-platform-port.ts` around lines 183 - 185, The loop currently logs transpiler warnings using log(warning, 'info'); change the log level to 'warning' so diagnostics use the correct severity (update the call inside the for (const warning of result.warnings) loop to use 'warning' rather than 'info'); keep using the existing log function and PlatformLog type (PlatformLog supports 'warning') so only the second argument needs to change.src/backend/shared/transpilers/generate-st-from-json/ld/walker.ts (1)
501-525: 💤 Low valueConsider removing unused variable
primaryFormal.The variable
primaryFormalis assigned on line 501 but never used, and line 525 explicitly silences the unused variable warning withvoid primaryFormal. If this variable isn't needed for future functionality, removing it would reduce code noise.♻️ Proposed cleanup
const isPrimary = block.outputs.length === 1 || out.formalParameter === '' || out.formalParameter === 'OUT' if (isPrimary && primaryName === null) { primaryName = tempName - primaryFormal = out.formalParameter primaryIdx = i } else { parts.push([ [out.formalParameter, [...info, 'output', i]], [` => ${tempName}`, []], ]) } } // No primary output → nothing meaningful to emit. Corpus never // triggers this (standard functions always have `OUT` or unnamed). if (primaryName === null) return state.program.push([state.currentIndent, []]) state.program.push([primaryName, [...info, 'output', primaryIdx]]) state.program.push([' := ', []]) state.program.push([block.typeName, [...info, 'type']]) state.program.push(['(', []]) for (let i = 0; i < parts.length; i++) { if (i > 0) state.program.push([', ', []]) state.program.push(...parts[i]) } state.program.push([');\n', []]) - void primaryFormal }And remove the declaration:
let primaryName: string | null = null - let primaryFormal = '' let primaryIdx = 0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/ld/walker.ts` around lines 501 - 525, The variable primaryFormal is assigned but never used in the walker logic (see symbols primaryFormal, primaryIdx, primaryName, parts and the final void primaryFormal); remove the primaryFormal declaration and its assignment(s) and also remove the trailing void primaryFormal statement so the function no longer contains an unused variable; ensure related logic still uses primaryIdx/primaryName/parts unchanged.src/backend/shared/transpilers/generate-st-from-json/from-schema.ts (1)
218-219: 💤 Low valuePrefix unused parameters with underscore.
The
rangeandtagnameparameters are voided immediately, indicating they're unused. Consider either removing them or prefixing with_to follow TypeScript conventions for intentionally unused parameters.♻️ Proposed fix
function emitGlobalVarList( out: ProgramChunk[], variables: TranspileVariable[], tagname: string, indent: string, _varIndent: string, project: TranspileProject, ): void { if (variables.length === 0) return const variableType = 'var_local' - const range: [number, number] = [0, variables.length] out.push([`${indent}VAR_GLOBAL`, []]) // CONSTANT / RETAIN / NON_RETAIN modifiers come from the // <globalVars> wrapper in the DOM path; the IR doesn't surface // per-list modifiers today (only the bare variable list). - void range - void tagname out.push(['\n', []])Or if these will be used in future phases, document why they're currently unused.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/from-schema.ts` around lines 218 - 219, The parameters named range and tagname in the function(s) inside from-schema.ts are immediately voided and thus unused; update their declarations to prefix them with an underscore (e.g., _range, _tagname) or remove them if truly unnecessary, or add a brief comment explaining they are intentionally unused for future use—apply this change to the specific function signature(s) where range and tagname appear to silence TS/linters and follow convention.src/backend/shared/transpilers/generate-st-from-json/src/plcopen/accessors.ts (3)
485-498: ⚡ Quick winSimplify type handling to avoid unsafe casts.
The pattern
(project as ProjectTree).documentElement ?? (project as Element)relies on unsafe type assertions. Consider using the existingisDocumenthelper for consistency with other functions likegetpousandgetconfigurations.♻️ Proposed refactor
export function getdataType( project: ProjectTree | Element, name: string, ): Element | null { - const root = (project as ProjectTree).documentElement ?? (project as Element) + const root = isDocument(project) ? project.documentElement : project if (!root) return null const types = findChild(root, 'types')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/src/plcopen/accessors.ts` around lines 485 - 498, The function getdataType uses unsafe casts to obtain root via (project as ProjectTree).documentElement ?? (project as Element); change it to use the isDocument helper to determine whether project is a ProjectTree (document) or an Element like getpous/getconfigurations do, then read documentElement only when isDocument(project) is true and otherwise treat project as Element, avoiding direct type assertions; update references inside getdataType accordingly so ProjectTree.documentElement is accessed only after the isDocument check.
129-129: ⚡ Quick winFix grammatical error in error message.
The error message
"${tag} body don't have instances!"should use"doesn't"instead of"don't"sincebodyis singular.📝 Proposed fix
- throw new TypeError(`${tag} body don't have instances!`) + throw new TypeError(`${tag} body doesn't have instances!`)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/src/plcopen/accessors.ts` at line 129, Update the TypeError message in the throw statement in accessors.ts that currently reads `${tag} body don't have instances!` to use correct grammar; change it to `${tag} body doesn't have instances!` (or `${tag} body doesn't have any instances!`) so the singular "body" pairs with "doesn't"; locate the throw in the accessors.ts file where `tag` is interpolated and replace the string accordingly.
115-115: ⚡ Quick winFix grammatical error in error message.
The error message
"${tag} body don't have instances!"should use"doesn't"instead of"don't"sincebodyis singular.📝 Proposed fix
- throw new TypeError(`${tag} body don't have instances!`) + throw new TypeError(`${tag} body doesn't have instances!`)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/src/plcopen/accessors.ts` at line 115, The error message thrown by the throw expression that uses the variable tag is grammatically incorrect; update the TypeError message in the throw new TypeError(...) expression so it uses "doesn't" (e.g., change "body don't have instances" to "body doesn't have instances" or "body doesn't have any instances") while keeping the tag interpolation intact.src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/ctn_globals.ts (1)
205-210: 💤 Low valueDuck-typing check may fail on edge-case inputs.
Line 206 checks
'createElementNS' in project, which assumes that any object with acreateElementNSproperty is aDocument. This could pass for unrelated objects that happen to have this property, though in practice the typedProjectTree | Elementunion makes false positives unlikely. The type assertion at line 207 ((project).ownerDocument) also bypasses strict type checking.These patterns work but reduce type safety. Consider refining:
🔧 Optional refinement
function getOwnerDocument(project: ProjectTree | Element) { - if ('createElementNS' in project) return project - const owner = (project).ownerDocument + if ('nodeType' in project && project.nodeType === 9) { + // nodeType === 9 is DOCUMENT_NODE per DOM spec + return project as Document + } + const owner = (project as Element).ownerDocument if (owner) return owner return new DOMImplementation().createDocument(TC6_NS, 'project', null) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/ctn_globals.ts` around lines 205 - 210, The duck-typing using `'createElementNS' in project` in getOwnerDocument is fragile; add a proper runtime type guard (e.g., function isDocument(x): x is Document that checks typeof createElementNS === 'function' and/or x.nodeType === 9 or uses instanceof Document when available) and use it to narrow `project` before returning it, then safely access `project.ownerDocument` on the Element branch; keep the fallback to `new DOMImplementation().createDocument(TC6_NS, 'project', null)` unchanged.src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/path_tree.ts (1)
506-510: 💤 Low valueRemove unnecessary
void toInoutstatement.
toInoutis used at line 423 when callinggenerateBlock, so thisvoidstatement to suppress an unused variable warning is unnecessary and may be a leftover from earlier development.🧹 Suggested fix
return paths - // `toInout` is unused in Phase 4c; consumed by the Block branch in 4f. - void toInout }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/path_tree.ts` around lines 506 - 510, Remove the unnecessary "void toInout" statement at the end of the function: since toInout is already referenced (e.g., passed into generateBlock at line ~423) the void suppression is stale; simply delete the `void toInout` line that follows the `return paths` so the function returns normally without the unused-expression statement.src/backend/shared/transpilers/generate-st-from-json/ir-emit/configuration.ts (2)
172-194: 💤 Low valueConsider hoisting
elementaryTypesto module scope.The Set is recreated on every call to
tupleTypeToIr. While not a hot path, hoisting to module level aligns with the pattern inctn_globals.tswherePLC_BASE_TYPESis defined at module scope with the same values.+const ELEMENTARY_IEC_TYPES = new Set([ + 'BOOL', 'SINT', 'INT', 'DINT', 'LINT', 'USINT', 'UINT', 'UDINT', 'ULINT', + 'REAL', 'LREAL', 'TIME', 'DATE', 'TOD', 'DT', 'STRING', 'WSTRING', + 'BYTE', 'WORD', 'DWORD', 'LWORD', +]) + function tupleTypeToIr(typeName: string): TranspileVariable['type'] { - // CTN-globals tuples carry the type as a bare string. IEC base - // types resolve to `base-type`; anything else becomes a derived - // reference — mirrors PLCControler.py:1258-1273. const upper = typeName.toUpperCase() - const elementaryTypes = new Set([ - 'BOOL', 'SINT', 'INT', 'DINT', 'LINT', 'USINT', 'UINT', 'UDINT', 'ULINT', - 'REAL', 'LREAL', 'TIME', 'DATE', 'TOD', 'DT', 'STRING', 'WSTRING', - 'BYTE', 'WORD', 'DWORD', 'LWORD', - ]) - if (elementaryTypes.has(upper)) { + if (ELEMENTARY_IEC_TYPES.has(upper)) { return { definition: 'base-type', value: upper } } return { definition: 'derived', value: typeName } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/ir-emit/configuration.ts` around lines 172 - 194, Hoist the local Set named elementaryTypes out of the function into module scope to avoid recreating it on every tupleTypeToIr call; move its current initializer to a top-level const (e.g., reuse or mirror PLC_BASE_TYPES from ctn_globals.ts) and update tupleTypeToIr to reference that module-level const instead of constructing a new Set each invocation.
206-220: 💤 Low valueNaming and dead-code inconsistencies.
_varIndentuses underscore prefix (convention for unused) but is used on line 223.rangeis defined on line 212 and voided on line 218 but never used.void tagnameon line 219 is redundant sincetagnameis used on line 224.If
rangeis scaffolding for future modifier support per the comment, consider adding a TODO or removing it until needed.function emitGlobalVarList( out: ProgramChunk[], variables: TranspileVariable[], tagname: string, indent: string, - _varIndent: string, + varIndent: string, project: TranspileProject, ): void { if (variables.length === 0) return - const variableType = 'var_local' - const range: [number, number] = [0, variables.length] + const variableType = 'var_local' // TODO: support CONSTANT/RETAIN/NON_RETAIN modifiers out.push([`${indent}VAR_GLOBAL`, []]) - // CONSTANT / RETAIN / NON_RETAIN modifiers come from the - // <globalVars> wrapper in the DOM path; the IR doesn't surface - // per-list modifiers today (only the bare variable list). - void range - void tagname out.push(['\n', []]) variables.forEach((variable, idx) => { - out.push([_varIndent, []]) + out.push([varIndent, []])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/ir-emit/configuration.ts` around lines 206 - 220, Rename the parameter _varIndent to varIndent (remove the misleading underscore) so its usage on the following lines is correct, remove the redundant "void tagname" and "void range" statements, and either delete the unused local "range: [number, number] = [0, variables.length]" or replace it with a TODO comment explaining it's scaffolding for future per-list modifiers (so it remains but is clearly documented and not voided); ensure any other unused locals like variableType are similarly handled (remove or document) to eliminate dead-code warnings while preserving the intended behavior around emitting VAR_GLOBAL via out.push.src/backend/shared/transpilers/generate-st-from-json/index.ts (1)
161-166: 💤 Low valueUnused
contentvariable after assignment.
contentis assigned on line 161 and checked for truthiness, but the value is never passed togenerateProgram. If the purpose is purely to gate the call, consider simplifying to an inline check or removing the intermediate variable.- const content = getcontent(bodies[0]) - if (!content) { + if (!getcontent(bodies[0])) { warnings.push(`POU "${pou.name}" has empty body content; skipped`) continue } - const chunks = generateProgram(domPou, { project: tree })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/index.ts` around lines 161 - 166, The local variable content is assigned with getcontent(bodies[0]) but never used; replace the assignment with an inline guard to avoid the unused variable: call getcontent(bodies[0]) directly in the if condition (if (!getcontent(bodies[0])) { warnings.push(`POU "${pou.name}" has empty body content; skipped`); continue }) and then leave the call to generateProgram(domPou, { project: tree }) unchanged, or alternatively, if generateProgram actually needs the body text, pass content into generateProgram instead of discarding it (referencing getcontent, content, generateProgram, domPou, pou.name, and warnings).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/backend/shared/transpilers/generate-st-from-json/from-schema.ts`:
- Around line 12-13: The imports in from-schema.ts use deep relative paths;
replace them with path-alias imports using `@root` so the module resolution
follows project conventions — update the two import statements referencing
fbdToXml and ladderToXml to use the `@root/`* alias (e.g., import { fbdToXml }
from '`@root/frontend/utils/PLC/xml-generator/old-editor/language/fbd-xml`' and
similarly for ladderToXml) and ensure any tsconfig/webpack path mappings already
support `@root` so the build continues to resolve these symbols.
- Around line 146-160: The function projectStructureVariableType duplicates
projectVariableType; remove the duplicate by extracting the shared logic into a
single helper (keep projectVariableType as the canonical implementation or
create a new shared function name), replace projectStructureVariableType to call
that helper, and update any callers to use the shared function; ensure the
helper preserves behavior for 'array', 'derived', 'user-data-type', and default
'base-type' branches and references the same property names (type.definition,
type.data.dimensions, type.data.baseType.value, type.value) so behavior remains
identical.
In
`@src/backend/shared/transpilers/generate-st-from-json/ir-emit/pou-graphical.ts`:
- Around line 85-103: resolvedTempVars currently leaves tv.type as 'ANY' when
function temp resolution fails, which can break IEC61131-3 emission; in the
mapping over walkerState.functionTempVars ensure that after trying project.pous
lookup and resolveBlockType you add a final fallback that returns { ...tv, type:
'BOOL' } when tv.type (or the resolved value) is still 'ANY' so unresolved ANYs
are collapsed to 'BOOL' consistently with the existing ANY_* handling in the
outPort branch.
In `@src/backend/shared/transpilers/generate-st-from-json/ir-emit/pou-textual.ts`:
- Around line 82-84: The emitter currently throws when iface.length === 0 (throw
new Error(`No variable defined in "${pou.name}" POU`)), but the IR permits empty
interface.variables; change the logic in the textual POU emitter in
pou-textual.ts to treat an empty iface as valid: remove the throw and skip
emitting any VAR declaration block when iface.length === 0 so the POU is emitted
without VARs; keep existing behavior for non-empty iface so functions/methods
that iterate over iface or use pou.name remain unchanged.
In `@src/backend/shared/transpilers/generate-st-from-json/ir-emit/value.ts`:
- Around line 69-74: The quote-wrapping currently only wraps when both leading
and trailing quotes are absent; update the checks around baseType === 'STRING'
and baseType === 'WSTRING' so they wrap whenever the value is not fully quoted
(i.e., missing either the leading or trailing quote). Concretely, replace the
existing conditions that use && with a check that ensures both start and end
quotes are present (negate that) before returning the wrapped value for the
STRING and WSTRING branches.
In `@src/backend/shared/transpilers/generate-st-from-json/ld/walker_state.ts`:
- Around line 87-88: Replace the manual Map creation and population for byId
with the existing indexById helper: remove "const byId = new Map<number,
LdInstance>()" and the loop "for (const inst of body.instances)
byId.set(inst.localId, inst)" and instead call the helper (const byId =
indexById(body)); ensure indexById is imported from types (or the correct
module) and that byId continues to be used unchanged elsewhere in
walker_state.ts.
In `@src/backend/shared/transpilers/generate-st-from-json/pou-emission-order.ts`:
- Around line 111-114: The regex character class accidentally includes a literal
caret; update the pattern construction in the block that builds `re` (using
`escapeRegex(name.toUpperCase())`) so both `[^0-9^A-Z]` groups become
`[^0-9A-Z]` (i.e., remove the inner `^`) to correctly mean "not a digit or
uppercase letter", then keep the rest of the logic unchanged (the test against
`upper` and `deps.add(name)` remain the same).
In `@src/backend/shared/transpilers/generate-st-from-json/sfc/walker.ts`:
- Around line 393-422: The function emitTransitionCondition currently handles
SfcTransitionCondition kinds 'inline' and 'reference' but ignores 'connection',
producing missing output; update emitTransitionCondition to either (A) handle
condition.kind === 'connection' by emitting the expected connection text into
state.program (respecting state.currentIndent, using state.tagName and
transitionId for logging metadata), or (B) make the intent explicit by adding an
exhaustive else branch that asserts the unreachable case (e.g., const
_exhaustive: never = condition) so it's clear 'connection' is handled elsewhere;
ensure indentation (state.currentIndent) is set/restored and any newline
behavior matches the other branches.
- Around line 199-221: The recursive traversal in consumersReach is vulnerable
to cycles; update consumersReach(state, fromLocalId, stepLocalId) to accept an
optional visited: Set<number> (or create one at entry), immediately return false
if visited.has(candidate.localId) and add candidate.localId before recursing,
and pass the same visited set into recursive calls; apply the same pattern (add
a visited Set<number> param, check/add before recursing, and propagate it) to
walkBackToSteps and transitionLeadsTo to prevent infinite recursion on malformed
SFC bodies.
In
`@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/block_library.ts`:
- Around line 313-319: The project POU input matching currently uses strict
equality (sigTypes.every((t,i) => t === inputs[i])) which is inconsistent with
the catalog resolution; change this to use the isOfType compatibility check
(sigTypes.every((t,i) => isOfType(inputs[i], t))) so derived/compatible types
match the same way as in the catalog branch (where isOfType(callType, sig.type)
is used), and ensure isOfType is imported/available in the scope before
returning { source: 'project', infos }.
In
`@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/modifiers.ts`:
- Line 22: The import line in modifiers.ts currently lacks a space after the
comma; update the statement importing GenState and isAlreadyDefined from
'./gen_state' so it reads with a space after the comma (e.g. { type GenState,
isAlreadyDefined }) and ensure it follows project formatting (single quotes, no
semicolon) while keeping the same imported symbols (GenState, isAlreadyDefined)
and module ('./gen_state').
In `@src/backend/shared/transpilers/generate-st-from-json/src/util/py_compat.ts`:
- Around line 106-108: tupleCompare currently orders null against non-null
values (returning -1/1) which violates the mixed-type contract and Python
semantics; change the null handling so that only both-null returns 0, but if
exactly one of av or bv is null throw a TypeError instead of returning -1/1.
Update the logic in tupleCompare (the place that currently has "if (av === null)
return -1" / "if (bv === null) return 1") to: if (av === null && bv === null)
return 0; if (av === null || bv === null) throw new TypeError('Cannot compare
tuple elements of different types (null vs non-null)'); and keep the subsequent
mixed-type checks unchanged.
In
`@src/backend/shared/transpilers/generate-st-from-json/src/xmlclass/xsdschema.ts`:
- Line 136: The guard that validates parsed numeric value `n` is using the wrong
boolean operator: change the condition that currently combines
Number.isFinite(n) and Number.isInteger(n) with && to use || so the code reads
like `if (!Number.isFinite(n) || !Number.isInteger(n)) {` (look for the block
where `const n = Number(raw)` is parsed and throw on unparseable numbers) to
ensure non-finite or non-integer values are rejected.
---
Nitpick comments:
In `@src/backend/editor/compiler/desktop-library-build-port.ts`:
- Around line 92-94: The loop that logs compiler warnings currently calls
log(warning, 'info'); change it to use the 'warning' level instead so warnings
are emitted consistently (replace the 'info' level in the for (const warning of
result.warnings) block to 'warning'); mirror the same fix applied in
editor-compiler-platform-port.ts and ensure result.warnings uses log(warning,
'warning').
In `@src/backend/editor/compiler/editor-compiler-platform-port.ts`:
- Around line 183-185: The loop currently logs transpiler warnings using
log(warning, 'info'); change the log level to 'warning' so diagnostics use the
correct severity (update the call inside the for (const warning of
result.warnings) loop to use 'warning' rather than 'info'); keep using the
existing log function and PlatformLog type (PlatformLog supports 'warning') so
only the second argument needs to change.
In `@src/backend/shared/compile/pipeline.ts`:
- Line 7: Update the module docstring that currently mentions "xml2st transport"
to reflect the new in-process JSON transpiler: locate the top-of-file module
comment in pipeline.ts (the module docstring that contains the phrase "xml2st
transport") and replace that reference with wording like "in-process JSON
transpiler" or "JSON transpiler" so the docstring accurately describes the
platform differences and no longer references the retired xml2st transport.
In `@src/backend/shared/library/build-pipeline.ts`:
- Around line 7-26: Update the module docstring to reflect the new
JSON-transpiler-based flow: replace references to XmlGenerator and xml2st with
the current behavior where prepareXmlForLibraryBuild (or its renamed equivalent)
now synthesizes a transient PLCProject/projectData and emits projectData for the
JSON transpiler, the caller invokes the JSON transpiler to produce program.st,
and libraryBuildFromTranspiledSt consumes that program.st (splits per-POU, drops
the synthetic main, runs strucpp.compileStlib with knownPous and manifest) to
produce the .stlib bytes; ensure the docstring mentions projectData and the JSON
transpiler explicitly and removes stale XML/xml2st wording.
- Around line 242-257: Rename the misleading function prepareXmlForLibraryBuild
to a name that reflects it returns stubbed project data (e.g.,
prepareProjectForLibraryBuild or prepareDataForLibraryBuild); update the
function declaration, its export, and all internal references/usages to the new
identifier, and adjust any tests or imports that call prepareXmlForLibraryBuild
so they import/use the new name (keep the implementation and returned
PrepareXmlOutcome type intact, only change the symbol name).
In `@src/backend/shared/transpilers/generate-st-from-json/from-schema.ts`:
- Around line 218-219: The parameters named range and tagname in the function(s)
inside from-schema.ts are immediately voided and thus unused; update their
declarations to prefix them with an underscore (e.g., _range, _tagname) or
remove them if truly unnecessary, or add a brief comment explaining they are
intentionally unused for future use—apply this change to the specific function
signature(s) where range and tagname appear to silence TS/linters and follow
convention.
In `@src/backend/shared/transpilers/generate-st-from-json/index.ts`:
- Around line 161-166: The local variable content is assigned with
getcontent(bodies[0]) but never used; replace the assignment with an inline
guard to avoid the unused variable: call getcontent(bodies[0]) directly in the
if condition (if (!getcontent(bodies[0])) { warnings.push(`POU "${pou.name}" has
empty body content; skipped`); continue }) and then leave the call to
generateProgram(domPou, { project: tree }) unchanged, or alternatively, if
generateProgram actually needs the body text, pass content into generateProgram
instead of discarding it (referencing getcontent, content, generateProgram,
domPou, pou.name, and warnings).
In
`@src/backend/shared/transpilers/generate-st-from-json/ir-emit/configuration.ts`:
- Around line 172-194: Hoist the local Set named elementaryTypes out of the
function into module scope to avoid recreating it on every tupleTypeToIr call;
move its current initializer to a top-level const (e.g., reuse or mirror
PLC_BASE_TYPES from ctn_globals.ts) and update tupleTypeToIr to reference that
module-level const instead of constructing a new Set each invocation.
- Around line 206-220: Rename the parameter _varIndent to varIndent (remove the
misleading underscore) so its usage on the following lines is correct, remove
the redundant "void tagname" and "void range" statements, and either delete the
unused local "range: [number, number] = [0, variables.length]" or replace it
with a TODO comment explaining it's scaffolding for future per-list modifiers
(so it remains but is clearly documented and not voided); ensure any other
unused locals like variableType are similarly handled (remove or document) to
eliminate dead-code warnings while preserving the intended behavior around
emitting VAR_GLOBAL via out.push.
In `@src/backend/shared/transpilers/generate-st-from-json/ld/walker.ts`:
- Around line 501-525: The variable primaryFormal is assigned but never used in
the walker logic (see symbols primaryFormal, primaryIdx, primaryName, parts and
the final void primaryFormal); remove the primaryFormal declaration and its
assignment(s) and also remove the trailing void primaryFormal statement so the
function no longer contains an unused variable; ensure related logic still uses
primaryIdx/primaryName/parts unchanged.
In
`@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/ctn_globals.ts`:
- Around line 205-210: The duck-typing using `'createElementNS' in project` in
getOwnerDocument is fragile; add a proper runtime type guard (e.g., function
isDocument(x): x is Document that checks typeof createElementNS === 'function'
and/or x.nodeType === 9 or uses instanceof Document when available) and use it
to narrow `project` before returning it, then safely access
`project.ownerDocument` on the Element branch; keep the fallback to `new
DOMImplementation().createDocument(TC6_NS, 'project', null)` unchanged.
In
`@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/path_tree.ts`:
- Around line 506-510: Remove the unnecessary "void toInout" statement at the
end of the function: since toInout is already referenced (e.g., passed into
generateBlock at line ~423) the void suppression is stale; simply delete the
`void toInout` line that follows the `return paths` so the function returns
normally without the unused-expression statement.
In
`@src/backend/shared/transpilers/generate-st-from-json/src/plcopen/accessors.ts`:
- Around line 485-498: The function getdataType uses unsafe casts to obtain root
via (project as ProjectTree).documentElement ?? (project as Element); change it
to use the isDocument helper to determine whether project is a ProjectTree
(document) or an Element like getpous/getconfigurations do, then read
documentElement only when isDocument(project) is true and otherwise treat
project as Element, avoiding direct type assertions; update references inside
getdataType accordingly so ProjectTree.documentElement is accessed only after
the isDocument check.
- Line 129: Update the TypeError message in the throw statement in accessors.ts
that currently reads `${tag} body don't have instances!` to use correct grammar;
change it to `${tag} body doesn't have instances!` (or `${tag} body doesn't have
any instances!`) so the singular "body" pairs with "doesn't"; locate the throw
in the accessors.ts file where `tag` is interpolated and replace the string
accordingly.
- Line 115: The error message thrown by the throw expression that uses the
variable tag is grammatically incorrect; update the TypeError message in the
throw new TypeError(...) expression so it uses "doesn't" (e.g., change "body
don't have instances" to "body doesn't have instances" or "body doesn't have any
instances") while keeping the tag interpolation intact.
In `@src/middleware/shared/ports/compiler-platform-port.ts`:
- Around line 83-101: Remove the stale xml2st-focused docblock that describes
TranspileXmlToStArgs/xml2stArgs and update the surrounding comment area to
reflect the new API: delete the orphaned documentation above TranspileToStArgs
and, if needed, replace it with a short accurate comment describing
TranspileToStArgs and its projectData parameter (or leave no docblock) so the
comment matches the current symbols (TranspileToStArgs, projectData) in
compiler-platform-port.ts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: db5bb891-026d-4ae2-9859-b6065e57f024
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (53)
package.jsonsrc/backend/editor/compiler/compiler-module.tssrc/backend/editor/compiler/desktop-library-build-port.tssrc/backend/editor/compiler/editor-compiler-platform-port.tssrc/backend/shared/compile/pipeline.tssrc/backend/shared/library/build-pipeline.tssrc/backend/shared/library/library-build-orchestrator.tssrc/backend/shared/transpilers/generate-st-from-json/data/std_block_catalog.jsonsrc/backend/shared/transpilers/generate-st-from-json/from-schema.tssrc/backend/shared/transpilers/generate-st-from-json/index.tssrc/backend/shared/transpilers/generate-st-from-json/ir-emit/configuration.tssrc/backend/shared/transpilers/generate-st-from-json/ir-emit/data-types.tssrc/backend/shared/transpilers/generate-st-from-json/ir-emit/pou-graphical.tssrc/backend/shared/transpilers/generate-st-from-json/ir-emit/pou-textual.tssrc/backend/shared/transpilers/generate-st-from-json/ir-emit/type-text.tssrc/backend/shared/transpilers/generate-st-from-json/ir-emit/value.tssrc/backend/shared/transpilers/generate-st-from-json/ir-to-plcopen-dom.tssrc/backend/shared/transpilers/generate-st-from-json/ld/from-xmlbuilder.tssrc/backend/shared/transpilers/generate-st-from-json/ld/modifiers.tssrc/backend/shared/transpilers/generate-st-from-json/ld/path_tree.tssrc/backend/shared/transpilers/generate-st-from-json/ld/types.tssrc/backend/shared/transpilers/generate-st-from-json/ld/walker.tssrc/backend/shared/transpilers/generate-st-from-json/ld/walker_state.tssrc/backend/shared/transpilers/generate-st-from-json/pou-emission-order.tssrc/backend/shared/transpilers/generate-st-from-json/sfc/types.tssrc/backend/shared/transpilers/generate-st-from-json/sfc/walker.tssrc/backend/shared/transpilers/generate-st-from-json/sfc/walker_state.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/block_library.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/body_emit.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/configuration.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/connection_types.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/ctn_globals.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/data_type.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/gen_state.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/generate_data_type.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/graph_primitives.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/interface.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/modifiers.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/path_tree.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/pou_assembly.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/program.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/sfc.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/text_helpers.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/type_hierarchy.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/type_text.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/variable_type.tssrc/backend/shared/transpilers/generate-st-from-json/src/plcopen/accessors.tssrc/backend/shared/transpilers/generate-st-from-json/src/plcopen/plcopen.tssrc/backend/shared/transpilers/generate-st-from-json/src/util/py_compat.tssrc/backend/shared/transpilers/generate-st-from-json/src/xmlclass/xsdschema.tssrc/backend/shared/transpilers/generate-st-from-json/types.tssrc/middleware/shared/ports/compiler-platform-port.tssrc/middleware/shared/ports/library-build-port.ts
💤 Files with no reviewable changes (1)
- src/backend/editor/compiler/compiler-module.ts
Comment on lines
+146
to
+160
| function projectStructureVariableType(type: SchemaStructureVariable['type']): TranspileVariableType { | ||
| if (type.definition === 'array') { | ||
| return { | ||
| definition: 'array', | ||
| data: { | ||
| dimensions: type.data.dimensions.map((d) => ({ dimension: d.dimension })), | ||
| baseType: { value: type.data.baseType.value }, | ||
| }, | ||
| } | ||
| } | ||
| if (type.definition === 'derived' || type.definition === 'user-data-type') { | ||
| return { definition: type.definition, value: type.value } | ||
| } | ||
| return { definition: 'base-type', value: type.value } | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Extract duplicated function logic.
projectStructureVariableType is an exact duplicate of projectVariableType (lines 130-144). This violates DRY principles.
♻️ Proposed fix
function projectStructureVariableType(type: SchemaStructureVariable['type']): TranspileVariableType {
- if (type.definition === 'array') {
- return {
- definition: 'array',
- data: {
- dimensions: type.data.dimensions.map((d) => ({ dimension: d.dimension })),
- baseType: { value: type.data.baseType.value },
- },
- }
- }
- if (type.definition === 'derived' || type.definition === 'user-data-type') {
- return { definition: type.definition, value: type.value }
- }
- return { definition: 'base-type', value: type.value }
+ return projectVariableType(type)
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/shared/transpilers/generate-st-from-json/from-schema.ts` around
lines 146 - 160, The function projectStructureVariableType duplicates
projectVariableType; remove the duplicate by extracting the shared logic into a
single helper (keep projectVariableType as the canonical implementation or
create a new shared function name), replace projectStructureVariableType to call
that helper, and update any callers to use the shared function; ensure the
helper preserves behavior for 'array', 'derived', 'user-data-type', and default
'base-type' branches and references the same property names (type.definition,
type.data.dimensions, type.data.baseType.value, type.value) so behavior remains
identical.
Comment on lines
+85
to
+103
| const resolvedTempVars = walkerState.functionTempVars.map((tv) => { | ||
| if (tv.type !== 'ANY') return tv | ||
| const referenced = project.pous.find((p) => p.name === tv.originBlockTypeName) | ||
| if (referenced && referenced.pouType === 'function' && referenced.interface.returnType) { | ||
| return { ...tv, type: referenced.interface.returnType } | ||
| } | ||
| const stdResolved = resolveBlockType(null, tv.originBlockTypeName) | ||
| if (stdResolved) { | ||
| const outPort = stdResolved.infos.outputs.find( | ||
| (o) => o.name === tv.originFormalParameter, | ||
| ) | ||
| if (outPort) { | ||
| const t = outPort.type | ||
| const collapsed = t.startsWith('ANY') ? 'BOOL' : t | ||
| return { ...tv, type: collapsed } | ||
| } | ||
| } | ||
| return tv | ||
| }) |
Contributor
There was a problem hiding this comment.
Handle unresolved ANY type in function temps.
When type resolution fails for a function temp variable (line 102), it returns tv unchanged with type: 'ANY'. Since 'ANY' is not a valid IEC 61131-3 type, this could cause compilation failures downstream. Consider adding a final fallback to collapse unresolved types to 'BOOL' consistent with the generic ANY_* handling on line 98.
🛡️ Proposed fix to add fallback for unresolved ANY types
}
}
- return tv
+ // Final fallback: collapse unresolved ANY to BOOL
+ return tv.type === 'ANY' ? { ...tv, type: 'BOOL' } : tv
})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/backend/shared/transpilers/generate-st-from-json/ir-emit/pou-graphical.ts`
around lines 85 - 103, resolvedTempVars currently leaves tv.type as 'ANY' when
function temp resolution fails, which can break IEC61131-3 emission; in the
mapping over walkerState.functionTempVars ensure that after trying project.pous
lookup and resolveBlockType you add a final fallback that returns { ...tv, type:
'BOOL' } when tv.type (or the resolved value) is still 'ANY' so unresolved ANYs
are collapsed to 'BOOL' consistently with the existing ANY_* handling in the
outPort branch.
Comment on lines
+82
to
+84
| if (iface.length === 0) { | ||
| throw new Error(`No variable defined in "${pou.name}" POU`) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for POUs that might have empty variable arrays in project files
rg -n -C2 '"variables"\s*:\s*\[\s*\]' --type=jsonRepository: Autonomy-Logic/openplc-editor
Length of output: 55
Fix textual emitter to support IR-valid empty interfaces
src/backend/shared/transpilers/generate-st-from-json/ir-emit/pou-textual.ts throws when iface.length === 0 (“No variable defined in ... POU”). Since types.ts allows an empty interface for POUs with no parameters/locals, the textual emitter should accept an empty interface.variables and emit the POU without VAR declarations instead of failing compilation. A repo-wide search for JSON fixtures containing "variables": [] returned no matches, but that doesn’t remove the IR contract mismatch—empty interfaces can still be produced by the compilation pipeline.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/shared/transpilers/generate-st-from-json/ir-emit/pou-textual.ts`
around lines 82 - 84, The emitter currently throws when iface.length === 0
(throw new Error(`No variable defined in "${pou.name}" POU`)), but the IR
permits empty interface.variables; change the logic in the textual POU emitter
in pou-textual.ts to treat an empty iface as valid: remove the throw and skip
emitting any VAR declaration block when iface.length === 0 so the POU is emitted
without VARs; keep existing behavior for non-empty iface so functions/methods
that iterate over iface or use pou.name remain unchanged.
Comment on lines
+69
to
+74
| if (baseType === 'STRING' && !value.startsWith("'") && !value.endsWith("'")) { | ||
| return `'${value}'` | ||
| } | ||
| if (baseType === 'WSTRING' && !value.startsWith('"') && !value.endsWith('"')) { | ||
| return `"${value}"` | ||
| } |
Contributor
There was a problem hiding this comment.
Fix quote-wrapping logic to handle malformed inputs.
The current logic only wraps values when they have neither a leading nor trailing quote. This means malformed inputs like 'foo or foo' won't be wrapped, producing invalid IEC syntax. The condition should wrap whenever the value is not fully quoted (missing either quote).
🐛 Proposed fix for quote-wrapping logic
- if (baseType === 'STRING' && !value.startsWith("'") && !value.endsWith("'")) {
+ if (baseType === 'STRING' && !(value.startsWith("'") && value.endsWith("'"))) {
return `'${value}'`
}
- if (baseType === 'WSTRING' && !value.startsWith('"') && !value.endsWith('"')) {
+ if (baseType === 'WSTRING' && !(value.startsWith('"') && value.endsWith('"'))) {
return `"${value}"`
}This ensures that any value missing proper quotes (including malformed 'foo or foo') gets wrapped correctly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/shared/transpilers/generate-st-from-json/ir-emit/value.ts` around
lines 69 - 74, The quote-wrapping currently only wraps when both leading and
trailing quotes are absent; update the checks around baseType === 'STRING' and
baseType === 'WSTRING' so they wrap whenever the value is not fully quoted
(i.e., missing either the leading or trailing quote). Concretely, replace the
existing conditions that use && with a check that ensures both start and end
quotes are present (negate that) before returning the wrapped value for the
STRING and WSTRING branches.
Comment on lines
+393
to
+422
| function emitTransitionCondition( | ||
| state: SfcWalkerState, | ||
| transitionId: number, | ||
| condition: SfcTransitionCondition, | ||
| ): void { | ||
| state.program.push(['\n', []]) | ||
| state.currentIndent += ' ' | ||
| if (condition.kind === 'inline') { | ||
| state.program.push([state.currentIndent, []]) | ||
| state.program.push([':= ', []]) | ||
| state.program.push([ | ||
| condition.value, | ||
| [state.tagName, 'transition', transitionId, 'inline'], | ||
| ]) | ||
| state.program.push([';\n', []]) | ||
| } else if (condition.kind === 'reference') { | ||
| const subPou = state.body.transitionSubPous.get(condition.name) | ||
| if (subPou) { | ||
| state.program.push([state.currentIndent, []]) | ||
| state.program.push([ | ||
| subPou.value, | ||
| [state.tagName, 'transition', transitionId, 'reference', condition.name], | ||
| ]) | ||
| // Sub-POU bodies may or may not end with a newline. Add one | ||
| // if missing so END_TRANSITION lands on its own line. | ||
| if (!subPou.value.endsWith('\n')) state.program.push(['\n', []]) | ||
| } | ||
| } | ||
| state.currentIndent = state.currentIndent.slice(0, -2) | ||
| } |
Contributor
There was a problem hiding this comment.
Missing handler for 'connection' transition condition kind.
SfcTransitionCondition defines three variants (inline, reference, connection), but emitTransitionCondition only handles inline and reference. If a transition has condition.kind === 'connection', no condition text is emitted, which would produce invalid ST output.
🐛 Proposed fix to handle 'connection' case
} else if (condition.kind === 'reference') {
const subPou = state.body.transitionSubPous.get(condition.name)
if (subPou) {
state.program.push([state.currentIndent, []])
state.program.push([
subPou.value,
[state.tagName, 'transition', transitionId, 'reference', condition.name],
])
// Sub-POU bodies may or may not end with a newline. Add one
// if missing so END_TRANSITION lands on its own line.
if (!subPou.value.endsWith('\n')) state.program.push(['\n', []])
}
+ } else if (condition.kind === 'connection') {
+ // Connection-based conditions are resolved by the LD/FBD walker
+ // via the connections array. If this walker encounters one,
+ // emit a warning or placeholder.
+ state.warnings.push(
+ `Transition ${transitionId}: connection-based conditions not yet supported in JSON walker`,
+ )
}
state.currentIndent = state.currentIndent.slice(0, -2)
}Alternatively, if connection conditions should flow through a different code path (e.g., the LD walker handles them), add an exhaustive check to make the intent explicit:
} else {
// Exhaustive check: condition.kind === 'connection' handled elsewhere
const _exhaustive: never = condition
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/shared/transpilers/generate-st-from-json/sfc/walker.ts` around
lines 393 - 422, The function emitTransitionCondition currently handles
SfcTransitionCondition kinds 'inline' and 'reference' but ignores 'connection',
producing missing output; update emitTransitionCondition to either (A) handle
condition.kind === 'connection' by emitting the expected connection text into
state.program (respecting state.currentIndent, using state.tagName and
transitionId for logging metadata), or (B) make the intent explicit by adding an
exhaustive else branch that asserts the unreachable case (e.g., const
_exhaustive: never = condition) so it's clear 'connection' is handled elsewhere;
ensure indentation (state.currentIndent) is set/restored and any newline
behavior matches the other branches.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/backend/editor/compiler/editor-compiler-platform-port.ts (1)
157-192:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix lint warnings:
asyncmethods have noawaitexpressions.Both
computeMd5(line 157) andtranspileToSt(line 167) are markedasyncbut perform only synchronous operations. Remove theasynckeyword and returnPromise.resolve(...)explicitly to satisfy the linter while preserving the interface contract.🔧 Proposed fix for computeMd5
- async computeMd5(input: string): Promise<string> { - return createHash('md5').update(input).digest('hex') + computeMd5(input: string): Promise<string> { + return Promise.resolve(createHash('md5').update(input).digest('hex')) },🔧 Proposed fix for transpileToSt
- async transpileToSt(args: TranspileToStArgs, log: PlatformLog): Promise<TranspileToStResult> { + transpileToSt(args: TranspileToStArgs, log: PlatformLog): Promise<TranspileToStResult> { try { const ir = fromSchemaShape(args.projectData as unknown as SchemaProjectData) const result = runJsonTranspiler(ir) // ... existing logic ... - return { ok: true, programSt: result.programSt } + return Promise.resolve({ ok: true, programSt: result.programSt }) } catch (error) { const message = error instanceof Error ? error.message : String(error) log(`generate-st-from-json failed: ${message}`, 'error') - return { ok: false, errors: [{ message, line: 0, column: 0, severity: 'error' }] } + return Promise.resolve({ ok: false, errors: [{ message, line: 0, column: 0, severity: 'error' }] }) } },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/editor/compiler/editor-compiler-platform-port.ts` around lines 157 - 192, Remove the unnecessary async keywords: change computeMd5 and transpileToSt to synchronous functions that return resolved Promises (use Promise.resolve(...)) so their signatures still return Promise<string> and Promise<TranspileToStResult>; specifically, replace the async computeMd5 body with a direct return of Promise.resolve(createHash('md5').update(input).digest('hex')) and replace the async transpileToSt implementation by removing async/await and returning Promise.resolve(...) for each control-flow return (both success { ok: true, programSt: ... } and error branches) while keeping the same logic in fromSchemaShape, runJsonTranspiler, log calls, and the try/catch structure in transpileToSt.
🧹 Nitpick comments (14)
src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/type_hierarchy.ts (1)
73-80: 💤 Low valueConsider adding recursion guard for robustness.
The
isOfTypefunction recursively walks the type hierarchy (line 79) without a depth limit. While the fixedTypeHierarchytable bounds recursion depth in practice (~5 levels), adding an explicit guard would prevent potential issues if the hierarchy is later extended or if user-defined types inadvertently create cycles.Optional recursion guard
-export function isOfType(type: string, reference: string | null): boolean { +export function isOfType(type: string, reference: string | null, depth = 0): boolean { + if (depth > 20) return false // guard against unexpected cycles if (reference === null) return true if (type === reference) return true const parent = TypeHierarchy[type] if (parent === undefined) return false if (parent === null) return false - return isOfType(parent, reference) + return isOfType(parent, reference, depth + 1) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/type_hierarchy.ts` around lines 73 - 80, The isOfType function can recurse indefinitely if the TypeHierarchy contains cycles or is unexpectedly deep; update isOfType to include a recursion guard by tracking depth or visited nodes (e.g., add an optional parameter like depth or a visited Set) and return false (or stop) once a maxDepth is exceeded or a parent already seen; modify the recursive call in isOfType and any callers to pass the guard so TypeHierarchy and isOfType are protected against cycles and unbounded recursion.src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/gen_state.ts (1)
1-164: 💤 Low valueConsider running Prettier to align with project formatting standards.
The coding guidelines specify "no semicolons" for
.tsfiles, but this file uses semicolons throughout. While this may be intentional for ported code from openplc-web, ensuring consistency with the project's Prettier configuration would improve maintainability.As per coding guidelines, follow Prettier formatting: 120 char width, no semicolons, single quotes, trailing commas.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/gen_state.ts` around lines 1 - 164, The file deviates from project Prettier rules (uses semicolons, double quotes in imports, inconsistent trailing commas/line lengths); run the project's Prettier config (or apply its rules) to reformat GenState: remove all semicolons, use single quotes for strings, enforce 120-char wrap, and ensure trailing commas where required; you can verify by reformatting the whole file (interfaces GenState, SfcStepInfos, SfcStepActionInfos, SfcTransitionInfos and functions indentRight, indentLeft, isAlreadyDefined) to match the repo's lint/prettier settings and commit the resulting changes.src/backend/shared/transpilers/generate-st-from-json/sfc/walker.ts (1)
386-390: ⚡ Quick winRemove unnecessary type assertion.
Static analysis correctly flags that
stepNameis already typed asstringfrom theProgramChunktuple destructuring. Theas stringassertion is redundant.Fix
// Recurse into destination steps. for (const toGroup of infos.to) { for (const [stepName] of toGroup) { - computeSfcStep(state, stepName as string) + computeSfcStep(state, stepName) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/sfc/walker.ts` around lines 386 - 390, The loop is using an unnecessary type assertion on stepName; remove the redundant "as string" in the inner loop so it simply calls computeSfcStep(state, stepName). Locate the nested loops iterating over infos.to and the destructuring for [stepName] (in the same block where computeSfcStep is invoked) and update the call to pass stepName without casting.src/backend/shared/library/build-pipeline.ts (1)
7-14: 💤 Low valueOutdated comment references removed XmlGenerator.
The module docstring at lines 7-10 still mentions "runs the canonical XmlGenerator" but XML generation has been removed from this pipeline. Consider updating to reflect the JSON transpiler flow.
Suggested update
- 1. `prepareXmlForLibraryBuild(project, manifest)` — synthesizes - a stub main program / task / instance into a transient - PLCProject (the on-disk project remains untouched) and runs - the canonical XmlGenerator on it. xml2st rejects programless - projects, so the stub is mandatory; + 1. `prepareXmlForLibraryBuild(project, manifest)` — synthesizes + a stub main program / task / instance into a transient + PLCProject (the on-disk project remains untouched) and returns + the stubbed project data for the JSON transpiler. The transpiler + rejects programless projects, so the stub is mandatory;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/library/build-pipeline.ts` around lines 7 - 14, The module docstring for prepareXmlForLibraryBuild and surrounding description still mentions "runs the canonical XmlGenerator" and XML generation (and xml2st) even though the pipeline no longer generates XML; update the comment to reflect the current JSON transpiler flow (e.g., say it synthesizes a stub POU into a transient PLCProject and runs the JSON transpiler), remove or replace references to XmlGenerator and xml2st, and ensure the rationale about the non-empty stub body (LocalVar := 3; and a single INT local) is preserved and linked to the JSON transpiler behavior.src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/program.ts (1)
50-52: 💤 Low valueDuplicate type definitions.
LocationAtom,Location, andProgramChunkare defined identically here and inld/path_tree.ts. Consider consolidating into a shared types module to avoid drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/program.ts` around lines 50 - 52, The three duplicate type definitions (LocationAtom, Location, ProgramChunk) should be consolidated into a single shared type module and the local copies removed; create (or use) a shared types file that exports LocationAtom, Location, and ProgramChunk, update the current file to import those types instead of redefining them, and update any other files that declare the same types to import from that shared module so the single source of truth is used across the codebase.src/backend/shared/compile/pipeline.ts (1)
425-426: 💤 Low valueESLint warning: unsafe assignment of error typed value.
The
as unknown as nevercast triggers@typescript-eslint/no-unsafe-assignment. This pattern appears at lines 426, 463, and 728. Sincerawis typed asneverto indicate an opaque payload, consider either:
- Suppressing with an inline
// eslint-disable-next-linecomment with justification- Typing
rawmore precisely if the downstream consumer actually accesses itExample suppression
- stResult.errors.map((e) => ({ formatted: e.message, raw: e as unknown as never })), + // eslint-disable-next-line `@typescript-eslint/no-unsafe-assignment` -- raw is opaque, typed `never` to prevent access + stResult.errors.map((e) => ({ formatted: e.message, raw: e as unknown as never })),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/compile/pipeline.ts` around lines 425 - 426, The current maps use `raw: e as unknown as never` which triggers `@typescript-eslint/no-unsafe-assignment`; fix by either (A) updating the `raw` field's type (e.g., change its declaration from `never` to `unknown` or `Error`/`any` in the compile error/CompileError type used by emit) and remove the double-cast, or (B) if `raw` must remain opaque, add a single-line eslint suppression with a concise justification above the offending expressions (`stResult.errors.map((e) => ({ formatted: e.message, raw: ... }))` and the similar usages near the `emit` calls) so the linter is satisfied; ensure you update the type referenced by `emit`/CompileError (the `raw` property) or add the `// eslint-disable-next-line `@typescript-eslint/no-unsafe-assignment` -- raw is intentionally opaque` comment at each occurrence (the mappings around `stResult` and the other two locations).src/backend/shared/transpilers/generate-st-from-json/from-schema.ts (1)
12-13: ⚡ Quick winUse path alias
@root/*for imports.These imports should use the path alias instead of relative paths.
♻️ Proposed fix
-import { fbdToXml } from '../../../../frontend/utils/PLC/xml-generator/old-editor/language/fbd-xml' -import { ladderToXml } from '../../../../frontend/utils/PLC/xml-generator/old-editor/language/ladder-xml' +import { fbdToXml } from '`@root/frontend/utils/PLC/xml-generator/old-editor/language/fbd-xml`' +import { ladderToXml } from '`@root/frontend/utils/PLC/xml-generator/old-editor/language/ladder-xml`'As per coding guidelines: "Use path alias
@root/*to reference./src/*"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/from-schema.ts` around lines 12 - 13, Replace the relative imports in from-schema.ts with the project path alias: change the imports that reference '../../../../frontend/utils/PLC/xml-generator/old-editor/language/fbd-xml' and '../../../../frontend/utils/PLC/xml-generator/old-editor/language/ladder-xml' to use the `@root` alias (e.g. import { fbdToXml } from '`@root/frontend/utils/PLC/xml-generator/old-editor/language/fbd-xml`' and import { ladderToXml } from '`@root/frontend/utils/PLC/xml-generator/old-editor/language/ladder-xml`') so fbdToXml and ladderToXml are imported via `@root/`* per coding guidelines.src/backend/shared/transpilers/generate-st-from-json/ir-emit/configuration.ts (2)
172-194: ⚡ Quick winConsider centralizing elementary type list.
The
elementaryTypesset duplicates theBASE_PLC_TYPESconstant inir-to-plcopen-dom.ts(lines 40-62).Extract a shared constant to a common module (e.g., a new
src/backend/shared/transpilers/generate-st-from-json/constants.tsorsrc/backend/shared/transpilers/generate-st-from-json/type-hierarchy.ts) to avoid divergence.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/ir-emit/configuration.ts` around lines 172 - 194, The duplicate elementary type lists (the local Set named elementaryTypes and the BASE_PLC_TYPES constant) should be centralized: create a new shared constant (e.g., export const BASE_PLC_TYPES or ELEMENTARY_PLC_TYPES) in a new module (suggested name: generate-st-from-json/constants.ts or type-hierarchy.ts), replace the local elementaryTypes Set with an import from that module, and update ir-to-plcopen-dom.ts to import the same shared constant so both files reference the single source of truth.
201-220: 💤 Low valueRemove or document unused parameters.
The
_varIndentparameter (line 206) is unused and prefixed with underscore, whilerangeandtagname(lines 218-219) are declared but only voided. This suggests either incomplete implementation or parameters that can be removed.If these parameters are intentionally kept for future IR-path enhancements or to match the DOM-based emitter's signature, add a brief comment explaining why. Otherwise, remove them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/ir-emit/configuration.ts` around lines 201 - 220, The emitGlobalVarList function currently declares an unused parameter _varIndent and creates local identifiers range and tagname only to void them; either remove the unused parameter and the dead declarations or, if they are intentionally preserved for API symmetry/future use, add a short explanatory comment above emitGlobalVarList documenting why _varIndent, range and tagname are kept (e.g., "kept for DOM-emitter signature compatibility / future IR enhancements") and remove the void statements; update the function signature and callers if you remove _varIndent so no unused-parameter warnings remain, or add the comment and keep the current signature to silence reviewer concerns.src/backend/shared/transpilers/generate-st-from-json/ir-emit/type-text.ts (1)
27-30: 💤 Low valueReconsider empty string fallback for unreachable case.
The comment indicates this guard is needed for TypeScript's narrowing, but returning an empty string for an unreachable case could hide bugs. If the type system guarantees this is unreachable, consider throwing an error instead to catch unexpected runtime cases.
🛡️ Proposed defensive alternative
- if (type.definition !== 'array') return '' + if (type.definition !== 'array') { + throw new Error(`Unexpected type definition: ${(type as { definition: string }).definition}`) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/ir-emit/type-text.ts` around lines 27 - 30, The current guard "if (type.definition !== 'array') return ''" in type-text.ts hides unreachable cases by returning an empty string; replace that fallback with a defensive runtime failure (e.g., throw new Error(`Unexpected type.definition: ${type.definition}`) or call an existing assertion helper like assertNever/assertUnreachable) so that unexpected runtime values are surfaced; update the guard around the array-handling branch (the check using type.definition) to throw/assert instead of returning '' and include the offending value in the error message for easier debugging.src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/ctn_globals.ts (1)
205-210: 💤 Low valueConsider removing unnecessary parentheses.
Line 207 has unnecessary parentheses around
project. The expression(project).ownerDocumentcan be simplified toproject.ownerDocument.♻️ Proposed fix
function getOwnerDocument(project: ProjectTree | Element) { if ('createElementNS' in project) return project - const owner = (project).ownerDocument + const owner = project.ownerDocument if (owner) return owner return new DOMImplementation().createDocument(TC6_NS, 'project', null) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/ctn_globals.ts` around lines 205 - 210, In getOwnerDocument, remove the unnecessary parentheses around project when accessing ownerDocument; replace the expression (project).ownerDocument with project.ownerDocument so the function reads more idiomatically and still returns the owner or falls back to new DOMImplementation().createDocument(TC6_NS, 'project', null).src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/data_type.ts (1)
100-107: 💤 Low valueType assertion used to assign
initialfield.The pattern works because all
DataTypeInfosvariants includeinitial: string, but consider constructing the complete object inreadBaseContentby passinginitialEldown and having each branch return the fully-formed descriptor. This would eliminate the mutation and type assertion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/data_type.ts` around lines 100 - 107, The code mutates the object returned by readBaseContent and uses a type assertion to set initial; instead, propagate initialEl into readBaseContent so it returns a fully-formed DataTypeInfos (including initial) and remove the post-return mutation. Update readBaseContent to accept an extra parameter (initialEl or formattedInitial) and have each branch construct and return the complete descriptor (matching DataTypeInfos), call formatInitialValue(getinitialValue(datatype)) before calling readBaseContent or pass getinitialValue(datatype) through for formatting inside readBaseContent, and then remove the lines that cast infos and assign infos.initial in PLCGenerator/data_type.ts.src/backend/shared/transpilers/generate-st-from-json/ld/from-xmlbuilder.ts (1)
87-87: 💤 Low valueConsider cleaner conditional property assignment.
The type-assertion pattern
(result as { prop?: T }).prop = valueworks but can be replaced with a more idiomatic approach. Consider building the object in one expression:const result: LdInstance = { kind: 'coil', localId, position, variable: getTextChild(el, 'variable'), modifier: buildCoilModifier(el), connections: collectConnections(el), ...(eoid !== undefined && { executionOrderId: eoid }), }or constructing conditionally:
const result: LdInstance = eoid !== undefined ? { kind: 'coil', ..., executionOrderId: eoid } : { kind: 'coil', ... }Also applies to: 108-109
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/ld/from-xmlbuilder.ts` at line 87, Replace the ad-hoc assertion-based assignment to result.executionOrderId with an idiomatic conditional object construction: when building the LdInstance (the result variable in from-xmlbuilder.ts) include executionOrderId only if eoid !== undefined (e.g. using object spread ...(eoid !== undefined && { executionOrderId: eoid }) or by choosing between two literal objects), and apply the same change to the other occurrences around lines where executionOrderId is set (the other assignments at the other LdInstance constructions referenced in the review).src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/sfc.ts (1)
361-380: 💤 Low valueCircular dependency handled via dynamic require.
The
require('./program')pattern works but creates a runtime dependency that TypeScript cannot fully type-check. The eslint-disable is appropriate, and the cast totypeof import('./program')provides type safety. This is an acceptable solution for the sfc.ts ↔ program.ts cycle, but consider documenting this pattern in a code comment for future maintainers if not already documented elsewhere.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/sfc.ts` around lines 361 - 380, Add a short explanatory comment above the dynamic require block in sfc.ts describing why require('./program') is used (to break the sfc.ts ↔ program.ts circular dependency), why we cast to typeof import('./program') and disable the eslint rule (to retain TypeScript typing and avoid runtime import cycle), and note that computeProgram is intentionally invoked with the parent GenState so interface mutations and type inference remain shared; reference computeProgram, state.tagName, and state.sfcActions in the comment so future maintainers can locate and understand the rationale.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/backend/editor/compiler/desktop-library-build-port.ts`:
- Around line 75-78: The transpileToSt method is marked async but contains no
await; remove the async keyword and return an explicit Promise so the signature
still satisfies the interface—e.g. in the transpileToSt function (signature
using TranspileToStArgs, TranspileToStResult and log) replace the async
implementation with a synchronous body that computes the result and returns
Promise.resolve(theResult) (or alternatively keep async and return
Promise.resolve(...) to ensure an awaited path), ensuring the returned value
matches TranspileToStResult.
In `@src/backend/shared/library/library-build-orchestrator.ts`:
- Around line 143-150: The call to port.transpileToSt is using an unsafe "as
never" cast on stubbedData; replace that cast with "as unknown" (or better,
update the port.transpileToSt signature/generic so projectData accepts the
correct schema type) and keep the same emit callback; locate the call site using
the symbols stubbedData and port.transpileToSt and remove the never assertion,
using unknown or the proper typed union/generic to preserve TypeScript
strictness.
In `@src/backend/shared/transpilers/generate-st-from-json/from-schema.ts`:
- Around line 205-207: The function stringifyReturnType currently defaults to
'BOOL' when returnType isn't a string; instead, detect non-string/undefined
inputs in stringifyReturnType and either throw an Error or emit a warning (use
the project logging facility) with contextual info (e.g., the offending value
and the function name/PLCFunctionSchema reference) so malformed upstream data
isn't silently masked; update callers of stringifyReturnType if necessary to
handle the thrown error or to surface the warning.
In `@src/backend/shared/transpilers/generate-st-from-json/ir-emit/data-types.ts`:
- Around line 91-98: The code assumes dimension.dimension contains '..' and does
const [lower, upper] = dimension.dimension.split('..'), which can leave upper
undefined; update the logic in the dt.dimensions loop (the block using
dimension.dimension, dt.dimensions.forEach, tagname, and the pushed range
entries) to validate the format before emitting: if dimension.dimension does not
include '..' or split yields !== 2 parts, either (a) throw or log a clear error
including tagname and the dimension index i and skip emitting that range, or (b)
provide a safe fallback (e.g., set upper = lower or another explicit sentinel)
so you never push an undefined upper into the chunks; ensure the chosen behavior
is consistent with surrounding error handling and includes contextual info for
debugging.
In
`@src/backend/shared/transpilers/generate-st-from-json/ir-emit/pou-graphical.ts`:
- Around line 12-27: The imports in pou-graphical.ts are not sorted per the
simple-import-sort rule; reorder the import statements alphabetically (or run
npm run lint -- --fix) so that modules like computePouName, computeValue,
declaredTypeName, emitLdBody, getTypeAsText, newWalkerState, PLC_BASE_TYPES,
resolveBlockType, varTypeNames and the Transpile* type imports appear in the
correct sorted order; ensure the relative and absolute import groups remain
separated according to the project's simple-import-sort configuration and re-run
the linter to confirm the violation is resolved.
In `@src/backend/shared/transpilers/generate-st-from-json/ir-to-plcopen-dom.ts`:
- Around line 21-22: Combine and sort the two imports from '`@xmldom/xmldom`' into
a single sorted import to satisfy the simple-import-sort rule: replace the
separate lines importing DOMImplementation and the type imports Document and
Element with one import that exports DOMImplementation and the type-only symbols
(e.g., DOMImplementation, type Document, type Element) from '`@xmldom/xmldom`', or
simply run ESLint autofix for the simple-import-sort plugin to merge and sort
them automatically.
In `@src/backend/shared/transpilers/generate-st-from-json/ld/modifiers.ts`:
- Around line 18-20: The imports in modifiers.ts are not alphabetically sorted,
causing the simple-import-sort/imports ESLint rule to fail; reorder the import
statements so they are alphabetically sorted (e.g., ensure the import lines for
Location, ProgramChunk, ContactModifier, CoilModifier, and WalkerState are in
alphabetical order by module path or symbol per your project's import-sorting
convention) or run npm run lint -- --fix to auto-fix them; update the import
block containing Location, ProgramChunk, ContactModifier, CoilModifier, and
WalkerState accordingly to satisfy the linter.
In `@src/backend/shared/transpilers/generate-st-from-json/ld/walker_state.ts`:
- Around line 9-10: The imports in walker_state.ts are unsorted and violate
simple-import-sort; reorder the import statements alphabetically (e.g., ensure
"import type { LdBody, LdInstance } from './types'" and "import type {
ProgramChunk } from './path_tree'" are sorted) or run the auto-fixer with npm
run lint -- --fix so the simple-import-sort/imports rule passes; update the file
to maintain the required import order for LdBody, LdInstance, and ProgramChunk.
In `@src/backend/shared/transpilers/generate-st-from-json/ld/walker.ts`:
- Around line 25-36: The imports in walker.ts violate the
simple-import-sort/imports rule; reorder the import statements alphabetically
(grouping side-effect, external, and internal imports per project convention) so
the named imports like computePaths, extractModifier, factorizePaths, leafNode,
Location, PathNode, ProgramChunk, TRUE_NODE, Connection, LdInstance and
WalkerState appear in sorted order; update the import block accordingly and
verify by running npm run lint -- --fix (or run the simple-import-sort auto-fix)
to ensure the rule passes CI.
In
`@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/pou_assembly.ts`:
- Around line 51-82: The getBaseType function can recurse indefinitely; add a
recursion guard (either a maxDepth parameter with default like 50 or a visited
set) to stop traversal and return the current typename (or null) when the limit
is reached to avoid stack overflow; update getBaseType signature to accept the
guard (e.g., add optional depth: number = 0 or visited: Set<string>) and
increment/check it before calling getBaseType(project, baseType), and ensure
callers (if any) continue to call the new signature or pass the initial default.
In
`@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/type_text.ts`:
- Around line 77-89: contentToText currently returns elementary type names using
the raw tag (variable tag) while only string/wstring are uppercased, causing
inconsistent casing; update contentToText to return tag.toUpperCase() for the
default elementary-type branch and ensure derived handling (getname(content))
remains unchanged, and apply the same uppercase normalization inside arrayToText
so any returned primitive element type uses tag.toUpperCase() instead of the raw
tag; use getLocalTag(content) to read the tag and preserve existing behavior for
'derived' (getname) and 'array' (arrayToText) branches while normalizing casing
for all other primitive tags.
- Around line 77-80: The function contentToText currently returns an empty
string when a derived element lacks a name; change contentToText's return type
from string to string | null, and when tag === 'derived' return null if
getname(content) is falsy instead of ''. Update callers (notably gettypeAsText)
to handle a null return path and propagate or handle the invalid type
appropriately so missing derived names are treated as errors rather than empty
strings; references: contentToText, getLocalTag, getname, gettypeAsText.
In
`@src/backend/shared/transpilers/generate-st-from-json/src/plcopen/accessors.ts`:
- Around line 110-135: The TypeError messages in getcontentInstances and
getcontentInstance use incorrect grammar ("don't have"); update the thrown
messages in both functions (in getcontentInstances and getcontentInstance) to
read "doesn't have instances!" so the message becomes `${tag} body doesn't have
instances!`, keeping the same interpolation and TypeError usage.
---
Outside diff comments:
In `@src/backend/editor/compiler/editor-compiler-platform-port.ts`:
- Around line 157-192: Remove the unnecessary async keywords: change computeMd5
and transpileToSt to synchronous functions that return resolved Promises (use
Promise.resolve(...)) so their signatures still return Promise<string> and
Promise<TranspileToStResult>; specifically, replace the async computeMd5 body
with a direct return of
Promise.resolve(createHash('md5').update(input).digest('hex')) and replace the
async transpileToSt implementation by removing async/await and returning
Promise.resolve(...) for each control-flow return (both success { ok: true,
programSt: ... } and error branches) while keeping the same logic in
fromSchemaShape, runJsonTranspiler, log calls, and the try/catch structure in
transpileToSt.
---
Nitpick comments:
In `@src/backend/shared/compile/pipeline.ts`:
- Around line 425-426: The current maps use `raw: e as unknown as never` which
triggers `@typescript-eslint/no-unsafe-assignment`; fix by either (A) updating the
`raw` field's type (e.g., change its declaration from `never` to `unknown` or
`Error`/`any` in the compile error/CompileError type used by emit) and remove
the double-cast, or (B) if `raw` must remain opaque, add a single-line eslint
suppression with a concise justification above the offending expressions
(`stResult.errors.map((e) => ({ formatted: e.message, raw: ... }))` and the
similar usages near the `emit` calls) so the linter is satisfied; ensure you
update the type referenced by `emit`/CompileError (the `raw` property) or add
the `// eslint-disable-next-line `@typescript-eslint/no-unsafe-assignment` -- raw
is intentionally opaque` comment at each occurrence (the mappings around
`stResult` and the other two locations).
In `@src/backend/shared/library/build-pipeline.ts`:
- Around line 7-14: The module docstring for prepareXmlForLibraryBuild and
surrounding description still mentions "runs the canonical XmlGenerator" and XML
generation (and xml2st) even though the pipeline no longer generates XML; update
the comment to reflect the current JSON transpiler flow (e.g., say it
synthesizes a stub POU into a transient PLCProject and runs the JSON
transpiler), remove or replace references to XmlGenerator and xml2st, and ensure
the rationale about the non-empty stub body (LocalVar := 3; and a single INT
local) is preserved and linked to the JSON transpiler behavior.
In `@src/backend/shared/transpilers/generate-st-from-json/from-schema.ts`:
- Around line 12-13: Replace the relative imports in from-schema.ts with the
project path alias: change the imports that reference
'../../../../frontend/utils/PLC/xml-generator/old-editor/language/fbd-xml' and
'../../../../frontend/utils/PLC/xml-generator/old-editor/language/ladder-xml' to
use the `@root` alias (e.g. import { fbdToXml } from
'`@root/frontend/utils/PLC/xml-generator/old-editor/language/fbd-xml`' and import
{ ladderToXml } from
'`@root/frontend/utils/PLC/xml-generator/old-editor/language/ladder-xml`') so
fbdToXml and ladderToXml are imported via `@root/`* per coding guidelines.
In
`@src/backend/shared/transpilers/generate-st-from-json/ir-emit/configuration.ts`:
- Around line 172-194: The duplicate elementary type lists (the local Set named
elementaryTypes and the BASE_PLC_TYPES constant) should be centralized: create a
new shared constant (e.g., export const BASE_PLC_TYPES or ELEMENTARY_PLC_TYPES)
in a new module (suggested name: generate-st-from-json/constants.ts or
type-hierarchy.ts), replace the local elementaryTypes Set with an import from
that module, and update ir-to-plcopen-dom.ts to import the same shared constant
so both files reference the single source of truth.
- Around line 201-220: The emitGlobalVarList function currently declares an
unused parameter _varIndent and creates local identifiers range and tagname only
to void them; either remove the unused parameter and the dead declarations or,
if they are intentionally preserved for API symmetry/future use, add a short
explanatory comment above emitGlobalVarList documenting why _varIndent, range
and tagname are kept (e.g., "kept for DOM-emitter signature compatibility /
future IR enhancements") and remove the void statements; update the function
signature and callers if you remove _varIndent so no unused-parameter warnings
remain, or add the comment and keep the current signature to silence reviewer
concerns.
In `@src/backend/shared/transpilers/generate-st-from-json/ir-emit/type-text.ts`:
- Around line 27-30: The current guard "if (type.definition !== 'array') return
''" in type-text.ts hides unreachable cases by returning an empty string;
replace that fallback with a defensive runtime failure (e.g., throw new
Error(`Unexpected type.definition: ${type.definition}`) or call an existing
assertion helper like assertNever/assertUnreachable) so that unexpected runtime
values are surfaced; update the guard around the array-handling branch (the
check using type.definition) to throw/assert instead of returning '' and include
the offending value in the error message for easier debugging.
In `@src/backend/shared/transpilers/generate-st-from-json/ld/from-xmlbuilder.ts`:
- Line 87: Replace the ad-hoc assertion-based assignment to
result.executionOrderId with an idiomatic conditional object construction: when
building the LdInstance (the result variable in from-xmlbuilder.ts) include
executionOrderId only if eoid !== undefined (e.g. using object spread ...(eoid
!== undefined && { executionOrderId: eoid }) or by choosing between two literal
objects), and apply the same change to the other occurrences around lines where
executionOrderId is set (the other assignments at the other LdInstance
constructions referenced in the review).
In `@src/backend/shared/transpilers/generate-st-from-json/sfc/walker.ts`:
- Around line 386-390: The loop is using an unnecessary type assertion on
stepName; remove the redundant "as string" in the inner loop so it simply calls
computeSfcStep(state, stepName). Locate the nested loops iterating over infos.to
and the destructuring for [stepName] (in the same block where computeSfcStep is
invoked) and update the call to pass stepName without casting.
In
`@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/ctn_globals.ts`:
- Around line 205-210: In getOwnerDocument, remove the unnecessary parentheses
around project when accessing ownerDocument; replace the expression
(project).ownerDocument with project.ownerDocument so the function reads more
idiomatically and still returns the owner or falls back to new
DOMImplementation().createDocument(TC6_NS, 'project', null).
In
`@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/data_type.ts`:
- Around line 100-107: The code mutates the object returned by readBaseContent
and uses a type assertion to set initial; instead, propagate initialEl into
readBaseContent so it returns a fully-formed DataTypeInfos (including initial)
and remove the post-return mutation. Update readBaseContent to accept an extra
parameter (initialEl or formattedInitial) and have each branch construct and
return the complete descriptor (matching DataTypeInfos), call
formatInitialValue(getinitialValue(datatype)) before calling readBaseContent or
pass getinitialValue(datatype) through for formatting inside readBaseContent,
and then remove the lines that cast infos and assign infos.initial in
PLCGenerator/data_type.ts.
In
`@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/gen_state.ts`:
- Around line 1-164: The file deviates from project Prettier rules (uses
semicolons, double quotes in imports, inconsistent trailing commas/line
lengths); run the project's Prettier config (or apply its rules) to reformat
GenState: remove all semicolons, use single quotes for strings, enforce 120-char
wrap, and ensure trailing commas where required; you can verify by reformatting
the whole file (interfaces GenState, SfcStepInfos, SfcStepActionInfos,
SfcTransitionInfos and functions indentRight, indentLeft, isAlreadyDefined) to
match the repo's lint/prettier settings and commit the resulting changes.
In
`@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/program.ts`:
- Around line 50-52: The three duplicate type definitions (LocationAtom,
Location, ProgramChunk) should be consolidated into a single shared type module
and the local copies removed; create (or use) a shared types file that exports
LocationAtom, Location, and ProgramChunk, update the current file to import
those types instead of redefining them, and update any other files that declare
the same types to import from that shared module so the single source of truth
is used across the codebase.
In
`@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/sfc.ts`:
- Around line 361-380: Add a short explanatory comment above the dynamic require
block in sfc.ts describing why require('./program') is used (to break the sfc.ts
↔ program.ts circular dependency), why we cast to typeof import('./program') and
disable the eslint rule (to retain TypeScript typing and avoid runtime import
cycle), and note that computeProgram is intentionally invoked with the parent
GenState so interface mutations and type inference remain shared; reference
computeProgram, state.tagName, and state.sfcActions in the comment so future
maintainers can locate and understand the rationale.
In
`@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/type_hierarchy.ts`:
- Around line 73-80: The isOfType function can recurse indefinitely if the
TypeHierarchy contains cycles or is unexpectedly deep; update isOfType to
include a recursion guard by tracking depth or visited nodes (e.g., add an
optional parameter like depth or a visited Set) and return false (or stop) once
a maxDepth is exceeded or a parent already seen; modify the recursive call in
isOfType and any callers to pass the guard so TypeHierarchy and isOfType are
protected against cycles and unbounded recursion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 970e204d-4451-4c8e-9d02-b4305c29f136
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (53)
package.jsonsrc/backend/editor/compiler/compiler-module.tssrc/backend/editor/compiler/desktop-library-build-port.tssrc/backend/editor/compiler/editor-compiler-platform-port.tssrc/backend/shared/compile/pipeline.tssrc/backend/shared/library/build-pipeline.tssrc/backend/shared/library/library-build-orchestrator.tssrc/backend/shared/transpilers/generate-st-from-json/data/std_block_catalog.jsonsrc/backend/shared/transpilers/generate-st-from-json/from-schema.tssrc/backend/shared/transpilers/generate-st-from-json/index.tssrc/backend/shared/transpilers/generate-st-from-json/ir-emit/configuration.tssrc/backend/shared/transpilers/generate-st-from-json/ir-emit/data-types.tssrc/backend/shared/transpilers/generate-st-from-json/ir-emit/pou-graphical.tssrc/backend/shared/transpilers/generate-st-from-json/ir-emit/pou-textual.tssrc/backend/shared/transpilers/generate-st-from-json/ir-emit/type-text.tssrc/backend/shared/transpilers/generate-st-from-json/ir-emit/value.tssrc/backend/shared/transpilers/generate-st-from-json/ir-to-plcopen-dom.tssrc/backend/shared/transpilers/generate-st-from-json/ld/from-xmlbuilder.tssrc/backend/shared/transpilers/generate-st-from-json/ld/modifiers.tssrc/backend/shared/transpilers/generate-st-from-json/ld/path_tree.tssrc/backend/shared/transpilers/generate-st-from-json/ld/types.tssrc/backend/shared/transpilers/generate-st-from-json/ld/walker.tssrc/backend/shared/transpilers/generate-st-from-json/ld/walker_state.tssrc/backend/shared/transpilers/generate-st-from-json/pou-emission-order.tssrc/backend/shared/transpilers/generate-st-from-json/sfc/types.tssrc/backend/shared/transpilers/generate-st-from-json/sfc/walker.tssrc/backend/shared/transpilers/generate-st-from-json/sfc/walker_state.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/block_library.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/body_emit.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/configuration.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/connection_types.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/ctn_globals.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/data_type.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/gen_state.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/generate_data_type.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/graph_primitives.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/interface.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/modifiers.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/path_tree.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/pou_assembly.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/program.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/sfc.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/text_helpers.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/type_hierarchy.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/type_text.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/variable_type.tssrc/backend/shared/transpilers/generate-st-from-json/src/plcopen/accessors.tssrc/backend/shared/transpilers/generate-st-from-json/src/plcopen/plcopen.tssrc/backend/shared/transpilers/generate-st-from-json/src/util/py_compat.tssrc/backend/shared/transpilers/generate-st-from-json/src/xmlclass/xsdschema.tssrc/backend/shared/transpilers/generate-st-from-json/types.tssrc/middleware/shared/ports/compiler-platform-port.tssrc/middleware/shared/ports/library-build-port.ts
💤 Files with no reviewable changes (1)
- src/backend/editor/compiler/compiler-module.ts
Comment on lines
+75
to
+78
| async transpileToSt( | ||
| args: TranspileToStArgs, | ||
| log: (message: string, level: 'info' | 'warning' | 'error') => void, | ||
| ): Promise<TranspileXmlToStResult> { | ||
| // xml2st takes a file path on stdin, so materialise the | ||
| // in-memory XML to a unique temp file before spawning. | ||
| // Lives in `os.tmpdir()` because the user-visible `plc.xml` | ||
| // is written separately by the orchestrator via | ||
| // writeBuildFile — the intermediate here exists only for the | ||
| // subprocess. | ||
| const sessionDir = path.join(os.tmpdir(), `openplc-lib-xml2st-${randomUUID()}`) | ||
| ): Promise<TranspileToStResult> { |
Contributor
There was a problem hiding this comment.
Fix lint warning: async method has no await expression.
The method is marked async but performs only synchronous operations. Since the interface requires a Promise return, wrap the synchronous path explicitly to satisfy the linter.
🔧 Proposed fix
- async transpileToSt(
- args: TranspileToStArgs,
- log: (message: string, level: 'info' | 'warning' | 'error') => void,
- ): Promise<TranspileToStResult> {
+ transpileToSt(
+ args: TranspileToStArgs,
+ log: (message: string, level: 'info' | 'warning' | 'error') => void,
+ ): Promise<TranspileToStResult> {
try {
// ... existing synchronous logic ...
+ return Promise.resolve({ ok: true, programSt: result.programSt })
- return { ok: true, programSt: result.programSt }
} catch (error) {
const message = error instanceof Error ? error.message : String(error)
log(`transpile-from-json failed: ${message}`, 'error')
+ return Promise.resolve({ ok: false, errors: [{ message, line: 0, column: 0, severity: 'error' }] })
- return { ok: false, errors: [{ message, line: 0, column: 0, severity: 'error' }] }
}
},🧰 Tools
🪛 GitHub Actions: CI / 3_lint _ Lint Check.txt
[warning] 75-75: @typescript-eslint/require-await: Async method 'transpileToSt' has no 'await' expression
🪛 GitHub Actions: CI / lint _ Lint Check
[warning] 75-75: Async method 'transpileToSt' has no 'await' expression @typescript-eslint/require-await
🪛 GitHub Check: lint / Lint Check
[warning] 75-75:
Async method 'transpileToSt' has no 'await' expression
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/editor/compiler/desktop-library-build-port.ts` around lines 75 -
78, The transpileToSt method is marked async but contains no await; remove the
async keyword and return an explicit Promise so the signature still satisfies
the interface—e.g. in the transpileToSt function (signature using
TranspileToStArgs, TranspileToStResult and log) replace the async implementation
with a synchronous body that computes the result and returns
Promise.resolve(theResult) (or alternatively keep async and return
Promise.resolve(...) to ensure an awaited path), ensuring the returned value
matches TranspileToStResult.
Comment on lines
143
to
150
| // `stubbedData` is editor schema-shape; the port's signature is | ||
| // port-shape. Each platform port impl knows the actual shape it | ||
| // receives (desktop → `fromSchemaShape`; web → `fromPortShape` after | ||
| // its adapter converts). See the matching cast site in | ||
| // `pipeline.ts` (Step 1) for the same comment. | ||
| const transpile = await port.transpileToSt( | ||
| { projectData: stubbedData as never }, | ||
| (message, level) => emit({ message, level }), |
Contributor
There was a problem hiding this comment.
Avoid as never cast; use unknown or the actual type.
The as never cast defeats TypeScript's type safety by telling the compiler this code is unreachable (which it isn't). While the comment explains the schema-shape vs port-shape mismatch, never is semantically incorrect here.
Consider using as unknown instead, which accurately represents "we're bypassing type checking here" without the false "unreachable code" claim. Better yet, define a proper type union or use a generic parameter.
Proposed fix using unknown
- const transpile = await port.transpileToSt(
- { projectData: stubbedData as never },
+ const transpile = await port.transpileToSt(
+ { projectData: stubbedData as unknown },
(message, level) => emit({ message, level }),
)As per coding guidelines, maintain TypeScript strict mode and avoid unsafe type assertions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/shared/library/library-build-orchestrator.ts` around lines 143 -
150, The call to port.transpileToSt is using an unsafe "as never" cast on
stubbedData; replace that cast with "as unknown" (or better, update the
port.transpileToSt signature/generic so projectData accepts the correct schema
type) and keep the same emit callback; locate the call site using the symbols
stubbedData and port.transpileToSt and remove the never assertion, using unknown
or the proper typed union/generic to preserve TypeScript strictness.
Comment on lines
+205
to
+207
| function stringifyReturnType(returnType: unknown): string { | ||
| return typeof returnType === 'string' ? returnType : 'BOOL' | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how returnType is typed in the schema
rg -nP --type=ts -C3 'returnType[?:]' -g '**/open-plc.ts' -g '**/types/PLC/*.ts'Repository: Autonomy-Logic/openplc-editor
Length of output: 679
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect baseTypeSchema definition
rg -n "baseTypeSchema" src/backend/shared/types/PLC/open-plc.ts
sed -n '1,260p' src/backend/shared/types/PLC/open-plc.ts | nl -ba | sed -n '1,260p'
# 2) Inspect stringifyReturnType and its usage
sed -n '150,260p' src/backend/shared/transpilers/generate-st-from-json/from-schema.ts | nl -ba
rg -n "stringifyReturnType\\(" -S src/backend/shared/transpilers/generate-st-from-json/from-schema.ts
rg -n "returnType" src/backend/shared/transpilers/generate-st-from-json/from-schema.ts | head -n 200Repository: Autonomy-Logic/openplc-editor
Length of output: 521
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# baseTypeSchema definition file
PLC_SCHEMA_FILE="$(rg -l --fixed-strings "export const baseTypeSchema" -g 'plc-schemas*' -g '*ts' src || true)"
echo "PLC_SCHEMA_FILE=$PLC_SCHEMA_FILE"
if [ -n "${PLC_SCHEMA_FILE:-}" ]; then
echo "---- baseTypeSchema context ----"
sed -n '1,220p' "$PLC_SCHEMA_FILE" | cat -n
else
echo "Could not locate baseTypeSchema export in src."
exit 0
fi
# Inspect stringifyReturnType + surrounding code in from-schema.ts
echo "---- from-schema.ts stringifyReturnType context ----"
sed -n '150,280p' src/backend/shared/transpilers/generate-st-from-json/from-schema.ts | cat -n
# Find stringifyReturnType call sites (and nearby lines)
echo "---- stringifyReturnType call sites ----"
rg -n "stringifyReturnType\\(" src/backend/shared/transpilers/generate-st-from-json/from-schema.ts
rg -n "returnType" src/backend/shared/transpilers/generate-st-from-json/from-schema.ts | head -n 200Repository: Autonomy-Logic/openplc-editor
Length of output: 137
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate where baseTypeSchema is actually defined/exported
rg -n --hidden --glob='*.ts' "export (const|let) baseTypeSchema" src || true
rg -n --hidden --glob='*.ts' "baseTypeSchema" src/backend/shared/transpilers/generate-st-from-json/from-schema.ts || true
rg -n --hidden --glob='*.ts' "baseTypeSchema" src/middleware/shared/ports || true
# Print the plc-schemas file that exports baseTypeSchema (best-effort)
PLC_SCHEMAS_FILE="$(rg -l --hidden --glob='*.ts' "export (const|let) baseTypeSchema" src/middleware/shared/ports || true)"
echo "PLC_SCHEMAS_FILE=${PLC_SCHEMAS_FILE:-<not-found>}"
if [ -n "${PLC_SCHEMAS_FILE:-}" ]; then
sed -n '1,240p' "$PLC_SCHEMAS_FILE" | cat -n
fi
# Inspect stringifyReturnType fully and nearby code in from-schema.ts
sed -n '160,250p' src/backend/shared/transpilers/generate-st-from-json/from-schema.ts | cat -n
rg -n "stringifyReturnType\\(" -n src/backend/shared/transpilers/generate-st-from-json/from-schema.tsRepository: Autonomy-Logic/openplc-editor
Length of output: 2746
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Show baseTypeEnum + baseTypeSchema implementation
sed -n '1,140p' src/middleware/shared/ports/plc-schemas.ts | cat -n
sed -n '180,260p' src/middleware/shared/ports/plc-schemas.ts | cat -n
# 2) Show from-schema.ts around the first use of stringifyReturnType (near the reported line ~60)
sed -n '1,120p' src/backend/shared/transpilers/generate-st-from-json/from-schema.ts | cat -n
# 3) Show from-schema.ts around the reported second hit (~205) to ensure it's the same function
sed -n '180,240p' src/backend/shared/transpilers/generate-st-from-json/from-schema.ts | cat -n
# 4) Find what pou.data.returnType type/source is in from-schema.ts
rg -n "pou\\.data\\.returnType" src/backend/shared/transpilers/generate-st-from-json/from-schema.ts
rg -n "pou\\.data" src/backend/shared/transpilers/generate-st-from-json/from-schema.ts | head -n 80Repository: Autonomy-Logic/openplc-editor
Length of output: 11257
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect open-plc.ts around PLCFunctionSchema and the overall project schema typing
sed -n '160,240p' src/backend/shared/types/PLC/open-plc.ts | cat -n
rg -n "PLCProjectDataSchema|z\\.object\\(\\{|export type PLCProjectData" src/backend/shared/types/PLC/open-plc.ts
rg -n "type PLCProjectData" src/backend/shared/types/PLC/open-plc.ts
rg -n "returnType:" src/backend/shared/types/PLC/open-plc.ts
# Check how fromSchemaShape is used
rg -n "fromSchemaShape\\(" src/backend | head -n 200
rg -n "fromSchemaShape\\(" src | head -n 200Repository: Autonomy-Logic/openplc-editor
Length of output: 8016
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '40,140p' src/backend/editor/compiler/desktop-library-build-port.ts | cat -n
sed -n '140,240p' src/backend/editor/compiler/editor-compiler-platform-port.ts | cat -n
rg -n "PLCProjectDataSchema|parse\\(|safeParse\\(" src/backend/editor/compiler/desktop-library-build-port.ts src/backend/editor/compiler/editor-compiler-platform-port.ts
rg -n "projectData" src/backend/editor/compiler/desktop-library-build-port.ts src/backend/editor/compiler/editor-compiler-platform-port.ts | head -n 200Repository: Autonomy-Logic/openplc-editor
Length of output: 12236
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find any runtime parsing/validation usage of PLCProjectDataSchema
rg -n "PLCProjectDataSchema\\.(parse|safeParse)" src || true
rg -n "PLCProjectDataSchema" src/backend | head -n 200 || true
rg -n "fromSchemaShape\\(" src/backend | head -n 200 || trueRepository: Autonomy-Logic/openplc-editor
Length of output: 1103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '110,190p' src/backend/shared/project/create-project-files.ts | cat -n
rg -n "PLCProject(Schema|DataSchema)\\.(parse|safeParse)" src || true
rg -n "PLCProjectSchema\\.(parse|safeParse)" src || true
rg -n "parse\\(.*PLCProject" src || true
rg -n "safeParse\\(.*PLCProject" src || trueRepository: Autonomy-Logic/openplc-editor
Length of output: 3344
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect parse-project-files around the PLCProjectSchema.safeParse call
sed -n '300,430p' src/backend/shared/utils/parse-project-files.ts | cat -n
# Find where parse-project-files exports are used
rg -n "parse-project-files|parseProjectFiles|parseProject" src/backend | head -n 200
# Trace what calls createDesktopLibraryBuildPort/editor-compiler-platform-port transpileToSt
rg -n "transpileToSt\\(" src/backend/editor | head -n 200
rg -n "transpileToSt\\(" src/backend | head -n 200
# Look for the shape of args.projectData passed into transpileToSt
rg -n "projectData: " src/backend/editor src/backend/shared src/backend/editor/compiler | head -n 200Repository: Autonomy-Logic/openplc-editor
Length of output: 22868
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate getDefaultSchemaValues implementation
rg -n "function getDefaultSchemaValues|getDefaultSchemaValues\\(" src/backend/shared/utils -S
rg -n "getDefaultSchemaValues" src/backend/shared/utils/parse-project-files.ts src/backend/shared/utils -S
# Inspect parseProjectFiles POU parsing section where PLCFunctionSchema/returnType would be handled
# (search within parse-project-files.ts for "PLCFunctionSchema" and/or "returnType")
rg -n "PLCFunctionSchema|returnType" src/backend/shared/utils/parse-project-files.ts
sed -n '200,330p' src/backend/shared/utils/parse-project-files.ts | cat -n
sed -n '330,540p' src/backend/shared/utils/parse-project-files.ts | cat -n
# Check how compile pipeline passes projectData to transpileToSt
sed -n '360,480p' src/backend/shared/compile/pipeline.ts | cat -n
sed -n '180,320p' src/backend/shared/library/build-pipeline.ts | cat -n
# Inspect generate-st-from-json transpileToSt to see any additional validation/parsing
sed -n '1,220p' src/backend/shared/transpilers/generate-st-from-json/index.ts | cat -nRepository: Autonomy-Logic/openplc-editor
Length of output: 48501
Warn/throw when returnType is missing or non-string instead of defaulting to 'BOOL'.
PLCFunctionSchemadefinesreturnTypeasz.union([baseTypeSchema, z.string()]), so with Zod-validated data it should always be a string; the'BOOL'fallback instringifyReturnTypeis only reachable when upstream data bypasses validation.- Legacy JSON POU parsing assigns
returnTypeviaipcPou.data.returnType as string | undefined(no runtime Zod check), and fallback POU creation setsreturnType: 'BOOL', so malformed/missingreturnTypecan be silently masked by the default.
Emit a warning or fail compilation when stringifyReturnType hits the non-string/undefined branch (around src/backend/shared/transpilers/generate-st-from-json/from-schema.ts:205-207).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/shared/transpilers/generate-st-from-json/from-schema.ts` around
lines 205 - 207, The function stringifyReturnType currently defaults to 'BOOL'
when returnType isn't a string; instead, detect non-string/undefined inputs in
stringifyReturnType and either throw an Error or emit a warning (use the project
logging facility) with contextual info (e.g., the offending value and the
function name/PLCFunctionSchema reference) so malformed upstream data isn't
silently masked; update callers of stringifyReturnType if necessary to handle
the thrown error or to surface the warning.
Comment on lines
+91
to
+98
| dt.dimensions.forEach((dimension, i) => { | ||
| if (i > 0) chunks.push([',', []]) | ||
| const [lower, upper] = dimension.dimension.split('..') | ||
| chunks.push( | ||
| [`${lower}`, [tagname, 'range', i, 'lower']], | ||
| ['..', []], | ||
| [`${upper}`, [tagname, 'range', i, 'upper']], | ||
| ) |
Contributor
There was a problem hiding this comment.
Potential undefined value if dimension format is unexpected.
If dimension.dimension doesn't contain .., the split('..') returns a single-element array, making upper undefined. This would emit malformed ST like 0..undefined.
Consider adding validation or a fallback:
Defensive fix
dt.dimensions.forEach((dimension, i) => {
if (i > 0) chunks.push([',', []])
- const [lower, upper] = dimension.dimension.split('..')
+ const parts = dimension.dimension.split('..')
+ const lower = parts[0] ?? ''
+ const upper = parts[1] ?? parts[0] ?? ''
chunks.push(
[`${lower}`, [tagname, 'range', i, 'lower']],
['..', []],
[`${upper}`, [tagname, 'range', i, 'upper']],
)
})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/shared/transpilers/generate-st-from-json/ir-emit/data-types.ts`
around lines 91 - 98, The code assumes dimension.dimension contains '..' and
does const [lower, upper] = dimension.dimension.split('..'), which can leave
upper undefined; update the logic in the dt.dimensions loop (the block using
dimension.dimension, dt.dimensions.forEach, tagname, and the pushed range
entries) to validate the format before emitting: if dimension.dimension does not
include '..' or split yields !== 2 parts, either (a) throw or log a clear error
including tagname and the dimension index i and skip emitting that range, or (b)
provide a safe fallback (e.g., set upper = lower or another explicit sentinel)
so you never push an undefined upper into the chunks; ensure the chosen behavior
is consistent with surrounding error handling and includes contextual info for
debugging.
…walker CI lint failed with 5 import-sort errors (autofixable) and one unnecessary `as string` cast in `sfc/walker.ts:388` flagged by `@typescript-eslint/no-unnecessary-type-assertion`. Both fixed by `eslint --fix`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…call sites CI's format check flagged 38 transpiler files (carried over from openplc-web with subtly different prettier config) plus three pipeline call sites touched in the previous commit. Pure formatting — no behavioural change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/backend/shared/transpilers/generate-st-from-json/ir-emit/type-text.ts (1)
27-30:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSilent empty-string return for unexpected type definitions.
Line 30 returns an empty string when
type.definitionis not'array'after exhausting the other cases. If a new definition type is added to the IR, this would silently produce invalid output (e.g.,: ;in variable declarations). Consider throwing an error for truly unexpected definitions.🛡️ Proposed fix with exhaustive check
// array — explicit guard so editor's stricter narrowing keeps the // `data` property in scope. Web's tsconfig accepted the fall- // through; editor's doesn't. - if (type.definition !== 'array') return '' + if (type.definition !== 'array') { + throw new Error(`Unexpected type definition "${type.definition}"`) + } const baseName = typeof type.data.baseType === 'string' ? type.data.baseType : type.data.baseType.value🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/ir-emit/type-text.ts` around lines 27 - 30, Replace the silent fallback that returns an empty string when type.definition !== 'array' with an explicit error so unexpected IR kinds fail fast: locate the guard that checks type.definition (the block that currently does "if (type.definition !== 'array') return ''") and change it to throw a descriptive Error including the unexpected type.definition value (and ideally the surrounding type.name or similar context available) so consumers of the emitter cannot produce invalid output silently.
🧹 Nitpick comments (2)
src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/type_text.ts (1)
10-11: ⚡ Quick winUse
@root/*alias for internal imports in TS files.Line 10 and Line 11 use relative imports under
src/*; switch them to the configured alias to match repo conventions and avoid path-fragility over time.Suggested diff
-import { getbaseType, getcontentOfType, getdimension, getlower, getname, gettype, getupper } from '../plcopen/accessors' -import { childElements, type Element, getLocalTag, isElement } from '../xmlclass/xsdschema' +import { getbaseType, getcontentOfType, getdimension, getlower, getname, gettype, getupper } from '`@root/backend/shared/transpilers/generate-st-from-json/src/plcopen/accessors`' +import { childElements, type Element, getLocalTag, isElement } from '`@root/backend/shared/transpilers/generate-st-from-json/src/xmlclass/xsdschema`'As per coding guidelines: "
**/*.{ts,tsx,js,jsx}: Use path alias@root/*to reference./src/*."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/type_text.ts` around lines 10 - 11, Replace the relative imports with the project path alias `@root/`*: change the import that brings in getbaseType, getcontentOfType, getdimension, getlower, getname, gettype, getupper to use the `@root` alias instead of '../plcopen/accessors', and change the import that brings in childElements, Element, getLocalTag, isElement to use `@root` instead of '../xmlclass/xsdschema'; locate these by the referenced symbols (getbaseType, getcontentOfType, getdimension, getlower, getname, gettype, getupper and childElements, Element, getLocalTag, isElement) and update the module specifiers to the configured `@root/`* paths so TypeScript path mapping is used.src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/program.ts (1)
82-85: 💤 Low valueStale docstring: SFC is implemented, not throwing.
The docstring states "For SFC: throws
NotYetImplementedErroruntil Phase 6 lands" but the actual implementation at lines 108-115 callsemitSfcBodywithout throwing. This inconsistency could mislead maintainers.📝 Suggested doc fix
- * For SFC: throws `NotYetImplementedError` until Phase 6 lands. + * For SFC: builds/reuses GenState, runs type inference, and calls `emitSfcBody`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/program.ts` around lines 82 - 85, Update the stale docstring to reflect current behavior: remove the line claiming "For SFC: throws `NotYetImplementedError`" and instead state that SFC is handled by calling emitSfcBody and returns the same single-chunk array shape (e.g., [(reindentedText, [tagName, "body", indent])]) as IL/ST; reference the emitSfcBody call in program.ts to make it clear where the body is emitted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/backend/shared/transpilers/generate-st-from-json/ir-emit/pou-textual.ts`:
- Around line 54-63: The lookup for kindKeyword using pou.pouType can return
undefined and produce "undefined " in output; update the logic around
kindKeyword (the lookup for pou.pouType) to validate the value returned and
handle unexpected cases: either throw a clear error or provide a safe default
string before pushing into program (where ProgramChunk[] is built and
program.push is called), e.g., assert pou.pouType is one of
'program'|'function'|'function-block' or use a default like 'POU' and log/throw
on unknown values so malformed output is prevented.
---
Outside diff comments:
In `@src/backend/shared/transpilers/generate-st-from-json/ir-emit/type-text.ts`:
- Around line 27-30: Replace the silent fallback that returns an empty string
when type.definition !== 'array' with an explicit error so unexpected IR kinds
fail fast: locate the guard that checks type.definition (the block that
currently does "if (type.definition !== 'array') return ''") and change it to
throw a descriptive Error including the unexpected type.definition value (and
ideally the surrounding type.name or similar context available) so consumers of
the emitter cannot produce invalid output silently.
---
Nitpick comments:
In
`@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/program.ts`:
- Around line 82-85: Update the stale docstring to reflect current behavior:
remove the line claiming "For SFC: throws `NotYetImplementedError`" and instead
state that SFC is handled by calling emitSfcBody and returns the same
single-chunk array shape (e.g., [(reindentedText, [tagName, "body", indent])])
as IL/ST; reference the emitSfcBody call in program.ts to make it clear where
the body is emitted.
In
`@src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/type_text.ts`:
- Around line 10-11: Replace the relative imports with the project path alias
`@root/`*: change the import that brings in getbaseType, getcontentOfType,
getdimension, getlower, getname, gettype, getupper to use the `@root` alias
instead of '../plcopen/accessors', and change the import that brings in
childElements, Element, getLocalTag, isElement to use `@root` instead of
'../xmlclass/xsdschema'; locate these by the referenced symbols (getbaseType,
getcontentOfType, getdimension, getlower, getname, gettype, getupper and
childElements, Element, getLocalTag, isElement) and update the module specifiers
to the configured `@root/`* paths so TypeScript path mapping is used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 64d429f8-0db3-438c-88bb-7569ba91f931
📒 Files selected for processing (38)
src/backend/shared/compile/pipeline.tssrc/backend/shared/compile/steps/compose-firmware-bundle.tssrc/backend/shared/library/library-build-orchestrator.tssrc/backend/shared/transpilers/generate-st-from-json/from-schema.tssrc/backend/shared/transpilers/generate-st-from-json/index.tssrc/backend/shared/transpilers/generate-st-from-json/ir-emit/configuration.tssrc/backend/shared/transpilers/generate-st-from-json/ir-emit/data-types.tssrc/backend/shared/transpilers/generate-st-from-json/ir-emit/pou-graphical.tssrc/backend/shared/transpilers/generate-st-from-json/ir-emit/pou-textual.tssrc/backend/shared/transpilers/generate-st-from-json/ir-emit/type-text.tssrc/backend/shared/transpilers/generate-st-from-json/ir-emit/value.tssrc/backend/shared/transpilers/generate-st-from-json/ir-to-plcopen-dom.tssrc/backend/shared/transpilers/generate-st-from-json/ld/from-xmlbuilder.tssrc/backend/shared/transpilers/generate-st-from-json/ld/modifiers.tssrc/backend/shared/transpilers/generate-st-from-json/ld/walker.tssrc/backend/shared/transpilers/generate-st-from-json/ld/walker_state.tssrc/backend/shared/transpilers/generate-st-from-json/pou-emission-order.tssrc/backend/shared/transpilers/generate-st-from-json/sfc/walker.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/block_library.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/body_emit.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/configuration.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/connection_types.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/ctn_globals.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/data_type.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/generate_data_type.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/graph_primitives.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/interface.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/modifiers.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/path_tree.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/pou_assembly.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/program.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/sfc.tssrc/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/type_text.tssrc/backend/shared/transpilers/generate-st-from-json/src/plcopen/accessors.tssrc/backend/shared/transpilers/generate-st-from-json/src/plcopen/plcopen.tssrc/backend/shared/transpilers/generate-st-from-json/src/util/py_compat.tssrc/backend/shared/transpilers/generate-st-from-json/src/xmlclass/xsdschema.tssrc/backend/shared/transpilers/generate-st-from-json/types.ts
✅ Files skipped from review due to trivial changes (2)
- src/backend/shared/compile/steps/compose-firmware-bundle.ts
- src/backend/shared/transpilers/generate-st-from-json/types.ts
🚧 Files skipped from review as they are similar to previous changes (30)
- src/backend/shared/transpilers/generate-st-from-json/ld/walker_state.ts
- src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/data_type.ts
- src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/modifiers.ts
- src/backend/shared/transpilers/generate-st-from-json/ir-emit/value.ts
- src/backend/shared/transpilers/generate-st-from-json/ld/from-xmlbuilder.ts
- src/backend/shared/transpilers/generate-st-from-json/ir-emit/configuration.ts
- src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/graph_primitives.ts
- src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/ctn_globals.ts
- src/backend/shared/transpilers/generate-st-from-json/from-schema.ts
- src/backend/shared/transpilers/generate-st-from-json/ir-emit/data-types.ts
- src/backend/shared/transpilers/generate-st-from-json/src/util/py_compat.ts
- src/backend/shared/transpilers/generate-st-from-json/ir-to-plcopen-dom.ts
- src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/interface.ts
- src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/pou_assembly.ts
- src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/configuration.ts
- src/backend/shared/transpilers/generate-st-from-json/ir-emit/pou-graphical.ts
- src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/generate_data_type.ts
- src/backend/shared/library/library-build-orchestrator.ts
- src/backend/shared/transpilers/generate-st-from-json/index.ts
- src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/path_tree.ts
- src/backend/shared/transpilers/generate-st-from-json/ld/modifiers.ts
- src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/block_library.ts
- src/backend/shared/transpilers/generate-st-from-json/pou-emission-order.ts
- src/backend/shared/transpilers/generate-st-from-json/src/plcopen/plcopen.ts
- src/backend/shared/transpilers/generate-st-from-json/sfc/walker.ts
- src/backend/shared/transpilers/generate-st-from-json/src/xmlclass/xsdschema.ts
- src/backend/shared/transpilers/generate-st-from-json/src/PLCGenerator/connection_types.ts
- src/backend/shared/transpilers/generate-st-from-json/ld/walker.ts
- src/backend/shared/transpilers/generate-st-from-json/src/plcopen/accessors.ts
- src/backend/shared/compile/pipeline.ts
Comment on lines
+54
to
+63
| const kindKeyword = ( | ||
| { | ||
| program: 'PROGRAM', | ||
| function: 'FUNCTION', | ||
| 'function-block': 'FUNCTION_BLOCK', | ||
| } as Record<string, string> | ||
| )[pou.pouType] | ||
|
|
||
| const program: ProgramChunk[] = [] | ||
| program.push([`${kindKeyword} `, []]) |
Contributor
There was a problem hiding this comment.
kindKeyword can be undefined if pouType is unexpected, causing malformed output.
If pou.pouType is not 'program', 'function', or 'function-block', the record lookup returns undefined, and line 63 emits "undefined " into the output. Consider adding a validation guard or using a default/exhaustive check.
🛡️ Proposed fix with validation
- const kindKeyword = (
- {
- program: 'PROGRAM',
- function: 'FUNCTION',
- 'function-block': 'FUNCTION_BLOCK',
- } as Record<string, string>
- )[pou.pouType]
+ const pouTypeKeywords: Record<string, string> = {
+ program: 'PROGRAM',
+ function: 'FUNCTION',
+ 'function-block': 'FUNCTION_BLOCK',
+ }
+ const kindKeyword = pouTypeKeywords[pou.pouType]
+ if (!kindKeyword) {
+ throw new Error(`Unsupported POU type "${pou.pouType}" in "${pou.name}"`)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const kindKeyword = ( | |
| { | |
| program: 'PROGRAM', | |
| function: 'FUNCTION', | |
| 'function-block': 'FUNCTION_BLOCK', | |
| } as Record<string, string> | |
| )[pou.pouType] | |
| const program: ProgramChunk[] = [] | |
| program.push([`${kindKeyword} `, []]) | |
| const pouTypeKeywords: Record<string, string> = { | |
| program: 'PROGRAM', | |
| function: 'FUNCTION', | |
| 'function-block': 'FUNCTION_BLOCK', | |
| } | |
| const kindKeyword = pouTypeKeywords[pou.pouType] | |
| if (!kindKeyword) { | |
| throw new Error(`Unsupported POU type "${pou.pouType}" in "${pou.name}"`) | |
| } | |
| const program: ProgramChunk[] = [] | |
| program.push([`${kindKeyword} `, []]) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/shared/transpilers/generate-st-from-json/ir-emit/pou-textual.ts`
around lines 54 - 63, The lookup for kindKeyword using pou.pouType can return
undefined and produce "undefined " in output; update the logic around
kindKeyword (the lookup for pou.pouType) to validate the value returned and
handle unexpected cases: either throw a clear error or provide a safe default
string before pushing into program (where ProgramChunk[] is built and
program.push is called), e.g., assert pou.pouType is one of
'program'|'function'|'function-block' or use a default like 'POU' and log/throw
on unknown values so malformed output is prevented.
Two byte-identity nits the cross-repo Shared Surface Sync flagged: - `plcopen.ts:normalizePlcOpenXml` used `String.prototype.replaceAll`, which is es2021 and unavailable under web's tsconfig target lib. `split/join` is identical in behaviour for non-regex inputs (`TC6_OLD_NS` has no metacharacters). - `data/std_block_catalog.json` was missing the trailing newline that the web copy has. Both file pairs are now byte-identical across repos. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The shared compile pipeline already routed through the in-process JSON → ST transpiler (`generate-st-from-json/`), but two editor-only paths still spawned the bundled `xml2st` binary: - `CompilerModule.compileForDebugger` ran `handleGenerateXMLfromJSON` + `handleTranspileXMLtoST` instead of calling the JSON transpiler directly. Replaced with the same `fromSchemaShape + runJsonTranspiler` pattern the program-compile platform port uses. - `HardwareModule.getAvailableSerialPorts` shelled out to `xml2st --list-ports` to enumerate USB serial devices. Replaced with `SerialPort.list()` from the `serialport` npm package that was already a project dep (used by `modbus-rtu-client`). With nothing spawning xml2st anymore, the bundle no longer needs to ship it: - `binary-versions.json` no longer pins xml2st. - `scripts/download-binaries.ts` no longer downloads / caches it. - `resources/bin/<platform>/<arch>/xml2st` local copies removed + gitignore entries dropped. Dead-code cleanup: `CompilerModule.handleTranspileXMLtoST`, `#executeXml2st`, `#constructXml2stBinaryPath`, `xml2stBinaryPath` field, `EditorCompilerHandlers.handleTranspileXMLtoST`, and the matching tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two safe, output-neutral cleanups from the CodeRabbit review pass: - `from-schema.ts`: swap the three deep relative imports for the `@root/*` alias (per the project's coding-guidelines convention). - `plcopen/accessors.ts`: fix grammar in the two `body don't have instances` error messages — singular subject, so `doesn't have`. Skipped every CodeRabbit suggestion that would alter what the transpiler emits (regex character-class fix, ANY→BOOL fallback, quote-wrapping logic, kindKeyword default, dimensions fallback, isOfType vs strict equality, etc.) — those need separate review against the golden-fixture corpus. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SFC support is still under development on the JSON-native walker.
The previous fallback ("passthrough as ST body") silently produced
broken output because `body.value` for SFC is React Flow data, not
ST text — strucpp would either reject the bogus payload or compile
into incorrect logic.
Fail-fast with a clear message instead. When the JSON-native SFC
walker lands, this branch goes away.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ia React Flow walker
- Drop the React Flow walker subtree (canonical at xml2st, validated
byte-identical against the python oracle on 169 fixtures) into
src/backend/.../st-transpiler/walker/.
- Rewire from-schema.ts so LD/FBD bodies skip the PLCOpen XML hop
entirely and feed the walker directly. TranspileBody.value now
carries the raw React Flow shape; the walker returns
{ bodySt, syntheticVars }, the wrap composes header + VAR
sections + body + END.
- Merge generate-st-from-json/ and generate-st-from-react-flow/
into one st-transpiler/ folder with index/types/from-schema/
pou-emission-order at the root and emit/, walker/, core/,
helpers/, data/ subfolders.
- Prune ~2000 LOC of dead DOM-walking infrastructure
(src/plcopen/, src/xmlclass/, body_emit/graph_primitives/
pou_assembly/gen_state and other DOM-only PLCGenerator files);
trim surviving helpers to just the live exports.
Collaborator
Author
|
Pushed a follow-up refactor on top of this PR (commit LD/FBD now route through a React Flow walker, not the LdBody DOM IR.
Folder consolidation + dead-code prune.
Validation:
Final tree (~3400 LOC total, down from ~5400 + the ~1200 in ``` |
`asBlockData` only dispatched on `'input'` / `'output'`, dropping the `'inOut'` class entirely. Result: FB calls whose declared signature had VAR_IN_OUT parameters emitted the call without binding them — e.g. `IRRIGATION_MAIN_CONTROLLER0(Moisture := …, T_Max := …)` instead of the oracle's `IRRIGATION_MAIN_CONTROLLER0 (State := State, Moisture := …, T_Max := …)`. Treat `inOut` (also `'inout'`) the same as `input` so the call emission sweeps the inOut formal in source order. Source order puts VAR_IN_OUT before VAR_INPUT, which matches the python oracle's argument-list order for FB calls. Caught by the new `edge__staging__autonomy_factory_fork` fixture (harvested from editor-staging) — `Irrigation_Main_Controller` declares `State : Irrigation_State` as `VAR_IN_OUT`, and `main` calls it as `(State := State, Moisture := Low_moisture_sensor, T_Max := T#5s)`. All 170 walker fixtures green.
Collaborator
Author
|
Pushed one more fix (commit `2b1f26b0a`) — caught by adding a new harvested fixture. Bug: Trigger: A real project ( ``` Our walker was emitting just `IRRIGATION_MAIN_CONTROLLER0(Moisture := …, T_Max := …)`. Fix: Add `else if (cls === 'inOut' || cls === 'inout') inputs.push(name)` in `asBlockData`. Source order in the editor's stored `variant.variables` array puts VAR_IN_OUT before VAR_INPUT, which matches the python oracle's argument-list order. No further changes needed. Walker tests in xml2st: 170/170 green (up from 169/169 with the new fixture). |
eslint --fix on the four st-transpiler files CI flagged for simple-import-sort/imports. No behavioural change.
eslint --fix's import-sort pass collapsed the comma without a space; prettier requires the space. Local lint passed but the CI prettier --check pass caught it.
This was referenced Jun 8, 2026
thiagoralves
added a commit
that referenced
this pull request
Jun 8, 2026
…o concrete output types
Reported by user on the desktop editor compiling a project that
worked on web. STruC++ rejected the program with:
main.st:15:30: error: Undefined type 'ANY' in PROGRAM 'MAIN'
15 | _TMP_TO_UINT874561_OUT : ANY;
`ANY` is a generic placeholder in IEC 61131-3 — valid as a function
argument type but never as a variable declaration type. STruC++ is
correct to reject it. The bug is in the editor's JSON→ST
transpiler (PR #843, recently landed): it was leaving synthetic
output temps for polymorphic IEC type-conversion functions
(`TO_INT`, `TO_UINT`, `TO_REAL`, …) at `ANY` instead of resolving
them to their concrete destination types.
The web editor still compiles this project because it uses xml2st
in a worker (PR #482), not the new JSON→ST transpiler — different
compiler, different code path. Once the web side also migrates to
JSON→ST, this fix needs to be mirrored.
## Root cause
`generateGraphicalPou` (emit/pou-graphical.ts:71) resolves `ANY` in
synthetic vars by:
1. Looking for a user-defined project function with that name →
use its `interface.returnType`.
2. Looking the name up in `data/std_block_catalog.json` via
`resolveBlockType` → use the formal output port's type.
3. (Was missing) Fallback: leave as `ANY`.
The catalog has entries for every *source-typed* conversion
(`BOOL_TO_UINT`, `INT_TO_UINT`, `REAL_TO_UINT`, …) — but NOT for the
polymorphic-input shortcuts (`TO_UINT`, `TO_INT`, …). IEC 61131-3
defines `TO_<TYPE>(in: ANY) → <TYPE>` as a generic conversion whose
output type is fixed by the function name, with the input type
inferred from the connected source. The catalog's enumerate-every-
combination approach doesn't capture this family; resolveBlockType
returns null, the fallback kept `ANY`, strucpp rejected.
## Fix
Adds a third resolution case after the catalog lookup fails: if
`originBlockTypeName` matches `^TO_([A-Z]+)$` and the suffix is one
of the 21 known IEC destination types (BOOL, INT, UINT, REAL, DINT,
LREAL, BYTE, WORD, DWORD, LWORD, SINT, USINT, LINT, ULINT, UDINT,
BCD, TIME, DATE, TOD, DT, STRING), use the suffix as the output
type. Otherwise the existing fallback keeps `ANY` and strucpp
reports the clear "Undefined type" error like before — no regression
on truly-unknown names.
The 21-entry allowlist is hard-coded in pou-graphical.ts with a
comment pinning it to the catalog's `*_TO_<X>` set; a comment notes
that future catalog additions need to be reflected here too. This
is intentionally more visible than deriving the set at runtime —
the allowlist serves as documentation of which polymorphic
conversions the transpiler claims to support.
## Why not a deeper fix
A more principled solution would have the walker carry the block's
own declared output type (which IS present in the project JSON —
the FBD node's `variant.variables` lists OUT as `UINT` for this
exact case) all the way through to synthetic-var emit, replacing
the catalog round-trip entirely. That's a walker-side refactor
(walker/ld.ts:687-700 currently only forwards typeName +
numericId + formal parameter name). Out of scope for a targeted
bug fix.
## Verification
`tsc --noEmit` clean. No existing unit tests in the transpiler
subtree — verification is via the user retesting the project that
hit the bug (`/Users/thiagoralves/Downloads/Blink Demo Multilanguage`,
which has two `TO_UINT` blocks at numericId 874561 and 9788614).
Expected result: ST compiles cleanly, both temps now declared as
`UINT` instead of `ANY`.
## Shared-surface mirror
This file is part of the byte-identical shared surface between
openplc-editor and openplc-web. A matching commit on the web side
is required before either branch is merged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
thiagoralves
added a commit
that referenced
this pull request
Jun 8, 2026
…o concrete output types
Reported by user on the desktop editor compiling a project that
worked on web. STruC++ rejected the program with:
main.st:15:30: error: Undefined type 'ANY' in PROGRAM 'MAIN'
15 | _TMP_TO_UINT874561_OUT : ANY;
`ANY` is a generic placeholder in IEC 61131-3 — valid as a function
argument type but never as a variable declaration type. STruC++ is
correct to reject it. The bug is in the editor's JSON→ST
transpiler (PR #843, recently landed): it was leaving synthetic
output temps for polymorphic IEC type-conversion functions
(`TO_INT`, `TO_UINT`, `TO_REAL`, …) at `ANY` instead of resolving
them to their concrete destination types.
The web editor still compiles this project because it uses xml2st
in a worker (PR #482), not the new JSON→ST transpiler — different
compiler, different code path. Once the web side also migrates to
JSON→ST, this fix needs to be mirrored.
## Root cause
`generateGraphicalPou` (emit/pou-graphical.ts:71) resolves `ANY` in
synthetic vars by:
1. Looking for a user-defined project function with that name →
use its `interface.returnType`.
2. Looking the name up in `data/std_block_catalog.json` via
`resolveBlockType` → use the formal output port's type.
3. (Was missing) Fallback: leave as `ANY`.
The catalog has entries for every *source-typed* conversion
(`BOOL_TO_UINT`, `INT_TO_UINT`, `REAL_TO_UINT`, …) — but NOT for the
polymorphic-input shortcuts (`TO_UINT`, `TO_INT`, …). IEC 61131-3
defines `TO_<TYPE>(in: ANY) → <TYPE>` as a generic conversion whose
output type is fixed by the function name, with the input type
inferred from the connected source. The catalog's enumerate-every-
combination approach doesn't capture this family; resolveBlockType
returns null, the fallback kept `ANY`, strucpp rejected.
## Fix
Adds a third resolution case after the catalog lookup fails: if
`originBlockTypeName` matches `^TO_([A-Z]+)$` and the suffix is one
of the 21 known IEC destination types (BOOL, INT, UINT, REAL, DINT,
LREAL, BYTE, WORD, DWORD, LWORD, SINT, USINT, LINT, ULINT, UDINT,
BCD, TIME, DATE, TOD, DT, STRING), use the suffix as the output
type. Otherwise the existing fallback keeps `ANY` and strucpp
reports the clear "Undefined type" error like before — no regression
on truly-unknown names.
The 21-entry allowlist is hard-coded in pou-graphical.ts with a
comment pinning it to the catalog's `*_TO_<X>` set; a comment notes
that future catalog additions need to be reflected here too. This
is intentionally more visible than deriving the set at runtime —
the allowlist serves as documentation of which polymorphic
conversions the transpiler claims to support.
## Why not a deeper fix
A more principled solution would have the walker carry the block's
own declared output type (which IS present in the project JSON —
the FBD node's `variant.variables` lists OUT as `UINT` for this
exact case) all the way through to synthetic-var emit, replacing
the catalog round-trip entirely. That's a walker-side refactor
(walker/ld.ts:687-700 currently only forwards typeName +
numericId + formal parameter name). Out of scope for a targeted
bug fix.
## Verification
`tsc --noEmit` clean. No existing unit tests in the transpiler
subtree — verification is via the user retesting the project that
hit the bug (`/Users/thiagoralves/Downloads/Blink Demo Multilanguage`,
which has two `TO_UINT` blocks at numericId 874561 and 9788614).
Expected result: ST compiles cleanly, both temps now declared as
`UINT` instead of `ANY`.
## Shared-surface mirror
This file is part of the byte-identical shared surface between
openplc-editor and openplc-web. A matching commit on the web side
is required before either branch is merged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
thiagoralves
added a commit
that referenced
this pull request
Jul 20, 2026
…tion (#947) Restore informative Arduino-IDE-style serial-port names (regressed in 4.2.3, #843): arduino-cli board list identifies boards via VID/PID; serialport supplies the port set + manufacturer fallback; macOS tty. paths canonicalized to cu.. Editor-only, no shared surface changed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
generate-st-from-json/) from openplc-web into the editor's backend and wires it through the sharedCompilerPlatformPort/LibraryBuildPortsurface.xml2stsubprocess hop and theXmlGeneratorpre-step from compile + library-build paths — both now run entirely in-process against the renderer's project IR.transpileXmlToSt→transpileToStacross both ports; editor port impls project the IPC schema-shape payload via the newfromSchemaShapeadapter.Test plan
compileProgramfrom the editor against a small ST project locallyrunLibraryBuildPipeline)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes