Clean code

chamilo · Dec 19, 2019 · d794586 · d794586
1 parent 530e2e4
commit d794586
Show file tree

Hide file tree

Showing 25 changed files with 146 additions and 86 deletions.
diff --git a/ecs.yaml b/ecs.yaml
@@ -1,6 +1,8 @@
+services:
+    SlevomatCodingStandard\Sniffs\Variables\UnusedVariableSniff:
+        ignoreUnusedValuesWhenOnlyKeysAreUsedInForeach: 'true'
+
 parameters:
     sets:
         - 'clean-code'
         - 'symfony'
-    skip:
-        SlevomatCodingStandard\Sniffs\Variables\UnusedVariableSniff: ~
diff --git a/psalm.xml b/psalm.xml
@@ -18,6 +18,8 @@
     <issueHandlers>
         <LessSpecificReturnType errorLevel="info" />
 
+        <LessSpecificReturnType errorLevel="info" />
+
         <!-- level 3 issues - slightly lazy code writing, but provably low false-negatives -->
 
         <DeprecatedMethod errorLevel="info" />
@@ -53,5 +55,91 @@
         <RawObjectIteration errorLevel="info" />
 
         <InvalidStringClass errorLevel="info" />
+
+        <!-- level 4 issues - points to possible deficiencies in logic, higher false-positives -->
+
+        <MoreSpecificReturnType errorLevel="info" />
+        <LessSpecificReturnStatement errorLevel="info" />
+        <TypeCoercion errorLevel="info" />
+
+        <PossiblyFalseArgument errorLevel="info" />
+        <PossiblyFalseIterator errorLevel="info" />
+        <PossiblyFalseOperand errorLevel="info" />
+        <PossiblyFalsePropertyAssignmentValue errorLevel="info" />
+        <PossiblyFalseReference errorLevel="info" />
+        <PossiblyInvalidArgument errorLevel="info" />
+        <PossiblyInvalidArrayAccess errorLevel="info" />
+        <PossiblyInvalidArrayAssignment errorLevel="info" />
+        <PossiblyInvalidArrayOffset errorLevel="info" />
+        <PossiblyInvalidCast errorLevel="info" />
+        <PossiblyInvalidFunctionCall errorLevel="info" />
+        <PossiblyInvalidIterator errorLevel="info" />
+        <PossiblyInvalidMethodCall errorLevel="info" />
+        <PossiblyInvalidOperand errorLevel="info" />
+        <PossiblyInvalidPropertyAssignment errorLevel="info" />
+        <PossiblyInvalidPropertyAssignmentValue errorLevel="info" />
+        <PossiblyInvalidPropertyFetch errorLevel="info" />
+        <PossiblyNullArgument errorLevel="info" />
+        <PossiblyNullArrayAccess errorLevel="info" />
+        <PossiblyNullArrayAssignment errorLevel="info" />
+        <PossiblyNullArrayOffset errorLevel="info" />
+        <PossiblyNullFunctionCall errorLevel="info" />
+        <PossiblyNullIterator errorLevel="info" />
+        <PossiblyNullOperand errorLevel="info" />
+        <PossiblyNullPropertyAssignment errorLevel="info" />
+        <PossiblyNullPropertyAssignmentValue errorLevel="info" />
+        <PossiblyNullPropertyFetch errorLevel="info" />
+        <PossiblyNullReference errorLevel="info" />
+        <PossiblyUndefinedGlobalVariable errorLevel="info" />
+        <PossiblyUndefinedVariable errorLevel="info" />
+        <PossiblyUndefinedArrayOffset errorLevel="info" />
+        <PossiblyUndefinedMethod errorLevel="info" />
+        <PossibleRawObjectIteration errorLevel="info" />
+
+        <!-- level 5 issues - should be avoided at most costs... -->
+
+        <ForbiddenCode errorLevel="info" />
+        <ImplicitToStringCast errorLevel="info" />
+        <InvalidScalarArgument errorLevel="info" />
+        <InvalidToString errorLevel="info" />
+        <InvalidOperand errorLevel="info" />
+        <NoInterfaceProperties errorLevel="info" />
+        <TooManyArguments errorLevel="info" />
+        <TypeDoesNotContainType errorLevel="info" />
+        <TypeDoesNotContainNull errorLevel="info" />
+        <MissingDocblockType errorLevel="info" />
+        <ImplementedReturnTypeMismatch errorLevel="info" />
+        <ImplementedParamTypeMismatch errorLevel="info" />
+
+        <!-- level 6 issues - really bad things -->
+
+        <InvalidNullableReturnType errorLevel="info" />
+        <NullableReturnStatement errorLevel="info" />
+        <InvalidFalsableReturnType errorLevel="info" />
+        <FalsableReturnStatement errorLevel="info" />
+
+        <MoreSpecificImplementedParamType errorLevel="info" />
+        <LessSpecificImplementedReturnType errorLevel="info" />
+
+        <InvalidReturnStatement errorLevel="info" />
+        <InvalidReturnType errorLevel="info" />
+        <UndefinedThisPropertyAssignment errorLevel="info" />
+        <UndefinedInterfaceMethod errorLevel="info" />
+
+        <!-- level 7 issues - even worse -->
+
+        <UndefinedThisPropertyAssignment errorLevel="info" />
+        <UndefinedPropertyAssignment errorLevel="info" />
+        <UndefinedThisPropertyFetch errorLevel="info" />
+        <UndefinedPropertyFetch errorLevel="info" />
+
+        <InvalidReturnStatement errorLevel="info" />
+        <InvalidReturnType errorLevel="info" />
+        <InvalidArgument errorLevel="info" />
+        <InvalidPropertyAssignmentValue errorLevel="info" />
+        <InvalidArrayOffset errorLevel="info" />
+        <InvalidArrayAssignment errorLevel="info" />
+        <InvalidArrayAccess errorLevel="info" />
+        <InvalidClone errorLevel="info" />
     </issueHandlers>
 </psalm>
diff --git a/src/Command/Common/CommonCommand.php b/src/Command/Common/CommonCommand.php
@@ -994,8 +994,6 @@ public function getPackage(OutputInterface $output, $version, $updateInstallatio
             }
         }
 
-        $updateInstallationOriginal = $updateInstallation;
-
         if (!empty($updateInstallation)) {
             // Check temp folder
             if (!is_writable($defaultTempFolder)) {

diff --git a/src/Command/Database/FullBackupCommand.php b/src/Command/Database/FullBackupCommand.php
@@ -85,21 +85,21 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         $f = $_configuration['db_user'];
         //backup the files (this requires root permissions)
         $bkp_dir = $tmpFolder.'/'.$f.'-'.date('Ymdhis');
-        $err = @mkdir($bkp_dir);
+        @mkdir($bkp_dir);
         $tgz = $bkp_dir.'/'.$f.'.tgz';
         $sql = $bkp_dir.'/'.$f.'-db.sql';
-        $err = @system('tar zcf '.$tgz.' '.$cha_dir);
+        @system('tar zcf '.$tgz.' '.$cha_dir);
 
         $output->writeln('<comment>Generating mysqldump</comment>');
 
-        $err = @system(
+        @system(
             'mysqldump -h '.$_configuration['db_host'].' -u '.$_configuration['db_user'].' -p'.$_configuration['db_password'].' '.$_configuration['main_database'].' --result-file='.$sql
         );
 
         $output->writeln('<comment>Generating tarball </comment>');
 
-        $err = @system('tar zcf '.$resultPath.' '.$bkp_dir);
-        $err = @system('rm -rf '.$bkp_dir);
+        @system('tar zcf '.$resultPath.' '.$bkp_dir);
+        @system('rm -rf '.$bkp_dir);
 
         $output->writeln(
             '<comment>End Chamilo backup. File can be found here: '.realpath($resultPath).' </comment>'

diff --git a/src/Command/Email/SendEmailCommand.php b/src/Command/Email/SendEmailCommand.php
@@ -67,8 +67,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int
     {
         parent::execute($input, $output);
         $mailLib = $this->getHelper('configuration')->getLibFile('mail.lib.inc.php');
-        $mainApiLib = $this->getHelper('configuration')->getLibFile('main_api.lib.php');
-        $cnfFiles = $this->getHelper('configuration')->getConfFile('mail.conf.php');
         $conn = $this->getConnection($input);
 
         if (empty($mailLib)) {

diff --git a/src/Command/Files/CleanDeletedDocumentsCommand.php b/src/Command/Files/CleanDeletedDocumentsCommand.php
@@ -99,7 +99,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             if ($deleteFromDb) {
                 $connection = $this->getConnection($input);
                 $sql = "DELETE FROM c_document WHERE path LIKE '%_DELETED%'";
-                $stmt = $connection->query($sql);
+                $connection->query($sql);
                 /*
                 while ($row = $stmt->fetch()) {
                     $sql2 = "SELECT id FROM c_item_property

diff --git a/src/Command/Files/ConvertVideosCommand.php b/src/Command/Files/ConvertVideosCommand.php
@@ -162,7 +162,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
                 $origNameCommand = preg_replace('/\(/', '\(', $origNameCommand);
                 $origNameCommand = preg_replace('/\)/', '\)', $origNameCommand);
                 $output->writeln('ffmpeg -i '.$newNameCommand.' -b '.$bitRate.'k -f '.$this->ext.' -vcodec '.$vcodec.' -acodec copy -r '.$fps.' '.$origNameCommand);
-                $exec = @system('ffmpeg -i '.$newNameCommand.' -b '.$bitRate.'k -f '.$this->ext.' -vcodec '.$vcodec.' -acodec copy -r '.$fps.' '.$origNameCommand, $out);
+                @system('ffmpeg -i '.$newNameCommand.' -b '.$bitRate.'k -f '.$this->ext.' -vcodec '.$vcodec.' -acodec copy -r '.$fps.' '.$origNameCommand, $out);
                 $sizeNew += filesize($origName);
                 ++$counter;
             }

diff --git a/src/Command/Files/DeleteCoursesCommand.php b/src/Command/Files/DeleteCoursesCommand.php
@@ -76,7 +76,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             $courseCode = $input->getOption('course-code'); //1 if the option was set
             $courseCategory = $input->getOption('course-category'); //1 if the option was set
             $beforeDate = $input->getOption('before-date'); //1 if the option was set
-            $confirmDelete = $input->getOption('delete'); //1 if the option was set
+            //$confirmDelete = $input->getOption('delete'); //1 if the option was set
             $du = $input->getOption('show-disk-usage'); //1 if the option was set
             $connection = $this->getConnection($input);
 
@@ -190,11 +190,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int
                     foreach ($coursesList as $id) {
                         $countUrl = count($coursesUrl[$id]);
                         $unique = ($countUrl <= 1 ? 'yes' : 'no');
-                        $diskUsage = '';
+                        //$diskUsage = '';
                         if ($du) {
                             $courseDir = $coursesPath.$coursesDir[$id];
                             if (!is_dir($courseDir)) {
-                                $size = 'N/A';
+                                //$size = 'N/A';
                             } else {
                                 $res = @exec('du -s '.$courseDir);
                                 $res = preg_split('/\s/', $res);
@@ -306,19 +306,19 @@ private function unlinkCourse(InputInterface $input, OutputInterface $output, st
                     // No course within this session => delete the session
                     // @todo: use sessionmanager::delete_session($sessionId)
                     $sqlDelete = "DELETE FROM session WHERE id = $sessionId";
-                    $stmtDelete = $connection->query($sqlDelete);
+                    $connection->query($sqlDelete);
                     $sqlDelete = "DELETE FROM session_rel_course_rel_user WHERE id_session = $sessionId";
-                    $stmtDelete = $connection->query($sqlDelete);
+                    $connection->query($sqlDelete);
                     $sqlDelete = "DELETE FROM session_rel_user WHERE id_session = $sessionId";
-                    $stmtDelete = $connection->query($sqlDelete);
+                    $connection->query($sqlDelete);
                     $sqlDelete = "DELETE FROM session_rel_course WHERE id_session = $sessionId";
-                    $stmtDelete = $connection->query($sqlDelete);
+                    $connection->query($sqlDelete);
                     $sqlDelete = "DELETE FROM access_url_rel_session WHERE session_id = $sessionId";
-                    $stmtDelete = $connection->query($sqlDelete);
+                    $connection->query($sqlDelete);
                     $sqlDelete = "DELETE FROM session_field_values WHERE session_id = $sessionId";
-                    $stmtDelete = $connection->query($sqlDelete);
+                    $connection->query($sqlDelete);
                     $sqlDelete = "DELETE FROM session WHERE id = $sessionId";
-                    $stmtDelete = $connection->query($sqlDelete);
+                    $connection->query($sqlDelete);
                 }
             }
         }

diff --git a/src/Command/Files/DeleteMultiUrlCommand.php b/src/Command/Files/DeleteMultiUrlCommand.php
@@ -473,41 +473,39 @@ private function deleteUser(InputInterface $input, OutputInterface $output, int
                 WHERE cu.user_id = $userId AND c.code = cu.course_code";
         $stmt = $connection->query($sql);
 
-        while ($course = $stmt->fetch()) {
-            $sql = "DELETE FROM c_group_rel_user AND user_id = $userId";
-            $stmt2 = $connection->query($sql);
-        }
+        $sql = "DELETE FROM c_group_rel_user AND user_id = $userId";
+        $connection->query($sql);
 
         // Unsubscribe user from usergroup_rel_user
         $sql = "DELETE FROM usergroup_rel_user WHERE user_id = $userId";
-        $stmt = $connection->query($sql);
+        $connection->query($sql);
 
         // Unsubscribe user from all courses
         $sql = "DELETE FROM course_rel_user WHERE user_id = $userId";
-        $stmt = $connection->query($sql);
+        $connection->query($sql);
 
         // Unsubscribe user from all courses in sessions
         $sql = "DELETE FROM session_rel_course_rel_user WHERE id_user = $userId";
-        $stmt = $connection->query($sql);
+        $connection->query($sql);
 
         // If the user was added as a id_coach then set the current admin as coach see BT#
         $sql = "UPDATE session SET id_coach = 1 WHERE id_coach = $userId";
-        $stmt = $connection->query($sql);
+        $connection->query($sql);
 
         $sql = "UPDATE session SET id_coach = 1 WHERE session_admin_id = $userId";
-        $stmt = $connection->query($sql);
+        $connection->query($sql);
 
         // Unsubscribe user from all sessions
         $sql = "DELETE FROM session_rel_user WHERE id_user = $userId";
-        $stmt = $connection->query($sql);
+        $connection->query($sql);
 
         // Delete user picture
         /* TODO: Logic about api_get_setting('split_users_upload_directory') == 'true'
         a user has 4 different sized photos to be deleted. */
         $sysPath = $this->getConfigurationHelper()->getSysPath();
         $sub = '';
         $sql = "SELECT selected_value FROM settings_current WHERE variable = 'split_users_upload_directory'";
-        $stmt = $connection->query($sql);
+        $connection->query($sql);
         $row = $stmt->fetch();
         if ('true' == $row['selected_value']) {
             $sub = substr($userId, 0, 1).'/';
@@ -519,38 +517,38 @@ private function deleteUser(InputInterface $input, OutputInterface $output, int
 
         // Delete the personal course categories
         $sql = "DELETE FROM user_course_category WHERE user_id = $userId";
-        $stmt = $connection->query($sql);
+        $connection->query($sql);
 
         // Delete user from database
         $sql = "DELETE FROM user WHERE user_id = $userId";
-        $stmt = $connection->query($sql);
+        $connection->query($sql);
 
         // Delete user from the admin table
         $sql = "DELETE FROM admin WHERE user_id = $userId";
-        $stmt = $connection->query($sql);
+        $connection->query($sql);
 
         // Delete the personal agenda-items from this user
         $sql = "DELETE FROM personal_agenda WHERE user = $userId";
-        $stmt = $connection->query($sql);
+        $connection->query($sql);
 
         $sql = "DELETE FROM gradebook_result WHERE user_id = $userId";
-        $stmt = $connection->query($sql);
+        $connection->query($sql);
 
         $sql = "DELETE FROM user_field_values WHERE user_id = $userId";
-        $stmt = $connection->query($sql);
+        $connection->query($sql);
 
         $sql = "DELETE FROM group_rel_user WHERE user_id = $userId";
-        $stmt = $connection->query($sql);
+        $connection->query($sql);
 
         $sql = "DELETE FROM user_rel_user WHERE user_id = $userId";
-        $stmt = $connection->query($sql);
+        $connection->query($sql);
 
         // Removing survey invitation
         //survey_manager::delete_all_survey_invitations_by_user($user_id);
 
         // Delete students works
         $sql = "DELETE FROM c_student_publication WHERE user_id = $userId AND c_id <> 0";
-        $stmt = $connection->query($sql);
+        $connection->query($sql);
 
         // Add event to system log
         //$user_id_manager = api_get_user_id();

diff --git a/src/Command/Files/UpdateDirectoryMaxSizeCommand.php b/src/Command/Files/UpdateDirectoryMaxSizeCommand.php
@@ -61,8 +61,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             $this->writeCommandHeader($output, 'Checking courses dir...');
 
             // Get database and path information
-            $coursesPath = $this->getConfigurationHelper()->getSysPath();
-            $connection = $this->getConnection($input);
+            //$coursesPath = $this->getConfigurationHelper()->getSysPath();
+            //$connection = $this->getConnection($input);
             $_configuration = $this->getConfigurationHelper()->getConfiguration();
 
             $courseTable = $_configuration['main_database'].'.course';
@@ -96,7 +96,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
                     $res = preg_split('/\s/', $res);
                     $size = round($res[0] / 1024, 1); // $size is stored in MB
                     if (isset($globalCourses[$file]['code'])) {
-                        $code = $globalCourses[$file]['code'];
+                        //$code = $globalCourses[$file]['code'];
                         $quota = round(
                             $globalCourses[$file]['quota'] / (1024 * 1024),
                             0

diff --git a/src/Command/Info/WhichCommand.php b/src/Command/Info/WhichCommand.php
@@ -104,7 +104,6 @@ protected function configure(): void
     protected function execute(InputInterface $input, OutputInterface $output): int
     {
         parent::execute($input, $output);
-        $_configuration = $this->getHelper('configuration')->getConfiguration();
         $version = $this->getHelper('configuration')->getMajorVersion();
         $tool = $input->getArgument('tool');
         $found = false;

diff --git a/src/Command/Installation/InstallCommand.php b/src/Command/Installation/InstallCommand.php
@@ -315,8 +315,8 @@ public function install(InputInterface $input, OutputInterface $output)
                 return 0;
             }
 
-            $eventManager = $connectionToHost->getSchemaManager();
-            $databases = $eventManager->listDatabases();
+            //$eventManager = $connectionToHost->getSchemaManager();
+            //$databases = $eventManager->listDatabases();
             /*if (in_array($databaseSettings['dbname'], $databases)) {
                 if (false == $this->silent) {
                     $helper = $this->getHelperSet()->get('question');

diff --git a/src/Command/Installation/UpgradeDatabaseCommand.php b/src/Command/Installation/UpgradeDatabaseCommand.php
@@ -637,7 +637,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
 
         $output->writeln("<comment>Current version: </comment><info>$currentVersion</info>");
         $output->writeln("<comment>Latest version: </comment><info>$version</info>");
-        $oldVersion = $version;
+        //$oldVersion = $version;
 
         // Handle 1.10.x as 1.10.1000
         if ('1.9.x' == $currentVersion) {

diff --git a/src/Command/Translation/AddSubLanguageCommand.php b/src/Command/Translation/AddSubLanguageCommand.php
@@ -111,7 +111,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             //require_once $_configuration['root_sys'].'main/inc/lib/main_api.lib.php';
             //$perm = api_get_permissions_for_new_directories();
             // @todo Improve permissions to force creating as user www-data
-            $r = @mkdir($_configuration['root_sys'].'main/lang/'.$lang, 0777);
+            @mkdir($_configuration['root_sys'].'main/lang/'.$lang, 0777);
             $output->writeln('Sub-language '.$lang.' of language '.$parent.' has been created but is disabled. Fill it, then enable to make available to users. Make sure you check the permissions for the newly created directory as well ('.$_configuration['root_sys'].'main/lang/'.$lang.')');
         } else {
             $output->writeln('The connection does not seem to be a valid PDO connection');

diff --git a/src/Command/Translation/DisableLanguageCommand.php b/src/Command/Translation/DisableLanguageCommand.php
@@ -32,7 +32,6 @@ protected function configure(): void
     protected function execute(InputInterface $input, OutputInterface $output): int
     {
         parent::execute($input, $output);
-        $_configuration = $this->getHelper('configuration')->getConfiguration();
         $conn = $this->getConnection($input);
         $lang = $input->getArgument('language');
         if ($conn instanceof \Doctrine\DBAL\Connection) {
@@ -63,7 +62,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             $us = 'UPDATE language SET available = 0 WHERE id = '.$lr['id'];
 
             try {
-                $uq = $stmt2 = $conn->prepare($us);
+                $stmt2 = $conn->prepare($us);
                 $stmt2->execute();
             } catch (\PDOException $e) {
                 $output->write('SQL error!'.PHP_EOL);