Follow-up to #2708179: Fix 'Drupal.Commenting.FunctionComment' coding standard - Issues related to @throws

This is a part of the attempt to fix #2572645: [Meta] Fix 'Drupal.Commenting.FunctionComment' coding standard

Since we don't have an automated coding standards test, we see regressions.

We also see 'new regressions' since Coder 8.2.8 was released.

This issue catches those regressions, so we can move forward on other coding standards issues.

CommentFileSizeAuthor
#6 2747073_6.patch1.87 KBMile23
#3 2747073_3.patch1.34 KBMile23
#2 2742599_2.patch1.9 KBMile23
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mile23 created an issue. See original summary.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Issue summary: View changes
Status: Active » Needs review
FileSize
2742599_2.patch1.9 KB

Comment in TwigTransTokenParser.php is taken from the exception message.

I removed @throws from JavascriptTestBase.php because it doesn't throw \PHPUnit_Framework_AssertionFailedError. None of the other functions it calls return that error, either. So if it does throw that, it's through a side-effect I don't understand.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
FileSize
2747073_3.patch1.34 KB

Hmm. Looks like that's the wrong patch. :-)

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented
Status: Needs review » Needs work

OH yeah, that annoyed me all the time already :)

+++ b/core/tests/Drupal/FunctionalJavascriptTests/JavascriptTestBase.php
@@ -74,8 +74,6 @@ protected function assertElementNotVisible($css_selector, $message = '') {
    *
-   * @throws \PHPUnit_Framework_AssertionFailedError
-   *
    * @see \Behat\Mink\Driver\DriverInterface::evaluateScript()

This is wrong. $this->assertTrue() throws it, see \PHPUnit_Framework_Assert::assertTrue()

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Status: Needs work » Closed (outdated)

The patch is not needed for coding standards because exceptions do not need to have a description per Drupal coding standards. This was fixed in the latest Coder release already, please upgrade.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Title: Fix Drupal.Commenting.FunctionComment.ThrowsComment (regression) » Fix Drupal CS regressions for Coder 8.2.8
Issue summary: View changes
Status: Closed (outdated) » Needs review
FileSize
2747073_6.patch1.87 KB

OK, so the patch above solves regressions from Coder 8.2.7.

Here's a patch that solves regressions from Coder 8.2.8.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented

Can we open new issues for this new 2 rules?

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Status: Needs review » Reviewed & tested by the community

I think it is fine to fix it in this issue. I tested the patch with the most recent dev version of Coder and it work as expected. No errors found when phpcs is executed.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Committed 4babdeb and pushed to 8.1.x. Thanks!
Committed c945189 and pushed to 8.2.x. Thanks!
Committed f24af6a and pushed to 8.3.x. Thanks!

Just the changes to phpcs.xml.dist was back ported to 8.1.x

  • alexpott committed f24af6a on 8.3.x 
    Issue #2747073 by Mile23: Fix Drupal CS regressions for Coder 8.2.8

  • alexpott committed c945189 on 8.2.x 
    Issue #2747073 by Mile23: Fix Drupal CS regressions for Coder 8.2.8

(...

  • alexpott committed 4babdeb on 8.1.x 
    Issue #2747073 by Mile23: Fix Drupal CS regressions for Coder 8.2.8

(...
pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented

Created followup issues to fix the new sniffs: #2783393: Fix 'Drupal.Commenting.FunctionComment.IncorrectParamVarName' coding standard and #2783395: Fix 'Drupal.Commenting.FunctionComment.InvalidReturn' coding standard.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.