Sensor development : How can I access files inside a maven project with "pom" packaging ?

[Michel Pawlak]

Hi,

I'm writing a plugin that has to analyse Maven pom.xml files. It works fine except for one (major) point : while I can access and analyse pom.xml file that are located in non-pom packaging modules, I cannot acces the ones packaged as pom... (such as the project's root pom) 

I tried with the following two predicates but in both cases with no success : 

// This one returns the pom.xml file only... It works on all projects but on those where the pom.xml has a <packaging>pom</packaging>
this.filesPredicates= this.fs.predicates().hasRelativePath("pom.xml");

// Used this one to see if I'm not doing something wrong, but it only showed me that directories with a pom.xml packaged as pom seem to have no files at all
// These directories seem to be simply ignored.

this.filesPredicates= this.fs.predicates().all();

In fact, all files in the root directory of our projects are not reachable with FileSystem. Here is an extract of the console logs, it shows that the pom.xml file of the root is not detected :

[INFO] [08:35:47.054] -------------  Scan foo-module-with-jar-packaging

...

[INFO] [08:35:47.441] Base dir: /path/to/jenkins/workspaces/QA-jenkins/QA_DEVELOPMENT_TEST_JOB/foo

[INFO] [08:35:47.441] Working dir: /path/to/jenkins/workspaces/QA-jenkins/QA_DEVELOPMENT_TEST_JOB/foo/target/sonar

[INFO] [08:35:22.964] Source paths: pom.xml, src/main/java

[INFO] [08:35:27.197] Sensor MavenChecksSensor...

[INFO] [08:35:27.199] Analysing file '/path/to/jenkins/workspaces/QA-jenkins/QA_DEVELOPMENT_TEST_JOB/foo/pom.xml'
...

[INFO] [08:35:27.207] Sensor MavenChecksSensor done: 10 ms
...

[INFO] [08:35:47.716] -------------  Scan root

...

[INFO] [08:35:48.149] Base dir: /path/to/jenkins/workspaces/QA-jenkins/QA_DEVELOPMENT_TEST_JOB

[INFO] [08:35:48.149] Working dir: /path/to/jenkins/workspaces/QA-jenkins/QA_DEVELOPMENT_TEST_JOB/target/sonar
...

[INFO] [08:35:48.149] Sensor MavenChecksSensor...

[INFO] [08:35:48.149] Sensor MavenChecksSensor done: 0 ms
...

Here is the code of the relevant part of my Sensor that dumped this output :

@Override

public boolean shouldExecuteOnProject(final Project project) {
    return true;

}

@Override

public void analyse(final Project project, final SensorContext sensorContext) {

    Iterable<InputFile> inputFiles = this.fs.inputFiles(this.filesPredicates); // here i'm using a "all" predicate

    for (InputFile inputFile : inputFiles) {

        LOGGER.info("Analysing file '{}'", inputFile.absolutePath());

        }

}

What is it due to (FileSystem implementation for instance) and what can I do to have access to these files ?

If it is due to FileSystem implementation, do you think that I would be able to inject my own FileSystem implementation instead of the one provided by the platform ?

Thanks in advance for your help.

Michel

[Julien HENRY]

Hi Michel,

We do not support modules having both files and sub modules. Either this is an aggregator modules having no source and some sub modules, either this is a leaf module with source. This is a very old limitation but I hope we will be able to remove it some day.

++

Julien

[Michel Pawlak]

Hi Julien,

Thanks for your reply. Is there any workaround to this ? (for instance what would happen if I create my own InputFile instance ?)

Can you please create a ticket to add this feature ?

Thanks in advance,

Michel

[Julien HENRY]

I don't know any workaround. Even if you hack on batch side the UI will not display anything..

[Michel Pawlak]

Hi Julien,

Sorry for un-digging this post after 9.5 months, but do you have a roadmap for removing this "very old limitation" ? 

I have plugins with (implemented) features waiting for the platform being able to display information they provide. :-)

Thanks in advance.

Michel

On Monday, July 20, 2015 at 12:03:09 PM UTC+2, Julien HENRY wrote:

[Julien HENRY]

Hi Michel,

This is not scheduled, but still in our plans:

https://jira.sonarsource.com/browse/SONAR-6724

++

Julien

[Loïc]

Hello,

I am also affected by this issue.
I just submitted a pull request : https://github.com/SonarSource/sonarqube/pull/1293

I made my changes on branch 5.6 since I would need them applied on the LTS version.

With my patched version :
- install XML plugin
- activate all rules with tag "maven" in Java profile "Sonar way"
- analyze a multimodule maven project => issues are reported on the root pom as well as "child" ones.

I also tested with some custom rules implementing XPathXmlCheck and PomCheck : associated violations are also reported for the root pom.

More details in the PR.

Comments are welcome,
Have a nice week-end,

Loïc

[Michel Pawlak]

Hi,

I'm undigging this thread again. As a reminder, I asked the question in July 2015. Loïc submitted his PR in October 2016.

@SonarSource can you please give us some information about the status of this topic ? Have you tried to apply the fix Loïc sent you on the 5.6.x branch (not counting tests, it represents a +28/-15 change to the code base, which is not that big) and if yes is there anything wrong with it that would prevent having it applied as is to 6.x ? How can we help ? Do you want us to help ? Is this topic important at all for you ?

This reminds me my PR for analysing test sources: the community provides a PR, you kind of refuse it telling that you plan to work on a better implementation, but as you are under heavy load you can't work on it yet (I don't doubt it) and finally the community waits for a great feature (which anyway could be reimplemented in a better way later, at the pace you want, how you want).

I don't know the opinion of other people, but as a user, I would prefer having a first version of this feature implemented in next 6.x release rather than having 10 new Java rules in the next version of the Java plugin. Releasing a first version (even in a separate branch) would allow us to provide you feedback about what is working correctly or not.

Best regards,

Michel

[Michel Pawlak]

Hi again,

I just checked, and saw that the two (non test) files that were modified in the PR submitted by Loïc were moved to another package, and that there has been no impacting changes since october (only two quality-related commits impacting non involved lines). I'll port Loïc's PR to current codebase version this WE and will start testing it.

Michel

[Julien HENRY]

Hi Michel,

As you may have read in the associated JIRA ticket, the difficulty is not in "unlocking" the feature. The problem rely in our way to configure multi-module projects, and especially regarding inheritance and property validation. Today it is pretty clear: when you set a property on an aggregator module, it means "don't try to apply this property but apply it to all child modules". If we start having modules with a mix of source code and sub-modules, resolving properties like sonar.sources or sonar.exclusions may be difficult since we don't know the intend of the user (or the scanner).

One concrete example: on scanner side our moto is to fail fast in case of mis-configuration. This is important to not people analyze their project, thinking everything is fine, while in fact they miss something. On a single module project (or on a leaf module) if you set sonar.sources=fileA,dirB we will validate that fileA and dirB actually exists. If they don't we fail the analysis. For multi-module projects, if you set sonar.sources=src/main/java on the root module, we currently don't try to "apply" this property to the non leaf modules so it is simply inherited. With the proposed change we will be in an unclear situation. We would apply property with a "best effort" approach, ie look if there is a matching file/directory. If there is one, files will be analyzed. If there is no matches then we will silently ignore and move to child modules.

You may consider it is not that bad, but I can tell you that from a support point of view, having such a behavior could be dangerous.

Another problem I'm thinking of is the potential to include the same file twice (in parent and child module). I'm not sure at all how we will react to that.

All that to say that this is not as easy as merging the P/R :)

++

Julien

[Michel Pawlak]

Hi Julien,

While I'm well aware of the fact that "as is" Loïc's PR may cause issues in some cases, it's a bit more difficult to understand that this topic received not much attention for the last year and a half and it was never really discussed with the community. 

As you wrote in your first message, it is a long known limitation. And its impact is important:  many types of plugins cannot be written correctly as files located at an agregator level in a multi-module context are simply ignored. However these files are central to the build (at least IMHO).

In this context, I think that porting the PR, start testing, report issues, think about them, and discuss would help specify a solution, and would help have (hopefully) quickly a first version covering 80% of common cases. This approach looks like to me a better (or at least more constructive) approach than just continue waiting for the specification SonarSource is working on (again you set your priorities, have limited resources, and I understand it, in return please just understand that other people may have different priorities.)

By the way (and to start the discussion), concerning the issue you mentionned: what about only considering files located at the root of the aggregator level directory as sources (i.e. avoid mixing the aggregator logic with non aggregator logic) ? what about using another property for aggregator level sources (and thus a different property validation) ? what about considering that aggregator level don't have test sources?

Let's discuss about small and unperfect steps toward a great feature!

Best regards,

Michel

[Julien HENRY]

Hi Michel,

 

By the way (and to start the discussion), concerning the issue you mentionned: what about only considering files located at the root of the aggregator level directory as sources (i.e. avoid mixing the aggregator logic with non aggregator logic) ?

Are you suggesting to hardcode "pom.xml" as an exception somewhere :) For me the main use case of this feature is to improve the support of Gradle projects, where "intermediate" modules are really allowed to contain source code (possibly in sub folders). So I would not hack something only to make pom.xml analyzed.

 

what about using another property for aggregator level sources (and thus a different property validation) ?

My opinion is that we should move away from property based communication between scanners (Maven, Gradle, ...) and scanner engine (SQ). We already have several limitations (spaces in path, multi valued properties, ...) that we need to fix. I would rather introduce a new way to pass configuration that would be more hierarchical friendly.

 

what about considering that aggregator level don't have test sources?

Again, that look such a minimal hackish way to support only the pom.xml use case...

FYI I'm pushing to have this topic scheduled for 6.4, but I'm not alone to decide.