-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add script to add overlay annotations #19631
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds a new Python script to automatically insert default overlay annotations into .qll
files that lack them, based on naming and content heuristics.
- Implements
add-overlay-annotations.py
to add module-leveloverlay[local?]
and inlineoverlay[caller]
annotations. - Skips files with existing annotations, tests, dataflow configs, or empty content.
- Targets specified languages under
<lang>/ql/lib
and a shared directory.
Comments suppressed due to low confidence (1)
config/add-overlay-annotations.py:106
- New annotation logic isn't covered by any tests; consider adding unit tests for
annotate_as_appropriate
,insert_overlay_caller_annotations
, and the top-level function to prevent regressions.
def annotate_as_appropriate(filename, lines):
1356bd9
to
9572739
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy with this - but it is mostly my own code.
Maybe it would make sense to have a QL developer as additional reviewer. That way, we could already pass on knowledge about this code as a positive side effect.
''' | ||
overlays = ["local", "local?", "global", "caller"] | ||
annotations = [f"overlay[{t}]" for t in overlays] | ||
return any(any(ann in line for ann in annotations) for line in lines) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return any(any(ann in line for ann in annotations) for line in lines) | |
return any(ann in line for ann in annotations for line in lines) |
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 predicatesoverlay[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 check that the script has been 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.