fix(rename_object): rewrite \bOldName\b refs in every POU/DUT/GVL by default
Empirical failure: rename_object Application/ST_Sample -> ST_SampleRenamed updated the struct's own TYPE header but Application/PLC_PRG kept 's : ST_Sample;' -- the old name -- breaking the project. Root cause: scriptengine.ScriptObject.rename()/set_name() is a node-local rename only; the IDE's project-wide Rename refactor lives above the scripting layer (no documented find_references() / refactor variant). Fix: after the local rename succeeds, walk every text-bearing object (textual_declaration / textual_implementation), word-boundary regex-replace \bOldName\b -> NewName via a callback (so backslashes in NewName don't get interpreted as backrefs), set_text the changed nodes, save once. New optional updateReferences param defaults to true; pass false for the legacy minimal-rename behaviour. Docs: https://content.helpme-codesys.com/en/ScriptingEngine/ScriptObject.html Risk: false positives in comments / string literals are theoretically possible but rare for IEC identifiers. Documented in the tool description. The target node itself is skipped during the references walk (matched by get_id()) so the rename's already-updated TYPE/FUNCTION_BLOCK/PROGRAM header isn't double-rewritten. Two new vitest e2e checks added: assert UPDATE_REFERENCES=1 renders the re.escape + word-boundary regex, and UPDATE_REFERENCES=0 still produces a fully-substituted script with no leftover placeholders. ### Manual smoke test 1. Open a project with: a DUT 'ST_Sample', a POU 'PLC_PRG' with 'VAR s : ST_Sample; END_VAR', and a third POU referencing 'ST_Sample.foo'. 2. mcp__codesys__rename_object objectPath=Application/ST_Sample newName=ST_SampleRenamed. 3. Expect SCRIPT_SUCCESS with 'References Updated In: 2 node(s)'. 4. mcp__codesys__compile_project should succeed (no unresolved-symbol errors for ST_Sample). 5. With updateReferences=false, the same rename should leave PLC_PRG stale and compile_project should fail -- validates the opt-out. 6. Word-boundary check: rename 'Foo' -> 'Bar' must NOT touch 'FooBar' or 'BarFoo' anywhere.
This commit is contained in:
Karstein Phobic Nyvold Kvistad
parent
57ad449c58
commit
0f8981d6bd
3 changed files with 234 additions and 12 deletions
|
|
@ -1,13 +1,80 @@
|
|||
import sys, scriptengine as script_engine, os, traceback
|
||||
import sys, scriptengine as script_engine, os, traceback, re
|
||||
|
||||
OBJECT_PATH = "{OBJECT_PATH}"
|
||||
NEW_NAME = "{NEW_NAME}"
|
||||
UPDATE_REFERENCES = "{UPDATE_REFERENCES}" == "1"
|
||||
|
||||
# Per OPEN-BUGS-CROSS-REFERENCE Bug 5: CODESYS scripting's rename()/set_name()
|
||||
# is a node-local rename only -- callers in other POUs keep referring to the
|
||||
# old identifier, which is what the IDE's "Rename" command silently fixes
|
||||
# above the scriptengine layer. We brute-force a project-wide identifier
|
||||
# rewrite via word-boundary regex on every POU/DUT/GVL's textual_declaration
|
||||
# and textual_implementation. False positives in comments / string literals
|
||||
# are possible but rare for IEC identifiers; documented in the tool description.
|
||||
#
|
||||
# Docs: https://content.helpme-codesys.com/en/ScriptingEngine/ScriptObject.html
|
||||
# (rename() exists; no documented refactor variant or find_references() in
|
||||
# the public scripting API.)
|
||||
|
||||
|
||||
def _walk_pou_like(node, out):
|
||||
"""Walk every descendant of node and append objects that expose
|
||||
textual_declaration or textual_implementation -- POUs, DUTs, GVLs,
|
||||
methods, properties, action blocks. Skips nodes that don't have
|
||||
text content (folders, devices, application root)."""
|
||||
has_text = False
|
||||
try:
|
||||
if hasattr(node, 'textual_declaration') or hasattr(node, 'textual_implementation'):
|
||||
has_text = True
|
||||
except Exception:
|
||||
pass
|
||||
if has_text:
|
||||
out.append(node)
|
||||
try:
|
||||
for child in node.get_children(False):
|
||||
_walk_pou_like(child, out)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
|
||||
def _read_text(text_obj):
|
||||
"""Return the .text from a textual_declaration / textual_implementation
|
||||
object, or '' if unavailable. Defensive: some SPs raise on access."""
|
||||
if text_obj is None:
|
||||
return ''
|
||||
try:
|
||||
t = text_obj.text
|
||||
except Exception:
|
||||
return ''
|
||||
return t or ''
|
||||
|
||||
|
||||
def _safe_set_text(target_node, attr_name, new_text):
|
||||
"""Replace the text content. Mirrors set_pou_code.py's pattern:
|
||||
target.<attr>.replace(new_text). Returns (ok, error_str)."""
|
||||
try:
|
||||
text_obj = getattr(target_node, attr_name, None)
|
||||
except Exception as e:
|
||||
return False, "getattr %s: %s" % (attr_name, e)
|
||||
if text_obj is None:
|
||||
return False, "%s is None" % attr_name
|
||||
if not hasattr(text_obj, 'replace'):
|
||||
return False, "%s has no replace()" % attr_name
|
||||
try:
|
||||
text_obj.replace(new_text)
|
||||
return True, None
|
||||
except Exception as e:
|
||||
return False, "%s.replace failed: %s" % (attr_name, e)
|
||||
|
||||
|
||||
try:
|
||||
print("DEBUG: rename_object script: ObjectPath='%s', NewName='%s', Project='%s'" % (OBJECT_PATH, NEW_NAME, PROJECT_FILE_PATH))
|
||||
print("DEBUG: rename_object script: ObjectPath='%s', NewName='%s', UpdateReferences=%s, Project='%s'" % (
|
||||
OBJECT_PATH, NEW_NAME, UPDATE_REFERENCES, PROJECT_FILE_PATH))
|
||||
primary_project = ensure_project_open(PROJECT_FILE_PATH)
|
||||
if not OBJECT_PATH: raise ValueError("Object path empty.")
|
||||
if not NEW_NAME: raise ValueError("New name empty.")
|
||||
if not OBJECT_PATH:
|
||||
raise ValueError("Object path empty.")
|
||||
if not NEW_NAME:
|
||||
raise ValueError("New name empty.")
|
||||
|
||||
# Find the target object
|
||||
target_object = find_object_by_path_robust(primary_project, OBJECT_PATH, "target object")
|
||||
|
|
@ -18,7 +85,21 @@ try:
|
|||
target_type = type(target_object).__name__
|
||||
print("DEBUG: Found target object: %s (Type: %s)" % (old_name, target_type))
|
||||
|
||||
# Rename the object
|
||||
# Capture the old identifier BEFORE the rename so we can rewrite
|
||||
# references against it.
|
||||
old_identifier = old_name
|
||||
target_object_id = None
|
||||
try:
|
||||
# Some ScriptObjects expose .get_id(); used to skip the target
|
||||
# itself in the references walk (the rename() updated the
|
||||
# target's textual_declaration's TYPE/PROGRAM/FUNCTION_BLOCK
|
||||
# header in place, so we must NOT regex it again).
|
||||
if hasattr(target_object, 'get_id'):
|
||||
target_object_id = target_object.get_id()
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
# Step 1: rename the target itself (existing behaviour).
|
||||
if hasattr(target_object, 'set_name'):
|
||||
print("DEBUG: Calling set_name('%s') on object '%s'" % (NEW_NAME, old_name))
|
||||
target_object.set_name(NEW_NAME)
|
||||
|
|
@ -28,7 +109,91 @@ try:
|
|||
target_object.rename(NEW_NAME)
|
||||
print("DEBUG: Object renamed.")
|
||||
else:
|
||||
raise TypeError("Object '%s' of type %s does not support set_name() or rename()." % (old_name, target_type))
|
||||
raise TypeError("Object '%s' of type %s does not support set_name() or rename()." % (
|
||||
old_name, target_type))
|
||||
|
||||
# Step 2: optionally rewrite references in every OTHER text-bearing
|
||||
# object. Word-boundary regex so identifiers that share a prefix
|
||||
# (foo vs foobar) don't bleed.
|
||||
refs_updated = []
|
||||
refs_skipped_errors = []
|
||||
if UPDATE_REFERENCES and old_identifier and old_identifier != NEW_NAME:
|
||||
print("DEBUG: Updating references: \\b%s\\b -> %s" % (old_identifier, NEW_NAME))
|
||||
# IronPython 2.7's re module needs the pattern escaped in case the
|
||||
# old name contains regex metacharacters. IEC identifiers are
|
||||
# ASCII-letters/digits/underscore so this is normally a no-op,
|
||||
# but stay defensive.
|
||||
pattern = re.compile(r'\b' + re.escape(old_identifier) + r'\b')
|
||||
# Use a function callback for the replacement -- pattern.sub
|
||||
# treats \1, \g<...> etc in a string replacement, so a NEW_NAME
|
||||
# that happens to contain a backslash would be interpreted as a
|
||||
# backref. Callback form returns the literal string verbatim.
|
||||
def _replace_fn(m):
|
||||
return NEW_NAME
|
||||
|
||||
all_text_nodes = []
|
||||
try:
|
||||
for child in primary_project.get_children(False):
|
||||
_walk_pou_like(child, all_text_nodes)
|
||||
except Exception as walk_err:
|
||||
print("WARN: walking project tree for references failed: %s" % walk_err)
|
||||
|
||||
print("DEBUG: %d text-bearing node(s) to scan for references." % len(all_text_nodes))
|
||||
|
||||
for node in all_text_nodes:
|
||||
# Skip the target object itself -- rename() already updated
|
||||
# its TYPE/FUNCTION_BLOCK header.
|
||||
try:
|
||||
node_id = node.get_id() if hasattr(node, 'get_id') else None
|
||||
except Exception:
|
||||
node_id = None
|
||||
if target_object_id is not None and node_id is not None and node_id == target_object_id:
|
||||
continue
|
||||
|
||||
try:
|
||||
node_name = node.get_name() if hasattr(node, 'get_name') else '?'
|
||||
except Exception:
|
||||
node_name = '?'
|
||||
|
||||
# Read declaration + implementation
|
||||
decl_obj = getattr(node, 'textual_declaration', None) if hasattr(node, 'textual_declaration') else None
|
||||
impl_obj = getattr(node, 'textual_implementation', None) if hasattr(node, 'textual_implementation') else None
|
||||
old_decl = _read_text(decl_obj)
|
||||
old_impl = _read_text(impl_obj)
|
||||
|
||||
new_decl = pattern.sub(_replace_fn, old_decl) if old_decl else old_decl
|
||||
new_impl = pattern.sub(_replace_fn, old_impl) if old_impl else old_impl
|
||||
|
||||
decl_changed = (new_decl != old_decl)
|
||||
impl_changed = (new_impl != old_impl)
|
||||
|
||||
if not (decl_changed or impl_changed):
|
||||
continue
|
||||
|
||||
print("DEBUG: rewriting refs in '%s' (decl_changed=%s, impl_changed=%s)" % (
|
||||
node_name, decl_changed, impl_changed))
|
||||
|
||||
if decl_changed:
|
||||
ok, err = _safe_set_text(node, 'textual_declaration', new_decl)
|
||||
if not ok:
|
||||
refs_skipped_errors.append("%s decl: %s" % (node_name, err))
|
||||
continue
|
||||
if impl_changed:
|
||||
ok, err = _safe_set_text(node, 'textual_implementation', new_impl)
|
||||
if not ok:
|
||||
refs_skipped_errors.append("%s impl: %s" % (node_name, err))
|
||||
continue
|
||||
|
||||
refs_updated.append(node_name)
|
||||
|
||||
print("DEBUG: Updated references in %d node(s); skipped %d on errors." % (
|
||||
len(refs_updated), len(refs_skipped_errors)))
|
||||
for err in refs_skipped_errors:
|
||||
print("WARN: ref-update skipped: %s" % err)
|
||||
elif not UPDATE_REFERENCES:
|
||||
print("DEBUG: UPDATE_REFERENCES=0 -- skipping reference rewrite (caller opted out).")
|
||||
else:
|
||||
print("DEBUG: old==new -- skipping reference rewrite.")
|
||||
|
||||
try:
|
||||
print("DEBUG: Saving Project...")
|
||||
|
|
@ -37,15 +202,25 @@ try:
|
|||
except Exception as save_err:
|
||||
print("ERROR: Failed to save Project after renaming object: %s" % save_err)
|
||||
detailed_error = traceback.format_exc()
|
||||
error_message = "Error saving Project after renaming '%s' to '%s': %s\n%s" % (old_name, NEW_NAME, save_err, detailed_error)
|
||||
print(error_message); print("SCRIPT_ERROR: %s" % error_message); sys.exit(1)
|
||||
error_message = "Error saving Project after renaming '%s' to '%s': %s\n%s" % (
|
||||
old_name, NEW_NAME, save_err, detailed_error)
|
||||
print(error_message)
|
||||
print("SCRIPT_ERROR: %s" % error_message)
|
||||
sys.exit(1)
|
||||
|
||||
print("Object Renamed: '%s' -> '%s'" % (old_name, NEW_NAME))
|
||||
print("Object Type: %s" % target_type)
|
||||
print("Path: %s" % OBJECT_PATH)
|
||||
if UPDATE_REFERENCES:
|
||||
print("References Updated In: %d node(s)" % len(refs_updated))
|
||||
if refs_updated:
|
||||
print("Updated Nodes: %s" % ", ".join(refs_updated))
|
||||
print("SCRIPT_SUCCESS: Object renamed successfully.")
|
||||
sys.exit(0)
|
||||
except Exception as e:
|
||||
detailed_error = traceback.format_exc()
|
||||
error_message = "Error renaming object '%s' in project '%s': %s\n%s" % (OBJECT_PATH, PROJECT_FILE_PATH, e, detailed_error)
|
||||
print(error_message); print("SCRIPT_ERROR: %s" % error_message); sys.exit(1)
|
||||
error_message = "Error renaming object '%s' in project '%s': %s\n%s" % (
|
||||
OBJECT_PATH, PROJECT_FILE_PATH, e, detailed_error)
|
||||
print(error_message)
|
||||
print("SCRIPT_ERROR: %s" % error_message)
|
||||
sys.exit(1)
|
||||
|
|
|
|||
|
|
@ -1401,21 +1401,26 @@ export async function startMcpServer(config: ServerConfig): Promise<void> {
|
|||
|
||||
s.tool(
|
||||
'rename_object',
|
||||
'Renames a project object (POU, DUT, GVL, folder, etc.) in the CODESYS project.',
|
||||
"Renames a project object (POU, DUT, GVL, folder, etc.) in the CODESYS project. By default also updates references in every other POU/DUT/GVL by word-boundary regex (\\bOldName\\b -> NewName) so the rename behaves like the IDE's Rename refactor. Pass updateReferences=false to opt out (keep the legacy minimal-rename behaviour, which leaves callers stale and breaks compilation when renaming a type/FB).",
|
||||
{
|
||||
projectFilePath: z.string().describe("Path to the project file."),
|
||||
objectPath: z.string().describe("Full relative path to the object to rename (e.g., 'Application/MyPOU')."),
|
||||
newName: z.string().describe("New name for the object (must be a valid IEC identifier)."),
|
||||
updateReferences: z.boolean().optional().describe("If true (default), regex-replace \\bOldName\\b -> NewName in every other POU/DUT/GVL declaration AND implementation. False: rename target only -- callers will be stale and the project may stop compiling."),
|
||||
},
|
||||
async (args: { projectFilePath: string; objectPath: string; newName: string }) => {
|
||||
async (args: { projectFilePath: string; objectPath: string; newName: string; updateReferences?: boolean }) => {
|
||||
const escProjPath = resolvePath(args.projectFilePath, workspaceDir);
|
||||
const sanObjPath = sanitizePouPath(args.objectPath);
|
||||
// Default updateReferences=true (the safer behaviour). Caller must
|
||||
// explicitly pass false to disable.
|
||||
const updateRefs = args.updateReferences === false ? false : true;
|
||||
const script = scriptManager.prepareScriptWithHelpers(
|
||||
'rename_object',
|
||||
{
|
||||
PROJECT_FILE_PATH: escProjPath,
|
||||
OBJECT_PATH: sanObjPath,
|
||||
NEW_NAME: args.newName.trim(),
|
||||
UPDATE_REFERENCES: updateRefs ? '1' : '0',
|
||||
},
|
||||
['ensure_project_open', 'find_object_by_path']
|
||||
);
|
||||
|
|
|
|||
|
|
@ -164,6 +164,48 @@ describe('E2E Script Preparation', () => {
|
|||
expect(script).toContain('build()');
|
||||
});
|
||||
|
||||
it('rename_object script renders the references-update branch with UPDATE_REFERENCES=1', () => {
|
||||
// Bug 5: rename_object historically only updated the target's own
|
||||
// decl, not callers in other POUs. The fixed script must:
|
||||
// - default to project-wide identifier rewrite via word-boundary regex
|
||||
// - opt-out via UPDATE_REFERENCES=0
|
||||
// - import re and walk text-bearing nodes
|
||||
const script = mgr.prepareScriptWithHelpers(
|
||||
'rename_object',
|
||||
{
|
||||
PROJECT_FILE_PATH: 'C:\\test.project',
|
||||
OBJECT_PATH: 'Application/ST_Sample',
|
||||
NEW_NAME: 'ST_SampleRenamed',
|
||||
UPDATE_REFERENCES: '1',
|
||||
},
|
||||
['ensure_project_open', 'find_object_by_path']
|
||||
);
|
||||
expect(script).toContain('def find_object_by_path_robust');
|
||||
expect(script).toContain('OBJECT_PATH = "Application/ST_Sample"');
|
||||
expect(script).toContain('NEW_NAME = "ST_SampleRenamed"');
|
||||
expect(script).toContain('UPDATE_REFERENCES = "1" == "1"');
|
||||
expect(script).toContain('import sys, scriptengine as script_engine, os, traceback, re');
|
||||
expect(script).toContain('re.escape(old_identifier)');
|
||||
expect(script).toContain('textual_declaration');
|
||||
expect(script).toContain('textual_implementation');
|
||||
expect(script).not.toMatch(/\{[A-Z_]+\}/);
|
||||
});
|
||||
|
||||
it('rename_object script honours UPDATE_REFERENCES=0 opt-out', () => {
|
||||
const script = mgr.prepareScriptWithHelpers(
|
||||
'rename_object',
|
||||
{
|
||||
PROJECT_FILE_PATH: 'C:\\test.project',
|
||||
OBJECT_PATH: 'Application/Foo',
|
||||
NEW_NAME: 'Bar',
|
||||
UPDATE_REFERENCES: '0',
|
||||
},
|
||||
['ensure_project_open', 'find_object_by_path']
|
||||
);
|
||||
expect(script).toContain('UPDATE_REFERENCES = "0" == "1"');
|
||||
expect(script).not.toMatch(/\{[A-Z_]+\}/);
|
||||
});
|
||||
|
||||
it('all scripts are loadable', () => {
|
||||
const scriptNames = [
|
||||
'check_status', 'compile_project', 'create_method', 'create_pou',
|
||||
|
|
|
|||
Loading…
Reference in a new issue