Add a configuration to skip DocBlock Custom Tag detection (#336)

[LuigiCardamone]

This PR implements the feature described in this issue: #336

To skip the check you need to add this configuration:

return static function (Config $config): void {
    $config->setParseDocBlockCustomTags(false);
    //...
}

No breaking change is added: if the configuration is not modified the default behavior is used (ParseDocBlockCustomTags=true).

[AlessandroMinoccheri]

Can you please change the readme file documenting how to skip DocBlock Custom Tag detection ?

[AlessandroMinoccheri]

Is it possible to avoid another nested level?

[LuigiCardamone]

done! I also updated the docs.

[codecov-commenter]

Codecov Report

Merging #337 (102465a) into main (3c0eecc) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main     #337      +/-   ##
============================================
+ Coverage     93.99%   94.02%   +0.03%     
- Complexity      502      505       +3     
============================================
  Files            60       60              
  Lines          1365     1372       +7     
============================================
+ Hits           1283     1290       +7     
  Misses           82       82              

Impacted Files	Coverage Δ
src/Analyzer/FileParserFactory.php	100.00% <100.00%> (ø)
src/Analyzer/NameResolver.php	77.91% <100.00%> (+0.13%)	⬆️
src/CLI/Config.php	95.45% <100.00%> (+1.70%)	⬆️
src/CLI/Runner.php	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[LuigiCardamone]

With my last commit I updated FileVisitorTest to solve a conflict with #335 and ConfigTest to increase test code coverage

[AlessandroMinoccheri]

LGTM 👍

[micheleorselli]

I am not sure if we should skip entirely the parsing of dependencies in a dobclock, since they are "soft" dependencies. What is your opinion on that?

If we stick to exclude only custom docblock may I suggest to change the name of the config method to make it more explicit? Something like $config->skipParsingAnnotations or $config->doNotParseAnnotations()

[LuigiCardamone]

I am not sure if we should skip entirely the parsing of dependencies in a dobclock, since they are "soft" dependencies. What is your opinion on that?

If we stick to exclude only custom docblock may I suggest to change the name of the config method to make it more explicit? Something like $config->skipParsingAnnotations or $config->doNotParseAnnotations()

In my opinion with DocBlocks you can handle two complete different cases:

1. Express the type of the variable:

    /**
     * @var QueryBuilder
     */
    private $foo;

that is just the legacy version of private QueryBuilder $foo.

2. Give additional information about that variable:

    /**
     * @JMS\Expose
     */
    private string $foo;

The class may work event without that information (that is used outside).

I guess that violations about Case 1 are much more critical than Case 2. In our project we cannot easily remove dependency about Case 2. We need an option to skip violations of Case 2 (like @JMS\Expose and similar).

How would you call such an option?
I used the term "customTag" since a $customTag variable was already used in NameResolver.php.
I am afraid that skipParsingAnnotations can be misleading with Case 1. What about skipParsingCustomAnnotations or parseOnlyTypingAnnotations?

[micheleorselli]

ok, let's keep the current PR behaviour. regarding the name skipParsingCustomAnnotations sounds fine to me

[LuigiCardamone]

ok, let's keep the current PR behaviour. regarding the name skipParsingCustomAnnotations sounds fine to me

@micheleorselli I just updated the code, the test and the documentation.

The GitHub check that failed is for something not related to this issue: "Composer 2.3.0 dropped support for PHP <7.2.5 and you are running 7.1.33-51+ubuntu20.04.1+deb.sury.org+1, please upgrade PHP or use Composer 2.2 LTS via "composer self-update --2.2". Aborting."

[AlessandroMinoccheri]

@LuigiCardamone it's Github/composer issue, I run actions, and now it's totally green.