Skip to content

Begin multi-phase conversion of the CodeQL CDS extractor : Improve Modularity and Testing #188

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

Conversation

data-douser
Copy link
Collaborator

Sets up and begins a multi-phase process of converting the CDS extractor to a more modular, performant, testable, and overall better-architected solution.

The first phases of this process are implemented in this PR and are focused on improving the Modularity and Testability of the CDS extractor.

The extractors/cds/tools/autobuild.md file provides an overview of the entire (expected) conversion process.

An outline of changes follows:

Configuration and Code Quality Improvements:

  • Added .eslintrc.js in extractors/cds/tools to enforce linting rules, including TypeScript-specific rules, import ordering, and Prettier integration.
  • Added .prettierrc.js in extractors/cds/tools to define consistent code formatting rules, such as single quotes, trailing commas, and a 100-character print width.

Project Documentation:

  • Introduced autobuild.md in extractors/cds/tools, outlining the goals, challenges, and technical changes involved in transitioning to an autobuild-based extractor. This document emphasizes modularity, performance improvements, and testability.

Build Process Updates:

  • Updated index-files.cmd to include a TypeScript build step (npm run build) and to use the compiled out/index-files.js script instead of the uncompiled version. [1] [2]

Git Ignore Updates:

  • Updated .gitignore files to exclude generated directories (debug/ and out/) to prevent committing build artifacts and debug files. [1] [2]

Replaces the JS-based indes-files script with a TS-based index-files
script, and also sets up TypeScript (TS) build command with eslint
support such that the compiling the `index-files.ts` script will
create (one giant) `index-files.js` script (with a `index-files.js.map`
companion). Thus, sets up a TS-based equivalent of the index-files.js
CDS extractor script as preparation for a broader rewrite of the CDS
extractor to be more performant and maintainable.

Adds a documentation file, `autobuild.md`, to help document and guide
the multi-stage process of rewriting the CDS extractor to use an
autobuild-based approach, where this commit represents the first stage
of the multi-stage process.

Adds `.gitignore` files in subdirectories where testing the CDS
extractor tends to create changes that we never want to commit.
Refactors the index-files.ts script to be follow an object-orient
programming design, with the bulk of the script logic abstracted into
re-useable functions in the new `extractors/cds/tools/src` directory.

Adds a `.prettierrc.js` and updates the `.eslintrc.js` and
`package.json` files for the CDS extractor in order to provide better
linting coverage / rules for the TypeScript (`.ts`) code for the CDS
extractor sub-project.
Adds unit tests, and unit testing configs, for CDS extractor
`extractor/cds/tools/src/*.ts` files. Sets up test coverage reporting
as part of an updated `npm run build:all` command for the CDS extractor
project. Updates other project configs to supporte the "Testability"
phase of a multi-stage rewrite process.
@data-douser data-douser requested a review from Copilot April 30, 2025 05:34
@data-douser data-douser self-assigned this Apr 30, 2025
Copy link

@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 begins the conversion of the CDS extractor to improve modularity, testability, and overall code quality. Key changes include the introduction of extensive unit tests for both CodeQL and CDS compilation functions, updates to build and dependency installation scripts, and the addition of linting/formatting configurations along with updated documentation.

Reviewed Changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
extractors/cds/tools/test/src/codeql.test.ts Added unit tests for the CodeQL extractor functions.
extractors/cds/tools/test/src/cdsCompiler.test.ts Added unit tests for the CDS compiler functions.
extractors/cds/tools/test/.gitignore Updated to ignore generated JS files from TypeScript builds.
extractors/cds/tools/src/utils.ts Introduced helper functions for argument parsing and validation.
extractors/cds/tools/src/packageManager.ts Added utilities to discover package.json directories and install dependencies.
extractors/cds/tools/src/filesystem.ts Modularized filesystem operations for response file handling and renaming.
extractors/cds/tools/src/environment.ts Centralized platform and environment variable management.
extractors/cds/tools/src/codeql.ts Implements CodeQL extraction steps with improved requirement validation.
extractors/cds/tools/src/cdsCompiler.ts Introduces CDS compilation functionality with enhanced error handling.
extractors/cds/tools/package.json Updated scripts for TypeScript builds, linting, testing, and dependency management.
extractors/cds/tools/jest.config.js Configured Jest for testing TypeScript code.
extractors/cds/tools/index-files.ts Reworked main driver to leverage modular design and compiled code output.
extractors/cds/tools/index-files.sh & index-files.cmd Updated wrapper scripts to run build steps before executing the compiled code.
extractors/cds/tools/autobuild.md Provides documentation and guidance for future refactoring using an autobuild approach.
extractors/cds/tools/.prettierrc.js New Prettier configuration for consistent code formatting.
extractors/cds/tools/.eslintrc.js Introduced linting rules to enforce code quality and style guidelines.
extractors/cds/.gitignore Updated to ignore debug directories and related artifacts.

@data-douser data-douser requested a review from lcartey April 30, 2025 05:57
Upgrade node dependencies to the latest available versions for
TypeScript/JavaScript code used to implement the CDS extractor.

Removes the `extractors/cds/tools/package-lock.json` file from further
tracking and updates gitignore accordingly.
Translates the eslint config for the CDS extractor to match the
requirements of the latest eslint dependency. Migrates to a "flat"
config file (aka `eslint.config.mjs`). Updates other sub-project
configs to support the modified/translated eslint config for the
CDS extractor.
Atttempts fix the "Incomplete string escaping or encoding" code scanning
alert generated for the `extractors/cds/tools/src/cdsCompiler.ts` file.

Re-adds shell-quote sanitization logic that was lost during the
translation from "one big JavaScript script" to a modular solution
implemented primarily in TypeScript.
Adds the `extractors/README.md` file as an evolution of, and a
replace ment for, PR advanced-security#168 from this `advanced-security/codeql-sap-js`
repository.

Updates the CDS tools documentation to reflect progress in the
multi-stage process of rewriting the CDS extractor to be more
maintainable (WIP) and performant (TODO).
@data-douser data-douser marked this pull request as ready for review May 1, 2025 19:25
@data-douser
Copy link
Collaborator Author

The latest changes should allow for PR #168 to be closed as this PR now contains an equivalent, or improved, version of all changes from #168 .

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.

1 participant