Variable vs. literal confusion

[Stephan202]

Consider the following POM. It defines two identical dependencies in its <dependencyManagement> section, but in one case the groupdId is specified as a variable, while in the other case it's specified verbatim:

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">

    <modelVersion>4.0.0</modelVersion>

    <groupId>com.lewisd</groupId>
    <artifactId>example</artifactId>
    <version>1.0-SNAPSHOT</version>

    <dependencyManagement>
        <dependencies>
            <dependency>
                <groupId>${project.groupId}</groupId>
                <artifactId>lint-maven-plugin</artifactId>
                <version>0.0.8</version>
            </dependency>
            <dependency>
                <groupId>com.lewisd</groupId>
                <artifactId>lint-maven-plugin</artifactId>
                <version>0.0.8</version>
            </dependency>
        </dependencies>
    </dependencyManagement>

    <dependencies>
        <dependency>
            <groupId>com.lewisd</groupId>
            <artifactId>lint-maven-plugin</artifactId>
        </dependency>
    </dependencies>

    <build>
        <plugins>
            <plugin>
                <groupId>com.lewisd</groupId>
                <artifactId>lint-maven-plugin</artifactId>
                <version>0.0.8</version>
                <executions>
                    <execution>
                        <id>validate-pom</id>
                        <goals>
                            <goal>check</goal>
                        </goals>
                    </execution>
                </executions>
                <configuration>
                    <rules>
                        <excludes>
                            <exclude>OSS*</exclude>
                        </excludes>
                    </rules>
                </configuration>
            </plugin>
        </plugins>
    </build>

</project>

Running mvn com.lewisd:lint-maven-plugin:0.0.9-SNAPSHOT:check on this POM causes the following stacktrace to be logged:

[WARNING] 
java.lang.IllegalStateException: Found 2 objects using path dependencyManagement/dependencies[groupId='com.lewisd' and artifactId='lint-maven-plugin' and type='jar' and not(classifier)]
    at com.lewisd.maven.lint.rules.AbstractReduntantVersionRule.resolveObject(AbstractReduntantVersionRule.java:137)
    at com.lewisd.maven.lint.rules.AbstractReduntantVersionRule.tryResolveObject(AbstractReduntantVersionRule.java:87)
    at com.lewisd.maven.lint.rules.AbstractReduntantVersionRule.checkForRedundantVersions(AbstractReduntantVersionRule.java:50)
    at com.lewisd.maven.lint.rules.basic.RedundantDependencyVersionsRule.invoke(RedundantDependencyVersionsRule.java:56)
    at com.lewisd.maven.lint.RuleInvoker.invokeRule(RuleInvoker.java:21)
    at com.lewisd.maven.lint.plugin.CheckMojo.executeRule(CheckMojo.java:181)
    at com.lewisd.maven.lint.plugin.CheckMojo.executeRules(CheckMojo.java:174)
    at com.lewisd.maven.lint.plugin.CheckMojo.execute(CheckMojo.java:138)
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:132)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:208)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:153)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute(MojoExecutor.java:145)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:116)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject(LifecycleModuleBuilder.java:80)
    at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build(SingleThreadedBuilder.java:51)
    at org.apache.maven.lifecycle.internal.LifecycleStarter.execute(LifecycleStarter.java:120)
    at org.apache.maven.DefaultMaven.doExecute(DefaultMaven.java:355)
    at org.apache.maven.DefaultMaven.execute(DefaultMaven.java:155)
    at org.apache.maven.cli.MavenCli.execute(MavenCli.java:584)
    at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:216)
    at org.apache.maven.cli.MavenCli.main(MavenCli.java:160)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:497)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:289)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:229)
    at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:415)
    at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:356)

This happens both before and after the merge of #19. (And also when one uses 0.0.8, though in that case the stack trace is suppressed.) The DuplicateDep rule is not triggered and the build will not fail (!).

Note that the implicated IllegalStateException is precisely the one discussed in #19. It's good that we didn't decide to change it to an AssertionError, cause clearly this is a valid (albeit undesirable) state.

What's extra interesting about this is that this POM throws off even Maven: normally Maven itself will warn about the duplicate dependency declaration in the <dependencyManagement> section, but for this POM it doesn't. Replacing ${project.groupId} with com.lewisd will make both Maven and the plugin complain.

[lewisd32]

Wow, that's gnarly one.

While looking at the code to fix issue 15, did you see all the stuff around model vs originalModel? The originalModel is the representation of the pom as written. No variables have been replaced yet. The model has had properties replaced. I think it has transformations performed as well, but I'm not sure.

When I wrote all this originally, I had some concerns around the differences between the two, but I don't remember whether they turned out to be legitimate or not.

1. Does the model contain the same location information (for reporting useful errors) as the originalModel?
2. Does the model have versions from, for example, dependencyManagement dependencies added in where they were lacking in the originalModel? (I think it must for what I mention below to work.)

That's where the whole AbstractReduntantVersionRule.resolveObject thing came from. Rather than use the originalModel for everything, and have to codify in my plugin how to determine the version of a dependency (or plugin) when one isn't included (ie. get it from dependencyManagement in this pom, or the parent pom, etc) I wanted to let Maven do that, which is what I intended by resolveObject finding the 'same' thing in the model as the object that was gotten from the originalModel.

Because of the transformations that have been done in model, the plugin can't use it, since it's too far removed from what was actually in the pom. But, because the originalModel doesn't have those transformations done, the plugin can't use only it, or it would have to reimplement a lot of the transformations. So it's stuck with having to use both, and having some way of mapping to the 'same' object in the other model.

resolveObject attempts to do that by using artifactId, groupId, classifier, and type to map between them. When any one of those is a property though, that whole idea breaks down.

One thought I've had, but haven't investigated, is that there may be a way to use only the model. Most elements have a Map of InputLocations that tell you what pom file they were in, and where in the pom. It may be possible to use this to determine whether elements like versions, etc, were in the original model, or were the result of the transformations applied. If that works, I think it would eliminate the need for the originalModel in this plugin, and therefor eliminate the need for mapping objects between them.

[lewisd32]

I've realized recently that with only the model, redundant dependencies are already removed, so there's no way to find them without resorting to the originalModel. This means we do still need a way of mapping between objects from one model, in the other. Using InputLocations still may make this easier to do more robustly than we currently do.