Page MenuHomePhabricator
Create Task
Maniphest T233788

Doxygen output should strip @suppress annotations
Closed, ResolvedPublic
Actions

Description

Spotted by chance as part of T233787. If a method docblock has a @suppress annotation, doxygen will merge it to the previous annotation.

An example https://gerrit.wikimedia.org/g/mediawiki/core/+/5e9c40b66d41cece8f47fdef7a1905b908f86c9a/includes/MagicWordArray.php#134

includes/MagicWordArray.php
	/**
	 * Get an unanchored regex that does not match parameters
	 * @return string[]
	 * @suppress PhanTypeArraySuspiciousNullable False positive
	 */
	public function getRegex() {

Results in Doxygen generating:

Get an unanchored regex that does not match parameters.

Returns

string[] PhanTypeArraySuspiciousNullable False positive

The @suppress is stripped and its content is appended to the content for the previous annotation (@return).

Confirmed to happen with Doxygen 1.8.16 and 1.8.13

Details

SubjectRepoBranchLines +/-
docs: Improve Doxygen aliases and map more tool annotations to @noopmediawiki/coremaster+11 -9
doc: Let Doxygen strip content of @suppress or @phan annotationsmediawiki/coremaster+5 -5
Customize query in gerrit

Related Objects
Search...

Event Timeline

Daimona created this task.Sep 25 2019, 9:38 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 25 2019, 9:38 AM
Aklapper added a comment.Nov 27 2019, 10:21 PM

@Daimona: For a clueless person like me, could you elaborate what exactly you expect [not] to see where? This is what I see on https://doc.wikimedia.org/mediawiki-core/master/php/classTitle.html#a02b13717e7d4368e227d1c1975b9b883 :

Screenshot from 2019-11-27 23-17-30.png (949×948 px, 137 KB)

Daimona added a comment.Nov 28 2019, 2:54 PM

@Aklapper Uh-oh, the example in the task description is outdated, that's why everything is looking normal :)

Here is another example:

getregex.png (311×444 px, 15 KB)

Note the suppression text next to string[].

Aklapper added a project: WMF-General-or-Unknown.Nov 29 2019, 9:05 AM

Ah, thanks! :)

Aklapper edited projects, added Doxygen; removed WMF-General-or-Unknown.Nov 30 2019, 12:38 AM
hashar updated the task description. (Show Details)Dec 16 2019, 1:04 PM
hashar triaged this task as Medium priority.Dec 16 2019, 1:11 PM
hashar added projects: MediaWiki-General, Upstream.
hashar subscribed.

We have upgraded to Doxygen 1.8.16 and it still happens. I have edited the task description with the latest example.

Same happen with @method which I believe is for Phan.

Timo commented on Upstream change https://github.com/doxygen/doxygen/issues/1413 which has a patch in Doxygen master branch https://github.com/doxygen/doxygen/pull/7264 . That introduces a \noop command to strip all content.

hashar moved this task from Backlog to Patch merged upstream on the Upstream board.Dec 16 2019, 1:12 PM
hashar added a comment.Dec 16 2019, 1:37 PM

TLDR: I thought about using \internal as an alias, but it suppress everything until the end of the code block...

From Doxygen documentation http://www.doxygen.nl/manual/commands.html#cmdinternal :

\internal

This command starts a documentation fragment that is meant for internal use only. The fragment naturally ends at the end of the comment block. You can also force the internal section to end earlier by using the \endinternal command.

If the \internal command is put inside a section (see for example \section) all subsections after the command are considered to be internal as well. Only a new section at the same level will end the fragment that is considered internal.

You can use INTERNAL_DOCS in the configuration file to show (YES) or hide (NO) the internal documentation.

See also

section `\endinternal`.

Our Doxyfile has INTERNAL_DOCS=NO, so we should be able to alias @method or @suppress to \internal and it should not show up.

Which can be configured using:

maintenance/Doxyfile
ALIASES =
                         "codeCoverageIgnore=\internal" \
                         "codingStandardsIgnoreEnd=\internal" \
                         "codingStandardsIgnoreStart=\internal" \
                         "phan=\internal" \
                         "suppress=\internal"

Which does suppress the output. BUT \internal ends at the end of the code block, not at the end of the line. Which mean that anything after @suppress or @method disappears :-\

I thought about using wrapping the argument, but there can be any arbitrary number of arguments. Eg:

ALIASES=suppress[1]=\internal\1\internal

Would work for @suppress PhanTypeInvalidDimOffset but would not work for @suppress SecurityCheck-XSS Escaping provided by $this->outputHandler, since it would only shallow the first argument....

Daimona mentioned this in T241182: Doxygen: Ignore `@phan` notation (and more).Dec 20 2019, 12:33 PM
Krinkle mentioned this in T95116: Fix Doxygen "Warning: Found unknown command".Jan 7 2020, 8:19 PM
Krinkle subscribed.
In T233788#5744301, @hashar wrote:

Upstream change: https://github.com/doxygen/doxygen/issues/1413 which has a patch in Doxygen master branch https://github.com/doxygen/doxygen/pull/7264 . That introduces a \noop command to strip all content.

Doxygen 1.8.17 has been released which includes the new @noop tag. Next step: Upgrade Doxygen in WMF CI.

Krinkle mentioned this in T242155: Update Doxygen in CI to 1.8.17 or greater.Jan 7 2020, 8:22 PM
Krinkle added a subtask: T242155: Update Doxygen in CI to 1.8.17 or greater.
gerritbot added a comment.Feb 12 2020, 4:58 PM

Change 571559 had a related patch set uploaded (by Anomie; owner: Anomie):
[mediawiki/core@master] Improve Doxygen aliases

https://gerrit.wikimedia.org/r/571559

gerritbot added a project: Patch-For-Review.Feb 12 2020, 4:58 PM
hashar closed subtask T242155: Update Doxygen in CI to 1.8.17 or greater as Resolved.May 25 2020, 2:09 PM
hashar added a subtask: T253793: Update Doxygen to 1.8.18.Oct 19 2020, 9:20 AM
hashar added a comment.Oct 19 2020, 9:23 AM

We had Doxygen 1.8.18 upgraded for most repositories. MediaWiki extensions were still using Doxygen 1.8.16 because later versions crashed while processing Wikibase (T254465).

I have updated all jobs to use Doxygen 1.8.19. So tentatively this issue should be solved now.

hashar added a comment.Oct 19 2020, 9:26 AM

Sorry my bad. This task is blocked on aliasing @suppress to the newish \noop Doxygen command.

The related MediaWiki core change is https://gerrit.wikimedia.org/r/c/mediawiki/core/+/571559/2/maintenance/Doxyfile though it introduces a few other things as well.

hashar added a comment.Mar 11 2021, 3:59 PM

This task apparently depended on an upgrade of Doxygen. CI now runs Doxygen 1.8.19.

Looks like we just need MediaWiki core DoxyFile to alias \suppress to \noop.

gerritbot added a comment.Mar 11 2021, 4:07 PM

Change 670854 had a related patch set uploaded (by Hashar; owner: Hashar):
[mediawiki/core@master] doc: strip content of @suppress or @phan annotations

https://gerrit.wikimedia.org/r/670854

gerritbot added a comment.Mar 11 2021, 9:36 PM

Change 670854 abandoned by Hashar:
[mediawiki/core@master] doc: Let Doxygen strip content of @suppress or @phan annotations

Reason:
Bah Anomie did the exact same thing a year ago and I missed his change while browsing the change. So abandoning in favor of https://gerrit.wikimedia.org/r/c/mediawiki/core/ /571559 which has a better commit message and some other enhancements. Sorry Timo!

https://gerrit.wikimedia.org/r/670854

gerritbot added a comment.Mar 17 2021, 11:57 PM

Change 571559 merged by jenkins-bot:
[mediawiki/core@master] docs: Improve Doxygen aliases and map more tool annotations to @noop

https://gerrit.wikimedia.org/r/571559

hashar closed this task as Resolved.Mar 18 2021, 9:41 AM
hashar claimed this task.

I checked:

includes/utils/AvroValidator.php
class AvroValidator {
    /**
     * @param AvroSchema $schema The rules to conform to.
     * @param mixed $datum The value to validate against $schema.
     * @return string|string[] An error or list of errors in the
     *  provided $datum. When no errors exist the empty array is
     *  returned.
     * @suppress PhanUndeclaredMethod,PhanUndeclaredProperty
     */
    public static function getErrors( AvroSchema $schema, $datum ) {

Rendered at https://doc.wikimedia.org/mediawiki-core/master/php/classAvroValidator.html and show up as:

doxygen_suppresserror_aliases_to_noop.png (514×637 px, 69 KB)

So that is fixed now (finally).

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL