Comment standards - missing/invalid @throws tags

Hm. We only want @throws to be on a method/function if the method/function *directly* throws that exception. I do not have time to look through this in detail right now, but ... do all of these functions directly throw exceptions, or are you attempting to document exceptions that they might not catch that come from other classes/methods deeper in the code?

The @throws tag should *only* be used on the function that throws the exception directly. If it is being used otherwise, it should be removed from the other functions.

@jhodgdon

Correct me if i am wrong , What i understand from throws the exception directly.
It should not be used in the try -catch.
It should be thrown directly, like

if (!isset($ids)) {
      throw new BadRequestHttpException(t('No contextual ids specified.'));
    }

In that case. I have cleaned it. Please review the attached patch.

Thanks! I didn't check over whether the @throws were correct (that is very time consuming), but I have a few comments on the docs and formatting and stuff like that:

1. +++ b/core/modules/contextual/src/ContextualController.php
   @@ -28,6 +28,8 @@ class ContextualController implements ContainerAwareInterface {
   +   * @throws \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
   +   *
       * @return \Symfony\Component\HttpFoundation\JsonResponse
   

   Normally we want @throws after @return.

   See
   https://www.drupal.org/node/1354#order

2. +++ b/core/modules/field/field.purge.inc
   @@ -156,7 +156,7 @@ function field_purge_field(FieldConfigInterface $field) {
   + * @throws \Drupal\Core\Field\FieldException
   

   It seems like this @throws might need some documentation? The name "FieldException" is kind of generic. Our standards suggest adding docs if the name of the exception class doesn't tell you immediately why this would be thrown, and I think this is one of those cases.

3. +++ b/core/modules/field/src/Entity/FieldConfig.php
   @@ -88,6 +88,8 @@ class FieldConfig extends FieldConfigBase implements FieldConfigInterface {
   +   * @throws \Drupal\Core\Field\FieldException
   

   Same here, and for the other FieldException throws.

4. +++ b/core/modules/hal/src/Normalizer/ContentEntityNormalizer.php
   @@ -219,6 +219,8 @@ protected function getEntityUri(EntityInterface $entity) {
   +   * @throws \Symfony\Component\Serializer\Exception\UnexpectedValueException
   +   *
       * @return array
   

   Move to after @return... not going to mark the rest of the patch but they need to be done too.

5. +++ b/core/modules/locale/src/Gettext.php
   @@ -39,6 +39,8 @@ class Gettext {
   +   * @throws \Exception
   

   OK here for sure we need to know why it would throw an exception!

6. +++ b/core/modules/locale/src/PoDatabaseWriter.php
   @@ -149,7 +149,7 @@ public function getHeader() {
   -   * @throws Exception
   +   * @throws \Exception
   

   needs docs

7. +++ b/core/modules/responsive_image/responsive_image.module
   @@ -358,6 +358,8 @@ function template_preprocess_responsive_image(&$variables) {
   + * @throws \LogicException
   

   needs docs

8. +++ b/core/modules/simpletest/src/WebTestBase.php
   @@ -1255,6 +1255,8 @@ protected function tearDown() {
   +   * @throws \UnexpectedValueException
   

   needs docs

9. +++ b/core/modules/simpletest/src/WebTestBase.php
   @@ -1684,6 +1686,8 @@ protected function drupalGetXHR($path, array $options = array(), array $headers
   +   * @throws \Exception
   

   needs docs

   OK I'm not going to mark the rest of these but ...

10. +++ b/core/modules/user/src/PrivateTempStore.php
    @@ -115,6 +115,8 @@ public function get($key) {
    +   * @throws TempStoreException
    

    This needs a full namespace

11. +++ b/core/modules/views/src/Controller/ViewAjaxController.php
    @@ -111,6 +111,8 @@ public static function create(ContainerInterface $container) {
    +   * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
    +   *   Thrown when the view name and view display id are not found.
    

    Um. AccessDenied when the view is not found??? I doubt it.

@jhodgdon

I am working it, will update you soon

@jhodgdon

I have fixed all those points you mentioned

1. change the order
2. added the docs
3. given the full path of

RE #14 - please provide an interdiff file. This is essential on a patch of this size when you make a new patch. Thanks!

See
https://www.drupal.org/documentation/git/interdiff

@jhodgdon

Please find the attached interdiff file between the patches 2595999-2.patch and 2595999-3.patch

I reviewed the interdiff file -- much improved, thanks!

A few smaller things to fix:

1. +++ b/core/modules/contextual/src/ContextualController.php
   @@ -28,10 +28,11 @@
   +   *   Thrown when the contextual ids are not found.
   

   ids => IDs

2. +++ b/core/modules/field/field.purge.inc
   @@ -157,6 +157,7 @@
   + *   Thrown when there is already field exists for the given field storage.
   

   This is a bit garbled? Not sure what it means.

3. +++ b/core/modules/field/src/Entity/FieldConfig.php
   @@ -89,6 +89,14 @@
       * @throws \Drupal\Core\Field\FieldException
   +   *   Thrown when the given field storage is not an instance of
   +   *   FieldStorageConfigInterface.
   +   * @throws \Drupal\Core\Field\FieldException
   +   *   Thrown when there is no field name associated.
   +   * @throws \Drupal\Core\Field\FieldException
   +   *   Thrown when there is no entity type associated.
   +   * @throws \Drupal\Core\Field\FieldException
   +   *   Thrown when there is no bundle associated.
   

   These are all throwing the same exception. So I think we only want one @throws, and then the explanation can be something like:

   Thrown when the ..., ..., ..., ..., or ....

4. +++ b/core/modules/field/src/Entity/FieldStorageConfig.php
   @@ -242,6 +242,13 @@
       * @throws \Drupal\Core\Field\FieldException
   +   *   Thrown when there is no field name associated.
   +   * @throws \Drupal\Core\Field\FieldException
   +   *   Thrown when there is special characters found in field name.
   +   * @throws \Drupal\Core\Field\FieldException
   +   *   Thrown when there is no field type specified.
   +   * @throws \Drupal\Core\Field\FieldException
   +   *   Thrown when there is no entity type associated.
   

   Same here.

5. +++ b/core/modules/hal/src/Normalizer/ContentEntityNormalizer.php
   @@ -219,11 +219,10 @@
   +   *  @throws \Symfony\Component\Serializer\Exception\UnexpectedValueException
   

   Do you think we want an explanation of what type of unexpected value this would be?

6. +++ b/core/modules/language/src/Entity/ContentLanguageSettings.php
   @@ -78,6 +78,9 @@
       * @throws \Drupal\language\ContentLanguageSettingsException
   +   *   Thrown when there is no entity type associated.
   +   * @throws \Drupal\language\ContentLanguageSettingsException
   +   *   Thrown when there is no bundle associated.
   

   Combine into one @throws

7. +++ b/core/modules/locale/src/PoDatabaseWriter.php
   @@ -150,6 +150,9 @@
       * @throws \Exception
   +   *   Thrown when the options are already exists.
   +   * @throws \Exception
   +   *   Thrown when the language code is not assigned.
   

   Combine into one @throws

8. +++ b/core/modules/simpletest/src/WebTestBase.php
   @@ -1257,6 +1257,7 @@
   +   *   Thrown when the specified curl options are already been set.
   

   are => have

9. +++ b/core/modules/simpletest/src/WebTestBase.php
   @@ -1688,6 +1689,7 @@
   +   *   Thrown when there is no ajax path specified.
   

   ajax => Ajax

10. +++ b/core/modules/simpletest/src/WebTestBase.php
    @@ -1828,11 +1830,12 @@
    +   *   Thrown when there is no ajax path specified.
    

    ajax => Ajax

11. +++ b/core/modules/system/src/Tests/Routing/MockAliasManager.php
    @@ -53,6 +53,9 @@
        * @throws \InvalidArgumentException
    +   *   Thrown when the path is no begin with a slash.
    +   *  @throws \InvalidArgumentException
    +   *   Thrown when the alias is no begin with a slash.
    

    Combine into one @throws

    Also grammar:

    "is no begin" => "does not begin"

12. +++ b/core/modules/user/src/PrivateTempStore.php
    @@ -116,7 +116,7 @@
    +   * @throws \Drupal\user\TempStoreException
    

    I think this could use an explanation doc

13. +++ b/core/modules/user/src/PrivateTempStore.php
    @@ -163,11 +163,11 @@
    +   * @throws \Drupal\user\TempStoreException
    

    Needs explanation

14. +++ b/core/modules/user/src/SharedTempStore.php
    @@ -191,7 +191,7 @@
    +   * @throws \Drupal\user\TempStoreException
    

    Needs explanation

15. +++ b/core/modules/user/src/SharedTempStore.php
    @@ -236,7 +236,7 @@
    +   * @throws \Drupal\user\TempStoreException
    

    Needs explanation

16. +++ b/core/modules/views/src/Controller/ViewAjaxController.php
    @@ -112,7 +112,8 @@
    -   *   Thrown when the view name and view display id are not found.
    +   *   Thrown when the view and access for the particular views id are not
    +   *   found.
    

    Actually you changed the meaning here. It previously said it was thrown when the *display* ID wasn't found, and now it says the view ID. Please verify.

17. +++ b/core/modules/views/src/Controller/ViewAjaxController.php
    @@ -112,7 +112,8 @@
    +   *   Thrown when the view and access for the particular views id are not
    +   *   found.
    

    id => ID

@jhodgdon

I have done the changes which you mentioned. Please find the attached patch and interdiff files.

About the point 16, you are right , I changed it again, the following code is the condition

$view = $this->executableFactory->get($entity);
      if ($view && $view->access($display_id)) {

So changed the explanation to

Thrown when the view and access for the particular display ID of the
+   *   view are not found.

About point 16, it looks like it is thrown if either the view isn't found, or the access check fails? I don't think I would say the access is "not found", when what is happening is that the access check says "you don't have permission", right?

Some other notes -- again I reviewed the interdiff -- which mostly looks very good!

1. +++ b/core/modules/field/src/Entity/FieldConfig.php
   @@ -90,13 +90,8 @@
   +   *   entity type or bundle associated.
   

   In Drupal docs, our style guide says we need to use serial commas.

   So we need a comma before the "or" in the last line:

   ... no field name, entity type, or bundle ...

2. +++ b/core/modules/locale/src/PoDatabaseWriter.php
   @@ -150,9 +150,8 @@
   +   *   Thrown when the options are already exists or the language code is not
   

   ... options are already exists... => ... options already exist ...

3. +++ b/core/modules/simpletest/src/WebTestBase.php
   @@ -1257,7 +1257,7 @@
   +   *   Thrown when the specified curl options have already been set.
   

   Um. I think curl should be cURL? I am not sure, please verify the correct capitalization.

4. +++ b/core/modules/user/src/PrivateTempStore.php
   @@ -117,6 +117,7 @@
   +   *   Thrown when the key of the data couldn't acquire lock.
   

   The key is not aquiring a lock..

   Maybe say this:

   Thrown when a lock cannot be acquired.

5. +++ b/core/modules/user/src/PrivateTempStore.php
   @@ -168,6 +169,7 @@
   +   *   Thrown when the key of the data couldn't acquire lock.
   

   same here

6. +++ b/core/modules/views/src/Controller/ViewAjaxController.php
   @@ -112,8 +112,8 @@
   +   *   Thrown when the view and access for the particular display ID of the
   +   *   view are not found.
   

   see earlier note about point 16.

Thanks!

@jhodgdon

I have done the changes you mentioned. Please find the attached files.

Thank You

Almost!

1. +++ b/core/modules/locale/src/PoDatabaseWriter.php
   @@ -150,7 +150,7 @@
   +   *   Thrown when the options already exists or the language code is not
   

   exists -> exist

2. +++ b/core/modules/simpletest/src/WebTestBase.php
   @@ -1687,9 +1687,6 @@
   -   *
   -   * @throws \Exception
   -   *   Thrown when there is no Ajax path specified.
   

   was removing this from the patch here intentional?

So... I have been reviewing the Interdiff files.

We need to find someone to check over the entire patch and make sure the @throws documentation is accurate. This will take some time, and right now I don't have the time to spend on it, sorry!

Maybe you can ask in IRC, especially at the next mentoring hours?

Adding explanation of the review that is needed to the issue summary.

Marking the review as a good Novice task

Thanks for the patch.
Meanwhile done with #24

@jhodgdon

I have done the changes , 2nd point was fully unintentional.

I have added the patch that and interdiff,

@snehi
thanks for the patch. Please check the way we use to name the files. Please follow that only here . So that it will clear the confusions.

Are you talking about interdiff files ?

Rerolled #47, changed core/modules/views/src/ViewExecutable.php to apply patch correctly.

@sdstyles

Can you please upload the interdiff file also

Thanks for the patches and interdiffs!

So I went through the latest patch. Again, I have not checked the *accuracy* of the @throws documentation, which is still a Remaining Task that someone needs to do.

But I am still finding some clarity problems:

1. +++ b/core/modules/editor/src/EditorController.php
   @@ -60,6 +60,9 @@ public function getUntransformedText(EntityInterface $entity, $field_name, $lang
   +   *   Thrown when there is no value found with requested object.
   +   *
       * @see editor_filter_xss()
       */
      public function filterXss(Request $request, FilterFormatInterface $filter_format) {
   

   What does "the requested object" mean here? Needs better docs, more specific.

2. +++ b/core/modules/field/src/Entity/FieldConfig.php
   @@ -88,6 +88,11 @@ class FieldConfig extends FieldConfigBase implements FieldConfigInterface {
   +   *   FieldStorageConfigInterface and also thrown if there is no field name,
   

   When using a class name in docs, include the full namespace starting with \

3. +++ b/core/modules/field/src/Entity/FieldStorageConfig.php
   @@ -241,6 +241,15 @@ class FieldStorageConfig extends ConfigEntityBase implements FieldStorageConfigI
   +   * @throws \Drupal\Core\Field\FieldException
   +   *   Thrown when there is no field name associated.
   +   * @throws \Drupal\Core\Field\FieldException
   +   *   Thrown when there is special characters found in field name.
   +   * @throws \Drupal\Core\Field\FieldException
   +   *   Thrown when there is no field type specified.
   +   * @throws \Drupal\Core\Field\FieldException
   +   *   Thrown when there is no entity type associated.
   

   These are all the same exception. Combine them into one @throws please.

4. +++ b/core/modules/hal/src/Normalizer/ContentEntityNormalizer.php
   @@ -222,6 +223,7 @@ protected function getEntityUri(EntityInterface $entity) {
   +   *  @throws \Symfony\Component\Serializer\Exception\UnexpectedValueException
   

   This needs docs about when/why it is thrown.

5. +++ b/core/modules/user/src/SharedTempStore.php
   @@ -190,6 +190,8 @@ public function setIfOwner($key, $value) {
   +   * @throws \Drupal\user\TempStoreException
   

   This needs docs about when/why it is thrown.

6. +++ b/core/modules/user/src/SharedTempStore.php
   @@ -233,6 +235,8 @@ public function getMetadata($key) {
   +   * @throws \Drupal\user\TempStoreException
   

   Needs docs.

@jhodgdon,

I have done the changes.

Thanks! Looking better.

There are still some clarity/typo things to clear up. And someone still needs to review this, as outlined in the issue summary, for accuracy (which I have not done).

1. +++ b/core/modules/editor/src/EditorController.php
   @@ -60,6 +60,9 @@ public function getUntransformedText(EntityInterface $entity, $field_name, $lang
   +   *   Thrown when there is no value found with requested object to filter.
   

   Not sure what this means. I think it means that the reqested filter object doesn't exist? Anyway, needs to be clarified.

2. +++ b/core/modules/field/src/Entity/FieldConfig.php
   @@ -88,6 +88,11 @@ class FieldConfig extends FieldConfigBase implements FieldConfigInterface {
   +   *   FieldStorageConfigInterface and also thrown if there is no field name,
   

   When using a class/interface in docs, provide the full namespace.

3. +++ b/core/modules/field/src/Entity/FieldStorageConfig.php
   @@ -241,6 +241,11 @@ class FieldStorageConfig extends ConfigEntityBase implements FieldStorageConfigI
   +   *   Thrown when there is no field name, there is special characters found in
   

   there is special characters => there are special characters

4. +++ b/core/modules/language/src/Entity/ContentLanguageSettings.php
   @@ -77,6 +77,9 @@ class ContentLanguageSettings extends ConfigEntityBase implements ContentLanguag
   +   *   Thrown when there is no entity type, or bundle associated.
   

   should not be a comma in here

5. +++ b/core/modules/system/src/Tests/Routing/MockAliasManager.php
   @@ -51,6 +51,9 @@ class MockAliasManager implements AliasManagerInterface {
   +   *   Thrown when the path, or alias does not begin with a slash.
   

   Should not be a comma here

6. +++ b/core/modules/views/src/ViewExecutable.php
   @@ -1781,6 +1781,11 @@ public function hasUrl($args = NULL, $display_id = NULL) {
   +   *   The display handlers URL object.
   

   shouldn't this be handler's rather than handlers? I think we're talking about the URL of the handler, but not sure. I would also probably word this as:

   An object representing the URL of the display.

   (assuming that this is correct)

Hello @jhodgdon,
I picked up @rakesh.gectcr last patch and done some of the points you mentioned in your last review comment.

I am not sure about points 1,2 so I picked up 3,4 & 5.

In my patch , there are some improvements that been done like :

For point 3

@@ -241,6 +241,11 @@ class FieldStorageConfig extends ConfigEntityBase implements FieldStorageConfigI
    * entity_create('field_storage_config', $values)), where $values is the same
    * parameter as in this constructor.
    *
+   * @throws \Drupal\Core\Field\FieldException
+   *   Thrown when there is no field name, there are special characters found in

For point 4

+++ b/core/modules/language/src/Entity/ContentLanguageSettings.php
@@ -77,6 +77,9 @@ class ContentLanguageSettings extends ConfigEntityBase implements ContentLanguag
    *   Other array elements will be used to set the corresponding properties on
    *   the class; see the class property documentation for details.
    *
+   * @throws \Drupal\language\ContentLanguageSettingsException
+   *   Thrown when there is no entity type or bundle associated.

For point 5

+++ b/core/modules/system/src/Tests/Routing/MockAliasManager.php
@@ -51,6 +51,9 @@ class MockAliasManager implements AliasManagerInterface {
    *   The alias of the system path.
    * @param type $path_language
    *   The language of this alias.
+   *
+   * @throws \InvalidArgumentException
+   *   Thrown when the path or alias does not begin with a slash.

For point 6

+++ b/core/modules/views/src/ViewExecutable.php
@@ -1781,6 +1781,11 @@ public function hasUrl($args = NULL, $display_id = NULL) {
    *   (optional) Specify the display ID to link to, fallback to the current ID.
    *
    * @return \Drupal\Core\Url
+   *   The display URL object.

Please review the above points.

@priya.chat

good work,

Please check how to name the interdiff files

@priya.chat, mentioned few changes.

+ * @throws \Drupal\Core\Entity\Exception\FieldStorageDefinitionUpdateForbiddenException

+   * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
+   * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException
+   *

+   *
+   * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
    */

+   *
+   * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException
    */

+   *
+   * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException
    */

+   *
+   * @throws \Symfony\Component\HttpKernel\Exception\UnsupportedMediaTypeHttpException
    */

+   *
+   * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
    */

Should have some documentation about the exception.

Hello heykarthikwithu,
I have made the changes in the comments kindly look into it.

@kaushalkishorejaiswal

Thanks for the patch, Can you please add the interdiff file of patches Comment #62,#64 and #68.

Seee :- https://www.drupal.org/documentation/git/interdiff

Hello Rakesh,
Kindly find the interdiff file. I appreciate your knowledge sharing.

  * @throws \Drupal\Core\Entity\Exception\FieldStorageDefinitionUpdateForbiddenException
+ * Thrown when a storage definition update is forbidden.
  *

Should have spaces.

  * @throws \Drupal\Core\Entity\Exception\FieldStorageDefinitionUpdateForbiddenException
+ *    Thrown when a storage definition update is forbidden.
  *

Similarly in few more places.

    * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
+   * AccessDeniedHttpException represents an "Access Denined for unauthorised user"
    * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException
+   * NotFoundHttpException represents a "Page Not found"
    *

+   * AccessDeniedHttpException represents an "Access Denined for unauthorised 
+   * user"

    * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException
+   * NotFoundHttpException represents a "Page Not found"

    * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException
+   * NotFoundHttpException represents a "Page Not found"

#71:patch applied, please review

Thanks for the patches! Sorry for the delay in reviewing -- I've been on vacation and no one else, unfortunately, ever reviews docs patches. :(

Since it's been so long, I just started by looking at the patch in #72, and did not review the interdiffs...

So I started to look through these, and found that the @throws documentation left me with a lot of questions. I got through about half of the patch and made these notes, and didn't review the whole patch, but you'll get the idea... If we're going to document these things, we need to have the documentation make sense.

1. +++ b/core/modules/editor/src/EditorController.php
   @@ -60,6 +60,9 @@ public function getUntransformedText(EntityInterface $entity, $field_name, $lang
   +   *   Thrown when there is no value found with requested object to filter.
   

   I don't understand what this line means at all.

2. +++ b/core/modules/field/field.api.php
   @@ -71,6 +71,9 @@ function hook_field_info_alter(&$info) {
   + *   Thrown when a storage definition update is forbidden.
   

   Hm. This is in the docs header for a hook... so... Are you saying that the hook implementation should throw this exception if it wants to forbid something? If so, it should be phrased that way. If not, I don't get this statement at all.

3. +++ b/core/modules/field/field.purge.inc
   @@ -156,7 +156,8 @@ function field_purge_field(FieldConfigInterface $field) {
   + *   Thrown when the field storage has fields.
   

   Um, what does this mean? This is the docs for field_purge_field_storage(). Are you saying that the storage cannot be purged if it has fields? Then what is the use of purging it?

4. +++ b/core/modules/field/src/Entity/FieldConfig.php
   @@ -88,6 +88,11 @@ class FieldConfig extends FieldConfigBase implements FieldConfigInterface {
   +   *   Thrown when the given field storage is not an instance of
   +   *   FieldStorageConfigInterface and also thrown if there is no field name,
   +   *   entity type, or bundle associated.
   

   What given field storage? The only args here are $values and $entity_type.

5. +++ b/core/modules/field/src/Entity/FieldStorageConfig.php
   @@ -241,6 +241,11 @@ class FieldStorageConfig extends ConfigEntityBase implements FieldStorageConfigI
   +   *   Thrown when there is no field name, there are special characters found in
   +   *   field name, there is no field type specified, or there is no entity type
   +   *   associated.
   

   Where are these things found? The only args here are $values and an entity type.

6. +++ b/core/modules/hal/src/Normalizer/ContentEntityNormalizer.php
   @@ -222,6 +223,8 @@ protected function getEntityUri(EntityInterface $entity) {
   +   *   Thrown when there is no link type attribute found.
   

   Not sure what this means, found where?

7. +++ b/core/modules/history/src/Controller/HistoryController.php
   @@ -25,6 +25,11 @@ class HistoryController extends ControllerBase {
   +   * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
   +   *   AccessDeniedHttpException represents an "Access Denined for unauthorised user"
   +   * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException
   +   *   NotFoundHttpException represents a "Page Not found"
   

   These two do not conform to our standards of how to document this.

8. +++ b/core/modules/history/src/Controller/HistoryController.php
   @@ -45,8 +50,13 @@ public function getNodeReadTimestamps(Request $request) {
   +   * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException
   +   *   AccessDeniedHttpException represents an "Access Denined for unauthorised ¶
   +   * user"
   

   Doesn't match the pattern of the others, and there is an extra space at the end of the first docs line, and the second one isn't indented.

9. +++ b/core/modules/language/src/Entity/ContentLanguageSettings.php
   @@ -77,6 +77,9 @@ class ContentLanguageSettings extends ConfigEntityBase implements ContentLanguag
   +   *   Thrown when there is no entity type or bundle associated.
   

   Associated with what?

Changes applied. Please review.

Please review.

Rerolling/fixing the patch in #75.

Thanks for the rerolls... The patch still needs considerable work though. The main problem is that the English grammar and spelling in the exception explanations needs some attention:

1. +++ b/core/modules/editor/src/EditorController.php
   @@ -63,6 +63,9 @@ public function getUntransformedText(EntityInterface $entity, $field_name, $lang
   +   *   Thrown when no value found with requested object.
   

   needs "is" in here.

2. +++ b/core/modules/field/field.purge.inc
   @@ -156,7 +156,9 @@ function field_purge_field(FieldConfigInterface $field) {
   + *   Thrown when attempt to create an instance of a field annotation that
   + *   doesn't exist.
   

   The grammar here is garbled.

3. +++ b/core/modules/field/src/Entity/FieldConfig.php
   @@ -88,6 +88,10 @@ class FieldConfig extends FieldConfigBase implements FieldConfigInterface {
   +   *   FieldStorageConfigInterface and also thrown if there is no field name,
   

   When mentioning a class/interface in API documentation, always include the namespace.

4. +++ b/core/modules/field/src/Entity/FieldStorageConfig.php
   @@ -241,6 +241,10 @@ class FieldStorageConfig extends ConfigEntityBase implements FieldStorageConfigI
   +   *   Thrown when special charecters are used other than lowercase alphanumeri
   

   Several spelling errors here.

   Also... this reads funny. I don't think we need "special" in the line at all, since it goes on to say what characters are allowed?

5. +++ b/core/modules/hal/src/Normalizer/ContentEntityNormalizer.php
   @@ -128,6 +128,7 @@ public function normalize($entity, $format = NULL, array $context = array()) {
   +   *   Thrown when type is not specified in link.
   

   grammar attention needed (needs a/an/the)

6. +++ b/core/modules/history/src/Controller/HistoryController.php
   @@ -25,6 +25,11 @@ class HistoryController extends ControllerBase {
   +   *   Thrown for unauthorised user.
   

   needs a verb

7. +++ b/core/modules/history/src/Controller/HistoryController.php
   @@ -25,6 +25,11 @@ class HistoryController extends ControllerBase {
   +   *   Thrown if requested node IDs not found.
   

   grammar a/an/the needed and no verb

8. +++ b/core/modules/history/src/Controller/HistoryController.php
   @@ -47,6 +52,9 @@ public function getNodeReadTimestamps(Request $request) {
   +   *   Thrown when an unauthorised user try to access the page
   

   bad grammar

9. +++ b/core/modules/quickedit/src/QuickEditController.php
   @@ -95,6 +95,9 @@ public static function create(ContainerInterface $container) {
   +   *   Thrown when the page requested not found.
   

   no verb

10. +++ b/core/modules/quickedit/src/QuickEditController.php
    @@ -146,6 +149,9 @@ public function metadata(Request $request) {
    +   *   Thrown when the requested page not found.
    

    no verb

11. +++ b/core/modules/rest/src/RequestHandler.php
    @@ -32,6 +32,9 @@ class RequestHandler implements ContainerAwareInterface {
    +   * UnsupportedMediaTypeHttpException represents an "Unsupported Media Type".
    

    needs 2 spaces of indentation

    But ... duh. This line does not tell me anything that the name of the exception doesn't tell me.

    It should say:

    Thrown when ....

    or be omitted.

12. +++ b/core/modules/user/src/Controller/UserController.php
    @@ -189,6 +189,10 @@ public function logout() {
    +   * AccessDeniedHttpException represents an "Access Denined for unauthorised
    +   * user"
    

    See comment above on the UnsupportedMediaType exception.

13. +++ b/core/modules/views/src/ViewExecutable.php
    @@ -1791,6 +1791,11 @@ public function hasUrl($args = NULL, $display_id = NULL) {
        * @return \Drupal\Core\Url
    +   *   This display URL object.
    

    Huh? What does this mean?

Hi!
I tried to fix the mentioned problems.
Clarification about points 11 and 12 - for the first one, I added a better comment, for the second one I removed the comment (unable to add a logical explanation for that one).
About the point 13 - I think this @return is out of scope, so removed that one.

I did a quick review on other changes, found one possible problem:

1. --- a/core/modules/editor/src/EditorController.php
   +++ b/core/modules/editor/src/EditorController.php
   @@ -63,6 +63,9 @@ public function getUntransformedText(EntityInterface $entity, $field_name, $lang
       * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException
       *   Thrown if no value to filter is specified.
       *
   +   * @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException
   +   *   Thrown when no value is found with requested object.
   +   *
       * @see editor_filter_xss()
       */

   This seems to be weird, am I right?

I'm sorry, but this issue is now violating our issue scope guidelines for this type of coding standards cleanup. See
https://www.drupal.org/core/scope

So. The issue needs rescoping before we can proceed. See that document for more information. Until this is done, and the issue is properly scoped, I'm not going to spend time reviewing it, because in the end the core committers will not commit patches any more on issues that are improperly scoped.

Thanks!

Hi!
Who can do the rescoping?
If I understand correctly, this issue address only missing @throws tags. This should be a good scope according to this table (last example) https://www.drupal.org/core/scope#examples

Yes, just doing @throws is good.

The scoping problem is that this issue/patch just addresses some subset of Drupal Core for @throws. It needs to be part of a "meta" issue with a plan for addressing all of Drupal Core, which needs to be a child issue of the main "Fix coding standards" issue. And it probably will need some kind of a coder sniffer thing. See the scoping document for more details.

What we're trying to avoid is having one-off patches like this issue, which address a coding standards issue without a coder sniffer rule and without covering all of Core or having a plan to do so.

Thanks!

Yes, coding standards are done by sniff. This no longer needs an issue re-scope, so removing tag. Adding tags for coding standards and Bug Smash (one of the reasons I found this issue). Changing to a Task in accordance with coding standards issues.

Some of the fixes in the patch are still relevant.

Postponing this until there is a sniff for the @throws.

Should have changed the version.