Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Activity
Kyle
@kylekatarnls
Like --exclude=migrations/*.php
napestershine
@napestershine

is this the right way?

<exclude-pattern>src/Migrations</exclude-pattern>

I added it in phpmd.xml
Kyle
@kylekatarnls
<exclude-pattern>src/Migrations/*</exclude-pattern>
If I recall well
napestershine
@napestershine
Okay perfect
Kyle
@kylekatarnls
:ok_hand:
napestershine
@napestershine
Another one. I generated login form by SF's make command. phpmd complains about it too
src/Security/LoginFormAuthenticator.php:30 The class LoginFormAuthenticator has a coupling between objects value of 16. Consider to reduce the number of dependencies under 13.
but I am not sure if I should change it because it works out of the box which is generated by maker
Kyle
@kylekatarnls
Actually I felt myself 13 to be too few sometimes.
You can:
<rule ref="rulesets/design.xml/CouplingBetweenObjects">
    <properties>
        <property name="maximum" value="20" />
    </properties>
</rule>
doesn't seem to workout that way, I shared config above
Am I doing something wrong there?
Kyle
@kylekatarnls
Sorry, the complete one:
<rule ref="rulesets/design.xml">
    <exclude name="CouplingBetweenObjects" />
</rule>
<rule ref="rulesets/design.xml/CouplingBetweenObjects">
    <properties>
        <property name="maximum" value="20" />
    </properties>
</rule>
The the next version the rule.exclude will no longer be required.
napestershine
@napestershine
Thanks
I have one more
because this is just a basic User Entity I am getting:
The class User has an overall complexity of 50 which is very high. The configured complexity threshold is 50.
it have nothing special just some fields and then relations only
Kyle
@kylekatarnls
complexity is about the number of branches: if/else/for/ternary, all methods of the class additionned.
So if it's too high, you may consider delegating some methods to traits.
napestershine
@napestershine
let me try that out
Kyle
@kylekatarnls
You also could change this threshold like any parameter, but before changing a parameter, you should first try some decomposition and see if it can make the responsibilities isolation better.
napestershine
@napestershine
Yes, it sorted out with trait
Thanks
Tobias van Beek
@tvbeek
@napestershine nice to see your questions :) After reviewing your code to see if something can be changed to be more readable / decomposition like @kylekatarnls sad. You can not only ignore the file or change the properties of the rule but you also have the possibility to annotate a specific part to suppress a specific warning. See https://phpmd.org/documentation/suppress-warnings.html for some examples.
napestershine
@napestershine
@tvbeek Thank you so much. Its now working well without any problems.
Anders Jenbo
@AJenbo

@napestershine in case you are interested here are configurations based on SIG (https://www.softwareimprovementgroup.com/) recommendation https://github.com/AJenbo/agcms/blob/master/phpmd.xml

I did relax the lines per function rule. Ideally, functions should be 16 lines, but 31 is generally acceptable.

Anders Jenbo
@AJenbo
I'm reminded since they also recommend a max of 20 couplings between objects.
Marc Würth
@ravage84
@AJenbo do you think we should adopt these recommendations for PHPMD in future versions?
napestershine
@napestershine
@AJenbo Thank you so much. I will use above config.

I just updated my application to SF5.1 and I have config and file
https://gist.github.com/napestershine/b8a571e8e85d246f100a745d1b217ba5

I am getting:

src/Kernel.php:14    Avoid assigning values to variables in if clauses and the like (line '22', column '27').
src/Kernel.php:27    Avoid assigning values to variables in if clauses and the like (line '34', column '27').

And I guess I never touched this file. so what is best practice in this case ?

Tobias van Beek
@tvbeek
@napestershine I ususally ignore the files delivered by the framework. Mostly with an annotation and add an author tag to be clear that it is from the framework. So in this case something like:
/**
 * @SuppressWarnings(PHPMD)
 * @author Symfony Framework
 */
class Kernel extends BaseKernel
...
Anders Jenbo
@AJenbo
@ravage84 I think it would be a good idea since it is rooted in an ISO standard. Which is probably better than most other settings we could choose :)
Also, most of the defaults in PHPMD are already fairly close so it wouldn't be a huge change.
ISO/IEC 25010:2011 if anyone cares to read :P
Anders Jenbo
@AJenbo

The biggest difference is that SIG allows for various percentages distributions, something like:
100% of functions are 60 lines or less
95% of functions are 30 lines or less
75% of functions are 15 lines or less

But PHPMD only allows for 100% are X or less.

If that is something we can work on supporting that would be a really good upgrade to the tool.

napestershine
@napestershine

The method setSlug() has a Cyclomatic Complexity of 10. The configured cyclomatic complexity threshold is 10.

how to fix it ?

napestershine
@napestershine
Anders Jenbo
@AJenbo
Going to work on the baseline feature.
Karel Němec
@MrSuit
Hello everybody, my apologies for my horrible english at first :). I have specific question about specific rule. It is "MissingImport" rule. Should it warn me about not importing default classes? E.g. \RuntimeException(), \PDO(), etc... . And what about methods? if I leave in code for example count() it warns me that in my class I should not use such method because it thinks it doesn't exists so I must do \count but for every class mentioned before is not considerable to leave them with backslash. What am I suppose to do, what is the best approach and why? Is it bad to leave classes right in placee.g. "new Psr\Log\Logger(...)" to immediately see from where is it originated? Thank you all for any response with explanation.
Kyle
@kylekatarnls
\RuntimeException should be indeed replaced by RuntimeException with an import use RuntimeException; in the top of the file. This way your file declare explicitly its dependencies.
About count, I never had such trouble. Can you create a minimal code chunk to reproduce the error and the complete error please?
Karel Němec
@MrSuit
Count was about different rule, my apologies. Thanks for answering my question :)
Tobias van Beek
@tvbeek
Anders Jenbo
@AJenbo
:thumbsup: