feat(justdummies): add JD011-JD013 for recipes reaching value positions - #370
Merged
Merged
Conversation
Completes the JustDummies.Usage category with the three remaining ways a generator stands in for the value it would draw. JD011 covers the residue ADR-0020 could not close. Generators are reference types, so an object, dynamic or params object[] position takes one with no conversion — there was nothing to remove there. The recipe then survives as an opaque object: Assert.NotNull(Any.String()) is green for ever, a theory row built as object[] feeds the recipe to the code under test, and gen.Equals(value) is false for every run and seed. JD012 and JD013 are the two Any.OneOf traps. Pooling generators infers the builder type as the element type, so drawing yields a recipe; and a held collection binds T to the collection itself, making a pool of one whose every draw returns the same list. ADR-0032 recorded the second as an accepted risk "mitigated by ElementOf being the documented path" — this turns that mitigation into a check. JD011 ships opt-in (isEnabledByDefault: false), which is a measurement and not a preference. ADR-0059's follow-up required its default to be decided on dogfooding evidence; across this repository's suites it found no true positive and two false ones, both in a convention test that collects generators into a List<object> on purpose. That shape cannot be told apart from the theory-row mistake the rule targets, so it cannot be narrowed away — and the rule earns its keep in a consumer suite instead, where object-typed assertion helpers are common and reflection over generators is not. Two exclusions came out of that same pass and are covered by tests citing the real sites: a comparison between two generators is never reported, because ReferenceEquals(original, narrowed) is how the property suite proves a constraint returned a new recipe, and Generate() there would destroy the property under test. The test harness gains the opt-in mechanism FirstClassErrors' harness already had, so a disabled-by-default rule can be forced on for its own tests exactly as an .editorconfig would. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GKqyhG9Y4AfdxEYekP2Evq
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Third wave of the analyzer study, completing the
JustDummies.Usagecategory: the three remaining ways a generator stands in for the value it would draw. JD011 ships opt-in, and that is the measurement ADR-0059's follow-up demanded, not a preference — see below.Type of change
Changes
GeneratorWhereValueExpected(Warning, opt-in) — the residue ADR-0020 could not close. Generators are reference types, so anobject,dynamicorparams object[]position takes one with no conversion; there was nothing to remove there.Assert.NotNull(Any.String())is green for ever, a theory row built asobject[]feeds the recipe to the code under test, andgen.Equals(value)is false for every run and every seed.GeneratorPooledAsValue(Warning) —Any.OneOf(Any.Int32(), Any.Int32())infers the builder as the pool's element type, so drawing yields a recipe. A trap rather than an obvious slip because the surface is inconsistent: pooled generators of different types fail inference and the compiler stops you, while two of the same type bind cleanly.HeldCollectionPassedToOneOf(Warning) — a held collection bindsTto the collection itself, making a pool of one whose every draw returns the same list. ADR-0032 recorded this as an accepted risk "mitigated byElementOfbeing the documented path"; this turns that mitigation into a check.FirstClassErrors.Analyzers.UnitTestsalready had, so a disabled-by-default rule can be forced on for its own tests exactly as an.editorconfigwould.Why JD011 ships opt-in
ADR-0059's follow-up said: "Decide the remaining
object-position rule on dogfooding evidence gathered from this repository's own suites, not before." So I measured before choosing.ReferenceEqualsexclusionThe four that disappeared were
ReferenceEquals(original, narrowed)in the interval property tests — that is how the suite proves a constraint returned a new recipe rather than mutating the receiver, andGenerate()there would destroy the property under test. MyEqualsbranch already excluded generator-vs-generator; the conversion branch had to draw the same line, and now does.The two that remain are in
SizeGuardConventionTests.cs, which collects generators into aList<object>on purpose to enumerate every generator shape. That is the same shape as theobject[]theory row the rule exists to catch, so it cannot be narrowed away.Net: no true positive, two false positives. That is weak evidence for enabling it everywhere and good evidence that it belongs in a consumer's suite, where
object-typed assertion helpers are common and reflection over generator instances is not. HenceisEnabledByDefault: false, with the enabling snippet in the help page. Flip it if a consumer suite ever provides better evidence — a unit test pins the current choice so the flip is deliberate.JD012 and JD013 produced zero hits and ship enabled.
Dogfooding
Run over
JustDummies.UnitTests,JustDummies.PropertyTests,JustDummies.Xunit.UnitTests,FirstClassErrors.Testing.UnitTests,FirstClassErrors.UnitTestsandFirstClassErrors.RequestBinder.UnitTests. A planted positive control confirmed JD012 and JD013 fire at the expected locations and that JD011 stays silent by default; the control was then removed.Testing
dotnet build FirstClassErrors.sln— 0 warnings, 0 errorsdotnet test FirstClassErrors.sln— 2 003 tests, 0 failures across all 13 test projectsFirstClassErrors.Analyzers.UnitTests, 132) — the rules added here live inJustDummies.Analyzers.UnitTests(86)Mutation testing was not run locally.
Documentation
doc/updated —JD011–JD013rule pages and the rule index, with JD011 marked opt-in and carrying its enabling snippetJD011.fr.md–JD013.fr.mdand the French entries ofREADME.fr.mdArchitecture decisions
Proposed: ADR-____No new decision: this executes ADR-0059's follow-up action rather than deciding anything fresh. If you consider that follow-up now discharged, its wording is the place to say so — but that is an edit to a
ProposedADR and therefore yours, not mine.Related issues
None. Third wave of the analyzer-opportunity study; #368 and #369 shipped the first two. Fifteen catalogued rules remain, all in the constraint-decidability family, which needs the shared fluent-chain and constant-folding helpers first.
🤖 Generated with Claude Code
https://claude.ai/code/session_01GKqyhG9Y4AfdxEYekP2Evq
Generated by Claude Code