Open
Conversation
While we want the CodeQL Action to work with third-party language support, having a list of all built-in languages can help us create better type-level checks to ensure that we don't miss things that we want to customize for each of our built-in languages.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR renames the “known language” concept to “built-in language” and introduces a curated, updateable source of truth for all built-in CodeQL languages (plus aliases) to enable stricter type-level exhaustiveness checks across the Action.
Changes:
- Replace
KnownLanguagewithBuiltInLanguagethroughout the TypeScript codebase and tests. - Add
src/languages/builtin.json+parseBuiltInLanguage/isBuiltInLanguageutilities, with unit tests to keep the enum and JSON synchronized. - Update the bundle update workflow to regenerate the built-in language list via a new script.
Show a summary per file
| File | Description |
|---|---|
| src/trap-caching.test.ts | Update tests to use BuiltInLanguage instead of KnownLanguage. |
| src/tracer-config.test.ts | Update tests to use BuiltInLanguage. |
| src/status-report.test.ts | Update tests to use BuiltInLanguage. |
| src/start-proxy/environment.ts | Switch language-specific proxy environment checks to BuiltInLanguage. |
| src/start-proxy/environment.test.ts | Update start-proxy environment tests for BuiltInLanguage. |
| src/start-proxy.ts | Replace KnownLanguage usage with BuiltInLanguage; remove old parseLanguage logic in favor of centralized parsing. |
| src/start-proxy.test.ts | Update tests to use BuiltInLanguage; remove parseLanguage tests (now covered in languages module tests). |
| src/start-proxy-action.ts | Use parseBuiltInLanguage and BuiltInLanguage for the language input. |
| src/overlay/index.test.ts | Update overlay tests to use BuiltInLanguage. |
| src/languages/index.ts | Introduce BuiltInLanguage, isBuiltInLanguage, and parseBuiltInLanguage, backed by builtin.json. |
| src/languages/index.test.ts | Add unit tests for parsing, membership checks, and enum↔JSON synchronization. |
| src/languages/builtin.json | New curated list of built-in languages and aliases. |
| src/languages.ts | Remove the old KnownLanguage/JavaEnvVars definitions (moved to src/languages/index.ts). |
| src/known-language-aliases.json | Remove the old alias JSON (replaced by src/languages/builtin.json). |
| src/init.ts | Update language checks to BuiltInLanguage. |
| src/init.test.ts | Update tests/types to BuiltInLanguage. |
| src/init-action.ts | Update language-specific behavior checks to BuiltInLanguage. |
| src/dependency-caching.ts | Update language-specific feature gating to BuiltInLanguage. |
| src/dependency-caching.test.ts | Update dependency caching tests to BuiltInLanguage. |
| src/database-upload.test.ts | Update database upload tests to BuiltInLanguage. |
| src/config/db-config.test.ts | Update config parsing tests to BuiltInLanguage. |
| src/config-utils.ts | Update supported-language filtering and language-specific logic to BuiltInLanguage. |
| src/config-utils.test.ts | Update config utils tests to BuiltInLanguage. |
| src/codeql.test.ts | Update CodeQL wrapper tests to BuiltInLanguage. |
| src/autobuild.ts | Update autobuild ordering and language checks to BuiltInLanguage. |
| src/analyze.ts | Update status-report typing and extraction conditionals to BuiltInLanguage. |
| src/analyze.test.ts | Update analysis tests to BuiltInLanguage. |
| src/analyze-action.ts | Update Go legacy workflow logic to use BuiltInLanguage. |
| pr-checks/sync.ts | Update PR checks generator typing to BuiltInLanguage. |
| package.json | Extend transpile script to also typecheck workflow scripts tsconfig. |
| .github/workflows/update-bundle.yml | Replace alias-update step with built-in-language regeneration via new script. |
| .github/workflows/script/update-builtin-languages.ts | New script to regenerate src/languages/builtin.json using the CodeQL CLI. |
| .github/workflows/script/tsconfig.json | New tsconfig for typechecking workflow scripts (no emit). |
| lib/upload-sarif-action.js | Generated output update (not reviewed). |
| lib/upload-sarif-action-post.js | Generated output update (not reviewed). |
| lib/upload-lib.js | Generated output update (not reviewed). |
| lib/start-proxy-action.js | Generated output update (not reviewed). |
| lib/start-proxy-action-post.js | Generated output update (not reviewed). |
| lib/setup-codeql-action.js | Generated output update (not reviewed). |
| lib/resolve-environment-action.js | Generated output update (not reviewed). |
| lib/init-action.js | Generated output update (not reviewed). |
| lib/init-action-post.js | Generated output update (not reviewed). |
| lib/autobuild-action.js | Generated output update (not reviewed). |
| lib/analyze-action.js | Generated output update (not reviewed). |
| lib/analyze-action-post.js | Generated output update (not reviewed). |
Copilot's findings
- Files reviewed: 33/45 changed files
- Comments generated: 1
| @@ -0,0 +1,88 @@ | |||
| #!/usr/bin/env npx tsx | |||
There was a problem hiding this comment.
The shebang #!/usr/bin/env npx tsx is not portable: most env implementations can’t take multiple arguments from a shebang line (without -S), so running this script directly would fail. Either remove the shebang (since the workflow already runs it via npx tsx ...) or switch to an env -S style shebang.
Suggested change
| #!/usr/bin/env npx tsx |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While we want the CodeQL Action to work with third-party language support, having a list of all built-in languages can help us create better type-level checks to ensure that we don't miss things that we want to customize for each of our built-in languages.
For example, in the
start-proxyAction, we want to make sure each built-in language is associated with a registries config. If we add a new language, we want to be reminded to update that config.This means that when we add a new language, we will need to update the
BuiltInLanguageenum (and potentially other places in the code that exhaustively enumerateBuiltInLanguagefor safety reasons). This is a rare event and probably worth it from a safety and correctness perspective.Commit-by-commit review recommended as we rename
KnownLanguagetoBuiltInLanguageto reflect the new motivation behind this type.Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
Workflow types:
dynamicworkflows (Default Setup, Code Quality, ...).Products:
analysis-kinds: code-scanning.analysis-kinds: code-quality.upload-sarifaction.Environments:
github.comand/or GitHub Enterprise Cloud with Data Residency.How did/will you validate this change?
.test.tsfiles).pr-checks).If something goes wrong after this change is released, what are the mitigation and rollback strategies?
How will you know if something goes wrong after this change is released?
Are there any special considerations for merging or releasing this change?
Merge / deployment checklist