Resolve issue reported by PHPStan and PHPCS

I've taken a first pass at the reported errors, and updated the issue description with what remains. I only updated stuff that should be non-breaking changes. What remains is mostly items that need to actually be tested in a working environment.

@james.williams I think I may have stepped on at least one of your changes from last night. I didn't realize you made a push before I applied my changes from local last night. Please review at your convenience.

Based on what is left, my major concerns revolve around the access logic and states where a method doesn't return anything. These could cause actual PHP errors and break the client side.

The pipelines are working correctly now, and I worked through the majority of the issues. I've also updated the issue summary with the remaining reported issues. What remains to fix could affect functionality of the module itself and warrants some real-world testing.

I took another swing at this, rolling back the refactors while still addressing the issues. At this point, all that remains is the call to file_validate_extensions() but for that we should really just cut a new major branch for ^10.2 || ^11 support.

We've got work going on in 📌 Compatibility with D11 Needs review to address file_validate_extensions(). I believe that the \Drupal\Component\Utility\DeprecationHelper class can help us avoid the need for a new major release version. I'd like to avoid starting on a new major version unless it's really necessary. (My biggest motivation being that anyone using the recommended composer version constraint of ^1.0@alpha' would not receive updates from the 2.x branch unless they manually update their requirements.) I actually merged the current 2.x branch into the 8.x-1.x branch yesterday, as there's nothing breaking in there yet anyway. So I'm also tempted to switch this ticket to target 8.x-1.x too.

I'm planning on some testing for that other ticket soon, so I may as well bring this into the scope of my testing too.

I've opened MR !3, which merges MR !1 here with the work from 📌 Compatibility with D11 Needs review since that covered 'file_validate_extensions'. I went with a separate MR to keep MR !1 pristine, if we wanted to keep those two pieces of work separate. (Plus I felt bad building on top of your MR !1 @devkinetic !) I haven't completed testing it, but I have managed to get phpcs and phpstan passing on it now. The 'next major' jobs fail because webform itself isn't compatible with D11 yet so the composer build fails.)

OK; I've now tested my MR !3 ... it did need a bit of fixing, but I'm now confident it's correct.

@devkinetic what do you reckon; would you be up for us merging MR !3 to cover the file extension validator too and without needing to bump the major version? (And therefore fixing 📌 Compatibility with D11 Needs review at the same time too, where we have users pushing for D11 compatibility.)

Gave some time, and no issues found. Confirmed over in 📌 Compatibility with D11 Needs review that MR !3 works well; I'm going to close these two issues and make a new 8.x-1.x release. I'll merge that work into 2.x as well though, but there's still no major need to move to that branch yet.

james.williams → closed merge request !1

Automatically closed - issue fixed for 2 weeks with no activity.