diff --git a/file.php b/file.php index 2c63472..ba8ddeb 100644 --- a/file.php +++ b/file.php @@ -38,6 +38,8 @@ class local_moodlecheck_file { protected $tokens = null; protected $tokenscount = 0; protected $classes = null; + protected $interfaces = null; + protected $traits = null; protected $functions = null; protected $filephpdocs = null; protected $allphpdocs = null; @@ -61,6 +63,8 @@ protected function clear_memory() { $this->tokens = null; $this->tokenscount = 0; $this->classes = null; + $this->interfaces = null; + $this->traits = null; $this->functions = null; $this->filephpdocs = null; $this->allphpdocs = null; @@ -172,22 +176,27 @@ public function &get_tokens() { } /** - * Returns all classes found in file + * Returns all artifacts (classes, interfaces, traits) found in file * - * Returns array of objects where each element represents a class: - * $class->name : name of the class - * $class->tagpair : array of two elements: id of token { for the class and id of token } (false if not found) - * $class->phpdocs : phpdocs for this class (instance of local_moodlecheck_phpdocs or false if not found) - * $class->boundaries : array with ids of first and last token for this class + * Returns 3 arrays (classes, interfaces and traits) of objects where each element represents an artifact: + * ->type : token type of the artifact (T_CLASS, T_INTERFACE, T_TRAIT) + * ->name : name of the artifact + * ->tagpair : array of two elements: id of token { for the class and id of token } (false if not found) + * ->phpdocs : phpdocs for this artifact (instance of local_moodlecheck_phpdocs or false if not found) + * ->boundaries : array with ids of first and last token for this artifact. * - * @return array + * @return array with 3 elements (classes, interfaces & traits), each being an array. */ - public function &get_classes() { + public function get_artifacts() { + $types = array(T_CLASS, T_INTERFACE, T_TRAIT); // We are interested on these. + $artifacts = array_combine($types, $types); if ($this->classes === null) { $this->classes = array(); + $this->interfaces = array(); + $this->traits = array(); $tokens = &$this->get_tokens(); for ($tid = 0; $tid < $this->tokenscount; $tid++) { - if ($this->tokens[$tid][0] == T_CLASS) { + if (isset($artifacts[$this->tokens[$tid][0]])) { if ($this->previous_nonspace_token($tid) === "::") { // Skip use of the ::class special constant. continue; @@ -211,17 +220,41 @@ public function &get_classes() { continue; } } - $class = new stdClass(); - $class->tid = $tid; - $class->name = $this->next_nonspace_token($tid); - $class->phpdocs = $this->find_preceeding_phpdoc($tid); - $class->tagpair = $this->find_tag_pair($tid, '{', '}'); - $class->boundaries = $this->find_object_boundaries($class); - $this->classes[] = $class; + $artifact = new stdClass(); + $artifact->type = $artifacts[$this->tokens[$tid][0]]; + $artifact->tid = $tid; + $artifact->name = $this->next_nonspace_token($tid); + $artifact->phpdocs = $this->find_preceeding_phpdoc($tid); + $artifact->tagpair = $this->find_tag_pair($tid, '{', '}'); + $artifact->boundaries = $this->find_object_boundaries($artifact); + switch ($artifact->type) { + case T_CLASS: + $this->classes[] = $artifact; + break; + case T_INTERFACE: + $this->interfaces[] = $artifact; + break; + case T_TRAIT: + $this->traits[] = $artifact; + break; + } } } } - return $this->classes; + return array(T_CLASS => $this->classes, T_INTERFACE => $this->interfaces, T_TRAIT => $this->traits); + } + + /** + * Returns all classes found in file + * + * Returns array of objects where each element represents a class: + * $class->name : name of the class + * $class->tagpair : array of two elements: id of token { for the class and id of token } (false if not found) + * $class->phpdocs : phpdocs for this class (instance of local_moodlecheck_phpdocs or false if not found) + * $class->boundaries : array with ids of first and last token for this class + */ + public function &get_classes() { + return $this->get_artifacts()[T_CLASS]; } /** diff --git a/rules/phpdocs_basic.php b/rules/phpdocs_basic.php index 722543f..096b8f0 100644 --- a/rules/phpdocs_basic.php +++ b/rules/phpdocs_basic.php @@ -70,6 +70,11 @@ function local_moodlecheck_noemptysecondline(local_moodlecheck_file $file) { * @return array of found errors */ function local_moodlecheck_filephpdocpresent(local_moodlecheck_file $file) { + // This rule doesn't apply if the file is 1-artifact file (see #66). + $artifacts = $file->get_artifacts(); + if (count($artifacts[T_CLASS]) + count($artifacts[T_INTERFACE]) + count($artifacts[T_TRAIT]) === 1) { + return array(); + } if ($file->find_file_phpdocs() === false) { $tokens = &$file->get_tokens(); for ($i = 0; $i < 30; $i++) { diff --git a/tests/fixtures/phpdoc_file_required_no1.php b/tests/fixtures/phpdoc_file_required_no1.php new file mode 100644 index 0000000..98f1a57 --- /dev/null +++ b/tests/fixtures/phpdoc_file_required_no1.php @@ -0,0 +1,24 @@ +. + +defined('MOODLE_INTERNAL') || die(); + +/** + * This is a dummy class without any tags. + */ +class dummy_class_without_tags { + // As far as this is an 1-artifact file, the file phpdoc block is not required. +} diff --git a/tests/fixtures/phpdoc_file_required_no2.php b/tests/fixtures/phpdoc_file_required_no2.php new file mode 100644 index 0000000..1c40b9a --- /dev/null +++ b/tests/fixtures/phpdoc_file_required_no2.php @@ -0,0 +1,24 @@ +. + +defined('MOODLE_INTERNAL') || die(); + +/** + * This is a dummy interface without any tags. + */ +interface dummy_class_without_tags { + // As far as this is an 1-artifact file, the file phpdoc block is not required. +} diff --git a/tests/fixtures/phpdoc_file_required_no3.php b/tests/fixtures/phpdoc_file_required_no3.php new file mode 100644 index 0000000..b1b6323 --- /dev/null +++ b/tests/fixtures/phpdoc_file_required_no3.php @@ -0,0 +1,24 @@ +. + +defined('MOODLE_INTERNAL') || die(); + +/** + * This is a dummy trait without any tags. + */ +trait dummy_class_without_tags { + // As far as this is an 1-artifact file, the file phpdoc block is not required. +} diff --git a/tests/fixtures/phpdoc_file_required_yes1.php b/tests/fixtures/phpdoc_file_required_yes1.php new file mode 100644 index 0000000..e4fc5d8 --- /dev/null +++ b/tests/fixtures/phpdoc_file_required_yes1.php @@ -0,0 +1,29 @@ +. + +defined('MOODLE_INTERNAL') || die(); + +/** + * This is a dummy class without any tags. + */ +class dummy_class_without_tags { +} + +/** + * This is another dummy class without any tags. Hence, file phpdoc block is required. + */ +class dummy2_class_without_tags { +} diff --git a/tests/fixtures/phpdoc_file_required_yes2.php b/tests/fixtures/phpdoc_file_required_yes2.php new file mode 100644 index 0000000..8cc73f9 --- /dev/null +++ b/tests/fixtures/phpdoc_file_required_yes2.php @@ -0,0 +1,23 @@ +. + +defined('MOODLE_INTERNAL') || die(); + +// This is library-style php file, no classes around. + +function dummy_function_without_tags() { + // No classes, hence the file phpdoc block is required. +} diff --git a/tests/fixtures/phpdoc_file_required_yes3.php b/tests/fixtures/phpdoc_file_required_yes3.php new file mode 100644 index 0000000..70a593c --- /dev/null +++ b/tests/fixtures/phpdoc_file_required_yes3.php @@ -0,0 +1,29 @@ +. + +defined('MOODLE_INTERNAL') || die(); + +/** + * This is a dummy interface without any tags. + */ +interface dummy_class_without_tags { +} + +/** + * This is a dummy trait without any tags. Hence, file phpdoc block is required. + */ +trait dummy2_class_without_tags { +} diff --git a/tests/moodlecheck_rules_test.php b/tests/moodlecheck_rules_test.php index c1820f7..8f88889 100644 --- a/tests/moodlecheck_rules_test.php +++ b/tests/moodlecheck_rules_test.php @@ -75,6 +75,45 @@ public function test_constantclass() { $this->assertStringNotContainsString('classesdocumented', $result); } + /** + * Assert that the file block is required for old files, and not for 1-artifact ones. + */ + public function test_file_block_required() { + global $PAGE; + + $output = $PAGE->get_renderer('local_moodlecheck'); + + // A file with multiple classes, require the file phpdoc block. + $path = new local_moodlecheck_path('local/moodlecheck/tests/fixtures/phpdoc_file_required_yes1.php', null); + $result = $output->display_path($path, 'xml'); + $this->assertStringContainsString('File-level phpdocs block is not found', $result); + + // A file without any class (library-like), require the file phpdoc block. + $path = new local_moodlecheck_path('local/moodlecheck/tests/fixtures/phpdoc_file_required_yes2.php', null); + $result = $output->display_path($path, 'xml'); + $this->assertStringContainsString('File-level phpdocs block is not found', $result); + + // A file with one interface and one trait, require the file phpdoc block. + $path = new local_moodlecheck_path('local/moodlecheck/tests/fixtures/phpdoc_file_required_yes3.php', null); + $result = $output->display_path($path, 'xml'); + $this->assertStringContainsString('File-level phpdocs block is not found', $result); + + // A file with only one class, do not require the file phpdoc block. + $path = new local_moodlecheck_path('local/moodlecheck/tests/fixtures/phpdoc_file_required_no1.php', null); + $result = $output->display_path($path, 'xml'); + $this->assertStringNotContainsString('File-level phpdocs block is not found', $result); + + // A file with only one interface, do not require the file phpdoc block. + $path = new local_moodlecheck_path('local/moodlecheck/tests/fixtures/phpdoc_file_required_no2.php', null); + $result = $output->display_path($path, 'xml'); + $this->assertStringNotContainsString('File-level phpdocs block is not found', $result); + + // A file with only one trait, do not require the file phpdoc block. + $path = new local_moodlecheck_path('local/moodlecheck/tests/fixtures/phpdoc_file_required_no3.php', null); + $result = $output->display_path($path, 'xml'); + $this->assertStringNotContainsString('File-level phpdocs block is not found', $result); + } + /** * Assert that classes do not need to have any particular phpdocs tags. */