Skip to content

feat: use tree-sitter to parse requirements.txt more correctly #391

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.

Sign up for GitHub

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
Strum355 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: use tree-sitter to parse requirements.txt more correctly #391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
414 changes: 300 additions & 114 deletions package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@
"https-proxy-agent": "^7.0.6",
"node-fetch": "^3.3.2",
"packageurl-js": "~1.0.2",
"tree-sitter-requirements": "github:Strum355/tree-sitter-requirements",
"web-tree-sitter": "^0.26.6",
Comment on lines +56 to +57
Copy link

@qodo-code-review qodo-code-review bot Mar 4, 2026

Choose a reason for hiding this comment

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

Action required

3. Git dep uses git+ssh 🐞 Bug ⛯ Reliability

The added GitHub dependency resolves in package-lock.json to a git+ssh URL, which commonly fails in
CI/user environments without GitHub SSH keys configured. This can block installation entirely.
Agent Prompt 
## Issue description
The dependency `tree-sitter-requirements` is declared via GitHub shorthand. In `package-lock.json` it resolves to `git+ssh://git@github.com/...`, which frequently fails without SSH keys.

## Issue Context
This affects every `npm ci` / `npm install` consumer of this package.

## Fix Focus Areas
- package.json[56-57]
- package-lock.json[7507-7511]

## Suggested fix
- Update `package.json` to use an HTTPS git URL pinned to a commit/tag, e.g. `"tree-sitter-requirements": "git+https://github.com/Strum355/tree-sitter-requirements.git#<sha>"` (or a published semver npm package).
- Re-run `npm install` (or `npm ci` with regeneration as appropriate) to update `package-lock.json` so `resolved` uses `git+https` not `git+ssh`.
- Verify CI can install without SSH configuration.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

"yargs": "^18.0.0"
},
"devDependencies": {
Expand All @@ -64,6 +66,7 @@
"c8": "^11.0.0",
"chai": "^4.3.7",
"eslint": "^8.42.0",
"eslint-import-resolver-typescript": "^4.4.4",
"eslint-plugin-editorconfig": "^4.0.3",
"eslint-plugin-import": "^2.29.1",
"esmock": "^2.6.2",
Expand Down
4 changes: 2 additions & 2 deletions src/analysis.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ function addProxyAgent(options, opts) {
async function requestStack(provider, manifest, url, html = false, opts = {}) {
opts["source-manifest"] = Buffer.from(fs.readFileSync(manifest).toString()).toString('base64')
opts["manifest-type"] = path.parse(manifest).base
let provided = provider.provideStack(manifest, opts) // throws error if content providing failed
let provided = await provider.provideStack(manifest, opts) // throws error if content providing failed
opts["source-manifest"] = ""
opts[rhdaOperationTypeHeader.toUpperCase().replaceAll("-", "_")] = "stack-analysis"
let startTime = new Date()
Expand Down Expand Up @@ -105,7 +105,7 @@ async function requestStack(provider, manifest, url, html = false, opts = {}) {
async function requestComponent(provider, manifest, url, opts = {}) {
opts["source-manifest"] = Buffer.from(fs.readFileSync(manifest).toString()).toString('base64')

let provided = provider.provideComponent(manifest, opts) // throws error if content providing failed
let provided = await provider.provideComponent(manifest, opts) // throws error if content providing failed
opts["source-manifest"] = ""
opts[rhdaOperationTypeHeader.toUpperCase().replaceAll("-", "_")] = "component-analysis"
if (process.env["TRUSTIFY_DA_DEBUG"] === "true") {
Expand Down
2 changes: 1 addition & 1 deletion src/provider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import Javascript_yarn from './providers/javascript_yarn.js';
import pythonPipProvider from './providers/python_pip.js'

/** @typedef {{ecosystem: string, contentType: string, content: string}} Provided */
/** @typedef {{isSupported: function(string): boolean, validateLockFile: function(string): void, provideComponent: function(string, {}): Provided, provideStack: function(string, {}): Provided}} Provider */
/** @typedef {{isSupported: function(string): boolean, validateLockFile: function(string): void, provideComponent: function(string, {}): Provided | Promise<Provided>, provideStack: function(string, {}): Provided | Promise<Provided>}} Provider */

/**
* MUST include all providers here.
Expand Down
135 changes: 66 additions & 69 deletions src/providers/python_controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import fs from "node:fs";
import path from 'node:path';
import os, {EOL} from "os";
import os, { EOL } from "os";

import {environmentVariableIsPopulated,getCustom, invokeCommand} from "../tools.js";
import { environmentVariableIsPopulated, getCustom, invokeCommand } from "../tools.js";

import { getParser, getRequirementQuery, getPinnedVersionQuery } from './requirements_parser.js';

function getPipFreezeOutput() {
try {
Expand All @@ -23,13 +25,15 @@ function getPipShowOutput(depNames) {
/** @typedef {{name: string, version: string, dependencies: DependencyEntry[]}} DependencyEntry */

export default class Python_controller {

pythonEnvDir
pathToPipBin
pathToPythonBin
realEnvironment
pathToRequirements
options
parser
requirementsQuery
pinnedVersionQuery

/**
* Constructor to create new python controller instance to interact with pip package manager
Expand All @@ -39,14 +43,18 @@ export default class Python_controller {
* @param {string} pathToRequirements
* @
*/
constructor(realEnvironment,pathToPip,pathToPython,pathToRequirements,options={}) {
constructor(realEnvironment, pathToPip, pathToPython, pathToRequirements, options={}) {
this.pathToPythonBin = pathToPython
this.pathToPipBin = pathToPip
this.realEnvironment= realEnvironment
this.prepareEnvironment()
this.pathToRequirements = pathToRequirements
this.options = options
this.parser = getParser()
this.requirementsQuery = getRequirementQuery()
this.pinnedVersionQuery = getPinnedVersionQuery()
}

prepareEnvironment() {
if(!this.realEnvironment) {
this.pythonEnvDir = path.join(path.sep, "tmp", "trustify_da_env_js")
Expand Down Expand Up @@ -87,6 +95,24 @@ export default class Python_controller {
}
}

/**
* Parse the requirements.txt file using tree-sitter and return structured requirement data.
* @return {Promise<{name: string, version: string|null}[]>}
*/
async #parseRequirements() {
const content = fs.readFileSync(this.pathToRequirements).toString();
const tree = (await this.parser).parse(content);
return Promise.all((await this.requirementsQuery).matches(tree.rootNode).map(async (match) => {
const reqNode = match.captures.find(c => c.name === 'req').node;
const name = match.captures.find(c => c.name === 'name').node.text;
const versionMatches = (await this.pinnedVersionQuery).matches(reqNode);
const version = versionMatches.length > 0
? versionMatches[0].captures.find(c => c.name === 'version').node.text
: null;
return { name, version };
}));
}

#decideIfWindowsOrLinuxPath(fileName) {
if (os.platform() === "win32") {
return fileName + ".exe"
Expand All @@ -97,9 +123,9 @@ export default class Python_controller {
/**
*
* @param {boolean} includeTransitive - whether to return include in returned object transitive dependencies or not
* @return {[DependencyEntry]}
* @return {Promise<[DependencyEntry]>}
*/
getDependencies(includeTransitive) {
async getDependencies(includeTransitive) {
let startingTime
let endingTime
if (process.env["TRUSTIFY_DA_DEBUG"] === "true") {
Expand All @@ -123,10 +149,10 @@ export default class Python_controller {
if(matchManifestVersions === "true") {
throw new Error("Conflicting settings, TRUSTIFY_DA_PYTHON_INSTALL_BEST_EFFORTS=true can only work with MATCH_MANIFEST_VERSIONS=false")
}
this.#installingRequirementsOneByOne()
await this.#installingRequirementsOneByOne()
}
}
let dependencies = this.#getDependenciesImpl(includeTransitive)
let dependencies = await this.#getDependenciesImpl(includeTransitive)
this.#cleanEnvironment()
if (process.env["TRUSTIFY_DA_DEBUG"] === "true") {
endingTime = new Date()
Expand All @@ -137,15 +163,13 @@ export default class Python_controller {
return dependencies
}

#installingRequirementsOneByOne() {
let requirementsContent = fs.readFileSync(this.pathToRequirements);
let requirementsRows = requirementsContent.toString().split(EOL);
requirementsRows.filter((line) => !line.trim().startsWith("#")).filter((line) => line.trim() !== "").forEach( (dependency) => {
let dependencyName = getDependencyName(dependency);
async #installingRequirementsOneByOne() {
const requirements = await this.#parseRequirements();
requirements.forEach(({name}) => {
try {
invokeCommand(this.pathToPipBin, ['install', dependencyName])
invokeCommand(this.pathToPipBin, ['install', name])
} catch (error) {
throw new Error(`Failed in best-effort installing ${dependencyName} in virtual python environment`, {cause: error})
throw new Error(`Failed in best-effort installing ${name} in virtual python environment`, {cause: error})
}
})
}
Expand All @@ -162,44 +186,33 @@ export default class Python_controller {
}
}

#getDependenciesImpl(includeTransitive) {
let dependencies = new Array()
async #getDependenciesImpl(includeTransitive) {
let dependencies = []
let usePipDepTree = getCustom("TRUSTIFY_DA_PIP_USE_DEP_TREE","false",this.options);
let freezeOutput
let lines
let depNames
let pipShowOutput
let allPipShowDeps
let pipDepTreeJsonArrayOutput
if(usePipDepTree !== "true") {
freezeOutput = getPipFreezeOutput.call(this);
lines = freezeOutput.split(EOL)
depNames = lines.map( line => getDependencyName(line))
}
else {
pipDepTreeJsonArrayOutput = getDependencyTreeJsonFromPipDepTree(this.pathToPipBin,this.pathToPythonBin)
}


if(usePipDepTree !== "true") {
pipShowOutput = getPipShowOutput.call(this, depNames);
const freezeOutput = getPipFreezeOutput.call(this);
const lines = freezeOutput.split(EOL)
const depNames = lines.map( line => getDependencyName(line))
const pipShowOutput = getPipShowOutput.call(this, depNames);
allPipShowDeps = pipShowOutput.split( EOL + "---" + EOL);
} else {
pipDepTreeJsonArrayOutput = getDependencyTreeJsonFromPipDepTree(this.pathToPipBin,this.pathToPythonBin)
}
//debug
// pipShowOutput = "alternative pip show output goes here for debugging"

let matchManifestVersions = getCustom("MATCH_MANIFEST_VERSIONS","true",this.options);
let linesOfRequirements = fs.readFileSync(this.pathToRequirements).toString().split(EOL).filter( (line) => !line.trim().startsWith("#")).map(line => line.trim())
let parsedRequirements = await this.#parseRequirements()
let CachedEnvironmentDeps = {}
if(usePipDepTree !== "true") {
allPipShowDeps.forEach((record) => {
allPipShowDeps.forEach(record => {
let dependencyName = getDependencyNameShow(record).toLowerCase()
CachedEnvironmentDeps[dependencyName] = record
CachedEnvironmentDeps[dependencyName.replace("-", "_")] = record
CachedEnvironmentDeps[dependencyName.replace("_", "-")] = record
})
} else {
pipDepTreeJsonArrayOutput.forEach( depTreeEntry => {
pipDepTreeJsonArrayOutput.forEach(depTreeEntry => {
let packageName = depTreeEntry["package"]["package_name"].toLowerCase()
let pipDepTreeEntryForCache = {
name: packageName,
Expand All @@ -211,41 +224,25 @@ export default class Python_controller {
CachedEnvironmentDeps[packageName.replace("_", "-")] = pipDepTreeEntryForCache
})
}
linesOfRequirements.forEach( (dep) => {
// if matchManifestVersions setting is turned on , then
if(matchManifestVersions === "true") {
let dependencyName
let manifestVersion
parsedRequirements.forEach(({ name: depName, version: manifestVersion }) => {
if(matchManifestVersions === "true" && manifestVersion != null) {
let installedVersion
let doubleEqualSignPosition
if(dep.includes("==")) {
doubleEqualSignPosition = dep.indexOf("==")
manifestVersion = dep.substring(doubleEqualSignPosition + 2).trim()
if(manifestVersion.includes("#")) {
let hashCharIndex = manifestVersion.indexOf("#");
manifestVersion = manifestVersion.substring(0,hashCharIndex)
if(CachedEnvironmentDeps[depName.toLowerCase()] !== undefined) {
if(usePipDepTree !== "true") {
installedVersion = getDependencyVersion(CachedEnvironmentDeps[depName.toLowerCase()])
} else {
installedVersion = CachedEnvironmentDeps[depName.toLowerCase()].version
}
dependencyName = getDependencyName(dep)
// only compare between declared version in manifest to installed version , if the package is installed.
if(CachedEnvironmentDeps[dependencyName.toLowerCase()] !== undefined) {
if(usePipDepTree !== "true") {
installedVersion = getDependencyVersion(CachedEnvironmentDeps[dependencyName.toLowerCase()])
} else {
installedVersion = CachedEnvironmentDeps[dependencyName.toLowerCase()].version
}
}
if(installedVersion) {
if (manifestVersion.trim() !== installedVersion.trim()) {
throw new Error(`Can't continue with analysis - versions mismatch for dependency name ${dependencyName} (manifest version=${manifestVersion}, installed version=${installedVersion}).If you want to allow version mismatch for analysis between installed and requested packages, set environment variable/setting MATCH_MANIFEST_VERSIONS=false`)
}
}
if(installedVersion) {
if (manifestVersion.trim() !== installedVersion.trim()) {
throw new Error(`Can't continue with analysis - versions mismatch for dependency name ${depName} (manifest version=${manifestVersion}, installed version=${installedVersion}).If you want to allow version mismatch for analysis between installed and requested packages, set environment variable/setting MATCH_MANIFEST_VERSIONS=false`)
}
}
}
let path = new Array()
let depName = getDependencyName(dep)
//array to track a path for each branch in the dependency tree
let path = []
path.push(depName.toLowerCase())
bringAllDependencies(dependencies,depName,CachedEnvironmentDeps,includeTransitive,path,usePipDepTree)
bringAllDependencies(dependencies, depName, CachedEnvironmentDeps, includeTransitive, path, usePipDepTree)
})
dependencies.sort((dep1,dep2) =>{
const DEP1 = dep1.name.toLowerCase()
Expand Down Expand Up @@ -350,12 +347,12 @@ function bringAllDependencies(dependencies, dependencyName, cachedEnvironmentDep
version = record.version
directDeps = record.dependencies
}
let targetDeps = new Array()
let targetDeps = []

let entry = { "name" : depName , "version" : version, "dependencies" : [] }
let entry = { "name": depName, "version": version, "dependencies": [] }
dependencies.push(entry)
directDeps.forEach( (dep) => {
let depArray = new Array()
let depArray = []
// to avoid infinite loop, check if the dependency not already on current path, before going recursively resolving its dependencies.
if(!path.includes(dep.toLowerCase())) {
// send to recurrsion the path + the current dep
Expand Down
Loading
Loading