Introducing Clean Code in our products

Clean Code Sonar Updates
, , ,
1

Our thinking has evolved based on feedback from our Community. While much of this post remains relevant, some details have changed.

Hello everyone,

You might have noticed we relaunched our website and started to talk about Clean Code. We are now introducing a detailed definition of Clean Code in Sonar products.

What is Clean Code?

Clean Code is consistent, intentional, adaptable, and responsible.

These attributes define what Clean Code is all about. Going forward, all Sonar products will adopt the Clean Code attributes as a way to more precisely categorize issues in your code.

This complements our current approach, where Sonar lets you know that code issues affect your software’s security, reliability, and maintainability. And since a single code issue can impact one or more software concerns, we are now referring to these aspects collectively as software qualities.

Starting with the upcoming product releases, issues in your code will have an associated Clean Code attribute and highlight the impact they have on different software qualities with three levels of severity: low, medium, and high.

In future iterations, we plan to extend software qualities to reflect the breadth of our analysis: for example for accessibility, sustainability, safety, and more.

Clean Code attributes

Consistent

The code is written in a uniform and conventional way. All the code looks similar and follows a regular pattern, even with multiple contributors at different times. Consistent code is formatted, conventional, and identifiable.

Intentional

The code is precise and purposeful. Intentional code reads like it is written with attention and care to convey its purpose. Every instruction makes sense, is adequately formed, and simply communicates its behavior. Intentional code is clear, logical, complete, and efficient.

Adaptable

The code is structured to evolve easily and with confidence. It makes extending or repurposing its parts easy and promotes localized changes without undesirable side effects. Adaptable code is focused, distinct, modular, and tested.

Responsible

The code takes into account its ethical obligations on data and the potential impact of societal norms. Responsible code is lawful, trustworthy, and respectful.

Clean Code attributes in Sonar products

sonarqube-issue-list
sonarqube-issue-list1546×848 165 KB

Screenshot from SonarQube 10.2 issues list - changes are highlighted in green.

sonarqube-issue-detail
sonarqube-issue-detail1590×905 200 KB

Screenshot from SonarQube 10.2 issue details - changes are highlighted in green.

sonarcloud-issue-list
sonarcloud-issue-list1600×859 169 KB

Screenshot from SonarCloud issues list - changes are highlighted in green.

sonarcloud-issue-detail
sonarcloud-issue-detail1472×790 173 KB

Screenshot from SonarCloud issue details - changes are highlighted in green.

sonarlint-issue-list
sonarlint-issue-list1600×433 321 KB

Screenshot from SonarLint - changes are highlighted in green.

Issue type deprecation and severity changes

As part of this change, we will be deprecating and gradually removing our previous notion of issue type (bugs, vulnerabilities, and code smells). Also, severity will have simplified values: low, medium, and high. Additionally, we are extending severity to multiple severities per issue, one per software quality.

All existing features and integrations that rely on issue type and severity will continue to work as before, and future changes will be introduced gradually.

Next steps

We still have lots to do to make Clean Code more visible in Sonar products. Stay tuned for more updates.

Until then, please feel free to provide feedback!

15 Likes
3

This really reads like the wet dream of every honorable developer. Though it might also drive aside of reality a bit.

This approach will definitely work is a big project, where coding guidelines are strict and to be strictly applied by everyone, where readability and maintainability is cranked up to maximum, because third and fourth persons need to be able to read and maintain any part of the code. But in smaller projects (which are actually not that small), where a single developer is involved, more freedom is usually granted to coding as long as this developer is able to read and maintain. From my experience it is hard to get developers to apply to very strict rules in this situation. Yes, I know, it is wrong. But it’s also reality.

That being said, I don’t think, this plan will actually work in these single dev projects. Correctly configured I would love it though. And for large projects the current rules and categories are good enough. They will force you in a direction, where the outcome will be clean code.

Well, just my few cents.

2 Likes
6

Good Point @mfroehlich :slightly_smiling_face: Ultimately, it’s up to the Sonar team to make the software and the Quality Gate adaptable to the project requirements without losing the principal benefit (easily accommodating change) and not vice versa.

7

Hi Marvin,

Coming back to your comment, regarding more freedom in some cases, I’m wondering what you mean specifically.

Do you mean when enforcing rules? In picking what rules matter to you? Something else?

8

Hi Gabriel,

well, it’s difficult to list all of them. But I will try my best without claim of completeness.

  • My very convinced and thought-through, but still personal opinion about curly braces is, that they must and can only be next line, having the opener in the same column as the closer. And this applies to all of them, including else and catch. But in the same team there can be developers with other ideas and opinion, some with strict same-line-braces, some with mixed concepts or simply from the stomach. And I know, this is a heated up discussion. So, you would push some devs out of the team, if you wanted to force them to use the one rule or the other. As a result, granting some freedom here can be a good thing for the team, even if it is hard to accept for me as a code-Monk.

  • Another thing is about naming. There are developers, who use a different naming scheme like underscores instead of camel case.

  • Of course there are lambdas. Some devs hate them, some don’t like them. Many love them. I am on none of these sides. I like them for a lot of tasks. But sometimes they lead to reduced comfort using the debugger at least in Java/Eclipse. So for larger bodies a closure can still be a good option. So, a strict rule would slap many devs into the face.

  • I think, code needs to and must be uniform and conventional with respect to certain rules inside of a defined scope (sub project or what ever). But across these scopes style can differ to some extend, so different devs can code a bit different. Again, as a code-Monk I don’t like that. But as a realist I know, it is necessary.

9

Hi Marvin,

I fully agree with all these points you raise.

I think Sonar should help both maintain consistency in the codebase and accommodate different codebase styles, to a reasonable degree. I also agree that “one true style” remains theoretical and in practice, local consistency is often the pragmatic compromise.

Currently in Sonar, this comes in the form of different rules, sometimes opposed, and you pick your preference depending on the circumstances. We also have some rules that can pick up hints from the context and hopefully do the right thing. I think a mix of this makes a lot of sense.

I don’t see this freedom going away!

1 Like
10

Although the general idea of clean code is good, the way it is implemented right now breaks our whole workflow which feds me up as a paying customer.

Two problems we have right now:

  1. We had a strict policy to only allow INFO severity issues on master, not a single issue with worse severity came on master. Now we have limitations to Maintainability of Grade A which means 5% is allowed. We had to add extra conditions to circumvent this, but how I understand those extra conditions will get removed in the future too.

  2. Certain rules are set to INFO severity. So we want to keep track of them in sonar, but they are not relevant for any quality gate.
    e.g: java:S1309 Track uses of “@SuppressWarnings” annotations
    If you just add suppressWarning in a PR this PR gets a maintainability rating worse than A and can not be merged.

To be honest those clean code changes made in Sonarqube especially for quality gates are kind of arrogant because they imply you know what is the best definition for code quality.

4 Likes
11

Hi,

i gave some feedback to the introduction of clean code here

Somehow those threads overlap.

Gilbert

1 Like
12

Hi Michael,

I’m coming back to your comment after a while.

To clarify, Clean Code in our products, as far as this announcement, is about how issues are classified and it shouldn’t disturb your Quality Gates.

Regarding the topic of customizable severity, which I assume you are referring to, it has been discussed in another thread. Maybe this is your main concern?

As for your Quality Gates configuration, I’m sorry that you feel SonarQube is somehow arrogant in terms of enforcing one view of code quality. I think if you open a dedicated thread you’d get help for your particular use case.

13

Hello Gilbert,

Regarding your comment in the other thread, I understand you were not consulted about this change. We do regular user research and this was not an exception.

Also, keep in mind many product changes come from Sonar’s vision and not necessarily from user input. Think about analysis innovation, we invest a lot in finding things before you do.

As to the motivation for this change of classification, we published a blog post on the topic.

Regarding your point about everyone calling static analysis findings “bugs” or “vulnerabilities”, I think that’s fair.

At the same time, I feel that what we really mean is code issues that might negatively impact the runtime or security, respectively. Some of them might certainly do, and some of them may be in the future. Nevertheless, these are things we don’t want in the code.

After thinking about this for a long time, we think making this explicit makes sense. So we came up with a new way to classify issues.

Are you maybe implying that you personally see no value in this new classification? Or that you prefer the previous one? I’m happy to hear your views.

14

Hello Gabriel,

In my opinion this is less about those new classifications. It is more about deprecating something that is relied upon and, in that sense, your last questions above shift the focus. I think, the valid points, made before, focused more on breaking things on the way. Things that are probably not that present in your field of view. Like how your product shapes workflows at/with your (potential) customers.

This is banking “on the budget of trust/good will” 
 the more ppl that are “getting invested” in analysis innovation, the higher the ratio of (possibly breaking) changes to the workflows of (potential) customers will be.

More possibly breaking workflows at (potential) customers mean less “budget of trust/good will”.

In the end it might be better to persuade/convince, than to force. (or make it optional, but enabled by default so that it is clear what Sonarsource’s POV is on that topic)

//one last edit: i personally prefer agency, when deciding how to live our workflow. guidance is very helpful and valued, though! But agency to decide and act on my own behalve is valued higher.

2 Likes
15

Hi Daniel,

I agree very much with what you share here.

about deprecating something that is relied upon

I’m assuming you are referring to the custom severity feature here, and how it can be used in several ways to indirectly set quality gates per rule per project, and much more.

There are many reasons why this was effectively a stretch of that feature, that breaks the product in subtle ways. For example, lowering severity gives a false sense of security in Ratings and Portfolios. Not something we take lightly.

However, we now know the underlying needs and workflows are super important to a part of the community (I’m assuming you too), so probably these should be first-class workflows in the product and not a stretch.

Unfortunately, it took us breaking this for you and then restoring it, to learn about how important the underlying workflows are to some of you.

Things that are probably not that present in your field of view. Like how your product shapes workflows at/with your (potential) customers.

I think that is fair, although we spend a lot of time in the Community and engage in many ways (support, sales, interviews, surveys, telemetry, conferences, etc). As you eloquently put it, it is in our interest to know how people are actually using Sonar and what matters.

Happy to discuss more.

2 Likes
16

Our thinking has evolved based on feedback from our Community. While much of this post remains relevant, some details have changed.

1 Like