Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support @param-taint in PHP (permit dashes in command aliases?) #9047

Closed
Krinkle opened this issue Jan 17, 2022 · 6 comments
Closed

Support @param-taint in PHP (permit dashes in command aliases?) #9047

Krinkle opened this issue Jan 17, 2022 · 6 comments
Labels
enhancement a request to enhance doxygen, not a bug

Comments

@Krinkle
Copy link
Contributor

Krinkle commented Jan 17, 2022

Describe the bug

Unable to remove lines like @param-taint $foo none from the HTML output.

Currently this results in:

LinkRenderer.php:266: warning: argument '$target' from the argument list of
  LinkRenderer\makeBrokenLink has multiple @param documentation sections

Screenshot 2022-01-17 at 23 29 39

Expected behavior

/**
 * Class that generates HTML anchor link elements for pages.
 *
 * @since 1.28
 */
class LinkRenderer {
	/**
	 * @param LinkTarget|PageReference $target
	 * @param-taint $target none
	 * @param string|HtmlArmor|null $text
	 * @param array $extraAttribs
	 * @param array $query
	 * @return string
	 */
	public function makeBrokenLink(
		$target, $text = null, array $extraAttribs = [], array $query = []
	) {
	}
}
ALIASES                = "codeCoverageIgnore=\noop" \
                         "codingStandardsIgnoreEnd=\noop" \
                         "codingStandardsIgnoreStart=\noop" \
                         "phan=\noop" \
                         "param-taint=\noop" \
                         "suppress=\noop"

Version
doxygen 1.9.2 and doxygen 1.9.3.

Additional context

error: Illegal ALIASES format 'param-taint=\noop'. Use "name=value" or "name{n}=value", where n is the number of arguments

I suspect there is (or once was) syntax ambiguity where a command that takes no argument needs to be distinct from other content. E.g. the full-stop in Hello @foo., which seems reasonable to allow as separator. I guess that means we currently also permit the dash to be used in this way, e.g. Hello @foo- How are you?. I'm hoping that maybe this is okay to change by considering the dash in this case (if there isn't a space after it) to be considered part of the command.

Or alternatively, we could perhaps do something clever where we first give it a chance to match a command including the adjecent dash, and if not known as command, fallback to rendering separately.

@Krinkle Krinkle changed the title Permit dash in command aliases Support @param-taint in PHP (permit dashes in command aliases?) Jan 17, 2022
@albert-github
Copy link
Collaborator

The - is indeed the character that is not possible in an ID (in this case a command name), the question that arises is why do you need a - in the ID, can't you use an _.?

I think that the background is that the - sign is the minus sign and in computer languages (as far as I know) this character is not allowed in identifiers as it is the subtraction sign. The command name also is defined as an identifier and the hence exclusion.

@albert-github albert-github added the needinfo reported bug is incomplete, please add additional info label Jan 18, 2022
@Krinkle
Copy link
Contributor Author

Krinkle commented Jan 18, 2022

@albert-github If I understand correctly, you're asking why the static analysis software declared its annotation as @param-taint rather than @param_taint. I'm guessing that the auhors felt a dash was more natural and consistent with other parts of the coding conventions in PHP.

For what it's worth, the defacto PHPDoc standard (draft) includes a - hyphen as a valid symbol part of a tag's name, with @phpdoc-event as example of a valid tag.

@albert-github albert-github added enhancement a request to enhance doxygen, not a bug and removed needinfo reported bug is incomplete, please add additional info labels Jan 19, 2022
@albert-github
Copy link
Collaborator

I was indeed asking why you didn't use @param_taint, the mentioned phpdoc draft indeed includes the - as a normal character for a tag (for doxygen command name). Doxygen can't / won't copy everything possible things from every other documentation system also because doxygen is general purpose (tries to supports multiple computer languages) and e.g. phpdoc only supports php (as far as I know).

I think the enhancement request for the - is a valid request to be allowed in ALIASES.
@doxygen what do you think?

albert-github added a commit to albert-github/doxygen that referenced this issue Jul 16, 2022
…and aliases?)

Add the possibility to have a dash in an ALIASES.
@albert-github
Copy link
Collaborator

I've just pushed a proposed patch, pull request #9451

albert-github added a commit to albert-github/doxygen that referenced this issue Jul 16, 2022
…and aliases?)

The documentation gave the warning:
```
doc/commands.doc:3461: error: Found unknown command '\LaTeX'
```
this was due to the fact that in the code there was:
```
\LaTex-only
```
and this is seen as a potential `ALIASES`
doxygen added a commit that referenced this issue Jul 18, 2022
issue #9047 Support @param-taint in PHP (permit dashes in command aliases)?
@albert-github albert-github added the fixed but not released Bug is fixed in github, but still needs to make its way to an official release label Jul 19, 2022
@albert-github
Copy link
Collaborator

Code has been integrated in master on GitHub (please don't close the issue as this will be done at the moment of an official release).

albert-github added a commit to albert-github/doxygen that referenced this issue Jul 19, 2022
Small regression on  doxygen#9451 issue doxygen#9047 Support @param-taint in PHP (permit dashes in command aliases)?
In a number of project (e.g. CGAL) we now got the warning like
```
warning: Found unknown command '\cgal'
```
as the code was:
```
\cgal-like
```
and
```
ALIASES += cgal=%CGAL
```

made the alias replacement so that when the expansion didn't do anything and the input string contains a `-` sign the last `-` sign is stripped as well as the part after it and a new attempt is made to resolve the command.
@doxygen
Copy link
Owner

doxygen commented Aug 26, 2022

This issue was previously marked 'fixed but not released',
which means it should be fixed in doxygen version 1.9.5.
Please verify if this is indeed the case. Reopen the
issue if you think it is not fixed and please include any additional information
that you think can be relevant (preferably in the form of a self-contained example).

@doxygen doxygen removed the fixed but not released Bug is fixed in github, but still needs to make its way to an official release label Aug 26, 2022
@doxygen doxygen closed this as completed Aug 26, 2022
wmfgerrit pushed a commit to wikimedia/integration-zuul-config that referenced this issue Nov 6, 2023
Various bug fixes, including support for tags and aliases for tags
that include dashes, such as `@param-taint` and `@return-taint`.
doxygen/doxygen#9047

See the full log at https://www.doxygen.nl/manual/changelog.html

Change-Id: Ie025bd8a5e959031952ff86aae49b91f6e108b1a
wmfgerrit pushed a commit to wikimedia/mediawiki that referenced this issue Nov 16, 2023
I've upgraded Doxygen to 1.9.8 in WMF CI (Ie025bd8a5e9), and among
the bug fixes was doxygen/doxygen#9047,
which makes it possible to use tags that contain dashes in ALIASES.

Change-Id: Ida5fddb89b76445922a87904745eff0a1e299043
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement a request to enhance doxygen, not a bug
Projects
None yet
Development

No branches or pull requests

3 participants