[VOTE] Code Smells Plugin 2.0.0

[Michel Pawlak]

Hi SonarQube community,

I would like to release a new version of the Code Smells (formerly known as WTF) Plugin version 2.0.0 and I'm asking you for your feedback.

The scope of this release is the following:

• Plugin renamed from "WTF" to "Code Smells"
• Annotations "@WTF" and "@WTFs" renamed to "@Smell" and "@Smells"
• Cleaned all documentation to align it with the new plugin name
• Ability to change the widget name from "Code Smells" to whatever you want

To test this release you need to :

• retrieve the snapshot version of the plugin: qualinsight-sonarqube-smell-plugin-2.0.0-RC1.jar             
  
• add the Java api dependency as follows:

<dependency>
    <groupId>com.qualinsight.plugins.sonarqube</groupId>
    <artifactId>qualinsight-plugins-sonarqube-smell-api</artifactId>
    <version>2.0.0-RC1</version>
</dependency>

Usage documentation is available on the plugin's GitHub page.

The vote is open for 72 hours. 

[ ] +1

[ ] +0

[ ] -1

Thank you in advance for your feedback !

Michel

[Adam Gabryś]

Hi,

I know it is a little too late, but maybe better to call @Issue and @Issues (or @Wrong/@Wrongs ) instead of @Smell and @Smells.

 

Regards,

    Adam Gabryś

--
You received this message because you are subscribed to the Google Groups "SonarQube" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sonarqube+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/1fc0067b-1a36-4ab5-9764-13c75331b739%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[G. Ann Campbell]

@Adam, I wonder if that would be confusing since what the language plugins raise automatically are also Issues...?

Ann

[G. Ann Campbell]

+1 Michel 

However, I find the use of a 'qualinsight' tag highly questionable. Tags are used to categorize rules/issues, so "bug", "bad-practice", "security", &etc. But "qualinsight" as a category doesn't make sense to me.

OTOH, I haven't quite figured out yet where in the issue the applied SmellTypes are used, but IMO, they would make excellent tags (once converted to lower case, of course :-)

[michel...@gmail.com]

Thanks for your feedback Ann.

Considering the tag, good point I'll remove it.

Considering the SmellTypes, they are not used in issues (only the "reason" and "minutes" fields are used to create a Code Smell issue), but as metrics (which in turn are used to display the widget). This could indeed be improved in a later version of the plugin.

Michel

[G. Ann Campbell]

Mmm... I missed the widget initially. 

Now that I try to use it, though, I get this:

2015.10.20 08:00:11 ERROR web[rails] Can not render widget smells-widget-id: undefined method `formatted_value' for nil:NilClass

I also missed the metrics. Now, even though I have relevant issues:

I have blank metric values. 

I'm testing in 5.2 RC.

---

G. Ann CAMPBELL | SonarSource

Product Owner

http://sonarsource.com

[Michel Pawlak]

Hi Ann,

Thanks for this feedback. Unfortunately I haven't tested the plugin with SQ 5.2 RC yet. I'll try to do it this evening. Meantime I can confirm that I wasn't able to reproduce the error on 4.5.4 / 5.1.2. I added a check to only display the widget if code smells have been detected and will publish it this evening.

Michel

[Michel Pawlak]

Dear Ann,

I confirm that custom Sensor and Decorator that compute Code Smell metrics are not run at all with SQ 5.2-RC3 and there is no error message, and debug displays nothing. To be honest, I don't know what I should do to make it run on this version, nor how to do it in a backward compatible way.

Do I have to release 2 different versions of my plugin ? (one for SQ < 5.2 and one for SQ >= 5.2) 

Any information on this topic from SQ would be great.

Michel

On Tuesday, October 20, 2015 at 2:08:26 PM UTC+2, G. Ann Campbell wrote:

[G. Ann Campbell]

Whoops! Original send didn't include the group:

In fact Michel, I'm working on the 5.2 docs now (we're in the final stages of that phase as well) & just got to this part:

Typically, plugins that do the following will need to be removed from your installation:

• Aggregation of raw metrics into higher metrics (implemented with a Decorator class)
• Computation of measures that require access to the history of the project
• Code that tries to access the database for whatever reason

Someone from the core team may step in and correct me, but I think you just don't have the ability any more to aggregate metrics at the end of an analysis.

If it were me, I'd drop the metrics & go with rule/issue tags. Then you could provide a pre-rolled widget that gave tag counts similar to the Security Issue Tags widgets in Widget Lab. (Screenshot from the front page of Nemo):

(Yes, I should format the numbers, it just never crossed my mind that I'd have security issues in the thousands!)

---

G. Ann CAMPBELL | SonarSource

Product Owner

http://sonarsource.com

--
You received this message because you are subscribed to a topic in the Google Groups "SonarQube" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/sonarqube/aKvLypA1M7M/unsubscribe.
To unsubscribe from this group and all its topics, send an email to sonarqube+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/8a906f59-5206-4e8f-82ba-4b0a28a6d446%40googlegroups.com.

[Michel Pawlak]

As there are issues on 5.2-RC and as I would like to find a way to fix them, I put this vote on hold.

I'll provide a RC3 version asap and will restart the vote then.

Michel

[Simon Brandhof]

Hi Michel,

As the plugin uses the Decorator extension point, it can't be compatible with SQ 5.2 and previous versions at the same time. A decision must be taken : [4.5.4,5.1.x] or 5.2+.

Regards

Simon BRANDHOF | SonarSource

Tech Lead & Co-Founder
http://twitter.com/SimonBrandhof

--

You received this message because you are subscribed to the Google Groups "SonarQube" group.

To unsubscribe from this group and stop receiving emails from it, send an email to sonarqube+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/11895a2a-ee76-49b0-8a64-c2150ce64dc3%40googlegroups.com.

[Michel Pawlak]

Hi Simon,

Isn't it possible to implement both org.sonar.api.batch.Decorator and org.sonar.api.ce.measure.MeasureComputer ?

Both interfaces are part of sonar-plugin-api version 5.2 if I'm not mistaken, and Decorator seems simply ignored on SQ 5.2 (and I guess tha MeasureComputer will be similarily ignored on SQ version < 5.2).

Regards,

Michel

[Julien Lancelot]

Hi Michel,

In theory it would be possible, but it's not recommended as a lot of things have changed between 4.5 and 5.2.

If you try, don't be surprised of any unexpected behaviour / error.

It's far much better to have too version of your plugin, one for 4.5 and one for 5.2.

Regards,

Julien LANCELOT | SonarSource

http://sonarsource.com

To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/14f73af4-aa69-48e9-8ed0-198df825275c%40googlegroups.com.

[Michel Pawlak]

Hi Julien,

Thanks for your answer. Two questions:

• Does it mean that you'll provide a specific version aimed at SQ 5.2 of all your plugins that used Decorators so far ?
• If no, does it mean that once 5.2 will be out only plugins written explicitly for SQ 5.2 will be updated ?

Kind regards,

Michel

[Simon Brandhof]

• Does it mean that you'll provide a specific version aimed at SQ 5.2 of all your plugins that used Decorators so far ?

Only the Views Portfolio plugin used Decorator at SonarSource. It will be indeed released again for only SQ 5.2+.

There's still a potential solution that needs to be tested. API 5.2-RC3 does not provide Decorator class at all. But let's imagine that we bring it back for 5.2-GA, just for compilation (not used at runtime). Then your plugin can :

• depend on API 5.2
• have two distinct implementations of Decorator and MeasureComputer (not the same class implementing both interfaces)
• SonarPlugin#getExtensions() uses reflection to know which implementation to be returned:
• if MeasureComputer class exists at runtime, then return the MC implementation   
• else return the Decorator implementation

The remaining problem is the minimal required version of SQ declared in the plugin manifest. It's currently automatically set by sonar-packaging-m-p by checking the version of API dependency. In this case value would be 5.2, whereas you want 4.5.x. I have to check sonar-packaging-m-p if the version can be overridden.

This approach is still risky. APIs are designed to be forward-compatible as much as possible, but not backward-compatible. Obviously using API 5.2 does not ensure that the plugin works correctly with previous versions.

I recommend to not go into this direction. Release your plugin for [4.5,5.1] then start a new development for 5.2+.

My 2 cents

[Michel Pawlak]

Hi Simon,

I just checked the master on GitHub and could find the Decorator class in org.sonar.api.batch.Decorator Your Versioning and API deprecation page states that deprecated classes are removed in major version +2 Was the Decorator interface marked as being deprecated in SQ 3.x ? http://docs.sonarqube.org/display/DEV/Versioning+and+API+Deprecation

Concerning the approach you're describing this is more or less what I was planning to test :

• depend on plugin api 5.2
• have two classes (one implementing Decorator, one implementing MeasureComputer)
• depending on SQ version (this can be retrieved through Settings if I'm not wrong) decide the behavior of each of these two classes

However I wasn't expecting the minimal version of SQ required by the plugin to be computed based on the sonar-plugin-api version. It would be great to allow plugin developers to specify manually such compatibility: developers have to check.

Indeed, IMHO, systematically forcing plugin developers to write, release and maintain multiple versions of their plugins if they want to support both new and older versions of SQ is IMHO not a good move. We are not talking about using features provided by a long forgotten version, but about current LTS version. 

Cheers, 

Michel

[Michel Pawlak]

By the way, it would be great to be able to specify a version compatibility range, not only a minimal version.

Michel

[Simon Brandhof]

I just checked the master on GitHub and could find the Decorator class in org.sonar.api.batch.Decorator 

My bad, I didn't verify my thoughts. 

 

Your Versioning and API deprecation page states that deprecated classes are removed in major version +2 Was the Decorator interface marked as being deprecated in SQ 3.x ? http://docs.sonarqube.org/display/DEV/Versioning+and+API+Deprecation

Here it's a technological breaking change. In the beginning of the sprint we believed that support of Decorator would be possible but it was definitely not. The choice between postponing the db cut to end of 2016 or keeping backward-compatibility of an extension point was obvious. Improving the software has, and will always have, priority.

For the record support of multi-languages projects in version 4.2 was quite a similar context and the generated value was huge.  

 

 

It would be great to allow plugin developers to specify manually such compatibility: developers have to check.

I know that I suggested that, but that is still risky. Compilation does not enforce compatibility anymore. It does not worth it IMO. Applying patches to a LTS branch is more sane and simple.

  

Indeed, IMHO, systematically forcing plugin developers to write, release and maintain multiple versions of their plugins if they want to support both new and older versions of SQ is IMHO not a good move. We are not talking about using features provided by a long forgotten version, but about current LTS version. 

"systematically" is not fair :-) 

[Simon Brandhof]

I have feedback about usage of this measure aggregation API in the code smell plugin. 

Computing issue-based measures in plugins is usually a non-sense. I already saw in the past some plugins that compute the numbers of checkstyle or findbugs issues, or even worth, sometimes the same numbers per severity (number of blocker checkstyle issues, ...). They came with specific widgets, which are more or less always the same. That could have make sense at a certain point of time with past releases of SQ, but now I'm quite sure this approach can be improved.

A generic widget based on issue tags and issue filters could do the stuff and bring lot of value. This idea should be challenged, but I'm sure that something nice can be done by simply tagging your issues with "code-smell" (it that the case already ?). That would remove the need for Decorator or MeasureComputer APIs.

My 2 cents

Simon BRANDHOF | SonarSource

Tech Lead & Co-Founder
http://twitter.com/SimonBrandhof

[Michel Pawlak]

Hi again Simon,

Thanks for your feedback.

The point is that I want to have both a distribution of code smells by type and a single Issue type. In such a context with a "code-smell" tag I will be able to count all issues, but not count them by type. 

The reason for having a single rule for all code smells types comes from the discussions I had with developers, discussions that led to the creation of the plugin. These people wanted to be able to say "hey there is a problem here", they also wanted to see these issues as a whole without having details in the issues list. They also wanted to be able, to have the global picture through a code smells distribution chart and have the ability to drill down through the widget in order to locate these smells if they were asked to. The underlying reason for this is that most of such manually declared smells have to be discussed first. IMHO it's pointless to add 20+ rules just to inform people that they should talk about these smells during next sprint planning or during coffee break. Last but not least, I want to be able to have trends and differential views on these code smells types, which would not be possible without computing metrics (or maybe I'm wrong).

By the way, using tags doesn't prevent other plugins from using the same tag, which in turn can make displayed information irrelevant (unless i cross check the issue type before counting the issue, but what should be simple becomes then quickly complicated and this is not what this plugin is supposed to be.)

Cheers,

Michel

[G. Ann Campbell]

@Simon, can tags that aren't applied to a rule be applied at issue creation time? I.e. individual smell tags?

@Michel, I understand your fear that other people would use your tag, but it doesn't make sense to me in the context of how the SonarQube ecosystem works. I have the ability to add or remove any tag I choose to a rule in my SonarQube instance. If some other plugin attaches "your" tags to its rules and that's not what I want as an administrator, I can easily fix it. And OTOH, I can add your tags to rules that didn't have them originally.

Either way, the tag is still an aggregation of what I and my users consider to be relevant to that smell. And as tool makers, I think empowering our users to make the best use of the platform for their contexts is a good thing.

my $.02

Ann

[Michel Pawlak]

Hi @Ann, 

IMHO it's better not to have to rely on tools administrators to fix plugins incompatibilities. I would prefer to avoid such incompatibilities before problems occur. Further, while I understand you want people to use tags, I still don't understand how I can have trends and differential views without being able to compute metrics (this is a requirement).

I think it would be great if you could provide a documentation page where you describe your philosophy about tags usage versus metrics usage. This would allow people to better position themselves.

Kind regards,

Michel

[Michel Pawlak]

Vote passed. Thanks for all your feedbacks. Release is ongoing.

Version 2.0.0 will only support partially SQ 5.2, a ticket has been created in order to fix this issue soon.

I suggest continuing discussions and implementation suggestions on a dedicated Google group: https://groups.google.com/forum/#!forum/code-smells

Cheers, 

Michel