BrighterCommand / BrighterCommand/Brighter

Doc-comment diagnostics are globally suppressed in src/Directory.Build.props, so malformed XML and dangling crefs are invisible

Open
#4,348 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C#
Stars
2.5k
Forks
296
Avg merge
1d 11h
Merged PRs (30d)
21

Description

`src/Directory.Build.props:24`:

```xml
$(NoWarn);CS0419;CS1570;CS1571;CS1572;CS1573;CS1574;CS1580;CS1581;CS1584;CS1587;CS1589;CS1590;CS1591;CS1592;CS1658;CS1711;CS1723;CS1734
```

`CS1591` (missing XML comment) is a reasonable thing to suppress — you do not want a warning for every undocumented member. The rest are different in kind: they fire when a doc comment **is** written and is **wrong**.

- `CS1574` / `CS1580` / `CS1584` — a `` that does not resolve
- `CS1570` / `CS1658` — badly formed XML in a doc comment
- `CS1573` / `CS1572` — a `` naming a parameter that does not exist

These are exactly the diagnostics that catch documentation drifting away from the code it describes. Suppressed globally, a `cref` keeps pointing at a member that was renamed or deleted and nothing ever says so. #4302 fixed one of these by hand — an XML comment pointing at `CommandProcessorBuilder.Policies`, a member that no longer exists — and it was found by reading, not by the build.

### Measured

Rebuilding **one project, one target framework** with everything except `CS1591` re-enabled:

```text
$ dotnet build src/Paramore.Brighter/Paramore.Brighter.csproj -p:NoWarn=CS1591 -t:Rebuild -f netstandard2.0

16 CS1658 12 CS1570 8 CS1584 8 CS1041 8 CS1003
52 diagnostics across 14 distinct source locations
```

The locations are concentrated rather than scattered — `IAmAnInboxAsync.cs:70` and `InMemoryInbox.cs:276` account for 8 of them between them, `MessageBatch.cs:14-15`, `Channel.cs:165`, `IAmAMessageScheduler.cs:4`, `IAmARequestScheduler.cs:4`, `FileSystemStorageProvider.cs:42` for most of the rest.

**I have not measured the whole solution**, and want to be honest about why rather than quote a number I cannot stand behind: with the suppression lifted these are promoted to errors, so a solution-wide build aborts at the first project instead of reporting a total. A full census needs the suppression lifted project by project.

### Suggested approach

Not "turn them all on", which would fail the build immediately. Rather:

1. Split the list — keep `CS1591` suppressed, take the correctness ones out.
2. Re-suppress them per-project with a `` in each `.csproj` that still has debt, so the list of projects still carrying it is visible and shrinking.
3. Fix a project, delete its line.

That way the ones already clean stay clean, and the build starts objecting the next time a `cref` goes stale.

Contributor guide

Open the contributing guide

Research direction

Start at src/Directory.Build.props:24 and inspect the listed NoWarn diagnostics, then reproduce the issue with the documented Paramore.Brighter.csproj netstandard2.0 rebuild command. Review the reported locations, including IAmAnInboxAsync.cs, InMemoryInbox.cs, MessageBatch.cs, Channel.cs, IAmAMessageScheduler.cs, IAmARequestScheduler.cs, and FileSystemStorageProvider.cs. Done means CS1591 remains suppressed, correctness diagnostics are visible per project, and the reported documentation errors are fixed.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
build-system, documentation
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.