Skip to content

Commit 042c853

Browse files
authored
Report a Retired Badge Written Inline, Not Only One Defined (#637)
readme_shield_findings scanned reference definitions alone for a retired badge service, so a repo writing it as an inline image rendered the dead badge and reported nothing. The comment above the loop claimed it was scanned wherever it sits, and the selftest case carried that name while testing the reference form only, so the claim and its evidence agreed with each other and not with the code. shield_endpoints already resolves both forms and its docstring records the same lesson; this was the one site that had not taken it. Three review rounds refined it. The finding text claimed every matching definition renders the badge, which contradicted the comment directly above it, so the wording now follows which of four shapes it is. Attribution moved from the URL to the reference name, because the same endpoint rendered inline leaves a definition unused and reading the URL credited that render to a reference nothing uses. unfenced_text is computed once, so the definitions and the rendered images cannot be read from two views of the document, and the endpoint set is held as a set named by its namespace. Measured against all 22 cataloged READMEs at every round: shield findings stay at 7 and no verdict or wording moves, since no repo carries either shape. Every new selftest case was A/B tested against the pre-fix function rather than reasoned about. Raised by Copilot as a suppressed finding on promotion pull request #635.
1 parent 1e2a4ba commit 042c853

1 file changed

Lines changed: 42 additions & 6 deletions

File tree

‎spec/audit.py‎

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -787,13 +787,32 @@ def readme_shield_findings(text, model, entry):
787787
repo can be measured against.
788788
"""
789789
findings= []
790-
defs= {m.group(1): m.group(2) formin_LINK_DEF.finditer(unfenced_text(text))}
790+
# One unfenced view serves both passes, so the definitions and the rendered images cannot be read from different documents.
791+
unfenced=unfenced_text(text)
792+
defs= {m.group(1): m.group(2) formin_LINK_DEF.finditer(unfenced)}
791793
# A retired badge service is scanned across the whole document rather than per section, since a dead badge is wrong wherever it sits.
792794
# It renders broken rather than absent, which a visitor reads as a failing build rather than as a stale badge.
795+
# Both forms are read, since reading definitions alone made an inline badge invisible rather than wrong, which is the reading shield_endpoints already takes for every other shield.
796+
# A definition is reported even where nothing renders it, because a retired service left in the reference block is removed with the badge rather than after it.
797+
# Which of the four it is decides the wording, since a definition nothing renders is not rendering anything and saying so sends the reader looking for a badge that is not on the page.
798+
# Attribution is by reference name and never by URL: the same endpoint rendered inline leaves this definition unused, so reading the URL alone would credit a render to a reference nothing uses.
799+
# A set rather than the list shield_endpoints returns, since every use here is membership or a difference, and the names say which of the two namespaces each holds.
800+
rendered_urls=set(shield_endpoints(unfenced, defs))
801+
used_refs= {m.group(1) formin_MD_IMAGE_REF.finditer(unfenced)}
793802
fordepinmodel.get("deprecatedShields", []):
803+
defined_urls=set()
794804
forref, urlinsorted(defs.items()):
795805
ifdep["match"] inurl:
796-
findings.append(("LETTER", f"readme: `[{ref}]` renders {dep['label']}, which is retired - {dep['reason']} (spec/readme-structure.md)"))
806+
defined_urls.add(url)
807+
ifrefinused_refs:
808+
verb=f"renders {dep['label']}"
809+
elifurlinrendered_urls:
810+
verb=f"defines {dep['label']} and it is rendered elsewhere"
811+
else:
812+
verb=f"defines {dep['label']} and nothing renders it"
813+
findings.append(("LETTER", f"readme: `[{ref}]` {verb}, which is retired - {dep['reason']} (spec/readme-structure.md)"))
814+
forurlinsorted({uforuinrendered_urlsifdep["match"] inu} -defined_urls):
815+
findings.append(("LETTER", f"readme: an inline image renders {dep['label']}, which is retired - {dep['reason']} (spec/readme-structure.md)"))
797816
targets= {(p.get("target") ifisinstance(p, dict) elsep) forpinentry.get("publish", [])}
798817
secrets=set(entry.get("requiredSecrets", []))
799818
want= []
@@ -1736,6 +1755,17 @@ def _selftest():
17361755
("a fenced badge sample does not satisfy a required shield", conformant.replace("[![GitHub Release][c]][x]\\\n", "```md\n[![GitHub Release][c]][x]\n```\n"), {}, 1),
17371756
("a fenced license shield does not trip the exclusive rule", conformant.replace("## Overview", "```md\n![License][license-shield]\n```\n\n## Overview"), {}, 0),
17381757
("a retired badge service is reported wherever it sits", conformant.replace("[license-shield]: https://img.shields.io/github/license/o/r\n", "[license-shield]: https://img.shields.io/github/license/o/r\n[last-build-shield]: https://byob.yarr.is/o/r/lastbuild\n"), {}, 1),
1758+
# The case above named every placement and read only the reference block, so an inline badge was invisible rather than wrong.
1759+
# That is the reading shield_endpoints already takes for every other shield, and this one had not taken it.
1760+
("a retired badge written inline is reported", conformant.replace("[![Last Commit][b]][x]", "[![Last Commit][b]][x]\\\n![Last Build](https://byob.yarr.is/o/r/lastbuild)"), {}, 1),
1761+
("a retired badge defined and rendered is one finding, not two", conformant.replace("[![Last Commit][b]][x]", "[![Last Commit][b]][x]\\\n![Last Build][last-build-shield]").replace("[license-shield]: https://img.shields.io/github/license/o/r\n", "[license-shield]: https://img.shields.io/github/license/o/r\n[last-build-shield]: https://byob.yarr.is/o/r/lastbuild\n"), {}, 1),
1762+
("a retired badge shown as a fenced sample is markup", conformant.replace("## Overview", "```md\n![Last Build](https://byob.yarr.is/o/r/lastbuild)\n```\n\n## Overview"), {}, 0),
1763+
# The wording follows which of the three shapes it is, since a definition nothing renders is not rendering anything.
1764+
# Saying it renders sends the reader looking for a badge that is not on the page.
1765+
("an unrendered definition says so rather than claiming a render", conformant.replace("[license-shield]: https://img.shields.io/github/license/o/r\n", "[license-shield]: https://img.shields.io/github/license/o/r\n[last-build-shield]: https://byob.yarr.is/o/r/lastbuild\n"), {}, 1, "nothing renders it"),
1766+
("a rendered definition says renders", conformant.replace("[![Last Commit][b]][x]", "[![Last Commit][b]][x]\\\n![Last Build][last-build-shield]").replace("[license-shield]: https://img.shields.io/github/license/o/r\n", "[license-shield]: https://img.shields.io/github/license/o/r\n[last-build-shield]: https://byob.yarr.is/o/r/lastbuild\n"), {}, 1, "renders the byob"),
1767+
# The same endpoint rendered inline leaves this definition unused, so attributing by URL would credit the render to a reference nothing uses.
1768+
("an unused definition beside an inline render is not credited with it", conformant.replace("[![Last Commit][b]][x]", "[![Last Commit][b]][x]\\\n![Last Build](https://byob.yarr.is/o/r/lastbuild)").replace("[license-shield]: https://img.shields.io/github/license/o/r\n", "[license-shield]: https://img.shields.io/github/license/o/r\n[last-build-shield]: https://byob.yarr.is/o/r/lastbuild\n"), {}, 1, "rendered elsewhere"),
17391769
("the pre-release shield is told from the release shield by its query", conformant.replace("?include_prereleases&label=GitHub%20Pre-Release", "?label=Another%20Release"), {}, 1),
17401770
# The license shield is an ordinary member of the base class, addressed to a different section.
17411771
("the license shield in the closing License section", conformant, {}, 0),
@@ -1747,12 +1777,18 @@ def _selftest():
17471777
("inline shields count as present", inline_all.replace("PLACEHOLDER-a", "github/actions/workflow/status/o/r").replace("PLACEHOLDER-b", "github/last-commit/o/r").replace("PLACEHOLDER-c", "github/v/release/o/r").replace("PLACEHOLDER-d", "github/v/release/o/r?include_prereleases").replace("PLACEHOLDER-license-shield", "github/license/o/r"), {}, 0),
17481778
("an inline shield in the wrong section is still exclusive", conformant.replace("## Overview", "![License](https://img.shields.io/github/license/o/r)\n\n## Overview"), {}, 1),
17491779
]
1750-
forlabel, text, ent, wantninshield_cases:
1780+
# A case may carry a fifth element, a substring the finding text must contain.
1781+
# A count alone cannot tell one wording from another, and the wording is the whole subject of some of these cases.
1782+
forcaseinshield_cases:
1783+
label, text, ent, wantn=case[:4]
1784+
want_text=case[4] iflen(case) >4elseNone
17511785
got=readme_shield_findings(text, rm, ent)
1752-
iflen(got) !=wantn:
1786+
good=len(got) ==wantnand (want_textisNoneorany(want_textintfor_, tingot))
1787+
ifnotgood:
17531788
ok=False
1754-
print(f" {'ok 'iflen(got) ==wantnelse'FAIL'} want={wantn} got={len(got)} readme shields: {label}")
1755-
iflen(got) !=wantn:
1789+
shown=f"want={wantn}"ifwant_textisNoneelsef"want={wantn}+'{want_text}'"
1790+
print(f" {'ok 'ifgoodelse'FAIL'}{shown} got={len(got)} readme shields: {label}")
1791+
ifnotgood:
17561792
for_, tingot:
17571793
print(f" {t}")
17581794

0 commit comments

Comments
 (0)