Proposal: Permanently disable subjective "Quality of content" rules

[sharwell]

Several documentation rules are subjective and aim to establish the quality of documentation content. These rules take CPU time to evaluate and are an overall poor indicator of documentation quality. I believe with the rules in place, developers will be focused on the wrong aspects of documentation during the development process.

• SA1628: Documentation text must begin with a capital letter
• SA1630: Documentation text must contain whitespace
• SA1631: Documentation must meet character percentage
• SA1632: Documentation text must meet minimum character length
• SA1650: Element documentation must be spelled correctly

I propose that each of these rules be permanently disabled in StyleCopAnalyzers.

[pdelvo]

I would add

• SA1650: Element documentation must be spelled correctly

to that list.

I agree that a automatic tool should not be in charge to decide if documentation is usefull or not.

[sharwell]

@pdelvo I added SA1650 to the list. You are correct, other tools are more suitable for that.

[Noryoko]

👍

[hickford]

Oh yes. We have volumes of nonsense undocumentation because "the tool made me write something"

/// <summary>
/// The count.
/// </summary>
public int Count {get;}

Dracula?

[whut]

Maybe those rules are not relevant in case of most of the development projects, but what about frameworks? When developing a framework for use by external developers the quality of documentation is very important. I use StyleCop SA1650 spell check to ensure that we do not do any spelling mistakes. Other rules noted here are indeed not that useful, but are better than nothing. Of coure passing those rules does not mean that documentation is good:)

@sharwell what other tools are good for spell check in documentation comments? There is a Visual Studio Spell Checker extension, but it slows down Visual Studio a lot for files with lots of comments and spell checks not only documentation comments but also regular comments, where spelling errors are indeed a non-issue and reporting them is just a noise

[sharwell]

Hi @whut, and welcome to the project 😄

Quality of documentation is certainly a goal to strive for. The problem with these rules is they provide little to no assurance that documentation is good. Since StyleCopAnalyzers is meant to run not only during every build but also while typing in the editor, performance is important and we need to make sure each enabled rule provides clear value to the project.

I use the same extension for spell checking. I imagine it could be rewritten as an analyzer dedicated to spell checking for users who wanted to use it.

[RobSiklos]

For the record, I agree completely with the comment made by @whut. We have a huge API, and there is no good tool (other than StyleCop SA1650) for finding spelling mistakes the API docs. It's one thing to say that you won't check spelling because it's a performance nightmare, but I don't agree that the rule should be disabled because it's not useful. It is incredibly useful.

Having spelling mistakes in public API docs makes a project look sloppy and unprofessional. StyleCop (classic) has been an ENORMOUS help ensuring that the docs are at least spelled correctly.

Please reconsider finding a way to add back SA1650.

[fancyDevelopment]

I also agree to @whut and @RobSiklos and vote for bringing back SA1650 and alle the other permanently disabled rules. I don't understand the problem and the discussion about the question whether this rule is usefull or not. Each one who feels this rule is useless or takes to much performance can easliy disable the rule in its project. Why do wee need to decide here for all developers whether this rule is usefull or not?

[sharwell]

@RobSiklos @fancyDevelopment The main problem with SA1650 is it has a very high rate of false positives. As mentioned in the title, it doesn't achieve the level of objectivity I would expect from a compiler-tied analyzer, making it excruciating to maintain with respect to backwards compatibility and other concerns. For the analyzers listed here, my recommendation is to check if they are implemented elsewhere, and if not consider creating a new analyzer project aimed at closing the gap in your expectations. For example, a "spelling analyzer" project could be created.

[RobSiklos]

@sharwell I disagree with the idea that SA1650 has a very high rate of false positives. We currently have over 1 million lines of code checked by SA1650, and not a single violation is reported. We've had to add a few words to the CustomDictionary.xml file, but that's just the nature of the beast.

[JimBobSquarePants]

So is there a way to enable these rules? I need to be able to enforce SA1630

[sharwell]

@JimBobSquarePants SA1630 isn't included with StyleCop Analyzers, but you could install another set of analyzers alongside StyleCop Analyzers which implements this diagnostic.

[JimBobSquarePants]

@sharwell Wouldn't that increase the workload of my IDE affecting the performance in a negative way? (Which appears to be a major reason why these rules are not enabled)

[pakdev]

@sharwell so if I implement SA1628, SA1630, SA1631, SA1632, and potentially SA1650, would you accept them?

[sharwell]

@pakdev The compiler analyzer infrastructure allows a project to include multiple analyzers. You could implement those analyzers as part of a separate project, and then add both StyleCop Analyzers and the separate project as analyzers.

I don't see any way to include these rules in StyleCop Analyzers without sacrificing the exceptionally low rate of false positives we've been working to maintain (near zero).

[pakdev]

@sharwell If they're disabled by default (which they are/could be), then what's the problem? I actually already implemented many of these in a private project, but then I discovered the skeletons already exist here. This could easily happen to others in the future.

Aside from the lack of visibility, though, the main problem with moving these into another project is the loss of free infrastructure/maintenance. For example, I'll have to (and did) re-create DocumentationCommentExtensions.cs, XmlCommentHelper.cs, and probably other nice-to-haves.

[pakdev]

Alternatively, would you consider moving the helpers into a StyleCop.Analyzers.Utilities NuGet?

[vweijsters]

@pakdev your suggestion of creating a separate NuGet package with (some of) the utility classes used by StyleCop.Analyzers may have benefits. Can you create a separate issue for that, so that we don't have to track it on a closed issue?

[pakdev]

Done