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

ISSUE-35: Lookup parent when inheritDoc is used. #36

Merged
merged 3 commits into from
Sep 2, 2020
Merged

Conversation

BowlOfSoup
Copy link
Owner

No description provided.

@@ -18,6 +18,18 @@ public function extractMethodAnnotations(\ReflectionMethod $objectMethod, $annot
{
$annotations = [];

$docComment = $objectMethod->getDocComment();
if ($docComment
&& false !== strpos(strtolower($docComment), '@inheritdoc')
Copy link
Collaborator

@raymondschouten raymondschouten Sep 1, 2020

Choose a reason for hiding this comment

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

Using strpos for searching "@inheritdoc" is not safe, since {@inheritdoc} is an inline tag that can be used in the long description to inherit the parent long description inlined, eg:

/**
 * Makes bars
 *
 * This class generates bars using the main algorithm.
 */
class Bar
{
}
 
/**
 * Makes chocolate bars
 *
 * There are two aspects to this class.
 * {@inheritdoc}  In addition, the foo class
 * makes the bars chocolate
 */
class Foo extends Bar
{
}

https://manual.phpdoc.org/HTMLSmartyConverter/HandS/phpDocumentor/tutorial_tags.inlineinheritdoc.pkg.html

Also for some particular reason, the text might exist in the doc comment, without it being an actual annotation.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Ok, solving it with checking inheritDoc is too risky. I now specifically fixed it for a Doctrine proxy class.

Copy link
Collaborator

@raymondschouten raymondschouten Sep 1, 2020

Choose a reason for hiding this comment

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

If you do this specifically for proxy classes, which are in essence nothing more than classes in an inheritance structure, don't you think you are doing yourself short here?

What about regular parent classes?

class Bar
{
    /**
     * @Bos\Normalize()
     */
    public function getName(): string
    {
        return 'john wick';
    }
}
 
class Foo extends Bar
{
    /**
     * PHPDoc without @Bos\Normalize annotation
     */
    public function getName(): string
    {
        return 'John Wick';
    }
}

And what about overridden methods without a docblock?

Copy link
Owner Author

@BowlOfSoup BowlOfSoup Sep 1, 2020

Choose a reason for hiding this comment

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

Thought about that, I don't want to 'just' take the parent normalize annotations.
If someone 'overwrites' a method, they need to make sure there are new annotations on the (new) method.

I've created ISSUE-37 for this use case :)

@BowlOfSoup BowlOfSoup merged commit 78f80f5 into master Sep 2, 2020
@BowlOfSoup BowlOfSoup deleted the ISSUE-35 branch September 2, 2020 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Doctrine proxy] Parent method should be checked when child method that does not contain annotations
2 participants