Check package tags

[andrewnicols]

This is a first attempt at migrating one of the checks in moodlecheck as part of #30

This change starts the work required to migrate the @package check.

There's a lot of duplicated code here right now, and a lot of missing tests, but I've tried to abstract out useful features or use phpcsextra where possible.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.06%. Comparing base (61a380a) to head (dbbffda).

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #110      +/-   ##
============================================
+ Coverage     96.73%   97.06%   +0.32%     
- Complexity      586      632      +46     
============================================
  Files            26       28       +2     
  Lines          1625     1807     +182     
============================================
+ Hits           1572     1754     +182     
  Misses           53       53              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[stronk7]

I won't merge this, coz my #111 will conflict then (I imagine, coz both are moving MoodleUtilTest)!

(joking, of course)

[stronk7]

Thanks @andrewnicols , this looks perfect for me.

I just would ask for a couple of details:

• Confirmation: Does this cover all the related tests and fixtures that we have in local_moodlecheck. Or, with other words, is this a complete replacement?
• Maybe, getting used to feed the CHANGELOG as part of the PR is good thing.

And, final reflexion... I think I commented elsewhere... let's give to this initiative a little bit of visibility (chat, forum, integration exposed, ...). Maybe there is something in the plan that we haven't detected, or somebody has a say about it...

Ciao :-)

PS: Feel free to self-merge whenever you read and consider the points above.

[andrewnicols]

Confirmation: Does this cover all the related tests and fixtures that we have in local_moodlecheck. Or, with other words, is this a complete replacement?

From what I can see in moodlecheck, there is very little testing of the package tests. See https://github.com/moodlehq/moodle-local_moodlecheck/blob/main/tests/moodlecheck_rules_test.php - there are 7 mentions of the packagevalid string, bug they're just checking that an error was raised in unrelated tests. There are no specific tests, nor any tests which check that it's in the right place.

As far as I'm aware, this is a complete replacement. There are two rules for packages:

• packagespecified: https://github.com/moodlehq/moodle-local_moodlecheck/blob/main/rules/phpdocs_package.php#L31-L67
• packagevalid: https://github.com/moodlehq/moodle-local_moodlecheck/blob/main/rules/phpdocs_package.php#L70-L87

The specified check notes:

Checks if all functions (outside class) and classes have package package tag may be inherited from file-level phpdocs

The new checks do the same thing.

The valid check notes:

Checks that wherever the package token is specified it is valid

It does this by checking the package against a list returned from local_moodlecheck_package_names. That list basically comes from \core_component, as does the new test.

Maybe, getting used to feed the CHANGELOG as part of the PR is good thing.

Bah - yes. I must get in this habit (done).

And, final reflexion... I think I commented elsewhere... let's give to this initiative a little bit of visibility (chat, forum, integration exposed, ...). Maybe there is something in the plan that we haven't detected, or somebody has a say about it...

Yes - you suggested this in PM. I'd say let's do:

• announcement in chat
• include in exposed post
• possibly a forum post

[stronk7]

Cool all!

[andrewnicols]

Announced here: https://moodle.org/mod/forum/discuss.php?d=455786