Configure the plugin based on the Maven plugin configuration

[nrayburn-tech]

Fixes #107.

The way this works is that when you click the Maven sync action within IntelliJ, the Maven plugin configuration is read and synced with the IntelliJ plugin configuration.

This hasn't been tested on Windows. I don't have a Windows device that I can easily test on until after a release is made.

[jshiell]

Ooo, interesting. There's definitely demand for this, and the implementation seems nice & contained. It's definitely something I'd be open to adding as long as it's covered by test cases.

Thank you!

[nrayburn-tech]

I think this is roughly feature complete, with the exception of it not having any tests.

I wonder if this should wait until any Checkstyle version can be configured (ideally an outcome of #672). Otherwise, if the Maven project has a Checkstyle version specified that isn't supported then it will fallback to the IntelliJ plugin default.

An alternative to this would be if the Maven project has 10.1.2 configured and the known compatible version is 10.4.5, then we could set the configuration to use 10.4.5 directly.

---

Testing IntelliJ plugins is not something I've got much experience with, so it'll probably take a while for me to get suitable testing in place.

[nrayburn-tech]

Hopefully this goes without saying, but please let me know if there's any suggested changes that should be made.

For example, I know using var can be a preference so I don't mind changing it to explicit types. There might also be a better way for users to opt-in to the Maven import functionality. Maybe something to keep in mind is that this should be doable with Gradle as well, although I haven't explored that at all.

[jshiell]

Thanks - preference is definitely for modern Java, as long as it's good for 17 (min IDEA version).

Tests are a definite for inclusion, as I've spent 20 years regretting not writing this project with tests (not enough to retrospectively add them, mind). I'll try have a poke around later this week.

[jshiell]

Sorry, have been very AFK, but here's a first review. You've already marked most of the points of interest with TODOs in any case.

Big blocker to merging remains tests.

[nrayburn-tech]

Not an issue about the speed that this or the other PR gets reviewed. I’ll change them from draft when I think I’m done with it, any review before then is mostly just to help save me from wasting time if you notice something you disagree with early on.

I only get to work on this for a few hours a week at most anyways, so I’ll be just as slow on my side for making updates.

[nrayburn-tech]

I need to add additional tests, but I think this should be getting close.

[nrayburn-tech]

This should be ready for review now. It's likely more tests could be added, but I think what is there covers most common scenarios at this point.

Just FYI, I won't have much availability until the beginning of October so if you want to wait until then to merge this and release it so I'm around to support it if needed, then that's not an issue for me.

[nrayburn-tech]

Sorry about the failing build, I don’t actually recall if I ran the full build/test locally, or if I only ran tests against my changes.

Feel free to fix it and push changes or wait for me to get back from vacation.

[jshiell]

No worries, go enjoy your holiday! 😄

[nrayburn-tech]

Updated the tests, ./gradlew build is passing locally now.

[nrayburn-tech]

Build still passes locally. I don't expect I changed anything that would fix CI though, so I'll have to look at that some more.

[jshiell]

I'm afraid it fails locally for me (Mac OS 26.0.1). It doesn't seem to be obviously OS-specific though - it's failing on being unable to find the Checkstyle dependency:

final var checkstyleDependency = mavenDomProjectModel.getDependencies()
                .getDependencies().stream().filter(
                    dependency -> CHECKSTYLE_MAVEN_ID.equals(dependency.getGroupId().getValue(),
                        dependency.getArtifactId().getValue())).findFirst().orElseThrow();

blows up with

Caused by: java.util.NoSuchElementException: No value present

e.g. for updatePluginConfigLocationsFromMavenPlugin_configLocationIsMissingAndMavenConfigExists_removesMavenConfigLocation the current code fails:

createProjectPom(PROJECT_INFO + """
    <build>
        <plugins>
            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-checkstyle-plugin</artifactId>
                <version>3.6.0</version>
            </plugin>
        </plugins>
    </build>
    """.stripIndent());

However, if you are explicit about the dependency then it passes:

createProjectPom(PROJECT_INFO + """
    <build>
        <plugins>
            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-checkstyle-plugin</artifactId>
                <version>3.6.0</version>
                <dependencies>
                    <dependency>
                        <groupId>com.puppycrawl.tools</groupId>
                        <artifactId>checkstyle</artifactId>
                        <version>10.26.1</version>
                    </dependency>
                </dependencies>
            </plugin>
        </plugins>
    </build>
    """.stripIndent());

[nrayburn-tech]

The code seems to rely on some files already being downloaded and cached into the local Maven repository. Mine are all there from my testing so it works for me.

I'll get the code updated and add a test for that specific scenario.

[nrayburn-tech]

Updated the code with the new logic to download the file if it isn't available. Most of that logic is in getOrDownloadCheckstyleMavenPluginPom. If for some reason the file isn't available and downloading fails, then the maven sync just won't modify the currently configured plugin version.

[jshiell]

Apologies for the delay here - I'm in the middle of 3 weeks of work travel, so hoping to look at this once I'm back 🤞

[nrayburn-tech]

No rush. It will be here whenever you get to it.

Looks like the tests are still failing in CI anyways.