tests: check: ast-grep: add static analysis rules

[pzmarzly]

If https://ast-grep.github.io/ is installed (e.g. via pip install ast-grep-cli), we can use it to enforce many of our coding standards. See #510. Previous attempt in #502.

Each rule is added in separate commit, so it's best to review this PR commit-by-commit.

Rules with existing violations are added as disabled. We should reenable them one by one later. Each reenablement will create a large PR, that may create merge conflicts for everyone.

Test

ast-grep scan --config tests/check/ast-grep/sgconfig.yml src/bfcli/ src/libbpfilter/
https://gist.github.com/pzmarzly/7dfba8ca061a942a271b14c6320ea13b

ast-grep scan --config tests/check/ast-grep/sgconfig.yml src/bfcli/ src/libbpfilter/ --off=assert-pointer-params --off=doxygen-prefer-backticks --off=doxygen-public-functions --off=no-direct-free --off=single-line-comment-style (current invocation in CMakeLists.txt):
[empty result, i.e. all OK]

List of rules

Status	Name	Description
disabled	assert-pointer-params	Functions with pointer parameters must assert() them at entry.
enabled	bf-prefix-public-functions	Non-static functions must use the bf_/bfc_ prefix (or _bf_/_bfc_ when static).
enabled	double-pointer-output	Constructor bf_*_new() must take a TYPE ** output parameter for ownership transfer.
disabled	doxygen-prefer-backticks	Prefer `name` over @p name, @ref name, @c name in Doxygen comments.
disabled	doxygen-public-functions	Public function declarations in headers must have a preceding /** @brief ... */ comment.
enabled	enum-bf-prefix	Named enums must use bf_/bfc_ prefix.
enabled	enum-sentinel-format	Enums must end with a _BF_*_MAX / _BFC_*_MAX sentinel (with listed exceptions for bitmasks and kernel ABI mirrors).
enabled	fd-init-minus-one	Variables declared with _cleanup_close_ must be initialized to -1 to avoid closing fd 0 on error paths.
enabled	free-function-pattern	bf_*_free() / _bf_*_free() must return void and accept a double-pointer parameter.
enabled	negative-errno-returns	Error returns must be negated (-ENOMEM, not ENOMEM).
disabled	no-direct-free	Disallows raw free(); use freep() or a typed bf_*_free() cleanup function instead (with exempt callers).
enabled	no-fprintf-stderr	Use logging macros (bf_err, bf_warn, …) instead of fprintf(stderr, …) / vfprintf(stderr, …).
enabled	no-ifndef-guards	Headers must use #pragma once, not #ifndef/#define include guards.
disabled	single-line-comment-style	Single-line comments must use //, reserving /* */ for multi-line (SPDX excluded).
enabled	spdx-license-header	Every source file must contain an SPDX-License-Identifier comment.
enabled	struct-bf-prefix	Struct definitions (with a body) must use the bf_/bfc_ prefix; external type references are ignored.

[pzmarzly]

@claude review

[github-actions]

Claude review of PR #545 (6c8bf90)

Suggestions

• bf_freep not covered by no-direct-free exemption — tests/check/ast-grep/rules/no-direct-free.yml:26 — The exemption regex won't match bf_freep (which legitimately wraps free()), causing a false positive when the rule is enabled
• Exclusion list sync comment is misleading — tests/check/ast-grep/rules/assert-pointer-params.yml:35 — Comment says "keep in sync with bf-prefix-public-functions.yml" but the lists intentionally differ; rephrase or align
• No version pin for ast-grep-cli — .github/fedora-42.Dockerfile:40 — All Dockerfiles install ast-grep-cli without a version pin, risking CI breakage on upstream changes

Nits

• Parenthesized errno returns escape detection — tests/check/ast-grep/rules/negative-errno-returns.yml:12 — return (ENOMEM); won't be caught due to missing stopBy: end
• Redundant .gitkeep files — tests/check/ast-grep/rules/.gitkeep — Both rules/ and rule-tests/ directories now have real content

Workflow run

[github-actions]

Claude: suggestion: ast-grep-cli is installed without a version pin (pipx install --global ast-grep-cli). A breaking change in ast-grep's rule syntax or --off flag behavior could cause CI failures across all branches simultaneously. Consider pinning to a specific version (e.g. pipx install --global ast-grep-cli==0.38.0) and updating periodically via a dedicated PR. Same applies to all other Dockerfiles (Fedora via pipx, Ubuntu via pip).

[pzmarzly]

On the other hand, it'd be nice to see failures introduced in new version immediately, not "whenever we remember to bump hardcoded versions". I think fetching the latest version should be fine.

[github-actions]

Claude: nit: The rule uses has: without stopBy: end, so return (ENOMEM); (parenthesized) would escape detection because the direct child of return_statement is parenthesized_expression, not identifier. This doesn't occur in the current codebase but is a latent gap. Adding stopBy: end to the has: clause would make it robust against this pattern.

[pzmarzly]

This is very niche case, ignoring.

[qdeslandes]

Quick question, otherwise LGTM.

[qdeslandes]

We already use pip for Ubuntu containers, why not using it too here?

[meta-cla]

Hi @pzmarzly!

Thank you for your pull request.

We require contributors to sign our Contributor License Agreement, and yours needs attention.

You currently have a record in our system, but the CLA is no longer valid, and will need to be resubmitted.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

[qdeslandes]

Pawel's contribution to this PR were made during his employment at Meta. While he would have to sign the CLA for new contribution, this specific one doesn't require it.