Skip to content

Add failing test to demonstrate parse error when @ is present in the description #195

Closed
wants to merge 1 commit into from

5 participants

@Seldaek
Doctrine member
Seldaek commented Oct 2, 2012

If someone can take this and fix the issue it'd be great. I couldn't figure it out at a quick glance and I don't really have time, but it's a pretty messed up bug and not so trivial to debug if you don't know what happens in the background so I'd say it's pretty important.

@doctrinebot

Hello,

thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link:

http://doctrine-project.org/jira/browse/DCOM-118

@doctrinebot

Oh btw, I just (automatically) realized that you are not creating this pull request against the master branch.

Unless there are good reasons for this, i would suggest to close and rebase the Pull Request against master and then create it again. Sorry!

@Seldaek
Doctrine member
Seldaek commented Oct 2, 2012

Oh btw, did you realize GitHub released a nice feature to help contributors before they fuck up? https://github.com/blog/1184-contributing-guidelines

@pscheit pscheit added a commit to pscheit/common that referenced this pull request Oct 3, 2012
@pscheit pscheit fix: #DCOM-118 #195 restrict quoted-strings only to strings within qu…
…otes without a newline character
f1516fb
@pscheit pscheit added a commit to pscheit/common that referenced this pull request Oct 5, 2012
@pscheit pscheit fix: #DCOM-118 #195 restrict quoted-strings only to strings within qu…
…otes without a newline character
06aedcc
@schmittjoh
Doctrine member

I'm not sure whether we can fix this at all.

Is it possible to change the doc comment?

@stof
Doctrine member
stof commented Oct 7, 2012
@schmittjoh
Doctrine member

I've seen this fix, but it is problematic as it breaks multiline strings.

@pscheit
pscheit commented Oct 7, 2012

i had only one good other idea: to restrict the definition of an annotation when it is a string with an @ at the beginning of a "new line" in a doc block

e.g.

\s*\*\s+@

something like that. Because i think thats the only difference from other annotations in the doc block.

@schmittjoh
Doctrine member

Currently, we have an optimization in the DocParser in that we trim everything from the doc comment until the first @.

We could instead add an additional search for the first " and then only trim to whatever comes first. This should solve this, but makes the parsing process potentially a bit slower, and if you have an unclosed " in your doc comment it would still mess up. So, I'm not really sure it is worth it.

@pscheit
pscheit commented Oct 8, 2012

yep, it does not help much, had this idea, too. what about escaping multi line lineends with an \

@multilineannotation(" bla bla bla \
la la la \
la la")
@Seldaek
Doctrine member
Seldaek commented Oct 8, 2012

Just for the record, yes of course I fixed my docblock in consequence, but it's a really nasty issue so it'd be nice if it could at least be detected and error out instead of returning no annotations.

@Seldaek
Doctrine member
Seldaek commented Mar 25, 2014

@schmittjoh can you point me to the place where the trimming is done? Then I can try to improve it without making it slower. This bug is really a pain.

@Seldaek
Doctrine member
Seldaek commented Mar 25, 2014

Thanks @pscheit - new version of the PR on the new repo, closing this. See doctrine/annotations#28

@Seldaek Seldaek closed this Mar 25, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.