Add option to ignore documentation errors on internal members and types

[dotMorten]

The documentation checkers are really useful for ensure consistency and completeness of XML doc for later generating API reference documentation. However if you put a <summary> tag on an internal member for your own sake, but don't follow all the rules for documenting the entire member as the rules dictate, you get a lot of warnings.

Most of the warnings aren't really that important for internal documentation, as most of it pertain to generating API doc. I'd like to be able to turn off the documentation checks for anything not public.
Or a general pattern to set the global suppression of a rule to only apply to public/internal/etc. (as long as we all promise not to misuse them 😄 )

[sharwell]

You can use stylecop.json to configure the documentation requirements which are relevant to your projects. If the available options do not provide the flexibility you need, feel free to open a new issue describing your proposed addition. 😄

[dotMorten]

Thanks @sharwell
I've got everything false, except 'documentExposedElements' but I still get lots of documentation errors on internal and private classes and members like SA1600, SA1601, SA1611, SA1612, SA1614, SA1616 and many more. These all from internal classes, or internal/private members.

{
	  "$schema": "https://raw.githubusercontent.com/DotNetAnalyzers/StyleCopAnalyzers/master/StyleCop.Analyzers/StyleCop.Analyzers/Settings/stylecop.schema.json",
	  "settings": {
	    "documentationRules": {
	      "companyName": "My Company",
              "copyrightText": "COPYRIGHT © 2016 etc etc",
              "documentInterfaces": false,
              "documentExposedElements": true,
              "documentInternalElements": false,
              "documentPrivateElements":  false,
              "documentPrivateFields": false,
              "xmlHeader": false
	    },
 ...

Also the doc example indicates there's an documentInternalMembers property, but that's not listed in the table, gives a schema error, and doesn't seem to do anything.

[dotMorten]

...actually it seems as if all this does is not require documentation. If you do provide some documentation but doesn't follow all the rules, you still get warnings. We should be allowed to use <summary /> tags on internal members, but not required to follow all the SA16** rules.

I still think this issue is valid and should be re-opened, as I'm doing exactly what you asked: Asked for another setting to ignore doc errors on internal members. This is different from requiring documentation in the first place.

[sharwell]

Also the doc example indicates there's an documentInternalMembers property

@dotMorten The table is correct; the example should be updated.

...actually it seems as if all this does is not require documentation. If you do provide some documentation but doesn't follow all the rules, you still get warnings.

In general this is the intended behavior.

I still think this issue is valid and should be re-opened, as I'm doing exactly what you asked

Sure

[dotMorten]

Thanks! Here's my proposal then:

{
	  "settings": {
	    "documentationRules": {
              "ignoreWarningsOnInternalElements": true,
              "ignoreWarningsOnPrivateElements": true
	    }
}

Alternatively specify which warnings you want to ignore on non-public members:

{
	  "settings": {
	    "documentationRules": {
              "ignoreWarningsOnInternalElements": [1601,1604,1621],
              "ignoreWarningsOnPrivateElements": [1601,1604,1621]
	    }
}

[sharwell]

@dotMorten Is there some reason why the internal documentation can't be updated? It sounds like the warnings being reported are legitimate and in my experience not particularly difficult to adhere to.

I would expect the following when documentInternalElements or documentPrivateElements is false:

• SA1600: Never reported
• SA1601: Never reported
• SA1611: Never reported
• SA1612: Reported only if a <param> element exists naming a non-existent parameter
• SA1614: Reported if a <param> element exists but is empty (fix by document or removing the element)
• SA1616: Reported if a <returns> element exists but is empty (fix by documenting or removing the element)

If you have counterexamples for these behaviors they can be filed as bugs without needing a new proposed option. Also let me know if there are other diagnostic IDs you are seeing.

[dotMorten]

Yes, we get over 500 warnings on internal members (and that's after lots of clean-up). Warnings like I don't use the proper Gets or sets a value indicating whether ..., or Creates an new instance of the <see cref="type"> class. etc. And that's only because the dev was kind enough to actually but some doc on a member (since no doc gives no warnings). The specific required wording is nice for a consistent publicly consumed API Ref and intellisense, but it's a lot of unnecessary extra typing for just internal doc meant for the internal dev team. The end result is we completely disabled these warnings, so other more important ones doesn't drown.

Often documenting just the summary and not worrying about the obvious parameter description is sufficient for internal members. It's sad that removing the entire doc tags is easier than allowing the devs to at least add a little that gives other devs on the team some intellisense for the method. We know what a constructor does, but might need more specifics up front instead for internal consumption of internal members which is quite often the case. It's better than the current state that encourages getting away with less doc over more doc.

Sure in a perfect world we have extensive documentation for all bits - public and internal, but it's a trade-off between no doc or too much doc, and striking the right balance.

[sharwell]

Often documenting just the summary and not worrying about the obvious parameter description is sufficient for internal members.

This is also supposed to work (i.e. <summary> by itself does not trigger warnings related to <param>).

[dotMorten]

...right but it triggers warnings if wording isn't correct for the summary, if <param> text is empty etc

[sharwell]

I need some time to think about this. Would be interested in feedback from others too.

[m-akinc]

We are also dissatisfied with the current behavior, which, based on what @sharwell posted earlier, should be considered buggy. Basically, documentInternalElements seems to only affect the behavior of SA1600/1. It avoids a violation when there is no perceived documentation, but if there is any triple-slash comment (or even C-style multi-line /**/ comment) at all on an element, the other rules (e.g. SA1604/5, SA1611, SA1615, etc) kick in to ensure the documentation is complete.

I'm glad to hear that this is not the desired behavior. I may be able to look into fixing this, if no one else has the time or interest.

[sharwell]

@m-akinc If you can, please file specific bugs for each case you find. This will save a whole bunch of time for whoever ends up working on the issues. 👍

[sharwell]

@dotMorten @m-akinc I created pull request #2447 to fix the behavior of SA1604, SA1611, SA1615, and SA1618.

[sharwell]

It looks like the remaining rules that need to be tested (and updated if they fail) are:

• SA1605
• SA1609
• SA1612 (maybe?)
• SA1619

[sharwell]

@dotMorten This should be substantially improved in today's release (1.1.0-beta004)

[daxian-dbw]

@sharwell I think SA1629 should also be disabled when documentInternalElements or documentPrivateElements is false, right?

SA1629DocumentationTextMustEndWithAPeriod: A section of the Xml header documentation for a C# element does not end with a period (also known as a full stop).