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 ./
Comments
Comment #3
taraskorpachI've created a MR.
FYI, I've added a phpcs.xml file to exclude warnings from the
Drupal.Semantics.FunctionT.NotLiteralStringrule.If you have any suggestions for appropriately fixing these warnings, please do not hesitate to share them.
The issue needs review.
Comment #4
clarkssquared commentedHi
I applied the MR !6 and it fixes most of the PHPCS issues but there is still a PHPCS errors
Comment #5
taraskorpachI'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.
Comment #6
avpadernoThe 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.
Comment #7
nitin_lamaComment #8
nitin_lamaComment #9
nitin_lamaComment #10
nitin_lamaComment #11
taraskorpachI 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?
Comment #12
taraskorpachI 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.
Comment #13
avpaderno$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.)Comment #14
taraskorpachYeah, 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.
Comment #15
avpadernoIf 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.
Comment #16
taraskorpachAbsolutely. Because of that, I've just added a sniffer rule to avoid unnecessary warnings. We just need a review rn.
Comment #17
nikolay shapovalov commentedThanks 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?
Comment #18
taraskorpachI'll update the MR according to your feedback. Thanks
Comment #19
taraskorpachAll threads seem to have been resolved. @nikolay-shapovalov, could you take a look at this?
Comment #20
nikolay shapovalov commentedTaras, 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:
Here you can find fresh report https://git.drupalcode.org/issue/date_range_formatter-3416347/-/jobs/676068
Comment #21
taraskorpachYep, I'll do this soon
Comment #22
taraskorpachI'm returning the entire PHPCS log to the IS as @nikolay-shapovalov has mentioned above.
Btw, the .module file has been removed.
Comment #23
taraskorpachRemoving the PATCHES.txt file from the log
Comment #24
nikolay shapovalov commentedGreat work, thanks. RTBC.