SonarQubeCommunity / sonar-erlang

12 stars 23 forks source link

Add rebar3 common test, xref and dialyzer report parsers #30

Closed hegedenes closed 6 years ago

hegedenes commented 6 years ago

This pull request is based on https://github.com/SonarQubeCommunity/sonar-erlang/pull/29

These commits include:

I had problems with the license check in the new files as it allows only one contributor in the pom.xml so there is a commit that disables it with -Dlicense.skip=true. Is there any better way to allow more contributors?

kalidasya commented 6 years ago

@hegedenes thanks for the MR I will review it later today hopefully. about the license thing the license plugin uses these variables from pom.xml

    <license.title>SonarQube Erlang Plugin</license.title>
    <license.mailto>kende.tamas@gmail.com</license.mailto>
    <license.owner>Tamas Kende</license.owner>
    <license.years>2012-2017</license.years>

you can just add your name and re-run the license generation (yes it will be in every file) I am fine with that, no need for disabling. also if you do not mind you can just run the format mvn command to get the license headers correct (see http://code.mycila.com/license-maven-plugin/ )

hegedenes commented 6 years ago

Thanks, I updated it with this format:

 * SonarQube Erlang Plugin
 * Copyright (C) 2012-2018 Tamas Kende; Denes Hegedus (Cursor Insight Ltd.)
 * kende.tamas@gmail.com
kalidasya commented 6 years ago

@hegedenes looks ok to me (not really following erlang changes anymore) does merging this means there is no need to merge #29 ? just to make sure I understood the code changes: you added a new hierarchy to parse the new diayzer and xref outputs and added new rules for xref. what is not clear who changed the output dialyzer or rebar? do you see any chance of an old style usecase get broken? like old rebar?

hegedenes commented 6 years ago

Yes, this includes #29 so no need to merge that separately.

The new hierarchy is only for the Xref reports, for dialyzer I only changed the part that matches the lines with sources. rebar3 dialyzer outputs full filenames to the 19.1.dialyzer_warnings file like this for me:

/home/denes/repo/_build/default/lib/someapp/src/someapp_something.erl:208: Record construction ...

And the file pattern was just "**/"+fileName so it searched in full path **//home/denes/... with the _build/<target>/ directory, not among the actual sources.

With bc1c91d the pattern becomes **/someapp_something.erl and it finds the files properly in the source directories regardless of absolute/relative paths in the output and also works with output from other targets like rebar3 as prod dialyzer.

Based on this issue it looks like it used _build/<target>/ in the output 3 years ago too.

kalidasya commented 6 years ago

@hegedenes understood, thanks for the contribution! köszi!