Skip to content

Commit d3b58ff

Browse files
committed
ddsidl: fix dead Python-keyword check in getAllowedName
`keyword.iskeyword(name.lower)` was missing its call parentheses, so the bound-method object `name.lower` was passed to `iskeyword` instead of the lowered string. `iskeyword` always returned False, leaving the Python-keyword arm of the check effectively dead — VSS signal names that are Python keywords but NOT also C/IDL keywords (e.g. 'class', 'def', 'import', 'lambda') were emitted unprefixed and produced syntactically invalid IDL. Compute `name.lower()` once into a local, pass it to `iskeyword` explicitly, and add a regression test in tests/test_ddsidl_naming.py covering both the bug case (Python-only keywords) and the existing behaviour (C/IDL keywords, normal names).
1 parent 1f44f17 commit d3b58ff

2 files changed

Lines changed: 64 additions & 1 deletion

File tree

src/vss_tools/exporters/ddsidl.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,15 @@
146146

147147

148148
def getAllowedName(name):
149-
if name.lower() in c_keywords or name.lower() in idl_keywords or keyword.iskeyword(name.lower):
149+
lower = name.lower()
150+
# `keyword.iskeyword(name.lower)` (without call parens) passed a
151+
# bound-method object to iskeyword instead of the lowered string,
152+
# so iskeyword always returned False. VSS signals named with Python
153+
# keywords that are NOT also C/IDL keywords (e.g. 'class', 'def',
154+
# 'import', 'lambda') were therefore emitted unprefixed and produced
155+
# syntactically invalid IDL. Compute lower once and pass it
156+
# explicitly to iskeyword.
157+
if lower in c_keywords or lower in idl_keywords or keyword.iskeyword(lower):
150158
return "_" + name
151159
else:
152160
return name

tests/test_ddsidl_naming.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
# Copyright (c) 2026 Contributors to COVESA
2+
#
3+
# This program and the accompanying materials are made available under the
4+
# terms of the Mozilla Public License 2.0 which is available at
5+
# https://www.mozilla.org/en-US/MPL/2.0/
6+
#
7+
# SPDX-License-Identifier: MPL-2.0
8+
9+
from vss_tools.exporters.ddsidl import getAllowedName
10+
11+
12+
class TestDdsIdlNaming:
13+
"""Tests for getAllowedName, which prefixes reserved-word identifiers."""
14+
15+
def test_passes_through_normal_names(self):
16+
"""Names that aren't reserved should be returned unchanged."""
17+
assert getAllowedName("speed") == "speed"
18+
assert getAllowedName("Vehicle") == "Vehicle"
19+
assert getAllowedName("Foo123") == "Foo123"
20+
21+
def test_prefixes_python_keywords(self):
22+
"""Python reserved words should get a leading underscore.
23+
24+
Regression test: previously the keyword.iskeyword() call was missing
25+
its parentheses on its argument (`name.lower` instead of
26+
`name.lower()`), so a bound-method object was passed to iskeyword
27+
instead of the lowered string. iskeyword returned False, so VSS
28+
signals named with Python keywords that aren't also C/IDL keywords
29+
(e.g. 'class', 'def', 'import', 'lambda') passed through silently
30+
and ended up as raw identifiers in the generated IDL.
31+
"""
32+
assert getAllowedName("class") == "_class"
33+
assert getAllowedName("def") == "_def"
34+
assert getAllowedName("import") == "_import"
35+
assert getAllowedName("lambda") == "_lambda"
36+
# Case-insensitive: same effect for differently-cased input.
37+
assert getAllowedName("Class") == "_Class"
38+
assert getAllowedName("CLASS") == "_CLASS"
39+
40+
def test_prefixes_idl_keywords(self):
41+
"""IDL reserved words should get a leading underscore."""
42+
assert getAllowedName("interface") == "_interface"
43+
assert getAllowedName("module") == "_module"
44+
assert getAllowedName("attribute") == "_attribute"
45+
46+
def test_prefixes_c_keywords(self):
47+
"""C reserved words should get a leading underscore.
48+
49+
Some of these (`if`, `for`, `while`, `return`) are also Python
50+
keywords, but C-keyword detection caught them already even with
51+
the iskeyword bug.
52+
"""
53+
assert getAllowedName("auto") == "_auto"
54+
assert getAllowedName("typedef") == "_typedef"
55+
assert getAllowedName("sizeof") == "_sizeof"

0 commit comments

Comments
 (0)