Problem/Motivation

After running a Pareview, there are a lot of issues related to coding standards and old usage of t().

See the full report here: https://pareview.sh/pareview/https-git.drupal.org-project-jsonapi_schema...

Proposed resolution

  • Add README.md
  • Fix convetntion issues

Remaining tasks

  • Add README.md
  • Fix convetntion issues
FILE: ...review_temp/src/Normalizer/RelationshipFieldDefinitionNormalizer.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 5 WARNINGS AFFECTING 5 LINES
--------------------------------------------------------------------------
  66 | WARNING | t() calls should be avoided in classes, use dependency
     |         | injection and $this->t() instead
  95 | WARNING | \Drupal calls should be avoided in classes, use
     |         | dependency injection instead
 102 | WARNING | t() calls should be avoided in classes, use dependency
     |         | injection and $this->t() instead
 105 | WARNING | t() calls should be avoided in classes, use dependency
     |         | injection and $this->t() instead
 126 | WARNING | Variable $resource_type is undefined.
--------------------------------------------------------------------------


FILE: ...pareviewsh/pareview_temp/src/Normalizer/DataDefinitionNormalizer.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
 82 | WARNING | \Drupal calls should be avoided in classes, use
    |         | dependency injection instead
--------------------------------------------------------------------------


FILE: ...viewsh/pareview_temp/src/Normalizer/ListDataDefinitionNormalizer.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
 60 | WARNING | t() calls should be avoided in classes, use dependency
    |         | injection and $this->t() instead
--------------------------------------------------------------------------

Time: 2.09 secs; Memory: 6Mb
No automated test cases were found, did you consider writing PHPUnit tests? This is not a requirement but encouraged for professional software development.
This automated report was generated with PAReview.sh, your friendly project application review script.

FILE: ...01/web/vendor/drupal/pareviewsh/pareview_temp/src/Routing/Routes.php
--------------------------------------------------------------------------
FOUND 9 ERRORS AND 1 WARNING AFFECTING 8 LINES
--------------------------------------------------------------------------
  16 | ERROR   | [x] Missing class doc comment
  24 | ERROR   | [ ] Missing short description in doc comment
  35 | ERROR   | [x] Missing function doc comment
  42 | ERROR   | [x] Missing function doc comment
  51 | ERROR   | [x] Missing function doc comment
 103 | ERROR   | [x] Array indentation error, expected 10 spaces but
     |         |     found 12
 104 | ERROR   | [x] Array indentation error, expected 10 spaces but
     |         |     found 12
 107 | WARNING | [ ] Line exceeds 80 characters; contains 124 characters
 107 | ERROR   | [x] No space found before comment text; expected "//
     |         |     $relationship_document_schema_route = new
     |         |     Route($base_path .
     |         |     "/resource/relationships/$public_field_name/schema");"
     |         |     but found "//$relationship_document_schema_route =
     |         |     new Route($base_path .
     |         |     "/resource/relationships/$public_field_name/schema");"
 107 | ERROR   | [x] Inline comments must end in full-stops, exclamation
     |         |     marks, colons, question marks, or closing
     |         |     parentheses
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 8 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...api_hypermedia/LinkProvider/CollectionTopLevelSchemaLinkProvider.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
 5 | WARNING | [x] Unused use statement
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...n/jsonapi_hypermedia/LinkProvider/TopLevelSchemaLinkProviderBase.php
--------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
--------------------------------------------------------------------------
 38 | ERROR | [ ] If the line declaring an array spans longer than 80
    |       |     characters, each element should be broken into its own
    |       |     line
 47 | ERROR | [ ] Missing parameter comment
 73 | ERROR | [x] Expected 1 newline at end of file; 2 found
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...gin/jsonapi_hypermedia/LinkProvider/EntrypointSchemaLinkProvider.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
 5 | WARNING | [x] Unused use statement
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...jsonapi_hypermedia/LinkProvider/ResourceObjectSchemaLinkProvider.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
 10 | WARNING | [x] Unused use statement
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...api_hypermedia/LinkProvider/IndividualTopLevelSchemaLinkProvider.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
 5 | WARNING | [x] Unused use statement
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...review_temp/src/Normalizer/RelationshipFieldDefinitionNormalizer.php
--------------------------------------------------------------------------
FOUND 3 ERRORS AND 1 WARNING AFFECTING 3 LINES
--------------------------------------------------------------------------
  67 | ERROR   | If the line declaring an array spans longer than 80
     |         | characters, each element should be broken into its own
     |         | line
  85 | ERROR   | Parameter $format is not described in comment
  85 | ERROR   | Parameter $context is not described in comment
 126 | WARNING | Code after RETURN statement cannot be executed
--------------------------------------------------------------------------


FILE: ...pareviewsh/pareview_temp/src/Normalizer/DataDefinitionNormalizer.php
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
 23 | ERROR | If the line declaring an array spans longer than 80
    |       | characters, each element should be broken into its own line
--------------------------------------------------------------------------


FILE: .../pareviewsh/pareview_temp/src/Controller/JsonApiSchemaController.php
--------------------------------------------------------------------------
FOUND 12 ERRORS AND 3 WARNINGS AFFECTING 15 LINES
--------------------------------------------------------------------------
  23 | ERROR   | [x] Missing class doc comment
  57 | ERROR   | [ ] Parameter $static_data_definition_extractor is not
     |         |     described in comment
  86 | ERROR   | [x] Missing function doc comment
 109 | ERROR   | [x] Expected 1 space after "=>"; 2 found
 122 | ERROR   | [x] Missing function doc comment
 138 | ERROR   | [x] Expected 1 space after "=>"; 2 found
 170 | ERROR   | [x] Case breaking statements must be followed by a
     |         |     single blank line
 176 | ERROR   | [x] Case breaking statements must be followed by a
     |         |     single blank line
 184 | ERROR   | [x] Missing function doc comment
 199 | WARNING | [x] A comma should follow the last multiline array item.
     |         |     Found: ]
 211 | ERROR   | [x] Missing function doc comment
 250 | ERROR   | [x] Missing function doc comment
 298 | ERROR   | [ ] Missing parameter type
 312 | WARNING | [ ] Only string literals should be passed to t() where
     |         |     possible
 318 | WARNING | [ ] Only string literals should be passed to t() where
     |         |     possible
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 11 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...rupal/pareviewsh/pareview_temp/src/StaticDataDefinitionExtractor.php
--------------------------------------------------------------------------
FOUND 8 ERRORS AFFECTING 8 LINES
--------------------------------------------------------------------------
  55 | ERROR | [ ] Parameter $typed_data_manager is not described in
     |       |     comment
  58 | ERROR | [ ] Missing parameter name
  93 | ERROR | [x] Use "elseif" in place of "else if"
 103 | ERROR | [x] Missing function doc comment
 109 | ERROR | [x] Missing function doc comment
 117 | ERROR | [x] Missing function doc comment
 134 | ERROR | [x] Missing function doc comment
 163 | ERROR | [x] No space found before comment text; expected "//
     |       |     static perspective." but found "//static perspective."
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 6 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

Time: 2.83 secs; Memory: 8Mb
CommentFileSizeAuthor
#4 3119105-3.patch1.81 KBtmaiochi

Comments

BramDriesen created an issue. See original summary.

bradjones1’s picture

bradjones1
Primary language English
commented
Title: Pareview feedback » Remove usages of t()
Priority: Normal » Minor

This is rather old but the t() uses are still a valid finding.

tmaiochi’s picture

tmaiochi commented
Assigned: Unassigned » tmaiochi

I'll work on this.

tmaiochi’s picture

tmaiochi commented
Assigned: tmaiochi » Unassigned
Status: Active » Needs review
StatusFileSize
new 3119105-3.patch1.81 KB

I've removed all t() calls and replace by $this->t() as @bradjones1 changed the title I believe that's all to do. If it's necessary to do more coding standards I'm willing to do.

mariaisp’s picture

mariaisp commented
Assigned: Unassigned » mariaisp

I will review this issue.

mariaisp’s picture

mariaisp commented
Assigned: mariaisp » Unassigned
Status: Needs review » Reviewed & tested by the community

I downloaded the patch, applied it and reviewed it.
I confirm that the usages of t() have been corrected to $this->t().
I also an phpcs in all of the module and verified there were no wrong usages of t() anymore.

m.stenta’s picture

m.stenta
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

We have automated tests now (#3257911: Add basic test coverage), but neither PHP CodeSniffer nor PHPStan are flagging the t() usages.

In order to proceed with this I propose we:

  1. Determine why PHP CodeSniffer / PHPStan are not flagging t().
  2. Tweak testing config so that the usage of t() causes tests to fail.
  3. Use the proposed changes to make the tests pass.