[SCI] add ProjectReference extractor for inter-module dependency analysis for csproj build file trees#178
Conversation
|
🎯 Code Coverage (details) 🔗 Commit SHA: 40db565 | Docs | Datadog PR Page | Give us feedback! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c21eb714b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // entries as ProjectDeps. Each reference is resolved to an absolute filesystem | ||
| // path relative to the .csproj file's directory. Non-existent targets are | ||
| // silently skipped. Duplicates are deduplicated. | ||
| func (e NuGetCsprojExtractor) GetArtifact(f extractor.DepFile, ctx extractor.ScanContext) (*models.ScannedArtifact, error) { |
There was a problem hiding this comment.
When a project has an adjacent packages.lock.json, scanDir selects NuGetLockExtractor; NuGetCsprojExtractor.ShouldExtract returns false for that .csproj, and ExtractDeps only calls GetArtifact on the selected extractor. Because this ProjectReference extraction is only implemented on NuGetCsprojExtractor, locked NuGet projects never populate ProjectDeps, so GetBuildFileTrees(..., FileTypeCsproj) reports empty dependency trees for the common locked-project case. Please wire the lock-extractor/matcher path to run the same csproj artifact extraction.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed. Added GetArtifact() to NuGetLockExtractor in the same file — it finds the adjacent .csproj in the lock file directory and reuses extractCsprojInternalDeps(), so locked projects now populate ProjectDeps the same way as non-locked ones.
28745d8 to
7c7eb16
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c7eb16d16
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Add GetArtifact() and IsManifestParser() methods to NuGetCsprojExtractor to extract internal <ProjectReference> dependencies from .csproj files. This enables transitive dependency closure via SimpleProcessor BFS, matching the existing pattern used by BazelBuildExtractor. Changes: - Add ProjectReference type and field to ItemGroup (types.go) - Handle ProjectReference in custom XML unmarshaller (xml-parser.go) - Implement GetArtifact() with path resolution, deduplication, and existence checking (csproj-project-refs.go) - Register SimpleProcessor for FileTypeCsproj (simple_processor.go) - Add comprehensive unit and integration tests Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
7c7eb16 to
40db565
Compare
anderruiz
changed the title
anderruiz
changed the title
2 participants
Summary
GetArtifact()method toNuGetCsprojExtractorthat extracts internal<ProjectReference>dependencies from.csprojfiles, resolving relative paths to absolute filesystem paths with backslash normalization, deduplication, and existence checkingSimpleProcessorforFileTypeCsprojto enable transitive dependency closure via BFS, matching the existing Bazel patternProjectReferenceXML type toItemGroupand handle it in the custom XML unmarshallerImplementation Details
Follows the
BazelBuildExtractorpattern exactly:GetArtifact()reads.csprojXML, extracts<ProjectReference Include="...">entries, resolves paths relative to the file's directoryIsManifestParser()returnsfalseArtifactExtractorandManifestExtractorcsproj-project-refs.go(notparse-prefix) to avoid triggeringexpectNumberOfParsersCalledtest countFiles Changed
pkg/extractor/dotnet/types.goProjectReferencestruct and field toItemGrouppkg/extractor/dotnet/xml-parser.goProjectReferencein custom XML unmarshallerpkg/extractor/dotnet/csproj-project-refs.goGetArtifact(),IsManifestParser(), interface checkspkg/extractor/dotnet/csproj-project-refs_test.gopkg/sbomgen/simple_processor.goSimpleProcessorforFileTypeCsprojTest plan
TestProjectReferenceXMLUnmarshal— XML parsing captures ProjectReference entriesTestNuGetCsprojExtractor_GetArtifact_ExtractsProjectDeps— resolves 2 ProjectReference entries to absolute pathsTestNuGetCsprojExtractor_GetArtifact_ResolvesRelativePaths— handles..segments and backslashesTestNuGetCsprojExtractor_GetArtifact_SkipsNonExistent— non-existent targets excludedTestNuGetCsprojExtractor_GetArtifact_DeduplicatesDeps— duplicate references yield one entryTestNuGetCsprojExtractor_GetArtifact_EmptyProjectRefs— no ProjectReference returns empty ProjectDepsTestNuGetCsprojExtractor_IsManifestParser— returns falseTestNuGetCsprojExtractor_Integration_TransitiveClosure— A->B->C chain produces correct direct depsgo test ./...passes with zero failures (no regressions)🤖 Generated with Claude Code