search

OpenLiberty / liberty-tools-intellij

IntelliJ IDEA extension for Liberty
https://plugins.jetbrains.com/plugin/14856-open-liberty-tools
Eclipse Public License 2.0
11 stars 20 forks source link

Stop Using Deprecated Maven Methods which are Marked for Removal #823

Closed vaisakhkannan closed 6 days ago

vaisakhkannan commented 2 weeks ago

The Liberty-Tools-Intellij build step uses two deprecated maven methods that are marked for removal. We need to fix this issue so that we won’t receive warnings in the future.

The error in the logs is

> Task :compileKotlin NO-SOURCE
/home/runner/work/liberty-tools-intellij/liberty-tools-intellij/liberty-tools-intellij/src/main/java/io/openliberty/tools/intellij/util/LibertyMavenUtil.java:207: warning: [removal] getMavenHome() in MavenGeneralSettings has been deprecated and marked for removal
        String mavenHome = mavenSettings.getMavenHome();
                                        ^
/home/runner/work/liberty-tools-intellij/liberty-tools-intellij/liberty-tools-intellij/src/main/java/io/openliberty/tools/intellij/util/LibertyMavenUtil.java:250: warning: [removal] resolveMavenHomeDirectory(String) in MavenUtil has been deprecated and marked for removal

> Task :compileJava
        File mavenHomeFile = MavenUtil.resolveMavenHomeDirectory(customMavenHome); // when customMavenHome path is invalid it returns null
                                      ^
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
2 warnings
vaisakhkannan commented 1 week ago

Scenarios that I took for the testing of the newly suggested methods getMavenHomeType() and getMavenHomeFile()

1) I double-checked the commands using each Dev Mode Start Action and Dev Mode Start in Container Action, testing with different Maven settings (using Maven Wrapper, Bundled Maven, Custom Maven Home).

2 When I compared the implementation of the old deprecated method (resolveMavenHomeDirectory), I noticed it uses a staticOrBundled method. What this method actually does is..

  1. It attempts to cast the current object (this) to StaticResolvedMavenHomeType.
  2. If the cast is successful, it returns the casted object.
  3. If the cast fails (i.e., this is not an instance of StaticResolvedMavenHomeType), it returns 'BundledMaven3'.
341367151-71c2a7fc-aff1-4801-9848-115ba222ab9b-2

Using the same approach, I modified the code to return the default 'BundledMaven3.INSTANCE'

If we use null as the default return value, we will get an IllegalArgumentException if the casting fails in the future. I encountered this issue during testing because I commented out the relevant code below.

https://github.com/OpenLiberty/liberty-tools-intellij/blob/54d1e896f432156a058642183e48b222a86b44c9/src/main/java/io/openliberty/tools/intellij/util/LibertyMavenUtil.java#L208C9-L211C10

(what this meant is, I commented out maven Wrapper, so there is no code checking maven wrapper and maven Wrapper is not an instance of StaticResolvedMavenHomeType , so we should return the default 'BundledMaven3.INSTANCE', that's a similar behaviour from the old deprecated method)