Add ExceptionList property for UnusedFormalParameter

[PedroPMS]

Type: feature

This PR add the exception property to the UnusedFormalParameter rule.

Some classes of some frameworks, such as Laravel ApiResource, uses some parameters that arent used by the method.

[AJenbo]

Isn't that then a fault with the class, correctly detected by PHPMD. and should be corrected instead of ignored?

[PedroPMS]

Isn't that then a fault with the class, correctly detected by PHPMD. and should be corrected instead of ignored?

Yeah, is a fault with the class, but the entire framework uses like this. So we can only ask for a fix in the framework and implement a exception list in PHMD 🤷‍♂️

[AJenbo]

Maybe a better solution is to have a setting to ignore violations that are inherited?

[kylekatarnls]

IIRC it's a method_exists($class, 'toArray') trick used from Laravel side, so no inheritence. I'm in favor of an unified exceptions option available in all the rules anyway.

[tvbeek]

In this case I should remove the request because it isn't required to have. (I have changed it in my stubs)
If that wasn't possible I should add an unset call on the first line of the function to be clear that it isn't used. Another option is to add a suppress comment.

A generic rule means that you have more change to hide a place where it should notice about a problem.

[AJenbo]

IIRC it's a method_exists($class, 'toArray') trick used from Laravel side, so no inheritence. I'm in favor of an unified exceptions option available in all the rules anyway.

If that is the case then there is no reason to actually have the unused argument. PHP doesn't have function overloading so if Laravel is calling the function with more arguments then it has it will still work the same.

[PedroPMS]

As you guys said, there are other ways to suppress this warning, suppress comment, ignore inherited and others. The exception property is just one more option that anybody can use as they wish. This feature has no breaking changes.

[Sandant589]

Ok

[kylekatarnls]

Handling inheritance better does not prevent to have an list for exceptions.

This PR still need to have documentation update and unit tests. If so I would approve. @AJenbo @tvbeek @ravage84 Are you fine with that? Let's have a definitive go/no-go decision so either we define what it needs to be merged, or we close.

[ravage84]

Since it's pointless to fight bad framework design and people potentially shooting themselves in the foot, we can add this.

But it needs docs, tests etc as usual.