Problem/Motivation

There are a few minor coding standard issues found using phpcbf --standard=vendor/drupal/coder/coder_sniffer/Drupal/

Proposed resolution

Fix these coding standard issues

Remaining tasks

review
commit

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#11 simplesamlphp_auth-cs-3052166-11.patch9.29 KBoknate
#11 3052166--interdiff-9-11.txt5.28 KBoknate
#9 simplesamlphp_auth-cs-3052166-9.patch4.85 KBoknate
#8 simplesamlphp_auth-cs-3052166-8.patch26.46 KBoknate
#5 simplesamlphp_auth-coding-standards-3052166-5.patch4.89 KBoknate
#5 3052166--interdiff-4-5.txt682 bytesoknate
#4 simplesamlphp_auth-coding-standards-3052166-4.patch4.85 KBoknate
#2 simplesamlphp_auth-coding-standards-3052166-2.patch2.59 KBoknate
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

oknate created an issue. See original summary.

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
FileSize
simplesamlphp_auth-coding-standards-3052166-2.patch2.59 KB
idebr’s picture

idebr CreditAttribution: idebr at ezCompany commented
Status: Needs review » Needs work

The code sniffer reports a few more code style issues, see https://www.drupal.org/pift-ci-job/1278654

simplesamlphp_auth.module ✓ 1 fewer
line 132	Type hint "array" missing for $form
src/EventSubscriber/SimplesamlSubscriber.php ✓ 1 fewer
110	Line exceeds 80 characters; contains 86 characters
115	Public method name "SimplesamlSubscriber::login_directly_with_external_IdP" is not in lowerCamel format
124	Line exceeds 80 characters; contains 94 characters
src/Service/SimplesamlphpAuthManager.php ✓ 2 fewer
156	Translatable strings must not begin or end with white spaces, use placeholders with t() for variables
156	Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
184	Translatable strings must not begin or end with white spaces, use placeholders with t() for variables
184	Concatenating translatable strings is not allowed, use placeholders instead and only one string literal
oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
FileSize
simplesamlphp_auth-coding-standards-3052166-4.patch4.85 KB

I address the issues mentioned in #3 in this patch. I haven't tested it yet. I'd like to test the message statements that I changed to make sure I didn't break anything. Other than that, everything is pretty strait forward.

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Status: Needs work » Needs review
FileSize
3052166--interdiff-4-5.txt682 bytes
simplesamlphp_auth-coding-standards-3052166-5.patch4.89 KB

Fixing one suggestion from @TR on #3047241: Remove deprecated code for Drupal 9,

[the change to the comment] fixes one coding standards issue - line > 80 characters, but causes a new coding standards issue - function short description should be one line.

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Issue summary: View changes
idebr’s picture

idebr CreditAttribution: idebr at ezCompany commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

Patch no longer applies after #3047241: Remove deprecated code for Drupal 9 was committed.

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Status: Needs work » Needs review
FileSize
simplesamlphp_auth-cs-3052166-8.patch26.46 KB

Ignore this one.

After I posted this I saw this:

- - drupal:user
- - externalauth:externalauth
+ - user
+ - externalauth

dang, that should not be there.

oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
FileSize
simplesamlphp_auth-cs-3052166-9.patch4.85 KB

Reroll

idebr’s picture

idebr CreditAttribution: idebr at ezCompany commented
Status: Needs review » Needs work

phpcs is finding a few more errors that should be fixed a well:

FILE: simplesamlphp_auth/tests/src/Unit/Service/SimplesamlphpAuthManagerTest.php
------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------
 158 | ERROR | Description for the @return value is missing
------------------------------------------------------------------------------------------------------------------


FILE: simplesamlphp_auth/tests/src/Unit/Service/SimplesamlphpDrupalAuthTest.php
-----------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
-----------------------------------------------------------------------------------------------------------------
   8 | WARNING | [x] Unused use statement
 109 | WARNING | [x] A comma should follow the last multiline array item. Found: 'mail'
-----------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------


FILE: simplesamlphp_auth/simplesamlphp_auth.api.php
---------------------------------------------------------------------------------------
FOUND 6 ERRORS AFFECTING 5 LINES
---------------------------------------------------------------------------------------
  21 | ERROR | Type hint "array" missing for &$roles
  21 | ERROR | Type hint "array" missing for $attributes
  52 | ERROR | Type hint "array" missing for $attributes
 100 | ERROR | Return type "\Drupal\user\UserInterface | bool" must not contain spaces
 103 | ERROR | Type hint "array" missing for $attributes
 130 | ERROR | Type hint "array" missing for $attributes
---------------------------------------------------------------------------------------


FILE: simplesamlphp_auth/src/Form/BasicSettingsForm.php
-------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------
 103 | WARNING | Avoid backslash escaping in translatable strings when possible, use "" quotes instead
-------------------------------------------------------------------------------------------------------


FILE: simplesamlphp_auth/src/Service/SimplesamlphpDrupalAuth.php
-------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------------
 254 | ERROR | If the line declaring an array spans longer than 80 characters, each element should be broken into its own line
-------------------------------------------------------------------------------------------------------------------------------


FILE: simplesamlphp_auth/src/Service/SimplesamlphpAuthManager.php
---------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------
 272 | ERROR | [x] Data types in @throws tags need to be fully namespaced
 272 | ERROR | [x] Data types in @throws tags need to be fully namespaced
---------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------------


FILE: simplesamlphp_auth/src/EventSubscriber/SimplesamlSubscriber.php
-----------------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
-----------------------------------------------------------------------------------------------------------------------
 123 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
 128 | ERROR | Public method name "SimplesamlSubscriber::login_directly_with_external_IdP" is not in lowerCamel format
-----------------------------------------------------------------------------------------------------------------------
oknate’s picture

oknate
Location Greater New York City Area
CreditAttribution: oknate as a volunteer and commented
Status: Needs work » Needs review
FileSize
3052166--interdiff-9-11.txt5.28 KB
simplesamlphp_auth-cs-3052166-11.patch9.29 KB

Fixing most of the errors in #10.

For missing type hints in simplesamlphp_auth.api.php, leaving those out, as they are API changes

Same with this one:
FILE: simplesamlphp_auth/src/EventSubscriber/SimplesamlSubscriber.php
128 | ERROR | Public method name "SimplesamlSubscriber::login_directly_with_external_IdP" is not in lowerCamel format

  • Berdir committed 4cb6fb8 on 8.x-3.x authored by oknate 
    Issue #3052166 by oknate, idebr, Berdir: Coding style fixes
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Fixed

Thanks, removed some double spaces and also updated some deprecated method calls, committed.

Status: Fixed » Closed (fixed)

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