Should "String literals should not be duplicated" in Laravel request rules be ignored?

Clean Code Report False-positive / False-negative...
,
1

Hi, we are currently running a self-hosted SonarQube for our Laravel 10 / 11 project but receive some issues on our Laravel-styled code for string literal duplication - especially in request files (namespace App\Http\Requests\).

  • Language: PHP 8.3 + Laravel 10 / 11
  • Rule: php:S1192 “String literals should not be duplicated”
  • SonarQube Developer Edition v10.6

Example:

<?php

namespace App\Http\Requests;

use Illuminate\Foundation\Http\FormRequest;

class CreateNewUser extends FormRequest
{
    /**
     * Get the validation rules that apply to the request.
     */
    public function rules(): array
    {
        return [
            'email' => ['required', 'email:rfc', 'max:100'],
            'token' => ['required', 'max:100', new TokenRule()],
            'password' => ['required', 'string', new PasswordRule()],
            'firstname' => ['required', 'string', 'max:100'],
            'surname' => ['required', 'string', 'max:100'],
            'company' => ['nullable', 'string', 'max:100'],
        ];
    }
}

After the scan, the max:100 strings are noted as the above issue. You can find this issue as well for various other Laravel parameterized rules like date_format:<format>, min:<x>, max:<x> as in the Laravel Docs Available Validation Rules. In this particular case, I am not convinced that this should be an issue because this is pure Laravel style to define request validation rules. Defining a constant like UserRequest::MAX_100 solves the issue but does not make the code more readable or more maintainable since it’s a default Laravel approach where duplications are okay for good readability. Also, for fixing this issue application-wide would require defining a constant for all possible occurrences. I think it’s some kind of special case Laravel config string, but not a duplicated string.

Since I am a newbie in SonarQube: is there any way to add a string like above to a deny-list? I do not want to add a NOSONAR to every file or line or disable the rule, to catch other errors in the file or catch real duplicated string in the project. If there is no way, I would request some kind of “ignore-option” or recognition for Laravel-Way styled request files.

Thanks!

4

Hello @mx-jhinz and welcome to community!

Thanks for raising awareness on this topic.

This is an interesting use case for the Laravel framework.
It makes sense to consider it a false positive; as you rightly mentioned, I’m not convinced that defining a constant to avoid those string literal repetitions would help and bring values; it is more likely that it would reduce code readability.

Unfortunately, I cannot find any workaround here. The rule has only two configurable parameters (threshold and minimal_literal_length), and none of them will help us here.
The only other ways are the ones you mentioned (NOSONAR or disable rule), with the implication you mentioned.
I created a ticket to find/setup a long-term solution for this rule.

Sorry for the inconvenience caused.
Best regards,
Rudy

6

Hey Rudy,

thanks for your reply and the topic is addressed.
One more hint that may make the decision easier (or harder) and you might want to consider:

In addition to typical request validation files, one can define validators manually in any Laravel file to validate data. So it can occur not only the mentioned request files. This case is much rarer, but can produce the same issues:

Validator::make($request->all(), [
    'email' => 'required|email:rfc|max:100',
    'firstname' => 'required|string|max:100',
    'surname' => 'required|string|max:100',
])->validate();

Also, you can use a |-separated rules as one long string for defining a set of validation rules, like in the example above. I’m not a big fan of these strings and much more like defining them in arrays, but probably other Laravel users do.

And there are some other similar findings in other Laravel files. These cases mostly occur in one file so that they are much easier to handle:

Route::pattern('userId', '[0-9]+') // for defining allowed inputs for URL parameters (`api/users/{userId}`)
Schedule::command('some-command')->dailyAt('12:00') // for defining scheduled tasks at specific time
8

Hello @mx-jhinz,

Thanks for the added details!
I updated the ticket to include them; it may change which solution to implement.

Thank you again.
Best,
Rudy