feat(extractor): accept .csproj/.sln input (roslyn-tools CLI-first project resolution) - #139
Conversation
…oject resolution) The first borrowed plyushka from the roslyn-tools tooling shape: the OwnSharp Roslyn extractor now takes a .csproj or .sln as input (positional, or via --project / --solution), not just a .cs file list. ProjectCsFiles resolves a project to its source set the dependency-free way — scan the project directory for *.cs (SDK default-compile-items) plus concrete linked <Compile> files, no MSBuild evaluation; SolutionProjects fans a .sln out over its member projects. This deliberately stops short of the full project/package/reference graph (the ProjectDependencies-category work parked for DI/solution scans). - Program.cs: ProjectCsFiles + SolutionProjects helpers, wired into Expand; --project/--solution flag aliases; usage text updated. - New project-input-sample/ (outside samples/ so it doesn't perturb the directory-scan jobs): an SDK csproj + an injected-bus event leak. - CI wpf-extractor: a step pinning the .csproj path to a golden, asserting the leak surfaces as OWN001 and that positional == --project. - README + roslyn-tools-and-cli note: document the input forms; mark this plyushka landed and re-aim the next PR at extract/check/explain subcommands. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KzA6KGQVRzP52tBEKwJ3SW
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe extractor now accepts ChangesRoslyn project input support
Sequence Diagram(s)sequenceDiagram
participant ownsharp-extract
participant Expand
participant ProjectCsFiles
participant SolutionProjects
ownsharp-extract->>Expand: raw input list
Expand->>ProjectCsFiles: resolve .csproj input
ProjectCsFiles-->>Expand: .cs files from scan and linked <Compile>
Expand->>SolutionProjects: resolve .sln input
SolutionProjects-->>Expand: member .csproj paths
Expand-->>ownsharp-extract: expanded .cs file set
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:a2c7d6270e
ℹ️ 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".
| var full = Path.GetFullPath(csproj); | ||
| var dir = Path.GetDirectoryName(full) ?? "."; | ||
| var result = new List<string>(); | ||
| foreach (var f in Directory.EnumerateFiles(dir, "*.cs", opts)) |
There was a problem hiding this comment.
When a direct .csproj input is misspelled or otherwise absent, ProjectCsFiles derives the parent directory and enumerates every *.cs under it before XDocument.Load(full) reports the missing project. For example, ownsharp-extract --project src/Missing.csproj will analyze src/**/*.cs (or throw if src also does not exist) instead of skipping/failing the bad input, so a typo can produce diagnostics for an unintended source set.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 84e4d6e. ProjectCsFiles now guards on File.Exists(full) before deriving the directory: a missing project warns (project not found: …) and returns empty, so a typo'd --project src/Missing.csproj no longer analyses src/**/*.cs. The directory-scan fallback is now reserved for a present-but-malformed project (one whose <Compile> links we couldn't read), never an absent one.
Generated by Claude Code
…odex P2) ProjectCsFiles built the parent-directory *.cs scan before loading the project XML, so a typo'd `--project src/Missing.csproj` would analyse all of src/**/*.cs (or throw if src is also absent) instead of skipping the bad input. Guard on File.Exists up front: a missing project warns and returns empty. The directory-scan fallback now applies only to a present-but-malformed project (whose <Compile> links could not be read), never an absent one. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KzA6KGQVRzP52tBEKwJ3SW
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 964-970: The current CI check in the workflow only compares the
rendered `ownlang ownir` diagnostics, which can hide differences between the
positional input and the `--project` path. Update the parity assertion around
the `dotnet run` invocation for OwnSharp.Extractor to compare the canonicalized
`proj-facts.json` and `proj-facts-flag.json` outputs directly, using the
existing `proj-facts*` artifacts, so the test verifies extractor equivalence at
the fact boundary rather than after Python normalization.
In `@frontend/roslyn/OwnSharp.Extractor/Program.cs`:
- Around line 125-167: The ProjectCsFiles source-set resolution is
over-inclusive because it scans every in-tree *.cs before considering the
project’s explicit compile set. Update ProjectCsFiles to derive files from the
.csproj’s actual Compile items and respect exclusions like
EnableDefaultCompileItems=false and <Compile Remove=...>, while still handling
explicit linked <Compile Include=...> entries; keep the existing unique paths
logic in ProjectCsFiles, IsSkipped, and the XML parsing flow, but change the
default scan to not treat all in-tree files as compiled input.
- Around line 193-196: The solution-entry parsing in
Program.ParseSolutionEntries is splitting on raw commas, which breaks quoted
`Project(...)= "Name", "relpath", "{guid}"` lines when names or paths contain
commas. Update the parsing logic to be quote-aware in this section so it
extracts the project path from the second quoted field without using a naive
Split(','); keep the change localized around the existing `t.Substring(eq + 1)`
/ `parts` handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 74c45e07-fccb-4348-9fc2-f4ccaf722f3c
📒 Files selected for processing (6)
.github/workflows/ci.ymldocs/notes/roslyn-tools-and-cli.mdfrontend/roslyn/OwnSharp.Extractor/Program.csfrontend/roslyn/README.mdfrontend/roslyn/project-input-sample/CustomerSubscription.csfrontend/roslyn/project-input-sample/ProjectInputSample.csproj
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
… fact-level CI parity (CodeRabbit)
Addresses three CodeRabbit review findings on the .csproj/.sln input PR:
- ProjectCsFiles (Major): no longer treats every in-tree *.cs as compiled. It now
honours <EnableDefaultCompileItems>false> (switch to include-driven) and
<Compile Remove="..."> (subtract excluded files), matched via a small glob
translator (SpecMatcher: concrete paths, *.cs, **/*.cs, Folder/**) — so .csproj
input doesn't emit findings from files the project doesn't compile. Still no
MSBuild evaluation.
- SolutionProjects (Major): replace the naive Split(',') with quote-aware field
extraction (QuotedFields), so a comma inside a quoted project name/path no longer
misreads the line.
- CI (Minor): assert positional == --project parity at the OwnIR fact boundary
(jq -S canonicalized facts diff) instead of after Python normalization.
README updated to describe the honoured exclusions.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KzA6KGQVRzP52tBEKwJ3SWThere was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/roslyn/OwnSharp.Extractor/Program.cs`:
- Around line 132-155: Add a matchTimeout when constructing the Regex inside
SpecMatcher so glob-derived patterns cannot hang on pathological inputs. Update
the regex creation in Program.SpecMatcher to use a bounded timeout while keeping
the existing IgnoreCase behavior and path-relative matching logic unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a613495a-891e-469b-b691-7421ca0ddc7f
📒 Files selected for processing (3)
.github/workflows/ci.ymlfrontend/roslyn/OwnSharp.Extractor/Program.csfrontend/roslyn/README.md
✅ Files skipped from review due to trivial changes (1)
- frontend/roslyn/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci.yml
Uh oh!
There was an error while loading. Please reload this page.
…deRabbit ReDoS) .csproj <Compile Include>/<Compile Remove> globs are untrusted in CI; adjacent wildcards (**/**, **/*) translate to ambiguous adjacent quantifiers the backtracking .NET regex engine can blow up on. Construct the Regex with a 1s matchTimeout, and catch RegexMatchTimeoutException in the predicate so a pathological glob degrades to "no match" with a warning instead of crashing the extractor. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KzA6KGQVRzP52tBEKwJ3SW
Uh oh!
There was an error while loading. Please reload this page.
Что и зачем
Prompted by "could
dotnet/roslyn-toolsgive us a ready-made C# frontend?" — the verdict (indocs/notes/roslyn-tools-and-cli.md) is no: it's engineering scaffolding, not an analysis source. This PR ships the first borrowable plyushka from its tooling shape: CLI-first project/solution input.The OwnSharp Roslyn extractor previously accepted only
.csfiles/dirs, while the README/note already advertised--project App.csproj. This makes that real, keeping the seam intact: the C# side stays a fact extractor (emits OwnIR facts); the verdict still comes from the Python core (python -m ownlang ownir). A.csprojonly changes which files are read, not who decides ownership.Changes
OwnSharp.Extractor/Program.cs:ProjectCsFiles— resolves a.csprojto its source set dependency-free (directory scan of*.cs, SDK default-compile-items), honouring<EnableDefaultCompileItems>false>and<Compile Remove="...">via a small glob translator (SpecMatcher), plus concrete linked<Compile Include>outside the tree. No MSBuild evaluation.SolutionProjects— quote-aware parse of a classic.sln, fanning out over member projects.Expand;--project/--solutionflag aliases beside the positional form.frontend/roslyn/project-input-sample/— an SDK.csproj+ an injected-bus event leak (outsidesamples/so it doesn't perturb directory-scan CI jobs).wpf-extractor: a step assertingOWN001surfaces via the.csproj, with positional==--projectparity at the OwnIR fact boundary (jq -S).extract/check/explainsubcommands.The full project/package/reference graph (real MSBuild evaluation) is deliberately deferred — that's the
ProjectDependencies-category work for DI/solution scans, not the v0 leak extractor.Тип изменения
.csproj/.slninputПроверка
dotnetin the dev sandbox, so the C# extractor compiles/runs only in CI — the newwpf-extractorstep exercises the.csprojpath there. New code is file/XML/text only (no Roslyn-analysis changes) to bound that blind spot.CustomerSubscription.cs:12: warning: [OWN001] …, matching the CI assertions.SpecMatcherglob translation was validated against the common forms (*.cs,**/*.cs,Folder/**, concrete paths) before commit.ci.yml(YAML) and the new.csproj(XML) parse cleanly.Связанный issue
Нет связанного issue — продолжение P-001 /
docs/notes/roslyn-tools-and-cli.md(the "Next PR" named there).Чеклист
Review follow-ups addressed
.csprojno longer directory-scans the parent (84e4d6e).ProjectCsFileshonours explicit compile-set exclusions (3ec8bc7)..slnparsing is quote-aware, not comma-split (3ec8bc7).jq -S(3ec8bc7).🤖 Generated with Claude Code
Summary by CodeRabbit
.csprojand.slninputs via both positional paths and--project/--solutionflags..csproj/.slnexpansion: handles default compile items, explicit<Compile>links, skip rules, parse failures (falls back), and missing/unreadable inputs (skips with warnings)..csprojinput mode correctness and ensures flag vs positional invocations produce identical extracted outputs.