Drupal sonarqube results are not Drupal compatible

We are using sonarqube 9.9 and we have a drupal 9 project configured to use the Drupal recommended quality profile sonarway.
If we apply all those rules Drupal code will not be readable by a Drupal developer and furthermore if we run the sonarqube rules over Drupal core (which is now excluded, since it comes from the community and is not touched by us), Drupal core and contributed modules will fail.

A very simple example is form building using Drupal form API.

Sonarqube does not allow us to use Drupal recommended way to create forms, as it raises issues using the form API attributes.
For example in this form item:
$form[‘afieldset’] = [
#type’ => ‘fieldset’,
Define a constant instead of duplicating this literal “#type” 14 times.
#title’ => $this->t(‘The title’),
Define a constant instead of duplicating this literal “#title” 14 times.
#collapsible’ => TRUE,
#collapsed’ => TRUE,
];

it raises issue like this one : Define a constant instead of duplicating this literal “#type” 14 times, because #type is used on other fields! Are we supposed to create constants for the attributes that Drupal has to build the form Drupal way???

Is there any ruleset based on he codesniffer ruleset that is more Drupal >9 wise??

2 Likes

Hi Margarita,

Also looking for an solution/best practice to the same question. The release tools that I use went from LTS 6 version to LTS 9. At one point, I found 1400+ new code smells! Clearrly, too much to expect anyone to wade through to find the code nuggets that should be addressed

The Drupal development standards are out of alignment with SonarQube or perhaps I should say SonarQube results are out of alignment with Drupal. This is a “no win” situation if one codes to please SonarQube as the result is not as Drupal developers would expect to and will be less readable. Furthermore, any code contributions will be rejected by Drupal community.

Here are some of personal favorites:

  • Placement of open/close curly brackets
  • TRUE/FALSE (Drupal) or true/false (PHP)
  • NULL constant expected to be in lowercase
  • Variable naming conventions. Usually find underscores which are allowed getting flagged.
  • Cognitive complexity seems to be too strict. I am on the fence with this one but it usually involves obfuscating code. To reduce CC, I ended up using return statements in some JS. I was taught to avoid goto statements and I consider return a form of the same.

Current solution is just to increase the number of code smells allowed.

AdvThanksance!!!

2 Likes

Hello,

FYI, the Drupal default Quality Profile that we were providing was dropped in Sept 2022 (SONARPHP-1324). If you still have a Drupal QP in your SonarQube instance or one inspired by this old Drupal QP, we advise you to remove it, or at least configure your projects to use the “Sonar Way” one.

That said, we need to do an effort to be sure our PHP rules, especially the Maintainability ones behave correctly in special contexts such as Drupal.

I know this thread is quite old but I’ll try.

What is the most unproductive/useless PHP rule raised by Sonar on your Drupal projects? Why?

Thanks
Alex

1 Like

Two that Kevin mentioned are a good start. Citing them here by rule:

  • php:S100 - This rule is correct for class method names. Function names not part of a class should follow the regex: ^[a-z][a-z0-9_]$ (link)
  • php:S1781 - Drupal’s coding standards invert this rule. TRUE, FALSE and NULL are correct. (link)

Hello,

In order to still provide a good out-of-the-box experience for users, we decided that the rules should automatically adapt to the convention of the context if it is known. This means that users who use a framework that has its own coding and formatting conventions will find an analysis in which the rules adhere to these conventions.
Specifically, the behavior of rules should adapt if we detect that the analyzed code is using the Drupal framework.

We’re currently in the process of specifying these changes, so suggestions about which rules to change to a context-based approach are welcome.
The rules php:S100 and php:S1781 are already added to the specification based on your suggestion.

We plan to implement the changes during the next development iteration of the PHP Analyzer.

Best,
Jonas

4 Likes

I’m also interested in adapting the php:S1192 rule in Drupal Form API context. Might not be easy though, but maybe remove the rule for array keys ?

Hi @McDuck,

Would you mind starting a new thread for this with all the details, please?

 
Thx,
Ann