Skip to content

Commit

Permalink
Warn/Report an error about duplicated CDDL keys/values
Browse files Browse the repository at this point in the history 
Code now detects when a CDDL block contains anonymous types that would create
duplicates. It dies when a key or a value is defined twice for the same CDDL
type, preventing any reference to the term. It warns when the duplicate is for
definitions that are not of the same type.

For example, an error is returned for:

```
err = [ "dupl", "dupl" ]
```

... while the following only returns an error because it remains possible to
reference individual terms by making the link type explicit:

```
warn = [ ( dupl: "val" ), "dupl" ]
```
  • Loading branch information
@tidoust
tidoust committed Jan 16, 2025
1 parent 6cee654 commit 16dd69f
Show file tree
Hide file tree
Showing 11 changed files with 1,747 additions and 128 deletions.
145 changes: 91 additions & 54 deletions bikeshed/cddl.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,11 @@ def serializeValue(self, prefix: str, value: str, suffix: str, node: cddlparser.
# Cannot easily link member key back to a definition
return name
else:
# Name of created type should not include the quotes
# Create a key with and without quotes as linking text
lts = [value, name]
return '<cddl data-cddl-type="key" data-cddl-for="{}" data-lt="{}">{}</cddl>'.format(
h.escapeAttr(forName),
h.escapeAttr(value),
h.escapeAttr("|".join(lts)),
name,
)
elif isinstance(parent, cddlparser.ast.Operator) and parent.controller == node:
Expand All @@ -49,9 +50,10 @@ def serializeValue(self, prefix: str, value: str, suffix: str, node: cddlparser.
if forName is None:
return name
else:
lts = [value, name]
return '<cddl data-cddl-type="value" data-cddl-for="{}" data-lt="{}">{}</cddl>'.format(
h.escapeAttr(forName),
h.escapeAttr(name),
h.escapeAttr("|".join(lts)),
name,
)

Expand Down Expand Up @@ -88,9 +90,14 @@ def serializeName(self, name: str, node: cddlparser.ast.CDDLNode) -> str:
# Cannot easily link member key back to a definition
return name
else:
lts = []
if name[0] == '"':
lts = [name[1:-1], name]
else:
lts = [name, '"' + name + '"']
return '<cddl data-cddl-type="key" data-cddl-for="{}" data-lt="{}">{}</cddl>'.format(
h.escapeAttr(forName),
h.escapeAttr(name),
h.escapeAttr("|".join(lts)),
name,
)
elif isinstance(parent, cddlparser.ast.GenericParameters):
Expand Down Expand Up @@ -162,66 +169,96 @@ def markupCDDLBlock(pre: t.ElementT, doc: t.SpecT) -> set[t.ElementT]:
Convert <cddl> blocks into "dfn" or links.
"""
localDfns = set()
forcedDfns = []
for x in (h.treeAttr(pre, "data-dfn-force") or "").split():
x = x.strip()
if x.endswith("<interface>"):
x = x[:-11]
forcedDfns.append(x)

# A CDDL definition may create duplicates, e.g. argh = [ "dupl", "dupl" ]
# To detect these duplicates, let's maintain a list of actual definitions
# contained in CDDL blocks.
cddlDfns = set()

def recordDfns(el: t.ElementT) -> bool:
cddlType = "cddl-" + (el.get("data-cddl-type") or "")
for cddlText in (el.get("data-lt") or "").split("|"):
linkFors = t.cast("list[str|None]", config.splitForValues(el.get("data-cddl-for", ""))) or [None]
for linkFor in linkFors:
dfnText = cddlType + ">" + (linkFor or "") + "/" + cddlText
if dfnText in cddlDfns:
forText = "" if linkFor is None else f' defined in type "{linkFor}"'
m.die(
f"CDDL {cddlType[5:]} {cddlText}{forText} creates a duplicate and cannot be referenced.\nPlease create additional CDDL types to disambiguate.",
)
return False
cddlDfns.add(dfnText)
if cddlType != "cddl-parameter":
warned = False
for cddlText in (el.get("data-lt") or "").split("|"):
linkFors = t.cast("list[str|None]", config.splitForValues(el.get("data-cddl-for", ""))) or [None]
for linkFor in linkFors:
dfnText = (linkFor or "") + "/" + cddlText
if dfnText in cddlDfns and not warned:
warned = True
forText = "" if linkFor is None else f' defined in type "{linkFor}"'
m.warn(
f"CDDL {cddlType[5:]} {cddlText}{forText} creates a duplicate with another CDDL definition.\nLink type needs to be specified to reference the term.\nConsider creating additional CDDL types to disambiguate.",
)
cddlDfns.add(dfnText)
return True

for el in h.findAll("cddl", pre):
# Prefix CDDL types with "cddl-" to avoid confusion with other
# types (notably CSS ones such as "value")
cddlType = "cddl-" + (el.get("data-cddl-type") or "")
assert isinstance(cddlType, str)
url = None
forceDfn = False
ref = None
cddlText = None
for cddlText in (el.get("data-lt") or "").split("|"):
if cddlType == "interface" and cddlText in forcedDfns:
forceDfn = True
linkFors = t.cast("list[str|None]", config.splitForValues(el.get("data-cddl-for", ""))) or [None]
for linkFor in linkFors:
ref = doc.refs.getRef(
cddlType,
cddlText,
linkFor=linkFor,
status="local",
el=el,
error=False,
)
# Record the dfns that the term would create and check whether
# it creates a duplicate. If it does, let's not link the term.
if not recordDfns(el):
el.tag = "span"
else:
for cddlText in (el.get("data-lt") or "").split("|"):
linkFors = t.cast("list[str|None]", config.splitForValues(el.get("data-cddl-for", ""))) or [None]
for linkFor in linkFors:
ref = doc.refs.getRef(
cddlType,
cddlText,
linkFor=linkFor,
status="local",
el=el,
error=True,
)
if ref:
url = ref.url
break
if ref:
url = ref.url
break
if ref:
break
if url is None or forceDfn:
el.tag = "dfn"
el.set("data-dfn-type", cddlType)
del el.attrib["data-cddl-type"]
if el.get("data-cddl-for"):
el.set("data-dfn-for", el.get("data-cddl-for") or "")
del el.attrib["data-cddl-for"]
else:
# Copy over the auto-generated linking text to the manual dfn.
dfn = h.find(url, doc)
# How in the hell does this work, the url is not a selector???
assert dfn is not None
lts = combineCDDLLinkingTexts(el.get("data-lt"), dfn.get("data-lt"))
dfn.set("data-lt", lts)
localDfns.add(dfn)

# Reset the <cddl> element to be a link to the manual dfn.
el.tag = "a"
el.set("data-link-type", cddlType)
el.set("data-lt", cddlText)
del el.attrib["data-cddl-type"]
if el.get("data-cddl-for"):
el.set("data-link-for", el.get("data-cddl-for") or "")
del el.attrib["data-cddl-for"]
if el.get("id"):
# ID was defensively added by the Marker.
del el.attrib["id"]
if url is None:
el.tag = "dfn"
el.set("data-dfn-type", cddlType)
del el.attrib["data-cddl-type"]
if el.get("data-cddl-for"):
el.set("data-dfn-for", el.get("data-cddl-for") or "")
del el.attrib["data-cddl-for"]
else:
# Copy over the auto-generated linking text to the manual dfn.
dfn = h.find(url, doc)
# How in the hell does this work, the url is not a selector???
assert dfn is not None
lts = combineCDDLLinkingTexts(el.get("data-lt"), dfn.get("data-lt"))
dfn.set("data-lt", lts)
localDfns.add(dfn)

# Reset the <cddl> element to be a link to the manual dfn.
el.tag = "a"
el.set("data-link-type", cddlType)
el.set("data-lt", cddlText)
del el.attrib["data-cddl-type"]
if el.get("data-cddl-for"):
el.set("data-link-for", el.get("data-cddl-for") or "")
del el.attrib["data-cddl-for"]
if el.get("id"):
# ID was defensively added by the Marker.
del el.attrib["id"]
return localDfns


Expand Down
7 changes: 0 additions & 7 deletions bikeshed/refs/utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,13 +181,6 @@ def linkTextVariations(str: str, linkType: str | None) -> t.Generator[str, None,
if str[0] != '"':
yield '"' + str + '"'

if config.linkTypeIn(linkType, "cddl-key") or config.linkTypeIn(linkType, "cddl-value"):
# Allow linking to a cddl-key or cddl-value with or without quotes
if str[0] == '"':
yield str[1:-1]
if str[0] != '"':
yield '"' + str + '"'


if t.TYPE_CHECKING:
U = t.TypeVar("U", bound="t.MutableMapping|t.MutableSequence")
Expand Down
Loading

0 comments on commit 16dd69f

Please sign in to comment.