Repository navigation
feat(formats): add csproj format handler for .NET project files - #198
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Adds a new csproj version-file format to FerrFlow so it can read/write .NET project versions from <Version> elements in MSBuild project files.
Changes:
- Introduces
src/formats/csproj.rsimplementing theVersionFiletrait for<Version>…</Version>handling. - Wires the new format into handler dispatch (
get_handler) and updates config/schema to allow"csproj". - Extends existing format dispatch tests to include the new enum variant.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/formats/mod.rs | Exposes the new module and dispatches FileFormat::Csproj to the new handler; expands handler dispatch tests. |
| src/formats/csproj.rs | Implements csproj read/write logic and adds unit tests for basic scenarios. |
| src/config.rs | Adds Csproj to the FileFormat enum for config serialization/deserialization. |
| schema/ferrflow.json | Extends the JSON Schema format enum with "csproj". |
| static VERSION_RE: OnceLock<Regex> = OnceLock::new(); | ||
|
|
||
| fn version_re() -> &'static Regex { | ||
| VERSION_RE.get_or_init(|| Regex::new(r"<Version>([^<]+)</Version>").unwrap()) | ||
| } |
There was a problem hiding this comment.
version_re matches the first <Version>…</Version> anywhere in the file and only when the opening tag has no attributes. This can (a) pick up <Version> elements outside <PropertyGroup> (e.g., a <PackageReference><Version>…</Version></PackageReference> pattern) and (b) fail to match common MSBuild patterns like <Version Condition="…">…</Version>. Consider parsing the XML and selecting the first <PropertyGroup>/<Version> element, or at least tighten the pattern to allow attributes while constraining the match to within a <PropertyGroup> block to avoid updating the wrong version.
| let mut count = 0; | ||
| let new_content = version_re().replace(&content, |_: ®ex::Captures| { | ||
| count += 1; | ||
| format!("<Version>{version}</Version>") | ||
| }); |
There was a problem hiding this comment.
write_version uses Regex::replace, which replaces all <Version> tags, but the PR description (and the tests in xml.rs) indicate only the first tag should be updated. Update the implementation to replace only the first match (e.g., replacen with a limit of 1 or manual slicing) and add an assertion/test that a second <Version> element remains unchanged.
| pub enum FileFormat { | ||
| Csproj, | ||
| #[serde(rename = "gomod")] | ||
| GoMod, | ||
| Gradle, |
There was a problem hiding this comment.
A new FileFormat::Csproj variant was added, but the CLI format prompt/parsing logic in this file is still hard-coded to [toml/json/xml/gradle/gomod/txt] and parse_file_format() doesn't map "csproj" (it currently falls back to Toml). This makes it impossible to select csproj via the interactive config flow and can silently misinterpret user input; update ALLOWED_FORMATS, the prompt text/error message, default_version_file(), and parse_file_format() to include csproj (and extend the existing file_format_serde_all_variants test to cover it).
There was a problem hiding this comment.
Benchmark
Details
| Benchmark suite | Current: c04728f | Previous: e7359fc | Ratio |
|---|---|---|---|
commit_parsing/100 |
17200 ns/iter (± 109) |
18553 ns/iter (± 63) |
0.93 |
commit_parsing/1000 |
175046 ns/iter (± 918) |
187423 ns/iter (± 1447) |
0.93 |
commit_parsing/10000 |
1809136 ns/iter (± 17254) |
1919342 ns/iter (± 49311) |
0.94 |
changelog/build_50 |
7876 ns/iter (± 34) |
7534 ns/iter (± 678) |
1.05 |
changelog/build_500 |
69300 ns/iter (± 256) |
64491 ns/iter (± 629) |
1.07 |
version_files/toml_read |
8431 ns/iter (± 46) |
9555 ns/iter (± 135) |
0.88 |
version_files/toml_write |
137633 ns/iter (± 12740) |
100378 ns/iter (± 13320) |
1.37 |
version_files/json_read |
5334 ns/iter (± 36) |
6345 ns/iter (± 68) |
0.84 |
version_files/json_write |
136025 ns/iter (± 21068) |
93901 ns/iter (± 17169) |
1.45 |
version_files/xml_read |
5107 ns/iter (± 22) |
5969 ns/iter (± 131) |
0.86 |
version_files/xml_write |
132919 ns/iter (± 9711) |
93639 ns/iter (± 4765) |
1.42 |
version_files/gradle_read |
4986 ns/iter (± 25) |
5849 ns/iter (± 27) |
0.85 |
version_files/gradle_write |
133873 ns/iter (± 9248) |
94488 ns/iter (± 7716) |
1.42 |
config_loading/single |
10365 ns/iter (± 71) |
11709 ns/iter (± 271) |
0.89 |
config_loading/mono_10 |
15500 ns/iter (± 283) |
16922 ns/iter (± 127) |
0.92 |
config_loading/mono_50 |
40767 ns/iter (± 405) |
41721 ns/iter (± 821) |
0.98 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: c04728f | Previous: e7359fc | Ratio |
|---|---|---|---|
version_files/toml_write |
137633 ns/iter (± 12740) |
100378 ns/iter (± 13320) |
1.37 |
version_files/json_write |
136025 ns/iter (± 21068) |
93901 ns/iter (± 17169) |
1.45 |
version_files/xml_write |
132919 ns/iter (± 9711) |
93639 ns/iter (± 4765) |
1.42 |
version_files/gradle_write |
133873 ns/iter (± 9248) |
94488 ns/iter (± 7716) |
1.42 |
This comment was automatically generated by workflow using github-action-benchmark.
Add a
csprojformat handler to read and write<Version>tags in .NET project files.src/formats/csproj.rsimplementing theVersionFiletrait<Version>elements (capital V), ignoringVersionattributes on<PackageReference>elements<Version>tag (consistent with the xml handler behavior).csproj(C#),.fsproj(F#), and.vbproj(VB.NET) since they share the same structureFileFormatenum, handler dispatch, schema, and testsConfig usage
{ "package": [{ "versionedFiles": [ { "path": "src/MyApp/MyApp.csproj", "format": "csproj" } ] }] }Closes #30