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

Inline tag handling with nested braces is not working as expected #255

Open
jaapio opened this issue Sep 20, 2020 · 5 comments
Open

Inline tag handling with nested braces is not working as expected #255

jaapio opened this issue Sep 20, 2020 · 5 comments

Comments

@jaapio
Copy link
Member

jaapio commented Sep 20, 2020

Given the following test case, our Description doesn't work as expected with nested braces in inline tags.

   /**
     * @uses \phpDocumentor\Reflection\DocBlock\Description
     * @uses \phpDocumentor\Reflection\DocBlock\Tags\Link
     * @uses \phpDocumentor\Reflection\DocBlock\Tags\BaseTag
     * @uses \phpDocumentor\Reflection\DocBlock\Tags\Formatter\PassthroughFormatter
     * @uses \phpDocumentor\Reflection\Types\Context
     *
     * @covers ::__construct
     * @covers ::create
     */
    public function testDescriptionCanParseStringWithInlineTagAndBraces() : void
    {
        $contents   = 'This description has a {@link http://phpdoc.org/ This contains {braces} }';
        $context    = new Context('');
        $tagFactory = m::mock(TagFactory::class);
        $tagFactory->shouldReceive('create')
            ->twice()
            ->andReturnValues(
                [
                    new LinkTag('http://phpdoc.org/', new Description('This contains {braces}')),
                ]
            );

        $factory     = new DescriptionFactory($tagFactory);
        $description = $factory->create($contents, $context);

        $this->assertSame($contents, $description->render());
        $this->assertSame('This description has a %1$s', $description->getBodyTemplate());
    }
@tipiak75
Copy link

tipiak75 commented Oct 12, 2020

@jaapio do you still need any help with this issue ?

If so I might have a candidate fix to contribute.

It involves loosening up a bit the regexp in DescriptionFactory::lex() so that the tag is properly matched up all the way to the last closing brace. That would allow nested braces inside the tag description without compromising any other test afaik. Would that be an acceptable solution ?

Even with that out of the way though, I'm still losing the trailing space between the closing braces in the provided test somewhere. Still investigating why that happens, any idea ?

@tipiak75
Copy link

Even with that out of the way though, I'm still losing the trailing space between the closing braces in the provided test somewhere. Still investigating why that happens, any idea ?

From what I've found, this secondary issue originates from the trim() call in StandardTagFactory::create() but this seems to be intended behavior (removing the trim() call fixes it but also breaks two other tests related to this class). So I guess the test caseshould expect the tag output in the rebuilt description to be trimmed, and needs to be changed acordingly. Thoughts ?

@jaapio
Copy link
Member Author

jaapio commented Oct 12, 2020

If you think you have a way to solve this, please create a PR. We can discuss the details later.

In general, we should be able to fix issues without breaking tests because people might use the situations that are tested. That's why the tests are there. Unless we can prove that the test case was invalid.

tipiak75 added a commit to tipiak75/ReflectionDocBlock that referenced this issue Oct 12, 2020
@tipiak75
Copy link

Thanks, please review the WIP fix at #258 .
It took me quite a while to come up with deleting this character 👍

@WinterSilence
Copy link
Contributor

if description of inline tags not auto trimmed, then test failed because you miss whitespace: new Description('This contains {braces}'), valid: new Description('This contains {braces} ')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants