Skip to content

Commit

Permalink
Check for correct whitespace in method declarations
Browse files Browse the repository at this point in the history
This completes other existing Sniffs that already are doing
the job partially:
- Squiz.Whitespace.ScopeKeywordSpacing (for visibility and static).
- PSR2.Methods.MethodDeclaration (for ordering).

What we are doing here is:
- Add the following Sniffs to verify that he return type is
  being used correctly (space-wise):
   - PSR12.Functions.ReturnTypeDeclaration
   - PSR12.Functions.NullableTypeDeclaration
- Create a new own Sniff: MethodDeclarationSpacingSniff that is
  in charge of checking all the remaining bits:
   - 1 exact space after "abstract" and "final".
   - 1 exact space after "function".
   - 0 spaces after the function name.
   - 0 spaces after the function open parenthesis.

With everything enabled, we'll get something like this:
```
<     final    abstract     protected static function     kk    ( $a, $b, $c  )   :?  int  {
---
>     final abstract protected static function kk($a, $b, $c  ): ?int {
```

Fixes #99
  • Loading branch information
stronk7 committed Feb 14, 2024
1 parent 42ab278 commit 3063d93
Show file tree
Hide file tree
Showing 2 changed files with 174 additions and 0 deletions.
170 changes: 170 additions & 0 deletions moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
<?php
// This file is part of Moodle - https://moodle.org/
//
// Moodle is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// Moodle is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with Moodle. If not, see <https://www.gnu.org/licenses/>.

/**
* This sniff checks that method declarations are correct, spacing-wise.
*
* This Sniff completes other Sniffs, already included in the moodle standard,
* to ensure that the methods declarations make a correct use of whitespace.
* More specifically, it completes the following:
* - PSR12.Functions.ReturnTypeDeclaration
* - PSR12.Functions.NullableTypeDeclaration
* - PSR2.Methods.MethodDeclaration
* - Squiz.Whitespace.ScopeKeywordSpacing
*
* If any of the above Sniffs is disabled, then we'll have to add more
* features to this one.
* @copyright 2024 onwards Eloy Lafuente (stronk7) {@link https://stronk7.com}
* @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/

namespace MoodleHQ\MoodleCS\moodle\Sniffs\Methods;

use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Sniffs\AbstractScopeSniff;
use PHP_CodeSniffer\Util\Tokens;

class MethodDeclarationSpacingSniff extends AbstractScopeSniff
{
public function __construct() {
parent::__construct(Tokens::$ooScopeTokens, [T_FUNCTION]);

Check warning on line 44 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L43-L44

Added lines #L43 - L44 were not covered by tests
}

/**
* Processes the function tokens within the class.
*
* @param File $file The file where this token was found.
* @param int $stackPtr The position where the token was found.
* @param int $currScope The current scope opener token.
*
* @return void
*/
protected function processTokenWithinScope(File $file, $stackPtr, $currScope)

Check warning on line 56 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L56

Added line #L56 was not covered by tests
{
// List of tokens that require one space after them.
$oneSpaceTokens = [

Check warning on line 59 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L59

Added line #L59 was not covered by tests
// T_PUBLIC, // Disabled. Squiz.WhiteSpace.ScopeKeywordSpacing handles it.
// T_PROTECTED, // Disabled. Squiz.WhiteSpace.ScopeKeywordSpacing handles it.
// T_PRIVATE, // Disabled. Squiz.WhiteSpace.ScopeKeywordSpacing handles it.
// T_STATIC, // Disabled. Squiz.WhiteSpace.ScopeKeywordSpacing handles it.
T_ABSTRACT,
T_FINAL,
T_FUNCTION,
];

Check warning on line 67 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L64-L67

Added lines #L64 - L67 were not covered by tests

// List of tokens that require no space after them.
$noSpaceTokens = [
T_STRING,
T_OPEN_PARENTHESIS,
];

Check warning on line 73 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L70-L73

Added lines #L70 - L73 were not covered by tests

$tokens = $file->getTokens();

Check warning on line 75 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L75

Added line #L75 was not covered by tests

// Determine if this is a function which needs to be examined.
$conditions = $tokens[$stackPtr]['conditions'];
end($conditions);
$deepestScope = key($conditions);
if ($deepestScope !== $currScope) {
return;

Check warning on line 82 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L78-L82

Added lines #L78 - L82 were not covered by tests
}

$methodName = $file->getDeclarationName($stackPtr);
if ($methodName === null) {

Check warning on line 86 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L85-L86

Added lines #L85 - L86 were not covered by tests
// Ignore closures.
return;

Check warning on line 88 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L88

Added line #L88 was not covered by tests
}

// Find the opening parenthesis of the argument list. That's the last token
// that this Sniff is interested in. If, for any reason, the opening parenthesis
// is not found, then we'll start from current T_FUNCTION token.
$lastToken = $tokens[$stackPtr]['parenthesis_opener'] ?? $stackPtr;

Check warning on line 94 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L94

Added line #L94 was not covered by tests

// These are the tokens that we are interested on.
$findTokens = array_merge($oneSpaceTokens, $noSpaceTokens);

Check warning on line 97 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L97

Added line #L97 was not covered by tests

// Find the first token that we are interested in.
$foundToken = $file->findPrevious($findTokens, $lastToken);

Check warning on line 100 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L100

Added line #L100 was not covered by tests

// Search backwards and examine all the matches found until we change of line.
while ($tokens[$foundToken]['line'] === $tokens[$lastToken]['line']) {

Check warning on line 103 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L103

Added line #L103 was not covered by tests
// If it's a token that requires one space after it, check if there is one.
if (in_array($tokens[$foundToken]['code'], $oneSpaceTokens) === true) {
$replacement = ' ';

Check warning on line 106 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L105-L106

Added lines #L105 - L106 were not covered by tests
if (
$tokens[$foundToken + 1]['code'] === T_WHITESPACE && // We only check whitespace.
$tokens[$foundToken + 1]['content'] !== ' '

Check warning on line 109 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L108-L109

Added lines #L108 - L109 were not covered by tests
) {
$error = 'Expected 1 space after "%s"; %s found';
$data = [
$tokens[$foundToken]['content'],
strlen($tokens[$foundToken + 1]['content']),
];
$fix = $file->addFixableError($error, $foundToken, 'OneExpectedAfter', $data);
if ($fix === true) {
$file->fixer->replaceToken($foundToken + 1, $replacement);

Check warning on line 118 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L111-L118

Added lines #L111 - L118 were not covered by tests
}
}
}

// If it's a token that requires no space after it, check if there is none.
if (in_array($tokens[$foundToken]['code'], $noSpaceTokens) === true) {
$replacement = '';
$process = true;

Check warning on line 126 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L124-L126

Added lines #L124 - L126 were not covered by tests

// We only process T_STRING if it matches the method name.
if ($tokens[$foundToken]['code'] === T_STRING) {
if ($tokens[$foundToken]['content'] !== $methodName) {
$process = false;

Check warning on line 131 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L129-L131

Added lines #L129 - L131 were not covered by tests
}
}

// We only process T_OPEN_PARENTHESIS if it has spaces, and only
// to get rid of them
if ($tokens[$foundToken]['code'] === T_OPEN_PARENTHESIS) {
if (strpos($tokens[$foundToken + 1]['content'], ' ') !== false) {
$replacement = str_replace(' ', '', $tokens[$foundToken + 1]['content']);

Check warning on line 139 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L137-L139

Added lines #L137 - L139 were not covered by tests
} else {
$process = false;

Check warning on line 141 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L141

Added line #L141 was not covered by tests
}
}

if (
$tokens[$foundToken + 1]['code'] === T_WHITESPACE && // We only check whitespace.
$tokens[$foundToken + 1]['content'] !== '' &&
$process === true

Check warning on line 148 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L146-L148

Added lines #L146 - L148 were not covered by tests
) {
$error = 'Expected 0 spaces after "%s"; %s found';
$data = [
$tokens[$foundToken]['content'],
strlen($tokens[$foundToken + 1]['content']),
];
$fix = $file->addFixableError($error, $foundToken, 'ZeroExpectedAfter', $data);
if ($fix === true) {
$file->fixer->replaceToken($foundToken + 1, $replacement);

Check warning on line 157 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L150-L157

Added lines #L150 - L157 were not covered by tests
}
}
}

// Move to the previous interesting token.
$foundToken = $file->findPrevious($findTokens, $foundToken - 1);

Check warning on line 163 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L163

Added line #L163 was not covered by tests
}
}

protected function processTokenOutsideScope(File $file, $stackPtr) {
return;

Check warning on line 168 in moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Methods/MethodDeclarationSpacingSniff.php#L167-L168

Added lines #L167 - L168 were not covered by tests
}
}
4 changes: 4 additions & 0 deletions moodle/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@
-->
<rule ref="PSR12.Files.ImportStatement.LeadingSlash"/>

<!-- PSR12 function return types (whitespace handling) -->
<rule ref="PSR12.Functions.ReturnTypeDeclaration"/>
<rule ref="PSR12.Functions.NullableTypeDeclaration"/>

<!-- Let's add the complete PHPCompatibility standard -->
<rule ref="PHPCompatibility" />

Expand Down

0 comments on commit 3063d93

Please sign in to comment.