Skip to content

Commit 955f273

Browse files
PhysShellPhysShell
committed
docs: bring P-028 (unneeded-dependency-profile) in from main
Same rebase as P-026/P-027 — copied verbatim from main.
1 parent 4cb620d commit 955f273

1 file changed

Lines changed: 155 additions & 0 deletions

File tree

‎docs/proposals/P-028-unneeded-dependency-profile.md‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
# P-028 — Unneeded-dependency profile (`Own.Lean`)
2+
3+
- **Status:** draft — not started.
4+
- **Depends on:** [P-001](P-001-csharp-extractor.md) (the Roslyn extractor
5+
seam), [P-006](P-006-di-lifetimes.md) (the DI `services[]` registration
6+
graph — YDN002 extends its facts with the closed generic arguments the
7+
existing graph collapses away, see Sketch), [P-015](P-015-configuration-surface.md)
8+
(severity/opt-in surface for the phase-2 heuristics). Bounded explicitly
9+
against [P-021](P-021-async-audit-pack.md) (`ASYNC040` already owns the
10+
"trivial async passthrough" case — not duplicated here) and
11+
[P-023](P-023-architecture-guard.md) (Own.Arch gates *forbidden* structure;
12+
this profile flags *provably redundant* structure — different verdict shape,
13+
never a build gate).
14+
15+
## Motivation
16+
17+
The `you-dont-need/You-Dont-Need` meta-list (a curated collection of
18+
"You Might Not Need Lodash/Moment/Redux/…" write-ups) makes one real point
19+
under all the individual takes: teams often reach for a popular dependency
20+
because it is popular, not because the problem in front of them needs it. The
21+
honest version of that point is not "dependencies are bad" — it is that every
22+
dependency has to clear a bar:
23+
24+
```text
25+
dependency_value > dependency_cost
26+
```
27+
28+
where cost is never just install size — it is maintenance, transitive CVEs,
29+
build complexity, onboarding, and the debugging friction of an indirection
30+
layer nobody on the team wrote. .NET has its own instances of the same
31+
pattern: an `AutoMapper` profile that copies five identically-named properties
32+
and nothing else, a `MediatR` handler with exactly one implementation and no
33+
pipeline behaviours standing in for a direct method call. The libraries are
34+
not the problem — using them where they buy nothing is.
35+
36+
The trap is that "you don't need X" is trivially easy to turn into an
37+
opinionated hot-take generator (see the source list's own "You Might Not Need
38+
TypeScript" entry) that flags a library's mere presence. That is exactly the
39+
kind of noisy, ungrounded quality gate this project's other proposals
40+
deliberately reject (see P-023's "no SOLID detector" stance). So the scope
41+
here is narrower and stricter than the inspiration:
42+
43+
> **Own.Lean never passes judgment on a library. It flags one call site at a
44+
> time, only when the code at that site proves the abstraction added nothing
45+
> — and the moment any real customization is visible, it stays silent.**
46+
47+
## Scope
48+
49+
### MVP — deterministic, evidence-only
50+
51+
| Code | Finding | Evidence required | Suggestion |
52+
|------|---------|--------------------|------------|
53+
| `YDN001` | `AutoMapper` `CreateMap<TSrc,TDest>()` (or `Profile`-declared map) that is a pure 1:1 copy | every public writable member of `TDest` has an exact-name, assignable-type public readable counterpart on `TSrc`; **no** `.ForMember`/`.Ignore`/`.ConvertUsing`/custom value resolver/`.ReverseMap`; member count within a configurable bound (default 10) | replace with an explicit object initializer or a mapping constructor |
54+
| `YDN002` | `MediatR` `IRequestHandler<TReq[,TResp]>` resolved via `ISender`/`IMediator` with exactly one registered implementation and zero registered `IPipelineBehavior<,>` (open or closed) anywhere in the DI graph | needs the DI registration graph's generic arguments preserved per `IRequestHandler<TReq[,TResp]>` registration (see Sketch — today's graph collapses these) | inject the handler directly instead of dispatching through the mediator |
55+
56+
`YDN001` is structurally the same shape already used for `DI001` (P-006): read
57+
a graph the extractor already builds, compare cardinalities and declared
58+
customization, emit a verdict only when the customization set is empty.
59+
`YDN002` needs the same shape but over a graph the extractor does not yet
60+
build in the needed resolution — see Sketch. Neither rule inspects call-site
61+
*style* — only the declared shape of the mapping/registration.
62+
63+
### Phase 2 — heuristic, report-only, opt-in via P-015
64+
65+
| Code | Finding | Confidence |
66+
|------|---------|------------|
67+
| `YDN010` | A DI-registered service with exactly one registration across the whole solution, not exposed as a public extension point, and never re-registered in a test project | heuristic — a real single-impl service and a "this interface is pure ceremony" service look identical without knowing intent; ships as report-only or not at all |
68+
69+
This tier stays report-only, never a build gate, and is the honest limit of
70+
what this profile should attempt — see Non-goals for the parts of the "You
71+
Don't Need" list that were deliberately left out rather than downgraded to
72+
Phase 2.
73+
74+
## Non-goals
75+
76+
- **No library blocklist.** "Don't use Lodash/Axios/Moment" has no .NET
77+
analogue that would be evidence rather than opinion, and even in spirit,
78+
Own.Lean does not ship a list of disfavoured packages. Every finding names a
79+
specific call site and the specific evidence at it.
80+
- **No "replace the ORM with hand-written SQL" suggestion.** Whether a
81+
hand-rolled query beats an ORM call depends on performance requirements this
82+
tool cannot observe statically. Not evidence-based; not built.
83+
- **No reflection → source-generator suggestion.** There is no oracle for
84+
"this reflection could have been codegen'd" short of writing the generator —
85+
guesswork, not a finding.
86+
- **No JSON → MessagePack/binary-format suggestion.** A wire-format choice
87+
depends on external constraints (interop, human-readability requirements)
88+
invisible to static analysis.
89+
- **No reimplementation of existing Roslyn/FxCop LINQ micro-optimizations**
90+
(`.Where(p).Count()` → `.Count(p)` and siblings — already `CA1826`/`CA1827`/
91+
`CA1828`/`CA1829`). Own.NET's differentiator is checks nobody else runs, not
92+
a third copy of a rule two analyzers already ship.
93+
- **No build-blocking severity, ever, for this family.** Every `YDN###` is
94+
info/warning and never wired into the P-023 architecture-guard ratchet. A
95+
false positive here costs a reviewer one comment, not a red PR.
96+
- **No bundle-size / transitive-CVE / dependency-count scoring.** That is a
97+
supply-chain audit tool (NuGet advisory scanning, dependency-graph size),
98+
a different project; if ever pursued it is its own proposal, not folded in
99+
here.
100+
- **No hostility to AutoMapper or MediatR as libraries.** Both are legitimate
101+
the moment they are used for what they are for — custom resolvers, cross-
102+
cutting pipeline behaviours, polymorphic dispatch over many handlers.
103+
`YDN001`/`YDN002` are silent the instant any of that evidence appears.
104+
105+
## Sketch
106+
107+
```text
108+
C# source --[Roslyn extractor]--> mapping-profile facts (YDN001)
109+
\-> services[] graph, extended with closed generic args (YDN002)
110+
|
111+
[core: same Python seam]
112+
|
113+
YDN### verdicts --> SARIF + markdown
114+
```
115+
116+
`YDN001` needs one new extractor fact family: for each `CreateMap<TSrc,TDest>`
117+
call (or `Profile`-declared map), emit the two member lists plus whichever
118+
customization calls (`.ForMember`, `.Ignore`, `.ConvertUsing`, `.ReverseMap`)
119+
appear in the same fluent chain.
120+
121+
`YDN002` is **not** a free ride on the existing P-006 `services[]` graph, and
122+
the MVP scope above was wrong to claim otherwise (caught in review): the
123+
extractor's `DiTypeName` helper
124+
(`frontend/roslyn/OwnSharp.Extractor/Program.cs`) deliberately reduces a
125+
generic registration to its rightmost identifier — `IRequestHandler<Foo,Bar>`
126+
and `IRequestHandler<Baz,Qux>` both become the bare `IRequestHandler` — because
127+
P-006's captive-lifetime checks never needed to distinguish closed generic
128+
arguments. `YDN002` does need that distinction: counting "implementations of
129+
*this* `TReq[,TResp]`" from an identifier-only graph would silently count
130+
every unrelated handler in the solution as the same bucket the moment a
131+
project has more than one MediatR request. The fix is a small, additive
132+
extractor change — preserve the closed type-argument pair (and the
133+
`IPipelineBehavior<,>` type arguments, open or closed) alongside the existing
134+
service/impl identifiers when the generic is one of the MediatR marker
135+
interfaces — not a reinterpretation of the current collapsed facts.
136+
137+
## Open questions
138+
139+
1. Where does the "still trivial" member-count bound for `YDN001` live —
140+
hardcoded default, or a P-015 per-project knob? Leaning: a default with a
141+
P-015 override, consistent with how severity is already configured
142+
elsewhere.
143+
2. Does `YDN002` also need to inspect the handler body for inline cross-
144+
cutting code (logging/validation) that a pipeline behaviour would normally
145+
own, or is DI-graph evidence (impl count + behaviour count) sufficient on
146+
its own? Needs a trial against a real MediatR-using corpus sample.
147+
3. Naming/positioning: a standalone `Own.Lean` family, or a phase-4 "ceremony"
148+
tier under `Own.Arch` (P-023)? Leaning: standalone — P-023 gates *forbidden*
149+
structure (a graph-edge violation); this profile flags *provably redundant*
150+
structure (an indirection with zero customization). The verdict shapes
151+
differ (a gate vs. a suggestion), which argues for keeping them separate
152+
families sharing only the extractor seam.
153+
4. Prefix: following the `ASYNC`/`ARCH`/`OBL` precedent of a family-specific
154+
code rather than overloading `OWN###` — `YDN###` as proposed above, unless
155+
a shorter/clearer prefix surfaces during naming review.

0 commit comments

Comments
 (0)