Extend the images' checker to check API docs (Qiskit#2426)

This PR extends the images' checker introduced in
Qiskit#2349 to check the API docs
when the new argument `--apis` is set.

---------

Co-authored-by: Frank Harkins <[email protected]>

.github/workflows/main.yml

@@ -29,7 +29,7 @@ jobs:
       - name: File metadata
         run: npm run check:metadata
       - name: Check images
-        run: npm run check:images
+        run: npm run check:images -- --apis
       - name: Spellcheck
         run: npm run check:spelling
       - name: Check Qiskit bot config

README.md

@@ -299,6 +299,10 @@ You can check it locally by running:
 # Only check images
 npm run check:images
 
+# By default, only the non-API docs are checked. You can add the
+# below argument to also check API docs.
+npm run check:images -- --apis
+
 # Or, run all the checks
 npm run check
 ```

scripts/js/commands/checkImages.ts

@@ -11,11 +11,43 @@
 // that they have been altered from the originals.
 
 import { globby } from "globby";
+import yargs from "yargs/yargs";
+import { hideBin } from "yargs/helpers";
 import { collectInvalidImageErrors } from "../lib/markdownImages.js";
 import { readMarkdown } from "../lib/markdownReader.js";
 
+// Known APIs containing images without alt text that will be skipped
+// by the checker.
+const APIS_TO_IGNORE__SHOULD_FIX: string[] = [
+  "qiskit",
+  "qiskit-ibm-runtime",
+  "qiskit-addon-cutting",
+  "qiskit-addon-mpf",
+  "qiskit-addon-obp",
+  "qiskit-addon-utils",
+];
+
+interface Arguments {
+  [x: string]: unknown;
+  apis: boolean;
+}
+
+const readArgs = (): Arguments => {
+  return yargs(hideBin(process.argv))
+    .version(false)
+    .option("apis", {
+      type: "boolean",
+      default: false,
+      description:
+        "Check the images in the current and dev versions of the API docs have alt text.",
+    })
+    .parseSync();
+};
+
 async function main() {
-  const files = await globby(["docs/**/*.{ipynb,mdx}", "!docs/api/**/*.mdx"]);
+  const args = readArgs();
+
+  const files = await determineTocFiles(args);
   const fileErrors: string[] = [];
 
   for (const file of files) {
@@ -24,19 +56,37 @@ async function main() {
 
     if (imageErrors.size) {
       fileErrors.push(
-        `Error in file '${file}':\n\t- ${[...imageErrors].join("\n\t- ")}`,
+        `Error in file '${file}':\n\t- ${[...imageErrors].join("\n\t- ")}\n`,
       );
     }
   }
 
   if (fileErrors.length) {
     fileErrors.forEach((error) => console.log(error));
     console.error(
-      "\n💔 Some images have problems. See https://github.com/Qiskit/documentation#images for instructions.\n",
+      "💔 Some images have problems. See https://github.com/Qiskit/documentation#images for instructions.\n",
     );
     process.exit(1);
   }
   console.log("✅ All images are valid.\n");
 }
 
+async function determineTocFiles(args: Arguments): Promise<string[]> {
+  // We always skip historical versions to simplify the code and to have a faster script.
+  // Even though it is possible for someone to add a new image without alt text to a
+  // historical version that wasn't in the original release, that's very unlikely.
+  // If it happens, it would probably be a backport from latest or dev, and the linter in
+  // the API repo should catch it.
+  //
+  // If an image is missed by the API repo's linter, it will still have an alt text defined,
+  // although it won't be very useful.
+  const globs = [
+    "docs/**/*.{ipynb,mdx}",
+    args.apis ? "!docs/api/*/([0-9]*)/*.mdx" : "!docs/api/**/*.mdx",
+    ...APIS_TO_IGNORE__SHOULD_FIX.map((api) => `!docs/api/${api}/**/*.mdx`),
+  ];
+
+  return await globby(globs);
+}
+
 main().then(() => process.exit());

scripts/js/lib/markdownImages.test.ts

@@ -33,13 +33,16 @@ test("Test the finding of invalid images", async () => {
 <img src="/images/HTMLexample2.jpg" alt="Example" width="200"/>
 
 ![And now, our last link](https://ibm.com)
+
+![../\_images/invalid\_img3.png](/images/invalid_img3.png)
     `;
   const images = await collectInvalidImageErrors(markdown);
   const correct_images = new Set([
     "The image '/images/HTMLexample1.jpg' uses an HTML <img> tag instead of markdown syntax.",
     "The image '/images/HTMLexample2.jpg' uses an HTML <img> tag instead of markdown syntax.",
     "The image '/images/invalid_img1.png' does not have alt text.",
     "The image '/images/invalid_img2.png' does not have alt text.",
+    "The image '/images/invalid_img3.png' does not have alt text.",
   ]);
 
   expect(images).toEqual(correct_images);

scripts/js/lib/markdownImages.ts

@@ -17,6 +17,7 @@ import { visit } from "unist-util-visit";
 import remarkParse from "remark-parse";
 import remarkGfm from "remark-gfm";
 import remarkStringify from "remark-stringify";
+import { last, split } from "lodash-es";
 
 export async function collectInvalidImageErrors(
   markdown: string,
@@ -28,7 +29,10 @@ export async function collectInvalidImageErrors(
     .use(remarkGfm)
     .use(() => (tree: Root) => {
       visit(tree, "image", (node) => {
-        if (!node.alt) {
+        // Sphinx uses the image path as alt text if it wasn't defined using the
+        // :alt: option.
+        const imageName = last(split(node.url, "/"));
+        if (!node.alt || node.alt.endsWith(imageName!)) {
           imagesErrors.add(`The image '${node.url}' does not have alt text.`);
         }
       });