The sniff is supposed to check the #options of a form element, but seems there are other cases where #options property is used.
In that case the following output error doesn't makes sense:

#options values usually have to run through t() for translation

For example:

  $build = array(
    '#type' => 'link',
    '#title' => t('More information about text formats'),
    '#href' => 'filter/tips',
    '#options' => array(
      'attributes' => array(
        'target' => '_blank',
      ),
    ),
  );
CommentFileSizeAuthor
#10 interdiff-8-10.txt1.51 KBtaran2l
#10 coder-false_positive-options-3034698-10.patch3.11 KBtaran2l
#8 interdiff-5-8.txt3.92 KBtaran2l
#8 coder-false_positive-options-3034698-8.patch3.06 KBtaran2l
#5 false_positive_options_element-3034698-05.patch2.04 KBzarabatana

Comments

zarabatana created an issue. See original summary.

zarabatana’s picture

zarabatana commented
Issue summary: View changes
zarabatana’s picture

zarabatana commented
Version: 8.x-2.x-dev » 8.x-2.12
zarabatana’s picture

zarabatana commented
Status: Active » Needs review
zarabatana’s picture

zarabatana commented
StatusFileSize
new false_positive_options_element-3034698-05.patch2.04 KB

Proposed solution:

Patch attached.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Version: 8.x-2.12 » 8.x-3.x-dev
Status: Needs review » Needs work
Issue tags: +Needs tests, +Needs issue summary update

Thanks for the report and the patch!

Can you also add a test case so that we see under which circumstances the false positive is triggered? Please also add that example to the issue summary.

taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented
Issue summary: View changes
Related issues: +#3062766: False positive with '#type' => 'link' and the use of t()
taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented
Status: Needs work » Needs review
Issue tags: -Needs tests, -Needs issue summary update
StatusFileSize
new coder-false_positive-options-3034698-8.patch3.06 KB
new interdiff-5-8.txt3.92 KB
klausi’s picture

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

Thanks! Can you make a pull request against https://github.com/pfrenssen/coder so that we see the automated tests run?

taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented
Status: Needs work » Needs review
StatusFileSize
new coder-false_positive-options-3034698-10.patch3.11 KB
new interdiff-8-10.txt1.51 KB

PR: https://github.com/pfrenssen/coder/pull/112

Also, attaching the updated patch

arkener’s picture

arkener commented
Assigned: zarabatana » Unassigned
Status: Needs review » Needs work

Thank you for working on this @Taran2L, I've left some comments on your PR.

taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented
Status: Needs work » Needs review

hi @Arkener - please review the updated version

arkener’s picture

arkener commented
Status: Needs review » Needs work

Left a request for some minor changes and then we're good to go!

taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented

Thanks for the thorough review. I'm on it.

taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented
Status: Needs work » Needs review

  • fb22679 committed on 8.x-3.x 
    fix(OptionsT): Fix false positive '#options' property have to run...
arkener’s picture

arkener commented
Status: Needs review » Fixed

Merged!, thanks allot for your work on this.

Status: Fixed » Closed (fixed)

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