Skip to content

Overlay: Add script to help maintain overlay annotations #19778

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kaspersv
Copy link
Contributor

@kaspersv kaspersv commented Jun 16, 2025

This PR adds a script to automatically add sensible default annotations to files without existing overlay annotations. The script uses naming heuristics to determine which files to annotate. The script adds a module-level overlay[local?] annotation and annotates all non-private inline predicates overlay[caller] for selected files. Maintenance of overlay annotations in already annotated files is not the responsibility of this script and will be handled using QL-for-QL queries. This is intended to allow QL authors to gradually take ownership of overlay annotations and manually add & remove automatically added overlay annotations.

A CI check will be added in a subsequent PR to enforce that the script is run for select languages.

The script is based on @ginsbach's annotation script. However, instead of removing existing overlay annotations, it only annotates files without existing annotations and offloads maintenance of annotated files to QL-for-QL queries.

For https://github.com/github/codeql-core/issues/4951.

@kaspersv kaspersv changed the title Add script to help maintain overlay annotations Overlay: Add script to help maintain overlay annotations Jun 16, 2025
@kaspersv
Copy link
Contributor Author

@ginsbach I've updated the script to add overlay annotations with a --check mode (to be used by the CI check) and expanded the script such that it can handle all our current QL code in the codeql repo. For the latter I've broken the logic for determining where to insert overlay[local?] into a couple of distinct phases (find file-level module declaration if one exists, find file-module qldoc if one exists, etc.). The code is a bit longer, but hopefully simpler to understand and maintain. The changes have no effect for Java (i.e., we insert annotations exactly as before for Java), but allows us to handle all languages.

I've closed the previous PR as it was based on a kaspersv/codeql branch rather than a github/codeql branch, which makes PR stacking more cumbersome.

Would you mind reviewing this PR before I send this PR and the PR that adds overlay annotations for Java to review with the Java team?

@kaspersv kaspersv marked this pull request as ready for review June 17, 2025 06:46
@Copilot Copilot AI review requested due to automatic review settings June 17, 2025 06:48
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a new Python script (add-overlay-annotations.py) that walks the .qll directory tree for specified languages, detects files without any overlay[...] annotations, and injects sensible default overlay[local?] and overlay[caller] annotations. It also supports a --check mode to surface missing annotations without modifying files.

  • Adds a script to auto-annotate unannotated .qll files and skip files per naming/dataflow heuristics
  • Implements a dry-run (--check) mode and prints a CI-friendly list of unannotated files
  • Defines language mappings to target directories and shared code
Comments suppressed due to low confidence (1)

config/add-overlay-annotations.py:1

  • [nitpick] There are no tests covering this new script. Consider adding unit or integration tests to verify the insertion logic in various scenarios (e.g., files with only comments, existing module statements, qldoc cases).
# This script is used to annotate .qll files without any existing overlay annotations

@kaspersv kaspersv requested a review from ginsbach June 17, 2025 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants