BLD: add more static analysis by tacaswell · Pull Request #31661 · matplotlib/matplotlib

tacaswell · 2026-05-12T03:10:16Z

PR summary

This adds more static analysis to our CI / prek config.

run clang-tidy on c/c++/objectiveC code (and fix or ignore issues found). This found a couple of real issues and will only run on CI (+ a helper script to run it locally). The dependencies are too heavy to try and launder through prek's virtual environments.
run zizmor in prek (and hence automatically on CI). Had to explictily allow the 4 places we use pull_request_target and a couple of places we allow caching. Also enabled cooldown windows on our dependabot config. This also flagged we had a redundant checkout step in
enable shellcheck in prek to hit the handful of bash scripts we ship and applied the hardening it found.
~~run ruff on our checked in notebooks as well~~ Our exsiting ruff config hits this already
added static validation of svg files to prek (I got in my head that this could be used to get information out of developer's machines which is probably not possible (beyond what you give up by going to any website)), but now it will be harder for someone to sneak an active SVG into our baseline images.

Not reflected in the commits here, but I did look at https://github.com/PyCQA/bandit but every single thing it flagged was a false positive (either flagging things like exec in the plot directive or being too simplistic to notice that we had already mitigated the thing it was flagging in the code).

AI Disclosure

I started from the prompt

Analyze this repository and identify any programming languages, markup languages, document formats, build artifacts, generated files, embedded assets, or other non-core technologies that could be overlooked by traditional SAST or standard code-scanning tools.

Focus especially on:

languages or file types that are not part of the repository's primary tech stack
obscure, uncommon, legacy, or less frequently scanned formats
executable or scriptable content hidden in non-obvious places
document or graphics formats that may contain logic, macros, scripts, links, or embedded content
Include examples such as LaTeX, PostScript, SVG, and similar formats, but do not limit the analysis to those.

For each item you identify, provide:

the language, format, or artifact type
why it may be missed by classic SAST or scanning tools
the security relevance or potential risk
typical file extensions or repository locations where it may appear
whether it is likely source, generated output, embedded content, or supporting artifact

with opus 4.6 and then pushing on addressing the coverage with a mix of opus 4.6 and sonnet 4.6. I reviewed everything as I committed it and took a very heavy editing pass at the initial version of the documentation.

PR checklist

"closes #0000" is in the body of the PR description to link the related issue
new and changed code is tested
Plotting related features are demonstrated in an example
New Features and API Changes are noted with a directive and release note
Documentation complies with general and docstring guidelines

github-advanced-security · 2026-05-12T03:10:41Z

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

QuLogic · 2026-05-12T05:23:59Z

@@ -0,0 +1,246 @@
+#!/usr/bin/env python3
+"""
+Run clang-tidy on Matplotlib's C/C++ and Objective-C sources.


I'm not sure this script is fully necessary; meson will create a clang-tidy target for you (but only if you have .clang-tidy in the root), so you just need to move it, make an editable install, and run ninja -C build/cp313 clang-tidy or similar

I think it is still useful for filtering out warnings from the Agg source, but this can indeed be greatly simplified.

I simplified this, still think it is useful to have to automate creating a build directory just for clang-tidy and for filtering the output.

These were false positives, but add code to give the analyzer a hint + asserts in a debugging build rather than NOLINT to be defensive. Aided by 🤖

When for no-change to code with the qhull wrapper rather than adjusting the code. Aided by 🤖

Fixes an issue found by clang-tidy Aided by 🤖

Simplified at Elliott's suggestions Aided by 🤖

Aided by 🤖

Added exceptions for things it thinks are questionable but we are OK with doing. Aided by 🤖

Aided by 🤖

tacaswell · 2026-05-13T14:36:24Z

This runs zizimor twice (once in its own job, once in the prek job). I think that is OK, particularly because its own job also has a cron attached to it, but dropping the action in favor of just the prek invocation would also be fine.

QuLogic · 2026-05-14T19:12:12Z

This runs zizimor twice (once in its own job, once in the prek job). I think that is OK, particularly because its own job also has a cron attached to it, but dropping the action in favor of just the prek invocation would also be fine.

The separate job also has a token and should be able to run more checks, I think?

QuLogic · 2026-05-16T06:17:02Z

+        run: brew install llvm
+
+      - name: Install build dependencies
+        run: pip3 install meson ninja pybind11 setuptools-scm


It might install a bit extra, but this might be better?

Suggested change

run: pip3 install meson ninja pybind11 setuptools-scm

run: pip3 install --group build

QuLogic · 2026-05-16T06:20:05Z

+          persist-credentials: false
+
+      - name: Run zizmor
+        uses: zizmorcore/zizmor-action@b1d7e1fb5de872772f31590499237e7cce841e8e  # v0.5.3


Suggested change

uses: zizmorcore/zizmor-action@b1d7e1fb5de872772f31590499237e7cce841e8e # v0.5.3

uses: zizmorcore/zizmor-action@b572f7b1a1c2d41efaab43d504f68d215c3cd727 # v0.5.4

QuLogic · 2026-05-16T06:21:16Z

+        with:
+          advanced-security: true


This defaults on.

Suggested change

with:

advanced-security: true

QuLogic · 2026-05-16T06:22:46Z

+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}


The action already fetches the token.

Suggested change

env:

GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}

QuLogic · 2026-05-16T06:25:03Z

+Static analysis with clang-tidy
+--------------------------------


Extra dash.

Suggested change

Static analysis with clang-tidy

--------------------------------

Static analysis with clang-tidy

-------------------------------

QuLogic · 2026-05-16T06:39:01Z

+The logic lives in :file:`tools/run_clang_tidy.py`.  It requires
+``clang-tidy`` on ``PATH`` and ``meson`` and ``pybind11`` installed::
+
+    pip install meson pybind11


The workflow also installs setuptools-scm? But maybe also use the group.

QuLogic · 2026-05-16T07:23:54Z

+            if current:
+                groups.append(current)
+            current = [line]
+        else:
+            current.append(line)
+
+    if current:
+        groups.append(current)


You can just append when you create it.

Suggested change

if current:

groups.append(current)

current = [line]

else:

current.append(line)

if current:

groups.append(current)

current = [line]

groups.append(current)

else:

current.append(line)

QuLogic · 2026-05-16T07:27:24Z

+    hooks:
+      - id: shellcheck
+  - repo: https://github.com/zizmorcore/zizmor-pre-commit
+    rev: a4727cbbcd26d7098e96b9cb738169b59711ae51  # frozen: v1.24.1


Suggested change

rev: a4727cbbcd26d7098e96b9cb738169b59711ae51 # frozen: v1.24.1

rev: 04a94ec45c8e92de281e3cc55754eb6d9db526a1 # frozen: v1.25.1

github-actions Bot added GUI: MacOSX topic: path handling topic: text/fonts topic: contour Documentation: devdocs files in doc/devel labels May 12, 2026

QuLogic reviewed May 12, 2026

View reviewed changes

tacaswell added 11 commits May 12, 2026 16:18

STY: add shellcheck

d053d95

MNT: fix warnings identified by clang-tidy

76e84f7

These were false positives, but add code to give the analyzer a hint + asserts in a debugging build rather than NOLINT to be defensive. Aided by 🤖

BLD: add NOLINT directives to _qhull_wrapper

b8fb1b4

When for no-change to code with the qhull wrapper rather than adjusting the code. Aided by 🤖

MNT: ensure that variables are always initialized

99ccdca

Fixes an issue found by clang-tidy Aided by 🤖

BLD: skip two more clang-tidy warnings in _tri.cpp

419087b

BLD: add tooling to run clang-tidy on c/c++ code

4b3ce4d

Simplified at Elliott's suggestions Aided by 🤖

CI: add cool-down periods on dependabot

5449bea

CI: remove redundancy and leaked credentials in workflow

edef1d8

Aided by 🤖

BLD: add zizmor to prek config

628a5f5

Added exceptions for things it thinks are questionable but we are OK with doing. Aided by 🤖

CI: add zizmor action to keep us in line

75b3192

Aided by 🤖

CI: add static validation of svg

050c2e8

Aided by 🤖

tacaswell force-pushed the bld/static_analysis branch from 1c966c2 to 050c2e8 Compare May 12, 2026 20:19

github-actions Bot added the topic: tri label May 12, 2026

tacaswell added 2 commits May 12, 2026 20:16

CI: ensure clang-tidy on PATH

0420bc4

CI: try to use the right compilers

dc46af5

tacaswell added this to the v3.12.0 milestone May 13, 2026

ksunden approved these changes May 14, 2026

View reviewed changes

DOC: clearify comment as to why only cygwin needs this config

6ff08e9

QuLogic reviewed May 15, 2026

View reviewed changes

Comment thread src/tri/_tri.cpp

QuLogic reviewed May 16, 2026

View reviewed changes

	run: pip3 install meson ninja pybind11 setuptools-scm
	run: pip3 install --group build

	uses: zizmorcore/zizmor-action@b1d7e1fb5de872772f31590499237e7cce841e8e # v0.5.3
	uses: zizmorcore/zizmor-action@b572f7b1a1c2d41efaab43d504f68d215c3cd727 # v0.5.4

		Static analysis with clang-tidy
		--------------------------------

	rev: a4727cbbcd26d7098e96b9cb738169b59711ae51 # frozen: v1.24.1
	rev: 04a94ec45c8e92de281e3cc55754eb6d9db526a1 # frozen: v1.25.1

Uh oh!

Conversation

tacaswell commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR summary

AI Disclosure

PR checklist

Uh oh!

github-advanced-security AI commented May 12, 2026

What Enabling Code Scanning Means:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tacaswell commented May 13, 2026

Uh oh!

QuLogic commented May 14, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tacaswell commented May 12, 2026 •

edited

Loading