Repository navigation
Release 3.0.1: CRAP completeness and exclusion corrections - #79
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (5)
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: c6483619b7
ℹ️ 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".
|
Addressed the two follow-up review findings in 511ea1e:
Added 12 regression cases through parser, analysis and CLI entrypoints, including a genuinely missing executable method that must still return NODATA. CI is running at https://github.com/ANcpLua/dotcov/actions/runs/38373024032; local CoreCLR startup remains blocked by 0x8007000E. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 511ea1e6a7
ℹ️ 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".
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: 511ea1e6a7
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
|
Addressed the latest P1 and advisory in 84f449d. Exclusion accounting now consumes only a unique arity-compatible metrics candidate; it no longer uses scoring's multi-candidate fallback. Added seven regression cases, including the CLI same-arity overload false PASS. Retained excluded methods now use a hash-set path lookup. The maintainer approved proceeding with the patch release. This commit also aligns VersionPrefix, badge default, README references and release notes for 3.0.1. CI must pass before merge/publication. @codex review |
|
The release-candidate build exposed new October 13 NuGet advisories in Fallout's existing transitive dependencies. fc66e14 pins System.Security.Cryptography.Pkcs and System.Security.Cryptography.Xml to the published patched 10.0.13 versions via central transitive pinning. Audit remains enabled and warnings remain errors; the packed Fallout dependency constraints will carry the fixes to consumers. The preceding run's build failed on NU1902/NU1903, before tests could establish validation. Waiting for the new CI run. @codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc66e14e10
ℹ️ 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".
|
Addressed both findings on fc66e14: measured overloads now consume only a unique executable metrics candidate, while conservative complexity scoring remains unchanged; zero-body declarations do not create ambiguity for either measured or excluded matches. Added library/CLI regressions and updated the unknown-signature expectation and 3.0.1 notes. @codex review the latest head. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 854bbd9256
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 242aed90c0
ℹ️ 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".
…ndows
- A metrics complexity below 1 on an executable member is no measurement: the reader skips
it like a non-integer value, so the method is unscored (NODATA) instead of scored 0 and
passing.
- Coverlet truncates a function-pointer parameter's signature at its last '(' and `crap`
threw "Unbalanced tuple type". Signatures with an inner parenthesis and metrics
`delegate*` parameters are now unknown, so such methods match by name and arity.
- Escaped identifiers (`@int`) match their metadata type; a generic `T?` that is a value
type in one method and a reference type in another is no longer forced to Nullable<T>
for the whole report.
- GateAwareTableWiringTests use the shared FakeDotnet, so both `dotcov test` gate cases
run on Windows.
- 3.0.1 release notes rewritten around user-visible behaviour.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…known The complexity rule lived in the XML reader, so a library caller passing a CodeMetricsMember with complexity 0 could still score 0 and pass. PrepareMetrics now drops a complexity below 1 unless the member is an explicit zero-body declaration, for every input, and the guards it made redundant go. InvalidTupleStructure_IsRejected pinned "(int))", the shape Coverlet emits for a function pointer, as an error. Cobertura signatures are unknown when they hold parentheses; malformed tuple syntax is still rejected in metrics displays. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d95a97a97f
ℹ️ 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".
…comparable - Treating a metrics delegate* as unknown let it match a method with other parameter types (a false PASS across builds). It now lowers to one canonical delegate*<...> identity; only Coverlet's truncated coverage spelling stays unknown. - Async overloads measured only through their state machines left every metrics row unmatched. When a file holds only such bodies for a member and the row count agrees, the rows are consumed; each body already carries the larger complexity. - --source-root keyed coverage files under the chosen root but left relative metrics paths unrooted when several roots were declared; the parsed report now keeps the chosen root. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11da5dc7e5
ℹ️ 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".
…ual Basic metrics - Dropping rows with a complexity below 1 also removed them from completeness accounting, so an invalid metrics-only row could vanish and the gate pass. Such rows now stay in the table, supply no complexity and count as unmatched. - Tuple structure in metrics displays is validated by the reader, so a malformed member is reported with the metrics file and member name instead of a bare message at analysis time. - Visual Basic displays (Sub/Function ... As T, ByRef, Optional, ParamArray, (Of T), T()) are normalized and match their IL signatures; VB tuples stay unknown. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9fc0b07922
ℹ️ 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".
…ad VB generics - The metrics reader dropped a member whose complexity was missing or malformed, so an incomplete metrics file could pass. Such members stay in the table as 0, which never scores, and count as unmatched. - Rows without a source file served the async bodies of every file, so two files' bodies could consume the same two rows. They now pair only when one file holds such bodies. - Visual Basic generic methods (M(Of T)) and Sub New constructors normalize to the C# shape; the raw display stays as DisplayName. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ANcpLua
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fed7c58a9
ℹ️ 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 = names.GetValueOrDefault(value[(value.LastIndexOf('.') + 1)..]); | ||
| return full is not null && (full == value || full.EndsWith("." + value, StringComparison.Ordinal)) | ||
| ? full : value; |
There was a problem hiding this comment.
When coverage is from void A.M(App.T) but the metrics file is from the generic void A.M<T>(T value), the unique coverage name App.T causes this replacement to qualify the metrics type parameter T as App.T. The generic marker has already been stripped from the member name, so the signatures then match, the wrong metrics row is consumed, and the CRAP gate can PASS for measurements from a different method. Treat declared method type parameters as non-qualifiable, or retain generic-method arity in the identity.
AGENTS.md reference: AGENTS.md:L21-L23
Useful? React with 👍 / 👎.
| return retainArity | ||
| ? TypeParameterLists().Replace(name, static match => | ||
| $"`{BalancedText.CountTopLevelItems(match.Groups[1].Value)}") | ||
| : StripGenericsPerSegment(name); |
There was a problem hiding this comment.
When metrics are generated for a Visual Basic type such as Result(Of T), the enclosing NamedType uses that VB spelling, but this code only strips and counts C# angle-bracket parameter lists. The metrics member is therefore keyed under App.Result(Of T) while Coverlet identifies the declaring type as App.Result1, so every fully covered method or constructor on a generic VB type remains unscored and crap --metricsreturns NODATA. Normalize(Of …)on the enclosing type before deriving bothTypeNameandTypeIdentity; the existing member-display conversion does not process NamedType.Name`. See Roslyn's metrics writer.
Useful? React with 👍 / 👎.
| var byRef = type.StartsWith("ref ", StringComparison.Ordinal) || | ||
| type.StartsWith("out ", StringComparison.Ordinal) || | ||
| type.StartsWith("in ", StringComparison.Ordinal); | ||
| if (byRef) type = type[(type.IndexOf(' ') + 1)..]; | ||
| if (byRef && type.StartsWith("readonly ", StringComparison.Ordinal)) type = type[9..]; |
There was a problem hiding this comment.
For a C# method such as M(scoped ref int value) (and likewise a scoped by-value ref-struct parameter), Roslyn's minimally qualified display includes scoped, but this prefix check recognizes only ref, out, and in. Normalization consequently produces scopedrefSystem.Int32 instead of Coverlet's runtime identity System.Int32&; the known-signature check rejects the measured row and returns NODATA even for the sole fully covered method. Remove scoped before processing the underlying ref kind and type. See Roslyn's minimally qualified format and parameter renderer.
Useful? React with 👍 / 👎.
| var space = BalancedText.LastIndexOfTopLevel(display.AsSpan(0, indexer), ' '); | ||
| if (space < 0) return null; | ||
| valueType = display[..space]; | ||
| if (valueType is "void") return null; |
There was a problem hiding this comment.
When real metrics contain an indexer setter, Roslyn renders its method return type as void while naming the associated indexer and .set accessor (return-type renderer, accessor renderer), so this branch makes every setter/init signature unknown. If coverage contains set_Item(int, string) but the sole same-arity metrics row is for this[bool], that incompatible row is consequently consumed and can let the CRAP gate PASS for a different setter. Carry the enclosing property's value type into child accessor normalization instead of discarding the signature.
AGENTS.md reference: AGENTS.md:L21-L23
Useful? React with 👍 / 👎.
3.0.1 fixes CRAP passes that 3.0.0 could not prove. Line and branch gates are unchanged; some CRAP gates that passed on 3.0.0 now return NODATA (exit 3) or FAIL (exit 1). Full notes:
docs/releases/3.0.1.md.ref readonly,@int, file-local types, function pointers, Visual Basic displays); declarations without a body need no coverage.crap;--source-rootroots relative metrics paths; malformed metrics members name the file and member.GateAwareTableWiringTestsrun theirdotcov testcases on Windows too.Every review finding on #78 and this PR is answered and resolved; each fix has a regression test.
fallout Coverage: 1,819 tests, 100.0% line / 100.0% branch.🤖 Generated with Claude Code