extractor icon indicating copy to clipboard operation
extractor copied to clipboard

Removed the try/catch that used to handle the AnnotationException(s)

Open antiftw opened this issue 1 year ago • 2 comments

This commit is code-cleanup after the Symfony7 bump. I took another look into the code because I wanted to figure out where the Exception was thrown regarding the unknown Attributes.

It indeed confirmed that that specific Exception was not thrown anymore. However, another exception seems to be able to be thrown, so I'm now at least properly catching that. Maybe we want to add a new test for this? Although I'm not really certain what the Exception indicates. Looking into that as we speak.

P.S. Not really sure why it includes all the other commits in this PR too, even though the changed code is only from the last one. Don't hesitate to tell me if I did something wrong/forgot to do something. I updated my master with the last commit from this one, but maybe because I did the other commit first?

antiftw avatar Jun 14 '24 09:06 antiftw

Hm, the diff looks good, probably so. Probably cherry-pick and squash commits may help, not sure.

You can squash when merging the PR right?

I've also looked into the NoSuchMetadataException, and inside the function we are calling it (MetadataFactoryInterface->getMetadataFor()) it is thrown twice:

Once if the argument is neither an object or a string:

if (!\is_object($value) && !\is_string($value)) {
   throw new NoSuchMetadataException(sprintf(
      'Cannot create metadata for non-objects. Got: "%s".',
      get_debug_type($value)
   ));
}

and then it also checks if its neither a class or an interface:

if (!class_exists($class) && !interface_exists($class, false)) {
   throw new NoSuchMetadataException(sprintf(
      'The class or interface "%s" does not exist.', 
      $class
   ));
}

So that seems like we would have to pass an integer, bool, float or array to that function but that does not really make sense to me in our context, because if you would put an attribute above a simple variable you will just get a syntax error.

Also, since we load our files using namespaces, I'm not even sure how we would get our variables there, since that assumes that the file contains a class, which would imply that there will be no exception thrown.

I'd like to hear what you think about this?

antiftw avatar Jun 14 '24 11:06 antiftw

You can squash when merging the PR right?

Yep, GitHub should squash it.

that does not really make sense to me in our context, because if you would put an attribute above a simple variable you will just get a syntax error

Hm, yeah, probably that can be simplified unless we're missing something here. But I can't think of anything.

bocharsky-bw avatar Jun 15 '24 22:06 bocharsky-bw