addressed GHA codechecker issues anthology-ally#1

ucl-isd · Jun 23, 2023 · f845c7a · f845c7a
1 parent 5a1a227
commit f845c7a
Show file tree

Hide file tree

Showing 55 changed files with 1,185 additions and 47 deletions.
diff --git a/classes/adminsetting/ally_trim.php b/classes/adminsetting/ally_trim.php
@@ -24,6 +24,8 @@
 
 namespace tool_ally\adminsetting;
 
+defined('MOODLE_INTERNAL') || die();
+
 require_once($CFG->dirroot . '/lib/adminlib.php');
 
 /**
@@ -34,7 +36,7 @@
 class ally_trim extends \admin_setting_configtext {
 
     public function write_setting($data) {
-        if ($this->paramtype === PARAM_INT and $data === '') {
+        if ($this->paramtype === PARAM_INT && $data === '') {
             // Do not complain if '' used instead of 0.
             $data = 0;
         }

diff --git a/classes/file_url_resolver.php b/classes/file_url_resolver.php
@@ -24,6 +24,8 @@
 
 namespace tool_ally;
 
+use stored_file;
+
 /**
  * Resolve the URL of a file to see it in context.
  *
@@ -49,10 +51,10 @@ public function __construct(\moodle_database $db = null) {
     /**
      * Given a file, find a URL to view it in Moodle UI.
      *
-     * @param \stored_file $file
+     * @param stored_file $file
      * @return \moodle_url|null
      */
-    public function resolve_url(\stored_file $file) {
+    public function resolve_url(stored_file $file) {
         $url = null;
         switch ($file->get_component()) {
             case 'forum':
@@ -76,10 +78,10 @@ public function resolve_url(\stored_file $file) {
     /**
      * Most generic way to get a file URL.
      *
-     * @param \stored_file $file
+     * @param stored_file $file
      * @return \moodle_url
      */
-    private function default_resolver(\stored_file $file) {
+    private function default_resolver(stored_file $file) {
         $context = \context::instance_by_id($file->get_contextid());
         return $context->get_url();
     }
@@ -89,10 +91,10 @@ private function default_resolver(\stored_file $file) {
      *
      * This also works for mod_hsuforum.
      *
-     * @param \stored_file $file
+     * @param stored_file $file
      * @return \moodle_url|null
      */
-    private function mod_forum_resolver(\stored_file $file) {
+    private function mod_forum_resolver(stored_file $file) {
         if (!in_array($file->get_filearea(), ['attachment', 'post'])) {
             return null;
         }
@@ -110,10 +112,10 @@ private function mod_forum_resolver(\stored_file $file) {
     /**
      * Resolve URL for questions.
      *
-     * @param \stored_file $file
+     * @param stored_file $file
      * @return \moodle_url|null
      */
-    private function question_resolver(\stored_file $file) {
+    private function question_resolver(stored_file $file) {
         $params  = [];
         $context = \context::instance_by_id($file->get_contextid());
         if ($context instanceof \context_course) {

diff --git a/classes/version_information.php b/classes/version_information.php
@@ -81,11 +81,11 @@ private function warn_on_site_policy_not_accepted() {
         global $CFG, $USER;
         $manager = new \core_privacy\local\sitepolicy\manager();
         // Check that the user has agreed to a site policy if there is one - do not test in case of admins.
-        if (empty($USER->policyagreed) and !is_siteadmin()) {
-            if ($manager->is_defined() and !isguestuser()) {
+        if (empty($USER->policyagreed) && !is_siteadmin()) {
+            if ($manager->is_defined() && !isguestuser()) {
                 $url = $manager->get_embed_url();
                 throw new moodle_exception('sitepolicynotagreed', 'error', '', $url->get_path());
-            } else if ($manager->is_defined(true) and isguestuser()) {
+            } else if ($manager->is_defined(true) && isguestuser()) {
                 $guesturl = $manager->get_embed_url(true);
                 throw new moodle_exception('sitepolicynotagreed', 'error', '', $guesturl->get_path());
             }

diff --git a/lti/view.php b/lti/view.php
@@ -30,7 +30,7 @@
 require_once($CFG->dirroot . '/mod/lti/locallib.php');
 
 $undertest = defined('BEHAT_SITE_RUNNING') || PHPUNIT_TEST;
-if (!$undertest and is_callable('mr_off') and mr_off('report_allylti', '_MR_MISC')) {
+if (!$undertest && is_callable('mr_off') && mr_off('report_allylti', '_MR_MISC')) {
     throw new moodle_exception('generalexceptionmessage', 'error', '', get_string('notenabled', 'report_allylti'));
 }
 

diff --git a/tests/abstract_testcase.php b/tests/abstract_testcase.php
@@ -27,6 +27,13 @@
 
 global $CFG;
 
+use coding_exception;
+use context;
+use file_exception;
+use moodle_exception;
+use stdClass;
+use stored_file;
+use stored_file_creation_exception;
 use tool_ally\local;
 use tool_ally\local_content;
 use tool_ally\models\component;

diff --git a/tests/adminsetting_ally_configpasswordunmask_test.php b/tests/adminsetting_ally_configpasswordunmask_test.php
@@ -27,6 +27,9 @@
 class adminsetting_ally_configpasswordunmask_test extends \advanced_testcase {
     /**
      * Test ally configpasswordunmask settings are trimmed.
+     *
+     * @return void
+     * @throws \dml_exception
      */
     public function test_configpasswordunmask() {
         $this->resetAfterTest();

diff --git a/tests/adminsetting_ally_trim_test.php b/tests/adminsetting_ally_trim_test.php
@@ -27,6 +27,9 @@
 class adminsetting_ally_trim_test extends \advanced_testcase {
     /**
      * Test settings are trimmed.
+     *
+     * @return void
+     * @throws \dml_exception
      */
     public function test_trim() {
         $this->resetAfterTest();

diff --git a/tests/auto_config_resolver_test.php b/tests/auto_config_resolver_test.php
@@ -38,6 +38,12 @@ public function setUp(): void {
         $this->resetAfterTest(true);
     }
 
+    /**
+     * test resolve clioption
+     *
+     * @return void
+     * @throws \coding_exception
+     */
     public function test_resolve_clioption() {
         $configs = [
             'secret'   => 'password!',
@@ -50,6 +56,12 @@ public function test_resolve_clioption() {
         $this->assertSame($configs, $resolver->resolve());
     }
 
+    /**
+     * test resolve envvar
+     *
+     * @return void
+     * @throws \coding_exception
+     */
     public function test_resolve_envvar() {
         $configs = [
             'secret'   => 'password!',
@@ -66,8 +78,13 @@ public function test_resolve_envvar() {
     }
 
     /**
+     * test resolve noconfigs
+     *
      * You provide configs by using the 'configs' CLI option or by setting them to MOODLE_TOOL_ALLY_AUTO_CONFIGS
      * environment variable
+     *
+     * @return void
+     * @throws \coding_exception
      */
     public function test_resolve_noconfigs() {
         $this->expectException(\coding_exception::class);
@@ -79,6 +96,12 @@ public function test_resolve_noconfigs() {
         $resolver->resolve();
     }
 
+    /**
+     * test resolve badconfigs
+     *
+     * @return void
+     * @throws \coding_exception
+     */
     public function test_resolve_badconfigs() {
         $this->expectException(\coding_exception::class);
         $this->expectExceptionMessage('Config string was not valid');

diff --git a/tests/auto_config_test.php b/tests/auto_config_test.php
@@ -26,7 +26,11 @@
 
 class auto_config_test extends \advanced_testcase {
     /**
+     * test auto config
+     *
      * @runInSeparateProcess
+     * @return void
+     * @throws \dml_exception
      */
     public function test_auto_config() {
         global $DB;
@@ -46,7 +50,11 @@ public function test_auto_config() {
     }
 
     /**
+     * test auto config update user
+     *
      * @runInSeparateProcess
+     * @return void
+     * @throws \dml_exception
      */
     public function test_auto_config_update_user() {
         global $DB;

diff --git a/tests/auto_configurator_test.php b/tests/auto_configurator_test.php
@@ -45,6 +45,8 @@ public function setUp(): void {
 
     /**
      * @runInSeparateProcess
+     * @return void
+     * @throws \dml_exception
      */
     public function test_configure_settings() {
         $configs = [
@@ -69,6 +71,9 @@ public function test_configure_settings() {
 
     /**
      * @runInSeparateProcess
+     *
+     * @return void
+     * @throws \dml_exception
      */
     public function test_configure_settings_invalid_setting() {
         $configs = [
@@ -97,6 +102,9 @@ public function test_configure_settings_invalid_setting() {
 
     /**
      * @runInSeparateProcess
+     *
+     * @return void
+     * @throws \Exception
      */
     public function test_configure_webservices() {
         $wsconfig = new auto_config();

diff --git a/tests/cleanup_log_task_test.php b/tests/cleanup_log_task_test.php
@@ -41,6 +41,9 @@
 class cleanup_log_task_test extends abstract_testcase {
     /**
      * Test the general behavior of the task execution.
+     *
+     * @return void
+     * @throws \dml_exception
      */
     public function test_execute() {
         global $DB;

diff --git a/tests/components_assign_component_test.php b/tests/components_assign_component_test.php
@@ -85,6 +85,8 @@ public function setUp(): void {
 
     /**
      * Test if file in use detection is working with this module.
+     *
+     * @return void
      */
     public function test_check_file_in_use() {
         $context = \context_module::instance($this->assign->cmid);

diff --git a/tests/components_block_html_component_test.php b/tests/components_block_html_component_test.php
@@ -80,6 +80,15 @@ public function setUp(): void {
         $this->component = local_content::component_instance('block_html');
     }
 
+    /**
+     * add block
+     *
+     * @param array|null $data
+     * @return \block_calendar_upcoming
+     * @throws \file_exception
+     * @throws \moodle_exception
+     * @throws \stored_file_creation_exception
+     */
     private function add_block( array $data = null) : \block_calendar_upcoming {
         global $USER;
 
@@ -116,7 +125,11 @@ private function add_block( array $data = null) : \block_calendar_upcoming {
     }
 
     /**
+     * test list content
+     *
      * @runInSeparateProcess
+     * @return void
+     * @throws \Exception
      */
     public function test_list_content() {
         $this->setAdminUser();
@@ -138,6 +151,12 @@ public function test_get_all_html_content_items() {
             $block->context->instanceid, 'block_html', 'block_instances', 'configdata');
     }
 
+    /**
+     * test get all html content
+     *
+     * @return void
+     * @throws \moodle_exception
+     */
     public function test_get_all_html_content() {
         $sctc = new search_content_test();
 
@@ -180,6 +199,12 @@ public function test_get_all_html_content() {
         $this->assertEquals($expectedformat, $content->contentformat);
     }
 
+    /**
+     * test get course html content items
+     *
+     * @return void
+     * @throws \moodle_exception
+     */
     public function test_get_course_html_content_items() {
         $sctc = new search_content_test();
 
@@ -222,6 +247,9 @@ public function test_get_course_html_content_items() {
 
     /**
      * Test if file in use detection is working with this module.
+     *
+     * @return void
+     * @throws \moodle_exception
      */
     public function test_file_in_use() {
         global $USER;