Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add cxxlint #791

Merged
merged 2 commits into from
Mar 2, 2016
Merged

add cxxlint #791

merged 2 commits into from
Mar 2, 2016

Conversation

jmecosta
Copy link
Member

@guwirth @Bertk one of the things of using clang was that it could be made into a standalone analyser and reuse in local analysis before files are commit.

in fact, there is not reason why we cant do the same for the cxx plugin as it us. ive create a small console application that can be run to generate issues using the current cxx checks.

the sintax is easy:
java -jar E:\Development\SonarQube\cxx\sonar-cxx\CxxLint\target\CxxLint-0.9.5-SNAPSHOT.jar -f d:\prod\structures\Core\Common\common\Common.cpp -s e:\CxxLintConfig.json

where CxxLintConfig.json is optional and is meant to give more info to the analysis:

{
  "version": "0.1",
  "includes": [
    "abc",
    "adc"
  ],
  "defines": [
    "abc",
    "adc"
  ],  
  "rules": [
    {
      "ruleId": "FileComplexity",
      "status": "Enabled",
      "properties": {
        "max": "1"
      }
    },
    {
      "ruleId": "ClassName",
      "status": "Enabled",
      "properties": {
        "format": "^sdasa[A-Z_][a-zA-Z0-9]+$"
      }
    },
    {
      "ruleId": "OtherName",
      "status": "Disabled"
    }
  ]
}

this will provide issues like this:

d:\prod\structures\Core\Common\common\Common.cpp:40:Unable to find the source for '#include "tool_date.hpp"'.
d:\prod\structures\Core\Common\common\Common.cpp:56:The regular expression matches this line
d:\prod\structures\Core\Common\common\Common.cpp:87:Rename function "LibMain" to match the regular expression ^[a-z_][a-z0-9_]{2,30}$.

ive change this plugin to embeed the jar so, users can download from sonar like this:

http://localhost:9000/static/cxx/CxxLint-0.9.5-SNAPSHOT.jar

i was thinking now to include this in my visual studio extension so i can report all missing issues also from the cxx plugin and not only for vera , cppcheck and so on

what do you guys think?

@guwirth
Copy link
Collaborator

guwirth commented Feb 28, 2016

@jmecosta in general I like the idea of testing the code locally before adding it to the database. Some thoughts from my side:

  • What is the difference between these XXXLint tools and the preview analysis: http://www.sonarqube.org/analysis-vs-preview-vs-incremental-preview-in-sonarqube/
  • There is also SonarLint (http://www.sonarlint.org/eclipse/) which is an own project. So question is if CxxLint should be an own project or integrated in cxx plugin?
  • SonarLint has the advantage that most checks are used from the plugins whereas we have very less checks. In our company we are using only external tools (static code analysers, unit test, coverage, ...) and cxx checks are mostly not in use. I also wondering if it makes sense to extend the checks because other tools are already available (e.g. cppcheck).

But I'm also interested to hear opinion from others...

@Bertk
Copy link
Contributor

Bertk commented Feb 28, 2016

A tool which runs the checks locally will be very helpful but the configuration from the SonarQube server has to be used as well. This is anyway the main stream approach and the developer should have the chance to fix the issue before a commit ist done.
Certainly the C++ community plug-in has only a small number of checks but this can be improved 😉
I started some time ago with the checks and I wanted to replaced Vera++. I feel we should create a backlog for new checks and ask for votes or prioritization.

@jmecosta
Copy link
Member Author

@guwirth regarding preview mode, for the cxx plugin is not really feasible. it takes to long for big projects and new versions of sq dont even allow running agains only modified files. anyway the lint idea is not to compete with those modes. but to provide analysis while you are writing code.

for the keeping this in separate plugin i dont like it, mainly because the linter references a version of the cxx-checks and soon people will start using different versions in server and locally. putting in the cxx-plugin makes the versions always the same. for example the sonar cs lint is embeeded also in the plugin but as a external artifact that will cause what i described earlier.

for the amount of checks the cxx plugin provides, there are a few that are very useful like complexity (this was the reason why i decided to put this) we dont have those in any external tool that can be configured according with the value in the rule. furthermore people are likely not using more the cxx checks because we dont provide a linter to run it locally.

@Bertk the profile can be passed via -s settings. from my side in visual studio i will handle this logic since its wise to set the file only once when solution is open rather than continuously polling the server everytime the linter runs. plus you should be able to run the linter without any access to server. for example a settings file can be exported and keep in the repository and the you build some tool to run the linter using this settings file. I dont think its wise to put more logic other than run the checks and report in the linter.

@guwirth @Bertk i agree with Bert that once this is available locally more people will be interested in creating more checks. and at least replace vera for starting up

@@ -109,6 +109,7 @@
<modules>
<module>cxx-squid</module>
<module>cxx-checks</module>
<module>CxxLint</module>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

proposal to align path name: cxx-lint

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, i notice that :)

integrated into visual studio:

image

@jmecosta
Copy link
Member Author

jmecosta commented Mar 1, 2016

@guwirth @Bertk this is ready. you can try this in visual studio by installing this: https://github.com/TrimbleSolutionsCorporation/VSSonarQubeExtension/releases/tag/5.1.0-RC23

highlights of the visual studio integration:

  • download linter from sonar server if you are using this version
  • syncs rules and parameters values, configures the rules in the linter so that the checks have the same values as in server.
  • on the fly analysis reported in source files.
  • compilation settings are retrived from project files using msbuild automation to get the best results for the linter (all headers should be properly feed to the linter).

feel free to try out and give feedback. i will soon release this to gallery

image

@jmecosta
Copy link
Member Author

jmecosta commented Mar 1, 2016

the windows build is failing, but i think is a issue with appveyour likely lack of space?

And the server log (if locatable) contains no error/warning messages
found warning '2016.03.01 19:22:30 WARN es[o.e.c.r.a.decider] [sonar-1456859822598] high disk watermark [90%] exceeded on [WIQgLbNLQKGb8Qe9QGRD1A][sonar-1456859822598] free: 13.9gb[9.2%], shards will be relocated away from this node
' Assertion Failed: Found following errors and/or warnings lines in the logfile:
2016.03.01 19:22:30 WARN es[o.e.c.r.a.decider] [sonar-1456859822598] high disk watermark [90%] exceeded on [WIQgLbNLQKGb8Qe9QGRD1A][sonar-1456859822598] free: 13.9gb[9.2%], shards will be relocated away from this node

anyway, if this still fails after merging. we can then later add those to the ignore list.

@guwirth
Copy link
Collaborator

guwirth commented Mar 2, 2016

Thanks for providing this.

Folder name seems to be still cxxlint instead of cxx-lint?

Build and too less space: maybe an idea to delete temporary files.

Am 01.03.2016 um 20:34 schrieb Jorge Costa [email protected]:

the windows build is failing, but i think is a issue with appveyour likely lack of space?

And the server log (if locatable) contains no error/warning messages
found warning '2016.03.01 19:22:30 WARN es[o.e.c.r.a.decider] [sonar-1456859822598] high disk watermark [90%] exceeded on [WIQgLbNLQKGb8Qe9QGRD1A][sonar-1456859822598] free: 13.9gb[9.2%], shards will be relocated away from this node
' Assertion Failed: Found following errors and/or warnings lines in the logfile:
2016.03.01 19:22:30 WARN es[o.e.c.r.a.decider] [sonar-1456859822598] high disk watermark [90%] exceeded on [WIQgLbNLQKGb8Qe9QGRD1A][sonar-1456859822598] free: 13.9gb[9.2%], shards will be relocated away from this node

anyway, if this still fails after merging. we can then later add those to the ignore list.


Reply to this email directly or view it on GitHub.

@jmecosta
Copy link
Member Author

jmecosta commented Mar 2, 2016

@guwirth you mean the package name:

package org.codehaus.sonarplugins.cxx.cxxlint;

i tought we did not use - in packages names, and only in the main module name. that one is changed

@jmecosta
Copy link
Member Author

jmecosta commented Mar 2, 2016

@guwirth regarding the build space, perhapps then its better to add those to the ignore list. since it should be quite random to happen

@guwirth
Copy link
Collaborator

guwirth commented Mar 2, 2016

@jmecosta sorry was wrong, saw only the last cxxlint in the path. The root is already cxx-lint.
cxx-lint/src/test/java/org/codehaus/sonarplugins/cxx/cxxlint/CxxLintTest.java
👍

@guwirth guwirth added this to the M 0.9.5 milestone Mar 2, 2016
guwirth added a commit that referenced this pull request Mar 2, 2016
@guwirth guwirth merged commit d01edba into SonarOpenCommunity:master Mar 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants