[MPMD-399] Incorrect warning: The project X does not seem to be compi…

[michael-o]

…led. PMD results might be inaccurate.

Following this checklist to help us incorporate your
contribution quickly and easily:

• Make sure there is a JIRA issue filed
  for the change (usually before you start working on it). Trivial changes like typos do not
  require a JIRA issue. Your pull request should address just this issue, without
  pulling in other changes.
• Each commit in the pull request should have a meaningful subject line and body.
• Format the pull request title like [MPMD-XXX] - Subject of the JIRA Ticket,
  where you replace MPMD-XXX with the appropriate JIRA issue. Best practice
  is to use the JIRA issue title in the pull request title and in the first line of the
  commit message.
• Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• Run mvn clean verify to make sure basic checks pass. A more thorough check will
  be performed on your pull request automatically.
• You have run the integration tests successfully (mvn -Prun-its clean verify).

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

• I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004

• In any other case, please file an Apache Individual Contributor License Agreement.

[adangel]

I added this warning back then, because users were configuring pmd plugin in a multi-module project with aggregate=true, which has the following effect: PMD is run on the parent project first, then the modules are built - which is simply the wrong order. PMD needs to be run after the project is build.

See my comment here: https://issues.apache.org/jira/browse/MPMD-277?focusedCommentId=16814718&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16814718

If we remove the warning, we should also remove it from the FAQ:

maven-pmd-plugin/src/site/fml/faq.fml

Lines 102 to 135 in 70fcdd0

	<faq id="typeresolution aggregate">
	<question>
	What does the warning "The project xyz does not seem to be compiled. PMD results might be inaccurate." mean?
	</question>
	<answer>
	<p>
	In order to improve PMD's results, type resolution should be used. It is enabled by default
	(property <a href="pmd-mojo.html#typeResolution">typeResolution</a>) and helps to avoid false positive
	findings by matching the exact types of method parameters or variables.
	</p>
	<p>
	However, this requires that the project is built first, so that not only the project's dependencies
	can be used for type resolution, but also the project's classes as well.
	</p>
	<p>
	When using the property <a href="pmd-mojo.html#aggregate">aggregate</a>, this is problematic: With
	aggregate=true, PMD is executed at the root of a multi-module project <em>before the individual
	modules are built</em>. Then the types of the individual projects are not available, which might lead to
	false positive findings e.g. for the rule "UnusedPrivateMethod".
	If this might be the case, then the warning "The project xyz does not seem
	to be compiled. PMD results might be inaccurate" is issued.
	</p>
	<p>
	In order to use type resolution and aggregate together, maven needs to be execute in two passes:
	First pass will compile the projects (e.g. <code>mvn clean package</code>) and the second pass
	will execute PMD without clean via the verify phase (e.g. <code>mvn verify</code>).
	</p>
	<p>
	Since version 3.15.0 the new goal <a href="aggregate-pmd-mojo.html">aggregate-pmd</a> can be used
	which allows to run everything with only one maven call. However, this goal invokes the lifecycle
	<em>test-compile</em> before executing itself, which might lead to duplicated execution of some
	plugins.
	</p>
	</answer>

Maybe the warning should only be output, when the executed goal is pmd? The problem is, users can run "mvn clean pmd:pmd" and get false positives/negatives. Alternatively PMD could throw/report errors, if it doesn't find any classes it needs during analysis. I think, currently we don't report these.

On another note: the parameter typeResolution should probably be deprecated as PMD doesn't make sense anymore without typeresolution. There was a time, when this was a new feature and not widely used by rules, but today most rules depend on that (and the feature is always enabled in PMD but doesn't work correctly if the auxclasspath is not provided or empty).

[michael-o]

Let's deprecate typeResolution first with an explanation. That is cheap to do.

[michael-o]

I see. Maybe this goal should fork compile or test-compile?

[michael-o]

I have the feeling that the goals need a logical cleanup with non-agggregate/aggregate compared to other plugins which need to for something...

[adangel]

I don't know if that's possible without breaking anything... I think it has to do, that the goal pmd is also a reporting mojo (that was probably the very first feature) as well as creating the pmd.xml file with the report, that is used by the goal verify to maybe fail the build (verify forks pmd) (and this feature was probably added on top just reusing the reporting mojo...). Not sure how other plugins deal with this (e.g. checkstyle, spotbugs, ...). What I've seen is, that m-pmd-p tries to avoid being called multiple times (which makes sense, it needs to run only once; see the canGenerateReport method which actually executes PMD, and executeReport only renders the result).

If we would add the fork to compile or test-compile, would the compile phase be called multiple times? e.g. mvn verify -> compile -> pmd:check >> pmd:pmd >> compile?

For the concrete issue MPMD-399, I would either remove the warning message (and ignore this whole discussion: if m-pmd-p is used correctly, there are no problems). Or try to improve the logic, when we issue the warning (we can easily fix the target/classes and/or target/test-classes is missing/empty - but there might be other cases. Maybe there is a different way to figure out, if the project has been compiled already).

[michael-o]

I doubt that the goals can be made right without breaking something. We also should split report and validation goals. We should move this to a separate dicussion. Too broad for now.

If we would add the fork to compile or test-compile, would the compile phase be called multiple times? e.g. mvn verify -> compile -> pmd:check >> pmd:pmd >> compile?

Could be the case w/o an analysis.

For the concrete issue MPMD-399, I would either remove the warning message (and ignore this whole discussion: if m-pmd-p is used correctly, there are no problems). Or try to improve the logic, when we issue the warning (we can easily fix the target/classes and/or target/test-classes is missing/empty - but there might be other cases. Maybe there is a different way to figure out, if the project has been compiled already).

One the problems, as layed out in MPMD-399, is that if the param is set in the parent and not all plugins have either/proper combo there will be always a warning. For now, I would not put too much logic into it. You know PMD best. If you think that the warning should stay as a safe guard I am fine with that. I trust you, if you think that the confusion is not worth the warning, we should remove it.

[adangel]

Let's remove the warning. In MPMD-399 m-pmd-p was already correctly called (via the extra aggregate-pmd goal).

I've updated the FAQ entry to remove the warning message and linked from the (already deprecated) aggregate property to this FAQ entry. That should be sufficient.