Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OPENEUROPA-1535: Fix various coding style inconsistencies. #92

Merged
merged 2 commits into from
Jan 30, 2019

Conversation

drupol
Copy link
Contributor

@drupol drupol commented Jan 15, 2019

No description provided.

@pfrenssen
Copy link
Member

I agree with these proposed changes, but ideally these should be detected automatically. In order to facilitate this I proposed a PR for the code review tool which will detect the changes which are proposed here: indentation of arrays and object chaining, and spaces around the concatenation operator: openeuropa/code-review#51

When I try out this PR in the task runner it appears that there are a number of coding standards violations that remain:

$ composer require --dev openeuropa/code-review:dev-OPENEUROPA-1535
$ ./vendor/bin/grumphp run


Deprecated: strpos(): Non-string needles will be interpreted as strings in the future. Use an explicit chr() call to preserve the current behavior in /home/pieter/v/task-runner/vendor/phpro/grumphp/src/Locator/ConfigurationFile.php on line 75
FILE: /home/pieter/v/task-runner/src/Commands/AbstractCommands.php
----------------------------------------------------------------------
FOUND 6 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
  37 | ERROR | [x] Expected at least 1 space before "."; 0 found
  37 | ERROR | [x] Expected at least 1 space after "."; 0 found
 118 | ERROR | [x] Expected at least 1 space before "."; 0 found
 118 | ERROR | [x] Expected at least 1 space after "."; 0 found
 118 | ERROR | [x] Expected at least 1 space before "."; 0 found
 118 | ERROR | [x] Expected at least 1 space after "."; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 6 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...ome/pieter/v/task-runner/src/Commands/AbstractDrupalCommands.php
----------------------------------------------------------------------
FOUND 12 ERRORS AFFECTING 7 LINES
----------------------------------------------------------------------
  33 | ERROR | [x] Line indented incorrectly; expected 8 spaces,
     |       |     found 12
  34 | ERROR | [x] Line indented incorrectly; expected 8 spaces,
     |       |     found 12
  42 | ERROR | [x] Expected at least 1 space before "."; 0 found
  42 | ERROR | [x] Expected at least 1 space after "."; 0 found
  97 | ERROR | [x] Expected at least 1 space before "."; 0 found
  97 | ERROR | [x] Expected at least 1 space after "."; 0 found
 173 | ERROR | [x] Expected at least 1 space before "."; 0 found
 173 | ERROR | [x] Expected at least 1 space after "."; 0 found
 295 | ERROR | [x] Expected at least 1 space before "."; 0 found
 295 | ERROR | [x] Expected at least 1 space after "."; 0 found
 296 | ERROR | [x] Expected at least 1 space before "."; 0 found
 296 | ERROR | [x] Expected at least 1 space after "."; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 12 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: /home/pieter/v/task-runner/src/Commands/ChangelogCommands.php
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
----------------------------------------------------------------------
 23 | ERROR | [x] Expected at least 1 space before "."; 0 found
 23 | ERROR | [x] Expected at least 1 space after "."; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: /home/pieter/v/task-runner/src/Services/Composer.php
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
----------------------------------------------------------------------
 80 | ERROR | [x] Expected at least 1 space before "."; 0 found
 80 | ERROR | [x] Expected at least 1 space after "."; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: /home/pieter/v/task-runner/src/TaskRunner.php
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
----------------------------------------------------------------------
 144 | ERROR | [x] Expected at least 1 space before "."; 0 found
 144 | ERROR | [x] Expected at least 1 space after "."; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: .../v/task-runner/src/Tasks/CollectionFactory/CollectionFactory.php
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
----------------------------------------------------------------------
 153 | ERROR | [x] Expected at least 1 space before "."; 0 found
 153 | ERROR | [x] Expected at least 1 space after "."; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: /home/pieter/v/task-runner/tests/AbstractTest.php
----------------------------------------------------------------------
FOUND 12 ERRORS AFFECTING 5 LINES
----------------------------------------------------------------------
 33 | ERROR | [x] Expected at least 1 space before "."; 0 found
 33 | ERROR | [x] Expected at least 1 space after "."; 0 found
 61 | ERROR | [x] Expected at least 1 space before "."; 0 found
 61 | ERROR | [x] Expected at least 1 space after "."; 0 found
 71 | ERROR | [x] Expected at least 1 space before "."; 0 found
 71 | ERROR | [x] Expected at least 1 space after "."; 0 found
 71 | ERROR | [x] Expected at least 1 space before "."; 0 found
 71 | ERROR | [x] Expected at least 1 space after "."; 0 found
 81 | ERROR | [x] Expected at least 1 space before "."; 0 found
 81 | ERROR | [x] Expected at least 1 space after "."; 0 found
 95 | ERROR | [x] Expected at least 1 space before "/"; 0 found
 95 | ERROR | [x] Expected at least 1 space after "/"; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 12 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...home/pieter/v/task-runner/tests/Commands/ReleaseCommandsTest.php
----------------------------------------------------------------------
FOUND 10 ERRORS AFFECTING 8 LINES
----------------------------------------------------------------------
  39 | ERROR | [x] Expected at least 1 space before "."; 0 found
  39 | ERROR | [x] Expected at least 1 space after "."; 0 found
  70 | ERROR | [x] Expected at least 1 space before "."; 0 found
  70 | ERROR | [x] Expected at least 1 space after "."; 0 found
 124 | ERROR | [x] Object operator not indented correctly; expected
     |       |     12 spaces but found 10
 125 | ERROR | [x] Object operator not indented correctly; expected
     |       |     12 spaces but found 10
 131 | ERROR | [x] Object operator not indented correctly; expected
     |       |     12 spaces but found 10
 134 | ERROR | [x] Object operator not indented correctly; expected
     |       |     12 spaces but found 10
 135 | ERROR | [x] Object operator not indented correctly; expected
     |       |     12 spaces but found 10
 140 | ERROR | [x] Object operator not indented correctly; expected
     |       |     12 spaces but found 10
----------------------------------------------------------------------
PHPCBF CAN FIX THE 10 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: /home/pieter/v/task-runner/tests/CommandsTest.php
----------------------------------------------------------------------
FOUND 18 ERRORS AFFECTING 14 LINES
----------------------------------------------------------------------
  59 | ERROR | [x] Expected at least 1 space before "."; 0 found
  59 | ERROR | [x] Expected at least 1 space after "."; 0 found
  92 | ERROR | [x] Expected at least 1 space before "."; 0 found
  92 | ERROR | [x] Expected at least 1 space after "."; 0 found
 129 | ERROR | [x] Array key not indented correctly; expected 12
     |       |     spaces but found 10
 130 | ERROR | [x] Array key not indented correctly; expected 12
     |       |     spaces but found 10
 131 | ERROR | [x] Array key not indented correctly; expected 12
     |       |     spaces but found 10
 132 | ERROR | [x] Array key not indented correctly; expected 12
     |       |     spaces but found 10
 153 | ERROR | [x] Expected at least 1 space before "."; 0 found
 153 | ERROR | [x] Expected at least 1 space after "."; 0 found
 198 | ERROR | [x] Line indented incorrectly; expected 8 spaces,
     |       |     found 12
 199 | ERROR | [x] Line indented incorrectly; expected 8 spaces,
     |       |     found 12
 249 | ERROR | [x] Line indented incorrectly; expected 8 spaces,
     |       |     found 12
 250 | ERROR | [x] Line indented incorrectly; expected 8 spaces,
     |       |     found 12
 304 | ERROR | [x] Line indented incorrectly; expected 8 spaces,
     |       |     found 12
 305 | ERROR | [x] Line indented incorrectly; expected 8 spaces,
     |       |     found 12
 331 | ERROR | [x] Expected at least 1 space before "."; 0 found
 331 | ERROR | [x] Expected at least 1 space after "."; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 18 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: /home/pieter/v/task-runner/tests/Tasks/CollectionFactoryTest.php
----------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 5 LINES
----------------------------------------------------------------------
 56 | ERROR | [x] Array key not indented correctly; expected 12
    |       |     spaces but found 10
 57 | ERROR | [x] Array key not indented correctly; expected 12
    |       |     spaces but found 10
 58 | ERROR | [x] Array key not indented correctly; expected 12
    |       |     spaces but found 10
 59 | ERROR | [x] Array key not indented correctly; expected 12
    |       |     spaces but found 10
 60 | ERROR | [x] Array key not indented correctly; expected 12
    |       |     spaces but found 10
----------------------------------------------------------------------
PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Time: 213ms; Memory: 8MB

You can fix some errors by running following command:
'/home/pieter/v/task-runner/vendor/bin/phpcbf' '--standard=./vendor/openeuropa/code-review/resources/library-phpcs-ruleset.xml' '--report=full' '--warning-severity=0' '--ignore=vendor/,loadTasks.php' '/home/pieter/v/task-runner/src/Commands/AbstractCommands.php' '/home/pieter/v/task-runner/src/Commands/AbstractDrupalCommands.php' '/home/pieter/v/task-runner/src/Commands/ChangelogCommands.php' '/home/pieter/v/task-runner/src/Services/Composer.php' '/home/pieter/v/task-runner/src/TaskRunner.php' '/home/pieter/v/task-runner/src/Tasks/CollectionFactory/CollectionFactory.php' '/home/pieter/v/task-runner/tests/AbstractTest.php' '/home/pieter/v/task-runner/tests/Commands/ReleaseCommandsTest.php' '/home/pieter/v/task-runner/tests/CommandsTest.php' '/home/pieter/v/task-runner/tests/Tasks/CollectionFactoryTest.php'
To skip commit checks, add -n or --no-verify flag to commit command

Please make sure to not commit the dependency on the dev branch of the code review tool, this is not critical for this ticket. If this gets merged then it will be part of the next release and we will benefit from the changes automatically in the future.

Copy link
Member

@pfrenssen pfrenssen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When trying out the updated code review tool a number of coding standards violations are detected. These can also be fixed in scope of this PR.

@voidtek
Copy link
Member

voidtek commented Jan 24, 2019

The PR change the permission file of tests/sandbox/.gitkeep 100644 → 100755.

Copy link
Member

@voidtek voidtek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. We should update the composer.json with the new branch of code-review.

  2. I try in my local machine and these errors appears: (same of @pfrenssen )

devs-costvit:~/environment/task-runner (OPENEUROPA-1535) $ dckweb ./vendor/bin/grumphp run
GrumPHP is sniffing your code!
Running task 1/2: Phpcs... ✘
Running task 2/2: PhpMd... ✔
FILE: /var/www/html/src/Commands/AbstractCommands.php
----------------------------------------------------------------------
FOUND 6 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
  37 | ERROR | [x] Expected at least 1 space before "."; 0 found
  37 | ERROR | [x] Expected at least 1 space after "."; 0 found
 118 | ERROR | [x] Expected at least 1 space before "."; 0 found
 118 | ERROR | [x] Expected at least 1 space after "."; 0 found
 118 | ERROR | [x] Expected at least 1 space before "."; 0 found
 118 | ERROR | [x] Expected at least 1 space after "."; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 6 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: /var/www/html/src/Commands/AbstractDrupalCommands.php
----------------------------------------------------------------------
FOUND 12 ERRORS AFFECTING 7 LINES
----------------------------------------------------------------------
  33 | ERROR | [x] Line indented incorrectly; expected 8 spaces,
     |       |     found 12
  34 | ERROR | [x] Line indented incorrectly; expected 8 spaces,
     |       |     found 12
  42 | ERROR | [x] Expected at least 1 space before "."; 0 found
  42 | ERROR | [x] Expected at least 1 space after "."; 0 found
  97 | ERROR | [x] Expected at least 1 space before "."; 0 found
  97 | ERROR | [x] Expected at least 1 space after "."; 0 found
 173 | ERROR | [x] Expected at least 1 space before "."; 0 found
 173 | ERROR | [x] Expected at least 1 space after "."; 0 found
 295 | ERROR | [x] Expected at least 1 space before "."; 0 found
 295 | ERROR | [x] Expected at least 1 space after "."; 0 found
 296 | ERROR | [x] Expected at least 1 space before "."; 0 found
 296 | ERROR | [x] Expected at least 1 space after "."; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 12 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: /var/www/html/src/Commands/ChangelogCommands.php
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
----------------------------------------------------------------------
 23 | ERROR | [x] Expected at least 1 space before "."; 0 found
 23 | ERROR | [x] Expected at least 1 space after "."; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: /var/www/html/src/Services/Composer.php
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
----------------------------------------------------------------------
 80 | ERROR | [x] Expected at least 1 space before "."; 0 found
 80 | ERROR | [x] Expected at least 1 space after "."; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: /var/www/html/src/TaskRunner.php
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
----------------------------------------------------------------------
 144 | ERROR | [x] Expected at least 1 space before "."; 0 found
 144 | ERROR | [x] Expected at least 1 space after "."; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: /var/www/html/src/Tasks/CollectionFactory/CollectionFactory.php
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
----------------------------------------------------------------------
 153 | ERROR | [x] Expected at least 1 space before "."; 0 found
 153 | ERROR | [x] Expected at least 1 space after "."; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: /var/www/html/tests/AbstractTest.php
----------------------------------------------------------------------
FOUND 12 ERRORS AFFECTING 5 LINES
----------------------------------------------------------------------
 33 | ERROR | [x] Expected at least 1 space before "."; 0 found
 33 | ERROR | [x] Expected at least 1 space after "."; 0 found
 61 | ERROR | [x] Expected at least 1 space before "."; 0 found
 61 | ERROR | [x] Expected at least 1 space after "."; 0 found
 71 | ERROR | [x] Expected at least 1 space before "."; 0 found
 71 | ERROR | [x] Expected at least 1 space after "."; 0 found
 71 | ERROR | [x] Expected at least 1 space before "."; 0 found
 71 | ERROR | [x] Expected at least 1 space after "."; 0 found
 81 | ERROR | [x] Expected at least 1 space before "."; 0 found
 81 | ERROR | [x] Expected at least 1 space after "."; 0 found
 95 | ERROR | [x] Expected at least 1 space before "/"; 0 found
 95 | ERROR | [x] Expected at least 1 space after "/"; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 12 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: /var/www/html/tests/Commands/ReleaseCommandsTest.php
----------------------------------------------------------------------
FOUND 10 ERRORS AFFECTING 8 LINES
----------------------------------------------------------------------
  39 | ERROR | [x] Expected at least 1 space before "."; 0 found
  39 | ERROR | [x] Expected at least 1 space after "."; 0 found
  70 | ERROR | [x] Expected at least 1 space before "."; 0 found
  70 | ERROR | [x] Expected at least 1 space after "."; 0 found
 124 | ERROR | [x] Object operator not indented correctly; expected
     |       |     12 spaces but found 10
 125 | ERROR | [x] Object operator not indented correctly; expected
     |       |     12 spaces but found 10
 131 | ERROR | [x] Object operator not indented correctly; expected
     |       |     12 spaces but found 10
 134 | ERROR | [x] Object operator not indented correctly; expected
     |       |     12 spaces but found 10
 135 | ERROR | [x] Object operator not indented correctly; expected
     |       |     12 spaces but found 10
 140 | ERROR | [x] Object operator not indented correctly; expected
     |       |     12 spaces but found 10
----------------------------------------------------------------------
PHPCBF CAN FIX THE 10 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: /var/www/html/tests/CommandsTest.php
----------------------------------------------------------------------
FOUND 18 ERRORS AFFECTING 14 LINES
----------------------------------------------------------------------
  59 | ERROR | [x] Expected at least 1 space before "."; 0 found
  59 | ERROR | [x] Expected at least 1 space after "."; 0 found
  92 | ERROR | [x] Expected at least 1 space before "."; 0 found
  92 | ERROR | [x] Expected at least 1 space after "."; 0 found
 129 | ERROR | [x] Array key not indented correctly; expected 12
     |       |     spaces but found 10
 130 | ERROR | [x] Array key not indented correctly; expected 12
     |       |     spaces but found 10
 131 | ERROR | [x] Array key not indented correctly; expected 12
     |       |     spaces but found 10
 132 | ERROR | [x] Array key not indented correctly; expected 12
     |       |     spaces but found 10
 153 | ERROR | [x] Expected at least 1 space before "."; 0 found
 153 | ERROR | [x] Expected at least 1 space after "."; 0 found
 198 | ERROR | [x] Line indented incorrectly; expected 8 spaces,
     |       |     found 12
 199 | ERROR | [x] Line indented incorrectly; expected 8 spaces,
     |       |     found 12
 249 | ERROR | [x] Line indented incorrectly; expected 8 spaces,
     |       |     found 12
 250 | ERROR | [x] Line indented incorrectly; expected 8 spaces,
     |       |     found 12
 304 | ERROR | [x] Line indented incorrectly; expected 8 spaces,
     |       |     found 12
 305 | ERROR | [x] Line indented incorrectly; expected 8 spaces,
     |       |     found 12
 331 | ERROR | [x] Expected at least 1 space before "."; 0 found
 331 | ERROR | [x] Expected at least 1 space after "."; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 18 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: /var/www/html/tests/Tasks/CollectionFactoryTest.php
----------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 5 LINES
----------------------------------------------------------------------
 56 | ERROR | [x] Array key not indented correctly; expected 12
    |       |     spaces but found 10
 57 | ERROR | [x] Array key not indented correctly; expected 12
    |       |     spaces but found 10
 58 | ERROR | [x] Array key not indented correctly; expected 12
    |       |     spaces but found 10
 59 | ERROR | [x] Array key not indented correctly; expected 12
    |       |     spaces but found 10
 60 | ERROR | [x] Array key not indented correctly; expected 12
    |       |     spaces but found 10
----------------------------------------------------------------------
PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Time: 1.02 secs; Memory: 10MB

You can fix some errors by running following command:
'/var/www/html/vendor/bin/phpcbf' '--standard=./vendor/openeuropa/code-review/resources/library-phpcs-ruleset.xml' '--report=full' '--warning-severity=0' '--ignore=vendor/,loadTasks.php' '/var/www/html/src/Commands/AbstractCommands.php' '/var/www/html/src/Commands/AbstractDrupalCommands.php' '/var/www/html/src/Commands/ChangelogCommands.php' '/var/www/html/src/Services/Composer.php' '/var/www/html/src/TaskRunner.php' '/var/www/html/src/Tasks/CollectionFactory/CollectionFactory.php' '/var/www/html/tests/AbstractTest.php' '/var/www/html/tests/Commands/ReleaseCommandsTest.php' '/var/www/html/tests/CommandsTest.php' '/var/www/html/tests/Tasks/CollectionFactoryTest.php'

@drupol
Copy link
Contributor Author

drupol commented Jan 24, 2019

Thanks for the commits @voidtek !

@drupol drupol force-pushed the OPENEUROPA-1535 branch 2 times, most recently from 6ff542d to 7840ea8 Compare January 30, 2019 12:05
Copy link
Member

@voidtek voidtek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "operators spacing" changes are not requested on this ticket.

This repo is a library and we should follow the coding standard of Symfony.
https://symfony.com/doc/current/contributing/code/standards.html

Add a single space around binary operators (==, &&, ...), with the exception of the concatenation (.) operator;

@drupol
Copy link
Contributor Author

drupol commented Jan 30, 2019

Due to the conflicts with PHPCS rules, I opened a PR to revert those rules.

See: openeuropa/code-review#69

sergepavle
sergepavle previously approved these changes Jan 30, 2019
voidtek
voidtek previously approved these changes Jan 30, 2019
sergepavle
sergepavle previously approved these changes Jan 30, 2019
@drupol drupol merged commit 2aa63e4 into master Jan 30, 2019
@drupol drupol deleted the OPENEUROPA-1535 branch January 30, 2019 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants