Add missing-raises-doc rule to the linter and subsequent refactor.

[thanacles]

This is associated to issue #3388

[balopat]

LGTM

[thanacles]

@balopat Fixed the merge conflicts and a few places where problem pydocs where added since this PR was formed. I feel like we only have a small window before this PR will have further merge conflicts and lint violations.

[balopat]

Agreed, let's do it!

[mpharrigan]

I think this change adds more developer overhead than value it provides. I've rarely seen exceptions documented this way in Python. It's usually very clear what has happened when an exception is raised because of the message. If it's not, we should fix the message and not the docstring.

[thanacles]

@mpharrigan I agree it adds more developer overhead, and it would be hard for me to show how much value it provides. However, I don't think this is for helping people who are getting the error. This is for people to know what can happen when calling the function, so that they can prevent ever getting the error in the first place. That said, I recognize it is a judgement call whether it should be implemented. This commit was called for by bug #3388 , so that bug should be changed if you don't want more commits like this.

[mpharrigan]

Summarizing some offline discussion; my apologies in advance if I've mischaracterized anyone's opinion:

We should only be documenting the public API. If we document ValueErrors that arise from bad inputs, we're irionically making those bad inputs part of the public API. See https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings "raises" section.

You should not document exceptions that get raised if the API specified in the docstring is violated (because this would paradoxically make behavior under violation of the API part of the API).

---

@Strilanc wants to clarify that he's ok with exceptions being documented in the Raises section but he doesn't think it should be mandated.

I personally think we should document exceptions in only rare cases.

@MichaelBroughton thinks we should include this check because a similar check is present for internal-google code

@maffoo made analogy to Java's "checked" vs "unchecked" exceptions. You'd want to document the former but not the latter. Python doesn't have this distinction and the tool isn't sophisticated enough to distinguish

[thanacles]

I agree with @maffoo and @MichaelBroughton. I'm willing to remove the check and disable/enable comments if consensus is reached that we shouldn't have it. Also, there are a lot of similar checks that haven't been implemented yet, I think now would be the perfect time to discuss those rules. See: #3388 (comment)