Skip to content

Shared: Remove the language-specific model generator scripts #19452

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 1 commit into
base: main
Choose a base branch
from

Conversation

paldepind
Copy link
Contributor

@paldepind paldepind commented May 2, 2025

This PR

  • Makes it possible to invoke the shared MaD generator script directly and adds a new --language flag that specifies which language to generate models for.
  • Removes the language specific scripts.
  • Renamed generate_flow_model.py to generate_mad.py. Seems like a slightly nicer name as the script can also generate sinks and sources.

I think this will make it easier to expand on the script in a cross-language way going forward.

The model difference CI job is failing. I think that's because it's trying to use the new script path on the old main, which can't work. I think we can just ignore that?

@github-actions github-actions bot added C# C++ documentation Java Rust Pull requests that update Rust code labels May 2, 2025
@paldepind paldepind marked this pull request as ready for review May 2, 2025 12:36
@Copilot Copilot AI review requested due to automatic review settings May 2, 2025 12:36
@paldepind paldepind requested review from a team as code owners May 2, 2025 12:36
@paldepind paldepind requested a review from michaelnebel May 2, 2025 12:36
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 consolidates the per-language flow model generator scripts into a single shared generate_mad.py tool with a new --language flag, and removes the language‑specific stubs.

  • Refactor misc/scripts/models-as-data/generate_mad.py to handle all languages and add a command‑line entry point.
  • Delete each GenerateFlowModel.py under rust/ql, java/ql, csharp/ql, and cpp/ql.
  • Update RegenerateModels.py (Java) and the CI workflow to invoke the new script.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
rust/ql/src/utils/modelgenerator/GenerateFlowModel.py Removed Rust‑specific generator stub.
misc/scripts/models-as-data/generate_mad.py Refactored to support --language, cleaned imports, added __main__.
java/ql/src/utils/modelgenerator/RegenerateModels.py Updated to call generate_mad.py with --language java.
java/ql/src/utils/modelgenerator/GenerateFlowModel.py Removed Java‑specific generator stub.
java/ql/src/change-notes/2025-05-02-mad-generator-renamed.md Documented removal of old script and new --language requirement.
csharp/ql/src/utils/modelgenerator/GenerateFlowModel.py Removed C#‑specific generator stub.
csharp/ql/src/change-notes/2025-05-02-mad-generator-renamed.md Documented removal of old script and new --language requirement.
cpp/ql/src/utils/modelgenerator/GenerateFlowModel.py Removed C++‑specific generator stub.
.github/workflows/mad_modelDiff.yml Updated CI to invoke generate_mad.py with --language java.
Comments suppressed due to low confidence (1)

misc/scripts/models-as-data/generate_mad.py:81

  • The sys.exit(0) in the language‑not‑specified branch is mis‑indented (it ends up running unconditionally) and exits with a success code. It should be indented inside the else: block alongside the print(), and use a non‑zero exit code (e.g. sys.exit(1)), so that missing --language is treated as an error.
sys.exit(0)

@@ -68,7 +68,7 @@ jobs:
DATABASE=$2
cd codeql-$QL_VARIANT
SHORTNAME=`basename $DATABASE`
python java/ql/src/utils/modelgenerator/GenerateFlowModel.py --with-summaries --with-sinks $DATABASE $SHORTNAME/$QL_VARIANT
python misc/scripts/models-as-data/generate_mad.py --language java --with-summaries --with-sinks $DATABASE $SHORTNAME/$QL_VARIANT
Copy link
Preview

Copilot AI May 2, 2025

Choose a reason for hiding this comment

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

The CI invocation omits the --with-neutrals flag, but RegenerateModels.py includes it. Add --with-neutrals here to ensure neutral models are generated and compared in the diff job.

Suggested change
python misc/scripts/models-as-data/generate_mad.py --language java --with-summaries --with-sinks $DATABASE $SHORTNAME/$QL_VARIANT
python misc/scripts/models-as-data/generate_mad.py --language java --with-summaries --with-sinks --with-neutrals $DATABASE $SHORTNAME/$QL_VARIANT

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# C++ documentation Java Rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant