phpDocumentor / ReflectionDocBlock

MIT License
9.35k stars 119 forks source link

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

Open jaapio opened 4 years ago

jaapio commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

WinterSilence commented 2 years ago

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} ')