Skip to content

Commit

Permalink
Move dupl detection logic to CDDL marker
Browse files Browse the repository at this point in the history 
That's a more natural place to put the logic and it simplifies things a bit as
well (no need to deal with additional linking texts).
  • Loading branch information
@tidoust
tidoust committed Jan 17, 2025
1 parent 96c3f60 commit 1771bb9
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 95 deletions.
194 changes: 102 additions & 92 deletions bikeshed/cddl.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,39 @@ class CDDLMarker(cddlparser.ast.Marker):
so that cross-referencing logic may take place.
"""

currentRule: cddlparser.ast.Rule
# Keep pointers on the current rule context to track generic parameters
# that are scoped to it
currentRule: cddlparser.ast.Rule | None
currentParameters: list[str]

# List of all CDDL terms defined so far to track and report duplicates
defined: list

def __init__(self) -> None:
self.currentRule = None
self.currentParameters = []
self.defined = []

def _recordDefinition(self, type: str, name: str, dfnFor: str | None = None) -> bool:
for term in self.defined:
if term["type"] == type and term["name"] == name and term["dfnFor"] == dfnFor:
forText = "" if dfnFor is None else f' defined in type "{dfnFor}"'
m.die(
f"CDDL {type} {name}{forText} creates a duplicate and cannot be referenced.\nPlease create additional CDDL types to disambiguate.",
)
return False
if type != "parameter":
for term in self.defined:
if term["type"] != "parameter" and term["name"] == name and term["dfnFor"] == dfnFor:
forText = "" if dfnFor is None else f' defined in type "{dfnFor}"'
m.warn(
f"CDDL {type} {name}{forText} creates a duplicate with a CDDL {term['type']}.\nLink type needs to be specified to reference the term.\nConsider creating additional CDDL types to disambiguate.",
)
break
term = {"type": type, "name": name, "dfnFor": dfnFor}
self.defined.append(term)
return True

def serializeValue(self, prefix: str, value: str, suffix: str, node: cddlparser.ast.Value) -> str:
name = prefix + value + suffix
if node.type not in {"text", "bytes"}:
Expand All @@ -32,14 +62,17 @@ def serializeValue(self, prefix: str, value: str, suffix: str, node: cddlparser.
if forName is None:
# Cannot easily link member key back to a definition
return name
else:
elif self._recordDefinition("key", value, forName):
# 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("|".join(lts)),
name,
)
else:
# Duplicate found, don't create a dfn
return name
elif isinstance(parent, cddlparser.ast.Operator) and parent.controller == node:
# Probably a ".default" value. It may be possible to link the value
# back to an enumeration but it's equally possible that this is just
Expand All @@ -49,13 +82,16 @@ def serializeValue(self, prefix: str, value: str, suffix: str, node: cddlparser.
forName = self._getFor(node)
if forName is None:
return name
else:
elif self._recordDefinition("value", value, forName):
lts = [value, name]
return '<cddl data-cddl-type="value" data-cddl-for="{}" data-lt="{}">{}</cddl>'.format(
h.escapeAttr(forName),
h.escapeAttr("|".join(lts)),
name,
)
else:
# Duplicate found, don't create a dfn
return name

def serializeName(self, name: str, node: cddlparser.ast.CDDLNode) -> str:
# The node is a Typename. Such a node may appear in a Rule, a Type,
Expand All @@ -73,8 +109,11 @@ def serializeName(self, name: str, node: cddlparser.ast.CDDLNode) -> str:
if parent.assign.type in {cddlparser.Tokens.TCHOICEALT, cddlparser.Tokens.GCHOICEALT}:
# The definition extends a base definition
return '<a data-link-type="cddl" data-link-for="/">{}</a>'.format(name)
else:
elif self._recordDefinition("type", name):
return '<cddl data-cddl-type="type" data-lt="{}">{}</cddl>'.format(h.escapeAttr(name), name)
else:
# Duplicate found, don't create a dfn
return name
elif isinstance(parent, cddlparser.ast.Memberkey):
# Member definition
if not parent.hasColon:
Expand All @@ -95,25 +134,34 @@ def serializeName(self, name: str, node: cddlparser.ast.CDDLNode) -> str:
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("|".join(lts)),
name,
)
if self._recordDefinition("key", lts[0], forName):
return '<cddl data-cddl-type="key" data-cddl-for="{}" data-lt="{}">{}</cddl>'.format(
h.escapeAttr(forName),
h.escapeAttr("|".join(lts)),
name,
)
else:
# Duplicate found, don't create a dfn
return name
elif isinstance(parent, cddlparser.ast.GenericParameters):
typename = parent.parentNode
assert isinstance(typename, cddlparser.ast.Typename)
return '<cddl data-cddl-type="parameter" data-cddl-for="{}" data-lt="{}">{}</cddl>'.format(
h.escapeAttr(typename.name),
h.escapeAttr(name),
name,
)
if self._recordDefinition("parameter", name, typename.name):
return '<cddl data-cddl-type="parameter" data-cddl-for="{}" data-lt="{}">{}</cddl>'.format(
h.escapeAttr(typename.name),
h.escapeAttr(name),
name,
)
else:
# Duplicate found, don't create a dfn
return name
elif name in get_args(cddlparser.ast.PreludeType):
# Do not link types that come from the CDDL prelude
# defined in RFC 8610
return name
elif name in self.currentParameters:
# Name is a reference to a generic parameter
assert self.currentRule is not None
return '<a data-link-type="cddl-parameter" data-link-for="{}">{}</a>'.format(
h.escapeAttr(self.currentRule.name.name),
name,
Expand Down Expand Up @@ -170,39 +218,6 @@ def markupCDDLBlock(pre: t.ElementT, doc: t.SpecT) -> set[t.ElementT]:
"""
localDfns = set()

# 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")
Expand All @@ -211,54 +226,49 @@ def recordDfns(el: t.ElementT) -> bool:
url = None
ref = None
cddlText = None
# 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
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 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)
if ref:
break
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"]
# 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
2 changes: 1 addition & 1 deletion tests/cddl003.console.txt
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
WARNING: CDDL value dupl defined in type "warn" creates a duplicate with another CDDL definition.
WARNING: CDDL value dupl defined in type "warn" creates a duplicate with a CDDL key.
Link type needs to be specified to reference the term.
Consider creating additional CDDL types to disambiguate.
LINE 94: No 'cddl' refs found for '"unique"'.
Expand Down
4 changes: 2 additions & 2 deletions tests/cddl005.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@ <h2 class="no-num no-toc no-ref" id="contents">Table of Contents</h2>
</nav>
<main>
<h2 class="heading settled" data-level="1" id="cddl"><span class="secno">1. </span><span class="content">The CDDL</span><a class="self-link" href="#cddl"></a></h2>
<pre class="cddl highlight def"><dfn class="dfn-paneled" data-dfn-type="cddl-type" data-noexport id="cddl-type-error"><code>error</code></dfn> = [ <dfn class="dfn-paneled" data-dfn-for="error" data-dfn-type="cddl-value" data-lt="dupl|&quot;dupl&quot;" data-noexport id="cddl-value-error-dupl"><code>"dupl"</code></dfn>, <span data-cddl-for="error" data-cddl-type="value">"dupl"</span> ]
<pre class="cddl highlight def"><dfn class="dfn-paneled" data-dfn-type="cddl-type" data-noexport id="cddl-type-error"><code>error</code></dfn> = [ <dfn class="dfn-paneled" data-dfn-for="error" data-dfn-type="cddl-value" data-lt="dupl|&quot;dupl&quot;" data-noexport id="cddl-value-error-dupl"><code>"dupl"</code></dfn>, "dupl" ]
</pre>
<h2 class="heading settled" data-level="2" id="links"><span class="secno">2. </span><span class="content">Linking to the CDDL</span><a class="self-link" href="#links"></a></h2>
<p>The <code class="cddl"><a data-link-type="cddl" href="#cddl-type-error" id="ref-for-cddl-type-error">error</a></code> type defines the "dupl" value twice. Bikeshed dies with an error message, because there is no way to disambiguate the values. If user forces generation, no dfn is created for the second value.</p>
Expand All @@ -615,7 +615,7 @@ <h3 class="no-num no-ref heading settled" id="index-defined-here"><span class="c
<li><a href="#cddl-type-error">error</a><span>, in § 1</span>
</ul>
<h2 class="no-num no-ref heading settled" id="cddl-index"><span class="content">CDDL Index</span><a class="self-link" href="#cddl-index"></a></h2>
<pre class="cddl highlight def"><a href="#cddl-type-error"><code>error</code></a> = [ <a href="#cddl-value-error-dupl"><code>"dupl"</code></a>, <span data-cddl-for="error" data-cddl-type="value">"dupl"</span> ]
<pre class="cddl highlight def"><a href="#cddl-type-error"><code>error</code></a> = [ <a href="#cddl-value-error-dupl"><code>"dupl"</code></a>, "dupl" ]

</pre>
<script>/* Boilerplate: script-dom-helper */
Expand Down

0 comments on commit 1771bb9

Please sign in to comment.