Problem/Motivation

#2776975: March 3, 2017: Convert core to array syntax coding standards for Drupal 8.3.x RC phase landed for Drupal 8. We noticed that many of the API docs using long array syntax and we'd like to build a rule that would check the comments of the PHP files if possible.

Proposed resolution

Possibly parse comments ala annotations and token parse array() (not sure if this is possible but a suggestion)

Remaining tasks

Adding a rule has postponed this implementation of the rule #2874067: Fix Drupal.Commenting.DocCommentLongArraySyntax coding standard

CommentFileSizeAuthor
#4 enforce_short_array-2857906-4.patch2.82 KBhampercm

Comments

joelpittet created an issue. See original summary.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Issue summary: View changes
hampercm’s picture

hampercm commented
Assigned: Unassigned » hampercm
Issue tags: +MidCamp2017

Sprinting on this for MidCamp2017

hampercm’s picture

hampercm commented
Status: Active » Needs review
StatusFileSize
new enforce_short_array-2857906-4.patch2.82 KB

This patch adds a sniff for long array syntax use inside @code annotations.

hampercm’s picture

hampercm commented
Assigned: hampercm » Unassigned
blakehall’s picture

blakehall commented
Status: Needs review » Reviewed & tested by the community

This is looking good to me! (after wrestling with coder/phpcs for too long)

Thanks @hampercm!

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Version: 8.x-2.x-dev » 8.x-3.x-dev
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Hm, the regex comparison is a bit raw for PHP code, but maybe good enough for short @code blocks?

We also need a test case for this. And can you file this as pull request against https://github.com/pfrenssen/coder where the automated tests run? Thanks!

spokje’s picture

spokje
Primary language Dutch
commented
Issue summary: View changes
Status: Needs work » Needs review

https://github.com/pfrenssen/coder/pull/160

spokje’s picture

spokje
Primary language Dutch
commented
Issue tags: -Needs tests
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Needs work

Thanks, left some minor comments on the pull request.

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs work » Needs review

Thanks @klausi.

I've changed the PR.

(Note: I'm not the original patch creator, just the guy that whacked it into a PR).

  • klausi committed 0d39a71 on 8.3.x 
    fix(DocCommentLongArraySyntax): Imrove sniff error message (#2857906)
  • 97e91f1 committed on 8.3.x
    feat(DocCommentLongArraySyntax): Enforce short array syntax for Drupal...
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Fixed

Merged, thanks!

Status: Fixed » Closed (fixed)

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

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented

This new rule did introduce a bug #3306999: PHPCS hangs indefinetly with standard Drupal on file FieldPluginBase.php