Exclude Squiz.Commenting.FunctionComment.ThrowsNotCapital sniff

[deeky666]

I suggest allowing descriptions for @throws to be started with a non-capital letter as it is more natural to compose a complete sentence out of the line like:

 @throws RuntimeException if the file could not be written.

As a further improvement one might even enforce sentences like this (starting with if).
What do you guys think?

[lcobucci]

This is a 👎 for me. For me there are two distinct elements in @throws lines: the type of the exception and the reason (and only the latter should be read as a sentence).

[deeky666]

@lcobucci can you give an example of a sentence you would put there? Because IMO @throws tag descriptions are used to tell the user under which circumstances that particular exception is thrown, thus if is used anyways most of the time. As a positive side effect you don't have to duplicate words like "throws" which saves you some words and leaves you more space in the line.

[kukulich]

@deeky666 You should throw more specific exception with more specific name so you don’t have to write the description at all

[deeky666]

@kukulich that is true but is not really the topic of this RFC. There will always be situation where you might not be able to throw a distinct exception for every type of error. Imagine file operations, where you cannot safely say what exactly went wrong when trying to write a file. You can check some preconditions beforehand but that does not prevent you from running into race conditions. This is just one example. In a perfect world I would 100% agree with you. Also we are not explicitly disallowing descriptions for @throws do this RFC is perfectly valid IMO.

[deeky666]

Descriptions for thrown exceptions especially are useful in interfaces where you might not know the technical details of when to throw which exception during implementation.

[lcobucci]

@deeky666 https://github.com/doctrine/doctrine2/blob/0b7d878cd340fed70e22404daa9656bb06cec74a/lib/Doctrine/ORM/Cache/Region.php#L35 surely we could rephrase it to have If or When.

[carusogabriel]

👎 For me as well, same reason from Luis

[deeky666]

@lcobucci I see but as a fun fact this contradicts to https://github.com/doctrine/doctrine2/blob/0b7d878cd340fed70e22404daa9656bb06cec74a/lib/Doctrine/ORM/Cache/Region.php#L24 where it is a similar situation :D

[lcobucci]

@deeky666 we're quite far from perfection 😂

[Majkl578]

I disagree, basically for the reasons highlighted by @lcobucci.

The format is as follows:

/**
 * @throws <type> <explanation>
 */

The fact it looks like a sentence in phpDoc could differ i.e. in API docs or IDE.

(I also agree with @kukulich.)

[Ocramius]

This patch is a clear 👍 from my side for a few reasons:

1. Last time I generated API docs, I was still in high school
2. I don't rely on API descriptions in the IDE either: I jump to the method definitions and read the docblock (I do the same in Java btw)
3. @throws AnApple when gravity is to be discovered is perfectly fine, and just reads awkwardly when having to restart a sentence

[carusogabriel]

Can we vote for this one?

/cc @Ocramius @Majkl578 @lcobucci @jwage @morozov @malarzm @doctrine/team-coding-standards

[Majkl578]

I already voted against.

[morozov]

The RFC looks like a matter of personal taste to me. Depending on whether we want to use the exception descriptions somewhere else than the source code, the way how we interpret the original annotation may be different.

The @throws if […] construct reads naturally but looks inconsistent with @params, @return and maybe some others. Besides, according to #52 (review), it doesn't have to read as a sentence.

dismissed due to 👍 votes (in order to allow merge)

[Ocramius]

Note: had to dismiss the 3 👎 reviews in order to allow merg: unsure why github started behaving like that

[Majkl578]

It was 3 👎 + one rather -1-ish review from @morozov.

I would prefer not merging such controversial RFCs.

[Ocramius]

That's what the voting is for: if we introduce an even tougher process for this we'll just get into gridlock. You can always open a new PR to revert this one.