[NEW PLUGIN] ecoCode - Requesting inclusion in SonarQube Marketplace

Hello,

We would like to request addition of the ecoCode plugins to the SonarQube Marketplace.

Description

ecoCode is a collective project aiming to reduce environmental footprint of software at the code level. The goal of the project is to provide a list of static code analyzers to highlight code structures that may have a negative ecological impact: energy and resources over-consumption, “fatware”, shortening terminals’ lifespan, etc.

ecoCode is based on evolving catalogs of good practices, for various technologies.

Available SonarQube plugins

Targeting server languages

Targeting mobile platforms

Thank you

3 Likes

Hi,

Congrats on your plugins!

For the record, the PR is here:

Looking at the bureaucratic requirements it appears that you’ve mis-read the pattern for plugin keys, which is [a-z0-9]. Those dashes aren’t allowed, in the key, they’re there to describe ranges. So I’ll need you to update your plugin keys. The metadata job simply won’t work like they are.

The other bureaucratic requirements look fine.

The PR is also missing something. Specifically, the part where you register your new files/plugins in the master list in update-center-source.properties.

But before you do that, I’ll need to test the plugins. Can you provide or point me to projects I can/should test with?

 
Thx,
Ann

Thanks for pointing out these issues.
I will work with the Green Code Initiative community to fix projectKey of plugins.
I’ll let you know directly via PR SonarSource/sonar-update-center-properties#389

I will also inquire about your need to have a “protocol” for testing plugins. For this point, I will answer you directly in this thread

1 Like

Hi,

I’ve started testing with the Java version of the plugin, using the project you provided (as communicated via GH since I was already commenting in your project).

Thanks very much for providing a project that seems to raise at least one issue on each rule. TBH, I don’t usually give rule-by-rule feedback, but it’s a small set and I was curious to see your rules in action.

Here’s what I saw in my testing.

To Do

IMO these things should be handled on your side, but if you don’t want to, we need to talk about it.

  • For (cnumr-java:S63), the issue message doesn’t match up with the rule title/description:
    Selection_875
    Selection_876
    While I don’t disagree that what you’ve highlighted is bad practice and should be avoided, having an issue and message that don’t match up to the rule will be confusing for end users.

  • Some of your rules seem to have Snnn ids. While I don’t think there will be a direct clash with our own Snnn rule ids, this still makes me uneasy. Would you consider changing those rule numbers to maybe add a character to the prefix? Or a suffix? Or… something?

  • Correct the plugin key. Yes, I know you know this already. I’m including it here so I have one place to go back to with all the things to double-check.

Optional

  • You might want to shorten, and perhaps translate your rule repo name:
    Selection_874

  • You may also want to re-proof-read your rules (emphasis mine)

…never get it during iteration, It saves CPU cycles so unless energy consumption.

  • After looking at only a few of your rules, it strikes me that most (all?) of them are really about performance. I suggest you also tag them with that: performance

  • At least one Noncompliant Code Example (cnumr-javaS78) doesn’t include a //Noncompliant comment. Without it I, as a user, spend a lot of time going back and forth between the two code samples to figure out what changes & why.

  • Also on S78, I find the wording slightly odd: “in batch update” vs… “in a loop”?

  • S69

Do not call a function when declaring a for-type loop in order to avoid function calls each iterations. It saves CPU cycles.

Might want to sharpen this language a bit. Theoretically it’s fine to call a method in the initialization (int i=foo.size()), right?

  • GRSP0028 - there’s really not enough to go on here. In fact, there’s nothing to go on here. This needs a description.

*GSCIL is a subset of S69. In general you only want one rule to raise an issue on a given piece of code. I suggest dropping this rule.

  • S79, about using try-with-resources raises an issue on code where, at a glance, it looks like all the resources are properly disposed. Users will perceive this as a false positive.

Observation

  • You haven’t provided a default profile. That’s totally fine since you’re not handling the basic language implementation. And I wonder if it makes sense to create a profile that’s all of your rules and all of the built-in #performance rules…?
    My thinking here is not that people would necessarily use your profile as-is, but that perhaps they would copy it into their own profiles & thus ensure that all #performance rules got turned on (and hopefully followed!). This is mostly an idle thought on my side, so feel free to completely ignore it.

 
Ann

Huge thank you for your very valuable feedback.

We have fix pluginKey. Now it’s:

We have also fix missing values in update-center-source.properties.

And we are fixing majority of above issues in: Fixes issues raised for adding plugin to SonarQube marketplace by jycr · Pull Request #79 · green-code-initiative/ecoCode · GitHub

Hi,

I’ll take another look at the Java plugin. I still need to test each of the other plugins.

 
Ann

Hi,

I realize now I jumped the gun on re-testing. Let me know when a new version is ready (and no need to update your PR until you’ve got a release that addresses at least the must-fix stuff).

 
Ann

As soon as the “green-code-initiative” community integrates the fixes and releases them in a new version, I will update the PR: Add ecoCode plugins by jycr · Pull Request #389 · SonarSource/sonar-update-center-properties · GitHub

1 Like

@ganncamp : I have 2 questions:

  • As for rule key prefix, we have several “fairly identical” rules like Java/S67 and PHP/S67. Do we need to define different prefixes for each rule repository (one for Java and another for PHP), or do we need to define the same prefix for each rule repository of ecoCode (an identical prefix for the repositories of each of our 6 different ecoCode plugins)?
  • Also, do we need to define properties (name, description, priority, tags) for each of the rules both in the class file (via the @org.sonar.check.Rule annotation) and in the JSON file? Or is it better to just set the key property of the @org.sonar.check.Rule annotation in the class, and the rest of properties in the JSON file?

Hi,

In production, the repo key is prefixed onto the rule key, like java:S123. So there’s no need to differentiate rule key prefixes across languages. In fact, it’s better if the keys line up across languages for what’s essentially the same rule.

That’s an implementation detail I’m slightly fuzzy on, but I believe it’s one or the other, with the JSON files being what’s used internally since we can easily generate them from our rule repo.

 
HTH,
Ann

Hi,

FYI, in case y’all want to chime in:

 
Ann

1 Like

1.1.0 plugin release fixes your issues. I have updated PR#389

Hi,

FYI, I’m digging out from under a few days out of the office. You’re on my list, but not at the top, since I don’t want to be rushed when I come back to this.

 
:smiley:
Ann

Hi,

Here’s what I saw in my new round of testing:

Kudos

  • I really like the energy comparison at the bottom of the PHP rules. This is really nice, and I think it will likely help adoption.

Optional

  • In checking all the plugins’ change logs, I see this:

    • WARNING : since this plugin version, ids of plugin rules changed. In consequence, if you have already made some issue checks in your SonarQube instance, you will have to do them again (example : false-positive issues will appear again)

    There is a mechanism available to migrate rule ids (and even repository names). Using it would mean that all the old issues would be seamlessly migrated to the new rule ids and no manual work, e.g. FP-ing, would be lost. (BTW, I can’t tell you the particulars, but it’s likely visible in the API.)
    Before we put these versions into the marketplace, do you want to take a look at that? It’s up to you.

  • I find it odd that the Java plugin has only Code Smell rules and the PHP and Python plugins have only Bug rules, especially when some of the rules are the same from language to language (e.g. ++i instead of i++)

  • ecocode-php:EC34’s title is “Avoid using try-catch-finally”, but it raises issues on try-catch blocks without the finally clause. The code sample clears it up, but it seems the rule is really “Avoid using try-catch” or perhaps “Avoid using try-catch and try-catch-finally”?

    • Ditto Python.

ToDo

  • For ecocode-java:EC63 (I see you’ve re-keyed it from S63. Thank you!) I see that the issue message still doesn’t make sense:
    Selection_1002

    Explicitly, the variable is assigned. I’m guessing it’s not used…?

  • I’ve mentioned this in the pull request, but

    • The file for the JavaScript plugin gives the download link for the Java plugin.
    • When I correct the download link to point to the JavaScript plugin I get a plugin that doesn’t expose any rules

The JavaScript, PHP and Python plugins all pass the bureaucratic requirements, and there are no showstoppers for PHP and Python. They’re good to enter the Marketplace. But because you’ve ganged all 4 together in one PR, they’re going to have to wait until the Java and JavaScript plugins’ problems get sorted out.

 
Ann

Thank you once again for your constructive and kind feedback.

We are very interested in the rule ids migration mechanism. Where can we find the information to implement the necessary?

I will report the other points to the community to see what can be done.

Hi,

For this I think the easiest thing is going to be to create a new thread under plugin development, probably targeting your Java plugin :wink:.

 
Ann

1 Like

@ganncamp: The green-code-initiative community has released a new version (1.2.0) of their plugins with fixes of the issues/advices you raised:

PR SonarSource/sonar-update-center-properties#389 has been updated.

Existing “test projects” for each concerning language:

NB: ecoCode JavaScript, ecoCode Android and ecoCode iOS will be added in a future version.

Hi,

They look good. Congrats! You’re in!

 
:smiley:
Ann

Aaand… I spoke too soon.

I get an error running the metadata generation job:

The plugin ‘php’ required by ‘ecocodephp’ is missing. Unable to find plugin with key php → [Help 1]

If the other two plugins explicitly “require” their respective language analyzers, we’ll hit this problem with them too. Since the analyzers were rolled more tightly into SonarQube, they no longer appear in the Marketplace. So I need you to update the poms to remove this requirement & issue new releases.

 
:frowning:
Ann

I don’t see this error when I install ecoCode PHP plugin.
How can I reproduce this error?
We just have to remove <basePlugin>php</basePlugin> definition from pom.xml?