From cfcf146057d3be0da13c54c9f5f29ec8b9ed3d33 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Mon, 19 Feb 2024 10:31:12 +0100 Subject: [PATCH] TODO: Improve the fixer to save passes need to accumulate everything in each pass in mamory array and then execute all the moves in a row, to avoid going 1 by 1. --- .../Sniffs/Files/LangFilesOrderingSniff.php | 216 ++++++++++++++++++ .../Sniffs/Files/LangFilesOrderingTest.php | 87 +++++++ .../lang/en/langFilesOrdering.php | 64 ++++++ .../lang/en/langFilesOrdering.php.fixed | 64 ++++++ .../lang/en@wrong/langFilesOrdering.php | 25 ++ 5 files changed, 456 insertions(+) create mode 100644 moodle/Sniffs/Files/LangFilesOrderingSniff.php create mode 100644 moodle/Tests/Sniffs/Files/LangFilesOrderingTest.php create mode 100644 moodle/Tests/Sniffs/Files/fixtures/langFilesOrdering/lang/en/langFilesOrdering.php create mode 100644 moodle/Tests/Sniffs/Files/fixtures/langFilesOrdering/lang/en/langFilesOrdering.php.fixed create mode 100644 moodle/Tests/Sniffs/Files/fixtures/langFilesOrdering/lang/en@wrong/langFilesOrdering.php diff --git a/moodle/Sniffs/Files/LangFilesOrderingSniff.php b/moodle/Sniffs/Files/LangFilesOrderingSniff.php new file mode 100644 index 0000000..3d188ec --- /dev/null +++ b/moodle/Sniffs/Files/LangFilesOrderingSniff.php @@ -0,0 +1,216 @@ +. + +/** + * This sniff verifies that lang files are sorted alphabetically by string key. + * + * @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\Files; + +use MoodleHQ\MoodleCS\moodle\Util\MoodleUtil; +use PHP_CodeSniffer\Files\File; +use PHP_CodeSniffer\Sniffs\Sniff; + +class LangFilesOrderingSniff implements Sniff +{ + /** + * @var int The pointer to the previous string key found. + */ + protected int $previousStringPtr = 0; + + /** + * @var bool Whether fixing is enabled or not. Some stuff in lang files can lead to disabling it. + */ + protected $fixingEnabled = true; + + public function __construct() { + static $times = 0; + $times++; + error_log('-----'); + error_log('LangFilesOrderingSniff initialized (' . $times . ') times'); + } + + public function register(): array { + return [ + T_VARIABLE, // We are interested in all the variables in the file. + T_COMMENT, // and, also, some comments to find the different "sections". + ]; + } + + public function process(File $phpcsFile, $stackPtr): void { + // If the file is not a lang file, return. + if (!MoodleUtil::isLangFile($phpcsFile)) { + return; + } + + // Only for Moodle 4.4dev (404) and up. + // Make and exception for unit tests, so they are run always. + if (!MoodleUtil::meetsMinimumMoodleVersion($phpcsFile, 404) && !MoodleUtil::isUnitTestRunning()) { + return; // @codeCoverageIgnore + } + + // Get the file tokens, for ease of use. + $tokens = $phpcsFile->getTokens(); + + // If we have arrived to a "Deprecated since" T_COMMENT, we are starting a new section to sort. + if ( + $tokens[$stackPtr]['code'] === T_COMMENT && + strpos($tokens[$stackPtr]['content'], '// Deprecated since Moodle') !== false + ) { + // Start over within the new section, reset the previous string pointer. + $this->previousStringPtr = 0; + return; + } + + // If fixing is still enabled, and we find some unexpected (different from the "Deprecated since" + // ones) comment, we will disable fixing for the rest of the file. Because it can be + // problematic to properly find the deprecation sections. + if ( + $this->previousStringPtr > 0 && + $tokens[$stackPtr]['code'] === T_COMMENT && + strpos($tokens[$stackPtr]['content'], '// Deprecated since Moodle') === false + ) { + $phpcsFile->addError( + 'Unexpected comment found in the lang file, only "// Deprecated since" comments are allowed', + $stackPtr, + 'UnexpectedComment' + ); + $this->fixingEnabled = false; + } + + // No further checks are needed for T_COMMENT tokens. + if ($tokens[$stackPtr]['code'] === T_COMMENT) { + return; + } + + // From now on, we are only working with T_VARIABLE tokens. + + // If the variable is not "$string", that's completely unexpected, error. + if ($tokens[$stackPtr]['content'] !== '$string') { + $phpcsFile->addError( + 'Variable "%s" is not expected in a lang file', + $stackPtr, + 'UnexpectedVariable', + [$tokens[$stackPtr]['content']] + ); + return; + } + + // Get the string key, if any. + if (!$stringKey = $this->getStringKey($phpcsFile, $stackPtr)) { + return; + } + + // Compare the current string key with the previous one. + if ($this->previousStringPtr > 0) { + $previousStringKey = $this->getStringKey($phpcsFile, $this->previousStringPtr); + + // Detect duplicates and add error on them. + if ($previousStringKey === $stringKey) { + $phpcsFile->addError( + 'The string key "%s" is duplicated', + $stackPtr, + 'DuplicatedKey', + [$stringKey] + ); + } + + // Manage unordered keys and fix them by simple swapping. + if (strcmp($previousStringKey, $stringKey) > 0) { + // We are going to get the current string, delete it and insert it before the previous one. + // Find where (with which token) the current string ends (T_SEMICOLON + line feed). + $currentEndToken = $phpcsFile->findNext(T_SEMICOLON, $stackPtr + 1) + 1; + + // Verify that the current end token is a line feed, if not, we won't be able to fix (swap). + $swappable = true; + if ( + !isset($tokens[$currentEndToken]) || + $tokens[$currentEndToken]['code'] !== T_WHITESPACE || + $tokens[$currentEndToken]['content'] !== "\n" + ) { + $swappable = false; + } + $fixable = $this->fixingEnabled && $swappable; + + $fix = $phpcsFile->addWarning( + 'The string key "%s" is not in the correct order, it should be before "%s"', + $stackPtr, + 'IncorrectOrder', + [$stringKey, $previousStringKey], + 0, + $fixable + ); + + if ($fix && $fixable) { + error_Log(' - Swap ' . $stringKey . ' and ' . $previousStringKey); + // Note that the swapping technique used here (bubbling) for the fixes is + // not the best one, and it will lead to phpcbf having to + // perform a lot of passes until the whole file is fixed. But + // it's the easiest one to implement using exclusively the CodeSniffer + // API (and assuming that the majority of strings are already sorted + // implies that we won't face worst-case scenarios). A better approach + // would be to use a custom fixer, loading the whole file and applying + // the changes in a single pass. + + // Now we are going to swap the strings. + $phpcsFile->fixer->beginChangeset(); + // For every token in the current string. + foreach (range($stackPtr, $currentEndToken) as $tokenIndex) { + $tempToken = $tokens[$tokenIndex]; // Store the token. + $phpcsFile->fixer->replaceToken($tokenIndex, ''); // Delete the current string token. + $phpcsFile->fixer->addContent( + $this->previousStringPtr - 1, + $tempToken['content'] // Insert the token before the previous string. + ); + } + $phpcsFile->fixer->endChangeset(); + } + } + } + $this->previousStringPtr = $stackPtr; + } + + /** + * Return the string key corresponding to the string at the pointer. + * Note that the key has got any quote (single or double) trimmed. + * + * return string|null + */ + protected function getStringKey(File $phpcsFile, $stackPtr): ?string { + $tokens = $phpcsFile->getTokens(); + + // If the structure is not exactly: $string[KEY], add error and return null. + if ( + $tokens[$stackPtr + 1]['code'] !== T_OPEN_SQUARE_BRACKET || + $tokens[$stackPtr + 2]['code'] !== T_CONSTANT_ENCAPSED_STRING || + $tokens[$stackPtr + 3]['code'] !== T_CLOSE_SQUARE_BRACKET + ) { + $phpcsFile->addError( + "Unexpected string syntax, it should be `\$string['key']`", + $stackPtr, + 'UnexpectedSyntax' + ); + return null; + } + + // Now we can safely extract the string key and return it. + return trim($tokens[$stackPtr + 2]['content'], "'\""); + } +} diff --git a/moodle/Tests/Sniffs/Files/LangFilesOrderingTest.php b/moodle/Tests/Sniffs/Files/LangFilesOrderingTest.php new file mode 100644 index 0000000..cc511d7 --- /dev/null +++ b/moodle/Tests/Sniffs/Files/LangFilesOrderingTest.php @@ -0,0 +1,87 @@ +. + +namespace MoodleHQ\MoodleCS\moodle\Tests\Files; + +use MoodleHQ\MoodleCS\moodle\Tests\MoodleCSBaseTestCase; + +/** + * Test the LangFilesOrderingSniff sniff. + * + * @copyright 2024 onwards Eloy Lafuente (stronk7) {@link https://stronk7.com} + * @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * + * @covers \MoodleHQ\MoodleCS\moodle\Sniffs\Files\LangFilesOrderingSniff + */ +class LangFilesOrderingTest extends MoodleCSBaseTestCase +{ + /** + * @dataProvider filesOrderingProvider + */ + public function testLangFilesOrdering( + string $fixture, + array $warnings, + array $errors + ) { + $this->setStandard('moodle'); + $this->setSniff('moodle.Files.LangFilesOrdering'); + $this->setFixture(__DIR__ . '/fixtures/langFilesOrdering/' . $fixture); + $this->setWarnings($warnings); + $this->setErrors($errors); + + $this->verifyCsResults(); + } + + /** + * Data provider for testLangFilesOrdering tests. + * + * @return array + */ + public static function filesOrderingProvider(): array { + return [ + 'processed' => [ + 'lang/en/langFilesOrdering.php', + [ + 27 => '"modvisiblewithstealth_help" is not in the correct order', + 30 => 1, + 34 => 1, + 37 => 1, + 38 => 1, + 42 => 1, + 45 => 1, + 46 => 1, + 47 => 1, + 51 => 1, + 61 => 1, + 63 => 1, + 64 => 1, + ], + [ + 31 => 'Variable "$anothervar" is not expected', + 33 => 'Unexpected string syntax, it should be', + 40 => 'The string key "yourself" is duplicated', + 60 => 'Unexpected comment found in the lang file', + ], + ], + 'not processed' => [ + 'lang/en@wrong/langFilesOrdering.php', + [], + [], + ], + ]; + } +} diff --git a/moodle/Tests/Sniffs/Files/fixtures/langFilesOrdering/lang/en/langFilesOrdering.php b/moodle/Tests/Sniffs/Files/fixtures/langFilesOrdering/lang/en/langFilesOrdering.php new file mode 100644 index 0000000..7a5df8e --- /dev/null +++ b/moodle/Tests/Sniffs/Files/fixtures/langFilesOrdering/lang/en/langFilesOrdering.php @@ -0,0 +1,64 @@ +. + +/** + * A fixture lang-like file in correct place to test the LangFilesOrderingSniff. + * + * @package core + * @copyright 1999 onwards Martin Dougiamas {@link http://moodle.com} + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +$string['abouttobeinstalled'] = 'about to be installed'; +$string['yourlastlogin'] = 'Your last login was'; +$string['modvisiblewithstealth_help'] = '* Show on course page: Available to students (subject to any access restrictions which may be set). +* Hide on course page: Not available to students. +* Make available but don\'t show on course page: Available to students if you provide a link. Activities will still appear in the gradebook and other reports.'; +$string['action'] = 'Action'; +$anothervar['choco'] = 'kk'; +$string['youneedtoenrol'] = 'To perform that action you need to enrol in this course.'; +$string['bad' ] = 'Bad'; +$string['actionchoice'] = 'What do you want to do with the file \'{$a}\'?'; +$string['actions'] = 'Actions'; +$string['moodlenet:sharesuccesstext'] = "Almost done! Visit your drafts in MoodleNet to finish sharing your content."; +$string['actionsmenu2'] = "Actions menu2"; +$string["actionsmenu"] = 'Actions menu'; +$string['yourself'] = 'yourself'; +$string['yourself'] = 'yourself'; +$string['yourteacher'] = 'your {$a}'; +$string['withoutlinefeed'] = 'Without line feed';Any content after semicolon prevents the fixer to move this line. +$string['yourwordforx'] = 'Your word for \'{$a}\''; +$string['zippingbackup'] = 'Zipping backup'; +$string['deprecatedeventname'] = '{$a} (no longer in use)'; +$string['accept'] = 'Accept'; +$string['1111'] = '1111'; + +// Deprecated since Moodle 4.0. +$string['cupplyinfo'] = 'More details'; +$string['bcreateuserandpass'] = 'Choose your username and password'; + +// Deprecated since Moodle 4.3. +$string['clicktochangeinbrackets'] = '{$a} (Click to change)'; + +// Deprecated since Moodle 4.4. +$string['asocialheadline'] = 'Social forum - latest topics'; +$string['topicshow'] = 'Show this topic to {$a}'; + +$string['zzzz'] = 'zzzz'; // This comment shouldn't be here. No more auto-fixing after this line. +$string['yyyy'] = 'yyyy'; + +$string['bbbb'] = 'bbbb'; +$string['aaaa'] = 'aaaa'; diff --git a/moodle/Tests/Sniffs/Files/fixtures/langFilesOrdering/lang/en/langFilesOrdering.php.fixed b/moodle/Tests/Sniffs/Files/fixtures/langFilesOrdering/lang/en/langFilesOrdering.php.fixed new file mode 100644 index 0000000..083c140 --- /dev/null +++ b/moodle/Tests/Sniffs/Files/fixtures/langFilesOrdering/lang/en/langFilesOrdering.php.fixed @@ -0,0 +1,64 @@ +. + +/** + * A fixture lang-like file in correct place to test the LangFilesOrderingSniff. + * + * @package core + * @copyright 1999 onwards Martin Dougiamas {@link http://moodle.com} + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +$string['1111'] = '1111'; +$string['abouttobeinstalled'] = 'about to be installed'; +$string['accept'] = 'Accept'; +$string['action'] = 'Action'; +$string['actionchoice'] = 'What do you want to do with the file \'{$a}\'?'; +$string['actions'] = 'Actions'; +$string["actionsmenu"] = 'Actions menu'; +$string['actionsmenu2'] = "Actions menu2"; +$string['deprecatedeventname'] = '{$a} (no longer in use)'; +$string['modvisiblewithstealth_help'] = '* Show on course page: Available to students (subject to any access restrictions which may be set). +* Hide on course page: Not available to students. +* Make available but don\'t show on course page: Available to students if you provide a link. Activities will still appear in the gradebook and other reports.'; +$string['moodlenet:sharesuccesstext'] = "Almost done! Visit your drafts in MoodleNet to finish sharing your content."; +$string['youneedtoenrol'] = 'To perform that action you need to enrol in this course.'; +$string['yourlastlogin'] = 'Your last login was'; +$anothervar['choco'] = 'kk'; +$string['bad' ] = 'Bad'; +$string['yourself'] = 'yourself'; +$string['yourself'] = 'yourself'; +$string['yourteacher'] = 'your {$a}'; +$string['withoutlinefeed'] = 'Without line feed';Any content after semicolon prevents the fixer to move this line. +$string['yourwordforx'] = 'Your word for \'{$a}\''; +$string['zippingbackup'] = 'Zipping backup'; + +// Deprecated since Moodle 4.0. +$string['bcreateuserandpass'] = 'Choose your username and password'; +$string['cupplyinfo'] = 'More details'; + +// Deprecated since Moodle 4.3. +$string['clicktochangeinbrackets'] = '{$a} (Click to change)'; + +// Deprecated since Moodle 4.4. +$string['asocialheadline'] = 'Social forum - latest topics'; +$string['topicshow'] = 'Show this topic to {$a}'; + +$string['zzzz'] = 'zzzz'; // This comment shouldn't be here. No more auto-fixing after this line. +$string['yyyy'] = 'yyyy'; + +$string['bbbb'] = 'bbbb'; +$string['aaaa'] = 'aaaa'; diff --git a/moodle/Tests/Sniffs/Files/fixtures/langFilesOrdering/lang/en@wrong/langFilesOrdering.php b/moodle/Tests/Sniffs/Files/fixtures/langFilesOrdering/lang/en@wrong/langFilesOrdering.php new file mode 100644 index 0000000..673b4b3 --- /dev/null +++ b/moodle/Tests/Sniffs/Files/fixtures/langFilesOrdering/lang/en@wrong/langFilesOrdering.php @@ -0,0 +1,25 @@ +. + +/** + * A fixture lang-like file in incorrect place to test the LangFilesOrderingSniff. + * + * @package core + * @copyright 1999 onwards Martin Dougiamas {@link http://moodle.com} + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +$string['wedontneedmany'] = 'As far as this file is not going to be processed (wrong location), one string is enough.'; \ No newline at end of file