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.

Sign up for GitHub

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

chore: update phpcs.xml from Drupal examples #670

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

chore: update phpcs.xml from Drupal examples #670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d7a8f01
chore: update phpcs.xml from Drupal examples
hussainweb Nov 12, 2024
4d68e5e
fix: phpcs violations
hussainweb Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 5 additions & 42 deletions phpcs.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,54 +1,26 @@
<?xml version="1.0" ?>
<?xml version="1.0" encoding="UTF-8"?>
<!--
Based on Examples for Developers phpcs.xml.dist. Modified for use for Drupal
projects. See https://git.drupalcode.org/project/examples/blob/HEAD/phpcs.xml.dist
for source.
-->
<ruleset name="AxelerantDrupal">
<description>Axelerant baseline rules for PHP CodeSniffer</description>

<!--Exclude folders used by common frontend tools.-->
<exclude-pattern>*/bower_components/*</exclude-pattern>
<exclude-pattern>*/node_modules/*</exclude-pattern>
<!--Exclude third party code.-->
<exclude-pattern>./assets/vendor/*</exclude-pattern>
Copy link
Collaborator

Choose a reason for hiding this comment

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

@hussainweb Exclude is mostly for use when we run phpcs directly. but again, if we don't pass include directories, it will run on whole codebase so if we plan to keep this up and allow user to run phpcs -v then we will have to exclude other directories as well and I think, this will be overkill. We can remove this as we have exclude dirs specified in grumphp conf file.

Mostly exclude all these dirs

<!-- Exclude core, vendor, and contrib directories -->
  <exclude-pattern>./web/core/*</exclude-pattern>
  <exclude-pattern>./vendor/*</exclude-pattern>
  <exclude-pattern>./web/modules/contrib/*</exclude-pattern>
  <exclude-pattern>./web/hemes/contrib/*</exclude-pattern>
  <exclude-pattern>./web/profiles/contrib/*</exclude-pattern>
  <exclude-pattern>./private/*</exclude-pattern>
  <exclude-pattern>./config/*</exclude-pattern>
  <exclude-pattern>./web/sites/*</exclude-pattern>
  <exclude-pattern>./.ddev/*</exclude-pattern>


<file>.</file>

<!-- PHPCS Arguments -->
<arg name="extensions" value="inc,yml,install,module,php,profile,test,theme" />
<arg name="colors" />

<!--Allow global variables in settings file.-->
<rule ref="DrupalPractice.CodeAnalysis.VariableAnalysis.UndefinedVariable">
<exclude-pattern>settings(\.[a-zA-Z0-9]+)?\.php</exclude-pattern>
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we remove this, phpcs will stop catching UndefinedVariable. Currently it did not catch it in settings.php because /web/sites/default is excluded in grumphp. So if don't want that exclude pattern, we should only keep the rule without any exclude as settings file are already excluded in grumphp.

<rule ref="DrupalPractice.CodeAnalysis.VariableAnalysis.UndefinedVariable" />

Same applies for other DrupalPractice.Commenting rules.

</rule>

<!--Allow section separators in settings.php file.-->
<rule ref="DrupalPractice.Commenting.CommentEmptyLine.SpacingAfter">
<exclude-pattern>settings(\.[a-zA-Z0-9]+)?\.php</exclude-pattern>
</rule>
<rule ref="Drupal.Commenting.InlineComment.InvalidEndChar">
<exclude-pattern>settings(\.[a-zA-Z0-9]+)?\.php</exclude-pattern>
</rule>
<rule ref="Drupal.Commenting.InlineComment.NoSpaceBefore">
<exclude-pattern>settings(\.[a-zA-Z0-9]+)?\.php</exclude-pattern>
</rule>
<rule ref="Drupal.Commenting.InlineComment.SpacingAfter">
<exclude-pattern>settings(\.[a-zA-Z0-9]+)?\.php</exclude-pattern>
</rule>

<!-- Only include specific sniffs that pass. This ensures that, if new sniffs are added, HEAD does not fail.-->
<!-- Drupal sniffs -->
<rule ref="Drupal"/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Spacing in doc comment can be achieved using <rule ref="Drupal.Commenting.DocCommentAlignment"/>.

<rule ref="Drupal.Arrays.Array">
<!-- Sniff for these errors: CommaLastItem -->
<exclude name="Drupal.Arrays.Array.ArrayClosingIndentation"/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

These rules exclude were present earlier too. I removed it. It improves code readability when arrays are properly indented. So I will suggest to remove it.

<exclude name="Drupal.Arrays.Array.ArrayIndentation"/>
<exclude name="Drupal.Arrays.Array.LongLineDeclaration"/>
</rule>
<rule ref="Drupal.Classes.ClassCreateInstance"/>
<rule ref="Drupal.Classes.ClassDeclaration"/>
<rule ref="Drupal.Classes.FullyQualifiedNamespace"/>
<rule ref="Drupal.Classes.InterfaceName"/>
<rule ref="Drupal.Classes.UnusedUseStatement"/>
<rule ref="Drupal.Classes.UseGlobalClass"/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any specific reason for removing this? This checks the following.

You cannot use the use statement for non-namespaced classes; instead, you must reference them with a leading \, e.g., \DateTimeImmutable instead of use DateTimeImmutable;.

<rule ref="Drupal.Classes.UseLeadingBackslash"/>
<rule ref="Drupal.CSS.ClassDefinitionNameSpacing"/>
<rule ref="Drupal.CSS.ColourDefinition"/>
Expand Down Expand Up @@ -342,17 +314,8 @@
<rule ref="Squiz.WhiteSpace.LanguageConstructSpacing" />
<rule ref="Squiz.WhiteSpace.SemicolonSpacing"/>
<rule ref="Squiz.WhiteSpace.SuperfluousWhitespace"/>
<rule ref="Squiz.WhiteSpace.OperatorSpacing">
<properties>
<property name="ignoreNewlines" value="true"/>
</properties>
</rule>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This checks operator spacing rules.

Here are few examples.

Correct: $a = $b + $c;
Incorrect: $a=$b+$c;

Correct

  $abc = [
    "abc3" => "dssdsd3",
    "abc41" => [
      "dssdsd4"
    ],
  ];

  $abc = [
    "abc3" => "dssdsd3",
    "abc41" => 
    [
      "dssdsd4"
    ],
  ];

Incorrect

$abc = [
    "abc3"=> "dssdsd3",
    "abc41" =>[
      "dssdsd4"
    ],
  ];


<!-- Zend sniffs -->
<rule ref="Zend.Files.ClosingTag"/>

<!-- sniffs from https://github.com/acquia/coding-standards-php -->
<rule ref="SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

These are from Acquia coding standards and sorts use statements in ascending order. This can be removed as sorting manually is a challenge but phpcbf can help sort it.

<rule ref="SlevomatCodingStandard.Variables.DisallowSuperGlobalVariable" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can also keep this to discourage direct use of $_GET, $_POST, $_SERVER and $GLOBALS etc.


</ruleset>
2 changes: 1 addition & 1 deletion web/modules/custom/contrib_tracker/src/EventSubscriber/RavenSubscriber.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php

declare(strict_types = 1);
declare(strict_types=1);

namespace Drupal\contrib_tracker\EventSubscriber;

Expand Down
4 changes: 2 additions & 2 deletions web/modules/custom/ct_manager/src/Data/CodeContribution.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ final class CodeContribution {
protected int $patchCount = 0;

/**
* @var int File count.
*/
* @var int File count.
*/
protected int $filesCount = 0;

/**
Expand Down
16 changes: 8 additions & 8 deletions web/modules/custom/ct_user/src/Plugin/Block/ContribGraph.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
use Drupal\Core\Block\BlockBase;

/**
* Provides a block with a contribution commit graph.
*
* @Block(
* id = "contrib_graph_blocl",
* admin_label = @Translation("Contribution commit graph"),
* category = "Custom"
* )
*/
* Provides a block with a contribution commit graph.
*
* @Block(
* id = "contrib_graph_blocl",
* admin_label = @Translation("Contribution commit graph"),
* category = "Custom"
* )
*/
class ContribGraph extends BlockBase {

/**
Expand Down
4 changes: 2 additions & 2 deletions web/themes/custom/contribtracker/contribtracker.theme
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ function contribtracker_theme_preprocess_page_alter(array &$suggestions, array $
}

/**
* Implements hook_theme_suggestions_input_alter().
*/
* Implements hook_theme_suggestions_input_alter().
*/
function contribtracker_theme_suggestions_input_alter(&$suggestions, array $variables) {
if (!empty($variables['element']['#id'])) {
$suggestions[] = 'input__' . str_replace('-', '_', $variables['element']['#id']);
Expand Down