Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add comptibility for 2024.1 IDEs #3569

Merged
merged 15 commits into from
Jul 15, 2024
Merged

Add comptibility for 2024.1 IDEs #3569

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
05780d0
Update gradlewrapper to v7.6.4
ashleysommer Apr 7, 2024
060ae30
Remove use of `FileUtil.FILE_HASHING_STRATEGY` from Intellij FileUtil…
ashleysommer Apr 7, 2024
f032c1f
Add support for 2024.1 IDEs (and runs tests correctly against 2024.1)
ashleysommer Apr 7, 2024
21f7562
Update usages group wording for 2024.1
ashleysommer Apr 8, 2024
398243f
Fix more sdk configuration commits in application RW thread, fixes co…
ashleysommer Apr 8, 2024
6f55b2c
Merge branch 'main' into compat_2024_1
joshuataylor Jul 13, 2024
564e98f
Fix whitespace in tests due to 2024.1 change
joshuataylor Jul 13, 2024
d7481d0
pin versions
joshuataylor Jul 13, 2024
b9f59f4
use BasePlatformTestCase to stop warning
joshuataylor Jul 14, 2024
4649b09
Merge branch 'main' into compat_2024_1
joshuataylor Jul 14, 2024
b4369d3
add do block match test
joshuataylor Jul 15, 2024
7289afd
set 241.0 as the version, to fix certain intellij warnings
joshuataylor Jul 15, 2024
2f2ef1f
fix key
joshuataylor Jul 15, 2024
2c97d9e
revert tests
joshuataylor Jul 15, 2024
ea98112
use thread
joshuataylor Jul 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ jobs:

strategy:
matrix:
ideaVersion: [ "2023.3" ]
ideaVersion: [ "2024.1" ]

steps:
- uses: actions/checkout@v2
Expand Down
8 changes: 4 additions & 4 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
plugins {
id "org.jetbrains.intellij" version "1.13.3"
id "org.jetbrains.intellij" version "1.17.3"
id "org.jetbrains.kotlin.jvm" version "1.9.10"
id "de.undercouch.download" version "4.1.2"
}
Expand Down Expand Up @@ -69,8 +69,8 @@ allprojects {
changeNotes.set(bodyInnerHTML("resources/META-INF/changelog.html"))
pluginDescription.set(bodyInnerHTML("resources/META-INF/description.html"))

sinceBuild = "233.11799.241"
untilBuild = "233.*"
sinceBuild = "241.*"
untilBuild = "241.*"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on previous compatibility PRs sinceBuild typically matches untilBuild since we're dropping support for previous versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't seen any compatibility PRs in this repo where the sinceBuild matches the untilBuild, that doesn't seem right.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following warning shows for this:

> Task :verifyPluginConfiguration
[gradle-intellij-plugin :verifyPluginConfiguration] The following plugin configuration issues were found:
- The 'since-build' property is lower than the target IntelliJ Platform major version: 233.11799.241 < 241.

I believe it needs to match.

Since we also only support the last version of the IDE per version, I think this is fine?

}

publishPlugin {
Expand All @@ -82,7 +82,7 @@ allprojects {
}

runPluginVerifier {
ideVersions = ["2023.3"]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on previous compatibility PRs we normally drop support for the previous version, so this should probably just be ["2024.1"]

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old version is only dropped if a required code change makes it impossible to build for both versions. It's a coincidence that that has been the case for recent one.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, thanks for the clarification!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit I used for reference was this one: 876b770
That does simply add the new version to the list, as I've done here.

ideVersions = ["2024.1"]
}
}

Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# https://www.jetbrains.com/intellij-repository/releases
# https://www.jetbrains.com/intellij-repository/snapshots
baseVersion=17.0.1
ideaVersion=2023.3
ideaVersion=2024.1
# MUST stay at 1.8 for JPS (Build/Compile Project) compatibility even if JRE/JBR is newer
javaVersion=17
javaTargetVersion=17
Expand Down
Binary file modified gradle/wrapper/gradle-wrapper.jar
View file
Open in desktop
Binary file not shown.
3 changes: 2 additions & 1 deletion gradle/wrapper/gradle-wrapper.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-7.4.2-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-7.6.4-bin.zip
networkTimeout=10000
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
18 changes: 14 additions & 4 deletions gradlew
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
# Darwin, MinGW, and NonStop.
#
# (3) This script is generated from the Groovy template
# https://github.com/gradle/gradle/blob/master/subprojects/plugins/src/main/resources/org/gradle/api/internal/plugins/unixStartScript.txt
# https://github.com/gradle/gradle/blob/HEAD/subprojects/plugins/src/main/resources/org/gradle/api/internal/plugins/unixStartScript.txt
# within the Gradle project.
#
# You can find Gradle at https://github.com/gradle/gradle/.
Expand All @@ -80,10 +80,10 @@ do
esac
done

APP_HOME=$( cd "${APP_HOME:-./}" && pwd -P ) || exit

APP_NAME="Gradle"
# This is normally unused
# shellcheck disable=SC2034
APP_BASE_NAME=${0##*/}
APP_HOME=$( cd "${APP_HOME:-./}" && pwd -P ) || exit

# Add default JVM options here. You can also use JAVA_OPTS and GRADLE_OPTS to pass JVM options to this script.
DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"'
Expand Down Expand Up @@ -143,12 +143,16 @@ fi
if ! "$cygwin" && ! "$darwin" && ! "$nonstop" ; then
case $MAX_FD in #(
max*)
# In POSIX sh, ulimit -H is undefined. That's why the result is checked to see if it worked.
# shellcheck disable=SC3045
MAX_FD=$( ulimit -H -n ) ||
warn "Could not query maximum file descriptor limit"
esac
case $MAX_FD in #(
'' | soft) :;; #(
*)
# In POSIX sh, ulimit -n is undefined. That's why the result is checked to see if it worked.
# shellcheck disable=SC3045
ulimit -n "$MAX_FD" ||
warn "Could not set maximum file descriptor limit to $MAX_FD"
esac
Expand Down Expand Up @@ -205,6 +209,12 @@ set -- \
org.gradle.wrapper.GradleWrapperMain \
"$@"

# Stop when "xargs" is not available.
if ! command -v xargs >/dev/null 2>&1
then
die "xargs is not available"
fi

# Use "xargs" to parse quoted args.
#
# With -n1 it outputs one arg per line, with the quotes and backslashes removed.
Expand Down
15 changes: 9 additions & 6 deletions gradlew.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
@rem limitations under the License.
@rem

@if "%DEBUG%" == "" @echo off
@if "%DEBUG%"=="" @echo off
@rem ##########################################################################
@rem
@rem Gradle startup script for Windows
Expand All @@ -25,7 +25,8 @@
if "%OS%"=="Windows_NT" setlocal

set DIRNAME=%~dp0
if "%DIRNAME%" == "" set DIRNAME=.
if "%DIRNAME%"=="" set DIRNAME=.
@rem This is normally unused
set APP_BASE_NAME=%~n0
set APP_HOME=%DIRNAME%

Expand All @@ -40,7 +41,7 @@ if defined JAVA_HOME goto findJavaFromJavaHome

set JAVA_EXE=java.exe
%JAVA_EXE% -version >NUL 2>&1
if "%ERRORLEVEL%" == "0" goto execute
if %ERRORLEVEL% equ 0 goto execute

echo.
echo ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH.
Expand Down Expand Up @@ -75,13 +76,15 @@ set CLASSPATH=%APP_HOME%\gradle\wrapper\gradle-wrapper.jar

:end
@rem End local scope for the variables with windows NT shell
if "%ERRORLEVEL%"=="0" goto mainEnd
if %ERRORLEVEL% equ 0 goto mainEnd

:fail
rem Set variable GRADLE_EXIT_CONSOLE if you need the _script_ return code instead of
rem the _cmd.exe /c_ return code!
if not "" == "%GRADLE_EXIT_CONSOLE%" exit 1
exit /b 1
set EXIT_CODE=%ERRORLEVEL%
if %EXIT_CODE% equ 0 set EXIT_CODE=1
if not ""=="%GRADLE_EXIT_CONSOLE%" exit %EXIT_CODE%
exit /b %EXIT_CODE%

:mainEnd
if "%OS%"=="Windows_NT" endlocal
Expand Down
32 changes: 20 additions & 12 deletions jps-builder/tests/org/elixir_lang/jps/TestProjectBuilderLogger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,40 @@

import com.intellij.openapi.util.io.FileUtil;
import com.intellij.testFramework.UsefulTestCase;
import com.intellij.util.containers.MultiMap;
import gnu.trove.THashSet;
import com.intellij.util.containers.CollectionFactory;
import org.jetbrains.jps.builders.impl.logging.ProjectBuilderLoggerBase;

import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.HashMap;
import java.util.Set;



/**
* Created by zyuyou on 15/7/17.
*/
public class TestProjectBuilderLogger extends ProjectBuilderLoggerBase {
private MultiMap<String, File> myCompiledFiles = new MultiMap<String, File>();
private Set<File> myDeletedFiles = new THashSet<File>(FileUtil.FILE_HASHING_STRATEGY);
private HashMap<String, Set<String>> myCompiledFiles = new HashMap<String, Set<String>>();
private Set<String> myDeletedFiles = CollectionFactory.createFilePathSet();

@Override
public void logDeletedFiles(Collection<String> paths) {
for (String path:paths){
myDeletedFiles.add(new File(path));
myDeletedFiles.add(new File(path).getAbsolutePath());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a change in behavior? I'm not sure if path was previously absolute or relative (it looks relative).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change is the set is storing the string version of the file instead of the File object. In the previous version, the handler was calling .getAbsolutePath() on every File object in the set, so we simply do that here instead and save a step.

}
}

@Override
public void logCompiledFiles(Collection<File> files, String builderName, String description) throws IOException {
myCompiledFiles.putValues(builderName, files);
List<String> paths = new ArrayList<String>();
for (File file: files) {
paths.add(file.getPath());
}
myCompiledFiles.put(builderName, CollectionFactory.createFilePathSet(paths));
}

@Override
Expand All @@ -54,17 +60,19 @@ public void assertDeleted(File[] baseDirs, String... paths){
assertRelativePaths(baseDirs, myDeletedFiles, paths);
}

private static void assertRelativePaths(File[] baseDirs, Collection<File> files, String[] expected){
private static void assertRelativePaths(File[] baseDirs, Set<String> files, String[] expected){
List<String> relativePaths = new ArrayList<String>();
for (File file: files){
String path = file.getAbsolutePath();
for (String file: files) {
String path = new File(file).getAbsolutePath();
for(File baseDir:baseDirs){
if(baseDir != null && FileUtil.isAncestor(baseDir, file, false)){
path = FileUtil.getRelativePath(baseDir, file);
if(baseDir != null && FileUtil.isAncestor(baseDir.getPath(), file, false)){
path = FileUtil.getRelativePath(baseDir.getPath(), file, File.separatorChar);
break;
}
}
relativePaths.add(FileUtil.toSystemIndependentName(path));
if (path != null) {
relativePaths.add(FileUtil.toSystemIndependentName(path));
}
}
UsefulTestCase.assertSameElements(relativePaths, expected);

Expand Down
7 changes: 4 additions & 3 deletions resources/META-INF/plugin.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<vendor email="Kronic.Deth@gmail.com">Elle Imhoff</vendor>

<!-- please see http://confluence.jetbrains.com/display/IDEADEV/Build+Number+Ranges for description -->
<idea-version since-build="201.6668.113"/>
<idea-version since-build="241.0"/>

<!-- please see http://confluence.jetbrains.com/display/IDEADEV/Plugin+Compatibility+with+IntelliJ+Platform+Products
on how to target different products -->
Expand Down Expand Up @@ -213,8 +213,9 @@
<lang.parserDefinition language="Elixir" implementationClass="org.elixir_lang.ElixirParserDefinition"/>
<lang.psiStructureViewFactory language="Elixir" implementationClass="org.elixir_lang.structure_view.Factory"/>
<lang.quoteHandler language="Elixir" implementationClass="org.elixir_lang.QuoteHandler"/>
<lang.syntaxHighlighterFactory key="Elixir"
implementationClass="org.elixir_lang.ElixirSyntaxHighlighterFactory"/>
<lang.syntaxHighlighterFactory
implementationClass="org.elixir_lang.ElixirSyntaxHighlighterFactory"
language="Elixir"/>
<localInspection displayName="Ambiguous nested calls" enabledByDefault="true" groupName="Elixir"
implementationClass="org.elixir_lang.inspection.NoParenthesesManyStrict" language="Elixir"
level="ERROR" shortName="NoParenthesesManyStrict"/>
Expand Down
38 changes: 22 additions & 16 deletions src/org/elixir_lang/sdk/elixir/Type.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ import com.intellij.openapi.roots.ModuleRootManager
import com.intellij.openapi.roots.OrderRootType
import com.intellij.openapi.roots.ProjectFileIndex
import com.intellij.openapi.roots.ProjectRootManager
import com.intellij.openapi.util.*
import com.intellij.openapi.util.InvalidDataException
import com.intellij.openapi.util.SystemInfo
import com.intellij.openapi.util.Version
import com.intellij.openapi.util.WriteExternalException
import com.intellij.openapi.vfs.VfsUtil
import com.intellij.openapi.vfs.VirtualFile
import com.intellij.openapi.vfs.VirtualFileManager
Expand Down Expand Up @@ -44,6 +47,7 @@ import java.nio.file.Path
import java.nio.file.Paths
import java.util.*
import javax.swing.Icon
import java.util.concurrent.Callable

class Type : org.elixir_lang.sdk.erlang_dependent.Type(SerializerExtension.ELIXIR_SDK_TYPE_ID) {
/**
Expand Down Expand Up @@ -303,7 +307,7 @@ ELIXIR_SDK_HOME
addDocumentationPaths(sdkModificator)
addSourcePaths(sdkModificator)
configureInternalErlangSdk(sdk, sdkModificator)
sdkModificator.commitChanges()
ApplicationManager.getApplication().runWriteAction { sdkModificator.commitChanges() }
}

private fun configureInternalErlangSdk(elixirSdk: Sdk, elixirSdkModificator: SdkModificator): Sdk? {
Expand Down Expand Up @@ -415,7 +419,7 @@ ELIXIR_SDK_HOME
sdkModificator.homePath = sdkHome
sdkModificator.versionString =
getVersionString(release) // must be set after home path, otherwise setting home path clears the version string
sdkModificator.commitChanges()
ApplicationManager.getApplication().runWriteAction { sdkModificator.commitChanges() }
configureSdkPaths(sdk)
return sdk
}
Expand Down Expand Up @@ -527,21 +531,23 @@ ELIXIR_SDK_HOME
return if (!project.isDisposed) {
/* ModuleUtilCore.findModuleForPsiElement can fail with NullPointerException if the
ProjectFileIndex.SERVICE.getInstance(Project) returns {@code null}, so check that the
ProjectFileIndex is available first */
ProjectFileIndex is available first
*/
if (ProjectFileIndex.SERVICE.getInstance(project) != null) {
val module = try {
ReadAction.compute<Module, Throwable> {
ModuleUtilCore.findModuleForPsiElement(psiElement)
ApplicationManager.getApplication().executeOnPooledThread(Callable {
ReadAction.compute<Sdk?, Throwable> {
try {
val module = ModuleUtilCore.findModuleForPsiElement(psiElement)
if (module != null) {
mostSpecificSdk(module)
} else {
mostSpecificSdk(project)
}
} catch (_: AlreadyDisposedException) {
null
}
}
} catch (_: AlreadyDisposedException) {
null
}

if (module != null) {
mostSpecificSdk(module)
} else {
mostSpecificSdk(project)
}
}).get()
} else {
mostSpecificSdk(project)
}
Expand Down
4 changes: 2 additions & 2 deletions src/org/elixir_lang/sdk/erlang/Type.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.intellij.execution.ExecutionException;
import com.intellij.execution.process.ProcessOutput;
import com.intellij.openapi.application.ApplicationManager;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.projectRoots.Sdk;
import com.intellij.openapi.projectRoots.SdkModel;
Expand Down Expand Up @@ -155,8 +156,7 @@ public boolean isRootTypeApplicable(@NotNull OrderRootType type) {
public void setupSdkPaths(@NotNull Sdk sdk) {
SdkModificator sdkModificator = sdk.getSdkModificator();
addCodePaths(sdkModificator);

sdkModificator.commitChanges();
ApplicationManager.getApplication().runWriteAction(sdkModificator::commitChanges);
}

/**
Expand Down
12 changes: 9 additions & 3 deletions tests/org/elixir_lang/FindUsagesTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.elixir_lang

import com.intellij.openapi.application.ex.ApplicationInfoEx
import com.intellij.openapi.util.Version
import com.intellij.psi.PsiPolyVariantReference
import com.intellij.usages.UsageInfo2UsageAdapter
import org.elixir_lang.find_usages.handler.AlreadyResolved
Expand Down Expand Up @@ -650,10 +651,15 @@ class FindUsagesTest : PlatformTestCase() {
)
}

private fun usages(): String =
if (ApplicationInfoEx.getInstance().fullVersion == "2021.1.3") {
private fun usages(): String {
val ideVersion = Version.parseVersion(ApplicationInfoEx.getInstance().fullVersion)
val usagesString = if (ideVersion === null || ideVersion.lessThan(2021,2,0)) {
"Found usages"
} else {
} else if (ideVersion.lessThan(2024,1,0)) {
"Usages in"
} else {
"Usages"
}
return usagesString
}
Comment on lines +654 to +664
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand what's driving this change. Can you walk me through it please?

Copy link
Contributor Author

@ashleysommer ashleysommer Apr 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to help some tests pass.

In February, IntelliJ changed the wording of the text representation of the find usages response. That change made its way into all 2024.1 IDEs and broke these tests.
JetBrains/intellij-community@0706bea

Note, there are still some other changes in 2024.1 that are preventing these tests from actually passing, but this is the simple first step to remedy the bulk of the error messages.

Copy link

@noizu noizu Apr 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to support previous build versions if we are bocking these versions with the from-version attribute?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, we really don't need the case to handle the pre-2021.2 usages string.
It had been in the plugin code right up to and including the 2023.3 release, thats why I left it in there in this change, but refactoring it like this could be a good opportunity to remove that old version compatibility.

}
Loading
Loading