Skip to content

#162 Modified @deprecated validator to consider it valid if comment is absent but @see tag is set #181

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Sep 2, 2020
Merged

#162 Modified @deprecated validator to consider it valid if comment is absent but @see tag is set #181

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6a3f52f
#162 Modified @deprecated validator to consider it valid if comment i…
sinisa86 Apr 4, 2020
2b172c3
Merge branch 'develop' into ticket-162
lenaorobei Apr 13, 2020
3e79a8a
Merge branch 'develop' into ticket-162
sinisa86 Apr 16, 2020
029fada
Merge branch 'develop' of github.com:magento/magento-coding-standard …
sinisa86 Apr 28, 2020
c6bc189
#162 Reduced code around IF condition on seeTagRequired variable
sinisa86 Apr 28, 2020
76f5f2d
Merge branch 'ticket-162' of github.com:sinisa86/magento-coding-stand…
sinisa86 Apr 28, 2020
5b82e31
Merge branch 'develop' into ticket-162
lenaorobei Sep 2, 2020
4bd4113
magento/magento-coding-standard#162: @deprecated without @see tag fal…
lenaorobei Sep 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions Magento2/Helpers/Commenting/PHPDocFormattingValidator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,13 +122,18 @@ public function hasDeprecatedWellFormatted($commentStartPtr, $tokens)
return true;
}

$seeTagRequired = false;
if ($tokens[$deprecatedPtr + 2]['code'] !== T_DOC_COMMENT_STRING) {
return false;
$seeTagRequired = true;
}

$seePtr = $this->getTagPosition('@see', $commentStartPtr, $tokens);
if ($seePtr === -1) {
return true;
if ($seeTagRequired) {
return false;
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else can be eliminated here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we remove "else" branch there, it could lead to unexpected behavior because of the next IF condition:
if ($tokens[$seePtr + 2]['code'] !== T_DOC_COMMENT_STRING) { return false; }
That happens because $seePtr could be -1, so $seePtr + 2 points to how-knows-what.
"Else" part is required because it covers behavior when @see is not present at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, it really fails on some regular tests when I remove "else" branch.

Copy link
Contributor

@lenaorobei lenaorobei Apr 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$seeTagRequired is boolean and if it is false, execution will drop to else or directly to the return.

I propose to simplify code even more:

        $seePtr = $this->getTagPosition('@see', $commentStartPtr, $tokens);
        if ($seePtr === -1) {
            return !$seeTagRequired;
        }
        return $tokens[$seePtr + 2]['code'] === T_DOC_COMMENT_STRING;

return true;
}
}
if ($tokens[$seePtr + 2]['code'] !== T_DOC_COMMENT_STRING) {
return false;
Expand Down
9 changes: 9 additions & 0 deletions Magento2/Tests/Commenting/ClassAndInterfacePHPDocFormattingUnitTest.1.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,3 +153,12 @@ class DoNotCareHandler
{

}

/**
* @deprecated
* @see Magento\Framework\NewHandler
*/
class OldHandler
{

}
6 changes: 6 additions & 0 deletions Magento2/Tests/Commenting/ConstantsPHPDocFormattingUnitTest.1.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ class Profiler
*/
const COMPUTER = 'Deep Thought';

/**
* @deprecated
* @see \ComputationalMatrix\Mars
*/
const KEYBOARD = 'Ergonomic';

/**
* @see
*/
Expand Down