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

SuppressWarnings(CyclomaticComplexity, NPathComplexity) fails in function with internal docblock #914

Closed
SilkAndSlug opened this issue Oct 12, 2021 · 6 comments
Assignees

Comments

@SilkAndSlug
Copy link

  • PHPMD version: 2.10.2
  • PHP Version: 7.4.24
  • Installation type: composer
  • Operating System / Distribution & Version: Debian 11

Current Behavior

Adding a docblock (e.g. a @todo) inside the function prevents warnings from being suppressed.

doesNothing.php:6    The function doesNothing() has a Cyclomatic Complexity of 13. The configured cyclomatic complexity threshold is 10.
doesNothing.php:6    The function doesNothing() has an NPath complexity of 256. The configured NPath complexity threshold is 200.

Expected Behavior

Warnings should be suppressed, even with a docblock present.

[No output]

Steps To Reproduce:

doesNothing.php :

<?php
/**
 * @SuppressWarnings(PHPMD.CyclomaticComplexity)
 * @SuppressWarnings(PHPMD.NPathComplexity)
 */
function doesNothing() {
	if (true AND true AND false) return;
	/** This comment breaks SuppressWarnings */
	if (true AND true AND false) return;
	if (true AND true AND false) return;
	if (true AND true AND false) return;
}

From the command-line: phpmd doesNothing.php text codesize

Note that the bug doesn't occur if the @todo is on the first line.

@vertexvaar
Copy link

vertexvaar commented Jan 16, 2022

Annotations of functions inside conditions are also ignored.

if (!function_exists('doesNothing')) {
    /**
     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
     * @SuppressWarnings(PHPMD.NPathComplexity)
     */
    function doesNothing() {
        if (true AND true AND false) return;
        if (true AND true AND false) return;
        if (true AND true AND false) return;
        if (true AND true AND false) return;
    }
}
 390 | VIOLATION | The function doesNothing() has a Cyclomatic Complexity of 13. The configured cyclomatic complexity threshold is 10.
 390 | VIOLATION | The function doesNothing() has an NPath complexity of 256. The configured NPath complexity threshold is 200.

@vever001
Copy link

I'm seeing the same issue.
It looks like it is coming from pdepend, specifically \PDepend\Source\Language\PHP\AbstractPHPParser::parseFunctionOrClosureDeclaration
The comment for the ASTFunction is the wrong one (it is set to the last one inside the function and not the doc block of the function). Then of course it fails in phpmd when it checks if the comment contains the SuppressWarning.

One workaround seems to be to put those internal docblocks at the top of the function.

kylekatarnls added a commit to pdepend/pdepend that referenced this issue Nov 16, 2023
kylekatarnls added a commit to pdepend/pdepend that referenced this issue Nov 16, 2023
@kylekatarnls
Copy link
Member

Good catch! I was able to confirm with a unit test:
pdepend/pdepend#687

@kylekatarnls
Copy link
Member

Fix found and tested, it should be in the next PDepend release.

@vever001
Copy link

Amazing, thanks @kylekatarnls

@kylekatarnls
Copy link
Member

2.16.0 should soon be released.

BTW you can already test it using:

composer require "pdepend/pdepend:dev-master as 2.16.0"

renovate bot added a commit to line/line-bot-sdk-php that referenced this issue Dec 11, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [phpmd/phpmd](https://phpmd.org/)
([source](https://togithub.com/phpmd/phpmd)) | `2.14.1` -> `2.15.0` |
[![age](https://developer.mend.io/api/mc/badges/age/packagist/phpmd%2fphpmd/2.15.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/packagist/phpmd%2fphpmd/2.15.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/packagist/phpmd%2fphpmd/2.14.1/2.15.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/packagist/phpmd%2fphpmd/2.14.1/2.15.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>phpmd/phpmd (phpmd/phpmd)</summary>

###
[`v2.15.0`](https://togithub.com/phpmd/phpmd/blob/HEAD/CHANGELOG#phpmd-2150-20231211)

[Compare
Source](https://togithub.com/phpmd/phpmd/compare/2.14.1...2.15.0)

\========================

- Added [#&#8203;1036](https://togithub.com/phpmd/phpmd/issues/1036)
\[CLI] Allow option and value separated with equal sign
-   Require pdepend/pdepend 2.16.1
- Support PHP 8.3
[pdepend/pdepend#699](https://togithub.com/pdepend/pdepend/issues/699)
- Support Symfony 7
[pdepend/pdepend#692](https://togithub.com/pdepend/pdepend/issues/692)
- Fixed
[pdepend/pdepend#691](https://togithub.com/pdepend/pdepend/issues/691)
Float parsing for number starting with 0.
- Fixed
[pdepend/pdepend#689](https://togithub.com/pdepend/pdepend/issues/689)
Handle conversion to/detection of UTF-8 encoding using either mbstring
PHP extension or the polyfill provided by Symfony
- Fixed
[pdepend/pdepend#687](https://togithub.com/pdepend/pdepend/issues/687)
Parsing the correct comment for method doc-block (Allow correct
SuppressWarnings annotation handling on PHPMD)
([phpmd/phpmd#914)
- Fixed
[pdepend/pdepend#694](https://togithub.com/pdepend/pdepend/issues/694)
Handle yield termination depending on context
([phpmd/phpmd#804)
- Fixed [#&#8203;1044](https://togithub.com/phpmd/phpmd/issues/1044)
strict option on applyOnClassMethods
- Documented
[#&#8203;1041](https://togithub.com/phpmd/phpmd/issues/1041) Mention
public key used for signing the Phars
- Documented
[#&#8203;1042](https://togithub.com/phpmd/phpmd/issues/1042) Document
installation with PHIVE

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/line/line-bot-sdk-php).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy44Ny4yIiwidXBkYXRlZEluVmVyIjoiMzcuODcuMiIsInRhcmdldEJyYW5jaCI6Im1hc3RlciJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@ravage84 ravage84 added this to the 2.x (unspecific) milestone May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants