Problem/Motivation

Running the PHPCS test displays the following warnings and errors:

FILE: date_range_formatter/date_range_formatter.install
--------------------------------------------------------------------------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 4 LINES
--------------------------------------------------------------------------------------------------------------------------------------
  1 | ERROR | [x] Missing file doc comment
  4 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Entity\Entity\EntityViewDisplay.
  4 | ERROR | [x] There must be one blank line after the last USE statement; 0 found;
 11 | ERROR | [ ] Doc comment short description must be on a single line, further text should be a separate paragraph
 13 | ERROR | [x] Expected 1 blank line before function; 0 found
--------------------------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------------------------------------------------------------


FILE: date_range_formatter/date_range_formatter.module
--------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------
 1 | ERROR | [x] Missing file doc comment
--------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------------------------------------------


FILE: date_range_formatter.info.yml
----------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------
 7 | WARNING | All dependencies must be prefixed with the project name, for example "drupal:"
----------------------------------------------------------------------------------------------------------------------


FILE: date_range_formatter/src/Plugin/Field/FieldFormatter/DateRangeFormatterRangeFormatter.php
-------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 10 ERRORS AND 3 WARNINGS AFFECTING 13 LINES
-------------------------------------------------------------------------------------------------------------------------------------------------------------
  76 | WARNING | Only string literals should be passed to t() where possible
  87 | WARNING | Line exceeds 80 characters; contains 86 characters
  91 | WARNING | Only string literals should be passed to t() where possible
 172 | ERROR   | There must be no space between ? and :
 177 | ERROR   | There must be no space between ? and :
 182 | ERROR   | There must be no space between ? and :
 187 | ERROR   | There must be no space between ? and :
 192 | ERROR   | There must be no space between ? and :
 197 | ERROR   | There must be no space between ? and :
 216 | ERROR   | There must be no space between ? and :
 217 | ERROR   | There must be no space between ? and :
 218 | ERROR   | There must be no space between ? and :
 219 | ERROR   | There must be no space between ? and :
-------------------------------------------------------------------------------------------------------------------------------------------------------------

Time: 114ms; Memory: 12MB

Steps to reproduce

Run following command:

phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml ./
Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

taraskorpach created an issue. See original summary.

taraskorpach’s picture

Assigned: taraskorpach » Unassigned
Status: Active » Needs review

I've created a MR.

FYI, I've added a phpcs.xml file to exclude warnings from the Drupal.Semantics.FunctionT.NotLiteralString rule.
If you have any suggestions for appropriately fixing these warnings, please do not hesitate to share them.

The issue needs review.

clarkssquared’s picture

Status: Needs review » Needs work

Hi

I applied the MR !6 and it fixes most of the PHPCS issues but there is still a PHPCS errors

➜  date_range_formatter git:(9.0.x) curl https://git.drupalcode.org/project/date_range_formatter/-/merge_requests/6.diff | patch -p1
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  7372    0  7372    0     0  14690      0 --:--:-- --:--:-- --:--:-- 14892
patching file date_range_formatter.install
patching file date_range_formatter.module
patching file phpcs.xml
patching file 'src/Plugin/Field/FieldFormatter/DateRangeFormatterRangeFormatter.php'
➜  date_range_formatter git:(9.0.x) ✗ ..
➜  contrib git:(master) ✗ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml date_range_formatter 

FILE: /Users/clarksubing-subing/Projects/d9/d9-local/web/modules/contrib/date_range_formatter/date_range_formatter.info.yml
---------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------------------
 7 | WARNING | All dependencies must be prefixed with the project name, for example "drupal:"
---------------------------------------------------------------------------------------------------------------------------


FILE: ...ubing-subing/Projects/d9/d9-local/web/modules/contrib/date_range_formatter/src/Plugin/Field/FieldFormatter/DateRangeFormatterRangeFormatter.php
-----------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
-----------------------------------------------------------------------------------------------------------------------------------------------------
 70 | WARNING | Only string literals should be passed to t() where possible
 84 | WARNING | Only string literals should be passed to t() where possible
-----------------------------------------------------------------------------------------------------------------------------------------------------

Time: 272ms; Memory: 10MB

➜  contrib git:(master) ✗ 
taraskorpach’s picture

Status: Needs work » Needs review

I've fixed the module dependency warning and updated the MR.

Regarding the translation warnings I mentioned above, I've added a phpcs.xml file. This means you need to run the phpcs command directly in the module folder to allow phpcs to take into account all the provided exclusions.

avpaderno’s picture

Title: Fix CS errors and warnings » Fix the errors/warnings reported by PHP_CodeSniffer
Category: Bug report » Task
Status: Needs review » Needs work
Issue tags: +Coding standards, +Needs issue summary update

The issue summary should always describe what the issue is trying to fix and, in the case, of coding standards issues, show which command has been used, which arguments have been used, and which report that command shown.

nitin_lama’s picture

Assigned: Unassigned » nitin_lama
nitin_lama’s picture

Issue summary: View changes
nitin_lama’s picture

Status: Needs work » Needs review
nitin_lama’s picture

Assigned: nitin_lama » Unassigned
taraskorpach’s picture

I don't believe that removing translation functions is the best approach here. Someone may already have translated some strings on their sites, and removing these functions could probably cause some issues. This issue requires further consideration.

BTW, I'm currently running the PHPCS command in the project folder, and it isn't showing any issues. Could someone else try running the command?

taraskorpach’s picture

Issue summary: View changes

I am modifying some arguments in the PHPCS command to utilize the appropriate phpcs.xml file during command execution.

Another method to bypass these errors is to use the ignore command, as in Drupal.Semantics.FunctionT.NotLiteralString.

However, creating a phpcs.xml file seems like a more suitable approach in this context.

avpaderno’s picture

$this->t() is not the correct way to translate configuration object values. For that, there is a specific API that needs to be used, which is also documented on drupal.org. (See Translating configuration.)

taraskorpach’s picture

Yeah, but these settings are part of a formatter field form.

As far as I know, we are unable to translate them because the issue #2546212: Entity view/form mode formatter/widget settings have no translation UI is still in progress.

avpaderno’s picture

If that is the only way to get translated strings in that specific context, I would leave the code as it is. I think it is more important to get translated strings than a PHP_CodeSniffer report without errors/warnings.
Once #2546212: Entity view/form mode formatter/widget settings have no translation UI get fixed, the code can be changed.

taraskorpach’s picture

Absolutely. Because of that, I've just added a sniffer rule to avoid unnecessary warnings. We just need a review rn.

nikolay shapovalov’s picture

Status: Needs review » Needs work

Thanks for MR, please check my feedback.

There are more changes in the MR, then we have in the phpcs report in the issue.
I don't want to fix issues that is not reported by phpcs in this issue.
Can you please update report in the IS?

taraskorpach’s picture

Assigned: Unassigned » taraskorpach
Status: Needs work » Active

I'll update the MR according to your feedback. Thanks

taraskorpach’s picture

Assigned: taraskorpach » Unassigned
Status: Active » Needs review

All threads seem to have been resolved. @nikolay-shapovalov, could you take a look at this?

nikolay shapovalov’s picture

Status: Needs review » Needs work

Taras, thanks a lot, changes looks good. Please check my question.

Can you please update IS, and provide fresh phpcs report (check comment #6)?

Suggested command:

phpcs --standard=Drupal,DrupalPractice --extensions=php,inc,module,install,info,test,profile,theme .

Here you can find fresh report https://git.drupalcode.org/issue/date_range_formatter-3416347/-/jobs/676068

taraskorpach’s picture

Assigned: Unassigned » taraskorpach
Status: Needs work » Active

Yep, I'll do this soon

taraskorpach’s picture

Assigned: taraskorpach » Unassigned
Issue summary: View changes
Status: Active » Needs review

I'm returning the entire PHPCS log to the IS as @nikolay-shapovalov has mentioned above.

Btw, the .module file has been removed.

taraskorpach’s picture

Issue summary: View changes

Removing the PATCHES.txt file from the log

nikolay shapovalov’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs issue summary update

Great work, thanks. RTBC.