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.

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

MP-5395 in between interface #976

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

bric3
Copy link
Contributor

@bric3 bric3 commented Jul 24, 2023

@LChernigovskaya @udalov Here's the followup to #885

In particular for this #885 (comment)

Reference : MP-5395

Now when an interface in a plugin extends an interface of the platform that has a default method using an internal type. And that plugin interface is compiled, that interface will be generated with a DefaultImpls that will "forward" the call to the DefaultImpls of this parent interface.

classDiagram
    PlatformInterface <|-- PluginInterface

    class PlatformInterface{
      void defaultUsingInternalType(InternalType)
    }
    class PlatformInterfaceDefaultImpls{
      void defaultUsingInternalType(InternalType)
    }
    class PluginInterface {
    }
    class PluginInterfaceDefaultImpls{
      static void defaultUsingInternalType(InternalType)
    }
Loading

Ignore the missing $ in the schema above as mermaid don't like them in the class names.

An interesting bit is the PluginInterface$DefaultImpls generated by Kotlin is an inner class of both

  • PlatformInterface
  • PluginInterface

In this case the method forwarder is different as it is a static method in this case.


Also the previous implementation only handled parameters that were references, this limitation is now lifted.

bric3 added 2 commits July 24, 2023 13:58
Suppose an interface in the plugin that extends an an IDE interface that
has a default method using internal type. This interface, even if not
implemented will have a generated `DefaultImpls`, that uses this
internal type.

With this change the plugin verifier will ignore such cases.
The previous implementation incorrectly assumed all arguments were
references. This change handles arguments that are primitives.

Also reduced allocation by initializing the opcode arrays once.
@bric3
Copy link
Contributor Author

bric3 commented Jul 24, 2023

Test are failing locally also for a reason that escapes me at this time.

https://github.com/JetBrains/intellij-plugin-verifier/actions/runs/5644840413/job/15289536907#step:7:199

com.jetbrains.pluginverifier.tests.VerificationTest > check that all problems are found FAILED
    java.lang.AssertionError at VerificationTest.kt:156

But locally I have the same test failing with this output.

/*expected(PROBLEM)
Package 'kotlin' is not found

Package 'kotlin' is not found along with its 2 classes.
Probably the package 'kotlin' belongs to a library or dependency that is not resolved by the checker.
It is also possible, however, that this package was actually removed from a dependency causing the detected problems. Access to unresolved classes at runtime may lead to **NoSuchClassError**.
The following classes of 'kotlin' are not resolved:
  Class kotlin.jvm.internal.Intrinsics is referenced in
    mock.plugin.internal.defaultMethod.NoInternalTypeUsageInterface.DefaultImpls.internalArgsAndPrimitiveArgs-i_VnzYk(NoInternalTypeUsageInterface $this, AnInternalType anInternalType, int i, boolean b, int ui, String[] objectArray, Integer[] intArray) : void
    mock.plugin.internal.defaultMethod.NoInternalTypeUsageInterface.DefaultImpls.internalArgsReturningInternal(NoInternalTypeUsageInterface $this, AnInternalType anInternalType, String s) : AnInternalType
    mock.plugin.internal.defaultMethod.NoInternalTypeUsageInterface.DefaultImpls.internalArgsReturningVoid(NoInternalTypeUsageInterface $this, AnInternalType anInternalType, String s) : void
  Class kotlin.jvm.internal.markers.KMappedMarker is referenced in
    mock.plugin.internal.defaultMethod.NoInternalTypeUsageInterface

*/

@bric3
Copy link
Contributor Author

bric3 commented Jul 24, 2023

@udalov I think I understand why there's verification failure about missing kotlin package.

The mock-plugin project don't reference any Kotlin as a dependency.

dependencies {
//compile against "before-idea", "additional-before-idea" and verify against "after-idea", "additional-after-idea"
compileOnly(project(":verifier-test:before-idea"))
runtimeOnly(project(":verifier-test:after-idea"))
compileOnly(project(":verifier-test:additional-before-idea"))
runtimeOnly(project(":verifier-test:additional-after-idea"))
}

But since Kotlin is part of the platform should this check be skipped (as I tried here 85f2ce7) ?

Or should I add the kotlin std lib as a dependency ? Actually this doesn't work because the setup is only configured to use the jar, and not the dependencies.
image

val prepareMockPlugin by tasks.registering(Copy::class) {
dependsOn(":verifier-test:mock-plugin:build")
into("$buildDir/mocks")
val mockPluginBuildDir = project("mock-plugin").buildDir
from(File(mockPluginBuildDir, "libs/mock-plugin-1.0.jar"))
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants