RequestStaticValidateToInjectRector should ignore tests

[chrispmcgee]

A couple of our unit tests require calling Request::create() to function as intended. This RequestStaticValidateToInjectRector rule should never be applied to them since test methods cannot receive injected parameters. Here is an example test method:

    #[Test]
    public function it_transforms_provisional_load_transaction_correctly(): void
    {
        $transaction = ProvisionalLoadTransaction::factory()->create([
            'bill_type' => BillType::ACCESSORIAL,
            'rate_type' => ChargeType::OTHER,
            'amount' => 123.45,
            'currency' => 'USD',
            'note' => 'Test note',
        ]);

        $resource = new ProvisionalLoadTransactionResource($transaction);
        $request = Request::create('/dummy-url');
        $transformed = $resource->toArray($request);

        $this->assertSame($transaction->id, $transformed['id']);
        $this->assertSame(BillType::ACCESSORIAL, $transformed['bill_type']);
        $this->assertSame(ChargeType::OTHER, $transformed['rate_type']);
        $this->assertSame('123.45', $transformed['amount']);
        $this->assertSame('USD', $transformed['currency']);
        $this->assertSame('Test note', $transformed['note']);
    }

I propose that this rule should never be applied to unit tests for this reason.

PS: I've already tried manually skipping it in the rector.php config file:

    ->withSkip([
        RequestStaticValidateToInjectRector::class => [
            __DIR__ . '/tests',
            __DIR__ . '/Modules/*/tests',
        ],
    ]);

But since our project uses Laravel Modules, it seems Rector doesn't support glob patterns in paths. I've raised an issue about this over on their project, as well.

[calebdw]

It looks like this was figured out on the rector side

[chrispmcgee]

It looks like this was figured out on the rector side

How so? I did figure out the 'fnmatch' thing, so I have a workaround, sure. But that doesn't address the underlying issue that RequestStaticValidateToInjectRector shouldn't apply to unit tests because test methods cannot receive injected parameters.

[calebdw]

This repository doesn't have anything to do with which paths are analyzed by rector and which paths are ignored. That is completely up to the user.

The only thing we might could do is make the rule configurable to only modify classes that extend a certain parent, but this is complicated by the fact that Laravel does not necessarily require that controllers extend a particular base class. I'm also not a big fan of heuristics like "every class inside a Controllers/ directory".

Why are you using Request::validate() in the tests anyways? Surely there's a better way to write the test. If nothing else you can at least do something like request()->validate() to prevent the rule from executing

[chrispmcgee]

This repository doesn't have anything to do with which paths are analyzed by rector and which paths are ignored. That is completely up to the user.

The only thing we might could do is make the rule configurable to only modify classes that extend a certain parent, but this is complicated by the fact that Laravel does not necessarily require that controllers extend a particular base class. I'm also not a big fan of heuristics like "every class inside a Controllers/ directory".

That's fair. And I don't mind continuing to use the exclusion since it seems to an issue with our project only, anyway. That is, unless there's a better way to write these two tests…

Why are you using Request::validate() in the tests anyways?

We aren't. We are using Request::create() in a couple of tests, one of which is shown in the original post. Is there a request()->create() that we could use instead to avoid this issue?

The tests were written by other team members, so I do not know their rationale (and I don't think they are with our company anymore, so I cannot just ask them, sadly). I am completely open to alternative ways to write the tests if you have any to suggest, though.

[calebdw]

Hmmm, the rule is specifically about the validate method:

Change static validate() method to $request->validate()

Probably should just update the rule if there's not actually a validate call